diff --git a/hypershift-operator/controllers/nodepool/azure.go b/hypershift-operator/controllers/nodepool/azure.go index 0419a4af8576..007dd2b7bdb6 100644 --- a/hypershift-operator/controllers/nodepool/azure.go +++ b/hypershift-operator/controllers/nodepool/azure.go @@ -37,7 +37,7 @@ type azureMarketplaceImageInfo struct { // defaultAzureNodePoolImage applies Azure Marketplace image defaults for OCP >= 4.20 // when Type is AzureMarketplace and azureMarketplace data is not provided and marketplace metadata is available in the release payload. -func defaultAzureNodePoolImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) error { +func defaultAzureNodePoolImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage, streamName string) error { // Skip if ImageID is explicitly set if nodePool.Spec.Platform.Azure.Image.ImageID != nil { return nil @@ -82,7 +82,7 @@ func defaultAzureNodePoolImage(nodePool *hyperv1.NodePool, releaseImage *release } // Extract marketplace metadata from release payload - azureMarketplace, err := getAzureMarketplaceMetadata(releaseImage, streamArch) + azureMarketplace, err := getAzureMarketplaceMetadata(releaseImage, streamArch, streamName) if err != nil { return fmt.Errorf("failed to get Azure Marketplace metadata: %w", err) } @@ -120,15 +120,18 @@ 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) { - // TODO(CNTRLPLANE-3553): use releaseImage.StreamForName(rhelStream) instead of - // accessing StreamMetadata directly, to support dual-stream payloads. - if releaseImage.StreamMetadata == nil { - return nil, nil // No stream metadata available +// getAzureMarketplaceMetadata extracts Azure Marketplace metadata from the release payload. +// Note: unlike the pre-streamName code this returns an error (rather than nil, nil) +// when no stream metadata is available at all. The version guard in +// defaultAzureNodePoolImage (< 4.20 returns early) protects older payloads; +// for 4.20+ a missing StreamMetadata indicates a broken release image. +func getAzureMarketplaceMetadata(releaseImage *releaseinfo.ReleaseImage, arch string, streamName string) (*azureMarketplaceMetadata, error) { + streamMeta, err := releaseImage.StreamForName(streamName) + if err != nil { + return nil, fmt.Errorf("couldn't resolve stream metadata for stream %q: %w", streamName, err) } - 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) } @@ -270,7 +273,7 @@ func azureMachineTemplateSpec(nodePool *hyperv1.NodePool, acrIdentityResourceID func (c *CAPI) azureMachineTemplate(_ context.Context, templateNameGenerator func(spec any) (string, error)) (*capiazure.AzureMachineTemplate, error) { // Apply Azure Marketplace image defaults before generating machine template spec - if err := defaultAzureNodePoolImage(c.nodePool, c.ConfigGenerator.rolloutConfig.releaseImage); err != nil { + if err := defaultAzureNodePoolImage(c.nodePool, c.ConfigGenerator.rolloutConfig.releaseImage, c.resolvedRHELStreamForBootImage); err != nil { return nil, fmt.Errorf("failed to apply Azure image defaults: %w", err) } diff --git a/hypershift-operator/controllers/nodepool/azure_test.go b/hypershift-operator/controllers/nodepool/azure_test.go index db45dcb80a0a..a6403fa39fb8 100644 --- a/hypershift-operator/controllers/nodepool/azure_test.go +++ b/hypershift-operator/controllers/nodepool/azure_test.go @@ -969,6 +969,7 @@ func TestDefaultAzureNodePoolImage(t *testing.T) { name string nodePool *hyperv1.NodePool releaseImage *releaseinfo.ReleaseImage + streamName string expectedImageType hyperv1.AzureVMImageType expectedMarketplaceImage *hyperv1.AzureMarketplaceImage expectedError bool @@ -1271,13 +1272,67 @@ func TestDefaultAzureNodePoolImage(t *testing.T) { ImageGeneration: ptr.To(hyperv1.Gen1), }, }, + { + name: "When named stream is used with multi-stream ReleaseImage it should resolve marketplace from the named stream", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + Arch: hyperv1.ArchitectureAMD64, + Platform: hyperv1.NodePoolPlatform{ + Type: hyperv1.AzurePlatform, + Azure: &hyperv1.AzureNodePoolPlatform{ + Image: hyperv1.AzureVMImage{}, + }, + }, + }, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ + ObjectMeta: metav1.ObjectMeta{Name: "4.20.0"}, + }, + OSStreams: map[string]*stream.Stream{ + "rhel-9": { + Architectures: map[string]stream.Arch{ + "x86_64": { + RHELCoreOSExtensions: &rhcos.Extensions{ + AzureDisk: &rhcos.AzureDisk{ + Release: "9.6.20250701-0", + URL: "https://rhcos.blob.core.windows.net/imagebucket/rhcos-9.6.20250701-0-azure.x86_64.vhd", + }, + Marketplace: &rhcos.Marketplace{ + Azure: &rhcos.AzureMarketplace{ + NoPurchasePlan: &rhcos.AzureMarketplaceImages{ + Gen2: &rhcos.AzureMarketplaceImage{ + Publisher: "azureopenshift", + Offer: "aro4", + SKU: "aro_rhel9_420-v2", + Version: "420.9.20250701", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + streamName: "rhel-9", + expectedImageType: hyperv1.AzureMarketplace, + expectedMarketplaceImage: &hyperv1.AzureMarketplaceImage{ + Publisher: "azureopenshift", + Offer: "aro4", + SKU: "aro_rhel9_420-v2", + Version: "420.9.20250701", + ImageGeneration: ptr.To(hyperv1.Gen2), + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewGomegaWithT(t) - err := defaultAzureNodePoolImage(tc.nodePool, tc.releaseImage) + err := defaultAzureNodePoolImage(tc.nodePool, tc.releaseImage, tc.streamName) if tc.expectedError { g.Expect(err).To(HaveOccurred()) diff --git a/hypershift-operator/controllers/nodepool/conditions.go b/hypershift-operator/controllers/nodepool/conditions.go index 132aa4659b9f..a5304dc7353e 100644 --- a/hypershift-operator/controllers/nodepool/conditions.go +++ b/hypershift-operator/controllers/nodepool/conditions.go @@ -367,9 +367,7 @@ func (r *NodePoolReconciler) validMachineConfigCondition(ctx context.Context, no } // Validate osImageStream before expensive config generation to fail fast. - // TODO(CNTRLPLANE-3553): add integration test covering this condition path - // (invalid osImageStream.Name → ValidMachineConfig condition False + error return). - if err := validateOSImageStream(nodePool, releaseImage); err != nil { + if err := validateOSImageStream(ctx, r.Client, nodePool, releaseImage); err != nil { SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolValidMachineConfigConditionType, Status: corev1.ConditionFalse, diff --git a/hypershift-operator/controllers/nodepool/config.go b/hypershift-operator/controllers/nodepool/config.go index 45b5ee408b0e..7dffe44d4a9a 100644 --- a/hypershift-operator/controllers/nodepool/config.go +++ b/hypershift-operator/controllers/nodepool/config.go @@ -77,6 +77,9 @@ type rolloutConfig struct { // setting the default stream does not change the hash. // Only a non-default stream (e.g. "rhel-9" on a ≥5.0 release) produces // a non-empty value here and triggers a rollout. + // + // See also ConfigGenerator.resolvedRHELStreamForBootImage, which controls + // boot image resolution and is intentionally separate from the hash. rhelStream string } @@ -100,12 +103,16 @@ func NewConfigGenerator(ctx context.Context, client client.Client, hostedCluster // default explicitly does not change the hash and trigger a spurious rollout. rhelStream := nodePool.Spec.OSImageStream.Name if rhelStream != "" { - version, err := semver.Parse(releaseImage.Version()) + var version semver.Version + version, err = semver.Parse(releaseImage.Version()) if err != nil { return nil, fmt.Errorf("failed to parse release image version %q: %w", releaseImage.Version(), err) } - // TODO(CNTRLPLANE-3553): pass actual usesRunc once container runtime detection is wired in. - defaultStream, err := GetRHELStream("", version, false) + usesRunc, err := usesRuncRuntime(ctx, client, nodePool) + if err != nil { + return nil, fmt.Errorf("failed to detect container runtime: %w", err) + } + defaultStream, err := GetRHELStream("", version, usesRunc) if err != nil { return nil, fmt.Errorf("failed to resolve default RHEL stream: %w", err) } diff --git a/hypershift-operator/controllers/nodepool/config_test.go b/hypershift-operator/controllers/nodepool/config_test.go index 493e956f8b21..2d8d2f67d652 100644 --- a/hypershift-operator/controllers/nodepool/config_test.go +++ b/hypershift-operator/controllers/nodepool/config_test.go @@ -339,6 +339,63 @@ spec: client: true, error: nil, }, + { + name: "When runc ContainerRuntimeConfig on 5.0.0 with explicit rhel-9, it should normalize rhelStream to empty", + expectedHash: "72ea1773", + expectedHashWithoutVersion: "6d5a7b66", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + }, + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-9"}, + Config: []corev1.LocalObjectReference{ + {Name: "runc-ctrcfg"}, + }, + }, + }, + config: []crclient.Object{ + runcContainerRuntimeConfigMap("test", "runc-ctrcfg"), + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: "5.0.0", + }, + }, + }, + hostedCluster: hostedCluster, + client: true, + error: nil, + }, + { + name: "When runc ContainerRuntimeConfig on 5.0.0 with no osImageStream, it should match explicit rhel-9 hash", + expectedHash: "72ea1773", + expectedHashWithoutVersion: "6d5a7b66", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + }, + Spec: hyperv1.NodePoolSpec{ + Config: []corev1.LocalObjectReference{ + {Name: "runc-ctrcfg"}, + }, + }, + }, + config: []crclient.Object{ + runcContainerRuntimeConfigMap("test", "runc-ctrcfg"), + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: "5.0.0", + }, + }, + }, + hostedCluster: hostedCluster, + client: true, + error: nil, + }, { name: "When additionalTrustBundle is specified it should be included in rolloutConfig", expectedHash: "632801f8", diff --git a/hypershift-operator/controllers/nodepool/kubevirt.go b/hypershift-operator/controllers/nodepool/kubevirt.go index 3b0adc68aa11..c07726b1fdb7 100644 --- a/hypershift-operator/controllers/nodepool/kubevirt.go +++ b/hypershift-operator/controllers/nodepool/kubevirt.go @@ -65,7 +65,9 @@ func (r *NodePoolReconciler) setKubevirtConditions(ctx context.Context, nodePool nodePool.Status.Platform.KubeVirt.Credentials = hcluster.Spec.Platform.Kubevirt.Credentials.DeepCopy() } - kubevirtBootImage, err := kubevirt.GetImage(nodePool, releaseImage, infraNS) + // TODO(CNTRLPLANE-3553): hardcode to rhel-9 until the MCO can install + // rhel-10 OS images. Use getRHELStreamForBootImage once MCO support lands. + kubevirtBootImage, err := kubevirt.GetImage(nodePool, releaseImage, infraNS, StreamRHEL9) if err != nil { SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolValidPlatformImageType, @@ -166,7 +168,7 @@ func (r *NodePoolReconciler) setAllMachinesLMCondition(ctx context.Context, node func (c *CAPI) kubevirtMachineTemplate(templateNameGenerator func(spec any) (string, error)) (*capikubevirt.KubevirtMachineTemplate, error) { nodePool := c.nodePool - spec, err := kubevirt.MachineTemplateSpec(nodePool, c.hostedCluster, c.releaseImage, nil) + spec, err := kubevirt.MachineTemplateSpec(nodePool, c.hostedCluster, c.releaseImage, nil, c.resolvedRHELStreamForBootImage) if err != nil { SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolValidMachineTemplateConditionType, diff --git a/hypershift-operator/controllers/nodepool/kubevirt/kubevirt.go b/hypershift-operator/controllers/nodepool/kubevirt/kubevirt.go index c5c087a6f3d9..145c52cf5c26 100644 --- a/hypershift-operator/controllers/nodepool/kubevirt/kubevirt.go +++ b/hypershift-operator/controllers/nodepool/kubevirt/kubevirt.go @@ -36,7 +36,7 @@ var LocalStorageVolumes = []string{ "hotplug-disks", } -func defaultImage(nodePoolArch string, releaseImage *releaseinfo.ReleaseImage) (string, string, error) { +func defaultImage(nodePoolArch string, releaseImage *releaseinfo.ReleaseImage, streamName string) (string, string, error) { var archName string switch nodePoolArch { case hyperv1.ArchitectureS390X: @@ -44,7 +44,11 @@ func defaultImage(nodePoolArch string, releaseImage *releaseinfo.ReleaseImage) ( default: archName = hyperv1.ArchAliases[hyperv1.ArchitectureAMD64] } - arch, foundArch := releaseImage.StreamMetadata.Architectures[archName] + streamMeta, err := releaseImage.StreamForName(streamName) + if err != nil { + return "", "", fmt.Errorf("couldn't resolve stream metadata for stream %q: %w", streamName, err) + } + arch, foundArch := streamMeta.Architectures[archName] if !foundArch { return "", "", fmt.Errorf("couldn't find OS metadata for architecture %q", archName) @@ -74,7 +78,7 @@ func allowUnsupportedRHCOSVariants(nodePool *hyperv1.NodePool) bool { return false } -func GetImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage, hostedNamespace string) (BootImage, error) { +func GetImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage, hostedNamespace string, streamName string) (BootImage, error) { var rootVolume *hyperv1.KubevirtRootVolume isHTTP := false if nodePool.Spec.Platform.Kubevirt != nil { @@ -90,9 +94,9 @@ func GetImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage return newBootImage(imageName, isHTTP), nil } - imageName, imageHash, err := defaultImage(nodePool.Spec.Arch, releaseImage) + imageName, imageHash, err := defaultImage(nodePool.Spec.Arch, releaseImage, streamName) if err != nil && allowUnsupportedRHCOSVariants(nodePool) { - imageName, imageHash, err = openstack.OpenstackDefaultImage(releaseImage) + imageName, imageHash, err = openstack.OpenstackDefaultImage(releaseImage, streamName) if err != nil { return nil, err } @@ -351,7 +355,7 @@ func shouldAttachDefaultNetwork(kvPlatform *hyperv1.KubevirtNodePoolPlatform) bo return kvPlatform.AttachDefaultNetwork == nil || *kvPlatform.AttachDefaultNetwork } -func MachineTemplateSpec(nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, releaseImage *releaseinfo.ReleaseImage, bootImage BootImage) (*capikubevirt.KubevirtMachineTemplateSpec, error) { +func MachineTemplateSpec(nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, releaseImage *releaseinfo.ReleaseImage, bootImage BootImage, streamName string) (*capikubevirt.KubevirtMachineTemplateSpec, error) { if bootImage == nil { infraNS := manifests.HostedControlPlaneNamespace(hcluster.Namespace, hcluster.Name) if hcluster.Spec.Platform.Kubevirt != nil && @@ -361,7 +365,7 @@ func MachineTemplateSpec(nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedClu infraNS = hcluster.Spec.Platform.Kubevirt.Credentials.InfraNamespace } var err error - bootImage, err = GetImage(nodePool, releaseImage, infraNS) + bootImage, err = GetImage(nodePool, releaseImage, infraNS, streamName) if err != nil { return nil, fmt.Errorf("couldn't discover a KubeVirt Image in release payload image: %w", err) } diff --git a/hypershift-operator/controllers/nodepool/kubevirt/kubevirt_test.go b/hypershift-operator/controllers/nodepool/kubevirt/kubevirt_test.go index 2084a6b65cc5..a73d3f6e65d7 100644 --- a/hypershift-operator/controllers/nodepool/kubevirt/kubevirt_test.go +++ b/hypershift-operator/controllers/nodepool/kubevirt/kubevirt_test.go @@ -601,7 +601,7 @@ func TestKubevirtMachineTemplate(t *testing.T) { } bootImage := newCachedBootImage(bootImageName, imageHash, hostedClusterNamespace, false, np) - result, err := MachineTemplateSpec(tc.nodePool, tc.hcluster, &releaseinfo.ReleaseImage{}, bootImage) + result, err := MachineTemplateSpec(tc.nodePool, tc.hcluster, &releaseinfo.ReleaseImage{}, bootImage, "") g.Expect(err).ToNot(HaveOccurred()) g.Expect(result).To(Equal(tc.expected), "Comparison failed\n%v", cmp.Diff(tc.expected, result)) }) @@ -1189,7 +1189,7 @@ func TestJsonPatch(t *testing.T) { bootImage := newCachedBootImage(bootImageName, imageHash, hostedClusterNamespace, false, nil) bootImage.dvName = bootImageNamePrefix + "12345" - result, err := MachineTemplateSpec(tc.nodePool, tc.hcluster, &releaseinfo.ReleaseImage{}, bootImage) + result, err := MachineTemplateSpec(tc.nodePool, tc.hcluster, &releaseinfo.ReleaseImage{}, bootImage, "") g.Expect(err).ToNot(HaveOccurred()) g.Expect(result).To(Equal(tc.expected), "Comparison failed\n%v", cmp.Diff(tc.expected, result)) }) @@ -1535,6 +1535,7 @@ func TestDefaultImage(t *testing.T) { tests := []struct { name string arch string + streamName string releaseImage *releaseinfo.ReleaseImage expectedImage string expectedDigest string @@ -1572,6 +1573,28 @@ func TestDefaultImage(t *testing.T) { expectedImage: "quay.io/openshift/release@sha256:x86_641234", expectedDigest: "sha256:x86_641234", }, + { + name: "When named stream is used with multi-stream ReleaseImage it should resolve from the named stream", + arch: hyperv1.ArchitectureAMD64, + streamName: "rhel-9", + releaseImage: &releaseinfo.ReleaseImage{ + OSStreams: map[string]*stream.Stream{ + "rhel-9": { + Architectures: map[string]stream.Arch{ + hyperv1.ArchAliases[hyperv1.ArchitectureAMD64]: { + Images: stream.Images{ + KubeVirt: &stream.ContainerImage{ + DigestRef: "quay.io/openshift/release@sha256:rhel9kubevirt", + }, + }, + }, + }, + }, + }, + }, + expectedImage: "quay.io/openshift/release@sha256:rhel9kubevirt", + expectedDigest: "sha256:rhel9kubevirt", + }, } for _, tt := range tests { @@ -1580,7 +1603,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, tt.streamName) if tt.expectedError { if err == nil { t.Fatalf("expected error but got nil") diff --git a/hypershift-operator/controllers/nodepool/openstack.go b/hypershift-operator/controllers/nodepool/openstack.go index 09f1340aba19..ccc1b1e79c76 100644 --- a/hypershift-operator/controllers/nodepool/openstack.go +++ b/hypershift-operator/controllers/nodepool/openstack.go @@ -20,7 +20,7 @@ import ( func (c *CAPI) openstackMachineTemplate(templateNameGenerator func(spec any) (string, error)) (*capiopenstackv1beta1.OpenStackMachineTemplate, error) { nodePool := c.nodePool - spec, err := openstack.MachineTemplateSpec(c.hostedCluster, nodePool, c.releaseImage) + spec, err := openstack.MachineTemplateSpec(c.hostedCluster, nodePool, c.releaseImage, c.resolvedRHELStreamForBootImage) if err != nil { SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolValidMachineTemplateConditionType, @@ -50,8 +50,11 @@ func (c *CAPI) openstackMachineTemplate(templateNameGenerator func(spec any) (st return template, nil } func (r *NodePoolReconciler) setOpenStackConditions(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, _ string, releaseImage *releaseinfo.ReleaseImage) error { + // TODO(CNTRLPLANE-3553): hardcode to rhel-9 until the MCO can install + // rhel-10 OS images. Use getRHELStreamForBootImage once MCO support lands. + rhelStream := StreamRHEL9 if nodePool.Spec.Platform.OpenStack.ImageName == "" { - _, err := openstack.OpenStackReleaseImage(releaseImage) + _, err := openstack.OpenStackReleaseImage(releaseImage, rhelStream) if err != nil { SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolValidPlatformImageType, @@ -62,7 +65,7 @@ func (r *NodePoolReconciler) setOpenStackConditions(ctx context.Context, nodePoo }) return fmt.Errorf("couldn't discover an OpenStack Image for release image: %w", err) } - imageName, err := r.reconcileOpenStackImageCR(ctx, r.Client, hcluster, releaseImage, nodePool) + imageName, err := r.reconcileOpenStackImageCR(ctx, r.Client, hcluster, releaseImage, nodePool, rhelStream) if err != nil { return err } @@ -88,8 +91,8 @@ func (r *NodePoolReconciler) setOpenStackConditions(ctx context.Context, nodePoo // reconcileOpenStackImageCR reconciles the OpenStack Image CR for the given NodePool. // 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) +func (r *NodePoolReconciler) reconcileOpenStackImageCR(ctx context.Context, client client.Client, hcluster *hyperv1.HostedCluster, release *releaseinfo.ReleaseImage, nodePool *hyperv1.NodePool, streamName string) (string, error) { + releaseVersion, err := openstack.OpenStackReleaseImage(release, streamName) if err != nil { return "", err } @@ -119,7 +122,7 @@ func (r *NodePoolReconciler) reconcileOpenStackImageCR(ctx context.Context, clie } if _, err := r.CreateOrUpdate(ctx, client, &openStackImage, func() error { - err := openstack.ReconcileOpenStackImageSpec(hcluster, &openStackImage.Spec, release) + err := openstack.ReconcileOpenStackImageSpec(hcluster, &openStackImage.Spec, release, streamName) if err != nil { return err } diff --git a/hypershift-operator/controllers/nodepool/openstack/openstack.go b/hypershift-operator/controllers/nodepool/openstack/openstack.go index 75878b5bd515..e9e4619b95f4 100644 --- a/hypershift-operator/controllers/nodepool/openstack/openstack.go +++ b/hypershift-operator/controllers/nodepool/openstack/openstack.go @@ -17,7 +17,7 @@ import ( orc "github.com/k-orc/openstack-resource-controller/v2/api/v1alpha1" ) -func MachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) (*capiopenstackv1beta1.OpenStackMachineTemplateSpec, error) { +func MachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage, streamName string) (*capiopenstackv1beta1.OpenStackMachineTemplateSpec, error) { openStackMachineTemplate := &capiopenstackv1beta1.OpenStackMachineTemplateSpec{Template: capiopenstackv1beta1.OpenStackMachineTemplateResource{Spec: capiopenstackv1beta1.OpenStackMachineSpec{ Flavor: ptr.To(nodePool.Spec.Platform.OpenStack.Flavor), }}} @@ -27,7 +27,7 @@ func MachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hyperv1.Node Name: ptr.To(nodePool.Spec.Platform.OpenStack.ImageName), } } else { - releaseVersion, err := OpenStackReleaseImage(releaseImage) + releaseVersion, err := OpenStackReleaseImage(releaseImage, streamName) if err != nil { return nil, err } @@ -94,8 +94,8 @@ 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) +func ReconcileOpenStackImageSpec(hcluster *hyperv1.HostedCluster, openStackImageSpec *orc.ImageSpec, release *releaseinfo.ReleaseImage, streamName string) error { + imageURL, imageHash, err := OpenstackDefaultImage(release, streamName) if err != nil { return fmt.Errorf("failed to lookup RHCOS image: %w", err) } @@ -105,7 +105,7 @@ func ReconcileOpenStackImageSpec(hcluster *hyperv1.HostedCluster, openStackImage CloudName: hcluster.Spec.Platform.OpenStack.IdentityRef.CloudName, } - imageName, err := PrefixedClusterImageName(hcluster, release) + imageName, err := PrefixedClusterImageName(hcluster, release, streamName) if err != nil { return fmt.Errorf("failed to get image name: %w", err) } @@ -131,10 +131,12 @@ func ReconcileOpenStackImageSpec(hcluster *hyperv1.HostedCluster, openStackImage // OpenstackDefaultImage returns the default RHCOS image for the given release. // The image URL and SHA256 hash are returned. -func OpenstackDefaultImage(releaseImage *releaseinfo.ReleaseImage) (string, string, error) { - // TODO(CNTRLPLANE-3553): use releaseImage.StreamForName(rhelStream) instead of - // accessing StreamMetadata directly, to support dual-stream payloads. - arch, foundArch := releaseImage.StreamMetadata.Architectures["x86_64"] +func OpenstackDefaultImage(releaseImage *releaseinfo.ReleaseImage, streamName string) (string, string, error) { + streamMeta, err := releaseImage.StreamForName(streamName) + if err != nil { + return "", "", fmt.Errorf("couldn't resolve stream metadata for stream %q: %w", streamName, err) + } + arch, foundArch := streamMeta.Architectures["x86_64"] if !foundArch { return "", "", fmt.Errorf("couldn't find OS metadata for architecture %q", "x86_64") } @@ -155,10 +157,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) { - // TODO(CNTRLPLANE-3553): use releaseImage.StreamForName(rhelStream) instead of - // accessing StreamMetadata directly, to support dual-stream payloads. - arch, foundArch := releaseImage.StreamMetadata.Architectures["x86_64"] +func OpenStackReleaseImage(releaseImage *releaseinfo.ReleaseImage, streamName string) (string, error) { + streamMeta, err := releaseImage.StreamForName(streamName) + if err != nil { + return "", fmt.Errorf("couldn't resolve stream metadata for stream %q: %w", streamName, err) + } + arch, foundArch := streamMeta.Architectures["x86_64"] if !foundArch { return "", fmt.Errorf("couldn't find OS metadata for architecture %q", "x86_64") } @@ -170,8 +174,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) (orc.OpenStackName, error) { - releaseVersion, err := OpenStackReleaseImage(releaseImage) +func PrefixedClusterImageName(hcluster *hyperv1.HostedCluster, releaseImage *releaseinfo.ReleaseImage, streamName string) (orc.OpenStackName, error) { + releaseVersion, err := OpenStackReleaseImage(releaseImage, streamName) 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 1ca26db5fe76..384eb5fd09f4 100644 --- a/hypershift-operator/controllers/nodepool/openstack/openstack_test.go +++ b/hypershift-operator/controllers/nodepool/openstack/openstack_test.go @@ -185,7 +185,7 @@ func TestOpenStackMachineTemplate(t *testing.T) { Name: "tests", }, Spec: tc.nodePool, - }, &releaseinfo.ReleaseImage{}) + }, &releaseinfo.ReleaseImage{}, "") if tc.checkError != nil { tc.checkError(t, err) } else { @@ -206,6 +206,7 @@ func TestOpenstackDefaultImage(t *testing.T) { testCases := []struct { name string releaseImage *releaseinfo.ReleaseImage + streamName string expectedURL string expectedHash string expectedError bool @@ -286,11 +287,39 @@ func TestOpenstackDefaultImage(t *testing.T) { }, expectedError: true, }, + { + name: "When named stream is used with multi-stream ReleaseImage it should resolve from the named stream", + releaseImage: &releaseinfo.ReleaseImage{ + OSStreams: map[string]*stream.Stream{ + "rhel-9": { + 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/rhel9-image.qcow2.gz", + Sha256: "rhel9hash1234", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + streamName: "rhel-9", + expectedURL: "https://example.com/rhel9-image.qcow2.gz", + expectedHash: "rhel9hash1234", + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - url, hash, err := OpenstackDefaultImage(tc.releaseImage) + url, hash, err := OpenstackDefaultImage(tc.releaseImage, tc.streamName) if tc.expectedError { if err == nil { t.Error("expected error but got nil") @@ -314,6 +343,7 @@ func TestOpenStackReleaseImage(t *testing.T) { testCases := []struct { name string releaseImage *releaseinfo.ReleaseImage + streamName string expectedResult string expectedError bool }{ @@ -351,11 +381,31 @@ func TestOpenStackReleaseImage(t *testing.T) { }, expectedError: true, }, + { + name: "When named stream is used with multi-stream ReleaseImage it should resolve from the named stream", + releaseImage: &releaseinfo.ReleaseImage{ + OSStreams: map[string]*stream.Stream{ + "rhel-9": { + Architectures: map[string]stream.Arch{ + "x86_64": { + Artifacts: map[string]stream.PlatformArtifacts{ + "openstack": { + Release: "9.6.20250701", + }, + }, + }, + }, + }, + }, + }, + streamName: "rhel-9", + expectedResult: "9.6.20250701", + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - result, err := OpenStackReleaseImage(tc.releaseImage) + result, err := OpenStackReleaseImage(tc.releaseImage, tc.streamName) if tc.expectedError { if err == nil { t.Error("expected error but got nil") @@ -469,7 +519,7 @@ func TestReconcileOpenStackImageSpec(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { imageSpec := &orc.ImageSpec{} - err := ReconcileOpenStackImageSpec(tc.hostedCluster, imageSpec, tc.releaseImage) + err := ReconcileOpenStackImageSpec(tc.hostedCluster, imageSpec, tc.releaseImage, "") if tc.expectedError { if err == nil { @@ -564,7 +614,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.releaseImage, "") if tc.expectedError { if err == nil { t.Error("expected error but got nil") diff --git a/hypershift-operator/controllers/nodepool/osstream.go b/hypershift-operator/controllers/nodepool/osstream.go index 700b08a57fb1..862164b6c25f 100644 --- a/hypershift-operator/controllers/nodepool/osstream.go +++ b/hypershift-operator/controllers/nodepool/osstream.go @@ -1,14 +1,84 @@ package nodepool import ( + "bufio" + "context" + coreerrors "errors" "fmt" + "io" + "strings" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/support/api" "github.com/openshift/hypershift/support/releaseinfo" + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/util/yaml" + + "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/blang/semver" ) +// ctrcfgDecoder is reused across calls to avoid allocating a new Scheme + CodecFactory +// on every invocation. api.Scheme already has MCO types registered. +var ctrcfgDecoder = runtimeserializer.NewCodecFactory(api.Scheme).UniversalDeserializer() + +// usesRuncRuntime scans the NodePool's user-supplied ConfigMaps for any +// ContainerRuntimeConfig that sets defaultRuntime to "runc". +// Returns true if runc is explicitly requested by any config entry. +func usesRuncRuntime(ctx context.Context, c client.Client, nodePool *hyperv1.NodePool) (bool, error) { + if len(nodePool.Spec.Config) == 0 { + return false, nil + } + + for _, ref := range nodePool.Spec.Config { + cm := &corev1.ConfigMap{} + if err := c.Get(ctx, client.ObjectKey{ + Namespace: nodePool.Namespace, + Name: ref.Name, + }, cm); err != nil { + if apierrors.IsNotFound(err) { + // ConfigMap doesn't exist yet — validation catches this elsewhere. + continue + } + return false, fmt.Errorf("failed to get ConfigMap %s/%s: %w", nodePool.Namespace, ref.Name, err) + } + payload := cm.Data[TokenSecretConfigKey] + if payload == "" { + continue + } + yamlReader := yaml.NewYAMLReader(bufio.NewReader(strings.NewReader(payload))) + for { + raw, err := yamlReader.Read() + if err != nil && !coreerrors.Is(err, io.EOF) { + return false, fmt.Errorf("failed to read YAML from ConfigMap %s/%s: %w", nodePool.Namespace, ref.Name, err) + } + if len(strings.TrimSpace(string(raw))) > 0 { + obj, _, decodeErr := ctrcfgDecoder.Decode(raw, nil, nil) + // Decode errors are expected for non-ContainerRuntimeConfig resources + // (MachineConfig, KubeletConfig, etc.); only ContainerRuntimeConfig is relevant here. + if decodeErr == nil { + if ctrcfg, ok := obj.(*mcfgv1.ContainerRuntimeConfig); ok { + if ctrcfg.Spec.ContainerRuntimeConfig != nil && + ctrcfg.Spec.ContainerRuntimeConfig.DefaultRuntime == mcfgv1.ContainerRuntimeDefaultRuntimeRunc { + return true, nil + } + } + } + } + if coreerrors.Is(err, io.EOF) { + break + } + } + } + return false, nil +} + // getRHELStreamForBootImage returns the RHEL stream name to pass to // StreamForName when resolving platform-specific boot images (AMIs, VHDs, // GCE images, etc.). @@ -24,24 +94,28 @@ import ( // spec.osImageStream will transition from rhel-9 to rhel-10 boot // images. This is the intended behavior per the enhancement: // implicit-stream NodePools automatically adopt the new default. -func getRHELStreamForBootImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) (string, error) { +func getRHELStreamForBootImage(ctx context.Context, c client.Client, nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) (string, error) { version, err := semver.Parse(releaseImage.Version()) if err != nil { return "", fmt.Errorf("failed to parse release image version %q: %w", releaseImage.Version(), err) } - // TODO(CNTRLPLANE-3553): pass actual usesRunc once container runtime detection is wired in. - return GetRHELStream(nodePool.Spec.OSImageStream.Name, version, false) + usesRunc, err := usesRuncRuntime(ctx, c, nodePool) + if err != nil { + return "", fmt.Errorf("failed to detect container runtime: %w", err) + } + + return GetRHELStream(nodePool.Spec.OSImageStream.Name, version, usesRunc) } // validateOSImageStream checks that spec.osImageStream.Name, if set, is a -// valid stream for the given release version. Returns an error describing the -// problem or nil. It delegates to getRHELStreamForBootImage for version-aware -// validation. -func validateOSImageStream(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) error { +// valid stream for the given release version and container runtime +// configuration. Returns an error describing the problem or nil. +// It delegates to GetRHELStream for version-aware validation. +func validateOSImageStream(ctx context.Context, c client.Client, nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) error { if nodePool.Spec.OSImageStream.Name == "" { return nil } - _, err := getRHELStreamForBootImage(nodePool, releaseImage) + _, err := getRHELStreamForBootImage(ctx, c, nodePool, releaseImage) return err } diff --git a/hypershift-operator/controllers/nodepool/osstream_test.go b/hypershift-operator/controllers/nodepool/osstream_test.go index 04406a2d098c..c9efb9af93ca 100644 --- a/hypershift-operator/controllers/nodepool/osstream_test.go +++ b/hypershift-operator/controllers/nodepool/osstream_test.go @@ -6,11 +6,16 @@ import ( . "github.com/onsi/gomega" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/support/api" "github.com/openshift/hypershift/support/releaseinfo" imageapi "github.com/openshift/api/image/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func TestGetRHELStreamForBootImage(t *testing.T) { @@ -18,6 +23,7 @@ func TestGetRHELStreamForBootImage(t *testing.T) { name string nodePool *hyperv1.NodePool releaseImage *releaseinfo.ReleaseImage + configs []client.Object expectedStream string expectErr bool }{ @@ -133,13 +139,121 @@ func TestGetRHELStreamForBootImage(t *testing.T) { }, expectErr: true, }, + { + name: "When ContainerRuntimeConfig sets runc and release is 5.0 with no explicit stream, it should return rhel-9", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-np", + Namespace: "clusters", + }, + Spec: hyperv1.NodePoolSpec{ + Config: []corev1.LocalObjectReference{ + {Name: "runc-config"}, + }, + }, + }, + configs: []client.Object{ + runcContainerRuntimeConfigMap("clusters", "runc-config"), + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + expectedStream: "rhel-9", + }, + { + name: "When ContainerRuntimeConfig sets runc and explicit rhel-10, it should return error", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-np", + Namespace: "clusters", + }, + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-10"}, + Config: []corev1.LocalObjectReference{ + {Name: "runc-config"}, + }, + }, + }, + configs: []client.Object{ + runcContainerRuntimeConfigMap("clusters", "runc-config"), + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + expectErr: true, + }, + { + name: "When ContainerRuntimeConfig sets crun and release is 5.0, it should return rhel-10", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-np", + Namespace: "clusters", + }, + Spec: hyperv1.NodePoolSpec{ + Config: []corev1.LocalObjectReference{ + {Name: "crun-config"}, + }, + }, + }, + configs: []client.Object{ + crunContainerRuntimeConfigMap("clusters", "crun-config"), + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + expectedStream: "rhel-10", + }, + { + name: "When ContainerRuntimeConfig sets runc and release is 4.x, it should return rhel-9", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-np", + Namespace: "clusters", + }, + Spec: hyperv1.NodePoolSpec{ + Config: []corev1.LocalObjectReference{ + {Name: "runc-config"}, + }, + }, + }, + configs: []client.Object{ + runcContainerRuntimeConfigMap("clusters", "runc-config"), + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "4.18.0"}}, + }, + expectedStream: "rhel-9", + }, + { + name: "When config ConfigMap does not exist, it should not detect runc", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-np", + Namespace: "clusters", + }, + Spec: hyperv1.NodePoolSpec{ + Config: []corev1.LocalObjectReference{ + {Name: "missing-config"}, + }, + }, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + expectedStream: "rhel-10", + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - stream, err := getRHELStreamForBootImage(tc.nodePool, tc.releaseImage) + objs := make([]client.Object, 0, len(tc.configs)) + objs = append(objs, tc.configs...) + + fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(objs...).Build() + + stream, err := getRHELStreamForBootImage(t.Context(), fakeClient, tc.nodePool, tc.releaseImage) if tc.expectErr { g.Expect(err).To(HaveOccurred()) return @@ -155,6 +269,7 @@ func TestValidateOSImageStream(t *testing.T) { name string nodePool *hyperv1.NodePool releaseImage *releaseinfo.ReleaseImage + configs []client.Object expectErr bool }{ { @@ -212,13 +327,61 @@ func TestValidateOSImageStream(t *testing.T) { }, expectErr: true, }, + { + name: "When osImageStream.Name is rhel-10 and ContainerRuntimeConfig sets runc, it should return an error", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-np", + Namespace: "clusters", + }, + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-10"}, + Config: []corev1.LocalObjectReference{ + {Name: "runc-config"}, + }, + }, + }, + configs: []client.Object{ + runcContainerRuntimeConfigMap("clusters", "runc-config"), + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + expectErr: true, + }, + { + name: "When osImageStream.Name is rhel-9 and ContainerRuntimeConfig sets runc, it should succeed", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-np", + Namespace: "clusters", + }, + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-9"}, + Config: []corev1.LocalObjectReference{ + {Name: "runc-config"}, + }, + }, + }, + configs: []client.Object{ + runcContainerRuntimeConfigMap("clusters", "runc-config"), + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - err := validateOSImageStream(tc.nodePool, tc.releaseImage) + objs := make([]client.Object, 0, len(tc.configs)) + objs = append(objs, tc.configs...) + + fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(objs...).Build() + + err := validateOSImageStream(t.Context(), fakeClient, tc.nodePool, tc.releaseImage) if tc.expectErr { g.Expect(err).To(HaveOccurred()) } else { @@ -227,3 +390,201 @@ func TestValidateOSImageStream(t *testing.T) { }) } } + +func TestUsesRuncRuntime(t *testing.T) { + testCases := []struct { + name string + nodePool *hyperv1.NodePool + configs []client.Object + expected bool + }{ + { + name: "When no configs, it should return false", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{Name: "np", Namespace: "ns"}, + }, + expected: false, + }, + { + name: "When ContainerRuntimeConfig sets runc, it should return true", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{Name: "np", Namespace: "ns"}, + Spec: hyperv1.NodePoolSpec{ + Config: []corev1.LocalObjectReference{{Name: "runc-cm"}}, + }, + }, + configs: []client.Object{ + runcContainerRuntimeConfigMap("ns", "runc-cm"), + }, + expected: true, + }, + { + name: "When ContainerRuntimeConfig sets crun, it should return false", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{Name: "np", Namespace: "ns"}, + Spec: hyperv1.NodePoolSpec{ + Config: []corev1.LocalObjectReference{{Name: "crun-cm"}}, + }, + }, + configs: []client.Object{ + crunContainerRuntimeConfigMap("ns", "crun-cm"), + }, + expected: false, + }, + { + name: "When ContainerRuntimeConfig has empty defaultRuntime, it should return false", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{Name: "np", Namespace: "ns"}, + Spec: hyperv1.NodePoolSpec{ + Config: []corev1.LocalObjectReference{{Name: "empty-cm"}}, + }, + }, + configs: []client.Object{ + emptyRuntimeContainerRuntimeConfigMap("ns", "empty-cm"), + }, + expected: false, + }, + { + name: "When ConfigMap contains a MachineConfig instead of ContainerRuntimeConfig, it should return false", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{Name: "np", Namespace: "ns"}, + Spec: hyperv1.NodePoolSpec{ + Config: []corev1.LocalObjectReference{{Name: "mc-cm"}}, + }, + }, + configs: []client.Object{ + machineConfigConfigMap("ns", "mc-cm"), + }, + expected: false, + }, + { + name: "When ConfigMap does not exist, it should return false", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{Name: "np", Namespace: "ns"}, + Spec: hyperv1.NodePoolSpec{ + Config: []corev1.LocalObjectReference{{Name: "missing"}}, + }, + }, + expected: false, + }, + { + name: "When multiple configs and one sets runc, it should return true", + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{Name: "np", Namespace: "ns"}, + Spec: hyperv1.NodePoolSpec{ + Config: []corev1.LocalObjectReference{ + {Name: "mc-cm"}, + {Name: "runc-cm"}, + }, + }, + }, + configs: []client.Object{ + machineConfigConfigMap("ns", "mc-cm"), + runcContainerRuntimeConfigMap("ns", "runc-cm"), + }, + expected: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + objs := make([]client.Object, 0, len(tc.configs)) + objs = append(objs, tc.configs...) + + fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(objs...).Build() + + result, err := usesRuncRuntime(t.Context(), fakeClient, tc.nodePool) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).To(Equal(tc.expected)) + }) + } +} + +// runcContainerRuntimeConfigMap returns a ConfigMap containing a +// ContainerRuntimeConfig with defaultRuntime set to "runc". +func runcContainerRuntimeConfigMap(namespace, name string) *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: map[string]string{ + TokenSecretConfigKey: `apiVersion: machineconfiguration.openshift.io/v1 +kind: ContainerRuntimeConfig +metadata: + name: set-runc +spec: + containerRuntimeConfig: + defaultRuntime: runc +`, + }, + } +} + +// crunContainerRuntimeConfigMap returns a ConfigMap containing a +// ContainerRuntimeConfig with defaultRuntime set to "crun". +func crunContainerRuntimeConfigMap(namespace, name string) *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: map[string]string{ + TokenSecretConfigKey: `apiVersion: machineconfiguration.openshift.io/v1 +kind: ContainerRuntimeConfig +metadata: + name: set-crun +spec: + containerRuntimeConfig: + defaultRuntime: crun +`, + }, + } +} + +// emptyRuntimeContainerRuntimeConfigMap returns a ConfigMap containing a +// ContainerRuntimeConfig with no defaultRuntime set (empty). +func emptyRuntimeContainerRuntimeConfigMap(namespace, name string) *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: map[string]string{ + TokenSecretConfigKey: `apiVersion: machineconfiguration.openshift.io/v1 +kind: ContainerRuntimeConfig +metadata: + name: no-runtime +spec: + containerRuntimeConfig: + pidsLimit: 2048 +`, + }, + } +} + +// machineConfigConfigMap returns a ConfigMap containing a MachineConfig +// (not a ContainerRuntimeConfig) to verify the scanner ignores non-CRC resources. +func machineConfigConfigMap(namespace, name string) *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: map[string]string{ + TokenSecretConfigKey: `apiVersion: machineconfiguration.openshift.io/v1 +kind: MachineConfig +metadata: + name: custom-mc + labels: + machineconfiguration.openshift.io/role: worker +spec: + config: + ignition: + version: 3.2.0 +`, + }, + } +} diff --git a/hypershift-operator/controllers/nodepool/powervs.go b/hypershift-operator/controllers/nodepool/powervs.go index af06b9254442..b71999f5ab5b 100644 --- a/hypershift-operator/controllers/nodepool/powervs.go +++ b/hypershift-operator/controllers/nodepool/powervs.go @@ -48,10 +48,10 @@ func getImageRegion(region string) string { } } -func ibmPowerVSMachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) (*capipowervs.IBMPowerVSMachineTemplateSpec, error) { +func ibmPowerVSMachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage, streamName string) (*capipowervs.IBMPowerVSMachineTemplateSpec, error) { // Validate PowerVS platform specific input var coreOSPowerVSImage *stream.SingleObject - coreOSPowerVSImage, _, err := getPowerVSImage(hcluster.Spec.Platform.PowerVS.Region, releaseImage) + coreOSPowerVSImage, _, err := getPowerVSImage(hcluster.Spec.Platform.PowerVS.Region, releaseImage, streamName) if err != nil { return nil, fmt.Errorf("couldn't discover a PowerVS Image for release image: %w", err) } @@ -90,7 +90,7 @@ func ibmPowerVSMachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hy } func (c *CAPI) ibmPowerVSMachineTemplate(templateNameGenerator func(spec any) (string, error)) (*capipowervs.IBMPowerVSMachineTemplate, error) { - spec, err := ibmPowerVSMachineTemplateSpec(c.hostedCluster, c.nodePool, c.releaseImage) + spec, err := ibmPowerVSMachineTemplateSpec(c.hostedCluster, c.nodePool, c.releaseImage, c.resolvedRHELStreamForBootImage) if err != nil { return nil, fmt.Errorf("failed to generate PowerVSMachineTemplateSpec: %w", err) } @@ -110,10 +110,12 @@ func (c *CAPI) ibmPowerVSMachineTemplate(templateNameGenerator func(spec any) (s return template, nil } -func getPowerVSImage(region string, releaseImage *releaseinfo.ReleaseImage) (*stream.SingleObject, string, error) { - // TODO(CNTRLPLANE-3553): use releaseImage.StreamForName(rhelStream) instead of - // accessing StreamMetadata directly, to support dual-stream payloads. - arch, foundArch := releaseImage.StreamMetadata.Architectures["ppc64le"] +func getPowerVSImage(region string, releaseImage *releaseinfo.ReleaseImage, streamName string) (*stream.SingleObject, string, error) { + streamMeta, err := releaseImage.StreamForName(streamName) + if err != nil { + return nil, "", fmt.Errorf("couldn't resolve stream metadata for stream %q: %w", streamName, err) + } + arch, foundArch := streamMeta.Architectures["ppc64le"] if !foundArch { return nil, "", fmt.Errorf("couldn't find OS metadata for architecture %q", "ppc64le") } @@ -163,8 +165,11 @@ 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) + // TODO(CNTRLPLANE-3553): hardcode to rhel-9 until the MCO can install + // rhel-10 OS images. Use getRHELStreamForBootImage once MCO support lands. + rhelStream := StreamRHEL9 var coreOSPowerVSImage *stream.SingleObject - coreOSPowerVSImage, powervsImageRegion, err := getPowerVSImage(hcluster.Spec.Platform.PowerVS.Region, releaseImage) + coreOSPowerVSImage, powervsImageRegion, err := getPowerVSImage(hcluster.Spec.Platform.PowerVS.Region, releaseImage, rhelStream) 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..b7459b8d05ab 100644 --- a/hypershift-operator/controllers/nodepool/powervs_test.go +++ b/hypershift-operator/controllers/nodepool/powervs_test.go @@ -12,10 +12,13 @@ import ( func TestGetPowerVSImage(t *testing.T) { testCases := []struct { - name string - region string - releaseImage *releaseinfo.ReleaseImage - expectedError string + name string + region string + streamName string + releaseImage *releaseinfo.ReleaseImage + expectedError string + expectedRelease string + expectedRegion string }{ { name: "When PowerVS images is nil, it should return error", @@ -41,14 +44,66 @@ func TestGetPowerVSImage(t *testing.T) { }, expectedError: "couldn't find OS metadata for architecture", }, + { + name: "When named stream is used with multi-stream ReleaseImage it should resolve from the named stream", + region: "us-south", + streamName: "rhel-9", + releaseImage: &releaseinfo.ReleaseImage{ + StreamMetadata: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "ppc64le": { + Images: stream.Images{ + PowerVS: &stream.ReplicatedObject{ + Regions: map[string]stream.SingleObject{ + "us-south": { + Release: "default-4.18.0-ppc64le", + Object: "default-object", + Bucket: "default-bucket", + }, + }, + }, + }, + }, + }, + }, + OSStreams: map[string]*stream.Stream{ + "rhel-9": { + Architectures: map[string]stream.Arch{ + "ppc64le": { + Images: stream.Images{ + PowerVS: &stream.ReplicatedObject{ + Regions: map[string]stream.SingleObject{ + "us-south": { + Release: "rhel9-4.18.0-ppc64le", + Object: "rhel9-object", + Bucket: "rhel9-bucket", + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedRelease: "rhel9-4-18-0-ppc64le", + expectedRegion: "us-south", + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - _, _, err := getPowerVSImage(tc.region, tc.releaseImage) - g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) + img, cosRegion, err := getPowerVSImage(tc.region, tc.releaseImage, tc.streamName) + if tc.expectedError != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) + return + } + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(img).ToNot(BeNil()) + g.Expect(img.Release).To(Equal(tc.expectedRelease)) + g.Expect(cosRegion).To(Equal(tc.expectedRegion)) }) } } diff --git a/hypershift-operator/controllers/nodepool/stream_test.go b/hypershift-operator/controllers/nodepool/stream_test.go index b6ebd2b6e45f..2293c7f0bc64 100644 --- a/hypershift-operator/controllers/nodepool/stream_test.go +++ b/hypershift-operator/controllers/nodepool/stream_test.go @@ -5,6 +5,9 @@ import ( . "github.com/onsi/gomega" + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/support/releaseinfo" + "github.com/blang/semver" ) @@ -169,3 +172,13 @@ func TestGetRHELStream(t *testing.T) { }) } } + +// TestStreamConstantsMatch ensures the API and releaseinfo stream constants +// stay in sync so the cross-reference comments don't silently diverge. +func TestStreamConstantsMatch(t *testing.T) { + g := NewWithT(t) + g.Expect(hyperv1.OSImageStreamRHEL9).To(Equal(releaseinfo.StreamRHEL9), + "API constant OSImageStreamRHEL9 must match releaseinfo.StreamRHEL9") + g.Expect(hyperv1.OSImageStreamRHEL10).To(Equal(releaseinfo.StreamRHEL10), + "API constant OSImageStreamRHEL10 must match releaseinfo.StreamRHEL10") +}