OCPBUGS-4955: Set ImagePullPolicy of bundle unpacker to "IfNotPresent" for image digests

pkg/controller/bundle/bundle_unpacker.go

@@ -29,6 +29,7 @@ import (
 	listersoperatorsv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/image"
 )
 
 const (
@@ -170,7 +171,7 @@ func (c *ConfigMapUnpacker) job(cmRef *corev1.ObjectReference, bundlePath string
 						{
 							Name:            "pull",
 							Image:           bundlePath,
-							ImagePullPolicy: "Always",
+							ImagePullPolicy: image.InferImagePullPolicy(c.utilImage),
 							Command:         []string{"/util/cpb", "/bundle"}, // Copy bundle content to its mount
 							VolumeMounts: []corev1.VolumeMount{
 								{

pkg/controller/bundle/bundle_unpacker_test.go

@@ -33,6 +33,7 @@ const (
 	opmImage    = "opm-image"
 	utilImage   = "util-image"
 	bundlePath  = "bundle-path"
+	digestImage = "bundle@sha256:54d626e08c1c802b305dad30b7e54a82f102390cc92c7d4db112048935236e9c"
 	runAsUser   = 1001
 )
 
@@ -398,7 +399,7 @@ func TestConfigMapUnpacker(t *testing.T) {
 			},
 		},
 		{
-			description: "CatalogSourcePresent/ConfigMapPresent/JobPresent/Unpacked",
+			description: "CatalogSourcePresent/ConfigMapPresent/JobPresent/DigestImage/Unpacked",
 			fields: fields{
 				objs: []runtime.Object{
 					&batchv1.Job{
@@ -488,8 +489,8 @@ func TestConfigMapUnpacker(t *testing.T) {
 										},
 										{
 											Name:            "pull",
-											Image:           bundlePath,
-											ImagePullPolicy: "Always",
+											Image:           digestImage,
+											ImagePullPolicy: "IfNotPresent",
 											Command:         []string{"/util/cpb", "/bundle"}, // Copy bundle content to its mount
 											VolumeMounts: []corev1.VolumeMount{
 												{
@@ -731,8 +732,8 @@ func TestConfigMapUnpacker(t *testing.T) {
 										},
 										{
 											Name:            "pull",
-											Image:           bundlePath,
-											ImagePullPolicy: "Always",
+											Image:           digestImage,
+											ImagePullPolicy: "IfNotPresent",
 											Command:         []string{"/util/cpb", "/bundle"}, // Copy bundle content to its mount
 											VolumeMounts: []corev1.VolumeMount{
 												{

pkg/controller/registry/reconciler/reconciler.go

@@ -4,7 +4,6 @@ package reconciler
 import (
 	"fmt"
 	"hash/fnv"
-	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -14,6 +13,7 @@ import (
 
 	operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
 	controllerclient "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/controller-runtime/client"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/image"
 	hashutil "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/kubernetes/pkg/util/hash"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister"
@@ -107,17 +107,7 @@ func NewRegistryReconcilerFactory(lister operatorlister.OperatorLister, opClient
 	}
 }
 
-func Pod(source *operatorsv1alpha1.CatalogSource, name string, image string, saName string, labels map[string]string, annotations map[string]string, readinessDelay int32, livenessDelay int32, runAsUser int64) *corev1.Pod {
-	// Ensure the catalog image is always pulled if the image is not based on a digest, measured by whether an "@" is included.
-	// See https://github.com/docker/distribution/blob/master/reference/reference.go for more info.
-	// This means recreating non-digest based catalog pods will result in the latest version of the catalog content being delivered on-cluster.
-	var pullPolicy corev1.PullPolicy
-	if strings.Contains(image, "@") {
-		pullPolicy = corev1.PullIfNotPresent
-	} else {
-		pullPolicy = corev1.PullAlways
-	}
-
+func Pod(source *operatorsv1alpha1.CatalogSource, name string, img string, saName string, labels map[string]string, annotations map[string]string, readinessDelay int32, livenessDelay int32, runAsUser int64) *corev1.Pod {
 	// make a copy of the labels and annotations to avoid mutating the input parameters
 	podLabels := make(map[string]string)
 	podAnnotations := make(map[string]string)
@@ -141,7 +131,7 @@ func Pod(source *operatorsv1alpha1.CatalogSource, name string, image string, saN
 			Containers: []corev1.Container{
 				{
 					Name:  name,
-					Image: image,
+					Image: img,
 					Ports: []corev1.ContainerPort{
 						{
 							Name:          "grpc",
@@ -184,7 +174,7 @@ func Pod(source *operatorsv1alpha1.CatalogSource, name string, image string, saN
 					SecurityContext: &corev1.SecurityContext{
 						ReadOnlyRootFilesystem: pointer.Bool(false),
 					},
-					ImagePullPolicy:          pullPolicy,
+					ImagePullPolicy:          image.InferImagePullPolicy(img),
 					TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
 				},
 			},

pkg/lib/image/image.go

@@ -0,0 +1,17 @@
+package image
+
+import (
+	"strings"
+
+	corev1 "k8s.io/api/core/v1"
+)
+
+func InferImagePullPolicy(image string) corev1.PullPolicy {
+	// Ensure the image is always pulled if the image is not based on a digest, measured by whether an "@" is included.
+	// See https://github.com/docker/distribution/blob/master/reference/reference.go for more info.
+	// This means recreating non-digest based pods will result in the latest version of the content being delivered on-cluster.
+	if strings.Contains(image, "@") {
+		return corev1.PullIfNotPresent
+	}
+	return corev1.PullAlways
+}

pkg/lib/image/image_test.go

@@ -0,0 +1,35 @@
+package image_test
+
+import (
+	"testing"
+
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/image"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+)
+
+func TestInferImagePullPolicy(t *testing.T) {
+	tests := []struct {
+		description    string
+		img            string
+		expectedPolicy corev1.PullPolicy
+	}{
+		{
+			description:    "WithImageTag",
+			img:            "my-image:my-tag",
+			expectedPolicy: corev1.PullAlways,
+		},
+		{
+			description:    "WithImageDigest",
+			img:            "my-image@sha256:54d626e08c1c802b305dad30b7e54a82f102390cc92c7d4db112048935236e9c",
+			expectedPolicy: corev1.PullIfNotPresent,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.description, func(t *testing.T) {
+			policy := image.InferImagePullPolicy(tt.img)
+			require.Equal(t, tt.expectedPolicy, policy)
+		})
+	}
+}