Adds ability to set the imagePullPolicy for all Consul images (consul… (

#3991) * Adds ability to set the imagePullPolicy for all Consul images (consul, consul-dataplane, consul-k8s, consul-telemetry-collector)
hashicorp · May 16, 2024 · 2814255 · 2814255
1 parent 5ca164d
commit 2814255
Show file tree

Hide file tree

Showing 34 changed files with 191 additions and 59 deletions.
diff --git a/.changelog/3991.txt b/.changelog/3991.txt
@@ -0,0 +1,3 @@
+```release-note:feature
+helm: adds ability to set the Image Pull Policy for all Consul images (consul, consul-k8s, consul-dataplane, consul-telemetry-collector)
+```
diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl
@@ -246,6 +246,7 @@ This template is for an init container.
 {{- define "consul.getAutoEncryptClientCA" -}}
 - name: get-auto-encrypt-client-ca
   image: {{ .Values.global.imageK8S }}
+  {{ template "consul.imagePullPolicy" . }}
   command:
     - "/bin/sh"
     - "-ec"
@@ -682,4 +683,22 @@ Usage: {{ template "consul.versionInfo" }}
     {{- $sanitizedVersion = $versionInfo }}
 {{- end -}}
 {{- printf "%s" $sanitizedVersion | trunc 63 | quote }}
+{{- end -}}
+
+{{/*
+Sets the imagePullPolicy for all Consul images (consul, consul-dataplane, consul-k8s, consul-telemetry-collector)
+Valid values are:
+    IfNotPresent
+    Always
+    Never
+    In the case of empty, see https://kubernetes.io/docs/concepts/containers/images/#image-pull-policy for details
+
+Usage: {{ template "consul.imagePullPolicy" . }} TODO: melisa should we name this differently ?
+*/}}
+{{- define "consul.imagePullPolicy" -}}
+{{ if or (eq .Values.global.imagePullPolicy "IfNotPresent") (eq .Values.global.imagePullPolicy "Always") (eq .Values.global.imagePullPolicy "Never")}}imagePullPolicy: {{ .Values.global.imagePullPolicy }}
+{{ else if eq .Values.global.imagePullPolicy "" }}
+{{ else }}
+{{fail "imagePullPolicy can only be IfNotPresent, Always, Never, or empty" }}
+{{ end }}
 {{- end -}}
diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml
@@ -200,6 +200,7 @@ spec:
       containers:
         - name: consul
           image: "{{ default .Values.global.image .Values.client.image }}"
+          {{ template "consul.imagePullPolicy" . }}
           {{- if .Values.global.acls.manageSystemACLs }}
           lifecycle:
             preStop:
@@ -502,6 +503,7 @@ spec:
       {{- if .Values.global.acls.manageSystemACLs }}
       - name: client-acl-init
         image: {{ .Values.global.imageK8S }}
+        {{ template "consul.imagePullPolicy" . }}
         env:
         - name: NAMESPACE
           valueFrom:
@@ -554,6 +556,7 @@ spec:
       {{- if and .Values.global.tls.enabled (not .Values.global.tls.enableAutoEncrypt) }}
       - name: client-tls-init
         image: "{{ default .Values.global.image .Values.client.image }}"
+        {{ template "consul.imagePullPolicy" . }}
         env:
         - name: HOST_IP
           valueFrom:

diff --git a/charts/consul/templates/cni-daemonset.yaml b/charts/consul/templates/cni-daemonset.yaml
@@ -62,6 +62,7 @@ spec:
         # This container installs the consul CNI binaries and CNI network config file on each node
         - name: install-cni
           image: {{ .Values.global.imageK8S }}
+          {{ template "consul.imagePullPolicy" . }}
           securityContext:
             privileged: true
           command:

diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml
@@ -98,6 +98,7 @@ spec:
       containers:
         - name: sidecar-injector
           image: "{{ default .Values.global.imageK8S .Values.connectInject.image }}"
+          {{ template "consul.imagePullPolicy" . }}
           ports:
             - containerPort: 8080
               name: webhook-server

diff --git a/charts/consul/templates/create-federation-secret-job.yaml b/charts/consul/templates/create-federation-secret-job.yaml
@@ -94,6 +94,7 @@ spec:
       containers:
         - name: create-federation-secret
           image: "{{ .Values.global.imageK8S }}"
+          {{ template "consul.imagePullPolicy" . }}
           {{- include "consul.restrictedSecurityContext" . | nindent 10 }}
           env:
             - name: NAMESPACE

diff --git a/charts/consul/templates/enterprise-license-job.yaml b/charts/consul/templates/enterprise-license-job.yaml
@@ -59,6 +59,7 @@ spec:
       containers:
         - name: apply-enterprise-license
           image: "{{ default .Values.global.image .Values.server.image }}"
+          {{ template "consul.imagePullPolicy" . }}
           env:
             - name: ENTERPRISE_LICENSE
               {{- if .Values.global.secretsBackend.vault.enabled }}
@@ -125,6 +126,7 @@ spec:
       initContainers:
       - name: ent-license-acl-init
         image: {{ .Values.global.imageK8S }}
+        {{ template "consul.imagePullPolicy" . }}
         command:
           - "/bin/sh"
           - "-ec"

diff --git a/charts/consul/templates/gateway-cleanup-job.yaml b/charts/consul/templates/gateway-cleanup-job.yaml
@@ -38,6 +38,7 @@ spec:
       containers:
         - name: gateway-cleanup
           image: {{ .Values.global.imageK8S }}
+          {{ template "consul.imagePullPolicy" . }}
           {{- include "consul.restrictedSecurityContext" . | nindent 10 }}
           command:
             - consul-k8s-control-plane

diff --git a/charts/consul/templates/gateway-resources-job.yaml b/charts/consul/templates/gateway-resources-job.yaml
@@ -39,6 +39,7 @@ spec:
       containers:
         - name: gateway-resources
           image: {{ .Values.global.imageK8S }}
+          {{ template "consul.imagePullPolicy" . }}
           {{- include "consul.restrictedSecurityContext" . | nindent 10 }}
           command:
             - consul-k8s-control-plane

diff --git a/charts/consul/templates/gossip-encryption-autogenerate-job.yaml b/charts/consul/templates/gossip-encryption-autogenerate-job.yaml
@@ -49,6 +49,7 @@ spec:
       containers:
         - name: gossip-encryption-autogen
           image: "{{ .Values.global.imageK8S }}"
+          {{ template "consul.imagePullPolicy" . }}
           {{- include "consul.restrictedSecurityContext" . | nindent 10 }}
           command:
             - "/bin/sh"

diff --git a/charts/consul/templates/ingress-gateways-deployment.yaml b/charts/consul/templates/ingress-gateways-deployment.yaml
@@ -184,6 +184,7 @@ spec:
       # ingress-gateway-init registers the ingress gateway service with Consul.
       - name: ingress-gateway-init
         image: {{ $root.Values.global.imageK8S }}
+        {{ template "consul.imagePullPolicy" $root }}
         {{- include "consul.restrictedSecurityContext" $ | nindent 8 }}
         env:
         - name: NAMESPACE
@@ -245,6 +246,7 @@ spec:
       containers:
       - name: ingress-gateway
         image: {{ $root.Values.global.imageConsulDataplane | quote }}
+        {{ template "consul.imagePullPolicy" $root }}
         {{- include "consul.restrictedSecurityContext" $ | nindent 8 }}
         {{- if (default $defaults.resources .resources) }}
         resources: {{ toYaml (default $defaults.resources .resources) | nindent 10 }}

diff --git a/charts/consul/templates/mesh-gateway-deployment.yaml b/charts/consul/templates/mesh-gateway-deployment.yaml
@@ -128,6 +128,7 @@ spec:
       initContainers:
       - name: mesh-gateway-init
         image: {{ .Values.global.imageK8S }}
+        {{ template "consul.imagePullPolicy" . }}
         env:
         - name: NAMESPACE
           valueFrom:
@@ -186,6 +187,7 @@ spec:
       containers:
       - name: mesh-gateway
         image: {{ .Values.global.imageConsulDataplane | quote }}
+        {{ template "consul.imagePullPolicy" . }}
         securityContext:
           capabilities:
             {{ if not .Values.meshGateway.hostNetwork}}

diff --git a/charts/consul/templates/partition-init-job.yaml b/charts/consul/templates/partition-init-job.yaml
@@ -85,6 +85,7 @@ spec:
       containers:
         - name: partition-init-job
           image: {{ .Values.global.imageK8S }}
+          {{ template "consul.imagePullPolicy" . }}
           {{- include "consul.restrictedSecurityContext" . | nindent 10 }}
           env:
           {{- include "consul.consulK8sConsulServerEnvVars" . | nindent 10 }}

diff --git a/charts/consul/templates/server-acl-init-cleanup-job.yaml b/charts/consul/templates/server-acl-init-cleanup-job.yaml
@@ -61,6 +61,7 @@ spec:
       containers:
         - name: server-acl-init-cleanup
           image: {{ .Values.global.imageK8S }}
+          {{ template "consul.imagePullPolicy" . }}
           {{- if not .Values.server.containerSecurityContext.aclInit }}
           {{- include "consul.restrictedSecurityContext" . | nindent 10 }}
           {{- end }}

diff --git a/charts/consul/templates/server-acl-init-job.yaml b/charts/consul/templates/server-acl-init-job.yaml
@@ -137,6 +137,7 @@ spec:
       containers:
       - name: server-acl-init-job
         image: {{ .Values.global.imageK8S }}
+        {{ template "consul.imagePullPolicy" . }}
         {{- if not .Values.server.containerSecurityContext.aclInit }}
         {{- include "consul.restrictedSecurityContext" . | nindent 8 }}
         {{- end }}

diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml
@@ -321,6 +321,7 @@ spec:
       initContainers:
       - name: locality-init
         image: {{ .Values.global.imageK8S }}
+        {{ template "consul.imagePullPolicy" . }}
         env:
         - name: NODE_NAME
           valueFrom:
@@ -338,6 +339,7 @@ spec:
       containers:
         - name: consul
           image: "{{ default .Values.global.image .Values.server.image | trimPrefix "\"" | trimSuffix "\"" }}"
