From f053e438e94eaff35f695985ea80d887b148da78 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 6 Feb 2024 14:25:30 -0500 Subject: [PATCH] Ensure signals are passed to commands (#3548) (#3558) * Ensure signals are passed to commands Change `/bin/sh -ec ""` to `/bin/sh -ec "exec "`. Adding `exec` ensures that `` is not executed as a child process but replaces the `/bin/sh` process. This ensure that `` 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 Co-authored-by: Luke Kysow <1034429+lkysow@users.noreply.github.com> --- .changelog/3548.txt | 3 +++ .../templates/api-gateway-controller-deployment.yaml | 6 +++--- charts/consul/templates/client-daemonset.yaml | 8 +++----- .../consul/templates/connect-inject-deployment.yaml | 2 +- .../templates/create-federation-secret-job.yaml | 2 +- charts/consul/templates/enterprise-license-job.yaml | 2 +- .../gossip-encryption-autogenerate-job.yaml | 2 +- .../templates/ingress-gateways-deployment.yaml | 12 ++++++------ charts/consul/templates/mesh-gateway-deployment.yaml | 12 ++++++------ charts/consul/templates/partition-init-job.yaml | 2 +- charts/consul/templates/server-acl-init-job.yaml | 2 +- charts/consul/templates/server-statefulset.yaml | 2 +- charts/consul/templates/sync-catalog-deployment.yaml | 2 +- .../templates/telemetry-collector-deployment.yaml | 4 ++-- .../templates/terminating-gateways-deployment.yaml | 2 +- charts/consul/templates/tls-init-job.yaml | 2 +- .../templates/webhook-cert-manager-deployment.yaml | 2 +- .../test/unit/api-gateway-controller-deployment.bats | 2 +- charts/consul/test/unit/client-daemonset.bats | 2 +- 19 files changed, 36 insertions(+), 35 deletions(-) create mode 100644 .changelog/3548.txt diff --git a/.changelog/3548.txt b/.changelog/3548.txt new file mode 100644 index 0000000000..f859d3e94d --- /dev/null +++ b/.changelog/3548.txt @@ -0,0 +1,3 @@ +```release-note:improvement +helm: Change `/bin/sh -ec ""` to `/bin/sh -ec "exec "` in helm deployments +``` diff --git a/charts/consul/templates/api-gateway-controller-deployment.yaml b/charts/consul/templates/api-gateway-controller-deployment.yaml index 11396c8a03..e1fc004b95 100644 --- 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 index 2812417dbf..0f43c3e585 100644 --- 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 index 9531a9f612..a468b6e663 100644 --- 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 index 678a2af3ba..28e24e8212 100644 --- 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 index 0122690104..47aad0e599 100644 --- 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 index 02fb3ea168..af30061c78 100644 --- 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 index df9f500e3c..1d01033f2d 100644 --- 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 index ac050e7199..fe87debca0 100644 --- 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 index 6e21289f22..88c6501051 100644 --- 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 index 0b46697f31..7d56116d8d 100644 --- 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 index 7421fbc4a6..048d259197 100644 --- 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 index f4aeb1cdb8..c5a590d375 100644 --- 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 index d86c5b4bf5..45216600a6 100644 --- 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 index ea2131b8a2..69b44c60c3 100644 --- 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 index 47651fe14b..455df2c60c 100644 --- 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 index 7ba25b330c..2861d80216 100644 --- 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 index fbc8e6e581..696d5f7cbb 100755 --- 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 index 0a69df37a4..3b09fd783b 100755 --- 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" ] }