FrankChen021 commented on code in PR #20814:
URL: https://github.com/apache/druid/pull/20814#discussion_r4462739618
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/K3sClusterResource.java:
##########
@@ -339,9 +370,17 @@ private void applyManifest(File manifest)
}
/**
- * Waits for the given pod to be ready.
+ * Waits for the given pod to be ready in the {@link #DRUID_NAMESPACE}.
+ */
+ private void waitUntilPodIsReady(final PodResource pod)
Review Comment:
This overload is now unused: `waitUntilPodsAreReady` always calls
`waitUntilPodIsReady(pod, namespace)`. Please remove it. Same for
`K3sPodDiagnostics.onReadyTimeout(PodResource)`, which only tests call.
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/K3sPodDiagnostics.java:
##########
@@ -0,0 +1,315 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.testing.embedded.k8s;
+
+import io.fabric8.kubernetes.api.model.ContainerState;
+import io.fabric8.kubernetes.api.model.ContainerStatus;
+import io.fabric8.kubernetes.api.model.Event;
+import io.fabric8.kubernetes.api.model.ObjectReference;
+import io.fabric8.kubernetes.api.model.ObjectReferenceBuilder;
+import io.fabric8.kubernetes.api.model.Pod;
+import io.fabric8.kubernetes.api.model.PodCondition;
+import io.fabric8.kubernetes.api.model.PodStatus;
+import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.dsl.PodResource;
+import org.apache.druid.java.util.common.FileUtils;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.testing.embedded.EmbeddedDruidCluster;
+
+import javax.annotation.Nullable;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.List;
+
+/**
+ * Diagnostic helper for K3s pods. Gathers pod status, container states,
+ * Kubernetes events, and log tails on timeout, and dumps pod logs to disk.
+ */
+public class K3sPodDiagnostics
+{
+ private static final Logger log = new Logger(K3sPodDiagnostics.class);
+
+ @Nullable
+ private final KubernetesClient client;
+ @Nullable
+ private final String testClassName;
+
+ public K3sPodDiagnostics(@Nullable final KubernetesClient client)
+ {
+ this(client, (String) null);
+ }
+
+ public K3sPodDiagnostics(@Nullable final KubernetesClient client, @Nullable
final EmbeddedDruidCluster cluster)
+ {
+ this(client, cluster != null ? cluster.getTestClassName() : null);
+ }
+
+ public K3sPodDiagnostics(@Nullable final KubernetesClient client, @Nullable
final String testClassName)
+ {
+ this.client = client;
+ this.testClassName = testClassName;
+ }
+
+ /**
+ * Handles pod readiness timeout in the default {@link
K3sClusterResource#DRUID_NAMESPACE}:
+ * dumps pod logs, formats detailed diagnostics, and returns an {@link ISE}.
+ */
+ public ISE onReadyTimeout(final PodResource pod)
+ {
+ return onReadyTimeout(pod, K3sClusterResource.DRUID_NAMESPACE);
+ }
+
+ /**
+ * Handles pod readiness timeout in the given default namespace:
+ * dumps pod logs, formats detailed diagnostics, and returns an {@link ISE}.
+ */
+ public ISE onReadyTimeout(final PodResource pod, @Nullable final String
defaultNamespace)
+ {
+ final String resolvedNamespace = resolveNamespace(pod, defaultNamespace);
+ dumpPodLogs(resolvedNamespace);
+ final String diagnostics = buildPodDiagnostics(pod, resolvedNamespace);
+ return new ISE("%s", diagnostics);
+ }
+
+ public static String resolveNamespace(final PodResource podResource,
@Nullable final String fallbackNamespace)
+ {
+ try {
+ final Pod pod = podResource.get();
+ if (pod != null && pod.getMetadata() != null &&
pod.getMetadata().getNamespace() != null) {
+ return pod.getMetadata().getNamespace();
+ }
+ }
+ catch (Exception ignored) {
+ // ignore
+ }
+ return fallbackNamespace != null ? fallbackNamespace :
K3sClusterResource.DRUID_NAMESPACE;
+ }
+
+ public String buildPodDiagnostics(final PodResource podResource, @Nullable
final String defaultNamespace)
+ {
+ Pod pod = null;
+ try {
+ pod = podResource.get();
+ }
+ catch (Exception e) {
+ log.warn(e, "Could not get pod resource");
+ }
+
+ final String podName = pod != null && pod.getMetadata() != null
+ ? pod.getMetadata().getName()
+ : null;
+ final String namespace = pod != null && pod.getMetadata() != null &&
pod.getMetadata().getNamespace() != null
Review Comment:
nit: `onReadyTimeout` has already resolved the namespace through
`resolveNamespace`, and this repeats that logic. Consider passing the resolved
namespace through and using it directly.
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/K3sPodDiagnosticsTest.java:
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.testing.embedded.k8s;
+
+import io.fabric8.kubernetes.api.model.ContainerStateBuilder;
+import io.fabric8.kubernetes.api.model.ContainerStateTerminatedBuilder;
+import io.fabric8.kubernetes.api.model.ContainerStateWaitingBuilder;
+import io.fabric8.kubernetes.api.model.ContainerStatus;
+import io.fabric8.kubernetes.api.model.ContainerStatusBuilder;
+import io.fabric8.kubernetes.api.model.Event;
+import io.fabric8.kubernetes.api.model.EventBuilder;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.fabric8.kubernetes.api.model.Pod;
+import io.fabric8.kubernetes.api.model.PodBuilder;
+import io.fabric8.kubernetes.api.model.PodCondition;
+import io.fabric8.kubernetes.api.model.PodConditionBuilder;
+import io.fabric8.kubernetes.api.model.PodStatus;
+import io.fabric8.kubernetes.api.model.PodStatusBuilder;
+import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.dsl.PodResource;
+import org.apache.druid.java.util.common.ISE;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import java.util.Collections;
+import java.util.List;
+
+public class K3sPodDiagnosticsTest
+{
+ @Test
+ public final void testFormatPodDiagnosticsWithNullPod()
Review Comment:
nit: test methods elsewhere in the codebase are `public void`; the `final`
isn't needed. Applies to every test in this file and in
`K3sClusterResourceTest`.
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/K3sPodDiagnosticsTest.java:
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.testing.embedded.k8s;
+
+import io.fabric8.kubernetes.api.model.ContainerStateBuilder;
+import io.fabric8.kubernetes.api.model.ContainerStateTerminatedBuilder;
+import io.fabric8.kubernetes.api.model.ContainerStateWaitingBuilder;
+import io.fabric8.kubernetes.api.model.ContainerStatus;
+import io.fabric8.kubernetes.api.model.ContainerStatusBuilder;
+import io.fabric8.kubernetes.api.model.Event;
+import io.fabric8.kubernetes.api.model.EventBuilder;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.fabric8.kubernetes.api.model.Pod;
+import io.fabric8.kubernetes.api.model.PodBuilder;
+import io.fabric8.kubernetes.api.model.PodCondition;
+import io.fabric8.kubernetes.api.model.PodConditionBuilder;
+import io.fabric8.kubernetes.api.model.PodStatus;
+import io.fabric8.kubernetes.api.model.PodStatusBuilder;
+import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.dsl.PodResource;
+import org.apache.druid.java.util.common.ISE;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import java.util.Collections;
+import java.util.List;
+
+public class K3sPodDiagnosticsTest
+{
+ @Test
+ public final void testFormatPodDiagnosticsWithNullPod()
+ {
+ final String diagnostics = K3sPodDiagnostics.formatPodDiagnostics(null,
null, null);
+ Assertions.assertNotNull(diagnostics);
+ Assertions.assertTrue(diagnostics.contains("Pod metadata is unavailable"));
+ }
+
+ @Test
+ public final void testFormatPodDiagnosticsWithRunningAndReadyPod()
+ {
+ final PodCondition readyCondition = new PodConditionBuilder()
+ .withType("Ready")
+ .withStatus("True")
+ .withReason("PodReady")
+ .withMessage("All containers are ready")
+ .build();
+
+ final ContainerStatus containerStatus = new ContainerStatusBuilder()
+ .withName("druid-coordinator")
+ .withReady(true)
+ .withRestartCount(0)
+ .withState(new
ContainerStateBuilder().withNewRunning().withStartedAt("2026-11-01T12:00:00Z").endRunning().build())
+ .build();
+
+ final PodStatus status = new PodStatusBuilder()
+ .withPhase("Running")
+ .withConditions(Collections.singletonList(readyCondition))
+ .withContainerStatuses(Collections.singletonList(containerStatus))
+ .build();
+
+ final Pod pod = new PodBuilder()
+ .withMetadata(new
ObjectMetaBuilder().withName("druid-coordinator-1").withNamespace("druid").build())
+ .withStatus(status)
+ .build();
+
+ final String diagnostics = K3sPodDiagnostics.formatPodDiagnostics(pod,
null, null);
+ Assertions.assertTrue(diagnostics.contains("pod [druid-coordinator-1] in
namespace [druid]"));
+ Assertions.assertTrue(diagnostics.contains("Phase: [Running]"));
+ Assertions.assertTrue(diagnostics.contains("Type: [Ready], Status: [True],
Reason: [PodReady]"));
+ Assertions.assertTrue(diagnostics.contains("Container:
[druid-coordinator], Ready: [true], RestartCount: [0]"));
+ Assertions.assertTrue(diagnostics.contains("State:
Running(startedAt=[2026-11-01T12:00:00Z])"));
+ }
+
+ @Test
+ public final void testFormatPodDiagnosticsWithCrashLoopBackOffPod()
+ {
+ final PodCondition condition = new PodConditionBuilder()
+ .withType("Ready")
+ .withStatus("False")
+ .withReason("ContainersNotReady")
+ .withMessage("containers with unready status: [druid-coordinator]")
+ .build();
+
+ final ContainerStatus containerStatus = new ContainerStatusBuilder()
+ .withName("druid-coordinator")
+ .withReady(false)
+ .withRestartCount(3)
+ .withState(
+ new ContainerStateBuilder()
+ .withWaiting(
+ new ContainerStateWaitingBuilder()
+ .withReason("CrashLoopBackOff")
+ .withMessage("back-off 40s restarting failed
container")
+ .build()
+ )
+ .build()
+ )
+ .withLastState(
+ new ContainerStateBuilder()
+ .withTerminated(
+ new ContainerStateTerminatedBuilder()
+ .withExitCode(137)
+ .withReason("OOMKilled")
+ .withMessage("Command terminated")
+ .build()
+ )
+ .build()
+ )
+ .build();
+
+ final PodStatus status = new PodStatusBuilder()
+ .withPhase("Running")
+ .withConditions(Collections.singletonList(condition))
+ .withContainerStatuses(Collections.singletonList(containerStatus))
+ .build();
+
+ final Pod pod = new PodBuilder()
+ .withMetadata(new
ObjectMetaBuilder().withName("druid-coordinator-1").withNamespace("druid").build())
+ .withStatus(status)
+ .build();
+
+ final Event event = new EventBuilder()
+ .withType("Warning")
+ .withReason("BackOff")
+ .withMessage("Back-off restarting failed container")
+ .withCount(3)
+ .withLastTimestamp("2026-11-01T12:05:00Z")
+ .build();
+
+ final String logTail = "java.lang.OutOfMemoryError: Java heap space\n\tat
org.apache.druid.server.Main.start(Main.java:42)";
+
+ final String diagnostics = K3sPodDiagnostics.formatPodDiagnostics(pod,
List.of(event), logTail);
+ Assertions.assertTrue(diagnostics.contains("Timed out waiting for pod
[druid-coordinator-1] in namespace [druid] to be ready."));
+ Assertions.assertTrue(diagnostics.contains("Phase: [Running]"));
+ Assertions.assertTrue(diagnostics.contains("Type: [Ready], Status:
[False], Reason: [ContainersNotReady]"));
+ Assertions.assertTrue(diagnostics.contains("Container:
[druid-coordinator], Ready: [false], RestartCount: [3]"));
+ Assertions.assertTrue(diagnostics.contains("State:
Waiting(reason=[CrashLoopBackOff], message=[back-off 40s restarting failed
container])"));
+ Assertions.assertTrue(diagnostics.contains("LastState:
Terminated(exitCode=[137], reason=[OOMKilled], message=[Command terminated])"));
+ Assertions.assertTrue(diagnostics.contains("Events:"));
+ Assertions.assertTrue(diagnostics.contains("[Warning] BackOff: Back-off
restarting failed container (count: [3], lastObserved:
[2026-11-01T12:05:00Z])"));
+ Assertions.assertTrue(diagnostics.contains("Log Tail (last lines):"));
+ Assertions.assertTrue(diagnostics.contains("OutOfMemoryError: Java heap
space"));
+ }
+
+ @Test
+ public final void testFormatPodDiagnosticsWithUnhealthyReadinessProbeEvent()
+ {
+ final Pod pod = new PodBuilder()
+ .withMetadata(new
ObjectMetaBuilder().withName("druid-router-1").withNamespace("druid").build())
+ .withStatus(new PodStatusBuilder().withPhase("Running").build())
+ .build();
+
+ final Event event = new EventBuilder()
+ .withType("Warning")
+ .withReason("Unhealthy")
+ .withMessage("Readiness probe failed: HTTP probe failed with
statuscode: 503")
+ .withCount(5)
+ .withLastTimestamp("2026-11-01T12:03:00Z")
+ .build();
+
+ final String diagnostics = K3sPodDiagnostics.formatPodDiagnostics(pod,
List.of(event), null);
+ Assertions.assertTrue(diagnostics.contains("pod [druid-router-1] in
namespace [druid]"));
+ Assertions.assertTrue(diagnostics.contains("[Warning] Unhealthy: Readiness
probe failed: HTTP probe failed with statuscode: 503 (count: [5]"));
+ }
+
+ @Test
+ public final void testFormatPodDiagnosticsWithCustomNamespace()
+ {
+ final Pod pod = new PodBuilder()
+ .withMetadata(new
ObjectMetaBuilder().withName("druid-operator-1").withNamespace("druid-operator-system").build())
+ .withStatus(new PodStatusBuilder().withPhase("Pending").build())
+ .build();
+
+ final String diagnostics = K3sPodDiagnostics.formatPodDiagnostics(pod,
null, null);
+ Assertions.assertTrue(diagnostics.contains("pod [druid-operator-1] in
namespace [druid-operator-system]"));
+ Assertions.assertTrue(diagnostics.contains("Phase: [Pending]"));
+ }
+
+ @Test
+ public final void testResolveNamespaceFromPodMetadata()
+ {
+ final PodResource podResource = Mockito.mock(PodResource.class);
+ final Pod pod = new PodBuilder()
+ .withMetadata(new
ObjectMetaBuilder().withName("operator").withNamespace("druid-operator-system").build())
+ .build();
+ Mockito.when(podResource.get()).thenReturn(pod);
+
+ Assertions.assertEquals("druid-operator-system",
K3sPodDiagnostics.resolveNamespace(podResource, "druid"));
+ }
+
+ @Test
+ public final void testResolveNamespaceFallback()
+ {
+ final PodResource podResourceWithoutNamespace =
Mockito.mock(PodResource.class);
+ final Pod podWithoutNamespace = new PodBuilder()
+ .withMetadata(new ObjectMetaBuilder().withName("operator").build())
+ .build();
+
Mockito.when(podResourceWithoutNamespace.get()).thenReturn(podWithoutNamespace);
+
+ Assertions.assertEquals(
+ "druid-operator-system",
+ K3sPodDiagnostics.resolveNamespace(podResourceWithoutNamespace,
"druid-operator-system")
+ );
+ Assertions.assertEquals(
+ K3sClusterResource.DRUID_NAMESPACE,
+ K3sPodDiagnostics.resolveNamespace(podResourceWithoutNamespace, null)
+ );
+
+ final PodResource brokenPodResource = Mockito.mock(PodResource.class);
+ Mockito.when(brokenPodResource.get()).thenThrow(new RuntimeException("API
error"));
+ Assertions.assertEquals(
+ "fallback-ns",
+ K3sPodDiagnostics.resolveNamespace(brokenPodResource, "fallback-ns")
+ );
+ }
+
+ @Test
+ public final void testOnReadyTimeout()
+ {
+ final PodResource podResource = Mockito.mock(PodResource.class);
+ final Pod pod = new PodBuilder()
+ .withMetadata(new
ObjectMetaBuilder().withName("druid-broker-1").withNamespace("druid").build())
+ .withStatus(new PodStatusBuilder().withPhase("Pending").build())
+ .build();
+ Mockito.when(podResource.get()).thenReturn(pod);
+
+ final KubernetesClient client = Mockito.mock(KubernetesClient.class,
Mockito.RETURNS_DEEP_STUBS);
+ final K3sPodDiagnostics diagnostics = new K3sPodDiagnostics(client,
"testClass");
Review Comment:
`onReadyTimeout` calls `dumpPodLogs`, which creates
`druid-container-logs/testClass/k3s-pods` relative to the working directory,
i.e. inside `embedded-tests/`. That path is not gitignored, so every `mvn test`
leaves an untracked directory. With the `File` constructor suggested above,
this test can pass a JUnit `@TempDir`.
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/K3sPodDiagnostics.java:
##########
@@ -0,0 +1,315 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.testing.embedded.k8s;
+
+import io.fabric8.kubernetes.api.model.ContainerState;
+import io.fabric8.kubernetes.api.model.ContainerStatus;
+import io.fabric8.kubernetes.api.model.Event;
+import io.fabric8.kubernetes.api.model.ObjectReference;
+import io.fabric8.kubernetes.api.model.ObjectReferenceBuilder;
+import io.fabric8.kubernetes.api.model.Pod;
+import io.fabric8.kubernetes.api.model.PodCondition;
+import io.fabric8.kubernetes.api.model.PodStatus;
+import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.dsl.PodResource;
+import org.apache.druid.java.util.common.FileUtils;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.testing.embedded.EmbeddedDruidCluster;
+
+import javax.annotation.Nullable;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.List;
+
+/**
+ * Diagnostic helper for K3s pods. Gathers pod status, container states,
+ * Kubernetes events, and log tails on timeout, and dumps pod logs to disk.
+ */
+public class K3sPodDiagnostics
+{
+ private static final Logger log = new Logger(K3sPodDiagnostics.class);
+
+ @Nullable
+ private final KubernetesClient client;
+ @Nullable
+ private final String testClassName;
+
+ public K3sPodDiagnostics(@Nullable final KubernetesClient client)
+ {
+ this(client, (String) null);
+ }
+
+ public K3sPodDiagnostics(@Nullable final KubernetesClient client, @Nullable
final EmbeddedDruidCluster cluster)
+ {
+ this(client, cluster != null ? cluster.getTestClassName() : null);
+ }
+
+ public K3sPodDiagnostics(@Nullable final KubernetesClient client, @Nullable
final String testClassName)
+ {
+ this.client = client;
+ this.testClassName = testClassName;
+ }
Review Comment:
Could we collapse these three constructors into one
`K3sPodDiagnostics(KubernetesClient client, File podLogDirectory)` with a
non-null client? `K3sClusterResource.onStarted` can build the directory from
`cluster.getTestClassName()`.
That removes:
- the `cluster` field in `K3sClusterResource`
- `getPodDiagnostics()`
- the null guard in `K3sClusterResource.dumpPodLogs` and the `client ==
null` check here
- the `"k3s"` fallback
- the two null-client tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
