diff --git a/compose/compose-cryostat.yaml b/compose/compose-cryostat.yaml index 98552fdc84..e6822b86ff 100644 --- a/compose/compose-cryostat.yaml +++ b/compose/compose-cryostat.yaml @@ -6,7 +6,7 @@ services: user: '0' stdin_open: true tty: true - labels: + labels: - io.cryostat.discovery=true - io.cryostat.jmxUrl=service:jmx:rmi:///jndi/rmi://localhost:0/jmxrmi deploy: @@ -82,7 +82,7 @@ services: cryostat-duplicate: image: quay.io/cryostat/cryostat:latest container_name: cryostat-duplicate - labels: + labels: - io.cryostat.discovery=true - io.cryostat.jmxUrl=service:jmx:rmi:///jndi/rmi://cryostat-duplicate:8282/jmxrmi - io.cryostat.jmxPort=8282 @@ -154,9 +154,9 @@ services: # Testing apps scaled-app: - image: quay.io/andrewazores/vertx-fib-demo:0.12.2 + image: quay.io/andrewazores/vertx-fib-demo:0.13.0 container_name: scaled-app - labels: + labels: - io.cryostat.jmxUrl=service:jmx:rmi:///jndi/rmi://scaled-app:9093/jmxrmi environment: - JMX_PORT=9093 @@ -167,9 +167,9 @@ services: profiles: ["scaled"] stopped-app: - image: quay.io/andrewazores/vertx-fib-demo:0.12.2 + image: quay.io/andrewazores/vertx-fib-demo:0.13.0 container_name: stopped-app - labels: + labels: - io.cryostat.jmxUrl=service:jmx:rmi:///jndi/rmi://stopped-app:9093/jmxrmi environment: - JMX_PORT=9093 @@ -181,9 +181,9 @@ services: # invalid targets: invalid-podman: - image: quay.io/andrewazores/vertx-fib-demo:0.12.2 + image: quay.io/andrewazores/vertx-fib-demo:0.13.0 container_name: invalid-jmx - labels: + labels: - io.cryostat.jmxUrl=invalid:jmx:rmi:///jndi/rmi://invalid-podman:9093/jmxrmi # invalid serviceurl environment: - JMX_PORT=9093 diff --git a/compose/compose-vertx-jmx.yaml b/compose/compose-vertx-jmx.yaml index 12b7e60384..3ffd58203b 100644 --- a/compose/compose-vertx-jmx.yaml +++ b/compose/compose-vertx-jmx.yaml @@ -1,7 +1,7 @@ --- services: vertx-jmx: - image: quay.io/andrewazores/vertx-fib-demo:0.12.4 + image: quay.io/andrewazores/vertx-fib-demo:0.13.0 container_name: vertx-jmx deploy: mode: global diff --git a/smoketest-docker.sh b/smoketest-docker.sh index 62a6a80bd5..a492811054 100755 --- a/smoketest-docker.sh +++ b/smoketest-docker.sh @@ -106,7 +106,7 @@ runDemoApps() { --label io.cryostat.jmxPort="9093" \ --publish 8081:8081 \ --publish 9093:9093 \ - --rm -d quay.io/andrewazores/vertx-fib-demo:0.12.2 + --rm -d quay.io/andrewazores/vertx-fib-demo:0.13.0 CONTAINERS="${CONTAINERS:+${CONTAINERS} }vertx-fib-demo-1" docker run \ @@ -119,7 +119,7 @@ runDemoApps() { --label io.cryostat.jmxPort="9094" \ --publish 8082:8082 \ --publish 9094:9092 \ - --rm -d quay.io/andrewazores/vertx-fib-demo:0.12.2 + --rm -d quay.io/andrewazores/vertx-fib-demo:0.13.0 CONTAINERS="${CONTAINERS:+${CONTAINERS} }vertx-fib-demo-2" docker run \ @@ -133,7 +133,7 @@ runDemoApps() { --label io.cryostat.jmxPort="9095" \ --publish 8083:8083 \ --publish 9095:9095 \ - --rm -d quay.io/andrewazores/vertx-fib-demo:0.12.2 + --rm -d quay.io/andrewazores/vertx-fib-demo:0.13.0 CONTAINERS="${CONTAINERS:+${CONTAINERS} }vertx-fib-demo-3" # this config is broken on purpose (missing required env vars) to test the agent's behaviour diff --git a/smoketest.sh b/smoketest.sh index 0eff168243..f3f9cfb8db 100755 --- a/smoketest.sh +++ b/smoketest.sh @@ -116,6 +116,17 @@ runDemoApps() { --env QUARKUS_HTTP_PORT=10012 \ --rm -d quay.io/roberttoyonaga/jmx:jmxquarkus@sha256:b067f29faa91312d20d43c55d194a2e076de7d0d094da3d43ee7d2b2b5a6f100 + podman run \ + --name vertx-fib-demo-0 \ + --env HTTP_PORT=8079 \ + --env JMX_PORT=9089 \ + --env START_DELAY=60 \ + --pod cryostat-pod \ + --label io.cryostat.discovery="true" \ + --label io.cryostat.jmxHost="localhost" \ + --label io.cryostat.jmxPort="9089" \ + --rm -d quay.io/andrewazores/vertx-fib-demo:0.13.0 + podman run \ --name vertx-fib-demo-1 \ --env HTTP_PORT=8081 \ @@ -133,7 +144,7 @@ runDemoApps() { --label io.cryostat.discovery="true" \ --label io.cryostat.jmxHost="localhost" \ --label io.cryostat.jmxPort="9093" \ - --rm -d quay.io/andrewazores/vertx-fib-demo:0.12.3 + --rm -d quay.io/andrewazores/vertx-fib-demo:0.13.0 podman run \ --name vertx-fib-demo-2 \ @@ -154,7 +165,7 @@ runDemoApps() { --label io.cryostat.jmxHost="localhost" \ --label io.cryostat.jmxPort="9094" \ --label io.cryostat.jmxUrl="service:jmx:rmi:///jndi/rmi://localhost:9094/jmxrmi" \ - --rm -d quay.io/andrewazores/vertx-fib-demo:0.12.3 + --rm -d quay.io/andrewazores/vertx-fib-demo:0.13.0 podman run \ --name vertx-fib-demo-3 \ @@ -174,7 +185,7 @@ runDemoApps() { --pod cryostat-pod \ --label io.cryostat.discovery="true" \ --label io.cryostat.jmxUrl="service:jmx:rmi:///jndi/rmi://localhost:9095/jmxrmi" \ - --rm -d quay.io/andrewazores/vertx-fib-demo:0.12.3 + --rm -d quay.io/andrewazores/vertx-fib-demo:0.13.0 # this config is broken on purpose (missing required env vars) to test the agent's behaviour # when not properly set up diff --git a/src/main/java/io/cryostat/discovery/DiscoveryStorage.java b/src/main/java/io/cryostat/discovery/DiscoveryStorage.java index bca8a77c81..38d4938067 100644 --- a/src/main/java/io/cryostat/discovery/DiscoveryStorage.java +++ b/src/main/java/io/cryostat/discovery/DiscoveryStorage.java @@ -45,10 +45,13 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.UUID; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ForkJoinPool; +import java.util.function.Predicate; import javax.script.ScriptException; @@ -57,7 +60,6 @@ import io.cryostat.configuration.StoredCredentials; import io.cryostat.core.log.Logger; import io.cryostat.core.net.discovery.JvmDiscoveryClient.EventKind; -import io.cryostat.net.web.http.AbstractAuthenticatedRequestHandler; import io.cryostat.platform.ServiceRef; import io.cryostat.platform.ServiceRef.AnnotationKey; import io.cryostat.platform.discovery.AbstractNode; @@ -65,6 +67,7 @@ import io.cryostat.platform.discovery.EnvironmentNode; import io.cryostat.platform.discovery.TargetNode; import io.cryostat.recordings.JvmIdHelper; +import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; import io.cryostat.rules.MatchExpressionEvaluator; import io.cryostat.util.HttpStatusCodeIdentifier; @@ -96,9 +99,10 @@ public class DiscoveryStorage extends AbstractPlatformClientVerticle { private final Gson gson; private final WebClient http; private final Logger logger; - private long timerId = -1L; + private long pluginPruneTimerId = -1L; + private long targetRetryTimeId = -1L; - private final Map targetsToUpdate = new HashMap<>(); + private final Map nonConnectableTargets = new HashMap<>(); public static final String DISCOVERY_STARTUP_ADDRESS = "discovery-startup"; @@ -143,34 +147,53 @@ public void start(Promise future) throws Exception { + " deployed"))) .onFailure(future::fail); - this.timerId = getVertx().setPeriodic(pingPeriod.toMillis(), i -> pingPrune()); + this.pluginPruneTimerId = getVertx().setPeriodic(pingPeriod.toMillis(), i -> pingPrune()); + this.targetRetryTimeId = + getVertx() + .setPeriodic( + // TODO make this configurable, use an exponential backoff, have a + // maximum retry policy, etc. + 15_000, + i -> { + testNonConnectedTargets( + entry -> { + TargetNode targetNode = entry.getKey(); + try { + String id = + jvmIdHelper + .get() + .resolveId( + targetNode.getTarget()) + .getJvmId(); + return StringUtils.isNotBlank(id); + } catch (JvmIdGetException e) { + logger.info( + "Retain null jvmId for node [{}]", + targetNode.getName()); + logger.info(e); + return false; + } + }); + }); this.credentialsManager .get() .addListener( event -> { switch (event.getEventType()) { case ADDED: - Map copy = new HashMap<>(targetsToUpdate); - for (var entry : copy.entrySet()) { - try { - if (matchExpressionEvaluator - .get() - .applies( - event.getPayload(), - entry.getKey().getTarget())) { - targetsToUpdate.remove(entry.getKey()); - UUID id = entry.getValue(); - PluginInfo plugin = getById(id).orElseThrow(); - EnvironmentNode original = - gson.fromJson( - plugin.getSubtree(), - EnvironmentNode.class); - update(id, original.getChildren()); - } - } catch (JsonSyntaxException | ScriptException e) { - throw new RuntimeException(e); - } - } + testNonConnectedTargets( + entry -> { + try { + return matchExpressionEvaluator + .get() + .applies( + event.getPayload(), + entry.getKey().getTarget()); + } catch (ScriptException e) { + logger.error(e); + return false; + } + }); break; case REMOVED: break; @@ -181,9 +204,33 @@ public void start(Promise future) throws Exception { }); } + private void testNonConnectedTargets(Predicate> predicate) { + ForkJoinPool.commonPool() + .execute( + () -> { + Map copy = new HashMap<>(nonConnectableTargets); + for (var entry : copy.entrySet()) { + try { + if (predicate.test(entry)) { + nonConnectableTargets.remove(entry.getKey()); + UUID id = entry.getValue(); + PluginInfo plugin = getById(id).orElseThrow(); + EnvironmentNode original = + gson.fromJson( + plugin.getSubtree(), EnvironmentNode.class); + update(id, original.getChildren()); + } + } catch (JsonSyntaxException e) { + throw new RuntimeException(e); + } + } + }); + } + @Override public void stop() { - getVertx().cancelTimer(timerId); + getVertx().cancelTimer(pluginPruneTimerId); + getVertx().cancelTimer(targetRetryTimeId); } private CompositeFuture pingPrune() { @@ -315,17 +362,12 @@ private List modifyChildrenWithJvmIds( ServiceRef ref = ((TargetNode) child).getTarget(); try { ref = jvmIdHelper.get().resolveId(ref); + child = new TargetNode(child.getNodeType(), ref, child.getLabels()); } catch (Exception e) { - // if Exception is of SSL or JMX Auth, ignore warning and use null jvmId - if (!(AbstractAuthenticatedRequestHandler.isJmxAuthFailure(e) - || AbstractAuthenticatedRequestHandler.isJmxSslFailure(e))) { - logger.info("Ignoring target node [{}]", child.getName()); - continue; - } logger.info("Update node [{}] with null jvmId", child.getName()); - targetsToUpdate.putIfAbsent((TargetNode) child, id); + logger.info(e); + nonConnectableTargets.putIfAbsent((TargetNode) child, id); } - child = new TargetNode(child.getNodeType(), ref, child.getLabels()); modifiedChildren.add(child); } else if (child instanceof EnvironmentNode) { modifiedChildren.add( diff --git a/src/main/java/io/cryostat/recordings/JvmIdHelper.java b/src/main/java/io/cryostat/recordings/JvmIdHelper.java index f5725e754f..4e2001691c 100644 --- a/src/main/java/io/cryostat/recordings/JvmIdHelper.java +++ b/src/main/java/io/cryostat/recordings/JvmIdHelper.java @@ -119,7 +119,6 @@ public class JvmIdHelper extends AbstractEventEmitter() { - @Override - public PluginInfo answer(InvocationOnMock invocation) - throws Throwable { - List subtree = invocation.getArgument(1); - EnvironmentNode next = - new EnvironmentNode( - "next", BaseNodeType.REALM, Map.of(), subtree); - return new PluginInfo( - "test-realm", - URI.create("http://example.com"), - gson.toJson(next)); - } - }); - JvmIdGetException jige = Mockito.mock(JvmIdGetException.class); - ExecutionException ex = Mockito.mock(ExecutionException.class); - Mockito.when(jige.getCause()).thenReturn(ex); - Mockito.when(ex.getCause()).thenReturn(new SecurityException("test")); - - Mockito.when(jvmIdHelper.resolveId(Mockito.any(ServiceRef.class))) - .thenThrow(jige) - .thenReturn( - new ServiceRef( - "serviceRef2", serviceRef2.getServiceUri(), "serviceRef2")); - - var updatedSubtree = storage.update(id, List.of(realm2)); - MatcherAssert.assertThat(updatedSubtree, Matchers.notNullValue()); - MatcherAssert.assertThat(updatedSubtree, Matchers.hasSize(1)); - for (AbstractNode node : updatedSubtree) { - MatcherAssert.assertThat(node, Matchers.instanceOf(EnvironmentNode.class)); - EnvironmentNode env = (EnvironmentNode) node; - MatcherAssert.assertThat(env.getChildren(), Matchers.hasSize(2)); - for (AbstractNode nested : env.getChildren()) { - if (nested instanceof TargetNode) { - TargetNode target = (TargetNode) nested; - MatcherAssert.assertThat(target, Matchers.instanceOf(TargetNode.class)); - if (target.getTarget().getAlias().get().equals("serviceRef1")) { - MatcherAssert.assertThat( - target.getTarget().getJvmId(), Matchers.nullValue()); - } else if (target.getTarget().getAlias().get().equals("serviceRef2")) { - MatcherAssert.assertThat( - target.getTarget().getJvmId(), Matchers.equalTo("serviceRef2")); - } else { - throw new IllegalStateException("Unexpected alias"); - } - } - } - } - } } } diff --git a/src/test/java/itest/bases/ExternalTargetsTest.java b/src/test/java/itest/bases/ExternalTargetsTest.java index 1454b5b26e..524388bf94 100644 --- a/src/test/java/itest/bases/ExternalTargetsTest.java +++ b/src/test/java/itest/bases/ExternalTargetsTest.java @@ -45,7 +45,7 @@ public abstract class ExternalTargetsTest extends StandardSelfTest { - protected static final String FIB_DEMO_IMAGESPEC = "quay.io/andrewazores/vertx-fib-demo:0.8.0"; + protected static final String FIB_DEMO_IMAGESPEC = "quay.io/andrewazores/vertx-fib-demo:0.13.0"; static final int DISCOVERY_POLL_PERIOD_MS = Integer.parseInt(System.getProperty("cryostat.itest.jdp.poll.period", "2500"));