Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci(ITs): support JBoss ITs with GitHub secrets #3469

Merged
merged 16 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .ci/scripts/jboss-docker-images.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
registry.redhat.io/jboss-eap-7/eap70-openshift:1.7
registry.access.redhat.com/jboss-eap-7/eap71-openshift
registry.access.redhat.com/jboss-eap-7/eap72-openshift
registry.redhat.io/jboss-eap-7/eap73-openjdk11-openshift-rhel8:7.3.10
registry.redhat.io/jboss-eap-7/eap74-openjdk11-openshift-rhel8:7.4.0
registry.redhat.io/jboss-eap-7/eap74-openjdk17-openshift-rhel8:7.4.14
22 changes: 22 additions & 0 deletions .ci/scripts/jboss-pull.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/usr/bin/env bash

# Bash strict mode
set -eo pipefail

## This script is used by the CI to be able to re-tag the docker images
## to use the official docker namespace.
## Or by an Elastic employee to configure the docker images as needed.
while read -r i ; do
[[ -z $i ]] && continue
name="${i##*/}"
echo "::group::$name"
docker pull docker.elastic.co/observability-ci/$name --platform linux/amd64
docker tag docker.elastic.co/observability-ci/$name $i
echo "::endgroup::"
done < .ci/scripts/jboss-docker-images.txt

if [ "$CI" == "true" ] ;then
echo "::group::docker-images"
docker images
echo "::endgroup::"
fi
15 changes: 15 additions & 0 deletions .ci/scripts/jboss-upload.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/usr/bin/env bash

# Bash strict mode
set -eo pipefail

## This script is called manually when a new Docker image is added.
while read -r i ; do
[[ -z $i ]] && continue
name="${i##*/}"
echo "::group::$name"
docker pull --platform linux/amd64 $i
docker tag $i docker.elastic.co/observability-ci/$name
docker push docker.elastic.co/observability-ci/$name
echo "::endgroup::"
done < .ci/scripts/jboss-docker-images.txt
32 changes: 32 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -320,3 +320,35 @@ jobs:
path: |
**/junit-*.xml
**/TEST-*.xml

jboss:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] we could make that step a sub-step of the application server tests,but as it's quite fast (about 6min) in CI that's fine for now and allows us to keep an eye on it for now in case of failure.

name: JBoss integration tests
runs-on: ubuntu-latest
needs: build
if: github.event_name != 'pull_request' || github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should only happen when a non-forked pull request.

steps:
- uses: actions/checkout@v4
- uses: elastic/apm-pipeline-library/.github/actions/docker-login@current
with:
registry: docker.elastic.co
secret: secret/observability-team/ci/docker-registry/prod
url: ${{ secrets.VAULT_ADDR }}
roleId: ${{ secrets.VAULT_ROLE_ID }}
secretId: ${{ secrets.VAULT_SECRET_ID }}
- uses: ./.github/workflows/unstash
with:
name: build
path: ${{ github.workspace }}
- name: Pull JBoss docker images
run: .ci/scripts/jboss-pull.sh
- uses: ./.github/workflows/maven-goal
with:
command: ./mvnw -q --file ./integration-tests/application-server-integration-tests/pom.xml -P ci-jboss-integration-tests verify
- name: Store test results
if: success() || failure()
uses: actions/upload-artifact@v3
with:
name: test-results
path: |
**/junit-*.xml
**/TEST-*.xml
Original file line number Diff line number Diff line change
Expand Up @@ -270,16 +270,16 @@ public void start() {
log.info("starting container with {} = {}", jvmEnvironmentVariable, value);
}

// log app server output for easier debugging
withLogConsumer(new Slf4jLogConsumer(log));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was required because otherwise the container output wasn't captured


try {
super.start();
} catch (RuntimeException e) {
log.error("unable to start container, set breakpoint where this log is generated to debug", e);
}

// send container logs to logger for easier debug by default
followOutput(new Slf4jLogConsumer(log));

//.withLogConsumer(TestContainersUtils.createSlf4jLogConsumer(MockServerContainer.class))
}