+          {{ template "consul.imagePullPolicy" . }}
           imagePullPolicy: {{ .Values.global.imagePullPolicy }}
           env:
             - name: ADVERTISE_IP
@@ -657,6 +659,7 @@ spec:
         {{- if .Values.server.snapshotAgent.enabled }}
         - name: consul-snapshot-agent
           image: "{{ default .Values.global.image .Values.server.image }}"
+          {{ template "consul.imagePullPolicy" . }}
           env:
             {{- if .Values.server.snapshotAgent.caCert }}
             - name: SSL_CERT_DIR

diff --git a/charts/consul/templates/sync-catalog-deployment.yaml b/charts/consul/templates/sync-catalog-deployment.yaml
@@ -81,6 +81,7 @@ spec:
       containers:
       - name: sync-catalog
         image: "{{ default .Values.global.imageK8S .Values.syncCatalog.image }}"
+        {{ template "consul.imagePullPolicy" . }}
         {{- include "consul.restrictedSecurityContext" . | nindent 8 }}
         env:
         {{- include "consul.consulK8sConsulServerEnvVars" . | nindent 8 }}

diff --git a/charts/consul/templates/telemetry-collector-deployment.yaml b/charts/consul/templates/telemetry-collector-deployment.yaml
@@ -143,7 +143,7 @@ spec:
               -service-name=""
 
         image: {{ .Values.global.imageK8S }}
