Fix shutdown with jmx javaagent enabled (#899)

* Fix shutdown with jmx javaagent enabled * Skip CATALINA_OPTS for Bitbucket * Fix unit tests --------- Co-authored-by: Yevhen Ivantsov <[email protected]>
atlassian · Oct 30, 2024 · 1ef5e0b · 1ef5e0b
1 parent 139b31f
commit 1ef5e0b
Show file tree

Hide file tree

Showing 15 changed files with 57 additions and 25 deletions.
diff --git a/src/main/charts/bamboo/templates/config-jvm.yaml b/src/main/charts/bamboo/templates/config-jvm.yaml
@@ -13,6 +13,5 @@ data:
     {{- if or .Values.bamboo.additionalCertificates.secretName .Values.bamboo.additionalCertificates.secretList }}
     -Djavax.net.ssl.trustStore=/var/ssl/cacerts
     {{- end }}
-    {{ include "common.jmx.javaagent" . | indent 4 | trim }}
   max_heap: {{ .Values.bamboo.resources.jvm.maxHeap }}
   min_heap: {{ .Values.bamboo.resources.jvm.minHeap }}
diff --git a/src/main/charts/bamboo/templates/statefulset.yaml b/src/main/charts/bamboo/templates/statefulset.yaml
@@ -202,6 +202,10 @@ spec:
             {{ end }}
             - name: ATL_BAMBOO_ENABLE_UNATTENDED_SETUP
               value: {{ .Values.bamboo.unattendedSetup | quote }}
+            {{- if .Values.monitoring.exposeJmxMetrics }}
+            - name: CATALINA_OPTS
+              value: {{ include "common.jmx.javaagent" . | replace "\n" "" | quote }}
+            {{- end }}
             {{- include "bamboo.additionalEnvironmentVariables" . | nindent 12 }}
           ports:
             - name: http

diff --git a/src/main/charts/confluence/templates/config-jvm.yaml b/src/main/charts/confluence/templates/config-jvm.yaml
@@ -22,7 +22,6 @@ data:
     {{- if or .Values.confluence.additionalCertificates.secretName .Values.confluence.additionalCertificates.secretList }}
     -Djavax.net.ssl.trustStore=/var/ssl/cacerts
     {{- end }}
-    {{- include "common.jmx.javaagent" . | indent 4 -}}
     {{- include "confluence.sysprop.s3Config" . | indent 4 }}
     {{- if .Values.opensearch.enabled }}
     -Dsearch.platform=opensearch

diff --git a/src/main/charts/confluence/templates/statefulset.yaml b/src/main/charts/confluence/templates/statefulset.yaml
@@ -263,6 +263,10 @@ spec:
                 configMapKeyRef:
                   key: reserved_code_cache
                   name: {{ include "common.names.fullname" . }}-jvm-config
+            {{- if .Values.monitoring.exposeJmxMetrics }}
+            - name: CATALINA_OPTS
+              value: {{ include "common.jmx.javaagent" . | replace "\n" "" | quote }}
+            {{- end }}
             {{- include "confluence.additionalEnvironmentVariables" . | nindent 12 }}
           lifecycle:
           {{- if .Values.confluence.postStart.command }}

diff --git a/src/main/charts/crowd/templates/config-jvm.yaml b/src/main/charts/crowd/templates/config-jvm.yaml
@@ -15,6 +15,5 @@ data:
     {{- if or .Values.crowd.additionalCertificates.secretName .Values.crowd.additionalCertificates.secretList }}
     -Djavax.net.ssl.trustStore=/var/ssl/cacerts
     {{- end }}
-    {{ include "common.jmx.javaagent" . | indent 4 | trim }}
   max_heap: {{ .Values.crowd.resources.jvm.maxHeap }}
   min_heap: {{ .Values.crowd.resources.jvm.minHeap }}
diff --git a/src/main/charts/crowd/templates/statefulset.yaml b/src/main/charts/crowd/templates/statefulset.yaml
@@ -231,6 +231,10 @@ spec:
                 configMapKeyRef:
                   key: max_heap
                   name: {{ include "common.names.fullname" . }}-jvm-config
+            {{- if .Values.monitoring.exposeJmxMetrics }}
+            - name: CATALINA_OPTS
+              value: {{ include "common.jmx.javaagent" . | replace "\n" "" | quote }}
+            {{- end }}
             {{- include "crowd.additionalEnvironmentVariables" . | nindent 12 }}
           lifecycle:
           {{- if .Values.crowd.postStart.command }}

diff --git a/src/main/charts/jira/templates/config-jvm.yaml b/src/main/charts/jira/templates/config-jvm.yaml
@@ -17,7 +17,7 @@ data:
     {{- if or .Values.jira.additionalCertificates.secretName .Values.jira.additionalCertificates.secretList }}
     -Djavax.net.ssl.trustStore=/var/ssl/cacerts
     {{- end }}
-    {{ include "common.jmx.javaagent" . | indent 4 | trim }}
+
   max_heap: {{ .Values.jira.resources.jvm.maxHeap }}
   min_heap: {{ .Values.jira.resources.jvm.minHeap }}
   reserved_code_cache: {{ .Values.jira.resources.jvm.reservedCodeCache }}
diff --git a/src/main/charts/jira/templates/statefulset.yaml b/src/main/charts/jira/templates/statefulset.yaml
@@ -160,6 +160,10 @@ spec:
                 configMapKeyRef:
                   key: reserved_code_cache
                   name: {{ include "common.names.fullname" . }}-jvm-config
+            {{- if .Values.monitoring.exposeJmxMetrics }}
+            - name: CATALINA_OPTS
+              value: {{ include "common.jmx.javaagent" . | replace "\n" "" | quote }}
+            {{- end }}
             {{- include "jira.additionalEnvironmentVariables" . | nindent 12 }}
           ports:
             - name: http

diff --git a/src/test/java/test/JmxMetricsTest.java b/src/test/java/test/JmxMetricsTest.java
@@ -55,9 +55,13 @@ void expose_jmx_metrics_enabled_init_container(Product product) throws Exception
         // assert no resources are set
         assertThat(statefulSet.getInitContainer("fetch-jmx-exporter").get().path("resources")).isEmpty();
 
-        // assert jvm configmap has javaagent
-        final var jmvConfigMap = resources.getConfigMap(product.getHelmReleaseName() + "-jvm-config").getDataByKey("additional_jvm_args");
-        assertThat(jmvConfigMap).hasTextContaining("-javaagent:"+sharedHomePath+"/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml");
+        if (product.name().equals("bitbucket")) {
+            final var jmvConfigMap = resources.getConfigMap(product.getHelmReleaseName() + "-jvm-config").getDataByKey("additional_jvm_args");
+            assertThat(jmvConfigMap).hasTextContaining("-javaagent:"+sharedHomePath+"/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml");
+        } else {
+            // assert CATALINA_OPTS env var has javaagent
+            statefulSet.getContainer().getEnv().assertHasValue("CATALINA_OPTS", "-javaagent:"+sharedHomePath+"/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml");
+        }
 
         // assert jmx configmap created and has expected config
         final var jmxConfigMap = resources.getConfigMap(product.getHelmReleaseName() + "-jmx-config").getDataByKey("jmx-config.yaml");
@@ -144,9 +148,15 @@ void expose_jmx_metrics_enabled_custom_vol_paths(Product product) throws Excepti
         assertThat(statefulSet.getInitContainer("fetch-jmx-exporter").get().path("args").get(1)).hasTextEqualTo(sharedHomePath);
         assertThat(statefulSet.getInitContainer("fetch-jmx-exporter").get().path("volumeMounts").get(0).path("mountPath")).hasTextEqualTo(sharedHomePath);
         assertThat(statefulSet.getInitContainer("fetch-jmx-exporter").get().path("volumeMounts").get(0).path("subPath")).hasTextEqualTo(product.name());
-        // assert jvm configmap has javaagent
-        final var jmvConfigMap = resources.getConfigMap(product.getHelmReleaseName() + "-jvm-config").getDataByKey("additional_jvm_args");
-        assertThat(jmvConfigMap).hasTextContaining("-javaagent:"+ sharedHomePath + "/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml");
+
+        if (product.name().equals("bitbucket")) {
+            // assert jvm configmap has javaagent
+            final var jmvConfigMap = resources.getConfigMap(product.getHelmReleaseName() + "-jvm-config").getDataByKey("additional_jvm_args");
+            assertThat(jmvConfigMap).hasTextContaining("-javaagent:"+ sharedHomePath + "/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml");
+        } else {
+            // assert CATALINA_OPTS env var has javaagent
+            statefulSet.getContainer().getEnv().assertHasValue("CATALINA_OPTS", "-javaagent:"+ sharedHomePath + "/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml");
+        }
     }
 
     @ParameterizedTest