private static String javaAgentArg(AgentFileAccessor.Variant javaAgentArgumentVariant) {
Expand Down
15 changes: 15 additions & 0 deletions integration-tests/application-server-integration-tests/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -188,22 +188,37 @@
</dependencies>

<profiles>
<!-- application servers only jboss -->
<profile>
<id>ci-jboss-integration-tests</id>
<activation>
<activeByDefault>false</activeByDefault>
</activation>
<properties>
<skip.integration.test>false</skip.integration.test>
<failsafe.includes>**/JBossIT.java</failsafe.includes>
</properties>
</profile>
<!-- application servers but jboss -->
<profile>
<id>ci-application-server-integration-tests</id>
<activation>
<activeByDefault>false</activeByDefault>
</activation>
<properties>
<skip.integration.test>false</skip.integration.test>
<failsafe.excludes>**/JBossIT.java</failsafe.excludes>
</properties>
</profile>
<!-- non application servers -->
<profile>
<id>ci-non-application-server-integration-tests</id>
<activation>
<activeByDefault>false</activeByDefault>
</activation>
<properties>
<skip.integration.test>true</skip.integration.test>
<failsafe.excludes>**/JBossIT.java</failsafe.excludes>
</properties>
</profile>
</profiles>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,41 +19,56 @@
package co.elastic.apm.servlet;

import co.elastic.apm.agent.test.AgentTestContainer;
import co.elastic.apm.servlet.tests.CdiApplicationServerTestApp;
import co.elastic.apm.servlet.tests.JBossServletApiTestApp;
import co.elastic.apm.servlet.tests.JavaxExternalPluginTestApp;
import co.elastic.apm.servlet.tests.JsfApplicationServerTestApp;
import co.elastic.apm.servlet.tests.SoapTestApp;
import co.elastic.apm.servlet.tests.TestApp;
import co.elastic.apm.servlet.tests.*;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.testcontainers.containers.wait.strategy.HttpWaitStrategy;

import java.util.Arrays;

@RunWith(Parameterized.class)
public class JBossIT extends AbstractServletContainerIntegrationTest {

public JBossIT(final String jbossVersion) {
super(AgentTestContainer.appServer("registry.access.redhat.com/" + jbossVersion)
.withContainerName("jboss")
.withHttpPort(8080)
// set JVM arguments through JAVA_OPTS
.withJvmArgumentsVariable("JAVA_OPTS")
// this overrides the defaults, so we have to manually re-add preferIPv4Stack
.withSystemProperty("java.net.preferIPv4Stack", "true")
// the other defaults don't seem to be important
.withSystemProperty("jboss.modules.system.pkgs", "org.jboss.logmanager,jdk.nashorn.api,com.sun.crypto.provider")
.withDeploymentPath("/opt/eap/standalone/deployments"),
public JBossIT(final String jbossImage, boolean preserveDefaults) {
super(jbossContainer(jbossImage, preserveDefaults),
"jboss-application");
}

private static AgentTestContainer.AppServer jbossContainer(String image, boolean preserveDefaults) {
AgentTestContainer.AppServer jboss = AgentTestContainer.appServer(image)
.withContainerName("jboss")
.withHttpPort(8080)
// set JVM arguments through JAVA_OPTS
.withJvmArgumentsVariable("JAVA_OPTS")

.withDeploymentPath("/opt/eap/standalone/deployments")
.waitingFor(new HttpWaitStrategy().forPort(8080).forStatusCode(200));

if (preserveDefaults) {
// for older versions setting JAVA_OPTS means overwriting the defaults
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is only needed for some of the older jboss versions, the latest ones are able to deal with customized JAVA_OPTS variable.

// so we have to manually re-add some of them like 'preferIPv4Stack' and 'jboss.modules.system.pkgs'
// other defaults do not seem to impact test thus we ignore them
jboss.withSystemProperty("java.net.preferIPv4Stack", "true")
// the other defaults don't seem to be important
.withSystemProperty("jboss.modules.system.pkgs", "org.jboss.logmanager,jdk.nashorn.api,com.sun.crypto.provider");
}

return jboss;
}

@Parameterized.Parameters(name = "JBoss {0}")
public static Iterable<Object[]> data() {
// When running in GitHub actions if a new docker image is added, please
// update the list of these docker images in .ci/scripts/jboss-docker-images.txt
// then you can run .ci/scripts/jboss-upload.sh to upload these new docker images
// to the internal docker registry.
return Arrays.asList(new Object[][]{
{"jboss-eap-6/eap64-openshift"},
// {"jboss-eap-7/eap70-openshift"}, // disabled as not available anymore in public registry
{"jboss-eap-7/eap71-openshift"},
{"jboss-eap-7/eap72-openshift"}
{"registry.redhat.io/jboss-eap-7/eap70-openshift:1.7", true},
{"registry.access.redhat.com/jboss-eap-7/eap71-openshift", true},
{"registry.access.redhat.com/jboss-eap-7/eap72-openshift", true},
{"registry.redhat.io/jboss-eap-7/eap73-openjdk11-openshift-rhel8:7.3.10", true},
{"registry.redhat.io/jboss-eap-7/eap74-openjdk11-openshift-rhel8:7.4.0", false},
{"registry.redhat.io/jboss-eap-7/eap74-openjdk17-openshift-rhel8:7.4.14", false},
});
}

Expand Down
Loading