From 3fb8c656d37d60437b63bbfc10a93efd604b5498 Mon Sep 17 00:00:00 2001 From: Lewis Tierney Date: Wed, 22 Apr 2026 13:47:26 +0100 Subject: [PATCH 1/5] fix: source jvm.config from peon.sh for K8s peon containers --- distribution/docker/peon.sh | 7 +++++++ docs/development/extensions-core/k8s-jobs.md | 2 ++ 2 files changed, 9 insertions(+) diff --git a/distribution/docker/peon.sh b/distribution/docker/peon.sh index 3b0c172ed5c7..47e1d6b271a0 100755 --- a/distribution/docker/peon.sh +++ b/distribution/docker/peon.sh @@ -165,6 +165,13 @@ fi # If TASK_JSON is not set, CliPeon will pull the task.json file from deep storage. mkdir -p ${TASK_DIR}; [ -n "$TASK_JSON" ] && echo ${TASK_JSON} | base64 -d | gzip -d > ${TASK_DIR}/task.json; +# Combine options from jvm.config and those given as JAVA_OPTS +# If a value is specified in both then JAVA_OPTS will take precedence when using OpenJDK +# However this behavior is not part of the spec and is thus implementation specific +if [ -f "$SERVICE_CONF_DIR/jvm.config" ]; then + JAVA_OPTS="$(cat $SERVICE_CONF_DIR/jvm.config | xargs) $JAVA_OPTS" +fi + # Start peon using CliPeon, with variables `Main internal peon TASK_DIR ATTEMPT_ID` if [ -n "$TASK_ID" ]; then # TASK_ID is only set from PodTemplateTaskAdapter diff --git a/docs/development/extensions-core/k8s-jobs.md b/docs/development/extensions-core/k8s-jobs.md index 5ebd97c04ee9..0346348c7ff8 100644 --- a/docs/development/extensions-core/k8s-jobs.md +++ b/docs/development/extensions-core/k8s-jobs.md @@ -483,6 +483,8 @@ template: Any runtime property or JVM config used by the peon process can also be passed. E.G. below is an example of a ConfigMap that can be used to generate the `nodetype-config-volume` mount in the above template. +The peon container startup script (`peon.sh`) reads `jvm.config` from the directory mounted as `nodetype-config-volume` and prepends its contents to the peon's `JAVA_OPTS` before launching the JVM. If an option is set both in `jvm.config` and in `JAVA_OPTS` (for example via `druid.indexer.runner.javaOptsArray` when using the `overlordSingleContainer` or `overlordMultiContainer` adapters, which inject `JAVA_OPTS` as a container env var), the `JAVA_OPTS` value takes precedence under OpenJDK. +
Example ConfigMap From 206ab1d5fb8b95e6d79888c1076b83c634fd3e34 Mon Sep 17 00:00:00 2001 From: Lewis Tierney Date: Wed, 22 Apr 2026 14:12:05 +0100 Subject: [PATCH 2/5] test: add K8s integration test for peon jvm.config sourcing --- .../BaseKubernetesTaskRunnerDockerTest.java | 16 +- .../embedded/k8s/K3sClusterResource.java | 9 + .../KubernetesPeonJvmConfigDockerTest.java | 201 ++++++++++++++++++ ...d-service-with-operator-peonjvmconfig.yaml | 115 ++++++++++ 4 files changed, 338 insertions(+), 3 deletions(-) create mode 100644 embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/KubernetesPeonJvmConfigDockerTest.java create mode 100644 embedded-tests/src/test/resources/manifests/druid-service-with-operator-peonjvmconfig.yaml diff --git a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/BaseKubernetesTaskRunnerDockerTest.java b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/BaseKubernetesTaskRunnerDockerTest.java index c3c98cc9da45..3c60d1616297 100644 --- a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/BaseKubernetesTaskRunnerDockerTest.java +++ b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/BaseKubernetesTaskRunnerDockerTest.java @@ -32,13 +32,23 @@ */ abstract class BaseKubernetesTaskRunnerDockerTest extends IngestionSmokeTest implements LatestImageDockerTest { - protected static final String MANIFEST_TEMPLATE = "manifests/druid-service-with-operator.yaml"; + private static final String DEFAULT_MANIFEST_TEMPLATE = "manifests/druid-service-with-operator.yaml"; + + protected K3sClusterResource k3sCluster; /** * Subclasses override to enable/disable SharedInformer caching. */ protected abstract boolean useSharedInformers(); + /** + * Subclasses override to swap in a different operator manifest template. + */ + protected String getManifestTemplate() + { + return DEFAULT_MANIFEST_TEMPLATE; + } + @Override protected EmbeddedDruidCluster addServers(EmbeddedDruidCluster cluster) { @@ -54,9 +64,9 @@ protected EmbeddedDruidCluster addServers(EmbeddedDruidCluster cluster) .addProperty("druid.indexer.runner.k8sSharedInformerResyncPeriod", "PT1s") .usingPort(30090); - final K3sClusterResource k3sCluster = new K3sClusterWithOperatorResource() + this.k3sCluster = new K3sClusterWithOperatorResource() .usingDruidTestImage() - .usingDruidManifestTemplate(MANIFEST_TEMPLATE) + .usingDruidManifestTemplate(getManifestTemplate()) .addService(new K3sDruidService(DruidCommand.Server.COORDINATOR).usingPort(30081)) .addService(overlordService) .addService(new K3sDruidService(DruidCommand.Server.HISTORICAL).usingPort(30083)) diff --git a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/K3sClusterResource.java b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/K3sClusterResource.java index 41636afa62d5..f47bf3a7b32a 100644 --- a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/K3sClusterResource.java +++ b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/K3sClusterResource.java @@ -211,6 +211,15 @@ protected void waitUntilPodsAreReady(String namespace) client.pods().inNamespace(namespace).resources().forEach(this::waitUntilPodIsReady); } + /** + * Exposes the fabric8 {@link KubernetesClient} for tests that need to interact + * with the cluster directly (e.g. discover task-launched peon pods). + */ + public KubernetesClient getKubernetesClient() + { + return client; + } + @Override public void stop() { diff --git a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/KubernetesPeonJvmConfigDockerTest.java b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/KubernetesPeonJvmConfigDockerTest.java new file mode 100644 index 000000000000..aa99ac815d75 --- /dev/null +++ b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/KubernetesPeonJvmConfigDockerTest.java @@ -0,0 +1,201 @@ +/* + * 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 com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.fabric8.kubernetes.api.model.OwnerReference; +import io.fabric8.kubernetes.api.model.Pod; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.LocalPortForward; +import org.apache.druid.common.utils.IdUtils; +import org.apache.druid.indexing.common.task.NoopTask; +import org.apache.druid.query.DruidMetrics; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; + +import java.io.InputStream; +import java.net.HttpURLConnection; +import java.net.URL; +import java.util.List; +import java.util.Map; + +/** + * Regression test for https://github.com/apache/druid/issues/18791. + * + * Verifies that {@code distribution/docker/peon.sh} sources options from + * {@code jvm.config} and that those options reach the peon JVM as system + * properties. Before the fix, {@code peon.sh} silently ignored + * {@code jvm.config}, so any JVM flags set there — including memory limits + * users had configured to prevent OOMs — never applied. + * + *

The test uses an operator manifest that injects + * {@code -Ddruid.test.peon.jvmconfig.marker=true} into the cluster-level + * {@code jvm.options}. The Druid operator writes these to {@code jvm.config} + * on each node, including the overlord. When a peon is launched via the + * {@code K8sTaskAdapter}, it inherits the overlord's pod spec (including the + * mounted {@code jvm.config}); {@code peon.sh} then sources that file and + * prepends its contents to {@code JAVA_OPTS}. The marker therefore appears in + * the peon JVM's system properties, which this test asserts by querying + * {@code /status/properties} on the peon pod. + */ +@Disabled("requires charts.datainfra.io chart, see https://github.com/apache/druid/pull/19047") +public class KubernetesPeonJvmConfigDockerTest extends BaseKubernetesTaskRunnerDockerTest +{ + private static final String MARKER_KEY = "druid.test.peon.jvmconfig.marker"; + private static final String MARKER_VALUE = "true"; + private static final String MARKER_MANIFEST = + "manifests/druid-service-with-operator-peonjvmconfig.yaml"; + + /** + * Matches {@code DruidK8sConstants.PORT} but duplicated here to avoid + * pulling the whole {@code druid-kubernetes-overlord-extensions} module in + * as a test-scope dep just for one integer. + */ + private static final int PEON_HTTP_PORT = 8100; + + private static final long PEON_POD_READY_TIMEOUT_MILLIS = 180_000L; + private static final long PROPERTIES_POLL_TIMEOUT_MILLIS = 60_000L; + + private static final ObjectMapper MAPPER = new ObjectMapper(); + private static final TypeReference> MAP_TYPE = new TypeReference<>() {}; + + @Override + protected boolean useSharedInformers() + { + return false; + } + + @Override + protected String getManifestTemplate() + { + return MARKER_MANIFEST; + } + + @Test + public void test_peonSourcesJvmConfigMarker() throws Exception + { + final String taskId = IdUtils.getRandomId(); + // Keep the peon alive long enough to discover its pod, port-forward, and hit the status endpoint. + final long runDurationMillis = 240_000L; + + cluster.callApi().onLeaderOverlord( + o -> o.runTask( + taskId, + new NoopTask(taskId, null, dataSource, runDurationMillis, 0L, null) + ) + ); + + try { + eventCollector.latchableEmitter().waitForEvent( + event -> event.hasMetricName(NoopTask.EVENT_STARTED) + .hasDimension(DruidMetrics.TASK_ID, taskId) + ); + + final KubernetesClient client = k3sCluster.getKubernetesClient(); + final Pod peonPod = waitForReadyPeonPod(client); + + try (LocalPortForward portForward = client.pods() + .inNamespace(K3sClusterResource.DRUID_NAMESPACE) + .withName(peonPod.getMetadata().getName()) + .portForward(PEON_HTTP_PORT)) { + final Map peonProperties = pollForStatusProperties(portForward.getLocalPort()); + Assertions.assertEquals( + MARKER_VALUE, + peonProperties.get(MARKER_KEY), + "Expected jvm.config marker to reach peon JVM as a system property. " + + "This is a regression: peon.sh must source $SERVICE_CONF_DIR/jvm.config." + ); + } + } + finally { + try { + cluster.callApi().onLeaderOverlord(o -> o.cancelTask(taskId)); + } + catch (Exception ignore) { + // Best-effort cleanup. + } + } + } + + private Pod waitForReadyPeonPod(KubernetesClient client) throws InterruptedException + { + final long deadline = System.currentTimeMillis() + PEON_POD_READY_TIMEOUT_MILLIS; + while (System.currentTimeMillis() < deadline) { + for (Pod pod : client.pods().inNamespace(K3sClusterResource.DRUID_NAMESPACE).list().getItems()) { + if (!ownedByJob(pod)) { + continue; + } + if (isReady(pod)) { + return pod; + } + } + Thread.sleep(2_000L); + } + throw new AssertionError( + "No Job-owned pod became Ready within " + + (PEON_POD_READY_TIMEOUT_MILLIS / 1000L) + "s — expected a peon pod to appear" + ); + } + + private static boolean ownedByJob(Pod pod) + { + final List owners = pod.getMetadata().getOwnerReferences(); + return owners != null && owners.stream().anyMatch(o -> "Job".equals(o.getKind())); + } + + private static boolean isReady(Pod pod) + { + return pod.getStatus() != null + && pod.getStatus().getConditions() != null + && pod.getStatus().getConditions().stream().anyMatch( + c -> "Ready".equals(c.getType()) && "True".equals(c.getStatus()) + ); + } + + private Map pollForStatusProperties(int localPort) throws InterruptedException + { + final long deadline = System.currentTimeMillis() + PROPERTIES_POLL_TIMEOUT_MILLIS; + Exception lastException = null; + while (System.currentTimeMillis() < deadline) { + try { + final URL url = new URL("http://localhost:" + localPort + "/status/properties"); + final HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + conn.setConnectTimeout(2_000); + conn.setReadTimeout(2_000); + if (conn.getResponseCode() == 200) { + try (InputStream is = conn.getInputStream()) { + return MAPPER.readValue(is, MAP_TYPE); + } + } + } + catch (Exception e) { + lastException = e; + } + Thread.sleep(1_000L); + } + final String suffix = lastException == null ? "" : " Last error: " + lastException; + throw new AssertionError( + "Peon /status/properties did not return 200 within " + + (PROPERTIES_POLL_TIMEOUT_MILLIS / 1000L) + "s." + suffix + ); + } +} diff --git a/embedded-tests/src/test/resources/manifests/druid-service-with-operator-peonjvmconfig.yaml b/embedded-tests/src/test/resources/manifests/druid-service-with-operator-peonjvmconfig.yaml new file mode 100644 index 000000000000..c380822fe0a6 --- /dev/null +++ b/embedded-tests/src/test/resources/manifests/druid-service-with-operator-peonjvmconfig.yaml @@ -0,0 +1,115 @@ +apiVersion: "druid.apache.org/v1alpha1" +kind: "Druid" +metadata: + name: test-cluster-${service} +spec: + image: ${image} + startScript: /druid.sh + scalePvcSts: true + rollingDeploy: true + defaultProbes: false + podLabels: + environment: stage + release: alpha + podAnnotations: + dummy: k8s_extn_needs_atleast_one_annotation + volumes: + - name: mysqlconnector + emptyDir: { } + securityContext: + fsGroup: 0 + runAsUser: 0 + runAsGroup: 0 + containerSecurityContext: + privileged: true + commonConfigMountPath: "/opt/druid/conf/druid/cluster/_common" + common.runtime.properties: | +${commonRuntimeProperties} + jvm.options: |- + -server + -Djava.net.preferIPv4Stack=true + -XX:MaxDirectMemorySize=10240g + -Duser.timezone=UTC + -Dfile.encoding=UTF-8 + -Dlog4j.debug + -Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager + -Ddruid.test.peon.jvmconfig.marker=true + log4j.config: |- + + + + + + + + + + + + + + + + + + env: + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + nodes: + ${service}s: + nodeType: ${service} + priorityClassName: system-cluster-critical + druid.port: ${port} + services: + - spec: + type: NodePort + ports: + - name: http + port: ${port} + targetPort: ${port} + nodePort: ${port} + replicas: 1 + nodeConfigMountPath: "/opt/druid/conf/druid/cluster/${serviceFolder}" + runtime.properties: | +${nodeRuntimeProperties} + livenessProbe: + failureThreshold: 10 + httpGet: + path: /status/health + port: ${port} + initialDelaySeconds: 5 + periodSeconds: 10 + successThreshold: 1 + timeoutSeconds: 5 + readinessProbe: + failureThreshold: 20 + httpGet: + path: /status/health + port: ${port} + initialDelaySeconds: 5 + periodSeconds: 10 + successThreshold: 1 + timeoutSeconds: 5 + startUpProbe: + failureThreshold: 20 + httpGet: + path: /status/health + port: ${port} + initialDelaySeconds: 60 + periodSeconds: 30 + successThreshold: 1 + timeoutSeconds: 10 + volumeMounts: + - mountPath: /druid/data + name: druid-shared-storage + volumes: + - name: druid-shared-storage + hostPath: + path: /druid/shared-storage + type: DirectoryOrCreate From 2f7ee8aa817490f1371dda71cb3185e0d18ae94e Mon Sep 17 00:00:00 2001 From: Lewis Tierney Date: Wed, 22 Apr 2026 19:55:44 +0100 Subject: [PATCH 3/5] fix: spelling mistake --- docs/development/extensions-core/k8s-jobs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/extensions-core/k8s-jobs.md b/docs/development/extensions-core/k8s-jobs.md index 0346348c7ff8..f5a9e73b0818 100644 --- a/docs/development/extensions-core/k8s-jobs.md +++ b/docs/development/extensions-core/k8s-jobs.md @@ -483,7 +483,7 @@ template: Any runtime property or JVM config used by the peon process can also be passed. E.G. below is an example of a ConfigMap that can be used to generate the `nodetype-config-volume` mount in the above template. -The peon container startup script (`peon.sh`) reads `jvm.config` from the directory mounted as `nodetype-config-volume` and prepends its contents to the peon's `JAVA_OPTS` before launching the JVM. If an option is set both in `jvm.config` and in `JAVA_OPTS` (for example via `druid.indexer.runner.javaOptsArray` when using the `overlordSingleContainer` or `overlordMultiContainer` adapters, which inject `JAVA_OPTS` as a container env var), the `JAVA_OPTS` value takes precedence under OpenJDK. +The peon container startup script (`peon.sh`) reads `jvm.config` from the directory mounted as `nodetype-config-volume` and prepends its contents to the peon's `JAVA_OPTS` before launching the JVM. If an option is set both in `jvm.config` and in `JAVA_OPTS` (for example via `druid.indexer.runner.javaOptsArray` when using the `overlordSingleContainer` or `overlordMultiContainer` adapters, which inject `JAVA_OPTS` as a container environment variable), the `JAVA_OPTS` value takes precedence under OpenJDK.

Example ConfigMap From 272f1b9d8d572414049d318c73ecd0381987f0a2 Mon Sep 17 00:00:00 2001 From: Lewis Tierney Date: Wed, 22 Apr 2026 19:58:27 +0100 Subject: [PATCH 4/5] fix: change from deprecated URL.URL --- .../embedded/k8s/KubernetesPeonJvmConfigDockerTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/KubernetesPeonJvmConfigDockerTest.java b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/KubernetesPeonJvmConfigDockerTest.java index aa99ac815d75..d5de603756c1 100644 --- a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/KubernetesPeonJvmConfigDockerTest.java +++ b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/k8s/KubernetesPeonJvmConfigDockerTest.java @@ -34,6 +34,7 @@ import java.io.InputStream; import java.net.HttpURLConnection; +import java.net.URI; import java.net.URL; import java.util.List; import java.util.Map; @@ -177,7 +178,7 @@ private Map pollForStatusProperties(int localPort) throws Interr Exception lastException = null; while (System.currentTimeMillis() < deadline) { try { - final URL url = new URL("http://localhost:" + localPort + "/status/properties"); + final URL url = URI.create("http://localhost:" + localPort + "/status/properties").toURL(); final HttpURLConnection conn = (HttpURLConnection) url.openConnection(); conn.setConnectTimeout(2_000); conn.setReadTimeout(2_000); From f62f9e7ebe052f2b696852957a33e9f5ceae63de Mon Sep 17 00:00:00 2001 From: Lewis Tierney Date: Tue, 28 Apr 2026 01:30:18 +0100 Subject: [PATCH 5/5] test: add active script-level test for peon.sh jvm.config sourcing --- .../k8s/overlord/PeonShellJvmConfigTest.java | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 extensions-core/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/PeonShellJvmConfigTest.java diff --git a/extensions-core/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/PeonShellJvmConfigTest.java b/extensions-core/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/PeonShellJvmConfigTest.java new file mode 100644 index 000000000000..7ef84b58a09f --- /dev/null +++ b/extensions-core/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/PeonShellJvmConfigTest.java @@ -0,0 +1,158 @@ +/* + * 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.k8s.overlord; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Active regression coverage for the {@code peon.sh} jvm.config sourcing fix + * (https://github.com/apache/druid/issues/18791). + * + * The full end-to-end test + * ({@code KubernetesPeonJvmConfigDockerTest} under {@code embedded-tests}) is + * {@code @Disabled} alongside its sibling K8s tests pending the + * {@code charts.datainfra.io} chart availability issue (see #19047), so it + * does not run in CI. This test exercises the actual sourcing logic from + * {@code distribution/docker/peon.sh} in a sandboxed shell so a regression is + * still caught while the heavier test stays disabled. + */ +public class PeonShellJvmConfigTest +{ + /** + * Maven runs tests with CWD set to the module directory, so this resolves + * to the repository's {@code distribution/docker/peon.sh}. + */ + private static final Path PEON_SH = Paths.get("../../distribution/docker/peon.sh"); + + /** + * Exact snippet that {@code peon.sh} now contains. Kept verbatim so a drift + * from {@code peon.sh} surfaces as a failure of + * {@link #test_peonShFileContainsJvmConfigSourcing}. + */ + private static final String JVM_CONFIG_SOURCING_SNIPPET = + "if [ -f \"$SERVICE_CONF_DIR/jvm.config\" ]; then\n" + + " JAVA_OPTS=\"$(cat $SERVICE_CONF_DIR/jvm.config | xargs) $JAVA_OPTS\"\n" + + "fi\n"; + + /** + * Verifies the jvm.config sourcing block prepends {@code jvm.config} + * contents to {@code JAVA_OPTS}, leaving the existing {@code JAVA_OPTS} + * after — so flags duplicated between the two paths still let + * {@code JAVA_OPTS} win under OpenJDK precedence. + */ + @Test + public void test_peonShPrependsJvmConfigContentsToJavaOpts(@TempDir Path tempDir) throws Exception + { + Files.writeString( + tempDir.resolve("jvm.config"), + String.join( + "\n", + "-Xmx2g", + "-XX:MaxDirectMemorySize=500m", + "-Dpeon.test.marker=fromJvmConfig" + ) + ); + + final String script = JVM_CONFIG_SOURCING_SNIPPET + "echo \"$JAVA_OPTS\"\n"; + + final ProcessBuilder pb = new ProcessBuilder("sh", "-c", script); + pb.environment().put("SERVICE_CONF_DIR", tempDir.toString()); + pb.environment().put("JAVA_OPTS", "-Dpeon.test.marker=fromJavaOpts -Dexisting=true"); + pb.redirectErrorStream(true); + + final Process p = pb.start(); + final String output = new String(p.getInputStream().readAllBytes(), StandardCharsets.UTF_8).trim(); + assertEquals(0, p.waitFor(), "Snippet exited non-zero. Output: " + output); + + assertTrue(output.contains("-Xmx2g"), output); + assertTrue(output.contains("-XX:MaxDirectMemorySize=500m"), output); + assertTrue(output.contains("-Dexisting=true"), output); + + // jvm.config options are prepended; existing JAVA_OPTS stays after. + final int xmxIdx = output.indexOf("-Xmx2g"); + final int existingIdx = output.indexOf("-Dexisting=true"); + assertTrue( + xmxIdx >= 0 && xmxIdx < existingIdx, + "Expected jvm.config options before existing JAVA_OPTS, got: " + output + ); + + // Both occurrences of the marker key are present, and the JAVA_OPTS value + // appears later in the string so it wins under OpenJDK precedence. + final int fromJvmIdx = output.indexOf("-Dpeon.test.marker=fromJvmConfig"); + final int fromJavaIdx = output.indexOf("-Dpeon.test.marker=fromJavaOpts"); + assertTrue(fromJvmIdx >= 0, "jvm.config marker missing from: " + output); + assertTrue(fromJavaIdx >= 0, "JAVA_OPTS marker missing from: " + output); + assertTrue( + fromJvmIdx < fromJavaIdx, + "Expected jvm.config marker before JAVA_OPTS marker (so JAVA_OPTS wins): " + output + ); + } + + /** + * The {@code -f} guard is the only deviation from {@code druid.sh}; without + * it, peon containers without a mounted {@code jvm.config} would emit + * {@code cat: ... No such file or directory} on every startup. Verify the + * snippet leaves {@code JAVA_OPTS} untouched in that case. + */ + @Test + public void test_peonShDoesNothingWhenJvmConfigAbsent(@TempDir Path tempDir) throws Exception + { + final String script = JVM_CONFIG_SOURCING_SNIPPET + "echo \"$JAVA_OPTS\"\n"; + + final ProcessBuilder pb = new ProcessBuilder("sh", "-c", script); + pb.environment().put("SERVICE_CONF_DIR", tempDir.toString()); + pb.environment().put("JAVA_OPTS", "-Dexisting=true"); + pb.redirectErrorStream(true); + + final Process p = pb.start(); + final String output = new String(p.getInputStream().readAllBytes(), StandardCharsets.UTF_8).trim(); + assertEquals(0, p.waitFor()); + assertEquals( + "-Dexisting=true", + output, + "JAVA_OPTS must be unchanged when no jvm.config is mounted" + ); + } + + /** + * Static guard — {@code peon.sh} must continue to contain the jvm.config + * sourcing logic so a naive removal of the fix is caught even if the + * snippet test above is somehow stubbed out. + */ + @Test + public void test_peonShFileContainsJvmConfigSourcing() throws IOException + { + final String content = Files.readString(PEON_SH); + assertTrue( + content.contains(JVM_CONFIG_SOURCING_SNIPPET), + "peon.sh must contain the jvm.config sourcing block. See #18791." + ); + } +}