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

fix(registration): handle 401 on update with reregistration #16

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

andrewazores
Copy link
Member

Fixes #15

To test, I have two possible procedures. The first is the way I accidentally noticed this behaviour. Run the Cryostat smoketest.sh and put the laptop to sleep for a while with everything spun up and running. The default discovery ping period is 5 minutes, but the CRYOSTAT_DISCOVERY_PING_PERIOD variable can be set to reduce that time for faster testing cycles. Put the host machine to sleep for at least the ping period, then awaken the machine again and check the logs. The Cryostat logs should reflect that it is responding 401 to the agent(s) because their JWTs are expired. Before this PR this will continue repeating. After this PR, the agents should see the first 401 and react by refreshing their registration, so no subsequent 401s should occur.

To simplify that testing procedure, this patch can also be applied to the Cryostat backend:

diff --git a/src/main/java/io/cryostat/net/web/http/api/v2/DiscoveryPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/v2/DiscoveryPostHandler.java
index d2a91465..0b4914c6 100644
--- a/src/main/java/io/cryostat/net/web/http/api/v2/DiscoveryPostHandler.java
+++ b/src/main/java/io/cryostat/net/web/http/api/v2/DiscoveryPostHandler.java
@@ -117,6 +117,9 @@ class DiscoveryPostHandler extends AbstractDiscoveryJwtConsumingHandler<Void> {
 
     @Override
     void handleWithValidJwt(RoutingContext ctx, JWT jwt) throws Exception {
+        if (Math.random() > 0.9) {
+            throw new ApiException(401, "bad luck");
+        }
         try {
             UUID id =
                     this.uuidFromString.apply(

This means 10% of the time, the agents' attempt to publish updates will fail with a 401. The 0.9 can be adjusted to make this more likely.

@tthvo
Copy link
Member

tthvo commented Dec 19, 2022

Not sure if I undestand it correctly, but i saw this when loggin quarkus-test-agent-2:

2022-12-19 19:08:37,371 SEVERE [io.cry.age.Registration] (vert.x-eventloop-thread-0) Update failure: io.vertx.core.impl.NoStackTraceThrowable: Response status code 401 is not between 200 and 300
2022-12-19 19:08:37,384 INFO  [io.cry.age.Registration] (vert.x-eventloop-thread-0) Deregistered from Cryostat discovery plugin [fcc73aa6-49c0-4b3c-8e83-1f944cf9fe77]
2022-12-19 19:08:37,384 INFO  [io.cry.age.WebServer] (vert.x-eventloop-thread-0) HTTP Server stopping
2022-12-19 19:08:37,387 INFO  [io.cry.age.WebServer] (vert.x-eventloop-thread-0) HTTP Server stopped

Seems to deregister and just "stopped"? Same happened to quarkus-test-agent-1?

Editted: Oh never mind. I think I forgot to rebuild the agent.

@maxcao13
Copy link
Member

2022-12-19 19:37:41,780 INFO  [io.cry.age.Registration] (cryostat-agent-worker-2) publishing self as service:jmx:rmi:///jndi/rmi://cryostat:9097/jmxrmi
2022-12-19 19:37:41,806 SEVERE [io.cry.age.CryostatClient] (ForkJoinPool.commonPool-worker-1) Non-OK response (401) on HTTP API https://localhost:8181/api/v2.2/discovery/79ed4412-9488-40b9-92b1-3e23a152d0b1?token=eyJjdHkiOiJKV1QiLCJlbmMiOiJBMjU2R0NNIiwiYWxnIjoiZGlyIn0..qrlA00sWLx-7fcOR.vS4VQyM_8dXz4pH7gfHZRtESFys1gzzkMcfKWszfoz7zbkF-I10kC5GrhuDCzSwUKtbSUaOfXYrUhtf1DDNNaX1noHXy41AmQxRkX9pWIpDlTjwmdxPhwX-f3GLZGd-HJlYJhftyu0zIx7eRneJ4biI33FB2j32SnpTI33VQZezPMfZbI962MDY3qSXdApHs-AIh0rVRG7oiIGmllmMAtTfyZTzvpc6N2YV8nxMOPQoHWpO55OSR94Z0ty4jnDFOLgmksjLsxC71wperom_Mb0eTsGUIb_fZwTEr308ADcpYGCwWBqgIHD0WIMqzn8ZA1goZrO6s6eYP1dXVvXCLS5Bmr0vV0zg2Y44hGSzV4kJpK_NVkdsW5oxBJ9aQv9pfyJZRGC3GsdUcUIWlloEF0bLQWA0CQguju7Lcrr0nDr6SSPDZNWu568tmlKOaaHUVCyz89c_sz6ckN9Db-spIBaxGokLO9aS6Uvc9tMgGkRnY0mmGsYkfkBz7Y91a_IzMxGoP9xmNscCfp3-LHEyHbrY4eXQiEVO7REF7RsTdlWVvZYvyg_KbS5fH2WOCQVdFYfse8bWvSA.4NgkJYJOAqIRlzZCB8GNqA
2022-12-19 19:37:41,806 SEVERE [io.cry.age.Registration] (cryostat-agent-worker-0) Update failure: java.util.concurrent.CompletionException: io.cryostat.agent.HttpException: Unexpected non-OK status code 401 on API path https://localhost:8181/api/v2.2/discovery/79ed4412-9488-40b9-92b1-3e23a152d0b1
        at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315)
        at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:320)
        at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:649)
        at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
        at java.base/java.util.concurrent.CompletableFuture.postFire(CompletableFuture.java:614)
        at java.base/java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:844)
        at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:483)
        at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
        at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
        at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
        at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
        at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)