-        imagePullPolicy: IfNotPresent
+        {{ template "consul.imagePullPolicy" . }}
         {{- if .Values.telemetryCollector.initContainer.resources }}
         resources:
           {{- toYaml .Values.telemetryCollector.initContainer.resources | nindent 12 }}
@@ -171,7 +171,7 @@ spec:
       containers:
       - name: consul-telemetry-collector
         image: {{ .Values.telemetryCollector.image }}
-        imagePullPolicy: {{ .Values.global.imagePullPolicy }}
+        {{ template "consul.imagePullPolicy" . }}
         ports:
         - containerPort: 9090
           name: metrics
@@ -299,7 +299,7 @@ spec:
       # consul-dataplane container
       - name: consul-dataplane
         image: "{{ .Values.global.imageConsulDataplane }}"
-        imagePullPolicy: IfNotPresent
+        {{ template "consul.imagePullPolicy" . }}
         command:
           - consul-dataplane
         args:

diff --git a/charts/consul/templates/terminating-gateways-deployment.yaml b/charts/consul/templates/terminating-gateways-deployment.yaml
@@ -169,6 +169,7 @@ spec:
         # terminating-gateway-init registers the terminating gateway service with Consul.
         - name: terminating-gateway-init
           image: {{ $root.Values.global.imageK8S }}
