diff --git a/hypershift-operator/controllers/nodepool/nodepool_controller.go b/hypershift-operator/controllers/nodepool/nodepool_controller.go index 2a170c1137de..0cad871fa155 100644 --- a/hypershift-operator/controllers/nodepool/nodepool_controller.go +++ b/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 } diff --git a/hypershift-operator/controllers/nodepool/nodepool_controller_test.go b/hypershift-operator/controllers/nodepool/nodepool_controller_test.go index 4c2e77bd6c37..cc560748d6e9 100644 --- a/hypershift-operator/controllers/nodepool/nodepool_controller_test.go +++ b/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 diff --git a/support/releaseinfo/fake/fake.go b/support/releaseinfo/fake/fake.go index bd5866aa04a0..7883b90091a8 100644 --- a/support/releaseinfo/fake/fake.go +++ b/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 diff --git a/support/releaseinfo/registry_mirror_provider.go b/support/releaseinfo/registry_mirror_provider.go index 4f582cf51a0d..dc601a7bc41a 100644 --- a/support/releaseinfo/registry_mirror_provider.go +++ b/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 { diff --git a/support/releaseinfo/releaseinfo.go b/support/releaseinfo/releaseinfo.go index c95c575b4385..2bd482698cd1 100644 --- a/support/releaseinfo/releaseinfo.go +++ b/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 {