Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
csrwng wants to merge 1 commit into
base: main
Choose a base branch
from
+98 −14
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions hypershift-operator/controllers/nodepool/nodepool_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ const (
// possible values are 'required' (i.e. IMDSv2) or 'optional' which is the default.
ec2InstanceMetadataHTTPTokensAnnotation = "hypershift.openshift.io/ec2-instance-metadata-http-tokens"

nodePoolAnnotationPlatformMachineTemplate = "hypershift.openshift.io/nodePoolPlatformMachineTemplate"
nodePoolAnnotationTaints = "hypershift.openshift.io/nodePoolTaints"
nodePoolAnnotationPlatformMachineTemplate = "hypershift.openshift.io/nodePoolPlatformMachineTemplate"
nodePoolAnnotationTaints = "hypershift.openshift.io/nodePoolTaints"
nodePoolAnnotationCanonicalDataPlaneImages = "hypershift.openshift.io/canonical-data-plane-images"

@enxebre enxebre Jun 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we // doc this annotation? You pr desc seems pretty explanatory "Gate the fix behind a hypershift.openshift.io/canonical-data-plane-images annotation to avoid triggering rollouts on existing stable NodePools. The annotation is set automatically on new NodePools and during version upgrades."

nodePoolCoreIgnitionConfigLabel = "hypershift.openshift.io/core-ignition-config"

tuningConfigKey = "tuning"
Expand Down Expand Up @@ -1089,7 +1090,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,
// any management-cluster registry overrides are reversed. 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, releaseProvider releaseinfo.Provider, useCanonicalImages bool) (string, error) {
if annotationImage := strings.TrimSpace(nodePool.Annotations[hyperv1.NodePoolHAProxyImageAnnotation]); annotationImage != "" {
return annotationImage, nil
}
Expand All @@ -1102,11 +1109,32 @@ func resolveHAProxyImage(nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedClu
if !ok {
return "", fmt.Errorf("release image doesn't have a %s image", haproxy.HAProxyRouterImageName)
}

if useCanonicalImages {
if p, ok := releaseProvider.(releaseinfo.ProviderWithRegistryOverrides); ok {
for registrySource, registryDest := range p.GetRegistryOverrides() {
haProxyImage = strings.Replace(haProxyImage, registryDest, registrySource, 1)
}
}
}

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, r.ReleaseProvider, useCanonicalImages)
if err != nil {
return "", err
}
Expand Down
67 changes: 61 additions & 6 deletions hypershift-operator/controllers/nodepool/nodepool_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2507,11 +2507,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
registryOverrides map[string]string
componentImage string
expectedImage string
}{
{
name: "When NodePool annotation is set it should use annotation image",
Expand Down Expand Up @@ -2566,6 +2569,49 @@ func TestResolveHAProxyImage(t *testing.T) {
useSharedIngress: false,
expectedImage: testReleaseImage,
},
{
name: "When registry overrides exist and NodePool is new it should reverse them for the release payload image",
useSharedIngress: false,
componentImage: "mirror.example.com/openshift/haproxy-router:v4.16",
registryOverrides: map[string]string{"registry.test.io": "mirror.example.com"},
expectedImage: "registry.test.io/openshift/haproxy-router:v4.16",
},
{
name: "When registry overrides exist and NodePool is upgrading it should reverse them",
nodePoolStatusVersion: "4.17.0",
useSharedIngress: false,
componentImage: "mirror.example.com/openshift/haproxy-router:v4.16",
registryOverrides: map[string]string{"registry.test.io": "mirror.example.com"},
expectedImage: "registry.test.io/openshift/haproxy-router:v4.16",
},
{
name: "When registry overrides exist and canonical-data-plane-images annotation is set it should reverse them",
nodePoolAnnotations: map[string]string{
nodePoolAnnotationCanonicalDataPlaneImages: "true",
},
nodePoolStatusVersion: "4.18.0",
useSharedIngress: false,
componentImage: "mirror.example.com/openshift/haproxy-router:v4.16",
registryOverrides: map[string]string{"registry.test.io": "mirror.example.com"},
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",
registryOverrides: map[string]string{"registry.test.io": "mirror.example.com"},
expectedImage: "mirror.example.com/openshift/haproxy-router:v4.16",
},
{
name: "When registry overrides exist it should not reverse them for an annotation image",
nodePoolAnnotations: map[string]string{
hyperv1.NodePoolHAProxyImageAnnotation: "mirror.example.com/custom/haproxy:latest",
},
componentImage: "mirror.example.com/openshift/haproxy-router:v4.16",
registryOverrides: map[string]string{"registry.test.io": "mirror.example.com"},
expectedImage: "mirror.example.com/custom/haproxy:latest",
},
}

for _, tc := range testCases {
Expand All @@ -2587,6 +2633,9 @@ func TestResolveHAProxyImage(t *testing.T) {
Namespace: "clusters",
Annotations: tc.nodePoolAnnotations,
},
Status: hyperv1.NodePoolStatus{
Version: tc.nodePoolStatusVersion,
},
}

// Create kubeconfig secret
Expand Down Expand Up @@ -2657,11 +2706,17 @@ 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{
Components: map[string]string{
haproxy.HAProxyRouterImageName: testReleaseImage,
haproxy.HAProxyRouterImageName: componentImage,
},
FakeRegistryOverrides: tc.registryOverrides,
}

// Create test HostedCluster
Expand Down
9 changes: 5 additions & 4 deletions support/releaseinfo/fake/fake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ type FakeReleaseProvider struct {
// Version of the returned release image. Defaults to 4.14.0 if unset.
Version string
// Allows image-based versioning
ImageVersion map[string]string
Components map[string]string
ImageVersion map[string]string
Components map[string]string
FakeRegistryOverrides map[string]string
}

func (f *FakeReleaseProvider) Lookup(_ context.Context, image string, _ []byte) (*releaseinfo.ReleaseImage, error) {
Expand Down Expand Up @@ -134,8 +135,8 @@ func (f *FakeReleaseProvider) Lookup(_ context.Context, image string, _ []byte)
return releaseImage, nil
}

func (*FakeReleaseProvider) GetRegistryOverrides() map[string]string {
return nil
func (f *FakeReleaseProvider) GetRegistryOverrides() map[string]string {
return f.FakeRegistryOverrides
}

func (*FakeReleaseProvider) GetOpenShiftImageRegistryOverrides() map[string][]string {
Expand Down
Loading