Skip to content

Commit d95be1b

Browse files
lmicciniclaude
andcommitted
Resolve InstanceHa container image from ConfigMap before fallback
The webhook was always defaulting spec.containerImage, which meant the controller never reached the ConfigMap lookup path. This made the infra-instanceha-config ConfigMap (managed by openstack-operator via OpenStackVersion) effectively dead code. Remove the ContainerImage webhook defaulting and reorder the resolution in GetContainerImage() so the priority is: 1. spec.containerImage (explicit user override) 2. infra-instanceha-config ConfigMap (openstack-operator managed) 3. RELATED_IMAGE env var / hardcoded fallback (standalone deployment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 66bf6fc commit d95be1b

4 files changed

Lines changed: 113 additions & 21 deletions

File tree

apis/instanceha/v1beta1/instanceha_webhook.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,6 @@ func (r *InstanceHa) Default() {
5858

5959
// Default - set defaults for this InstanceHa spec
6060
func (spec *InstanceHaSpec) Default() {
61-
if spec.ContainerImage == "" {
62-
spec.ContainerImage = instanceHaDefaults.ContainerImageURL
63-
}
6461
if spec.OpenStackCloud == "" {
6562
spec.OpenStackCloud = OpenStackCloud
6663
}

internal/controller/instanceha/instanceha_controller.go

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,33 +1005,26 @@ func (r *Reconciler) findInstanceHaWithPendingRotation(ctx context.Context, _ cl
10051005
return requests
10061006
}
10071007

1008-
// GetContainerImage returns the container image to use for the instance, either from
1009-
// the provided containerImage parameter or from the infra-instanceha-config ConfigMap
1008+
// GetContainerImage returns the container image for InstanceHa. Priority:
1009+
// 1. spec.containerImage (explicit user override)
1010+
// 2. infra-instanceha-config ConfigMap (managed by openstack-operator)
1011+
// 3. RELATED_IMAGE env var or hardcoded fallback (standalone deployment)
10101012
func (r *Reconciler) GetContainerImage(
10111013
ctx context.Context,
10121014
containerImage string,
10131015
src client.Object,
10141016
) (string, error) {
1015-
cm := &corev1.ConfigMap{}
1016-
instanceHaConfigMapName := "infra-instanceha-config"
1017-
10181017
if len(containerImage) > 0 {
10191018
return containerImage, nil
10201019
}
10211020

1022-
objectKey := client.ObjectKey{Namespace: src.GetNamespace(), Name: instanceHaConfigMapName}
1023-
err := r.Get(ctx, objectKey, cm)
1024-
if err != nil {
1025-
return "", err
1026-
}
1027-
1028-
if cm.Data == nil {
1029-
return util.GetEnvVar("RELATED_IMAGE_INFRA_INSTANCE_HA_IMAGE_URL_DEFAULT", ""), nil
1030-
}
1031-
1032-
if cmImage, exists := cm.Data["instanceha-image"]; exists {
1033-
return cmImage, nil
1021+
cm := &corev1.ConfigMap{}
1022+
objectKey := client.ObjectKey{Namespace: src.GetNamespace(), Name: "infra-instanceha-config"}
1023+
if err := r.Get(ctx, objectKey, cm); err == nil && cm.Data != nil {
1024+
if cmImage, exists := cm.Data["instanceha-image"]; exists && len(cmImage) > 0 {
1025+
return cmImage, nil
1026+
}
10341027
}
10351028

1036-
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)
1029+
return util.GetEnvVar("RELATED_IMAGE_INFRA_INSTANCE_HA_IMAGE_URL_DEFAULT", instancehav1.InstanceHaContainerImage), nil
10371030
}

internal/webhook/instanceha/v1beta1/instanceha_webhook_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ var _ = Describe("InstanceHa webhook", func() {
3636

3737
instanceha.Default()
3838

39+
Expect(instanceha.Spec.ContainerImage).To(BeEmpty())
3940
Expect(instanceha.Spec.OpenStackCloud).To(Equal(instancehav1beta1.OpenStackCloud))
4041
Expect(instanceha.Spec.OpenStackConfigMap).To(Equal("openstack-config"))
4142
Expect(instanceha.Spec.OpenStackConfigSecret).To(Equal("openstack-config-secret"))
@@ -47,6 +48,22 @@ var _ = Describe("InstanceHa webhook", func() {
4748
Expect(instanceha.Spec.MetricsTLS.CipherSuites).To(Equal("HIGH:!aNULL:!MD5:!RC4:!3DES:!kRSA"))
4849
})
4950

51+
It("should not override an explicitly set containerImage", func() {
52+
instanceha := &instancehav1beta1.InstanceHa{
53+
ObjectMeta: metav1.ObjectMeta{
54+
Name: "test-instanceha-explicit-image",
55+
Namespace: "default",
56+
},
57+
Spec: instancehav1beta1.InstanceHaSpec{
58+
ContainerImage: "my-custom-image:v1",
59+
},
60+
}
61+
62+
instanceha.Default()
63+
64+
Expect(instanceha.Spec.ContainerImage).To(Equal("my-custom-image:v1"))
65+
})
66+
5067
It("should not override explicitly set values", func() {
5168
instanceha := &instancehav1beta1.InstanceHa{
5269
ObjectMeta: metav1.ObjectMeta{

test/functional/instanceha_controller_test.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,91 @@ var _ = Describe("InstanceHa Controller", func() {
184184
})
185185
})
186186

187+
When("container image resolution", func() {
188+
BeforeEach(func() {
189+
DeferCleanup(k8sClient.Delete, ctx, th.CreateConfigMap(types.NamespacedName{
190+
Name: "openstack-config",
191+
Namespace: namespace,
192+
}, map[string]any{
193+
"clouds.yaml": "test-data",
194+
}))
195+
196+
DeferCleanup(k8sClient.Delete, ctx, th.CreateSecret(types.NamespacedName{
197+
Name: "openstack-config-secret",
198+
Namespace: namespace,
199+
}, map[string][]byte{
200+
"secure.yaml": []byte("test-data"),
201+
}))
202+
203+
DeferCleanup(k8sClient.Delete, ctx, th.CreateSecret(types.NamespacedName{
204+
Name: "fencing-secret",
205+
Namespace: namespace,
206+
}, map[string][]byte{
207+
"fencing.yaml": []byte("test-data"),
208+
}))
209+
})
210+
211+
It("should use explicit containerImage from the CR when set", func() {
212+
spec := GetDefaultInstanceHaSpec()
213+
spec["containerImage"] = "my-custom-image:v1"
214+
ih := CreateInstanceHaConfig(namespace, spec)
215+
instanceHaName.Name = ih.GetName()
216+
instanceHaName.Namespace = ih.GetNamespace()
217+
DeferCleanup(th.DeleteInstance, ih)
218+
219+
// Also create the ConfigMap to ensure CR takes precedence
220+
DeferCleanup(k8sClient.Delete, ctx, th.CreateConfigMap(types.NamespacedName{
221+
Name: "infra-instanceha-config",
222+
Namespace: namespace,
223+
}, map[string]any{
224+
"instanceha-image": "configmap-image:latest",
225+
}))
226+
227+
Eventually(func(g Gomega) {
228+
dep := &appsv1.Deployment{}
229+
g.Expect(k8sClient.Get(ctx, instanceHaName, dep)).Should(Succeed())
230+
g.Expect(dep.Spec.Template.Spec.Containers[0].Image).To(Equal("my-custom-image:v1"))
231+
}, timeout, interval).Should(Succeed())
232+
})
233+
234+
It("should use ConfigMap image when containerImage is not set on the CR", func() {
235+
spec := GetDefaultInstanceHaSpec()
236+
delete(spec, "containerImage")
237+
ih := CreateInstanceHaConfig(namespace, spec)
238+
instanceHaName.Name = ih.GetName()
239+
instanceHaName.Namespace = ih.GetNamespace()
240+
DeferCleanup(th.DeleteInstance, ih)
241+
242+
DeferCleanup(k8sClient.Delete, ctx, th.CreateConfigMap(types.NamespacedName{
243+
Name: "infra-instanceha-config",
244+
Namespace: namespace,
245+
}, map[string]any{
246+
"instanceha-image": "configmap-image:latest",
247+
}))
248+
249+
Eventually(func(g Gomega) {
250+
dep := &appsv1.Deployment{}
251+
g.Expect(k8sClient.Get(ctx, instanceHaName, dep)).Should(Succeed())
252+
g.Expect(dep.Spec.Template.Spec.Containers[0].Image).To(Equal("configmap-image:latest"))
253+
}, timeout, interval).Should(Succeed())
254+
})
255+
256+
It("should fall back to the default image when neither CR nor ConfigMap provide one", func() {
257+
spec := GetDefaultInstanceHaSpec()
258+
delete(spec, "containerImage")
259+
ih := CreateInstanceHaConfig(namespace, spec)
260+
instanceHaName.Name = ih.GetName()
261+
instanceHaName.Namespace = ih.GetNamespace()
262+
DeferCleanup(th.DeleteInstance, ih)
263+
264+
Eventually(func(g Gomega) {
265+
dep := &appsv1.Deployment{}
266+
g.Expect(k8sClient.Get(ctx, instanceHaName, dep)).Should(Succeed())
267+
g.Expect(dep.Spec.Template.Spec.Containers[0].Image).To(Equal(instancehav1.InstanceHaContainerImage))
268+
}, timeout, interval).Should(Succeed())
269+
})
270+
})
271+
187272
When("MetricsTLS is configured without the TLS secret", func() {
188273
BeforeEach(func() {
189274
spec := GetDefaultInstanceHaSpec()

0 commit comments

Comments
 (0)