diff --git a/apis/bases/instanceha.openstack.org_instancehas.yaml b/apis/bases/instanceha.openstack.org_instancehas.yaml index dce86d41..880f8196 100644 --- a/apis/bases/instanceha.openstack.org_instancehas.yaml +++ b/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" diff --git a/apis/instanceha/v1beta1/instanceha_types.go b/apis/instanceha/v1beta1/instanceha_types.go index e75eb86c..7fb181c9 100644 --- a/apis/instanceha/v1beta1/instanceha_types.go +++ b/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 diff --git a/apis/instanceha/v1beta1/instanceha_webhook.go b/apis/instanceha/v1beta1/instanceha_webhook.go index a09e874d..605c10f0 100644 --- a/apis/instanceha/v1beta1/instanceha_webhook.go +++ b/apis/instanceha/v1beta1/instanceha_webhook.go @@ -61,9 +61,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 } diff --git a/config/crd/bases/instanceha.openstack.org_instancehas.yaml b/config/crd/bases/instanceha.openstack.org_instancehas.yaml index dce86d41..880f8196 100644 --- a/config/crd/bases/instanceha.openstack.org_instancehas.yaml +++ b/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" diff --git a/internal/controller/instanceha/instanceha_controller.go b/internal/controller/instanceha/instanceha_controller.go index 913335dc..1b78d8f7 100644 --- a/internal/controller/instanceha/instanceha_controller.go +++ b/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 } diff --git a/internal/webhook/instanceha/v1beta1/instanceha_webhook_test.go b/internal/webhook/instanceha/v1beta1/instanceha_webhook_test.go index 02451698..7e41f609 100644 --- a/internal/webhook/instanceha/v1beta1/instanceha_webhook_test.go +++ b/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{ diff --git a/test/functional/instanceha_controller_test.go b/test/functional/instanceha_controller_test.go index f35a0081..3684a3eb 100644 --- a/test/functional/instanceha_controller_test.go +++ b/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()