OCPBUGS-86415: Use canonical image for kube-apiserver-proxy static pod

hypershift-operator/controllers/nodepool/nodepool_controller.go

@@ -76,7 +76,12 @@ const (
 
 	nodePoolAnnotationPlatformMachineTemplate = "hypershift.openshift.io/nodePoolPlatformMachineTemplate"
 	nodePoolAnnotationTaints                  = "hypershift.openshift.io/nodePoolTaints"
-	nodePoolCoreIgnitionConfigLabel           = "hypershift.openshift.io/core-ignition-config"
+	// nodePoolAnnotationCanonicalDataPlaneImages gates the use of canonical
+	// (pre-override) image references for data plane static pods. Set automatically
+	// on new NodePools and during version upgrades to avoid triggering rollouts on
+	// existing stable NodePools.
+	nodePoolAnnotationCanonicalDataPlaneImages = "hypershift.openshift.io/canonical-data-plane-images"
+	nodePoolCoreIgnitionConfigLabel            = "hypershift.openshift.io/core-ignition-config"
 
 	tuningConfigKey                                      = "tuning"
 	tunedConfigMapLabel                                  = "hypershift.openshift.io/tuned-config"
@@ -1096,7 +1101,13 @@ func (r *NodePoolReconciler) getAdditionalTrustBundle(ctx context.Context, hoste
 // 1. NodePool annotation (highest priority)
 // 2. Shared ingress image (when cluster uses shared ingress for public endpoints)
 // 3. Release payload (default)
-func resolveHAProxyImage(nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, releaseImage *releaseinfo.ReleaseImage) (string, error) {
+//
+// When useCanonicalImages is true and the image comes from the release payload,
+// canonical (pre-override) component images are used. The HAProxy image is
+// embedded in a static pod manifest that runs on data plane nodes, where CRI-O
+// handles mirroring natively via IDMS/ICSP — so the canonical (non-overridden)
+// image reference must be used.
+func resolveHAProxyImage(nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, releaseImage *releaseinfo.ReleaseImage, useCanonicalImages bool) (string, error) {
 	if annotationImage := strings.TrimSpace(nodePool.Annotations[hyperv1.NodePoolHAProxyImageAnnotation]); annotationImage != "" {
 		return annotationImage, nil
 	}
@@ -1105,15 +1116,33 @@ func resolveHAProxyImage(nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedClu
 		return images.GetSharedIngressHAProxyImage(), nil
 	}
 
-	haProxyImage, ok := releaseImage.ComponentImages()[haproxy.HAProxyRouterImageName]
+	componentImages := releaseImage.ComponentImages()
+	if useCanonicalImages {
+		componentImages = releaseImage.CanonicalComponentImages()
+	}
+
+	haProxyImage, ok := componentImages[haproxy.HAProxyRouterImageName]
 	if !ok {
 		return "", fmt.Errorf("release image doesn't have a %s image", haproxy.HAProxyRouterImageName)
 	}
+
 	return haProxyImage, nil
 }
 
 func (r *NodePoolReconciler) generateHAProxyRawConfig(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, releaseImage *releaseinfo.ReleaseImage) (string, error) {
-	haProxyImage, err := resolveHAProxyImage(nodePool, hcluster, releaseImage)
+	useCanonicalImages := nodePool.Annotations[nodePoolAnnotationCanonicalDataPlaneImages] == "true"
+	if !useCanonicalImages {
+		isNewOrUpgrading := nodePool.Status.Version == "" || nodePool.Status.Version != releaseImage.Version()
+		if isNewOrUpgrading {
+			useCanonicalImages = true
+			if nodePool.Annotations == nil {
+				nodePool.Annotations = make(map[string]string)
+			}
+			nodePool.Annotations[nodePoolAnnotationCanonicalDataPlaneImages] = "true"
+		}
+	}
+
+	haProxyImage, err := resolveHAProxyImage(nodePool, hcluster, releaseImage, useCanonicalImages)
 	if err != nil {
 		return "", err
 	}

hypershift-operator/controllers/nodepool/nodepool_controller_test.go

@@ -2519,11 +2519,14 @@ func TestResolveHAProxyImage(t *testing.T) {
 	)
 
 	testCases := []struct {
-		name                string
-		nodePoolAnnotations map[string]string
-		useSharedIngress    bool
-		envVarImage         string
-		expectedImage       string
+		name                  string
+		nodePoolAnnotations   map[string]string
+		nodePoolStatusVersion string
+		useSharedIngress      bool
+		envVarImage           string
+		canonicalComponents   map[string]string
+		componentImage        string
+		expectedImage         string
 	}{
 		{
 			name: "When NodePool annotation is set it should use annotation image",
@@ -2578,6 +2581,49 @@ func TestResolveHAProxyImage(t *testing.T) {
 			useSharedIngress: false,
 			expectedImage:    testReleaseImage,
 		},
+		{
+			name:                "When registry overrides exist and NodePool is new it should use canonical image",
+			useSharedIngress:    false,
+			componentImage:      "mirror.example.com/openshift/haproxy-router:v4.16",
+			canonicalComponents: map[string]string{haproxy.HAProxyRouterImageName: "registry.test.io/openshift/haproxy-router:v4.16"},
+			expectedImage:       "registry.test.io/openshift/haproxy-router:v4.16",
+		},
+		{
+			name:                  "When registry overrides exist and NodePool is upgrading it should use canonical image",
+			nodePoolStatusVersion: "4.17.0",
+			useSharedIngress:      false,
+			componentImage:        "mirror.example.com/openshift/haproxy-router:v4.16",
+			canonicalComponents:   map[string]string{haproxy.HAProxyRouterImageName: "registry.test.io/openshift/haproxy-router:v4.16"},
+			expectedImage:         "registry.test.io/openshift/haproxy-router:v4.16",
+		},
+		{
+			name: "When registry overrides exist and canonical-data-plane-images annotation is set it should use canonical image",
+			nodePoolAnnotations: map[string]string{
+				nodePoolAnnotationCanonicalDataPlaneImages: "true",
+			},
+			nodePoolStatusVersion: "4.18.0",
+			useSharedIngress:      false,
+			componentImage:        "mirror.example.com/openshift/haproxy-router:v4.16",
+			canonicalComponents:   map[string]string{haproxy.HAProxyRouterImageName: "registry.test.io/openshift/haproxy-router:v4.16"},
+			expectedImage:         "registry.test.io/openshift/haproxy-router:v4.16",
+		},
+		{
+			name:                  "When registry overrides exist and NodePool is stable without annotation it should preserve overridden image",
+			nodePoolStatusVersion: "4.18.0",
+			useSharedIngress:      false,
+			componentImage:        "mirror.example.com/openshift/haproxy-router:v4.16",
+			canonicalComponents:   map[string]string{haproxy.HAProxyRouterImageName: "registry.test.io/openshift/haproxy-router:v4.16"},
+			expectedImage:         "mirror.example.com/openshift/haproxy-router:v4.16",
+		},
+		{
+			name: "When registry overrides exist it should not affect an annotation image",
+			nodePoolAnnotations: map[string]string{
+				hyperv1.NodePoolHAProxyImageAnnotation: "mirror.example.com/custom/haproxy:latest",
+			},
+			componentImage:      "mirror.example.com/openshift/haproxy-router:v4.16",
+			canonicalComponents: map[string]string{haproxy.HAProxyRouterImageName: "registry.test.io/openshift/haproxy-router:v4.16"},
+			expectedImage:       "mirror.example.com/custom/haproxy:latest",
+		},
 	}
 
 	for _, tc := range testCases {
@@ -2599,6 +2645,9 @@ func TestResolveHAProxyImage(t *testing.T) {
 					Namespace:   "clusters",
 					Annotations: tc.nodePoolAnnotations,
 				},
+				Status: hyperv1.NodePoolStatus{
+					Version: tc.nodePoolStatusVersion,
+				},
 			}
 
 			// Create kubeconfig secret
@@ -2669,11 +2718,18 @@ kind: Config`),
 			// Create fake client
 			c := fake.NewClientBuilder().WithObjects(objects...).Build()
 
+			componentImage := testReleaseImage
+			if tc.componentImage != "" {
+				componentImage = tc.componentImage
+			}
+
 			// Create fake release provider with component images
 			releaseProvider := &fakereleaseprovider.FakeReleaseProvider{
+				Version: "4.18.0",
 				Components: map[string]string{
-					haproxy.HAProxyRouterImageName: testReleaseImage,
+					haproxy.HAProxyRouterImageName: componentImage,
 				},
+				CanonicalComponents: tc.canonicalComponents,
 			}
 
 			// Create test HostedCluster

support/releaseinfo/fake/fake.go

@@ -28,6 +28,9 @@ type FakeReleaseProvider struct {
 	// Allows image-based versioning
 	ImageVersion map[string]string
 	Components   map[string]string
+	// CanonicalComponents, when set, will be stored as canonical (pre-override)
+	// component images on the returned ReleaseImage.
+	CanonicalComponents map[string]string
 }
 
 func (f *FakeReleaseProvider) Lookup(_ context.Context, image string, _ []byte) (*releaseinfo.ReleaseImage, error) {
@@ -120,6 +123,10 @@ func (f *FakeReleaseProvider) Lookup(_ context.Context, image string, _ []byte)
 		})
 	}
 
+	if f.CanonicalComponents != nil {
+		releaseImage.SetCanonicalComponentImages(f.CanonicalComponents)
+	}
+
 	if len(f.ImageVersion) == 0 {
 		if f.Version != "" {
 			releaseImage.ImageStream.Name = f.Version

support/releaseinfo/registry_mirror_provider.go

@@ -33,16 +33,22 @@ func (p *RegistryMirrorProviderDecorator) Lookup(ctx context.Context, image stri
 		return nil, err
 	}
 
+	canonicalImages := releaseImage.CanonicalComponentImages()
+
 	imageStream := releaseImage.ImageStream.DeepCopy() // deepCopy so the cache is not overridden.
 	for i := range imageStream.Spec.Tags {
 		imageStream.Spec.Tags[i].From.Name = registryoverride.Replace(imageStream.Spec.Tags[i].From.Name, p.RegistryOverrides)
 	}
 
-	return &ReleaseImage{
+	result := &ReleaseImage{
 		ImageStream:    imageStream,
 		StreamMetadata: releaseImage.StreamMetadata,
 		OSStreams:      releaseImage.OSStreams,
-	}, nil
+	}
+	if len(canonicalImages) > 0 {
+		result.SetCanonicalComponentImages(canonicalImages)
+	}
+	return result, nil
 }
 
 func (p *RegistryMirrorProviderDecorator) GetRegistryOverrides() map[string]string {

support/releaseinfo/releaseinfo.go

@@ -48,6 +48,10 @@ type ReleaseImage struct {
 	// OSStreams holds per-stream metadata parsed from the ConfigMap "streams" key.
 	// Nil for single-stream payloads (OCP < 5.0).
 	OSStreams map[string]*stream.Stream `json:"-"`
+
+	// canonicalComponentImages holds component images before any registry
+	// overrides are applied. Set by RegistryMirrorProviderDecorator.
+	canonicalComponentImages map[string]string
 }
 
 // StreamForName returns stream metadata by name. If name is empty, returns
@@ -94,6 +98,21 @@ func (i *ReleaseImage) ComponentImages() map[string]string {
 	return images
 }
 
+// CanonicalComponentImages returns the component images before any registry
+// overrides were applied. If no canonical images were captured (e.g. no
+// registry overrides are configured), it falls back to ComponentImages().
+func (i *ReleaseImage) CanonicalComponentImages() map[string]string {
+	if i.canonicalComponentImages != nil {
+		return i.canonicalComponentImages
+	}
+	return i.ComponentImages()
+}
+
+// SetCanonicalComponentImages stores the pre-override component images.
+func (i *ReleaseImage) SetCanonicalComponentImages(images map[string]string) {
+	i.canonicalComponentImages = images
+}
+
 func (i *ReleaseImage) ComponentVersions() (map[string]string, error) {
 	componentVersions, err := readComponentVersions(i.ImageStream)
 	if err := errors.NewAggregate(err); err != nil {