+          {{ template "consul.imagePullPolicy" $root }}
           {{- include "consul.restrictedSecurityContext" $ | nindent 10 }}
           env:
           - name: NAMESPACE
@@ -230,6 +231,7 @@ spec:
       containers:
         - name: terminating-gateway
           image: {{ $root.Values.global.imageConsulDataplane | quote }}
+          {{ template "consul.imagePullPolicy" $root }}
           {{- include "consul.restrictedSecurityContext" $ | nindent 10 }}
           volumeMounts:
             - name: tmp

diff --git a/charts/consul/templates/tests/test-runner.yaml b/charts/consul/templates/tests/test-runner.yaml
@@ -37,6 +37,7 @@ spec:
   containers:
     - name: consul-test
       image: "{{ .Values.global.image }}"
+      {{ template "consul.imagePullPolicy" . }}
       env:
         - name: HOST_IP
           valueFrom:

diff --git a/charts/consul/templates/tls-init-cleanup-job.yaml b/charts/consul/templates/tls-init-cleanup-job.yaml
@@ -49,6 +49,7 @@ spec:
       containers:
         - name: tls-init-cleanup
           image: "{{ .Values.global.image }}"
+          {{ template "consul.imagePullPolicy" . }}
           {{- if not .Values.server.containerSecurityContext.tlsInit }}
           {{- include "consul.restrictedSecurityContext" . | nindent 10 }}
           {{- end }}

diff --git a/charts/consul/templates/tls-init-job.yaml b/charts/consul/templates/tls-init-job.yaml
@@ -64,6 +64,7 @@ spec:
       containers:
         - name: tls-init
           image: "{{ .Values.global.imageK8S }}"
+          {{ template "consul.imagePullPolicy" . }}
           {{- if not .Values.server.containerSecurityContext.tlsInit }}
           {{- include "consul.restrictedSecurityContext" . | nindent 10 }}
           {{- end }}

diff --git a/charts/consul/templates/webhook-cert-manager-deployment.yaml b/charts/consul/templates/webhook-cert-manager-deployment.yaml
@@ -51,6 +51,7 @@ spec:
             -deployment-name={{ template "consul.fullname" . }}-webhook-cert-manager \
             -deployment-namespace={{ .Release.Namespace }}
         image: {{ .Values.global.imageK8S }}
+        {{ template "consul.imagePullPolicy" . }}
         name: webhook-cert-manager
         {{- include "consul.restrictedSecurityContext" . | nindent 8 }}
         resources:

diff --git a/charts/consul/test/unit/helpers.bats b/charts/consul/test/unit/helpers.bats
@@ -454,3 +454,59 @@ load _helpers
   [ "$status" -eq 1 ]
   [[ "$output" =~ "When the value global.experiments.resourceAPIs is set, terminatingGateways.enabled is currently unsupported." ]]
 }
