Merge pull request #641 from openshift-cherrypick-robot/cherry-pick-634-to-18-stable

[18-stable] Resolve InstanceHa container image from ConfigMap before fallback

Author: openshift-merge-bot[bot]

apis/bases/instanceha.openstack.org_instancehas.yaml

@@ -68,8 +68,9 @@ spec:
                   bundle file
                 type: string
               containerImage:
-                description: ContainerImage for the InstanceHa container (will be
-                  set to environmental default if empty)
+                description: |-
+                  ContainerImage for the InstanceHa container. If empty, resolved from
+                  the infra-instanceha-config ConfigMap or the RELATED_IMAGE env var.
                 type: string
               disabled:
                 default: "False"

apis/instanceha/v1beta1/instanceha_types.go

@@ -77,7 +77,8 @@ type InstanceHaMetricsTLS struct {
 // InstanceHaSpec defines the desired state of InstanceHa
 type InstanceHaSpec struct {
 	// +kubebuilder:validation:Optional
-	// ContainerImage for the InstanceHa container (will be set to environmental default if empty)
+	// ContainerImage for the InstanceHa container. If empty, resolved from
+	// the infra-instanceha-config ConfigMap or the RELATED_IMAGE env var.
 	ContainerImage string `json:"containerImage"`
 
 	// +kubebuilder:validation:Required

apis/instanceha/v1beta1/instanceha_webhook.go

@@ -58,9 +58,6 @@ func (r *InstanceHa) Default() {
 
 // Default - set defaults for this InstanceHa spec
 func (spec *InstanceHaSpec) Default() {
-	if spec.ContainerImage == "" {
-		spec.ContainerImage = instanceHaDefaults.ContainerImageURL
-	}
 	if spec.OpenStackCloud == "" {
 		spec.OpenStackCloud = OpenStackCloud
 	}

config/crd/bases/instanceha.openstack.org_instancehas.yaml

@@ -68,8 +68,9 @@ spec:
                   bundle file
                 type: string
               containerImage:
-                description: ContainerImage for the InstanceHa container (will be
-                  set to environmental default if empty)
+                description: |-
+                  ContainerImage for the InstanceHa container. If empty, resolved from
+                  the infra-instanceha-config ConfigMap or the RELATED_IMAGE env var.
                 type: string
               disabled:
                 default: "False"

internal/controller/instanceha/instanceha_controller.go

@@ -1005,33 +1005,30 @@ func (r *Reconciler) findInstanceHaWithPendingRotation(ctx context.Context, _ cl
 	return requests
 }
 
-// GetContainerImage returns the container image to use for the instance, either from
-// the provided containerImage parameter or from the infra-instanceha-config ConfigMap
+// GetContainerImage returns the container image for InstanceHa. Priority:
+// 1. spec.containerImage (explicit user override)
+// 2. infra-instanceha-config ConfigMap (managed by openstack-operator)
+// 3. RELATED_IMAGE env var or hardcoded fallback (standalone deployment)
 func (r *Reconciler) GetContainerImage(
 	ctx context.Context,
 	containerImage string,
 	src client.Object,
 ) (string, error) {
-	cm := &corev1.ConfigMap{}
-	instanceHaConfigMapName := "infra-instanceha-config"
-
 	if len(containerImage) > 0 {
 		return containerImage, nil
 	}
 
-	objectKey := client.ObjectKey{Namespace: src.GetNamespace(), Name: instanceHaConfigMapName}
-	err := r.Get(ctx, objectKey, cm)
-	if err != nil {
-		return "", err
-	}
-
-	if cm.Data == nil {
-		return util.GetEnvVar("RELATED_IMAGE_INFRA_INSTANCE_HA_IMAGE_URL_DEFAULT", ""), nil
-	}
-
-	if cmImage, exists := cm.Data["instanceha-image"]; exists {
-		return cmImage, nil
+	cm := &corev1.ConfigMap{}
+	objectKey := client.ObjectKey{Namespace: src.GetNamespace(), Name: "infra-instanceha-config"}
+	if err := r.Get(ctx, objectKey, cm); err != nil {
+		if !k8s_errors.IsNotFound(err) {
+			return "", fmt.Errorf("failed to get ConfigMap %s: %w", objectKey.Name, err)
+		}
+	} else if cm.Data != nil {
+		if cmImage, exists := cm.Data["instanceha-image"]; exists && len(cmImage) > 0 {
+			return cmImage, nil
+		}
 	}
 
-	return "", fmt.Errorf("no container image found: key 'instanceha-image' not in ConfigMap %s and RELATED_IMAGE_INFRA_INSTANCE_HA_IMAGE_URL_DEFAULT not set", instanceHaConfigMapName)
+	return util.GetEnvVar("RELATED_IMAGE_INSTANCE_HA_IMAGE_URL_DEFAULT", instancehav1.InstanceHaContainerImage), nil
 }

internal/webhook/instanceha/v1beta1/instanceha_webhook_test.go

@@ -36,6 +36,7 @@ var _ = Describe("InstanceHa webhook", func() {
 
 			instanceha.Default()
 
+			Expect(instanceha.Spec.ContainerImage).To(BeEmpty())
 			Expect(instanceha.Spec.OpenStackCloud).To(Equal(instancehav1beta1.OpenStackCloud))
 			Expect(instanceha.Spec.OpenStackConfigMap).To(Equal("openstack-config"))
 			Expect(instanceha.Spec.OpenStackConfigSecret).To(Equal("openstack-config-secret"))
@@ -47,6 +48,22 @@ var _ = Describe("InstanceHa webhook", func() {
 			Expect(instanceha.Spec.MetricsTLS.CipherSuites).To(Equal("HIGH:!aNULL:!MD5:!RC4:!3DES:!kRSA"))
 		})
 
+		It("should not override an explicitly set containerImage", func() {
+			instanceha := &instancehav1beta1.InstanceHa{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-instanceha-explicit-image",
+					Namespace: "default",
+				},
+				Spec: instancehav1beta1.InstanceHaSpec{
+					ContainerImage: "my-custom-image:v1",
+				},
+			}
+
+			instanceha.Default()
+
+			Expect(instanceha.Spec.ContainerImage).To(Equal("my-custom-image:v1"))
+		})
+
 		It("should not override explicitly set values", func() {
 			instanceha := &instancehav1beta1.InstanceHa{
 				ObjectMeta: metav1.ObjectMeta{

test/functional/instanceha_controller_test.go

@@ -184,6 +184,91 @@ var _ = Describe("InstanceHa Controller", func() {
 		})
 	})
 
+	When("container image resolution", func() {
+		BeforeEach(func() {
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateConfigMap(types.NamespacedName{
+				Name:      "openstack-config",
+				Namespace: namespace,
+			}, map[string]any{
+				"clouds.yaml": "test-data",
+			}))
+
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateSecret(types.NamespacedName{
+				Name:      "openstack-config-secret",
+				Namespace: namespace,
+			}, map[string][]byte{
+				"secure.yaml": []byte("test-data"),
+			}))
+
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateSecret(types.NamespacedName{
+				Name:      "fencing-secret",
+				Namespace: namespace,
+			}, map[string][]byte{
+				"fencing.yaml": []byte("test-data"),
+			}))
+		})
+
+		It("should use explicit containerImage from the CR when set", func() {
+			spec := GetDefaultInstanceHaSpec()
+			spec["containerImage"] = "my-custom-image:v1"
+			ih := CreateInstanceHaConfig(namespace, spec)
+			instanceHaName.Name = ih.GetName()
+			instanceHaName.Namespace = ih.GetNamespace()
+			DeferCleanup(th.DeleteInstance, ih)
+
+			// Also create the ConfigMap to ensure CR takes precedence
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateConfigMap(types.NamespacedName{
+				Name:      "infra-instanceha-config",
+				Namespace: namespace,
+			}, map[string]any{
+				"instanceha-image": "configmap-image:latest",
+			}))
+
+			Eventually(func(g Gomega) {
+				dep := &appsv1.Deployment{}
+				g.Expect(k8sClient.Get(ctx, instanceHaName, dep)).Should(Succeed())
+				g.Expect(dep.Spec.Template.Spec.Containers[0].Image).To(Equal("my-custom-image:v1"))
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("should use ConfigMap image when containerImage is not set on the CR", func() {
+			spec := GetDefaultInstanceHaSpec()
+			delete(spec, "containerImage")
+			ih := CreateInstanceHaConfig(namespace, spec)
+			instanceHaName.Name = ih.GetName()
+			instanceHaName.Namespace = ih.GetNamespace()
+			DeferCleanup(th.DeleteInstance, ih)
+
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateConfigMap(types.NamespacedName{
+				Name:      "infra-instanceha-config",
+				Namespace: namespace,
+			}, map[string]any{
+				"instanceha-image": "configmap-image:latest",
+			}))
+
+			Eventually(func(g Gomega) {
+				dep := &appsv1.Deployment{}
+				g.Expect(k8sClient.Get(ctx, instanceHaName, dep)).Should(Succeed())
+				g.Expect(dep.Spec.Template.Spec.Containers[0].Image).To(Equal("configmap-image:latest"))
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("should fall back to the default image when neither CR nor ConfigMap provide one", func() {
+			spec := GetDefaultInstanceHaSpec()
+			delete(spec, "containerImage")
+			ih := CreateInstanceHaConfig(namespace, spec)
+			instanceHaName.Name = ih.GetName()
+			instanceHaName.Namespace = ih.GetNamespace()
+			DeferCleanup(th.DeleteInstance, ih)
+
+			Eventually(func(g Gomega) {
+				dep := &appsv1.Deployment{}
+				g.Expect(k8sClient.Get(ctx, instanceHaName, dep)).Should(Succeed())
+				g.Expect(dep.Spec.Template.Spec.Containers[0].Image).To(Equal(instancehav1.InstanceHaContainerImage))
+			}, timeout, interval).Should(Succeed())
+		})
+	})
+
 	When("MetricsTLS is configured without the TLS secret", func() {
 		BeforeEach(func() {
 			spec := GetDefaultInstanceHaSpec()