feat(jwt): implement JWT flow #9
Conversation
|
How can we test the agent itself? |
|
As of this PR it can be a little convoluted - the way I was doing it was #12 includes a I will update this PR soon to pull that |
|
This can now be manually tested just using cryostat's |
|
This PR/issue depends on:
|
|
Running on cryostat If I run smoketest while the agent is running, I assume due to timing, the agent actually does register itself once but after trying to update, it gets timed out again and also doesn't deregister itself. |
|
Huh. I can't reproduce that on my end. How did you build the agent? It's weird that the registration succeeds in your second case, but then updates and even deregistration fail afterward. I'm wondering if something else is going on on the Cryostat server side that is causing it to stop responding to requests, or not respond in a timely manner. Could be a re-appearance of https://github.com/cryostatio/cryostat/issues/929 or something similar. |
Hmm that's weird. Yeah my exact steps are in 1 terminal # -agent
$ mvn clean package
$ sh run.shother # in cryostat -main
$ mvn clean package && sh smoketest.shInterestingly it seems like my cryostat seems to hang on a discovery endpoint POST for 30 seconds sometimes. I'm not sure why, because I've packaged cryostat... Sep 20, 2022 6:08:03 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:44292): POST /api/v2.2/discovery 200 1000ms
Sep 20, 2022 6:08:09 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:48000): POST /api/v2.2/discovery 200 1001ms
Sep 20, 2022 6:08:15 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:48014): POST /api/v2.2/discovery 200 1001ms
Sep 20, 2022 6:08:21 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:57866): POST /api/v2.2/discovery 200 1001ms
Sep 20, 2022 6:08:26 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:42912): POST /api/v2.2/discovery 200 30030ms
Sep 20, 2022 6:08:27 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:57870): POST /api/v2.2/discovery 200 1001ms
Sep 20, 2022 6:08:33 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:36610): POST /api/v2.2/discovery 200 1001ms
Sep 20, 2022 6:08:39 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:53428): POST /api/v2.2/discovery 200 1001ms
Sep 20, 2022 6:08:45 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:53430): POST /api/v2.2/discovery 200 1001ms
Sep 20, 2022 6:08:46 PM io.cryostat.core.log.Logger info
INFO: (10.0.2.100:57864): POST /api/v2.2/discovery 200 30028ms |
|
Interesting. If that request is hanging then it shouldn't be blocking other things, so that does sound like https://github.com/cryostatio/cryostat/issues/929 again. Can you try Timeouts occurring during registration seem like it's probably related to the plugin callback. Can you try changing the callback in |
I don't believe the problem is with DNS or anything like that as I've tried changing host values. I think the problem is that when When I tried doing requests during this time like Then after a few minutes, the requests finally timeout and so I get a bunch of responses at the same time: This was happening in other places, so I think I "solved" this in the the archives PR #1047 by just adding a timeout to the future which gets the jvmId. |
|
Right, this behaviour happened exactly the same to me too :D |
|
Also interestingly though when I test on the branch I've been working on, the plugin will register and deregister every time.
and a jvmId cannot be computed on the FOUND agent plugin jvm every time as well. Not sure what's going on there but I will look at it after? What do you think should be the way to go about this? |
|
The -agent A proper setup would be like https://github.com/cryostatio/cryostat/pull/1048 , where the agent is attached to a JVM that runs within the pod. This way the Cryostat container and the agent container can actually talk to each other, the callback is "real", and JMX connections can be opened to do things like compute JVM IDs or whatever. |
|
I'll see if I can prepare a branch of github.com/andrewazores/vertx-fib-demo that includes the -agent so that we can use that for testing the agent in a more realistic way. It's still good that we can catch these kinds of timeout problems or failures that can occur from well-formed but invalid data input. More good bugs to tackle. It probably shouldn't take so long for a JMX connection attempt to fail, for example. |
|
Getting back on track with this - https://github.com/andrewazores/quarkus-test has been set up for a little while to build with the
It's still a little convoluted and takes a few manual steps, but it's fairly doable. It could probably be smoothed out by adding the |
|
Somewhat more concrete steps:
|
There was a problem hiding this comment.
Code looks good, and testing works well as well.
There is a bug now however with metadata labels. If you have an Active Recording with labels in an agent equipped discovered target, and the target is pinged to re-register, the associated active metadata files are deleted.
I investigated a bit and I think it's because when the DiscoveryPostHandler calls DiscoveryStorage#update, it calls JvmdIdHelper#resolveId on all target children of the plugin which always puts into the ids LoadingCache. I think when this happens, the RecordingMetadataManager is hooked in by the IdEvent.INVALIDATED event which is emitted everytime an id is "removed" from the cache (however this is a bug, because the jvmId will actually be the same, so it is replacing itself in the cache with itself), and event is emitted, the metadata manager calls #removeLostTargetMetadata which deletes all ActiveRecording metadata associated with the "lost" target.
I believe a simple if check inside the
future.thenAccept(
id -> {
if (this.ids.synchronous().get(sr.getServiceUri().toString()).equals(id)) {
return;
}
this.ids.synchronous().put(sr.getServiceUri().toString(), id);
});
would probably fix this.
@maxcao13 your explanation makes sense. Could you file a backend issue for it? |
Fixes #6
Fixes https://github.com/cryostatio/cryostat/issues/1051
Depends on https://github.com/cryostatio/cryostat/pull/1059
Depends on https://github.com/cryostatio/cryostat/issues/1061