From 87134ff9fad4e93effd3217c2b9b951f88b573ee Mon Sep 17 00:00:00 2001 From: Salvatore Dario Minonne Date: Fri, 19 Jun 2026 08:15:48 +0200 Subject: [PATCH 1/2] feat(nodepool): integrate osImageStream into NodePool controller Wire the osImageStream API field into the NodePool controller: - Validate spec.osImageStream via GetRHELStream before config generation (fail fast on invalid stream/version/runc combinations). - Return StreamRHEL9 (not "") for implicit pre-5.0 releases so that downstream consumers like StreamForName() always receive a concrete stream name, avoiding errors when legacy StreamMetadata is removed. - Normalize rhelStream in rolloutConfig so that setting the default stream does not change the config hash (no spurious fleet-wide rollouts). - Keep resolvedRHELStream on ConfigGenerator (not rolloutConfig) for downstream consumers that need a concrete stream name (GCP, AWS AMI, token secret). - Add getRHELStream wrapper and validateOSImageStream in osstream.go, both delegating to GetRHELStream from stream.go. - Write os-stream key to the token secret for future ignition-server consumption. - Infer status.osImageStream from Machine NodeInfo.OSImage using majority consensus (rhcosStreamFromOSImage, osImageStreamFromMachines, setOSImageStreamStatus in version.go). - Pass resolved stream to defaultNodePoolAMI / setAWSConditions for consistent boot image resolution. Co-Authored-By: Claude Opus 4.6 --- .../controllers/nodepool/aws.go | 29 +- .../controllers/nodepool/aws_test.go | 3 +- .../controllers/nodepool/capi_test.go | 2 +- .../controllers/nodepool/conditions.go | 13 + .../controllers/nodepool/config.go | 49 ++- .../controllers/nodepool/config_test.go | 106 +++++- .../controllers/nodepool/gcp.go | 8 +- .../controllers/nodepool/gcp_test.go | 47 ++- .../nodepool/nodepool_controller.go | 17 +- .../controllers/nodepool/osstream.go | 43 +++ .../controllers/nodepool/osstream_test.go | 205 ++++++++++++ .../controllers/nodepool/stream.go | 5 +- .../controllers/nodepool/stream_test.go | 8 +- .../controllers/nodepool/token.go | 11 +- .../controllers/nodepool/token_test.go | 2 +- .../controllers/nodepool/version.go | 75 +++++ .../controllers/nodepool/version_test.go | 314 ++++++++++++++++++ 17 files changed, 893 insertions(+), 44 deletions(-) create mode 100644 hypershift-operator/controllers/nodepool/osstream.go create mode 100644 hypershift-operator/controllers/nodepool/osstream_test.go diff --git a/hypershift-operator/controllers/nodepool/aws.go b/hypershift-operator/controllers/nodepool/aws.go index f3cde2061fde..c47fbeb2c70f 100644 --- a/hypershift-operator/controllers/nodepool/aws.go +++ b/hypershift-operator/controllers/nodepool/aws.go @@ -60,8 +60,8 @@ func isSpotEnabled(nodePool *hyperv1.NodePool) bool { return false } -func awsMachineTemplateSpec(infraName string, hostedCluster *hyperv1.HostedCluster, nodePool *hyperv1.NodePool, defaultSG bool, releaseImage *releaseinfo.ReleaseImage) (*capiaws.AWSMachineTemplateSpec, error) { - ami, err := resolveAWSAMI(hostedCluster, nodePool, releaseImage) +func awsMachineTemplateSpec(infraName string, hostedCluster *hyperv1.HostedCluster, nodePool *hyperv1.NodePool, defaultSG bool, releaseImage *releaseinfo.ReleaseImage, rhelStream string) (*capiaws.AWSMachineTemplateSpec, error) { + ami, err := resolveAWSAMI(hostedCluster, nodePool, releaseImage, rhelStream) if err != nil { return nil, err } @@ -118,7 +118,7 @@ func awsMachineTemplateSpec(infraName string, hostedCluster *hyperv1.HostedClust return awsMachineTemplateSpec, nil } -func resolveAWSAMI(hostedCluster *hyperv1.HostedCluster, nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) (string, error) { +func resolveAWSAMI(hostedCluster *hyperv1.HostedCluster, nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage, rhelStream string) (string, error) { // TODO: Should the region be included in the NodePool platform information? region := hostedCluster.Spec.Platform.AWS.Region arch := nodePool.Spec.Arch @@ -134,8 +134,7 @@ 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) + ami, err := defaultNodePoolAMI(region, arch, rhelStream, releaseImage) if err != nil { return "", fmt.Errorf("couldn't discover an AMI for release image: %w", err) } @@ -271,7 +270,7 @@ func awsAdditionalTags(nodePool *hyperv1.NodePool, hostedCluster *hyperv1.Hosted } func (c *CAPI) awsMachineTemplate(ctx context.Context, templateNameGenerator func(spec any) (string, error)) (*capiaws.AWSMachineTemplate, error) { - desiredSpec, err := awsMachineTemplateSpec(c.capiClusterName, c.hostedCluster, c.nodePool, c.cpoCapabilities.CreateDefaultAWSSecurityGroup, c.releaseImage) + desiredSpec, err := awsMachineTemplateSpec(c.capiClusterName, c.hostedCluster, c.nodePool, c.cpoCapabilities.CreateDefaultAWSSecurityGroup, c.releaseImage, c.resolvedRHELStream) if err != nil { return nil, fmt.Errorf("failed to generate AWSMachineTemplateSpec: %w", err) } @@ -361,9 +360,21 @@ func (r *NodePoolReconciler) setAWSConditions(_ context.Context, nodePool *hyper ObservedGeneration: nodePool.Generation, }) } 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) + // Default behavior for Linux/RHCOS AMIs. + // Use the resolved stream (via GetRHELStream) rather than the raw spec field, + // so that the AMI lookup is consistent with the CAPI path. + rhelStream, err := getRHELStream(nodePool, releaseImage) + if err != nil { + SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ + Type: hyperv1.NodePoolValidPlatformImageType, + Status: corev1.ConditionFalse, + Reason: hyperv1.NodePoolValidationFailedReason, + Message: fmt.Sprintf("Couldn't resolve RHEL stream for release image %q: %s", nodePool.Spec.Release.Image, err.Error()), + ObservedGeneration: nodePool.Generation, + }) + return fmt.Errorf("couldn't resolve RHEL stream for release image: %w", err) + } + ami, err := defaultNodePoolAMI(hcluster.Spec.Platform.AWS.Region, nodePool.Spec.Arch, rhelStream, releaseImage) if err != nil { SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolValidPlatformImageType, diff --git a/hypershift-operator/controllers/nodepool/aws_test.go b/hypershift-operator/controllers/nodepool/aws_test.go index fc38aeaab01a..01f899097b8e 100644 --- a/hypershift-operator/controllers/nodepool/aws_test.go +++ b/hypershift-operator/controllers/nodepool/aws_test.go @@ -298,6 +298,7 @@ func TestAWSMachineTemplateSpec(t *testing.T) { }, true, releaseImage, + "", ) if tc.checkError != nil { tc.checkError(t, err) @@ -1340,7 +1341,7 @@ func TestResolveAWSAMI(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - ami, err := resolveAWSAMI(tc.hostedCluster, tc.nodePool, tc.releaseImage) + ami, err := resolveAWSAMI(tc.hostedCluster, tc.nodePool, tc.releaseImage, "") if tc.expectError { g.Expect(err).To(HaveOccurred()) } else { diff --git a/hypershift-operator/controllers/nodepool/capi_test.go b/hypershift-operator/controllers/nodepool/capi_test.go index 1fe35dc09f60..700f66b6887c 100644 --- a/hypershift-operator/controllers/nodepool/capi_test.go +++ b/hypershift-operator/controllers/nodepool/capi_test.go @@ -2648,7 +2648,7 @@ func TestReconcileMachineDeploymentStatus(t *testing.T) { expectedReadyConditionSet bool }{ { - name: "When MachineDeployment is complete, it should update nodePool version and annotations", + name: "When MachineDeployment is complete, it should update nodePool version", machineDeployment: &capiv1.MachineDeployment{ ObjectMeta: metav1.ObjectMeta{Generation: 1}, Spec: capiv1.MachineDeploymentSpec{ diff --git a/hypershift-operator/controllers/nodepool/conditions.go b/hypershift-operator/controllers/nodepool/conditions.go index cff009e3590e..3d368186ebdc 100644 --- a/hypershift-operator/controllers/nodepool/conditions.go +++ b/hypershift-operator/controllers/nodepool/conditions.go @@ -368,6 +368,18 @@ func (r *NodePoolReconciler) validMachineConfigCondition(ctx context.Context, no return &ctrl.Result{}, nil } + // Validate osImageStream before expensive config generation to fail fast. + if err := validateOSImageStream(nodePool, releaseImage); err != nil { + SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ + Type: hyperv1.NodePoolValidMachineConfigConditionType, + Status: corev1.ConditionFalse, + Reason: hyperv1.NodePoolValidationFailedReason, + Message: err.Error(), + ObservedGeneration: nodePool.Generation, + }) + return &ctrl.Result{}, fmt.Errorf("failed to validate osImageStream: %w", err) + } + haproxyRawConfig, err := r.generateHAProxyRawConfig(ctx, nodePool, hcluster, releaseImage) if err != nil { return &ctrl.Result{}, fmt.Errorf("failed to generate HAProxy raw config: %w", err) @@ -385,6 +397,7 @@ func (r *NodePoolReconciler) validMachineConfigCondition(ctx context.Context, no }) return &ctrl.Result{}, fmt.Errorf("failed to generate config: %w", err) } + SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolValidMachineConfigConditionType, Status: corev1.ConditionTrue, diff --git a/hypershift-operator/controllers/nodepool/config.go b/hypershift-operator/controllers/nodepool/config.go index 8771f45ea83c..2ee3187f31d4 100644 --- a/hypershift-operator/controllers/nodepool/config.go +++ b/hypershift-operator/controllers/nodepool/config.go @@ -31,6 +31,8 @@ import ( "k8s.io/apimachinery/pkg/util/yaml" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/blang/semver" ) // ConfigGenerator knows how to: @@ -42,6 +44,15 @@ type ConfigGenerator struct { hostedCluster *hyperv1.HostedCluster nodePool *hyperv1.NodePool controlplaneNamespace string + // resolvedRHELStream is the explicit RHEL stream name resolved via + // getRHELStream. Unlike rhelStream (which may be empty after normalization + // for hash stability), this always holds a concrete stream name (e.g. + // "rhel-9", "rhel-10"). It must be used by CAPI/Token paths that pass + // the stream to StreamForName, which requires an explicit value once + // the legacy DefaultStream is removed (OCP > 5.3). + // This field is intentionally outside rolloutConfig because it does not + // participate in the config hash that drives rollouts. + resolvedRHELStream string *rolloutConfig } @@ -60,6 +71,13 @@ type rolloutConfig struct { // TODO(alberto): consider let haproxyRawConfig be an implementation detail of ConfigGenerator. // For now, it's a required input to keep the haproxy business logic and files outside the scope of this initial refactor. haproxyRawConfig string + // rhelStream is the OS image stream name used for hash computation. + // It is set from spec.osImageStream.Name but normalized: if the explicit + // value matches the version-derived default it is kept empty so that + // setting the default stream does not change the hash. + // Only a non-default stream (e.g. "rhel-10" on a 4.x release) produces + // a non-empty value here and triggers a rollout. + rhelStream string } // NewConfigGenerator is the contract to create a new ConfigGenerator. @@ -77,16 +95,43 @@ func NewConfigGenerator(ctx context.Context, client client.Client, hostedCluster return nil, err } + // Resolve the explicit RHEL stream for image lookup and token plumbing. + resolvedRHELStream, err := getRHELStream(nodePool, releaseImage) + if err != nil { + return nil, fmt.Errorf("failed to resolve RHEL stream: %w", err) + } + + // Normalize rhelStream for the config hash: when the resolved stream + // matches the version-derived default, keep it empty so that setting the + // 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()) + 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) + if err != nil { + return nil, fmt.Errorf("failed to resolve default RHEL stream: %w", err) + } + if rhelStream == defaultStream { + rhelStream = "" + } + } + cg := &ConfigGenerator{ Client: client, hostedCluster: hostedCluster, nodePool: nodePool, controlplaneNamespace: controlPlaneNamespace, + resolvedRHELStream: resolvedRHELStream, rolloutConfig: &rolloutConfig{ releaseImage: releaseImage, pullSecretName: hostedCluster.Spec.PullSecret.Name, globalConfig: globalConfig, haproxyRawConfig: haproxyRawConfig, + rhelStream: rhelStream, }, } @@ -118,7 +163,7 @@ func (cg *ConfigGenerator) CompressedAndEncoded() (*bytes.Buffer, error) { // TODO(alberto): hash the struct directly instead of the string representation field by field. // This is kept like this for now to contain the scope of the refactor and avoid backward compatibility issues. func (cg *ConfigGenerator) Hash() string { - return supportutil.HashSimple(cg.mcoRawConfig + cg.releaseImage.Version() + cg.pullSecretName + cg.additionalTrustBundleName + cg.globalConfig) + return supportutil.HashSimple(cg.mcoRawConfig + cg.releaseImage.Version() + cg.pullSecretName + cg.additionalTrustBundleName + cg.globalConfig + cg.rhelStream) } // HashWithOutVersion is like Hash but doesn't compute the release version. @@ -126,7 +171,7 @@ func (cg *ConfigGenerator) Hash() string { // TODO(alberto): This was left inconsistent in https://github.com/openshift/hypershift/pull/3795/files. It should also contain cg.globalConfig. // This is kept like this for now to contain the scope of the refactor and avoid backward compatibility issues. func (cg *ConfigGenerator) HashWithoutVersion() string { - return supportutil.HashSimple(cg.mcoRawConfig + cg.pullSecretName + cg.additionalTrustBundleName) + return supportutil.HashSimple(cg.mcoRawConfig + cg.pullSecretName + cg.additionalTrustBundleName + cg.rhelStream) } func (cg *ConfigGenerator) Version() string { diff --git a/hypershift-operator/controllers/nodepool/config_test.go b/hypershift-operator/controllers/nodepool/config_test.go index ef9096ab218c..49b74c946d2d 100644 --- a/hypershift-operator/controllers/nodepool/config_test.go +++ b/hypershift-operator/controllers/nodepool/config_test.go @@ -152,13 +152,13 @@ spec: }{ { name: "When all input is given it should not return an error", - expectedHash: "e1d8d58e", + expectedHash: "83935368", expectedHashWithoutVersion: "0db5756d", nodePool: &hyperv1.NodePool{}, releaseImage: &releaseinfo.ReleaseImage{ ImageStream: &imageapi.ImageStream{ ObjectMeta: metav1.ObjectMeta{ - Name: "latest", + Name: "4.18.0", }, }, }, @@ -184,7 +184,7 @@ spec: }, { name: "When nodepool has configs it should populate mcoRawConfig ", - expectedHash: "801aff6a", + expectedHash: "af67f27c", expectedHashWithoutVersion: "fef02451", nodePool: &hyperv1.NodePool{ ObjectMeta: metav1.ObjectMeta{ @@ -213,7 +213,7 @@ spec: releaseImage: &releaseinfo.ReleaseImage{ ImageStream: &imageapi.ImageStream{ ObjectMeta: metav1.ObjectMeta{ - Name: "latest", + Name: "4.18.0", }, }, }, @@ -236,14 +236,76 @@ spec: }, }, expectedMCORawConfig: machineConfigDefaulted, - releaseImage: &releaseinfo.ReleaseImage{}, - hostedCluster: hostedCluster, - client: true, - error: fmt.Errorf("configmaps \"does-not-exist\" not found"), + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: "4.18.0", + }, + }, + }, + hostedCluster: hostedCluster, + client: true, + error: fmt.Errorf("configmaps \"does-not-exist\" not found"), + }, + { + name: "When release version is 4.18.0 with no osImageStream it should produce baseline hash", + expectedHash: "83935368", + expectedHashWithoutVersion: "0db5756d", + nodePool: &hyperv1.NodePool{}, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: "4.18.0", + }, + }, + }, + hostedCluster: hostedCluster, + client: true, + error: nil, + }, + { + name: "When osImageStream is set to version-derived default it should produce the same hash as no stream", + expectedHash: "83935368", + expectedHashWithoutVersion: "0db5756d", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-9"}, + }, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: "4.18.0", + }, + }, + }, + hostedCluster: hostedCluster, + client: true, + error: nil, + }, + { + name: "When osImageStream is set to non-default it should produce a different hash", + expectedHash: "ccd46cc1", + expectedHashWithoutVersion: "3a158178", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-9"}, + }, + }, + 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: "dc74976e", + expectedHash: "632801f8", expectedHashWithoutVersion: "71375893", nodePool: &hyperv1.NodePool{ ObjectMeta: metav1.ObjectMeta{ @@ -272,7 +334,7 @@ spec: releaseImage: &releaseinfo.ReleaseImage{ ImageStream: &imageapi.ImageStream{ ObjectMeta: metav1.ObjectMeta{ - Name: "latest", + Name: "4.18.0", }, }, }, @@ -434,6 +496,7 @@ func TestHash(t *testing.T) { pullSecretName string additionalTrustBundleName string globalConfig string + rhelStream string expected string }{ { @@ -490,6 +553,16 @@ func TestHash(t *testing.T) { globalConfig: "different", expected: "e916ddfe", }, + { + name: "When rhelStream is a non-default stream, it should change the hash", + mcoRawConfig: baseCaseMCORawConfig, + releaseVersion: baseCaseReleaseVersion, + pullSecretName: baseCasePullSecretName, + additionalTrustBundleName: baseCaseAdditionalTrustBundleName, + globalConfig: baseCaseGlobalConfig, + rhelStream: "rhel-10", + expected: "2dbbd41b", + }, } for _, tc := range testCases { @@ -508,6 +581,7 @@ func TestHash(t *testing.T) { pullSecretName: tc.pullSecretName, additionalTrustBundleName: tc.additionalTrustBundleName, globalConfig: tc.globalConfig, + rhelStream: tc.rhelStream, releaseImage: releaseImage, }, } @@ -536,6 +610,7 @@ func TestHashWithoutVersion(t *testing.T) { pullSecretName string additionalTrustBundleName string globalConfig string + rhelStream string expected string }{ { @@ -594,6 +669,16 @@ func TestHashWithoutVersion(t *testing.T) { globalConfig: "different", expected: baseCaseHash, }, + { + name: "When rhelStream is a non-default stream, it should change the hash", + mcoRawConfig: baseCaseMCORawConfig, + releaseVersion: baseCaseReleaseVersion, + pullSecretName: baseCasePullSecretName, + additionalTrustBundleName: baseCaseAdditionalTrustBundleName, + globalConfig: baseCaseGlobalConfig, + rhelStream: "rhel-10", + expected: "671fe083", + }, } for _, tc := range testCases { @@ -612,6 +697,7 @@ func TestHashWithoutVersion(t *testing.T) { pullSecretName: tc.pullSecretName, additionalTrustBundleName: tc.additionalTrustBundleName, globalConfig: tc.globalConfig, + rhelStream: tc.rhelStream, releaseImage: releaseImage, }, } diff --git a/hypershift-operator/controllers/nodepool/gcp.go b/hypershift-operator/controllers/nodepool/gcp.go index 50a107b210b4..81a0c23b77c7 100644 --- a/hypershift-operator/controllers/nodepool/gcp.go +++ b/hypershift-operator/controllers/nodepool/gcp.go @@ -37,6 +37,7 @@ func (c *CAPI) gcpMachineTemplate(_ context.Context, templateNameGenerator func( hc, nodePool, c.releaseImage, + c.resolvedRHELStream, ) if err != nil { return nil, fmt.Errorf("failed to generate GCP machine template spec: %w", err) @@ -81,12 +82,13 @@ func gcpMachineTemplateSpec( hostedCluster *hyperv1.HostedCluster, nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage, + rhelStream string, ) (*capigcp.GCPMachineSpec, error) { gcpPlatform := nodePool.Spec.Platform.GCP hcGCPPlatform := hostedCluster.Spec.Platform.GCP // Resolve image - image, err := resolveGCPImage(nodePool, releaseImage) + image, err := resolveGCPImage(nodePool, releaseImage, rhelStream) if err != nil { return nil, fmt.Errorf("failed to resolve GCP image: %w", err) } @@ -159,7 +161,7 @@ func gcpMachineTemplateSpec( } // resolveGCPImage determines the correct image to use based on NodePool configuration and release info. -func resolveGCPImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) (string, error) { +func resolveGCPImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage, rhelStream string) (string, error) { gcpPlatform := nodePool.Spec.Platform.GCP // If user specified a custom image, use it @@ -168,7 +170,7 @@ func resolveGCPImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.Relea } // Resolve image from release metadata - image, err := defaultNodePoolGCPImage(nodePool.Spec.Arch, releaseImage) + image, err := defaultNodePoolGCPImage(nodePool.Spec.Arch, releaseImage, rhelStream) 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..ef6747375c7a 100644 --- a/hypershift-operator/controllers/nodepool/gcp_test.go +++ b/hypershift-operator/controllers/nodepool/gcp_test.go @@ -514,6 +514,7 @@ func TestGcpMachineTemplateSpec(t *testing.T) { tc.hc, tc.nodePool, releaseImage, + "", ) if tc.expectedErr { @@ -535,6 +536,7 @@ func TestDefaultNodePoolGCPImage(t *testing.T) { testCases := []struct { name string arch string + rhelStream string releaseImage *releaseinfo.ReleaseImage expectedImage string expectedErr bool @@ -655,13 +657,56 @@ func TestDefaultNodePoolGCPImage(t *testing.T) { expectedErr: true, expectedErrMsg: "release image metadata has no GCP image for architecture \"amd64\"", }, + { + name: "When named stream is set, it should look up OSStreams map", + arch: hyperv1.ArchitectureAMD64, + rhelStream: "rhel-10", + 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-default-gcp-x86-64", + }, + }, + }, + }, + }, + OSStreams: map[string]*stream.Stream{ + "rhel-10": { + Architectures: map[string]stream.Arch{ + "x86_64": { + Images: stream.Images{ + Gcp: &stream.GcpImage{ + Project: "rhcos-cloud", + Name: "rhcos-10-0-20251023-0-gcp-x86-64", + }, + }, + }, + }, + }, + }, + }, + expectedImage: "projects/rhcos-cloud/global/images/rhcos-10-0-20251023-0-gcp-x86-64", + expectedErr: false, + }, + { + name: "When stream metadata is nil with empty stream name, it should return error", + arch: hyperv1.ArchitectureAMD64, + rhelStream: "", + releaseImage: &releaseinfo.ReleaseImage{}, + expectedErr: true, + expectedErrMsg: "couldn't resolve stream metadata", + }, } 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.releaseImage, tc.rhelStream) if tc.expectedErr { g.Expect(err).To(HaveOccurred()) diff --git a/hypershift-operator/controllers/nodepool/nodepool_controller.go b/hypershift-operator/controllers/nodepool/nodepool_controller.go index 2a170c1137de..876ba766a0af 100644 --- a/hypershift-operator/controllers/nodepool/nodepool_controller.go +++ b/hypershift-operator/controllers/nodepool/nodepool_controller.go @@ -283,6 +283,12 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho log.Error(err, "Failed to set NodesInfo status") } + // Infer the observed RHEL stream from Machine NodeInfo.OSImage and set + // status.osImageStream when a majority of machines report a consistent stream. + if err := r.setOSImageStreamStatus(ctx, nodePool); err != nil { + log.Error(err, "Failed to set OSImageStream status") + } + // Loop over all conditions. // Order matter as conditions might choose to short circuit returning ctrl.Result or error. signalConditions := []func(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster) (*ctrl.Result, error){ @@ -737,8 +743,6 @@ func isAutoscalingEnabled(nodePool *hyperv1.NodePool) bool { } // 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") @@ -766,15 +770,16 @@ func defaultNodePoolAMI(region string, specifiedArch string, streamName string, } // defaultNodePoolGCPImage returns the default GCP image for a given architecture from release metadata. -func defaultNodePoolGCPImage(specifiedArch string, releaseImage *releaseinfo.ReleaseImage) (string, error) { +func defaultNodePoolGCPImage(specifiedArch string, releaseImage *releaseinfo.ReleaseImage, rhelStream string) (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) + streamMeta, err := releaseImage.StreamForName(rhelStream) + if err != nil { + return "", fmt.Errorf("couldn't resolve stream metadata: %w", err) } - 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/osstream.go b/hypershift-operator/controllers/nodepool/osstream.go new file mode 100644 index 000000000000..2c48ea492a85 --- /dev/null +++ b/hypershift-operator/controllers/nodepool/osstream.go @@ -0,0 +1,43 @@ +package nodepool + +import ( + "fmt" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/support/releaseinfo" + + "github.com/blang/semver" +) + +// getRHELStream returns the effective RHEL stream for the NodePool. +// It delegates to GetRHELStream, which validates stream/version +// combinations and handles runc constraints. +// When spec.osImageStream.Name is unset, GetRHELStream returns the +// version-derived default (StreamRHEL9 for < 5.0, StreamRHEL10 for >= 5.0). +func getRHELStream(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) +} + +// 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 GetRHELStream for version-aware validation. +func validateOSImageStream(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) error { + name := nodePool.Spec.OSImageStream.Name + if name == "" { + return nil + } + 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. + _, err = GetRHELStream(name, version, false) + return err +} diff --git a/hypershift-operator/controllers/nodepool/osstream_test.go b/hypershift-operator/controllers/nodepool/osstream_test.go new file mode 100644 index 000000000000..dd042aaf8791 --- /dev/null +++ b/hypershift-operator/controllers/nodepool/osstream_test.go @@ -0,0 +1,205 @@ +package nodepool + +import ( + "testing" + + . "github.com/onsi/gomega" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/support/releaseinfo" + + imageapi "github.com/openshift/api/image/v1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func Test_getRHELStream(t *testing.T) { + testCases := []struct { + name string + nodePool *hyperv1.NodePool + releaseImage *releaseinfo.ReleaseImage + expectedStream string + expectErr bool + }{ + { + name: "When spec.osImageStream.Name is rhel-10 and version is 5.x, it should return rhel-10", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-10"}, + }, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + expectedStream: "rhel-10", + }, + { + name: "When spec.osImageStream.Name is rhel-9 and version is 4.x, it should return rhel-9", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-9"}, + }, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "4.18.0"}}, + }, + expectedStream: "rhel-9", + }, + { + name: "When spec.osImageStream.Name is rhel-10 and version is 4.x, it should return an error", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-10"}, + }, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "4.18.0"}}, + }, + expectErr: true, + }, + { + name: "When spec.osImageStream.Name is invalid, it should return an error", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-8"}, + }, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + expectErr: true, + }, + { + name: "When spec.osImageStream.Name is empty and version is 4.x, it should return rhel-9", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{}, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "4.18.0"}}, + }, + expectedStream: "rhel-9", + }, + { + name: "When spec.osImageStream.Name is empty and version is 5.x, it should return rhel-10", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{}, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + expectedStream: "rhel-10", + }, + { + name: "When spec.osImageStream.Name is empty and version is 6.x, it should return rhel-10", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{}, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "6.1.0"}}, + }, + expectedStream: "rhel-10", + }, + { + name: "When spec.osImageStream.Name is empty and version is unparsable, it should return an error", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{}, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "not-a-version"}}, + }, + expectErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + stream, err := getRHELStream(tc.nodePool, tc.releaseImage) + if tc.expectErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(stream).To(Equal(tc.expectedStream)) + }) + } +} + +func TestValidateOSImageStream(t *testing.T) { + testCases := []struct { + name string + nodePool *hyperv1.NodePool + releaseImage *releaseinfo.ReleaseImage + expectErr bool + }{ + { + name: "When osImageStream.Name is empty, it should succeed", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{}, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + }, + { + name: "When osImageStream.Name is rhel-9 and version is 4.x, it should succeed", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-9"}, + }, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "4.18.0"}}, + }, + }, + { + name: "When osImageStream.Name is rhel-10 and version is 5.x, it should succeed", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-10"}, + }, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + }, + { + name: "When osImageStream.Name is rhel-10 and version is 4.x, it should return an error", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-10"}, + }, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "4.18.0"}}, + }, + expectErr: true, + }, + { + name: "When osImageStream.Name is invalid, it should return an error", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + OSImageStream: hyperv1.OSImageStreamReference{Name: "rhel-8"}, + }, + }, + releaseImage: &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ObjectMeta: metav1.ObjectMeta{Name: "5.0.0"}}, + }, + expectErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + err := validateOSImageStream(tc.nodePool, tc.releaseImage) + if tc.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} diff --git a/hypershift-operator/controllers/nodepool/stream.go b/hypershift-operator/controllers/nodepool/stream.go index 83e73af564c4..f0524dddfec1 100644 --- a/hypershift-operator/controllers/nodepool/stream.go +++ b/hypershift-operator/controllers/nodepool/stream.go @@ -15,7 +15,8 @@ const ( // GetRHELStream resolves which RHEL CoreOS stream a NodePool should use. // Returns the resolved stream name, or an error for invalid combinations. -// An empty return means "use legacy single-stream behavior" (OCP 4.x). +// For OCP < 5.0 with no explicit stream it returns StreamRHEL9 (the only +// stream available on those releases). // Exported for use by integration tests and future Phase 2 consumers // (token secret plumbing, validMachineConfigCondition). func GetRHELStream(explicitStream string, releaseVersion semver.Version, usesRunc bool) (string, error) { @@ -39,7 +40,7 @@ func GetRHELStream(explicitStream string, releaseVersion semver.Version, usesRun } if !isOCP5Plus { - return "", nil + return StreamRHEL9, nil } if usesRunc { diff --git a/hypershift-operator/controllers/nodepool/stream_test.go b/hypershift-operator/controllers/nodepool/stream_test.go index 202057cabb73..b6ebd2b6e45f 100644 --- a/hypershift-operator/controllers/nodepool/stream_test.go +++ b/hypershift-operator/controllers/nodepool/stream_test.go @@ -19,17 +19,17 @@ func TestGetRHELStream(t *testing.T) { }{ // --- Implicit stream (explicitStream = "") --- { - name: "When no explicit stream and release is 4.x it should return empty string", + name: "When no explicit stream and release is 4.x it should return rhel-9", explicitStream: "", releaseVersion: semver.MustParse("4.18.0"), - expectResult: "", + expectResult: "rhel-9", }, { - name: "When no explicit stream and release is 4.x with runc it should return empty string", + name: "When no explicit stream and release is 4.x with runc it should return rhel-9", explicitStream: "", releaseVersion: semver.MustParse("4.19.0"), usesRunc: true, - expectResult: "", + expectResult: "rhel-9", }, { name: "When no explicit stream and release is 5.0 it should return rhel-10", diff --git a/hypershift-operator/controllers/nodepool/token.go b/hypershift-operator/controllers/nodepool/token.go index 4a39caffba7d..4d79681eb742 100644 --- a/hypershift-operator/controllers/nodepool/token.go +++ b/hypershift-operator/controllers/nodepool/token.go @@ -45,6 +45,7 @@ const ( TokenSecretAnnotation = "hypershift.openshift.io/ignition-config" TokenSecretIgnitionReachedAnnotation = "hypershift.openshift.io/ignition-reached" TokenSecretNodePoolUpgradeType = "hypershift.openshift.io/node-pool-upgrade-type" + TokenSecretOSStreamKey = "os-stream" ) // Token knows how to create an UUUID token for a unique configGenerator Hash. @@ -354,6 +355,9 @@ func (t *Token) reconcileTokenSecret(tokenSecret *corev1.Secret) error { tokenSecret.Data[TokenSecretPullSecretHashKey] = t.pullSecretHash tokenSecret.Data[TokenSecretAdditionalTrustBundleKey] = t.additionalTrustBundleHash tokenSecret.Data[TokenSecretHCConfigurationHashKey] = t.globalConfigHash + // TODO(CNTRLPLANE-3553): consumed by the ignition-server's TokenSecretReconciler once + // multi-stream ignition support lands. Until then this key is written but not read downstream. + tokenSecret.Data[TokenSecretOSStreamKey] = []byte(t.resolvedRHELStream) } // TODO (alberto): Only apply this on creation and change the hash generation to only use triggering upgrade fields. // We let this change to happen inplace now as the tokenSecret and the mcs config use the whole spec.Config for the comparing hash. @@ -381,7 +385,7 @@ func (t *Token) reconcileUserDataSecret(log logr.Logger, userDataSecret *corev1. if karpenterutil.IsKarpenterEnabled(t.hostedCluster.Spec.AutoNode) { npLabels := t.nodePool.GetLabels() if npLabels != nil && npLabels[karpenterutil.ManagedByKarpenterLabel] == "true" { - err := setKarpenterAMILabels(log, userDataSecret, t.hostedCluster.Spec.Platform.AWS.Region, t.releaseImage, t.hostedCluster.Spec.Platform.Type) + err := setKarpenterAMILabels(log, userDataSecret, t.hostedCluster.Spec.Platform.AWS.Region, t.releaseImage, t.hostedCluster.Spec.Platform.Type, t.resolvedRHELStream) if err != nil { return err } @@ -403,15 +407,14 @@ func (t *Token) reconcileUserDataSecret(log logr.Logger, userDataSecret *corev1. return nil } -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 +func setKarpenterAMILabels(log logr.Logger, userDataSecret *corev1.Secret, region string, releaseImage *releaseinfo.ReleaseImage, platform hyperv1.PlatformType, rhelStream string) error { 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, rhelStream, releaseImage) if err != nil { // skip unavailable architectures gracefully log.Error(err, "failed to get default NodePool AMI for architecture", "architecture", arch) diff --git a/hypershift-operator/controllers/nodepool/token_test.go b/hypershift-operator/controllers/nodepool/token_test.go index 5266d08331cb..6a5d9e8cc5d5 100644 --- a/hypershift-operator/controllers/nodepool/token_test.go +++ b/hypershift-operator/controllers/nodepool/token_test.go @@ -1181,7 +1181,7 @@ func TestSetKarpenterAMILabels(t *testing.T) { if ri == nil { ri = testutils.InitReleaseImageOrDie("test-release") } - err := setKarpenterAMILabels(log, tc.userDataSecret, tc.region, ri, tc.platform) + err := setKarpenterAMILabels(log, tc.userDataSecret, tc.region, ri, tc.platform, "") if tc.expectedError != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(Equal(tc.expectedError)) diff --git a/hypershift-operator/controllers/nodepool/version.go b/hypershift-operator/controllers/nodepool/version.go index d63ca3417daa..64582c581053 100644 --- a/hypershift-operator/controllers/nodepool/version.go +++ b/hypershift-operator/controllers/nodepool/version.go @@ -3,6 +3,7 @@ package nodepool import ( "context" "fmt" + "regexp" "sort" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" @@ -100,3 +101,77 @@ func (r *NodePoolReconciler) setNodesInfoStatus(ctx context.Context, nodePool *h return nil } + +// rhcosOSImageRe matches the RHCOS version from the NodeInfo.OSImage string. +// The first capture group is the leading digit of the RHCOS version (e.g. "4" +// in "419.97…"), which determines the RHEL generation (4xx → RHEL 9, 5xx → RHEL 10). +var rhcosOSImageRe = regexp.MustCompile(`Red Hat Enterprise Linux CoreOS (\d)\d{2}\.`) + +// rhcosStreamFromOSImage parses a Machine's NodeInfo.OSImage string and +// returns the corresponding RHEL stream name. RHCOS versions starting with +// 4xx map to RHEL 9 and 5xx to RHEL 10. +// Returns empty string if the OS image string is unrecognized. +func rhcosStreamFromOSImage(osImage string) string { + matches := rhcosOSImageRe.FindStringSubmatch(osImage) + if len(matches) < 2 { + return "" + } + switch matches[1] { + case "4": + return StreamRHEL9 + case "5": + return StreamRHEL10 + default: + return "" + } +} + +// osImageStreamFromMachines determines the observed RHEL stream by examining +// Machine NodeInfo.OSImage across the pool. Returns the stream name when a +// majority of observed machines report the same stream, or empty string when +// no majority exists or no machines have reported yet. +func osImageStreamFromMachines(machines []*capiv1.Machine) string { + streamCounts := make(map[string]int) + total := 0 + for _, machine := range machines { + if machine.Status.NodeInfo == nil { + continue + } + stream := rhcosStreamFromOSImage(machine.Status.NodeInfo.OSImage) + if stream == "" { + continue + } + streamCounts[stream]++ + total++ + } + + if total == 0 { + return "" + } + + // Set status when a strict majority (> N/2) of observed nodes agree. + for stream, count := range streamCounts { + if count > total/2 { + return stream + } + } + + return "" +} + +// setOSImageStreamStatus infers the RHEL stream from observed Machine +// NodeInfo.OSImage and sets nodePool.Status.OSImageStream when a majority +// of machines report a consistent stream. +func (r *NodePoolReconciler) setOSImageStreamStatus(ctx context.Context, nodePool *hyperv1.NodePool) error { + machines, err := r.getMachinesForNodePool(ctx, nodePool) + if err != nil { + return fmt.Errorf("failed to get Machines for OSImageStream status: %w", err) + } + + stream := osImageStreamFromMachines(machines) + if stream != "" { + nodePool.Status.OSImageStream = hyperv1.OSImageStreamReference{Name: stream} + } + + return nil +} diff --git a/hypershift-operator/controllers/nodepool/version_test.go b/hypershift-operator/controllers/nodepool/version_test.go index 581ad975c5ab..eeec0c5c06a6 100644 --- a/hypershift-operator/controllers/nodepool/version_test.go +++ b/hypershift-operator/controllers/nodepool/version_test.go @@ -303,6 +303,320 @@ func TestSetNodesInfoStatus(t *testing.T) { } } +func TestRhcosStreamFromOSImage(t *testing.T) { + testCases := []struct { + name string + osImage string + expected string + }{ + { + name: "When OSImage is RHCOS 4xx it should return rhel-9", + osImage: "Red Hat Enterprise Linux CoreOS 419.97.202505081234-0 (Plow)", + expected: StreamRHEL9, + }, + { + name: "When OSImage is RHCOS 5xx it should return rhel-10", + osImage: "Red Hat Enterprise Linux CoreOS 510.97.202506011200-0 (Plow)", + expected: StreamRHEL10, + }, + { + name: "When OSImage has different 4xx version it should return rhel-9", + osImage: "Red Hat Enterprise Linux CoreOS 418.94.202501011200-0 (Plow)", + expected: StreamRHEL9, + }, + { + name: "When OSImage is empty it should return empty string", + osImage: "", + expected: "", + }, + { + name: "When OSImage is unrecognized it should return empty string", + osImage: "Ubuntu 22.04 LTS", + expected: "", + }, + { + name: "When OSImage has unknown major version it should return empty string", + osImage: "Red Hat Enterprise Linux CoreOS 300.97.202505081234-0 (Plow)", + expected: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + g.Expect(rhcosStreamFromOSImage(tc.osImage)).To(Equal(tc.expected)) + }) + } +} + +func TestOsImageStreamFromMachines(t *testing.T) { + testCases := []struct { + name string + machines []*v1beta1.Machine + expected string + }{ + { + name: "When there are no machines it should return empty string", + machines: nil, + expected: "", + }, + { + name: "When a single machine reports RHEL 9 it should return rhel-9", + machines: []*v1beta1.Machine{ + machineWithOSImage("m1", "Red Hat Enterprise Linux CoreOS 419.97.202505081234-0 (Plow)"), + }, + expected: StreamRHEL9, + }, + { + name: "When all machines report RHEL 9 it should return rhel-9", + machines: []*v1beta1.Machine{ + machineWithOSImage("m1", "Red Hat Enterprise Linux CoreOS 419.97.202505081234-0 (Plow)"), + machineWithOSImage("m2", "Red Hat Enterprise Linux CoreOS 419.97.202505081234-0 (Plow)"), + machineWithOSImage("m3", "Red Hat Enterprise Linux CoreOS 419.97.202505081234-0 (Plow)"), + }, + expected: StreamRHEL9, + }, + { + name: "When all machines report RHEL 10 it should return rhel-10", + machines: []*v1beta1.Machine{ + machineWithOSImage("m1", "Red Hat Enterprise Linux CoreOS 510.97.202506011200-0 (Plow)"), + machineWithOSImage("m2", "Red Hat Enterprise Linux CoreOS 510.97.202506011200-0 (Plow)"), + }, + expected: StreamRHEL10, + }, + { + name: "When a majority reports RHEL 10 during rolling upgrade it should return rhel-10", + machines: []*v1beta1.Machine{ + machineWithOSImage("m1", "Red Hat Enterprise Linux CoreOS 419.97.202505081234-0 (Plow)"), + machineWithOSImage("m2", "Red Hat Enterprise Linux CoreOS 510.97.202506011200-0 (Plow)"), + machineWithOSImage("m3", "Red Hat Enterprise Linux CoreOS 510.97.202506011200-0 (Plow)"), + }, + expected: StreamRHEL10, + }, + { + name: "When streams are evenly split it should return empty string", + machines: []*v1beta1.Machine{ + machineWithOSImage("m1", "Red Hat Enterprise Linux CoreOS 419.97.202505081234-0 (Plow)"), + machineWithOSImage("m2", "Red Hat Enterprise Linux CoreOS 510.97.202506011200-0 (Plow)"), + }, + expected: "", + }, + { + name: "When machines have no NodeInfo it should return empty string", + machines: []*v1beta1.Machine{ + {ObjectMeta: metav1.ObjectMeta{Name: "m1"}, Status: v1beta1.MachineStatus{}}, + }, + expected: "", + }, + { + name: "When machines have unrecognized OSImage it should return empty string", + machines: []*v1beta1.Machine{ + machineWithOSImage("m1", "Ubuntu 22.04 LTS"), + }, + expected: "", + }, + { + name: "When some machines have no NodeInfo it should count only those with NodeInfo", + machines: []*v1beta1.Machine{ + machineWithOSImage("m1", "Red Hat Enterprise Linux CoreOS 510.97.202506011200-0 (Plow)"), + {ObjectMeta: metav1.ObjectMeta{Name: "m2"}, Status: v1beta1.MachineStatus{}}, + {ObjectMeta: metav1.ObjectMeta{Name: "m3"}, Status: v1beta1.MachineStatus{}}, + }, + expected: StreamRHEL10, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + g.Expect(osImageStreamFromMachines(tc.machines)).To(Equal(tc.expected)) + }) + } +} + +func TestSetOSImageStreamStatus(t *testing.T) { + testCases := []struct { + name string + machines []client.Object + nodePool *hyperv1.NodePool + expectedOSImageStream hyperv1.OSImageStreamReference + }{ + { + name: "When no machines exist it should not change OSImageStream status", + machines: nil, + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-nodepool", + Namespace: "clusters", + }, + Spec: hyperv1.NodePoolSpec{ + ClusterName: "test-cluster", + }, + }, + expectedOSImageStream: hyperv1.OSImageStreamReference{}, + }, + { + name: "When all machines report RHEL 9 it should set status to rhel-9", + machines: []client.Object{ + &v1beta1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "m1", + Namespace: "clusters-test-cluster", + Annotations: map[string]string{ + nodePoolAnnotation: "clusters/test-nodepool", + }, + }, + Status: v1beta1.MachineStatus{ + NodeInfo: &corev1.NodeSystemInfo{ + OSImage: "Red Hat Enterprise Linux CoreOS 419.97.202505081234-0 (Plow)", + }, + }, + }, + &v1beta1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "m2", + Namespace: "clusters-test-cluster", + Annotations: map[string]string{ + nodePoolAnnotation: "clusters/test-nodepool", + }, + }, + Status: v1beta1.MachineStatus{ + NodeInfo: &corev1.NodeSystemInfo{ + OSImage: "Red Hat Enterprise Linux CoreOS 419.97.202505081234-0 (Plow)", + }, + }, + }, + }, + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-nodepool", + Namespace: "clusters", + }, + Spec: hyperv1.NodePoolSpec{ + ClusterName: "test-cluster", + }, + }, + expectedOSImageStream: hyperv1.OSImageStreamReference{Name: StreamRHEL9}, + }, + { + name: "When majority reports RHEL 10 it should set status to rhel-10", + machines: []client.Object{ + &v1beta1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "m1", + Namespace: "clusters-test-cluster", + Annotations: map[string]string{ + nodePoolAnnotation: "clusters/test-nodepool", + }, + }, + Status: v1beta1.MachineStatus{ + NodeInfo: &corev1.NodeSystemInfo{ + OSImage: "Red Hat Enterprise Linux CoreOS 419.97.202505081234-0 (Plow)", + }, + }, + }, + &v1beta1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "m2", + Namespace: "clusters-test-cluster", + Annotations: map[string]string{ + nodePoolAnnotation: "clusters/test-nodepool", + }, + }, + Status: v1beta1.MachineStatus{ + NodeInfo: &corev1.NodeSystemInfo{ + OSImage: "Red Hat Enterprise Linux CoreOS 510.97.202506011200-0 (Plow)", + }, + }, + }, + &v1beta1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "m3", + Namespace: "clusters-test-cluster", + Annotations: map[string]string{ + nodePoolAnnotation: "clusters/test-nodepool", + }, + }, + Status: v1beta1.MachineStatus{ + NodeInfo: &corev1.NodeSystemInfo{ + OSImage: "Red Hat Enterprise Linux CoreOS 510.97.202506011200-0 (Plow)", + }, + }, + }, + }, + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-nodepool", + Namespace: "clusters", + }, + Spec: hyperv1.NodePoolSpec{ + ClusterName: "test-cluster", + }, + }, + expectedOSImageStream: hyperv1.OSImageStreamReference{Name: StreamRHEL10}, + }, + { + name: "When previous status exists and no machines have NodeInfo it should preserve previous status", + machines: []client.Object{ + &v1beta1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "m1", + Namespace: "clusters-test-cluster", + Annotations: map[string]string{ + nodePoolAnnotation: "clusters/test-nodepool", + }, + }, + Status: v1beta1.MachineStatus{}, + }, + }, + nodePool: &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-nodepool", + Namespace: "clusters", + }, + Spec: hyperv1.NodePoolSpec{ + ClusterName: "test-cluster", + }, + Status: hyperv1.NodePoolStatus{ + OSImageStream: hyperv1.OSImageStreamReference{Name: StreamRHEL9}, + }, + }, + expectedOSImageStream: hyperv1.OSImageStreamReference{Name: StreamRHEL9}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + objs := make([]client.Object, 0, len(tc.machines)) + objs = append(objs, tc.machines...) + + fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(objs...).Build() + r := &NodePoolReconciler{ + Client: fakeClient, + } + + err := r.setOSImageStreamStatus(t.Context(), tc.nodePool) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(tc.nodePool.Status.OSImageStream).To(Equal(tc.expectedOSImageStream)) + }) + } +} + +func machineWithOSImage(name, osImage string) *v1beta1.Machine { + return &v1beta1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Status: v1beta1.MachineStatus{ + NodeInfo: &corev1.NodeSystemInfo{ + OSImage: osImage, + }, + }, + } +} + func machineWithVersionAndHealth(name, kubeletVersion string, healthy bool, annotations map[string]string) *v1beta1.Machine { healthStatus := corev1.ConditionTrue if !healthy { From 6a62ea3d3b4a95c904e9848443543dd53c0fca48 Mon Sep 17 00:00:00 2001 From: Salvatore Dario Minonne Date: Thu, 25 Jun 2026 15:33:51 +0200 Subject: [PATCH 2/2] feat(nodepool): wire usesRunc detection into RHEL stream resolution Scan NodePool spec.config ConfigMaps for ContainerRuntimeConfig resources with defaultRuntime set to "runc". When detected: - Implicit stream on OCP >= 5.0 falls back to rhel-9 (instead of rhel-10) - Explicit rhel-10 + runc returns a validation error This replaces the hardcoded usesRunc=false TODO(CNTRLPLANE-3553) in getRHELStream, validateOSImageStream, and the config hash normalization in NewConfigGenerator. JIRA: CNTRLPLANE-3553 Co-Authored-By: Claude Opus 4.6 --- .../controllers/nodepool/aws.go | 4 +- .../controllers/nodepool/conditions.go | 2 +- .../controllers/nodepool/config.go | 9 +- .../controllers/nodepool/osstream.go | 89 ++++- .../controllers/nodepool/osstream_test.go | 365 +++++++++++++++++- 5 files changed, 453 insertions(+), 16 deletions(-) diff --git a/hypershift-operator/controllers/nodepool/aws.go b/hypershift-operator/controllers/nodepool/aws.go index c47fbeb2c70f..a231c2e77851 100644 --- a/hypershift-operator/controllers/nodepool/aws.go +++ b/hypershift-operator/controllers/nodepool/aws.go @@ -331,7 +331,7 @@ func (c *CAPI) reconcileAWSMachines(ctx context.Context) error { return errors.NewAggregate(errs) } -func (r *NodePoolReconciler) setAWSConditions(_ context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, _ string, releaseImage *releaseinfo.ReleaseImage) error { +func (r *NodePoolReconciler) setAWSConditions(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, _ string, releaseImage *releaseinfo.ReleaseImage) error { if nodePool.Spec.Platform.Type == hyperv1.AWSPlatform { if hcluster.Spec.Platform.AWS == nil { return fmt.Errorf("the HostedCluster for this NodePool has no .Spec.Platform.AWS, this is unsupported") @@ -363,7 +363,7 @@ func (r *NodePoolReconciler) setAWSConditions(_ context.Context, nodePool *hyper // Default behavior for Linux/RHCOS AMIs. // Use the resolved stream (via GetRHELStream) rather than the raw spec field, // so that the AMI lookup is consistent with the CAPI path. - rhelStream, err := getRHELStream(nodePool, releaseImage) + rhelStream, err := getRHELStream(ctx, r.Client, nodePool, releaseImage) if err != nil { SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolValidPlatformImageType, diff --git a/hypershift-operator/controllers/nodepool/conditions.go b/hypershift-operator/controllers/nodepool/conditions.go index 3d368186ebdc..0f36bb37771f 100644 --- a/hypershift-operator/controllers/nodepool/conditions.go +++ b/hypershift-operator/controllers/nodepool/conditions.go @@ -369,7 +369,7 @@ func (r *NodePoolReconciler) validMachineConfigCondition(ctx context.Context, no } // Validate osImageStream before expensive config generation to fail fast. - 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 2ee3187f31d4..b719425d7337 100644 --- a/hypershift-operator/controllers/nodepool/config.go +++ b/hypershift-operator/controllers/nodepool/config.go @@ -96,7 +96,7 @@ func NewConfigGenerator(ctx context.Context, client client.Client, hostedCluster } // Resolve the explicit RHEL stream for image lookup and token plumbing. - resolvedRHELStream, err := getRHELStream(nodePool, releaseImage) + resolvedRHELStream, err := getRHELStream(ctx, client, nodePool, releaseImage) if err != nil { return nil, fmt.Errorf("failed to resolve RHEL stream: %w", err) } @@ -110,8 +110,11 @@ func NewConfigGenerator(ctx context.Context, client client.Client, hostedCluster 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/osstream.go b/hypershift-operator/controllers/nodepool/osstream.go index 2c48ea492a85..cf7fc6f71fc2 100644 --- a/hypershift-operator/controllers/nodepool/osstream.go +++ b/hypershift-operator/controllers/nodepool/osstream.go @@ -1,33 +1,102 @@ package nodepool import ( + "bufio" + "context" + coreerrors "errors" "fmt" + "io" + "strings" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/support/releaseinfo" + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/util/yaml" + "github.com/blang/semver" + + "sigs.k8s.io/controller-runtime/pkg/client" ) +// 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 + } + + scheme := runtime.NewScheme() + _ = mcfgv1.Install(scheme) + decoder := runtimeserializer.NewCodecFactory(scheme).UniversalDeserializer() + + 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 the ConfigMap doesn't exist, skip — validation catches this elsewhere. + continue + } + 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) { + break + } + if len(strings.TrimSpace(string(raw))) > 0 { + obj, _, decodeErr := decoder.Decode(raw, nil, nil) + 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 +} + // getRHELStream returns the effective RHEL stream for the NodePool. // It delegates to GetRHELStream, which validates stream/version // combinations and handles runc constraints. // When spec.osImageStream.Name is unset, GetRHELStream returns the // version-derived default (StreamRHEL9 for < 5.0, StreamRHEL10 for >= 5.0). -func getRHELStream(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage) (string, error) { +func getRHELStream(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 GetRHELStream 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 { name := nodePool.Spec.OSImageStream.Name if name == "" { return nil @@ -37,7 +106,11 @@ func validateOSImageStream(nodePool *hyperv1.NodePool, releaseImage *releaseinfo 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. - _, err = GetRHELStream(name, version, false) + usesRunc, err := usesRuncRuntime(ctx, c, nodePool) + if err != nil { + return fmt.Errorf("failed to detect container runtime: %w", err) + } + + _, err = GetRHELStream(name, version, usesRunc) return err } diff --git a/hypershift-operator/controllers/nodepool/osstream_test.go b/hypershift-operator/controllers/nodepool/osstream_test.go index dd042aaf8791..6d844d549c93 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 Test_getRHELStream(t *testing.T) { @@ -18,6 +23,7 @@ func Test_getRHELStream(t *testing.T) { name string nodePool *hyperv1.NodePool releaseImage *releaseinfo.ReleaseImage + configs []client.Object expectedStream string expectErr bool }{ @@ -109,13 +115,121 @@ func Test_getRHELStream(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 := getRHELStream(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 := getRHELStream(t.Context(), fakeClient, tc.nodePool, tc.releaseImage) if tc.expectErr { g.Expect(err).To(HaveOccurred()) return @@ -131,6 +245,7 @@ func TestValidateOSImageStream(t *testing.T) { name string nodePool *hyperv1.NodePool releaseImage *releaseinfo.ReleaseImage + configs []client.Object expectErr bool }{ { @@ -188,13 +303,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 { @@ -203,3 +366,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 +`, + }, + } +}