@@ -181,9 +191,14 @@ void expose_jmx_metrics_enabled_custom_jar(Product product) throws Exception {
         // assert jmx_exporter init container does not exist
         assertThat(statefulSet.getInitContainers().path(0).path("name")).hasTextNotContaining("fetch-jmx-exporter");
 
-        // assert jvm configmap has custom javaagent path and port
-        final var jmvConfigMap = resources.getConfigMap(product.getHelmReleaseName() + "-jvm-config").getDataByKey("additional_jvm_args");
-        assertThat(jmvConfigMap).hasTextContaining("-javaagent:/tmp/custom.jar=9000:/opt/atlassian/jmx/jmx-config.yaml");
+        if (product.name().equals("bitbucket")) {
+            // assert jvm configmap has custom javaagent path and port
+            final var jmvConfigMap = resources.getConfigMap(product.getHelmReleaseName() + "-jvm-config").getDataByKey("additional_jvm_args");
+            assertThat(jmvConfigMap).hasTextContaining("-javaagent:/tmp/custom.jar=9000:/opt/atlassian/jmx/jmx-config.yaml");
+        } else {
+            // assert CATALINA_OPTS has custom javaagent path and port
+            statefulSet.getContainer().getEnv().assertHasValue("CATALINA_OPTS", "-javaagent:/tmp/custom.jar=9000:/opt/atlassian/jmx/jmx-config.yaml");
+        }
     }
 
     @ParameterizedTest
@@ -246,7 +261,7 @@ void expose_jmx_metrics_bitbucket_mesh(Product product) throws Exception {
         assertThat(statefulSet.getContainer(product.name()+"-mesh").getPort("jmx").path("containerPort")).hasValueEqualTo(9999);
         assertThat(statefulSet.getContainer(product.name()+"-mesh").getPort("jmx").path("protocol")).hasTextEqualTo("TCP");
 
-        // assert jmx env var
+        // assert jmx env vars
         statefulSet.getContainer().getEnv().assertHasValue("JMX_ENABLED", "true");
 
         // assert jvm configmap has javaagent

diff --git a/src/test/resources/expected_helm_output/bamboo-agent/output.yaml b/src/test/resources/expected_helm_output/bamboo-agent/output.yaml
@@ -46,7 +46,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config-jvm: 5557e5a72e1652ec4f239a78f58a41dc269636b3b030ab797258dc4892c653ae
+        checksum/config-jvm: d3c6ac2201e94d43ef644ef4d23a9c50f266a07765a8eb61068df067aa032a3d
       labels:
         app.kubernetes.io/name: bamboo-agent
         app.kubernetes.io/instance: unittest-bamboo-agent

diff --git a/src/test/resources/expected_helm_output/bamboo/output.yaml b/src/test/resources/expected_helm_output/bamboo/output.yaml
@@ -25,7 +25,6 @@ metadata:
 data:
   additional_jvm_args: >-
     -XX:ActiveProcessorCount=2
-    -javaagent:/var/atlassian/application-data/shared-home/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml
   max_heap: 1024m
   min_heap: 512m
 ---
@@ -450,7 +449,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config-jvm: 793e38b045381183098c1c2418e59554e2573e0e260a348f539fa37641f6d4a8
+        checksum/config-jvm: 7517342dd38443e26c4ad7d7e72c11d1dbab53a2e833032e46b71df455691778
       labels:
         app.kubernetes.io/name: bamboo
         app.kubernetes.io/instance: unittest-bamboo
@@ -553,6 +552,8 @@ spec:
               value: 
             - name: ATL_BAMBOO_ENABLE_UNATTENDED_SETUP
               value: "true"
+            - name: CATALINA_OPTS
+              value: "-javaagent:/var/atlassian/application-data/shared-home/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml"
           ports:
             - name: http
               containerPort: 8085

diff --git a/src/test/resources/expected_helm_output/bitbucket/output.yaml b/src/test/resources/expected_helm_output/bitbucket/output.yaml
@@ -593,7 +593,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config-jvm: 8c3b2f44bad635bf70048f215c5a5144c8e922ef38df8388a5891b62a78f6b75
+        checksum/config-jvm: 650d3e9216cb50c05b86a652e0085ee25bbf884e0f1d38182e963d3e0d263206
       labels:
         app.kubernetes.io/name: bitbucket-mesh
         app.kubernetes.io/instance: unittest-bitbucket
@@ -714,7 +714,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config-jvm: b0b5f87a88780ab1acc1c714670dcabacc9f6d7984c744c3dca412915c9dfae6
+        checksum/config-jvm: 0047b616a0397cd0eff2856e33ee7ac5ca93c44e860d3912445c6a8f76b83dd8
       labels:
         app.kubernetes.io/name: bitbucket
         app.kubernetes.io/instance: unittest-bitbucket

diff --git a/src/test/resources/expected_helm_output/confluence/output.yaml b/src/test/resources/expected_helm_output/confluence/output.yaml
@@ -30,7 +30,6 @@ data:
     -Dconfluence.clusterNodeName.useHostname=true
     -Datlassian.logging.cloud.enabled=false
     -XX:ActiveProcessorCount=2    
-    -javaagent:/var/atlassian/application-data/shared-home/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml    
   max_heap: 1g
   min_heap: 1g
   reserved_code_cache: 256m
@@ -634,7 +633,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config-jvm: f429d69f3f4349a8244614ce43bb7966bf3bea49ae917572f67fbe7a7bce9bd7
+        checksum/config-jvm: 5ecf6d102ac01540d0b521813ca75d6cdc919f86262d79e1e7bfcd92b3fd488a
       labels:
         app.kubernetes.io/name: confluence
         app.kubernetes.io/instance: unittest-confluence
@@ -739,6 +738,8 @@ spec:
                 configMapKeyRef:
                   key: reserved_code_cache
                   name: unittest-confluence-jvm-config
+            - name: CATALINA_OPTS
+              value: "-javaagent:/var/atlassian/application-data/shared-home/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml"
           lifecycle:
             preStop:
               exec:

diff --git a/src/test/resources/expected_helm_output/crowd/output.yaml b/src/test/resources/expected_helm_output/crowd/output.yaml
@@ -27,7 +27,6 @@ data:
     -Dcluster.node.name=${KUBE_POD_NAME}
     -Datlassian.logging.cloud.enabled=false
     -XX:ActiveProcessorCount=2
-    -javaagent:/var/atlassian/application-data/crowd/shared/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml
   max_heap: 768m
   min_heap: 384m
 ---
@@ -375,7 +374,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config-jvm: fdf627893296baca9c1f8932080860cf429a4b34682274892d2cee0e00396fe4
+        checksum/config-jvm: 9dc9c943d7c40b8c7c70c066dec0ff512e2cff63f8d54f0518bffdb2da6c6c09
       labels:
         app.kubernetes.io/name: crowd
         app.kubernetes.io/instance: unittest-crowd
@@ -479,6 +478,8 @@ spec:
                 configMapKeyRef:
                   key: max_heap
                   name: unittest-crowd-jvm-config
+            - name: CATALINA_OPTS
+              value: "-javaagent:/var/atlassian/application-data/crowd/shared/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml"
           lifecycle:
             preStop:
               exec:

diff --git a/src/test/resources/expected_helm_output/jira/output.yaml b/src/test/resources/expected_helm_output/jira/output.yaml
@@ -26,7 +26,6 @@ data:
   additional_jvm_args: >-
     -Datlassian.logging.cloud.enabled=false
     -XX:ActiveProcessorCount=2
-    -javaagent:/var/atlassian/application-data/shared-home/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml
   max_heap: 768m
   min_heap: 384m
   reserved_code_cache: 512m
@@ -397,7 +396,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config-jvm: f7d87b05e5529a36d388bb9183253cbe5948fe8708847c71fb0d6e5853280c61
+        checksum/config-jvm: 579c934bbac667c917645262df123ff5afe749a2e250e1dbac74789f83e45e1f
       labels:
         app.kubernetes.io/name: jira
         app.kubernetes.io/instance: unittest-jira
@@ -465,6 +464,8 @@ spec:
                 configMapKeyRef:
                   key: reserved_code_cache
                   name: unittest-jira-jvm-config
+            - name: CATALINA_OPTS
+              value: "-javaagent:/var/atlassian/application-data/shared-home/jmx_prometheus_javaagent.jar=9999:/opt/atlassian/jmx/jmx-config.yaml"
           ports:
             - name: http
               containerPort: 8080