test(itest): replace JDP with agent-based discovery#1560
Draft
andrewazores wants to merge 40 commits intocryostatio:mainfrom
Draft
test(itest): replace JDP with agent-based discovery#1560andrewazores wants to merge 40 commits intocryostatio:mainfrom
andrewazores wants to merge 40 commits intocryostatio:mainfrom
Conversation
github-actions
bot
added
the
needs-triage
andrewazores
added
test
and removed
needs-triage
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
andrewazores
force-pushed
the
itest-agent-discovery
branch
2 times, most recently
from
ae247bc to
b325b70
Compare
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
andrewazores
force-pushed
the
itest-agent-discovery
branch
from
1e7c631 to
bae3d0d
Compare
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
Contributor
|
Test image available: |
andrewazores
force-pushed
the
itest-agent-discovery
branch
from
fd61fe3 to
eb392d4
Compare
Contributor
|
Test image available: |
Contributor
|
Test image available: |
andrewazores
force-pushed
the
itest-agent-discovery
branch
from
f2f7aca to
11d0cdd
Compare
Contributor
|
Test image available: |
Contributor
|
Test image available: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Welcome to Cryostat! 👋
Before contributing, make sure you have:
mainbranch[chore, ci, docs, feat, fix, test]git commit --amend --signoffRelated to #1553 - eliminates JDP from itest harness, so ARM64 images can be tested. Also just greatly reduces time spent waiting for JDP discovery/loss in integration testing, speeding up the run substantially.
As much as possible, I have tried to keep the existing tests' structure the same, and just swap out the underlying discovery implementation from relying on JDP to relying on Agent-based discovery. This was pretty straightforward since all of the "external targets" integration tests already used my
vertx-fib-democontainer, so I just needed to bump the version of that to a newer one that is built with the Cryostat Agent, and add a bit of logic to add environment variables to configure those agent instances. Other than that, a few tests also relied upon Cryostat being able to discover itself, which it previously did with JDP but would not anymore with agent-based discovery. I worked around that by having Cryostat define a Custom Target for itself before each test class (and remove it after each), in a way that is broadly compatible with the existing tests' expectations for what they would have seen from Cryostat discovering itself over JDP. Some tests needed some adjusting/loosening of particular expectations around target labels or annotations to compensate as well.For comparison, here is a recent CI test run which took just under 9 minutes to complete: https://github.com/cryostatio/cryostat/actions/runs/5283519061/jobs/9560039201
and here is the CI test run on this PR's changes: https://github.com/cryostatio/cryostat/actions/runs/5316093165/jobs/9625347531
The time savings are due to how the integration test harness has to wait for Cryostat to "lose" JDP targets in between many of the test classes. Since JDP is basically just a multicast packet heartbeat, target loss occurs when Cryostat's JDP client stops receiving heartbeats from a previously-seen JVM for a certain duration of time. So, after a test class completes, the test invokes Podman to stop the targets, and then waits for Cryostat to notice that they have all gone. With Agent discovery, invoking Podman to stop the containers sends a shutdown signal to the target JVMs, which the Agent intercepts, handles, and forwards. The Agent's signal handling then sends an immediate discovery plugin deregistration to Cryostat, which removes the Agent's target definition, so the time to wait for Cryostat's discovery to "settle" at the conclusion of a test class is very short, and the next test class can begin immediately.