Caused by: io.cryostat.agent.HttpException: Unexpected non-OK status code 401 on API path https://localhost:8181/api/v2.2/discovery/79ed4412-9488-40b9-92b1-3e23a152d0b1
        at io.cryostat.agent.CryostatClient.assertOkStatus(CryostatClient.java:206)
        at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:646)
        ... 9 more

2022-12-19 19:37:41,847 INFO  [io.cry.age.Registration] (cryostat-agent-worker-2) Deregistered from Cryostat discovery plugin [79ed4412-9488-40b9-92b1-3e23a152d0b1]
2022-12-19 19:37:46,863 INFO  [io.cry.age.Registration] (cryostat-agent-worker-0) Registered as a9418965-204f-4198-bf4d-cddfca949ee7
2022-12-19 19:37:46,864 INFO  [io.cry.age.Registration] (cryostat-agent-worker-0) publishing self as service:jmx:rmi:///jndi/rmi://cryostat:9097/jmxrmi
2022-12-19 19:37:47,024 INFO  [io.cry.age.Registration] (cryostat-agent-worker-2) Publish success

Looks working as expected.

Copy link
Member

@maxcao13 maxcao13 left a comment

Choose a reason for hiding this comment

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

So looks good to me.

Copy link
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

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

2022-12-19 19:37:41,780 INFO  [io.cry.age.Registration] (cryostat-agent-worker-2) publishing self as service:jmx:rmi:///jndi/rmi://cryostat:9097/jmxrmi
2022-12-19 19:37:41,806 SEVERE [io.cry.age.CryostatClient] (ForkJoinPool.commonPool-worker-1) Non-OK response (401) on HTTP API https://localhost:8181/api/v2.2/discovery/79ed4412-9488-40b9-92b1-3e23a152d0b1?token=eyJjdHkiOiJKV1QiLCJlbmMiOiJBMjU2R0NNIiwiYWxnIjoiZGlyIn0..qrlA00sWLx-7fcOR.vS4VQyM_8dXz4pH7gfHZRtESFys1gzzkMcfKWszfoz7zbkF-I10kC5GrhuDCzSwUKtbSUaOfXYrUhtf1DDNNaX1noHXy41AmQxRkX9pWIpDlTjwmdxPhwX-f3GLZGd-HJlYJhftyu0zIx7eRneJ4biI33FB2j32SnpTI33VQZezPMfZbI962MDY3qSXdApHs-AIh0rVRG7oiIGmllmMAtTfyZTzvpc6N2YV8nxMOPQoHWpO55OSR94Z0ty4jnDFOLgmksjLsxC71wperom_Mb0eTsGUIb_fZwTEr308ADcpYGCwWBqgIHD0WIMqzn8ZA1goZrO6s6eYP1dXVvXCLS5Bmr0vV0zg2Y44hGSzV4kJpK_NVkdsW5oxBJ9aQv9pfyJZRGC3GsdUcUIWlloEF0bLQWA0CQguju7Lcrr0nDr6SSPDZNWu568tmlKOaaHUVCyz89c_sz6ckN9Db-spIBaxGokLO9aS6Uvc9tMgGkRnY0mmGsYkfkBz7Y91a_IzMxGoP9xmNscCfp3-LHEyHbrY4eXQiEVO7REF7RsTdlWVvZYvyg_KbS5fH2WOCQVdFYfse8bWvSA.4NgkJYJOAqIRlzZCB8GNqA
2022-12-19 19:37:41,806 SEVERE [io.cry.age.Registration] (cryostat-agent-worker-0) Update failure: java.util.concurrent.CompletionException: io.cryostat.agent.HttpException: Unexpected non-OK status code 401 on API path https://localhost:8181/api/v2.2/discovery/79ed4412-9488-40b9-92b1-3e23a152d0b1
        at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315)
        at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:320)
        at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:649)
        at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
        at java.base/java.util.concurrent.CompletableFuture.postFire(CompletableFuture.java:614)
        at java.base/java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:844)
        at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:483)
        at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
        at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
        at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
        at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
        at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)
Caused by: io.cryostat.agent.HttpException: Unexpected non-OK status code 401 on API path https://localhost:8181/api/v2.2/discovery/79ed4412-9488-40b9-92b1-3e23a152d0b1
        at io.cryostat.agent.CryostatClient.assertOkStatus(CryostatClient.java:206)
        at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:646)
        ... 9 more

2022-12-19 19:37:41,847 INFO  [io.cry.age.Registration] (cryostat-agent-worker-2) Deregistered from Cryostat discovery plugin [79ed4412-9488-40b9-92b1-3e23a152d0b1]
2022-12-19 19:37:46,863 INFO  [io.cry.age.Registration] (cryostat-agent-worker-0) Registered as a9418965-204f-4198-bf4d-cddfca949ee7
2022-12-19 19:37:46,864 INFO  [io.cry.age.Registration] (cryostat-agent-worker-0) publishing self as service:jmx:rmi:///jndi/rmi://cryostat:9097/jmxrmi
2022-12-19 19:37:47,024 INFO  [io.cry.age.Registration] (cryostat-agent-worker-2) Publish success

Looks working as expected.

Worked as expected for me too.

@andrewazores andrewazores merged commit fa3e97c into cryostatio:main Dec 19, 2022
@andrewazores andrewazores deleted the exception-handling branch December 19, 2022 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] Agent does not retry registration if update fails with 401: Invalid JWT
3 participants