feat(deploy): use Always pull policy for development images (#286)

* feat(deploy): use Always pull policy for development images

* Account for digests and latest tag

* Regenerate bundle

* Invert conditional in Makefile

Makefile

@@ -54,6 +54,14 @@ else
 GOBIN=$(shell go env GOBIN)
 endif
 
+# Check whether this is a development or release version
+ifneq (,$(shell echo $(IMAGE_VERSION) | grep -iE '(:latest|SNAPSHOT|dev|BETA[[:digit:]]+)$$'))
+PULL_POLICY ?= Always
+else
+PULL_POLICY ?= IfNotPresent
+endif
+export PULL_POLICY
+
 # Run tests with Ginkgo CLI if available
 GINKGO ?= $(shell go env GOPATH)/bin/ginkgo
 GO_TEST ?= go test
@@ -130,6 +138,7 @@ undeploy:
 manifests: controller-gen
 	$(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
 	envsubst < hack/image_tag_patch.yaml.in > config/default/image_tag_patch.yaml
+	envsubst < hack/image_pull_patch.yaml.in > config/default/image_pull_patch.yaml
 
 # Run go fmt against code
 .PHONY: fmt

bundle/manifests/cryostat-operator.clusterserviceversion.yaml

@@ -342,6 +342,7 @@ spec:
                     fieldRef:
                       fieldPath: metadata.annotations['olm.targetNamespaces']
                 image: quay.io/cryostat/cryostat-operator:2.1.0-dev
+                imagePullPolicy: Always
                 livenessProbe:
                   httpGet:
                     path: /healthz

config/default/image_pull_patch.yaml

@@ -0,0 +1,11 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: controller-manager
+  namespace: system
+spec:
+  template:
+    spec:
+      containers:
+      - name: manager
+        imagePullPolicy: "Always"

config/default/kustomization.yaml

@@ -26,6 +26,7 @@ bases:
 
 patchesStrategicMerge:
 - image_tag_patch.yaml
+- image_pull_patch.yaml
 # Protect the /metrics endpoint by putting it behind auth.
 # If you want your controller-manager to expose the /metrics
 # endpoint w/o any authn/z, please comment the following line.

hack/image_pull_patch.yaml.in

@@ -0,0 +1,11 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: controller-manager
+  namespace: system
+spec:
+  template:
+    spec:
+      containers:
+      - name: manager
+        imagePullPolicy: "${PULL_POLICY}"

internal/controllers/common/resource_definitions/resource_definitions.go

@@ -41,6 +41,7 @@ import (
 	"fmt"
 	"math/rand"
 	"net/url"
+	"regexp"
 	"time"
 
 	operatorv1beta1 "github.com/cryostatio/cryostat-operator/api/v1beta1"
@@ -494,9 +495,10 @@ func NewCoreContainer(cr *operatorv1beta1.Cryostat, specs *ServiceSpecs, imageTa
 		},
 	}
 	return corev1.Container{
-		Name:         cr.Name,
-		Image:        imageTag,
-		VolumeMounts: mounts,
+		Name:            cr.Name,
+		Image:           imageTag,
+		ImagePullPolicy: getPullPolicy(imageTag),
+		VolumeMounts:    mounts,
 		Ports: []corev1.ContainerPort{
 			{
 				ContainerPort: 8181,
@@ -584,9 +586,10 @@ func NewGrafanaContainer(cr *operatorv1beta1.Cryostat, imageTag string, tls *TLS
 		livenessProbeScheme = corev1.URISchemeHTTPS
 	}
 	return corev1.Container{
-		Name:         cr.Name + "-grafana",
-		Image:        imageTag,
-		VolumeMounts: mounts,
+		Name:            cr.Name + "-grafana",
+		Image:           imageTag,
+		ImagePullPolicy: getPullPolicy(imageTag),
+		VolumeMounts:    mounts,
 		Ports: []corev1.ContainerPort{
 			{
 				ContainerPort: 3000,
@@ -622,8 +625,9 @@ const DatasourceURL = "http://" + datasourceHost + ":" + datasourcePort
 
 func NewJfrDatasourceContainer(cr *operatorv1beta1.Cryostat, imageTag string) corev1.Container {
 	return corev1.Container{
-		Name:  cr.Name + "-jfr-datasource",
-		Image: imageTag,
+		Name:            cr.Name + "-jfr-datasource",
+		Image:           imageTag,
+		ImagePullPolicy: getPullPolicy(imageTag),
 		Ports: []corev1.ContainerPort{
 			{
 				ContainerPort: 8080,
@@ -888,3 +892,15 @@ func clusterUniqueName(cr *operatorv1beta1.Cryostat) string {
 	suffix := fmt.Sprintf("%x", sha256.Sum256([]byte(nn.String())))
 	return "cryostat-" + suffix
 }
+
+// Matches image tags of the form "major.minor.patch"
+var develVerRegexp = regexp.MustCompile(`(?i)(:latest|SNAPSHOT|dev|BETA\d+)$`)
+
+func getPullPolicy(imageTag string) corev1.PullPolicy {
+	// Use Always for tags that have a known development suffix
+	if develVerRegexp.MatchString(imageTag) {
+		return corev1.PullAlways
+	}
+	// Likely a release, use IfNotPresent
+	return corev1.PullIfNotPresent
+}

internal/controllers/cryostat_controller_test.go

@@ -424,17 +424,95 @@ var _ = Describe("CryostatController", func() {
 			})
 		})
 		Context("with overriden image tags", func() {
+			var deploy *appsv1.Deployment
 			BeforeEach(func() {
 				t.objs = append(t.objs, test.NewCryostat())
-				coreImg := "my/core-image:1.0"
-				datasourceImg := "my/datasource-image:1.0"
-				grafanaImg := "my/grafana-image:1.0"
-				t.EnvCoreImageTag = &coreImg
-				t.EnvDatasourceImageTag = &datasourceImg
-				t.EnvGrafanaImageTag = &grafanaImg
-			})
-			It("should create deployment with the expected tags", func() {
-				t.expectDeployment()
+				deploy = &appsv1.Deployment{}
+			})
+			JustBeforeEach(func() {
+				t.reconcileCryostatFully()
+				err := t.Client.Get(context.Background(), types.NamespacedName{Name: "cryostat", Namespace: "default"}, deploy)
+				Expect(err).ToNot(HaveOccurred())
+			})
+			Context("for development", func() {
+				BeforeEach(func() {
+					coreImg := "my/core-image:1.0.0-SNAPSHOT"
+					datasourceImg := "my/datasource-image:1.0.0-BETA25"
+					grafanaImg := "my/grafana-image:1.0.0-dev"
+					t.EnvCoreImageTag = &coreImg
+					t.EnvDatasourceImageTag = &datasourceImg
+					t.EnvGrafanaImageTag = &grafanaImg
+				})
+				It("should create deployment with the expected tags", func() {
+					t.checkDeployment()
+				})
+				It("should set ImagePullPolicy to Always", func() {
+					containers := deploy.Spec.Template.Spec.Containers
+					Expect(containers).To(HaveLen(3))
+					for _, container := range containers {
+						Expect(container.ImagePullPolicy).To(Equal(corev1.PullAlways))
+					}
+				})
+			})
+			Context("for release", func() {
+				BeforeEach(func() {
+					coreImg := "my/core-image:1.0.0"
+					datasourceImg := "my/datasource-image:1.0.0"
+					grafanaImg := "my/grafana-image:1.0.0"
+					t.EnvCoreImageTag = &coreImg
+					t.EnvDatasourceImageTag = &datasourceImg
+					t.EnvGrafanaImageTag = &grafanaImg
+				})
+				It("should create deployment with the expected tags", func() {
+					t.checkDeployment()
+				})
+				It("should set ImagePullPolicy to IfNotPresent", func() {
+					containers := deploy.Spec.Template.Spec.Containers
+					Expect(containers).To(HaveLen(3))
+					for _, container := range containers {
+						Expect(container.ImagePullPolicy).To(Equal(corev1.PullIfNotPresent))
+					}
+				})
+			})
+			Context("by digest", func() {
+				BeforeEach(func() {
+					coreImg := "my/core-image@sha256:99b57e9b8880bc5d4d799b508603628c37c3e6a0d4bdd0988e9dc3ad8e04c495"
+					datasourceImg := "my/datasource-image@sha256:59ded87392077c2371b26e021aade0409855b597383fa78e549eefafab8fc90c"
+					grafanaImg := "my/grafana-image@sha256:e5bc16c2c5b69cd6fd8fdf1381d0a8b6cc9e01d92b9e1bb0a61ed89196563c72"
+					t.EnvCoreImageTag = &coreImg
+					t.EnvDatasourceImageTag = &datasourceImg
+					t.EnvGrafanaImageTag = &grafanaImg
+				})
+				It("should create deployment with the expected tags", func() {
+					t.checkDeployment()
+				})
+				It("should set ImagePullPolicy to IfNotPresent", func() {
+					containers := deploy.Spec.Template.Spec.Containers
+					Expect(containers).To(HaveLen(3))
+					for _, container := range containers {
+						Expect(container.ImagePullPolicy).To(Equal(corev1.PullIfNotPresent))
+					}
+				})
+			})
+			Context("with latest", func() {
+				BeforeEach(func() {
+					coreImg := "my/core-image:latest"
+					datasourceImg := "my/datasource-image:latest"
+					grafanaImg := "my/grafana-image:latest"
+					t.EnvCoreImageTag = &coreImg
+					t.EnvDatasourceImageTag = &datasourceImg
+					t.EnvGrafanaImageTag = &grafanaImg
+				})
+				It("should create deployment with the expected tags", func() {
+					t.checkDeployment()
+				})
+				It("should set ImagePullPolicy to Always", func() {
+					containers := deploy.Spec.Template.Spec.Containers
+					Expect(containers).To(HaveLen(3))
+					for _, container := range containers {
+						Expect(container.ImagePullPolicy).To(Equal(corev1.PullAlways))
+					}
+				})
 			})
 		})
 		Context("when deleted", func() {