diff --git a/hypershift-operator/controllers/nodepool/aws.go b/hypershift-operator/controllers/nodepool/aws.go index f3cde2061fde..32cdc4434c6d 100644 --- a/hypershift-operator/controllers/nodepool/aws.go +++ b/hypershift-operator/controllers/nodepool/aws.go @@ -134,8 +134,14 @@ func resolveAWSAMI(hostedCluster *hyperv1.HostedCluster, nodePool *hyperv1.NodeP return ami, nil } // Default behavior for Linux/RHCOS AMIs - // TODO(CNTRLPLANE-3553): resolve streamName via GetRHELStream once osImageStream API field is available - ami, err := defaultNodePoolAMI(region, arch, "", releaseImage) + if releaseImage == nil { + return "", fmt.Errorf("release image is nil") + } + streamMeta, err := releaseImage.StreamForName("") + if err != nil { + return "", fmt.Errorf("couldn't resolve stream metadata: %w", err) + } + ami, err := defaultNodePoolAMI(region, arch, streamMeta) if err != nil { return "", fmt.Errorf("couldn't discover an AMI for release image: %w", err) } @@ -362,8 +368,28 @@ func (r *NodePoolReconciler) setAWSConditions(_ context.Context, nodePool *hyper }) } else { // Default behavior for Linux/RHCOS AMIs - // TODO(CNTRLPLANE-3553): resolve streamName via GetRHELStream once osImageStream API field is available - ami, err := defaultNodePoolAMI(hcluster.Spec.Platform.AWS.Region, nodePool.Spec.Arch, "", releaseImage) + if releaseImage == nil { + SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ + Type: hyperv1.NodePoolValidPlatformImageType, + Status: corev1.ConditionFalse, + Reason: hyperv1.NodePoolValidationFailedReason, + Message: fmt.Sprintf("Release image metadata is nil for release image %q", nodePool.Spec.Release.Image), + ObservedGeneration: nodePool.Generation, + }) + return fmt.Errorf("release image is nil") + } + streamMeta, err := releaseImage.StreamForName("") + if err != nil { + SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ + Type: hyperv1.NodePoolValidPlatformImageType, + Status: corev1.ConditionFalse, + Reason: hyperv1.NodePoolValidationFailedReason, + Message: fmt.Sprintf("Couldn't resolve stream metadata for release image %q: %s", nodePool.Spec.Release.Image, err.Error()), + ObservedGeneration: nodePool.Generation, + }) + return fmt.Errorf("couldn't resolve stream metadata: %w", err) + } + ami, err := defaultNodePoolAMI(hcluster.Spec.Platform.AWS.Region, nodePool.Spec.Arch, streamMeta) if err != nil { SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolValidPlatformImageType, diff --git a/hypershift-operator/controllers/nodepool/azure.go b/hypershift-operator/controllers/nodepool/azure.go index 43a9252ce906..3e40acec8231 100644 --- a/hypershift-operator/controllers/nodepool/azure.go +++ b/hypershift-operator/controllers/nodepool/azure.go @@ -16,6 +16,7 @@ import ( capiazure "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "github.com/blang/semver" + "github.com/coreos/stream-metadata-go/stream" ) // dummySSHKey is a base64 encoded dummy SSH public key. @@ -81,7 +82,11 @@ func defaultAzureNodePoolImage(nodePool *hyperv1.NodePool, releaseImage *release } // Extract marketplace metadata from release payload - azureMarketplace, err := getAzureMarketplaceMetadata(releaseImage, streamArch) + streamMeta, err := releaseImage.StreamForName("") + if err != nil { + return fmt.Errorf("couldn't resolve stream metadata: %w", err) + } + azureMarketplace, err := getAzureMarketplaceMetadata(streamMeta, streamArch) if err != nil { return fmt.Errorf("failed to get Azure Marketplace metadata: %w", err) } @@ -119,13 +124,13 @@ func defaultAzureNodePoolImage(nodePool *hyperv1.NodePool, releaseImage *release return nil } -// getAzureMarketplaceMetadata extracts Azure Marketplace metadata from the release payload -func getAzureMarketplaceMetadata(releaseImage *releaseinfo.ReleaseImage, arch string) (*azureMarketplaceMetadata, error) { - if releaseImage.StreamMetadata == nil { +// getAzureMarketplaceMetadata extracts Azure Marketplace metadata from stream metadata. +func getAzureMarketplaceMetadata(streamMeta *stream.Stream, arch string) (*azureMarketplaceMetadata, error) { + if streamMeta == nil { return nil, nil // No stream metadata available } - archData, foundArch := releaseImage.StreamMetadata.Architectures[arch] + archData, foundArch := streamMeta.Architectures[arch] if !foundArch { return nil, fmt.Errorf("architecture %s not found in stream metadata", arch) } diff --git a/hypershift-operator/controllers/nodepool/gcp.go b/hypershift-operator/controllers/nodepool/gcp.go index 50a107b210b4..ea3cb93803dd 100644 --- a/hypershift-operator/controllers/nodepool/gcp.go +++ b/hypershift-operator/controllers/nodepool/gcp.go @@ -168,7 +168,11 @@ func resolveGCPImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.Relea } // Resolve image from release metadata - image, err := defaultNodePoolGCPImage(nodePool.Spec.Arch, releaseImage) + streamMeta, err := releaseImage.StreamForName("") + if err != nil { + return "", fmt.Errorf("couldn't resolve stream metadata: %w", err) + } + image, err := DefaultNodePoolGCPImage(nodePool.Spec.Arch, streamMeta) if err != nil { return "", fmt.Errorf("couldn't discover a GCP image for release image: %w", err) } diff --git a/hypershift-operator/controllers/nodepool/gcp_test.go b/hypershift-operator/controllers/nodepool/gcp_test.go index 14a3623acf82..9fd0612faf69 100644 --- a/hypershift-operator/controllers/nodepool/gcp_test.go +++ b/hypershift-operator/controllers/nodepool/gcp_test.go @@ -535,7 +535,7 @@ func TestDefaultNodePoolGCPImage(t *testing.T) { testCases := []struct { name string arch string - releaseImage *releaseinfo.ReleaseImage + streamMeta *stream.Stream expectedImage string expectedErr bool expectedErrMsg string @@ -543,55 +543,47 @@ func TestDefaultNodePoolGCPImage(t *testing.T) { { name: "When stream metadata has project and name for amd64, it should construct image path", arch: hyperv1.ArchitectureAMD64, - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Images: stream.Images{ - Gcp: &stream.GcpImage{ - Project: "rhcos-cloud", - Name: "rhcos-9-6-20251023-0-gcp-x86-64", - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Images: stream.Images{ + Gcp: &stream.GcpImage{ + Project: "rhcos-cloud", + Name: "rhcos-9-6-20251023-0-gcp-x86-64", }, }, }, }, }, expectedImage: "projects/rhcos-cloud/global/images/rhcos-9-6-20251023-0-gcp-x86-64", - expectedErr: false, }, { name: "When stream metadata has project and name for arm64, it should construct image path", arch: hyperv1.ArchitectureARM64, - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "aarch64": { - Images: stream.Images{ - Gcp: &stream.GcpImage{ - Project: "rhcos-cloud", - Name: "rhcos-9-6-20251023-0-gcp-aarch64", - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "aarch64": { + Images: stream.Images{ + Gcp: &stream.GcpImage{ + Project: "rhcos-cloud", + Name: "rhcos-9-6-20251023-0-gcp-aarch64", }, }, }, }, }, expectedImage: "projects/rhcos-cloud/global/images/rhcos-9-6-20251023-0-gcp-aarch64", - expectedErr: false, }, { - name: "When architecture is not found in release metadata, it should return error", + name: "When architecture is not found in stream metadata, it should return error", arch: "unsupported-arch", - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Images: stream.Images{ - Gcp: &stream.GcpImage{ - Project: "rhcos-cloud", - Name: "rhcos-9-6-20251023-0-gcp-x86-64", - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Images: stream.Images{ + Gcp: &stream.GcpImage{ + Project: "rhcos-cloud", + Name: "rhcos-9-6-20251023-0-gcp-x86-64", }, }, }, @@ -603,13 +595,11 @@ func TestDefaultNodePoolGCPImage(t *testing.T) { { name: "When GCP project and name are empty, it should return error", arch: hyperv1.ArchitectureAMD64, - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Images: stream.Images{ - Gcp: &stream.GcpImage{}, - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Images: stream.Images{ + Gcp: &stream.GcpImage{}, }, }, }, @@ -620,14 +610,12 @@ func TestDefaultNodePoolGCPImage(t *testing.T) { { name: "When GCP project is empty but name is set, it should return error", arch: hyperv1.ArchitectureAMD64, - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Images: stream.Images{ - Gcp: &stream.GcpImage{ - Name: "rhcos-x86-64", - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Images: stream.Images{ + Gcp: &stream.GcpImage{ + Name: "rhcos-x86-64", }, }, }, @@ -639,14 +627,12 @@ func TestDefaultNodePoolGCPImage(t *testing.T) { { name: "When GCP name is empty but project is set, it should return error", arch: hyperv1.ArchitectureAMD64, - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Images: stream.Images{ - Gcp: &stream.GcpImage{ - Project: "rhcos-cloud", - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Images: stream.Images{ + Gcp: &stream.GcpImage{ + Project: "rhcos-cloud", }, }, }, @@ -655,13 +641,20 @@ func TestDefaultNodePoolGCPImage(t *testing.T) { expectedErr: true, expectedErrMsg: "release image metadata has no GCP image for architecture \"amd64\"", }, + { + name: "When stream metadata is nil, it should return error", + arch: hyperv1.ArchitectureAMD64, + streamMeta: nil, + expectedErr: true, + expectedErrMsg: "stream metadata is nil", + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - image, err := defaultNodePoolGCPImage(tc.arch, tc.releaseImage) + image, err := DefaultNodePoolGCPImage(tc.arch, tc.streamMeta) if tc.expectedErr { g.Expect(err).To(HaveOccurred()) diff --git a/hypershift-operator/controllers/nodepool/kubevirt/kubevirt.go b/hypershift-operator/controllers/nodepool/kubevirt/kubevirt.go index c5c087a6f3d9..209ec1a330fe 100644 --- a/hypershift-operator/controllers/nodepool/kubevirt/kubevirt.go +++ b/hypershift-operator/controllers/nodepool/kubevirt/kubevirt.go @@ -20,6 +20,7 @@ import ( capikubevirt "sigs.k8s.io/cluster-api-provider-kubevirt/api/v1alpha1" + "github.com/coreos/stream-metadata-go/stream" jsonpatch "github.com/evanphx/json-patch/v5" kubevirtv1 "kubevirt.io/api/core/v1" "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" @@ -36,7 +37,7 @@ var LocalStorageVolumes = []string{ "hotplug-disks", } -func defaultImage(nodePoolArch string, releaseImage *releaseinfo.ReleaseImage) (string, string, error) { +func defaultImage(nodePoolArch string, streamMeta *stream.Stream) (string, string, error) { var archName string switch nodePoolArch { case hyperv1.ArchitectureS390X: @@ -44,7 +45,7 @@ func defaultImage(nodePoolArch string, releaseImage *releaseinfo.ReleaseImage) ( default: archName = hyperv1.ArchAliases[hyperv1.ArchitectureAMD64] } - arch, foundArch := releaseImage.StreamMetadata.Architectures[archName] + arch, foundArch := streamMeta.Architectures[archName] if !foundArch { return "", "", fmt.Errorf("couldn't find OS metadata for architecture %q", archName) @@ -90,9 +91,13 @@ func GetImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage return newBootImage(imageName, isHTTP), nil } - imageName, imageHash, err := defaultImage(nodePool.Spec.Arch, releaseImage) + streamMeta, err := releaseImage.StreamForName("") + if err != nil { + return nil, fmt.Errorf("couldn't resolve stream metadata: %w", err) + } + imageName, imageHash, err := defaultImage(nodePool.Spec.Arch, streamMeta) if err != nil && allowUnsupportedRHCOSVariants(nodePool) { - imageName, imageHash, err = openstack.OpenstackDefaultImage(releaseImage) + imageName, imageHash, err = openstack.OpenstackDefaultImage(streamMeta) if err != nil { return nil, err } diff --git a/hypershift-operator/controllers/nodepool/kubevirt/kubevirt_test.go b/hypershift-operator/controllers/nodepool/kubevirt/kubevirt_test.go index 2084a6b65cc5..54ae9a515afc 100644 --- a/hypershift-operator/controllers/nodepool/kubevirt/kubevirt_test.go +++ b/hypershift-operator/controllers/nodepool/kubevirt/kubevirt_test.go @@ -1580,7 +1580,7 @@ func TestDefaultImage(t *testing.T) { if testRI == nil { testRI = ri } - img, digest, err := defaultImage(tt.arch, testRI) + img, digest, err := defaultImage(tt.arch, testRI.StreamMetadata) if tt.expectedError { if err == nil { t.Fatalf("expected error but got nil") diff --git a/hypershift-operator/controllers/nodepool/nodepool_controller.go b/hypershift-operator/controllers/nodepool/nodepool_controller.go index 2a170c1137de..59f17b622ce1 100644 --- a/hypershift-operator/controllers/nodepool/nodepool_controller.go +++ b/hypershift-operator/controllers/nodepool/nodepool_controller.go @@ -55,6 +55,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/blang/semver" + "github.com/coreos/stream-metadata-go/stream" "github.com/pkg/errors" ) @@ -736,16 +737,9 @@ func isAutoscalingEnabled(nodePool *hyperv1.NodePool) bool { return nodePool.Spec.AutoScaling != nil } -// defaultNodePoolAMI resolves the default AWS AMI for a NodePool from release image stream metadata. -// TODO(CNTRLPLANE-3553): once the osImageStream API field is available, callers should resolve -// streamName via GetRHELStream and pass it here instead of hardcoding "". -func defaultNodePoolAMI(region string, specifiedArch string, streamName string, releaseImage *releaseinfo.ReleaseImage) (string, error) { - if releaseImage == nil { - return "", fmt.Errorf("release image is nil") - } - streamMeta, err := releaseImage.StreamForName(streamName) - if err != nil { - return "", fmt.Errorf("couldn't resolve stream metadata: %w", err) +func defaultNodePoolAMI(region string, specifiedArch string, streamMeta *stream.Stream) (string, error) { + if streamMeta == nil { + return "", fmt.Errorf("stream metadata is nil") } arch, foundArch := streamMeta.Architectures[hyperv1.ArchAliases[specifiedArch]] if !foundArch { @@ -765,16 +759,13 @@ func defaultNodePoolAMI(region string, specifiedArch string, streamName string, return regionData.Image, nil } -// defaultNodePoolGCPImage returns the default GCP image for a given architecture from release metadata. -func defaultNodePoolGCPImage(specifiedArch string, releaseImage *releaseinfo.ReleaseImage) (string, error) { - if releaseImage == nil { - return "", fmt.Errorf("release image is nil, cannot determine GCP image") - } - if releaseImage.StreamMetadata == nil { - return "", fmt.Errorf("release image stream metadata is nil, cannot determine GCP image for architecture %q", specifiedArch) +// DefaultNodePoolGCPImage returns the default GCP image for a given architecture from stream metadata. +func DefaultNodePoolGCPImage(specifiedArch string, streamMeta *stream.Stream) (string, error) { + if streamMeta == nil { + return "", fmt.Errorf("stream metadata is nil, cannot determine GCP image for architecture %q", specifiedArch) } - arch, foundArch := releaseImage.StreamMetadata.Architectures[hyperv1.ArchAliases[specifiedArch]] + arch, foundArch := streamMeta.Architectures[hyperv1.ArchAliases[specifiedArch]] if !foundArch { return "", fmt.Errorf("couldn't find OS metadata for architecture %q", specifiedArch) } diff --git a/hypershift-operator/controllers/nodepool/nodepool_controller_test.go b/hypershift-operator/controllers/nodepool/nodepool_controller_test.go index 4c2e77bd6c37..2c56e0299101 100644 --- a/hypershift-operator/controllers/nodepool/nodepool_controller_test.go +++ b/hypershift-operator/controllers/nodepool/nodepool_controller_test.go @@ -542,25 +542,23 @@ func TestCreateValidGeneratedPayloadCondition(t *testing.T) { func TestDefaultNodePoolAMI(t *testing.T) { t.Parallel() - basicReleaseImage := &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Images: stream.Images{ - Aws: &stream.AwsImage{ - Regions: map[string]stream.SingleImage{ - "us-east-1": {Release: "4.12.0", Image: "us-east-1-x86_64-image"}, - }, + basicStream := &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Images: stream.Images{ + Aws: &stream.AwsImage{ + Regions: map[string]stream.SingleImage{ + "us-east-1": {Release: "4.12.0", Image: "us-east-1-x86_64-image"}, }, }, }, - "aarch64": { - Images: stream.Images{ - Aws: &stream.AwsImage{ - Regions: map[string]stream.SingleImage{ - "us-east-1": {Release: "4.12.0", Image: "us-east-1-aarch64-image"}, - "us-west-1": {Release: "4.12.0", Image: ""}, - }, + }, + "aarch64": { + Images: stream.Images{ + Aws: &stream.AwsImage{ + Regions: map[string]stream.SingleImage{ + "us-east-1": {Release: "4.12.0", Image: "us-east-1-aarch64-image"}, + "us-west-1": {Release: "4.12.0", Image: ""}, }, }, }, @@ -572,14 +570,21 @@ func TestDefaultNodePoolAMI(t *testing.T) { if err != nil { t.Fatalf("failed to parse multi-stream fixture: %v", err) } - multiStreamReleaseImage := &releaseinfo.ReleaseImage{StreamMetadata: defaultStream, OSStreams: osStreams} + ri := &releaseinfo.ReleaseImage{StreamMetadata: defaultStream, OSStreams: osStreams} + rhel9Stream, err := ri.StreamForName("rhel-9") + if err != nil { + t.Fatalf("failed to resolve rhel-9 stream: %v", err) + } + rhel10Stream, err := ri.StreamForName("rhel-10") + if err != nil { + t.Fatalf("failed to resolve rhel-10 stream: %v", err) + } testCases := []struct { name string region string specifiedArch string - streamName string - releaseImage *releaseinfo.ReleaseImage + streamMeta *stream.Stream expectedImage string expectedErr string }{ @@ -588,45 +593,42 @@ func TestDefaultNodePoolAMI(t *testing.T) { name: "When resolving amd64 AMI it should return the correct image", region: "us-east-1", specifiedArch: "amd64", - releaseImage: basicReleaseImage, + streamMeta: basicStream, expectedImage: "us-east-1-x86_64-image", }, { name: "When resolving arm64 AMI it should return the correct image", region: "us-east-1", specifiedArch: "arm64", - releaseImage: basicReleaseImage, + streamMeta: basicStream, expectedImage: "us-east-1-aarch64-image", }, { name: "When resolving rhel-9 stream it should return the rhel-9 AMI", region: "us-east-1", specifiedArch: "amd64", - streamName: "rhel-9", - releaseImage: multiStreamReleaseImage, + streamMeta: rhel9Stream, expectedImage: "ami-06a6b025350ff1e23", }, { name: "When resolving rhel-10 stream it should return the rhel-10 AMI", region: "us-east-1", specifiedArch: "amd64", - streamName: "rhel-10", - releaseImage: multiStreamReleaseImage, + streamMeta: rhel10Stream, expectedImage: "ami-04b3d999e39d62c5b", }, { name: "When resolving rhel-10 arm64 stream it should return the rhel-10 arm64 AMI", region: "us-east-1", specifiedArch: "arm64", - streamName: "rhel-10", - releaseImage: multiStreamReleaseImage, + streamMeta: rhel10Stream, expectedImage: "ami-0d7237e6b04d9a9e1", }, { name: "When using default stream it should return the default AMI", region: "us-east-1", specifiedArch: "amd64", - releaseImage: multiStreamReleaseImage, + streamMeta: defaultStream, expectedImage: "ami-06a6b025350ff1e23", }, // --- Sad paths --- @@ -634,36 +636,29 @@ func TestDefaultNodePoolAMI(t *testing.T) { name: "When region is not found it should return error", region: "us-east-2", specifiedArch: "amd64", - releaseImage: basicReleaseImage, + streamMeta: basicStream, expectedErr: `couldn't find AWS image for region "us-east-2"`, }, { name: "When architecture is not found it should return error", region: "us-east-1", specifiedArch: "s390x", - releaseImage: basicReleaseImage, + streamMeta: basicStream, expectedErr: `couldn't find OS metadata for architecture "s390x"`, }, { name: "When image data is empty for region it should return error", region: "us-west-1", specifiedArch: "arm64", - releaseImage: basicReleaseImage, + streamMeta: basicStream, expectedErr: `release image metadata has no image for region "us-west-1"`, }, { name: "When stream metadata is nil it should return error", region: "us-east-1", specifiedArch: "amd64", - releaseImage: &releaseinfo.ReleaseImage{StreamMetadata: nil}, - expectedErr: "couldn't resolve stream metadata: no default stream metadata available", - }, - { - name: "When release image is nil it should return error", - region: "us-east-1", - specifiedArch: "amd64", - releaseImage: nil, - expectedErr: "release image is nil", + streamMeta: nil, + expectedErr: "stream metadata is nil", }, } @@ -672,7 +667,7 @@ func TestDefaultNodePoolAMI(t *testing.T) { t.Parallel() g := NewWithT(t) - image, err := defaultNodePoolAMI(tc.region, tc.specifiedArch, tc.streamName, tc.releaseImage) + image, err := defaultNodePoolAMI(tc.region, tc.specifiedArch, tc.streamMeta) if tc.expectedErr != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(Equal(tc.expectedErr)) diff --git a/hypershift-operator/controllers/nodepool/openstack.go b/hypershift-operator/controllers/nodepool/openstack.go index bd9773caa737..3ec029d49267 100644 --- a/hypershift-operator/controllers/nodepool/openstack.go +++ b/hypershift-operator/controllers/nodepool/openstack.go @@ -51,7 +51,11 @@ func (c *CAPI) openstackMachineTemplate(templateNameGenerator func(spec any) (st } func (r *NodePoolReconciler) setOpenStackConditions(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, _ string, releaseImage *releaseinfo.ReleaseImage) error { if nodePool.Spec.Platform.OpenStack.ImageName == "" { - _, err := openstack.OpenStackReleaseImage(releaseImage) + streamMeta, err := releaseImage.StreamForName("") + if err != nil { + return fmt.Errorf("couldn't resolve stream metadata: %w", err) + } + _, err = openstack.OpenStackReleaseImage(streamMeta) if err != nil { SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolValidPlatformImageType, @@ -89,7 +93,11 @@ func (r *NodePoolReconciler) setOpenStackConditions(ctx context.Context, nodePoo // An ORC object will be created or updated with the image spec. // The image name will be returned. func (r *NodePoolReconciler) reconcileOpenStackImageCR(ctx context.Context, client client.Client, hcluster *hyperv1.HostedCluster, release *releaseinfo.ReleaseImage, nodePool *hyperv1.NodePool) (string, error) { - releaseVersion, err := openstack.OpenStackReleaseImage(release) + streamMeta, err := release.StreamForName("") + if err != nil { + return "", fmt.Errorf("couldn't resolve stream metadata: %w", err) + } + releaseVersion, err := openstack.OpenStackReleaseImage(streamMeta) if err != nil { return "", err } diff --git a/hypershift-operator/controllers/nodepool/openstack/openstack.go b/hypershift-operator/controllers/nodepool/openstack/openstack.go index 48110bd40d1a..e0d05cd214c9 100644 --- a/hypershift-operator/controllers/nodepool/openstack/openstack.go +++ b/hypershift-operator/controllers/nodepool/openstack/openstack.go @@ -14,6 +14,7 @@ import ( capiopenstackv1beta1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/coreos/stream-metadata-go/stream" orc "github.com/k-orc/openstack-resource-controller/api/v1alpha1" ) @@ -27,7 +28,11 @@ func MachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hyperv1.Node Name: ptr.To(nodePool.Spec.Platform.OpenStack.ImageName), } } else { - releaseVersion, err := OpenStackReleaseImage(releaseImage) + streamMeta, err := releaseImage.StreamForName("") + if err != nil { + return nil, fmt.Errorf("couldn't resolve stream metadata: %w", err) + } + releaseVersion, err := OpenStackReleaseImage(streamMeta) if err != nil { return nil, err } @@ -95,7 +100,11 @@ func GetOpenStackClusterForHostedCluster(ctx context.Context, c client.Client, h // ReconcileOpenStackImageSpec reconciles the OpenStack ImageSpec for the given HostedCluster. // The image spec will be set to the default RHCOS image for the given release. func ReconcileOpenStackImageSpec(hcluster *hyperv1.HostedCluster, openStackImageSpec *orc.ImageSpec, release *releaseinfo.ReleaseImage) error { - imageURL, imageHash, err := OpenstackDefaultImage(release) + streamMeta, err := release.StreamForName("") + if err != nil { + return fmt.Errorf("couldn't resolve stream metadata: %w", err) + } + imageURL, imageHash, err := OpenstackDefaultImage(streamMeta) if err != nil { return fmt.Errorf("failed to lookup RHCOS image: %w", err) } @@ -105,7 +114,7 @@ func ReconcileOpenStackImageSpec(hcluster *hyperv1.HostedCluster, openStackImage CloudName: hcluster.Spec.Platform.OpenStack.IdentityRef.CloudName, } - imageName, err := PrefixedClusterImageName(hcluster, release) + imageName, err := PrefixedClusterImageName(hcluster, streamMeta) if err != nil { return fmt.Errorf("failed to get image name: %w", err) } @@ -129,10 +138,13 @@ func ReconcileOpenStackImageSpec(hcluster *hyperv1.HostedCluster, openStackImage return nil } -// OpenstackDefaultImage returns the default RHCOS image for the given release. +// OpenstackDefaultImage returns the default RHCOS image for the given stream metadata. // The image URL and SHA256 hash are returned. -func OpenstackDefaultImage(releaseImage *releaseinfo.ReleaseImage) (string, string, error) { - arch, foundArch := releaseImage.StreamMetadata.Architectures["x86_64"] +func OpenstackDefaultImage(streamMeta *stream.Stream) (string, string, error) { + if streamMeta == nil { + return "", "", fmt.Errorf("stream metadata is nil") + } + arch, foundArch := streamMeta.Architectures["x86_64"] if !foundArch { return "", "", fmt.Errorf("couldn't find OS metadata for architecture %q", "x86_64") } @@ -152,9 +164,12 @@ func OpenstackDefaultImage(releaseImage *releaseinfo.ReleaseImage) (string, stri } // OpenStackReleaseImage returns the release version for the OpenStack image. -// The release version is extracted from the release metadata. -func OpenStackReleaseImage(releaseImage *releaseinfo.ReleaseImage) (string, error) { - arch, foundArch := releaseImage.StreamMetadata.Architectures["x86_64"] +// The release version is extracted from stream metadata. +func OpenStackReleaseImage(streamMeta *stream.Stream) (string, error) { + if streamMeta == nil { + return "", fmt.Errorf("stream metadata is nil") + } + arch, foundArch := streamMeta.Architectures["x86_64"] if !foundArch { return "", fmt.Errorf("couldn't find OS metadata for architecture %q", "x86_64") } @@ -166,8 +181,8 @@ func OpenStackReleaseImage(releaseImage *releaseinfo.ReleaseImage) (string, erro } // PrefixedClusterImageName returns a prefixed name of the image for the given HostedCluster. -func PrefixedClusterImageName(hcluster *hyperv1.HostedCluster, releaseImage *releaseinfo.ReleaseImage) (string, error) { - releaseVersion, err := OpenStackReleaseImage(releaseImage) +func PrefixedClusterImageName(hcluster *hyperv1.HostedCluster, streamMeta *stream.Stream) (string, error) { + releaseVersion, err := OpenStackReleaseImage(streamMeta) if err != nil { return "", err } diff --git a/hypershift-operator/controllers/nodepool/openstack/openstack_test.go b/hypershift-operator/controllers/nodepool/openstack/openstack_test.go index 17da96ff3326..ef5d55322e0c 100644 --- a/hypershift-operator/controllers/nodepool/openstack/openstack_test.go +++ b/hypershift-operator/controllers/nodepool/openstack/openstack_test.go @@ -205,25 +205,28 @@ func TestOpenStackMachineTemplate(t *testing.T) { func TestOpenstackDefaultImage(t *testing.T) { testCases := []struct { name string - releaseImage *releaseinfo.ReleaseImage + streamMeta *stream.Stream expectedURL string expectedHash string expectedError bool }{ + { + name: "nil stream metadata", + streamMeta: nil, + expectedError: true, + }, { name: "valid metadata", - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Artifacts: map[string]stream.PlatformArtifacts{ - "openstack": { - Formats: map[string]stream.ImageFormat{ - "qcow2.gz": { - Disk: &stream.Artifact{ - Location: "https://example.com/image.qcow2.gz", - Sha256: "abcdef1234567890", - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Artifacts: map[string]stream.PlatformArtifacts{ + "openstack": { + Formats: map[string]stream.ImageFormat{ + "qcow2.gz": { + Disk: &stream.Artifact{ + Location: "https://example.com/image.qcow2.gz", + Sha256: "abcdef1234567890", }, }, }, @@ -238,29 +241,25 @@ func TestOpenstackDefaultImage(t *testing.T) { }, { name: "missing architecture", - releaseImage: &releaseinfo.ReleaseImage{StreamMetadata: &stream.Stream{Architectures: map[string]stream.Arch{}}}, + streamMeta: &stream.Stream{Architectures: map[string]stream.Arch{}}, expectedError: true, }, { name: "missing openstack artifact", - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": {Artifacts: map[string]stream.PlatformArtifacts{}}, - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": {Artifacts: map[string]stream.PlatformArtifacts{}}, }, }, expectedError: true, }, { name: "missing qcow2.gz format", - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Artifacts: map[string]stream.PlatformArtifacts{ - "openstack": {Formats: map[string]stream.ImageFormat{}}, - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Artifacts: map[string]stream.PlatformArtifacts{ + "openstack": {Formats: map[string]stream.ImageFormat{}}, }, }, }, @@ -269,15 +268,13 @@ func TestOpenstackDefaultImage(t *testing.T) { }, { name: "missing disk artifact", - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Artifacts: map[string]stream.PlatformArtifacts{ - "openstack": { - Formats: map[string]stream.ImageFormat{ - "qcow2.gz": {}, - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Artifacts: map[string]stream.PlatformArtifacts{ + "openstack": { + Formats: map[string]stream.ImageFormat{ + "qcow2.gz": {}, }, }, }, @@ -290,7 +287,7 @@ func TestOpenstackDefaultImage(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - url, hash, err := OpenstackDefaultImage(tc.releaseImage) + url, hash, err := OpenstackDefaultImage(tc.streamMeta) if tc.expectedError { if err == nil { t.Error("expected error but got nil") @@ -313,20 +310,23 @@ func TestOpenstackDefaultImage(t *testing.T) { func TestOpenStackReleaseImage(t *testing.T) { testCases := []struct { name string - releaseImage *releaseinfo.ReleaseImage + streamMeta *stream.Stream expectedResult string expectedError bool }{ + { + name: "nil stream metadata", + streamMeta: nil, + expectedError: true, + }, { name: "valid metadata", - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Artifacts: map[string]stream.PlatformArtifacts{ - "openstack": { - Release: "4.9.0", - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Artifacts: map[string]stream.PlatformArtifacts{ + "openstack": { + Release: "4.9.0", }, }, }, @@ -337,16 +337,14 @@ func TestOpenStackReleaseImage(t *testing.T) { }, { name: "missing architecture", - releaseImage: &releaseinfo.ReleaseImage{StreamMetadata: &stream.Stream{Architectures: map[string]stream.Arch{}}}, + streamMeta: &stream.Stream{Architectures: map[string]stream.Arch{}}, expectedError: true, }, { name: "missing openstack artifact", - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": {Artifacts: map[string]stream.PlatformArtifacts{}}, - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": {Artifacts: map[string]stream.PlatformArtifacts{}}, }, }, expectedError: true, @@ -355,7 +353,7 @@ func TestOpenStackReleaseImage(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - result, err := OpenStackReleaseImage(tc.releaseImage) + result, err := OpenStackReleaseImage(tc.streamMeta) if tc.expectedError { if err == nil { t.Error("expected error but got nil") @@ -456,9 +454,7 @@ func TestReconcileOpenStackImageSpec(t *testing.T) { }, releaseImage: &releaseinfo.ReleaseImage{ StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - // Missing x86_64 architecture data will cause the OpenstackDefaultImage to fail - }, + Architectures: map[string]stream.Arch{}, }, }, expectedError: true, @@ -498,10 +494,21 @@ func TestClusterImageName(t *testing.T) { testCases := []struct { name string hostedCluster *hyperv1.HostedCluster - releaseImage *releaseinfo.ReleaseImage + streamMeta *stream.Stream expectedResult string expectedError bool }{ + { + name: "nil stream metadata", + hostedCluster: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "clusters", + }, + }, + streamMeta: nil, + expectedError: true, + }, { name: "valid release image", hostedCluster: &hyperv1.HostedCluster{ @@ -510,14 +517,12 @@ func TestClusterImageName(t *testing.T) { Namespace: "clusters", }, }, - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Artifacts: map[string]stream.PlatformArtifacts{ - "openstack": { - Release: "4.19.0", - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Artifacts: map[string]stream.PlatformArtifacts{ + "openstack": { + Release: "4.19.0", }, }, }, @@ -534,10 +539,8 @@ func TestClusterImageName(t *testing.T) { Namespace: "clusters", }, }, - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{}, - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{}, }, expectedError: true, }, @@ -549,12 +552,10 @@ func TestClusterImageName(t *testing.T) { Namespace: "clusters", }, }, - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "x86_64": { - Artifacts: map[string]stream.PlatformArtifacts{}, - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + Artifacts: map[string]stream.PlatformArtifacts{}, }, }, }, @@ -564,7 +565,7 @@ func TestClusterImageName(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - result, err := PrefixedClusterImageName(tc.hostedCluster, tc.releaseImage) + result, err := PrefixedClusterImageName(tc.hostedCluster, tc.streamMeta) if tc.expectedError { if err == nil { t.Error("expected error but got nil") diff --git a/hypershift-operator/controllers/nodepool/powervs.go b/hypershift-operator/controllers/nodepool/powervs.go index 7f2bf8ee9fb4..742ba0caad3c 100644 --- a/hypershift-operator/controllers/nodepool/powervs.go +++ b/hypershift-operator/controllers/nodepool/powervs.go @@ -50,8 +50,12 @@ func getImageRegion(region string) string { func ibmPowerVSMachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) (*capipowervs.IBMPowerVSMachineTemplateSpec, error) { // Validate PowerVS platform specific input + streamMeta, err := releaseImage.StreamForName("") + if err != nil { + return nil, fmt.Errorf("couldn't resolve stream metadata: %w", err) + } var coreOSPowerVSImage *stream.SingleObject - coreOSPowerVSImage, _, err := getPowerVSImage(hcluster.Spec.Platform.PowerVS.Region, releaseImage) + coreOSPowerVSImage, _, err = getPowerVSImage(hcluster.Spec.Platform.PowerVS.Region, streamMeta) if err != nil { return nil, fmt.Errorf("couldn't discover a PowerVS Image for release image: %w", err) } @@ -110,8 +114,8 @@ func (c *CAPI) ibmPowerVSMachineTemplate(templateNameGenerator func(spec any) (s return template, nil } -func getPowerVSImage(region string, releaseImage *releaseinfo.ReleaseImage) (*stream.SingleObject, string, error) { - arch, foundArch := releaseImage.StreamMetadata.Architectures["ppc64le"] +func getPowerVSImage(region string, streamMeta *stream.Stream) (*stream.SingleObject, string, error) { + arch, foundArch := streamMeta.Architectures["ppc64le"] if !foundArch { return nil, "", fmt.Errorf("couldn't find OS metadata for architecture %q", "ppc64le") } @@ -162,7 +166,18 @@ func reconcileIBMPowerVSImage(ibmPowerVSImage *capipowervs.IBMPowerVSImage, hclu func (r *NodePoolReconciler) setPowerVSconditions(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, controlPlaneNamespace string, releaseImage *releaseinfo.ReleaseImage) error { log := ctrl.LoggerFrom(ctx) var coreOSPowerVSImage *stream.SingleObject - coreOSPowerVSImage, powervsImageRegion, err := getPowerVSImage(hcluster.Spec.Platform.PowerVS.Region, releaseImage) + streamMeta, err := releaseImage.StreamForName("") + if err != nil { + SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ + Type: hyperv1.NodePoolValidPlatformImageType, + Status: corev1.ConditionFalse, + Reason: hyperv1.NodePoolValidationFailedReason, + Message: fmt.Sprintf("Couldn't resolve stream metadata for release image %q: %s", nodePool.Spec.Release.Image, err.Error()), + ObservedGeneration: nodePool.Generation, + }) + return fmt.Errorf("couldn't resolve stream metadata: %w", err) + } + coreOSPowerVSImage, powervsImageRegion, err := getPowerVSImage(hcluster.Spec.Platform.PowerVS.Region, streamMeta) if err != nil { SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolValidPlatformImageType, diff --git a/hypershift-operator/controllers/nodepool/powervs_test.go b/hypershift-operator/controllers/nodepool/powervs_test.go index 4b27a0793be1..01d9c6b7eab7 100644 --- a/hypershift-operator/controllers/nodepool/powervs_test.go +++ b/hypershift-operator/controllers/nodepool/powervs_test.go @@ -5,8 +5,6 @@ import ( . "github.com/onsi/gomega" - "github.com/openshift/hypershift/support/releaseinfo" - "github.com/coreos/stream-metadata-go/stream" ) @@ -14,18 +12,16 @@ func TestGetPowerVSImage(t *testing.T) { testCases := []struct { name string region string - releaseImage *releaseinfo.ReleaseImage + streamMeta *stream.Stream expectedError string }{ { name: "When PowerVS images is nil, it should return error", region: "us-south", - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{ - "ppc64le": { - Images: stream.Images{}, - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "ppc64le": { + Images: stream.Images{}, }, }, }, @@ -34,10 +30,8 @@ func TestGetPowerVSImage(t *testing.T) { { name: "When architecture is not found, it should return error", region: "us-south", - releaseImage: &releaseinfo.ReleaseImage{ - StreamMetadata: &stream.Stream{ - Architectures: map[string]stream.Arch{}, - }, + streamMeta: &stream.Stream{ + Architectures: map[string]stream.Arch{}, }, expectedError: "couldn't find OS metadata for architecture", }, @@ -46,7 +40,7 @@ func TestGetPowerVSImage(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - _, _, err := getPowerVSImage(tc.region, tc.releaseImage) + _, _, err := getPowerVSImage(tc.region, tc.streamMeta) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) }) diff --git a/hypershift-operator/controllers/nodepool/token.go b/hypershift-operator/controllers/nodepool/token.go index 4a39caffba7d..4249abfccade 100644 --- a/hypershift-operator/controllers/nodepool/token.go +++ b/hypershift-operator/controllers/nodepool/token.go @@ -404,14 +404,17 @@ func (t *Token) reconcileUserDataSecret(log logr.Logger, userDataSecret *corev1. } func setKarpenterAMILabels(log logr.Logger, userDataSecret *corev1.Secret, region string, releaseImage *releaseinfo.ReleaseImage, platform hyperv1.PlatformType) error { - // TODO(CNTRLPLANE-3553): resolve streamName via GetRHELStream once osImageStream API field is available + streamMeta, err := releaseImage.StreamForName("") + if err != nil { + return fmt.Errorf("failed to resolve stream metadata: %w", err) + } supportedArchitectures, err := karpenterutil.SupportedArchitectures(platform) if err != nil { return fmt.Errorf("failed to get supported architectures: %w", err) } supported := 0 for _, arch := range supportedArchitectures { - ami, err := defaultNodePoolAMI(region, arch, "", releaseImage) + ami, err := defaultNodePoolAMI(region, arch, streamMeta) if err != nil { // skip unavailable architectures gracefully log.Error(err, "failed to get default NodePool AMI for architecture", "architecture", arch) diff --git a/support/releaseinfo/registryclient_provider.go b/support/releaseinfo/registryclient_provider.go index d2828588c179..708915d7cf85 100644 --- a/support/releaseinfo/registryclient_provider.go +++ b/support/releaseinfo/registryclient_provider.go @@ -41,6 +41,15 @@ func (p *RegistryClientProvider) Lookup(ctx context.Context, image string, pullS return nil, err } + if coreOSMeta == nil && len(osStreams) > 0 { + for _, s := range osStreams { + if s != nil { + coreOSMeta = s + break + } + } + } + return &ReleaseImage{ ImageStream: imageStream, StreamMetadata: coreOSMeta,