-
-
Notifications
You must be signed in to change notification settings - Fork 314
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
docker: switch to a bespoke test container #5683
base: master
Are you sure you want to change the base?
Conversation
Removing ADDITIONAL_LABEL values from Grinder run and filling in CLOUD_PROVIDER=azure, since that is how the tests are expected to be launched in general pipeline code: Run some additional grinders, varying test group and JDK version: sanity.system, JDK21:
special.functional, JDK11:
|
@ShelleyLambert looking at that job it didn't run in a docker container? I think it's because we need to add the |
Rebuilding https://ci.adoptium.net/view/Test_grinder/job/Grinder/11171/ (PARALLEL=Dynamic, NUM_MACHINES=5) |
Looking more closely, it looks like it found a static machine, but still ran on a dynamic machine (test-linux-x64-b97844), console output from https://ci.adoptium.net/view/Test_grinder/job/Grinder_testList_3/10/console
In 'real' pipeline runs, the test pipeline code will first try to send to idle static machines and spin up dynamic ones as needed after that. As per the current design, we will not be passing in ci.agent.dynamic explicitly when we trigger test pipelines (only updating to set CLOUD_PROVIDER=azure), so I wanted to check that it works as designed. |
Right okay, so if you don't expect the dynamic label to be used I'll have to tweak the existing code slightly, I'll have a playb |
@smlambert I've updated the code so it won't explicitly require someone to pass the dynamic label to the job anymore. As long as the cloud is set as Azure it will default to using a container image |
@gdams - you do not need to make the change on L461. I was explaining how the current logic already works, not asking for you to change your PR, which looks fine as it is, I am just running many additional tests to verify that each test group works on these agents (please see what we do on L351). |
reverted PTAL |
Couple extra Grinder runs to verify: |
https://ci.adoptium.net/view/Test_grinder/job/Grinder/11321/ fails with:
This failure is unrelated to this PR (related to: #5754) |
I will launch a couple of Grinders to 'exhaust' our list of static nodes, and see the use of dynamic agents shortly. |
docker.image('adoptopenjdk/centos7_build_image').pull() | ||
docker.image('adoptopenjdk/centos7_build_image').inside { | ||
// Set dockerimage for azure agent. Fyre has stencil to setup the right environment | ||
docker.image('ghcr.io/adoptium/test-containers:ubuntu2204').pull() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we extract that URL or the ubuntu version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referencing a set of containers from https://github.com/orgs/adoptium/packages/container/package/test-containers is good from my point of view (they have the test prereqs), and we can build out several to give us a variety of coverage.
Also moved
LIB_DIR
andSYSTEM_LIB_DIR
to be inside the current workspace for docker builds due to permissions errorsCurrently using
ghcr.io/adoptium/test-containers:ubuntu2204
which is a lightweight image built loosely off the dockerStatic base images. PR incoming to the infra repo to regularly build this and we can add more base images where appropriate.Grinder to show this working: https://ci.adoptium.net/view/Test_grinder/job/Grinder/11160/