+
+
+
+
+
+#--------------------------------------------------------------------
+# consul.imagePullPolicy
+# These tests use test-runner.yaml to "unit test" the imagePullPolicy function
+
+@test "helper/consul.imagePullPolicy: bad input" {
+  cd `chart_dir`
+  run helm template \
+      -s templates/tests/test-runner.yaml \
+      --set 'global.imagePullPolicy=Garbage' .
+ [ "$status" -eq 1 ]
+ [[ "$output" =~ "imagePullPolicy can only be IfNotPresent, Always, Never, or empty" ]]
+}
+
+@test "helper/consul.imagePullPolicy: empty input" {
+  cd `chart_dir`
+  local output=$(helm template \
+      -s templates/tests/test-runner.yaml \
+      . | tee /dev/stderr |
+      yq -r '.spec.containers[0].imagePullPolicy' | tee /dev/stderr)
+  [ "${output}" = null ]
+}
+
+@test "helper/consul.imagePullPolicy: IfNotPresent" {
+  cd `chart_dir`
+  local output=$(helm template \
+      -s templates/tests/test-runner.yaml \
+      --set 'global.imagePullPolicy=IfNotPresent' \
+      . | tee /dev/stderr |
+      yq -r '.spec.containers[0].imagePullPolicy' | tee /dev/stderr)
+  [ "${output}" = "IfNotPresent" ]
+}
+
+@test "helper/consul.imagePullPolicy: Always" {
+  cd `chart_dir`
+  local output=$(helm template \
+      -s templates/tests/test-runner.yaml \
+      --set 'global.imagePullPolicy=Always' \
+      . | tee /dev/stderr |
+      yq -r '.spec.containers[0].imagePullPolicy' | tee /dev/stderr)
+  [ "${output}" = "Always" ]
+}
+
+@test "helper/consul.imagePullPolicy: Never" {
+  cd `chart_dir`
+  local output=$(helm template \
+      -s templates/tests/test-runner.yaml \
+      --set 'global.imagePullPolicy=Never' \
+      . | tee /dev/stderr |
+      yq -r '.spec.containers[0].imagePullPolicy' | tee /dev/stderr)
+  [ "${output}" = "Never" ]
+}
diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml
@@ -88,6 +88,11 @@ global:
   # @default: hashicorp/consul-k8s-control-plane:<latest version>
   imageK8S: docker.mirror.hashicorp.services/hashicorppreview/consul-k8s-control-plane:1.5-dev
 
+  # The image pull policy used globally for images controlled by Consul (consul, consul-dataplane, consul-k8s, consul-telemetry-collector).
+  # One of "IfNotPresent", "Always", "Never", and "". Refer to https://kubernetes.io/docs/concepts/containers/images/#image-pull-policy
+  # @default: ""
+  imagePullPolicy: ""
+
   # The name of the datacenter that the agents should
   # register as. This can't be changed once the Consul cluster is up and running
   # since Consul doesn't support an automatic way to change this value currently:

diff --git a/control-plane/api-gateway/common/helm_config.go b/control-plane/api-gateway/common/helm_config.go
@@ -18,7 +18,9 @@ type HelmConfig struct {
 	// ImageDataplane is the Consul Dataplane image to use in gateway deployments.
 	ImageDataplane string
 	// ImageConsulK8S is the Consul Kubernetes Control Plane image to use in gateway deployments.
-	ImageConsulK8S             string
+	ImageConsulK8S string
+	// GlobalImagePullPolicy is the pull policy to use for all images used in gateway deployments.
+	GlobalImagePullPolicy      string
 	ConsulDestinationNamespace string
 	NamespaceMirroringPrefix   string
 	EnableNamespaces           bool

diff --git a/control-plane/api-gateway/gatekeeper/dataplane.go b/control-plane/api-gateway/gatekeeper/dataplane.go
@@ -53,8 +53,9 @@ func consulDataplaneContainer(metrics common.MetricsConfig, config common.HelmCo
 	}
 
 	container := corev1.Container{
-		Name:  name,
-		Image: config.ImageDataplane,
+		Name:            name,
+		Image:           config.ImageDataplane,
+		ImagePullPolicy: corev1.PullPolicy(config.GlobalImagePullPolicy),
 
 		// We need to set tmp dir to an ephemeral volume that we're mounting so that
 		// consul-dataplane can write files to it. Otherwise, it wouldn't be able to

diff --git a/control-plane/api-gateway/gatekeeper/init.go b/control-plane/api-gateway/gatekeeper/init.go
@@ -69,8 +69,9 @@ func initContainer(config common.HelmConfig, name, namespace string) (corev1.Con
 
 	initContainerName := injectInitContainerName
 	container := corev1.Container{
-		Name:  initContainerName,
-		Image: config.ImageConsulK8S,
+		Name:            initContainerName,
+		Image:           config.ImageConsulK8S,
+		ImagePullPolicy: corev1.PullPolicy(config.GlobalImagePullPolicy),
 
 		Env: []corev1.EnvVar{
 			{