From 0b58772e5e8bd90f329488193b8b6ad1c576be12 Mon Sep 17 00:00:00 2001 From: Melisa Griffin Date: Tue, 14 May 2024 13:58:27 -0400 Subject: [PATCH 1/7] Adds ability to set the imagePullPolicy for all Consul images (consul, consul-dataplane, consul-k8s, consul-telemetry-collector) --- charts/consul/templates/_helpers.tpl | 23 +++++- charts/consul/templates/client-daemonset.yaml | 3 + charts/consul/templates/cni-daemonset.yaml | 1 + .../templates/connect-inject-deployment.yaml | 1 + .../create-federation-secret-job.yaml | 1 + .../templates/enterprise-license-job.yaml | 2 + .../consul/templates/gateway-cleanup-job.yaml | 1 + .../templates/gateway-resources-job.yaml | 1 + .../gossip-encryption-autogenerate-job.yaml | 1 + .../ingress-gateways-deployment.yaml | 2 + .../templates/mesh-gateway-deployment.yaml | 2 + .../consul/templates/partition-init-job.yaml | 1 + .../server-acl-init-cleanup-job.yaml | 1 + .../consul/templates/server-acl-init-job.yaml | 1 + .../consul/templates/server-statefulset.yaml | 3 + .../templates/sync-catalog-deployment.yaml | 1 + .../telemetry-collector-deployment.yaml | 6 +- .../terminating-gateways-deployment.yaml | 2 + .../consul/templates/tests/test-runner.yaml | 1 + .../templates/tls-init-cleanup-job.yaml | 1 + charts/consul/templates/tls-init-job.yaml | 1 + .../webhook-cert-manager-deployment.yaml | 1 + charts/consul/test/unit/helpers.bats | 56 +++++++++++++ charts/consul/values.yaml | 5 +- .../api-gateway/common/helm_config.go | 4 +- .../api-gateway/gatekeeper/dataplane.go | 5 +- control-plane/api-gateway/gatekeeper/init.go | 5 +- .../webhook/consul_dataplane_sidecar.go | 7 +- .../connect-inject/webhook/container_init.go | 5 +- .../connect-inject/webhook/mesh_webhook.go | 3 + .../subcommand/inject-connect/command.go | 3 + .../inject-connect/v1controllers.go | 82 ++++++++++--------- 32 files changed, 177 insertions(+), 55 deletions(-) diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl index c3d9c36402..b1c6c9f27f 100644 --- 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" @@ -632,7 +633,9 @@ Usage: {{ template "consul.dogstatsdAaddressInfo" . }} {{- define "consul.dogstatsdAaddressInfo" -}} {{- if (and .Values.global.metrics.datadog.enabled .Values.global.metrics.datadog.dogstatsd.enabled) }} - "dogstatsd_addr": "{{- if eq .Values.global.metrics.datadog.dogstatsd.socketTransportType "UDS" }}unix://{{ .Values.global.metrics.datadog.dogstatsd.dogstatsdAddr }}{{- else }}{{ .Values.global.metrics.datadog.dogstatsd.dogstatsdAddr | trimAll "\"" }}{{- if ne ( .Values.global.metrics.datadog.dogstatsd.dogstatsdPort | int ) 0 }}:{{ .Values.global.metrics.datadog.dogstatsd.dogstatsdPort | toString }}{{- end }}{{- end }}",{{- end }} + "dogstatsd_addr": "{{- if eq .Values.global.metrics.datadog.dogstatsd.socketTransportType "UDS" }} + unix://{{ .Values.global.metrics.datadog.dogstatsd.dogstatsdAddr }}{{- else }} + {{ .Values.global.metrics.datadog.dogstatsd.dogstatsdAddr | trimAll "\"" }}{{- if ne ( .Values.global.metrics.datadog.dogstatsd.dogstatsdPort | int ) 0 }}:{{ .Values.global.metrics.datadog.dogstatsd.dogstatsdPort | toString }}{{- end }}{{- end }}",{{- end }} {{- end -}} {{/* @@ -682,4 +685,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 -}} \ No newline at end of file diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml index cf0cb1d686..9c607385cf 100644 --- 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 index 258924f449..a93e3aea90 100644 --- 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 index fe07c2581a..5aaa50a107 100644 --- 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 index aff6b5a934..2092b97852 100644 --- 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 index 8db9500a22..9dd0281978 100644 --- 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 index 0d4f84272c..0d38f6ec8b 100644 --- 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 index e43efc8a9a..b5e7b056cc 100644 --- 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 index cea13c77fe..485064f80f 100644 --- 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 index 4c7aa2142c..994b475c2e 100644 --- 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" . }} {{- 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" . }} {{- 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 index 3d75d55613..03bf2707a5 100644 --- 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 index 21ad2930b8..0ce8a921b2 100644 --- 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 index b47e04188f..3d7d6c8120 100644 --- 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 index ca10cb3e34..0156c60f74 100644 --- 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 index 7e6d5789a9..f8cb9b4def 100644 --- 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 index 3851f0a8e2..963e6b2485 100644 --- 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 index e03f6b9a4f..78326998f8 100644 --- 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 index b4a239308a..18a638d76c 100644 --- 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" . }} {{- 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" . }} {{- 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 index b8b078003b..4c3e81ccea 100644 --- 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 index 9500410a53..5ebe236df5 100644 --- 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 index 41c0c2827e..177472c9a4 100644 --- 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 index 45c87c9ceb..0301331b9b 100644 --- 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 index 4e33b91886..58f7fbf8a4 100644 --- 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" ] +} \ No newline at end of file diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 8e307d4265..ab5e3ed157 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -86,7 +86,10 @@ global: # image that is used for functionality such as catalog sync. # This can be overridden per component. # @default: hashicorp/consul-k8s-control-plane: - imageK8S: docker.mirror.hashicorp.services/hashicorppreview/consul-k8s-control-plane:1.5-dev + imageK8S: "" + + # The image pull policy that will be used for all + 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 diff --git a/control-plane/api-gateway/common/helm_config.go b/control-plane/api-gateway/common/helm_config.go index ecf245c04c..d551757c5b 100644 --- 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 index a2f4c8a0f3..6e5c97a699 100644 --- 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 index 3b6147f164..c76f584c65 100644 --- 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{ { diff --git a/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go b/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go index dc9ca0d0bf..71fb874d52 100644 --- a/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go +++ b/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go @@ -97,9 +97,10 @@ func (w *MeshWebhook) consulDataplaneSidecar(namespace corev1.Namespace, pod cor } container := corev1.Container{ - Name: containerName, - Image: w.ImageConsulDataplane, - Resources: resources, + Name: containerName, + Image: w.ImageConsulDataplane, + ImagePullPolicy: corev1.PullPolicy(w.GlobalImagePullPolicy), + Resources: resources, // 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 // because we set file system to be read-only. diff --git a/control-plane/connect-inject/webhook/container_init.go b/control-plane/connect-inject/webhook/container_init.go index 24c71461af..f1dcd83972 100644 --- a/control-plane/connect-inject/webhook/container_init.go +++ b/control-plane/connect-inject/webhook/container_init.go @@ -103,8 +103,9 @@ func (w *MeshWebhook) containerInit(namespace corev1.Namespace, pod corev1.Pod, initContainerName = fmt.Sprintf("%s-%s", injectInitContainerName, mpi.serviceName) } container := corev1.Container{ - Name: initContainerName, - Image: w.ImageConsulK8S, + Name: initContainerName, + Image: w.ImageConsulK8S, + ImagePullPolicy: corev1.PullPolicy(w.GlobalImagePullPolicy), Env: []corev1.EnvVar{ { Name: "POD_NAME", diff --git a/control-plane/connect-inject/webhook/mesh_webhook.go b/control-plane/connect-inject/webhook/mesh_webhook.go index c8dd3cc789..beee510b92 100644 --- a/control-plane/connect-inject/webhook/mesh_webhook.go +++ b/control-plane/connect-inject/webhook/mesh_webhook.go @@ -75,6 +75,9 @@ type MeshWebhook struct { // This image is used for the consul-sidecar container. ImageConsulK8S string + // GlobalImagePullPolicy is the pull policy for all Consul images (consul, consul-dataplane, consul-k8s) + GlobalImagePullPolicy string + // Optional: set when you need extra options to be set when running envoy // See a list of args here: https://www.envoyproxy.io/docs/envoy/latest/operations/cli EnvoyExtraArgs string diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index db59a98d68..36a25840a6 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -54,6 +54,7 @@ type Command struct { flagConsulImage string // Docker image for Consul flagConsulDataplaneImage string // Docker image for Envoy flagConsulK8sImage string // Docker image for consul-k8s + flagGlobalImagePullPolicy string // Pull policy for all Consul images (consul, consul-dataplane, consul-k8s) flagACLAuthMethod string // Auth Method to use for ACLs, if enabled flagEnvoyExtraArgs string // Extra envoy args when starting envoy flagEnableWebhookCAUpdate bool @@ -194,6 +195,8 @@ func (c *Command) init() { "Docker image for Consul Dataplane.") c.flagSet.StringVar(&c.flagConsulK8sImage, "consul-k8s-image", "", "Docker image for consul-k8s. Used for the connect sidecar.") + c.flagSet.StringVar(&c.flagGlobalImagePullPolicy, "global-image-pull-policy", "", + "ImagePullPolicy for all images used by Consul (consul, consul-dataplane, consul-k8s).") c.flagSet.BoolVar(&c.flagEnablePeering, "enable-peering", false, "Enable cluster peering controllers.") c.flagSet.BoolVar(&c.flagEnableFederation, "enable-federation", false, "Enable Consul WAN Federation.") c.flagSet.StringVar(&c.flagEnvoyExtraArgs, "envoy-extra-args", "", diff --git a/control-plane/subcommand/inject-connect/v1controllers.go b/control-plane/subcommand/inject-connect/v1controllers.go index df7fff50c1..1a24439d7e 100644 --- a/control-plane/subcommand/inject-connect/v1controllers.go +++ b/control-plane/subcommand/inject-connect/v1controllers.go @@ -117,6 +117,7 @@ func (c *Command) configureV1Controllers(ctx context.Context, mgr manager.Manage }, ImageDataplane: c.flagConsulDataplaneImage, ImageConsulK8S: c.flagConsulK8sImage, + GlobalImagePullPolicy: c.flagGlobalImagePullPolicy, ConsulDestinationNamespace: c.flagConsulDestinationNamespace, NamespaceMirroringPrefix: c.flagK8SNSMirroringPrefix, EnableNamespaces: c.flagEnableNamespaces, @@ -338,47 +339,48 @@ func (c *Command) configureV1Controllers(ctx context.Context, mgr manager.Manage } (&webhook.MeshWebhook{ - Clientset: c.clientset, - ReleaseNamespace: c.flagReleaseNamespace, - ConsulConfig: consulConfig, - ConsulServerConnMgr: watcher, - ImageConsul: c.flagConsulImage, - ImageConsulDataplane: c.flagConsulDataplaneImage, - EnvoyExtraArgs: c.flagEnvoyExtraArgs, - ImageConsulK8S: c.flagConsulK8sImage, - RequireAnnotation: !c.flagDefaultInject, - AuthMethod: c.flagACLAuthMethod, - ConsulCACert: string(c.caCertPem), - TLSEnabled: c.consul.UseTLS, - ConsulAddress: c.consul.Addresses, - SkipServerWatch: c.consul.SkipServerWatch, - ConsulTLSServerName: c.consul.TLSServerName, - DefaultProxyCPURequest: c.sidecarProxyCPURequest, - DefaultProxyCPULimit: c.sidecarProxyCPULimit, - DefaultProxyMemoryRequest: c.sidecarProxyMemoryRequest, - DefaultProxyMemoryLimit: c.sidecarProxyMemoryLimit, - DefaultEnvoyProxyConcurrency: c.flagDefaultEnvoyProxyConcurrency, - DefaultSidecarProxyStartupFailureSeconds: c.flagDefaultSidecarProxyStartupFailureSeconds, + Clientset: c.clientset, + ReleaseNamespace: c.flagReleaseNamespace, + ConsulConfig: consulConfig, + ConsulServerConnMgr: watcher, + ImageConsul: c.flagConsulImage, + ImageConsulDataplane: c.flagConsulDataplaneImage, + EnvoyExtraArgs: c.flagEnvoyExtraArgs, + ImageConsulK8S: c.flagConsulK8sImage, + GlobalImagePullPolicy: c.flagGlobalImagePullPolicy, + RequireAnnotation: !c.flagDefaultInject, + AuthMethod: c.flagACLAuthMethod, + ConsulCACert: string(c.caCertPem), + TLSEnabled: c.consul.UseTLS, + ConsulAddress: c.consul.Addresses, + SkipServerWatch: c.consul.SkipServerWatch, + ConsulTLSServerName: c.consul.TLSServerName, + DefaultProxyCPURequest: c.sidecarProxyCPURequest, + DefaultProxyCPULimit: c.sidecarProxyCPULimit, + DefaultProxyMemoryRequest: c.sidecarProxyMemoryRequest, + DefaultProxyMemoryLimit: c.sidecarProxyMemoryLimit, + DefaultEnvoyProxyConcurrency: c.flagDefaultEnvoyProxyConcurrency, + DefaultSidecarProxyStartupFailureSeconds: c.flagDefaultSidecarProxyStartupFailureSeconds, DefaultSidecarProxyLivenessFailureSeconds: c.flagDefaultSidecarProxyLivenessFailureSeconds, - LifecycleConfig: lifecycleConfig, - MetricsConfig: metricsConfig, - InitContainerResources: c.initContainerResources, - ConsulPartition: c.consul.Partition, - AllowK8sNamespacesSet: allowK8sNamespaces, - DenyK8sNamespacesSet: denyK8sNamespaces, - EnableNamespaces: c.flagEnableNamespaces, - ConsulDestinationNamespace: c.flagConsulDestinationNamespace, - EnableK8SNSMirroring: c.flagEnableK8SNSMirroring, - K8SNSMirroringPrefix: c.flagK8SNSMirroringPrefix, - CrossNamespaceACLPolicy: c.flagCrossNamespaceACLPolicy, - EnableTransparentProxy: c.flagDefaultEnableTransparentProxy, - EnableCNI: c.flagEnableCNI, - TProxyOverwriteProbes: c.flagTransparentProxyDefaultOverwriteProbes, - EnableConsulDNS: c.flagEnableConsulDNS, - EnableOpenShift: c.flagEnableOpenShift, - Log: ctrl.Log.WithName("handler").WithName("connect"), - LogLevel: c.flagLogLevel, - LogJSON: c.flagLogJSON, + LifecycleConfig: lifecycleConfig, + MetricsConfig: metricsConfig, + InitContainerResources: c.initContainerResources, + ConsulPartition: c.consul.Partition, + AllowK8sNamespacesSet: allowK8sNamespaces, + DenyK8sNamespacesSet: denyK8sNamespaces, + EnableNamespaces: c.flagEnableNamespaces, + ConsulDestinationNamespace: c.flagConsulDestinationNamespace, + EnableK8SNSMirroring: c.flagEnableK8SNSMirroring, + K8SNSMirroringPrefix: c.flagK8SNSMirroringPrefix, + CrossNamespaceACLPolicy: c.flagCrossNamespaceACLPolicy, + EnableTransparentProxy: c.flagDefaultEnableTransparentProxy, + EnableCNI: c.flagEnableCNI, + TProxyOverwriteProbes: c.flagTransparentProxyDefaultOverwriteProbes, + EnableConsulDNS: c.flagEnableConsulDNS, + EnableOpenShift: c.flagEnableOpenShift, + Log: ctrl.Log.WithName("handler").WithName("connect"), + LogLevel: c.flagLogLevel, + LogJSON: c.flagLogJSON, }).SetupWithManager(mgr) consulMeta := apicommon.ConsulMeta{ From bb6a5870da6fe8b895225126c6ca35e2d49e6984 Mon Sep 17 00:00:00 2001 From: Melisa Griffin Date: Tue, 14 May 2024 14:06:02 -0400 Subject: [PATCH 2/7] Accidentally left the consul-k8s image as empty string --- charts/consul/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index ab5e3ed157..74f46a1c80 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -86,7 +86,7 @@ global: # image that is used for functionality such as catalog sync. # This can be overridden per component. # @default: hashicorp/consul-k8s-control-plane: - imageK8S: "" + imageK8S: docker.mirror.hashicorp.services/hashicorppreview/consul-k8s-control-plane:1.5-dev # The image pull policy that will be used for all imagePullPolicy: "" From 11cd019b14e3d0b932190aa1d835a393fed22b02 Mon Sep 17 00:00:00 2001 From: Melisa Griffin Date: Tue, 14 May 2024 14:29:38 -0400 Subject: [PATCH 3/7] Adds changelog --- .changelog/3991.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/3991.txt diff --git a/.changelog/3991.txt b/.changelog/3991.txt new file mode 100644 index 0000000000..45ff3c90dd --- /dev/null +++ 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) +``` From 2e902f955d118d6ed29a8f67d2051c1f28841cad Mon Sep 17 00:00:00 2001 From: Melisa Griffin Date: Tue, 14 May 2024 16:18:58 -0400 Subject: [PATCH 4/7] Accidentally left in some changes I did to figure out bats --- charts/consul/templates/_helpers.tpl | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl index b1c6c9f27f..697959d5db 100644 --- a/charts/consul/templates/_helpers.tpl +++ b/charts/consul/templates/_helpers.tpl @@ -633,9 +633,7 @@ Usage: {{ template "consul.dogstatsdAaddressInfo" . }} {{- define "consul.dogstatsdAaddressInfo" -}} {{- if (and .Values.global.metrics.datadog.enabled .Values.global.metrics.datadog.dogstatsd.enabled) }} - "dogstatsd_addr": "{{- if eq .Values.global.metrics.datadog.dogstatsd.socketTransportType "UDS" }} - unix://{{ .Values.global.metrics.datadog.dogstatsd.dogstatsdAddr }}{{- else }} - {{ .Values.global.metrics.datadog.dogstatsd.dogstatsdAddr | trimAll "\"" }}{{- if ne ( .Values.global.metrics.datadog.dogstatsd.dogstatsdPort | int ) 0 }}:{{ .Values.global.metrics.datadog.dogstatsd.dogstatsdPort | toString }}{{- end }}{{- end }}",{{- end }} + "dogstatsd_addr": "{{- if eq .Values.global.metrics.datadog.dogstatsd.socketTransportType "UDS" }}unix://{{ .Values.global.metrics.datadog.dogstatsd.dogstatsdAddr }}{{- else }}{{ .Values.global.metrics.datadog.dogstatsd.dogstatsdAddr | trimAll "\"" }}{{- if ne ( .Values.global.metrics.datadog.dogstatsd.dogstatsdPort | int ) 0 }}:{{ .Values.global.metrics.datadog.dogstatsd.dogstatsdPort | toString }}{{- end }}{{- end }}",{{- end }} {{- end -}} {{/* From 09b33b49b8316a52483903d22572bc93ceb69b9f Mon Sep 17 00:00:00 2001 From: Melisa Griffin Date: Wed, 15 May 2024 13:34:44 -0400 Subject: [PATCH 5/7] Fix issue where values were not passed in --- charts/consul/templates/ingress-gateways-deployment.yaml | 4 ++-- charts/consul/templates/terminating-gateways-deployment.yaml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/charts/consul/templates/ingress-gateways-deployment.yaml b/charts/consul/templates/ingress-gateways-deployment.yaml index 994b475c2e..c7a38bb040 100644 --- a/charts/consul/templates/ingress-gateways-deployment.yaml +++ b/charts/consul/templates/ingress-gateways-deployment.yaml @@ -184,7 +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" . }} + {{ template "consul.imagePullPolicy" $root }} {{- include "consul.restrictedSecurityContext" $ | nindent 8 }} env: - name: NAMESPACE @@ -246,7 +246,7 @@ spec: containers: - name: ingress-gateway image: {{ $root.Values.global.imageConsulDataplane | quote }} - {{ template "consul.imagePullPolicy" . }} + {{ 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/terminating-gateways-deployment.yaml b/charts/consul/templates/terminating-gateways-deployment.yaml index 18a638d76c..c4970979b1 100644 --- a/charts/consul/templates/terminating-gateways-deployment.yaml +++ b/charts/consul/templates/terminating-gateways-deployment.yaml @@ -169,7 +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" . }} + {{ template "consul.imagePullPolicy" $root }} {{- include "consul.restrictedSecurityContext" $ | nindent 10 }} env: - name: NAMESPACE @@ -231,7 +231,7 @@ spec: containers: - name: terminating-gateway image: {{ $root.Values.global.imageConsulDataplane | quote }} - {{ template "consul.imagePullPolicy" . }} + {{ template "consul.imagePullPolicy" $root }} {{- include "consul.restrictedSecurityContext" $ | nindent 10 }} volumeMounts: - name: tmp From f6f4b8b0aff964cc97c6a7d35265fe3028ca84f2 Mon Sep 17 00:00:00 2001 From: Melisa Griffin Date: Wed, 15 May 2024 16:33:19 -0400 Subject: [PATCH 6/7] Adds command validation and test --- control-plane/subcommand/inject-connect/command.go | 10 ++++++++++ .../subcommand/inject-connect/command_test.go | 9 +++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index 36a25840a6..96e95bfa42 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -445,6 +445,16 @@ func (c *Command) validateFlags() error { return errors.New("-consul-dataplane-image must be set") } + switch corev1.PullPolicy(c.flagGlobalImagePullPolicy) { + case corev1.PullAlways: + case corev1.PullNever: + case corev1.PullIfNotPresent: + case "": + break + default: + return errors.New("-global-image-pull-policy must be `IfNotPresent`, `Always`, `Never`, or `` ") + } + // In Consul 1.17, multiport beta shipped with v2 catalog + mesh resources backed by v1 tenancy // and acls (experiments=[resource-apis]). // diff --git a/control-plane/subcommand/inject-connect/command_test.go b/control-plane/subcommand/inject-connect/command_test.go index e7ca3f12cd..7f4b93f89e 100644 --- a/control-plane/subcommand/inject-connect/command_test.go +++ b/control-plane/subcommand/inject-connect/command_test.go @@ -133,13 +133,10 @@ func TestRun_FlagValidation(t *testing.T) { expErr: "-default-envoy-proxy-concurrency must be >= 0 if set", }, { - flags: []string{ - "-consul-k8s-image", "hashicorp/consul-k8s", - "-consul-image", "hashicorp/consul", - "-consul-dataplane-image", "hashicorp/consul-dataplane", - "-enable-v2tenancy", "true", + flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-consul-dataplane-image", "consul-dataplane:1.14.0", + "-global-image-pull-policy", "garbage", }, - expErr: "-enable-resource-apis must be set to 'true' if -enable-v2tenancy is set", + expErr: "-global-image-pull-policy must be `IfNotPresent`, `Always`, `Never`, or `` ", }, } From 600825c23b2de0bdcae594c4dccc4b4c284d1526 Mon Sep 17 00:00:00 2001 From: Melisa Griffin Date: Thu, 16 May 2024 12:47:18 -0400 Subject: [PATCH 7/7] Update description of field in values.yaml --- charts/consul/values.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 74f46a1c80..758b1cf1af 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -88,7 +88,9 @@ global: # @default: hashicorp/consul-k8s-control-plane: imageK8S: docker.mirror.hashicorp.services/hashicorppreview/consul-k8s-control-plane:1.5-dev - # The image pull policy that will be used for all + # 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