Skip to content

Commit fab68e1

Browse files
jparrilljparrill
committed
feat(nodepool): decouple platform boot image resolvers from ReleaseImage
All platform-specific boot image resolution functions (AWS, Azure, GCP, KubeVirt, OpenStack, PowerVS) now receive *stream.Stream directly instead of *releaseinfo.ReleaseImage. Callers resolve the stream via ReleaseImage.StreamForName() before invoking the resolver, decoupling platform logic from release image internals. This prepares the codebase for stream-aware boot image selection (CNTRLPLANE-3027) where each NodePool can target a specific RHEL stream (rhel-9 or rhel-10) once the osImageStream API field is available. Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
1 parent c35f662 commit fab68e1

15 files changed

Lines changed: 301 additions & 237 deletions

File tree

‎hypershift-operator/controllers/nodepool/aws.go‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,14 @@ func resolveAWSAMI(hostedCluster *hyperv1.HostedCluster, nodePool *hyperv1.NodeP
134134
return ami, nil
135135
}
136136
// Default behavior for Linux/RHCOS AMIs
137-
// TODO(CNTRLPLANE-3553): resolve streamName via GetRHELStream once osImageStream API field is available
138-
ami, err := defaultNodePoolAMI(region, arch, "", releaseImage)
137+
if releaseImage == nil {
138+
return "", fmt.Errorf("release image is nil")
139+
}
140+
streamMeta, err := releaseImage.StreamForName("")
141+
if err != nil {
142+
return "", fmt.Errorf("couldn't resolve stream metadata: %w", err)
143+
}
144+
ami, err := defaultNodePoolAMI(region, arch, streamMeta)
139145
if err != nil {
140146
return "", fmt.Errorf("couldn't discover an AMI for release image: %w", err)
141147
}
@@ -362,8 +368,28 @@ func (r *NodePoolReconciler) setAWSConditions(_ context.Context, nodePool *hyper
362368
})
363369
} else {
364370
// Default behavior for Linux/RHCOS AMIs
365-
// TODO(CNTRLPLANE-3553): resolve streamName via GetRHELStream once osImageStream API field is available
366-
ami, err := defaultNodePoolAMI(hcluster.Spec.Platform.AWS.Region, nodePool.Spec.Arch, "", releaseImage)
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+
}
381+
streamMeta, err := releaseImage.StreamForName("")
382+
if err != nil {
383+
SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{
384+
Type: hyperv1.NodePoolValidPlatformImageType,
385+
Status: corev1.ConditionFalse,
386+
Reason: hyperv1.NodePoolValidationFailedReason,
387+
Message: fmt.Sprintf("Couldn't resolve stream metadata for release image %q: %s", nodePool.Spec.Release.Image, err.Error()),
388+
ObservedGeneration: nodePool.Generation,
389+
})
390+
return fmt.Errorf("couldn't resolve stream metadata: %w", err)
391+
}
392+
ami, err := defaultNodePoolAMI(hcluster.Spec.Platform.AWS.Region, nodePool.Spec.Arch, streamMeta)
367393
if err != nil {
368394
SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{
369395
Type: hyperv1.NodePoolValidPlatformImageType,

‎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: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import (
5555
"sigs.k8s.io/controller-runtime/pkg/reconcile"
5656

5757
"github.com/blang/semver"
58+
"github.com/coreos/stream-metadata-go/stream"
5859
"github.com/pkg/errors"
5960
)
6061

@@ -736,16 +737,9 @@ func isAutoscalingEnabled(nodePool *hyperv1.NodePool) bool {
736737
return nodePool.Spec.AutoScaling != nil
737738
}
738739

739-
// defaultNodePoolAMI resolves the default AWS AMI for a NodePool from release image stream metadata.
740-
// TODO(CNTRLPLANE-3553): once the osImageStream API field is available, callers should resolve
741-
// streamName via GetRHELStream and pass it here instead of hardcoding "".
742-
func defaultNodePoolAMI(region string, specifiedArch string, streamName string, releaseImage *releaseinfo.ReleaseImage) (string, error) {
743-
if releaseImage == nil {
744-
return "", fmt.Errorf("release image is nil")
745-
}
746-
streamMeta, err := releaseImage.StreamForName(streamName)
747-
if err != nil {
748-
return "", fmt.Errorf("couldn't resolve stream metadata: %w", err)
740+
func defaultNodePoolAMI(region string, specifiedArch string, streamMeta *stream.Stream) (string, error) {
741+
if streamMeta == nil {
742+
return "", fmt.Errorf("stream metadata is nil")
749743
}
750744
arch, foundArch := streamMeta.Architectures[hyperv1.ArchAliases[specifiedArch]]
751745
if !foundArch {
@@ -765,16 +759,13 @@ func defaultNodePoolAMI(region string, specifiedArch string, streamName string,
765759
return regionData.Image, nil
766760
}
767761

768-
// defaultNodePoolGCPImage returns the default GCP image for a given architecture from release metadata.
769-
func defaultNodePoolGCPImage(specifiedArch string, releaseImage *releaseinfo.ReleaseImage) (string, error) {
770-
if releaseImage == nil {
771-
return "", fmt.Errorf("release image is nil, cannot determine GCP image")
772-
}
773-
if releaseImage.StreamMetadata == nil {
774-
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)
775766
}
776767

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

0 commit comments

Comments
 (0)