[Bug] Duplicate discovery node for pod in container discovery

tthvo commented 2 months ago

Current Behavior

With PodmanDiscovery, containers under the same pod are represented under different discovery node (i.e. pod node).

Expected Behavior

Containers under the same pod should have a common (pod) discovery node.

Steps To Reproduce

Run smoketests bash smoketest.bash.

Launch a pod:

podman pod create --replace --name cryostat-pod

Launch containers into that pod:

podman run \
      --name jmxquarkus \
      --pod cryostat-pod \
      --label io.cryostat.discovery="true" \
      --label io.cryostat.jmxPort="51423" \
      --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="vertx-fib-demo-0" \
      --label io.cryostat.jmxPort="9089" \
      --rm -d quay.io/andrewazores/vertx-fib-demo:0.13.1

Environment

- OS: Fedora
- Environment: Local smoketest with docker-compose and podman API

Anything else?

Related bugs:

Looks like deleting a pod does not actually remove the pod subtree from the discovery tree.

Deleting a container without label io.cryostat.jmxHost will cause:

ERROR: Thread Thread[executor-thread-22,5,main] threw an uncaught exception
java.lang.NullPointerException: Cannot read field "Hostname" because "java.util.concurrent.CompletableFuture.get(long, java.util.concurrent.TimeUnit).Config" is null
at io.cryostat.discovery.ContainerDiscovery.handleContainerEvent(ContainerDiscovery.java:319)
at io.cryostat.discovery.PodmanDiscovery_Subclass.handleContainerEvent$$superforward(Unknown Source)
at io.cryostat.discovery.PodmanDiscovery_Subclass$$function$$1.apply(Unknown Source)
at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:73)
at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:62)
at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorBase.invokeInOurTx(TransactionalInterceptorBase.java:136)
at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorBase.invokeInOurTx(TransactionalInterceptorBase.java:107)
at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorRequired.doIntercept(TransactionalInterceptorRequired.java:38)
at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorBase.intercept(TransactionalInterceptorBase.java:61)
at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorRequired.intercept(TransactionalInterceptorRequired.java:32)
at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorRequired_Bean.intercept(Unknown Source)
at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42)
at io.quarkus.arc.impl.AroundInvokeInvocationContext.perform(AroundInvokeInvocationContext.java:30)
at io.quarkus.arc.impl.InvocationContexts.performAroundInvoke(InvocationContexts.java:27)
at io.cryostat.discovery.PodmanDiscovery_Subclass.handleContainerEvent(Unknown Source)
at io.cryostat.discovery.ContainerDiscovery.lambda$queryContainers$1(ContainerDiscovery.java:217)
at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
at java.base/java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1707)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
at io.cryostat.discovery.ContainerDiscovery.lambda$queryContainers$2(ContainerDiscovery.java:215)
at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:576)
at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
at java.base/java.lang.Thread.run(Thread.java:840)

tthvo commented 2 months ago

Seems like the bug above is caused by the pod discovery node being freshly created instead of taking the persisted version:

https://github.com/cryostatio/cryostat3/blob/d1008a24b6ac244222dc5fc5a937cad3a6445743/src/main/java/io/cryostat/discovery/ContainerDiscovery.java#L362

Not sure on the deletion issues tho...

tthvo commented 2 months ago

Not sure on the deletion issues tho...

Looks like if the container does not specify label io.cryostat.jmxHost, cryostat will query podman API for its hostname. But the container already shut down thus returning null on query response.

cryostatio / cryostat