Skip to content

Commit ad091ee

Browse files
jparrillclaude
andcommitted
feat(nodepool): decouple all platform boot image resolvers from ReleaseImage
Apply the same pattern from CNTRLPLANE-3026 (AWS) to all remaining platforms: GCP, Azure, PowerVS, OpenStack, and KubeVirt. Each leaf function now accepts *stream.Stream instead of *releaseinfo.ReleaseImage, and callers resolve the stream internally via StreamForName(""). - GCP: DefaultNodePoolGCPImage() exported, accepts *stream.Stream - PowerVS: getPowerVSImage() accepts *stream.Stream - Azure: getAzureMarketplaceMetadata() accepts *stream.Stream - OpenStack: OpenstackDefaultImage(), OpenStackReleaseImage(), PrefixedClusterImageName() accept *stream.Stream with nil guards - KubeVirt: defaultImage() accepts *stream.Stream, GetImage() resolves stream once and passes to both defaultImage and openstack.OpenstackDefaultImage (fallback path) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
1 parent 92957ce commit ad091ee

13 files changed

Lines changed: 238 additions & 182 deletions

File tree

hypershift-operator/controllers/nodepool/aws.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,9 @@ func resolveAWSAMI(hostedCluster *hyperv1.HostedCluster, nodePool *hyperv1.NodeP
134134
return ami, nil
135135
}
136136
// Default behavior for Linux/RHCOS AMIs
137+
if releaseImage == nil {
138+
return "", fmt.Errorf("release image is nil")
139+
}
137140
streamMeta, err := releaseImage.StreamForName("")
138141
if err != nil {
139142
return "", fmt.Errorf("couldn't resolve stream metadata: %w", err)
@@ -365,6 +368,16 @@ func (r *NodePoolReconciler) setAWSConditions(_ context.Context, nodePool *hyper
365368
})
366369
} else {
367370
// Default behavior for Linux/RHCOS AMIs
371+
if releaseImage == nil {
372+
SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{
373+
Type: hyperv1.NodePoolValidPlatformImageType,
374+
Status: corev1.ConditionFalse,
375+
Reason: hyperv1.NodePoolValidationFailedReason,
376+
Message: fmt.Sprintf("Release image metadata is nil for release image %q", nodePool.Spec.Release.Image),
377+
ObservedGeneration: nodePool.Generation,
378+
})
379+
return fmt.Errorf("release image is nil")
380+
}
368381
streamMeta, err := releaseImage.StreamForName("")
369382
if err != nil {
370383
SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{

hypershift-operator/controllers/nodepool/azure.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
capiazure "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
1717

1818
"github.com/blang/semver"
19+
"github.com/coreos/stream-metadata-go/stream"
1920
)
2021

2122
// dummySSHKey is a base64 encoded dummy SSH public key.
@@ -81,7 +82,11 @@ func defaultAzureNodePoolImage(nodePool *hyperv1.NodePool, releaseImage *release
8182
}
8283

8384
// Extract marketplace metadata from release payload
84-
azureMarketplace, err := getAzureMarketplaceMetadata(releaseImage, streamArch)
85+
streamMeta, err := releaseImage.StreamForName("")
86+
if err != nil {
87+
return fmt.Errorf("couldn't resolve stream metadata: %w", err)
88+
}
89+
azureMarketplace, err := getAzureMarketplaceMetadata(streamMeta, streamArch)
8590
if err != nil {
8691
return fmt.Errorf("failed to get Azure Marketplace metadata: %w", err)
8792
}
@@ -119,13 +124,13 @@ func defaultAzureNodePoolImage(nodePool *hyperv1.NodePool, releaseImage *release
119124
return nil
120125
}
121126

122-
// getAzureMarketplaceMetadata extracts Azure Marketplace metadata from the release payload
123-
func getAzureMarketplaceMetadata(releaseImage *releaseinfo.ReleaseImage, arch string) (*azureMarketplaceMetadata, error) {
124-
if releaseImage.StreamMetadata == nil {
127+
// getAzureMarketplaceMetadata extracts Azure Marketplace metadata from stream metadata.
128+
func getAzureMarketplaceMetadata(streamMeta *stream.Stream, arch string) (*azureMarketplaceMetadata, error) {
129+
if streamMeta == nil {
125130
return nil, nil // No stream metadata available
126131
}
127132

128-
archData, foundArch := releaseImage.StreamMetadata.Architectures[arch]
133+
archData, foundArch := streamMeta.Architectures[arch]
129134
if !foundArch {
130135
return nil, fmt.Errorf("architecture %s not found in stream metadata", arch)
131136
}

hypershift-operator/controllers/nodepool/gcp.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,11 @@ func resolveGCPImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.Relea
168168
}
169169

170170
// Resolve image from release metadata
171-
image, err := defaultNodePoolGCPImage(nodePool.Spec.Arch, releaseImage)
171+
streamMeta, err := releaseImage.StreamForName("")
172+
if err != nil {
173+
return "", fmt.Errorf("couldn't resolve stream metadata: %w", err)
174+
}
175+
image, err := DefaultNodePoolGCPImage(nodePool.Spec.Arch, streamMeta)
172176
if err != nil {
173177
return "", fmt.Errorf("couldn't discover a GCP image for release image: %w", err)
174178
}

hypershift-operator/controllers/nodepool/gcp_test.go

Lines changed: 48 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -535,63 +535,55 @@ func TestDefaultNodePoolGCPImage(t *testing.T) {
535535
testCases := []struct {
536536
name string
537537
arch string
538-
releaseImage *releaseinfo.ReleaseImage
538+
streamMeta *stream.Stream
539539
expectedImage string
540540
expectedErr bool
541541
expectedErrMsg string
542542
}{
543543
{
544544
name: "When stream metadata has project and name for amd64, it should construct image path",
545545
arch: hyperv1.ArchitectureAMD64,
546-
releaseImage: &releaseinfo.ReleaseImage{
547-
StreamMetadata: &stream.Stream{
548-
Architectures: map[string]stream.Arch{
549-
"x86_64": {
550-
Images: stream.Images{
551-
Gcp: &stream.GcpImage{
552-
Project: "rhcos-cloud",
553-
Name: "rhcos-9-6-20251023-0-gcp-x86-64",
554-
},
546+
streamMeta: &stream.Stream{
547+
Architectures: map[string]stream.Arch{
548+
"x86_64": {
549+
Images: stream.Images{
550+
Gcp: &stream.GcpImage{
551+
Project: "rhcos-cloud",
552+
Name: "rhcos-9-6-20251023-0-gcp-x86-64",
555553
},
556554
},
557555
},
558556
},
559557
},
560558
expectedImage: "projects/rhcos-cloud/global/images/rhcos-9-6-20251023-0-gcp-x86-64",
561-
expectedErr: false,
562559
},
563560
{
564561
name: "When stream metadata has project and name for arm64, it should construct image path",
565562
arch: hyperv1.ArchitectureARM64,
566-
releaseImage: &releaseinfo.ReleaseImage{
567-
StreamMetadata: &stream.Stream{
568-
Architectures: map[string]stream.Arch{
569-
"aarch64": {
570-
Images: stream.Images{
571-
Gcp: &stream.GcpImage{
572-
Project: "rhcos-cloud",
573-
Name: "rhcos-9-6-20251023-0-gcp-aarch64",
574-
},
563+
streamMeta: &stream.Stream{
564+
Architectures: map[string]stream.Arch{
565+
"aarch64": {
566+
Images: stream.Images{
567+
Gcp: &stream.GcpImage{
568+
Project: "rhcos-cloud",
569+
Name: "rhcos-9-6-20251023-0-gcp-aarch64",
575570
},
576571
},
577572
},
578573
},
579574
},
580575
expectedImage: "projects/rhcos-cloud/global/images/rhcos-9-6-20251023-0-gcp-aarch64",
581-
expectedErr: false,
582576
},
583577
{
584-
name: "When architecture is not found in release metadata, it should return error",
578+
name: "When architecture is not found in stream metadata, it should return error",
585579
arch: "unsupported-arch",
586-
releaseImage: &releaseinfo.ReleaseImage{
587-
StreamMetadata: &stream.Stream{
588-
Architectures: map[string]stream.Arch{
589-
"x86_64": {
590-
Images: stream.Images{
591-
Gcp: &stream.GcpImage{
592-
Project: "rhcos-cloud",
593-
Name: "rhcos-9-6-20251023-0-gcp-x86-64",
594-
},
580+
streamMeta: &stream.Stream{
581+
Architectures: map[string]stream.Arch{
582+
"x86_64": {
583+
Images: stream.Images{
584+
Gcp: &stream.GcpImage{
585+
Project: "rhcos-cloud",
586+
Name: "rhcos-9-6-20251023-0-gcp-x86-64",
595587
},
596588
},
597589
},
@@ -603,13 +595,11 @@ func TestDefaultNodePoolGCPImage(t *testing.T) {
603595
{
604596
name: "When GCP project and name are empty, it should return error",
605597
arch: hyperv1.ArchitectureAMD64,
606-
releaseImage: &releaseinfo.ReleaseImage{
607-
StreamMetadata: &stream.Stream{
608-
Architectures: map[string]stream.Arch{
609-
"x86_64": {
610-
Images: stream.Images{
611-
Gcp: &stream.GcpImage{},
612-
},
598+
streamMeta: &stream.Stream{
599+
Architectures: map[string]stream.Arch{
600+
"x86_64": {
601+
Images: stream.Images{
602+
Gcp: &stream.GcpImage{},
613603
},
614604
},
615605
},
@@ -620,14 +610,12 @@ func TestDefaultNodePoolGCPImage(t *testing.T) {
620610
{
621611
name: "When GCP project is empty but name is set, it should return error",
622612
arch: hyperv1.ArchitectureAMD64,
623-
releaseImage: &releaseinfo.ReleaseImage{
624-
StreamMetadata: &stream.Stream{
625-
Architectures: map[string]stream.Arch{
626-
"x86_64": {
627-
Images: stream.Images{
628-
Gcp: &stream.GcpImage{
629-
Name: "rhcos-x86-64",
630-
},
613+
streamMeta: &stream.Stream{
614+
Architectures: map[string]stream.Arch{
615+
"x86_64": {
616+
Images: stream.Images{
617+
Gcp: &stream.GcpImage{
618+
Name: "rhcos-x86-64",
631619
},
632620
},
633621
},
@@ -639,14 +627,12 @@ func TestDefaultNodePoolGCPImage(t *testing.T) {
639627
{
640628
name: "When GCP name is empty but project is set, it should return error",
641629
arch: hyperv1.ArchitectureAMD64,
642-
releaseImage: &releaseinfo.ReleaseImage{
643-
StreamMetadata: &stream.Stream{
644-
Architectures: map[string]stream.Arch{
645-
"x86_64": {
646-
Images: stream.Images{
647-
Gcp: &stream.GcpImage{
648-
Project: "rhcos-cloud",
649-
},
630+
streamMeta: &stream.Stream{
631+
Architectures: map[string]stream.Arch{
632+
"x86_64": {
633+
Images: stream.Images{
634+
Gcp: &stream.GcpImage{
635+
Project: "rhcos-cloud",
650636
},
651637
},
652638
},
@@ -655,13 +641,20 @@ func TestDefaultNodePoolGCPImage(t *testing.T) {
655641
expectedErr: true,
656642
expectedErrMsg: "release image metadata has no GCP image for architecture \"amd64\"",
657643
},
644+
{
645+
name: "When stream metadata is nil, it should return error",
646+
arch: hyperv1.ArchitectureAMD64,
647+
streamMeta: nil,
648+
expectedErr: true,
649+
expectedErrMsg: "stream metadata is nil",
650+
},
658651
}
659652

660653
for _, tc := range testCases {
661654
t.Run(tc.name, func(t *testing.T) {
662655
g := NewWithT(t)
663656

664-
image, err := defaultNodePoolGCPImage(tc.arch, tc.releaseImage)
657+
image, err := DefaultNodePoolGCPImage(tc.arch, tc.streamMeta)
665658

666659
if tc.expectedErr {
667660
g.Expect(err).To(HaveOccurred())

hypershift-operator/controllers/nodepool/kubevirt/kubevirt.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020

2121
capikubevirt "sigs.k8s.io/cluster-api-provider-kubevirt/api/v1alpha1"
2222

23+
"github.com/coreos/stream-metadata-go/stream"
2324
jsonpatch "github.com/evanphx/json-patch/v5"
2425
kubevirtv1 "kubevirt.io/api/core/v1"
2526
"kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
@@ -36,15 +37,15 @@ var LocalStorageVolumes = []string{
3637
"hotplug-disks",
3738
}
3839

39-
func defaultImage(nodePoolArch string, releaseImage *releaseinfo.ReleaseImage) (string, string, error) {
40+
func defaultImage(nodePoolArch string, streamMeta *stream.Stream) (string, string, error) {
4041
var archName string
4142
switch nodePoolArch {
4243
case hyperv1.ArchitectureS390X:
4344
archName = hyperv1.ArchitectureS390X
4445
default:
4546
archName = hyperv1.ArchAliases[hyperv1.ArchitectureAMD64]
4647
}
47-
arch, foundArch := releaseImage.StreamMetadata.Architectures[archName]
48+
arch, foundArch := streamMeta.Architectures[archName]
4849

4950
if !foundArch {
5051
return "", "", fmt.Errorf("couldn't find OS metadata for architecture %q", archName)
@@ -90,9 +91,13 @@ func GetImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage
9091
return newBootImage(imageName, isHTTP), nil
9192
}
9293

93-
imageName, imageHash, err := defaultImage(nodePool.Spec.Arch, releaseImage)
94+
streamMeta, err := releaseImage.StreamForName("")
95+
if err != nil {
96+
return nil, fmt.Errorf("couldn't resolve stream metadata: %w", err)
97+
}
98+
imageName, imageHash, err := defaultImage(nodePool.Spec.Arch, streamMeta)
9499
if err != nil && allowUnsupportedRHCOSVariants(nodePool) {
95-
imageName, imageHash, err = openstack.OpenstackDefaultImage(releaseImage)
100+
imageName, imageHash, err = openstack.OpenstackDefaultImage(streamMeta)
96101
if err != nil {
97102
return nil, err
98103
}

hypershift-operator/controllers/nodepool/kubevirt/kubevirt_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1580,7 +1580,7 @@ func TestDefaultImage(t *testing.T) {
15801580
if testRI == nil {
15811581
testRI = ri
15821582
}
1583-
img, digest, err := defaultImage(tt.arch, testRI)
1583+
img, digest, err := defaultImage(tt.arch, testRI.StreamMetadata)
15841584
if tt.expectedError {
15851585
if err == nil {
15861586
t.Fatalf("expected error but got nil")

hypershift-operator/controllers/nodepool/nodepool_controller.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -759,16 +759,13 @@ func defaultNodePoolAMI(region string, specifiedArch string, streamMeta *stream.
759759
return regionData.Image, nil
760760
}
761761

762-
// defaultNodePoolGCPImage returns the default GCP image for a given architecture from release metadata.
763-
func defaultNodePoolGCPImage(specifiedArch string, releaseImage *releaseinfo.ReleaseImage) (string, error) {
764-
if releaseImage == nil {
765-
return "", fmt.Errorf("release image is nil, cannot determine GCP image")
766-
}
767-
if releaseImage.StreamMetadata == nil {
768-
return "", fmt.Errorf("release image stream metadata is nil, cannot determine GCP image for architecture %q", specifiedArch)
762+
// DefaultNodePoolGCPImage returns the default GCP image for a given architecture from stream metadata.
763+
func DefaultNodePoolGCPImage(specifiedArch string, streamMeta *stream.Stream) (string, error) {
764+
if streamMeta == nil {
765+
return "", fmt.Errorf("stream metadata is nil, cannot determine GCP image for architecture %q", specifiedArch)
769766
}
770767

771-
arch, foundArch := releaseImage.StreamMetadata.Architectures[hyperv1.ArchAliases[specifiedArch]]
768+
arch, foundArch := streamMeta.Architectures[hyperv1.ArchAliases[specifiedArch]]
772769
if !foundArch {
773770
return "", fmt.Errorf("couldn't find OS metadata for architecture %q", specifiedArch)
774771
}

hypershift-operator/controllers/nodepool/nodepool_controller_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -571,8 +571,14 @@ func TestDefaultNodePoolAMI(t *testing.T) {
571571
t.Fatalf("failed to parse multi-stream fixture: %v", err)
572572
}
573573
ri := &releaseinfo.ReleaseImage{StreamMetadata: defaultStream, OSStreams: osStreams}
574-
rhel9Stream, _ := ri.StreamForName("rhel-9")
575-
rhel10Stream, _ := ri.StreamForName("rhel-10")
574+
rhel9Stream, err := ri.StreamForName("rhel-9")
575+
if err != nil {
576+
t.Fatalf("failed to resolve rhel-9 stream: %v", err)
577+
}
578+
rhel10Stream, err := ri.StreamForName("rhel-10")
579+
if err != nil {
580+
t.Fatalf("failed to resolve rhel-10 stream: %v", err)
581+
}
576582

577583
testCases := []struct {
578584
name string

hypershift-operator/controllers/nodepool/openstack.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,11 @@ func (c *CAPI) openstackMachineTemplate(templateNameGenerator func(spec any) (st
5151
}
5252
func (r *NodePoolReconciler) setOpenStackConditions(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, _ string, releaseImage *releaseinfo.ReleaseImage) error {
5353
if nodePool.Spec.Platform.OpenStack.ImageName == "" {
54-
_, err := openstack.OpenStackReleaseImage(releaseImage)
54+
streamMeta, err := releaseImage.StreamForName("")
55+
if err != nil {
56+
return fmt.Errorf("couldn't resolve stream metadata: %w", err)
57+
}
58+
_, err = openstack.OpenStackReleaseImage(streamMeta)
5559
if err != nil {
5660
SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{
5761
Type: hyperv1.NodePoolValidPlatformImageType,
@@ -89,7 +93,11 @@ func (r *NodePoolReconciler) setOpenStackConditions(ctx context.Context, nodePoo
8993
// An ORC object will be created or updated with the image spec.
9094
// The image name will be returned.
9195
func (r *NodePoolReconciler) reconcileOpenStackImageCR(ctx context.Context, client client.Client, hcluster *hyperv1.HostedCluster, release *releaseinfo.ReleaseImage, nodePool *hyperv1.NodePool) (string, error) {
92-
releaseVersion, err := openstack.OpenStackReleaseImage(release)
96+
streamMeta, err := release.StreamForName("")
97+
if err != nil {
98+
return "", fmt.Errorf("couldn't resolve stream metadata: %w", err)
99+
}
100+
releaseVersion, err := openstack.OpenStackReleaseImage(streamMeta)
93101
if err != nil {
94102
return "", err
95103
}

0 commit comments

Comments
 (0)