Ensure signals are passed to commands (#3548)

* 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]> --------- Signed-off-by: Ashwin Venkatesh <[email protected]> Co-authored-by: Ashwin Venkatesh <[email protected]>
hashicorp · Feb 6, 2024 · a03b9d5 · a03b9d5
1 parent 7cd2605
commit a03b9d5
Show file tree

Hide file tree

Showing 20 changed files with 38 additions and 37 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
@@ -143,7 +143,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 }}
@@ -189,7 +189,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 }}
@@ -275,7 +275,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
@@ -203,10 +203,8 @@ spec:
             preStop:
               exec:
                 command:
-                  - "/bin/sh"
-                  - "-ec"
-                  - |
-                    consul logout
+                  - "/bin/consul"
+                  - "logout"
           {{- end }}
           env:
             {{- if .Values.global.acls.manageSystemACLs }}
@@ -524,7 +522,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
@@ -140,7 +140,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
@@ -120,7 +120,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
@@ -129,7 +129,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
@@ -54,7 +54,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
@@ -212,12 +212,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
@@ -162,12 +162,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
@@ -112,7 +112,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
@@ -181,7 +181,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
@@ -250,7 +250,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
@@ -123,7 +123,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/telemetry-collector-v2-deployment.yaml b/charts/consul/templates/telemetry-collector-v2-deployment.yaml
@@ -122,7 +122,7 @@ spec:
           - /bin/sh
           - -ec
           - |-
-            consul-k8s-control-plane mesh-init -proxy-name=${POD_NAME} \
+            exec consul-k8s-control-plane mesh-init -proxy-name=${POD_NAME} \
               -log-level={{ default .Values.global.logLevel .Values.telemetryCollector.logLevel }} \
               -log-json={{ .Values.global.logJSON }} 
 
@@ -245,7 +245,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
@@ -197,7 +197,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
@@ -80,7 +80,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
@@ -44,7 +44,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
@@ -1445,7 +1445,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" ]
 }