Ensure signals are passed to commands (#3548) (#3558)

* Ensure signals are passed to commands Change `/bin/sh -ec "<command>"` to `/bin/sh -ec "exec <command>"`. Adding `exec` ensures that `<command>` is not executed as a child process but replaces the `/bin/sh` process. This ensure that `<command>` receives any signals. Specifically this is an issue when attempting to trap SIGTERMs as part of graceful pod shutdown. Without this change, we weren't receiving any signals because they aren't passed down by `/bin/sh -c`. * Fix broken bats tests and add changelog --------- Signed-off-by: Ashwin Venkatesh <[email protected]> Co-authored-by: Luke Kysow <[email protected]>
hashicorp · Feb 6, 2024 · f053e43 · f053e43
1 parent bf679c1
commit f053e43
Show file tree

Hide file tree

Showing 19 changed files with 36 additions and 35 deletions.
diff --git a/.changelog/3548.txt b/.changelog/3548.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+helm: Change `/bin/sh -ec "<command>"` to `/bin/sh -ec "exec <command>"` in helm deployments
+```
diff --git a/charts/consul/templates/api-gateway-controller-deployment.yaml b/charts/consul/templates/api-gateway-controller-deployment.yaml
@@ -142,7 +142,7 @@ spec:
         - "/bin/sh"
         - "-ec"
         - |
-          consul-api-gateway server \
+          exec consul-api-gateway server \
             -sds-server-host {{ template "consul.fullname" . }}-api-gateway-controller.{{ .Release.Namespace }}.svc \
             -k8s-namespace {{ .Release.Namespace }} \
             {{- if .Values.global.enableConsulNamespaces }}
@@ -188,7 +188,7 @@ spec:
         lifecycle:
           preStop:
             exec:
-              command: [ "/bin/sh", "-ec", "/consul-bin/consul logout" ]
+              command: ["/consul-bin/consul", "logout" ]
         {{- end }}
       volumes:
       {{- if .Values.global.acls.manageSystemACLs }}
@@ -274,7 +274,7 @@ spec:
         - "/bin/sh"
         - "-ec"
         - |
-          consul-k8s-control-plane acl-init \
+          exec consul-k8s-control-plane acl-init \
             {{- if and .Values.global.federation.enabled .Values.global.federation.primaryDatacenter }}
             -auth-method-name={{ template "consul.fullname" . }}-k8s-component-auth-method-{{ .Values.global.datacenter }} \
             {{- else }}

diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml
@@ -202,10 +202,8 @@ spec:
             preStop:
               exec:
                 command:
-                  - "/bin/sh"
-                  - "-ec"
-                  - |
-                    consul logout
+                  - "/bin/consul"
+                  - "logout"
           {{- end }}
           env:
             {{- if .Values.global.acls.manageSystemACLs }}
@@ -523,7 +521,7 @@ spec:
           - "/bin/sh"
           - "-ec"
           - |
-            consul-k8s-control-plane acl-init \
+            exec consul-k8s-control-plane acl-init \
               -log-level={{ default .Values.global.logLevel .Values.client.logLevel }} \
               -log-json={{ .Values.global.logJSON }} \
               -init-type="client"

diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml
@@ -138,7 +138,7 @@ spec:
             - "/bin/sh"
             - "-ec"
             - |
-              consul-k8s-control-plane inject-connect \
+              exec consul-k8s-control-plane inject-connect \
                 {{- if .Values.global.federation.enabled }}
                 -enable-federation \
                 {{- end }}

diff --git a/charts/consul/templates/create-federation-secret-job.yaml b/charts/consul/templates/create-federation-secret-job.yaml
@@ -119,7 +119,7 @@ spec:
             - "/bin/sh"
             - "-ec"
             - |
-                consul-k8s-control-plane create-federation-secret \
+               exec consul-k8s-control-plane create-federation-secret \
                   -log-level={{ default .Values.global.logLevel .Values.global.federation.logLevel }} \
                   -log-json={{ .Values.global.logJSON }} \
                   {{- if (or .Values.global.gossipEncryption.autoGenerate (and .Values.global.gossipEncryption.secretName .Values.global.gossipEncryption.secretKey)) }}

diff --git a/charts/consul/templates/enterprise-license-job.yaml b/charts/consul/templates/enterprise-license-job.yaml
@@ -128,7 +128,7 @@ spec:
           - "/bin/sh"
           - "-ec"
           - |
-            consul-k8s-control-plane acl-init \
+            exec consul-k8s-control-plane acl-init \
               -secret-name="{{ template "consul.fullname" . }}-enterprise-license-acl-token" \
               -k8s-namespace={{ .Release.Namespace }} \
               -consul-api-timeout={{ .Values.global.consulAPITimeout }}

diff --git a/charts/consul/templates/gossip-encryption-autogenerate-job.yaml b/charts/consul/templates/gossip-encryption-autogenerate-job.yaml
@@ -53,7 +53,7 @@ spec:
             - "/bin/sh"
             - "-ec"
             - |
-              consul-k8s-control-plane gossip-encryption-autogenerate \
+              exec consul-k8s-control-plane gossip-encryption-autogenerate \
                 -namespace={{ .Release.Namespace }} \
                 -secret-name={{ template "consul.fullname" . }}-gossip-encryption-key \
                 -secret-key="key" \

diff --git a/charts/consul/templates/ingress-gateways-deployment.yaml b/charts/consul/templates/ingress-gateways-deployment.yaml
@@ -211,12 +211,12 @@ spec:
         - "/bin/sh"
         - "-ec"
         - |
-          consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${NAMESPACE} \
-          -gateway-kind="ingress-gateway" \
-          -proxy-id-file=/consul/service/proxy-id \
-          -service-name={{ template "consul.fullname" $root }}-{{ .name }} \
-          -log-level={{ default $root.Values.global.logLevel $root.Values.ingressGateways.logLevel }} \
-          -log-json={{ $root.Values.global.logJSON }}
+          exec consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${NAMESPACE} \
+            -gateway-kind="ingress-gateway" \
+            -proxy-id-file=/consul/service/proxy-id \
+            -service-name={{ template "consul.fullname" $root }}-{{ .name }} \
+            -log-level={{ default $root.Values.global.logLevel $root.Values.ingressGateways.logLevel }} \
+            -log-json={{ $root.Values.global.logJSON }}
         volumeMounts:
         - name: consul-service
           mountPath: /consul/service

diff --git a/charts/consul/templates/mesh-gateway-deployment.yaml b/charts/consul/templates/mesh-gateway-deployment.yaml
@@ -160,12 +160,12 @@ spec:
         - "/bin/sh"
         - "-ec"
         - |
-          consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${NAMESPACE} \
-           -gateway-kind="mesh-gateway" \
-           -proxy-id-file=/consul/service/proxy-id \
-           -service-name={{ .Values.meshGateway.consulServiceName }} \
-           -log-level={{ default .Values.global.logLevel .Values.meshGateway.logLevel }} \
-           -log-json={{ .Values.global.logJSON }}
+          exec consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${NAMESPACE} \
+            -gateway-kind="mesh-gateway" \
+            -proxy-id-file=/consul/service/proxy-id \
+            -service-name={{ .Values.meshGateway.consulServiceName }} \
+            -log-level={{ default .Values.global.logLevel .Values.meshGateway.logLevel }} \
+            -log-json={{ .Values.global.logJSON }}
         volumeMounts:
         - name: consul-service
           mountPath: /consul/service

diff --git a/charts/consul/templates/partition-init-job.yaml b/charts/consul/templates/partition-init-job.yaml
@@ -111,7 +111,7 @@ spec:
             - "/bin/sh"
             - "-ec"
             - |
-              consul-k8s-control-plane partition-init \
+              exec consul-k8s-control-plane partition-init \
                 -log-level={{ .Values.global.logLevel }} \
                 -log-json={{ .Values.global.logJSON }} \
                 {{- if .Values.global.cloud.enabled }}

diff --git a/charts/consul/templates/server-acl-init-job.yaml b/charts/consul/templates/server-acl-init-job.yaml
@@ -180,7 +180,7 @@ spec:
         - |
           CONSUL_FULLNAME="{{template "consul.fullname" . }}"
 
-          consul-k8s-control-plane server-acl-init \
+          exec consul-k8s-control-plane server-acl-init \
             -log-level={{ default .Values.global.logLevel .Values.global.acls.logLevel}} \
             -log-json={{ .Values.global.logJSON }} \
             -resource-prefix=${CONSUL_FULLNAME} \

diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml
@@ -249,7 +249,7 @@ spec:
           - "/bin/sh"
           - "-ec"
           - |
-            consul-k8s-control-plane fetch-server-region -node-name "$NODE_NAME" -output-file /consul/extra-config/locality.json
+            exec consul-k8s-control-plane fetch-server-region -node-name "$NODE_NAME" -output-file /consul/extra-config/locality.json
         volumeMounts:
           - name: extra-config
             mountPath: /consul/extra-config

diff --git a/charts/consul/templates/sync-catalog-deployment.yaml b/charts/consul/templates/sync-catalog-deployment.yaml
@@ -122,7 +122,7 @@ spec:
         - "/bin/sh"
         - "-ec"
         - |
-          consul-k8s-control-plane sync-catalog \
+          exec consul-k8s-control-plane sync-catalog \
             -log-level={{ default .Values.global.logLevel .Values.syncCatalog.logLevel }} \
             -log-json={{ .Values.global.logJSON }} \
             -k8s-default-sync={{ .Values.syncCatalog.default }} \

diff --git a/charts/consul/templates/telemetry-collector-deployment.yaml b/charts/consul/templates/telemetry-collector-deployment.yaml
@@ -133,7 +133,7 @@ spec:
           - /bin/sh
           - -ec
           - |-
-            consul-k8s-control-plane connect-init \
+            exec consul-k8s-control-plane connect-init \
               -log-json={{ .Values.global.logJSON }} \
               -log-level={{ default .Values.global.logLevel .Values.telemetryCollector.logLevel }} \
               -pod-name=${POD_NAME} \
@@ -261,7 +261,7 @@ spec:
           {{- end }}
           {{- end }}
 
-          consul-telemetry-collector agent \
+          exec consul-telemetry-collector agent \
           {{- if .Values.telemetryCollector.customExporterConfig }}
             -config-file-path /consul/config/config.json \
           {{ end }}

diff --git a/charts/consul/templates/terminating-gateways-deployment.yaml b/charts/consul/templates/terminating-gateways-deployment.yaml
@@ -196,7 +196,7 @@ spec:
             - "/bin/sh"
             - "-ec"
             - |
-                consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${NAMESPACE} \
+              exec consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${NAMESPACE} \
                   -gateway-kind="terminating-gateway" \
                   -proxy-id-file=/consul/service/proxy-id \
                   -service-name={{ .name }} \

diff --git a/charts/consul/templates/tls-init-job.yaml b/charts/consul/templates/tls-init-job.yaml
@@ -79,7 +79,7 @@ spec:
               # Suppress globbing so we can interpolate the $NAMESPACE environment variable
               # and use * at the start of the dns name when setting -additional-dnsname.
               set -o noglob
-              consul-k8s-control-plane tls-init \
+              exec consul-k8s-control-plane tls-init \
                 -log-level={{ default .Values.global.logLevel .Values.global.tls.logLevel }} \
                 -log-json={{ .Values.global.logJSON }} \
                 -domain={{ .Values.global.domain }} \

diff --git a/charts/consul/templates/webhook-cert-manager-deployment.yaml b/charts/consul/templates/webhook-cert-manager-deployment.yaml
@@ -43,7 +43,7 @@ spec:
         - "/bin/sh"
         - "-ec"
         - |
-          consul-k8s-control-plane webhook-cert-manager \
+          exec consul-k8s-control-plane webhook-cert-manager \
             -log-level={{ .Values.global.logLevel }} \
             -log-json={{ .Values.global.logJSON }} \
             -config-file=/bootstrap/config/webhook-config.json \

diff --git a/charts/consul/test/unit/api-gateway-controller-deployment.bats b/charts/consul/test/unit/api-gateway-controller-deployment.bats
@@ -272,7 +272,7 @@ load _helpers
       --set 'apiGateway.image=foo' \
       --set 'global.acls.manageSystemACLs=true' \
       . | tee /dev/stderr |
-      yq '[.spec.template.spec.containers[0].lifecycle.preStop.exec.command[2]] | any(contains("consul logout"))' | tee /dev/stderr)
+      yq '[.spec.template.spec.containers[0].lifecycle.preStop.exec.command[1]] | any(contains("logout"))' | tee /dev/stderr)
   [ "${object}" = "true" ]
 }
 

diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats
@@ -1441,7 +1441,7 @@ load _helpers
       --set 'client.enabled=true' \
       --set 'global.acls.manageSystemACLs=true' \
       . | tee /dev/stderr |
-      yq '[.spec.template.spec.containers[0].lifecycle.preStop.exec.command[2]] | any(contains("consul logout"))' | tee /dev/stderr)
+      yq '[.spec.template.spec.containers[0].lifecycle.preStop.exec.command[1]] | any(contains("logout"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }