Skip to content

Commit 5c532dc

Browse files
sdminonneclaude
sdminonne
and
claude
committed
fix(nodepool): address PR #8834 review feedback
- Hardcode StreamRHEL9 in setKubevirtConditions, setOpenStackConditions, and setPowerVSconditions to match the AWS pattern, keeping condition- setter and CAPI machine-template paths consistent until MCO rhel-10 support lands. - Add init() assertion and TestStreamConstantsMatch to enforce that API constants (hyperv1.OSImageStreamRHEL9/10) match releaseinfo constants (releaseinfo.StreamRHEL9/10). - Add cross-reference comment on rolloutConfig.rhelStream pointing to ConfigGenerator.resolvedRHELStreamForBootImage. - Add named-stream test cases for Azure, KubeVirt, and OpenStack that exercise the OSStreams/StreamForName path with a concrete stream name. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 034768b commit 5c532dc

9 files changed

Lines changed: 172 additions & 17 deletions

File tree

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

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,7 @@ func TestDefaultAzureNodePoolImage(t *testing.T) {
969969
name string
970970
nodePool *hyperv1.NodePool
971971
releaseImage *releaseinfo.ReleaseImage
972+
streamName string
972973
expectedImageType hyperv1.AzureVMImageType
973974
expectedMarketplaceImage *hyperv1.AzureMarketplaceImage
974975
expectedError bool
@@ -1271,13 +1272,67 @@ func TestDefaultAzureNodePoolImage(t *testing.T) {
12711272
ImageGeneration: ptr.To(hyperv1.Gen1),
12721273
},
12731274
},
1275+
{
1276+
name: "When named stream is used with multi-stream ReleaseImage it should resolve marketplace from the named stream",
1277+
nodePool: &hyperv1.NodePool{
1278+
Spec: hyperv1.NodePoolSpec{
1279+
Arch: hyperv1.ArchitectureAMD64,
1280+
Platform: hyperv1.NodePoolPlatform{
1281+
Type: hyperv1.AzurePlatform,
1282+
Azure: &hyperv1.AzureNodePoolPlatform{
1283+
Image: hyperv1.AzureVMImage{},
1284+
},
1285+
},
1286+
},
1287+
},
1288+
releaseImage: &releaseinfo.ReleaseImage{
1289+
ImageStream: &imageapi.ImageStream{
1290+
ObjectMeta: metav1.ObjectMeta{Name: "4.20.0"},
1291+
},
1292+
OSStreams: map[string]*stream.Stream{
1293+
"rhel-9": {
1294+
Architectures: map[string]stream.Arch{
1295+
"x86_64": {
1296+
RHELCoreOSExtensions: &rhcos.Extensions{
1297+
AzureDisk: &rhcos.AzureDisk{
1298+
Release: "9.6.20250701-0",
1299+
URL: "https://rhcos.blob.core.windows.net/imagebucket/rhcos-9.6.20250701-0-azure.x86_64.vhd",
1300+
},
1301+
Marketplace: &rhcos.Marketplace{
1302+
Azure: &rhcos.AzureMarketplace{
1303+
NoPurchasePlan: &rhcos.AzureMarketplaceImages{
1304+
Gen2: &rhcos.AzureMarketplaceImage{
1305+
Publisher: "azureopenshift",
1306+
Offer: "aro4",
1307+
SKU: "aro_rhel9_420-v2",
1308+
Version: "420.9.20250701",
1309+
},
1310+
},
1311+
},
1312+
},
1313+
},
1314+
},
1315+
},
1316+
},
1317+
},
1318+
},
1319+
streamName: "rhel-9",
1320+
expectedImageType: hyperv1.AzureMarketplace,
1321+
expectedMarketplaceImage: &hyperv1.AzureMarketplaceImage{
1322+
Publisher: "azureopenshift",
1323+
Offer: "aro4",
1324+
SKU: "aro_rhel9_420-v2",
1325+
Version: "420.9.20250701",
1326+
ImageGeneration: ptr.To(hyperv1.Gen2),
1327+
},
1328+
},
12741329
}
12751330

12761331
for _, tc := range testCases {
12771332
t.Run(tc.name, func(t *testing.T) {
12781333
g := NewGomegaWithT(t)
12791334

1280-
err := defaultAzureNodePoolImage(tc.nodePool, tc.releaseImage, "")
1335+
err := defaultAzureNodePoolImage(tc.nodePool, tc.releaseImage, tc.streamName)
12811336

12821337
if tc.expectedError {
12831338
g.Expect(err).To(HaveOccurred())

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ type rolloutConfig struct {
7777
// setting the default stream does not change the hash.
7878
// Only a non-default stream (e.g. "rhel-9" on a ≥5.0 release) produces
7979
// a non-empty value here and triggers a rollout.
80+
//
81+
// See also ConfigGenerator.resolvedRHELStreamForBootImage, which controls
82+
// boot image resolution and is intentionally separate from the hash.
8083
rhelStream string
8184
}
8285

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,9 @@ func (r *NodePoolReconciler) setKubevirtConditions(ctx context.Context, nodePool
6565

6666
nodePool.Status.Platform.KubeVirt.Credentials = hcluster.Spec.Platform.Kubevirt.Credentials.DeepCopy()
6767
}
68-
rhelStream, err := getRHELStreamForBootImage(nodePool, releaseImage)
69-
if err != nil {
70-
return fmt.Errorf("failed to resolve RHEL stream: %w", err)
71-
}
72-
kubevirtBootImage, err := kubevirt.GetImage(nodePool, releaseImage, infraNS, rhelStream)
68+
// TODO(CNTRLPLANE-3553): hardcode to rhel-9 until the MCO can install
69+
// rhel-10 OS images. Use getRHELStreamForBootImage once MCO support lands.
70+
kubevirtBootImage, err := kubevirt.GetImage(nodePool, releaseImage, infraNS, StreamRHEL9)
7371
if err != nil {
7472
SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{
7573
Type: hyperv1.NodePoolValidPlatformImageType,

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1535,6 +1535,7 @@ func TestDefaultImage(t *testing.T) {
15351535
tests := []struct {
15361536
name string
15371537
arch string
1538+
streamName string
15381539
releaseImage *releaseinfo.ReleaseImage
15391540
expectedImage string
15401541
expectedDigest string
@@ -1572,6 +1573,28 @@ func TestDefaultImage(t *testing.T) {
15721573
expectedImage: "quay.io/openshift/release@sha256:x86_641234",
15731574
expectedDigest: "sha256:x86_641234",
15741575
},
1576+
{
1577+
name: "When named stream is used with multi-stream ReleaseImage it should resolve from the named stream",
1578+
arch: hyperv1.ArchitectureAMD64,
1579+
streamName: "rhel-9",
1580+
releaseImage: &releaseinfo.ReleaseImage{
1581+
OSStreams: map[string]*stream.Stream{
1582+
"rhel-9": {
1583+
Architectures: map[string]stream.Arch{
1584+
hyperv1.ArchAliases[hyperv1.ArchitectureAMD64]: {
1585+
Images: stream.Images{
1586+
KubeVirt: &stream.ContainerImage{
1587+
DigestRef: "quay.io/openshift/release@sha256:rhel9kubevirt",
1588+
},
1589+
},
1590+
},
1591+
},
1592+
},
1593+
},
1594+
},
1595+
expectedImage: "quay.io/openshift/release@sha256:rhel9kubevirt",
1596+
expectedDigest: "sha256:rhel9kubevirt",
1597+
},
15751598
}
15761599

15771600
for _, tt := range tests {
@@ -1580,7 +1603,7 @@ func TestDefaultImage(t *testing.T) {
15801603
if testRI == nil {
15811604
testRI = ri
15821605
}
1583-
img, digest, err := defaultImage(tt.arch, testRI, "")
1606+
img, digest, err := defaultImage(tt.arch, testRI, tt.streamName)
15841607
if tt.expectedError {
15851608
if err == nil {
15861609
t.Fatalf("expected error but got nil")

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,9 @@ func (c *CAPI) openstackMachineTemplate(templateNameGenerator func(spec any) (st
5050
return template, nil
5151
}
5252
func (r *NodePoolReconciler) setOpenStackConditions(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, _ string, releaseImage *releaseinfo.ReleaseImage) error {
53-
rhelStream, err := getRHELStreamForBootImage(nodePool, releaseImage)
54-
if err != nil {
55-
return fmt.Errorf("failed to resolve RHEL stream: %w", err)
56-
}
53+
// TODO(CNTRLPLANE-3553): hardcode to rhel-9 until the MCO can install
54+
// rhel-10 OS images. Use getRHELStreamForBootImage once MCO support lands.
55+
rhelStream := StreamRHEL9
5756
if nodePool.Spec.Platform.OpenStack.ImageName == "" {
5857
_, err := openstack.OpenStackReleaseImage(releaseImage, rhelStream)
5958
if err != nil {

‎hypershift-operator/controllers/nodepool/openstack/openstack_test.go‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ func TestOpenstackDefaultImage(t *testing.T) {
206206
testCases := []struct {
207207
name string
208208
releaseImage *releaseinfo.ReleaseImage
209+
streamName string
209210
expectedURL string
210211
expectedHash string
211212
expectedError bool
@@ -286,11 +287,39 @@ func TestOpenstackDefaultImage(t *testing.T) {
286287
},
287288
expectedError: true,
288289
},
290+
{
291+
name: "When named stream is used with multi-stream ReleaseImage it should resolve from the named stream",
292+
releaseImage: &releaseinfo.ReleaseImage{
293+
OSStreams: map[string]*stream.Stream{
294+
"rhel-9": {
295+
Architectures: map[string]stream.Arch{
296+
"x86_64": {
297+
Artifacts: map[string]stream.PlatformArtifacts{
298+
"openstack": {
299+
Formats: map[string]stream.ImageFormat{
300+
"qcow2.gz": {
301+
Disk: &stream.Artifact{
302+
Location: "https://example.com/rhel9-image.qcow2.gz",
303+
Sha256: "rhel9hash1234",
304+
},
305+
},
306+
},
307+
},
308+
},
309+
},
310+
},
311+
},
312+
},
313+
},
314+
streamName: "rhel-9",
315+
expectedURL: "https://example.com/rhel9-image.qcow2.gz",
316+
expectedHash: "rhel9hash1234",
317+
},
289318
}
290319

291320
for _, tc := range testCases {
292321
t.Run(tc.name, func(t *testing.T) {
293-
url, hash, err := OpenstackDefaultImage(tc.releaseImage, "")
322+
url, hash, err := OpenstackDefaultImage(tc.releaseImage, tc.streamName)
294323
if tc.expectedError {
295324
if err == nil {
296325
t.Error("expected error but got nil")
@@ -314,6 +343,7 @@ func TestOpenStackReleaseImage(t *testing.T) {
314343
testCases := []struct {
315344
name string
316345
releaseImage *releaseinfo.ReleaseImage
346+
streamName string
317347
expectedResult string
318348
expectedError bool
319349
}{
@@ -351,11 +381,31 @@ func TestOpenStackReleaseImage(t *testing.T) {
351381
},
352382
expectedError: true,
353383
},
384+
{
385+
name: "When named stream is used with multi-stream ReleaseImage it should resolve from the named stream",
386+
releaseImage: &releaseinfo.ReleaseImage{
387+
OSStreams: map[string]*stream.Stream{
388+
"rhel-9": {
389+
Architectures: map[string]stream.Arch{
390+
"x86_64": {
391+
Artifacts: map[string]stream.PlatformArtifacts{
392+
"openstack": {
393+
Release: "9.6.20250701",
394+
},
395+
},
396+
},
397+
},
398+
},
399+
},
400+
},
401+
streamName: "rhel-9",
402+
expectedResult: "9.6.20250701",
403+
},
354404
}
355405

356406
for _, tc := range testCases {
357407
t.Run(tc.name, func(t *testing.T) {
358-
result, err := OpenStackReleaseImage(tc.releaseImage, "")
408+
result, err := OpenStackReleaseImage(tc.releaseImage, tc.streamName)
359409
if tc.expectedError {
360410
if err == nil {
361411
t.Error("expected error but got nil")

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,9 @@ func reconcileIBMPowerVSImage(ibmPowerVSImage *capipowervs.IBMPowerVSImage, hclu
165165

166166
func (r *NodePoolReconciler) setPowerVSconditions(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, controlPlaneNamespace string, releaseImage *releaseinfo.ReleaseImage) error {
167167
log := ctrl.LoggerFrom(ctx)
168-
rhelStream, err := getRHELStreamForBootImage(nodePool, releaseImage)
169-
if err != nil {
170-
return fmt.Errorf("failed to resolve RHEL stream: %w", err)
171-
}
168+
// TODO(CNTRLPLANE-3553): hardcode to rhel-9 until the MCO can install
169+
// rhel-10 OS images. Use getRHELStreamForBootImage once MCO support lands.
170+
rhelStream := StreamRHEL9
172171
var coreOSPowerVSImage *stream.SingleObject
173172
coreOSPowerVSImage, powervsImageRegion, err := getPowerVSImage(hcluster.Spec.Platform.PowerVS.Region, releaseImage, rhelStream)
174173
if err != nil {

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package nodepool
33
import (
44
"fmt"
55

6+
hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
67
"github.com/openshift/hypershift/support/releaseinfo"
78

89
"github.com/blang/semver"
@@ -13,6 +14,20 @@ const (
1314
StreamRHEL10 = releaseinfo.StreamRHEL10
1415
)
1516

17+
func init() {
18+
// Go does not support compile-time string assertions. These init checks
19+
// are the next best thing: they run before any controller code and will
20+
// crash immediately if the API and releaseinfo stream constants diverge.
21+
if hyperv1.OSImageStreamRHEL9 != releaseinfo.StreamRHEL9 {
22+
panic(fmt.Sprintf("hyperv1.OSImageStreamRHEL9 (%q) != releaseinfo.StreamRHEL9 (%q)",
23+
hyperv1.OSImageStreamRHEL9, releaseinfo.StreamRHEL9))
24+
}
25+
if hyperv1.OSImageStreamRHEL10 != releaseinfo.StreamRHEL10 {
26+
panic(fmt.Sprintf("hyperv1.OSImageStreamRHEL10 (%q) != releaseinfo.StreamRHEL10 (%q)",
27+
hyperv1.OSImageStreamRHEL10, releaseinfo.StreamRHEL10))
28+
}
29+
}
30+
1631
// GetRHELStream resolves which RHEL CoreOS stream a NodePool should use.
1732
// Returns the resolved stream name, or an error for invalid combinations.
1833
// For OCP < 5.0 with no explicit stream it returns StreamRHEL9 (the only

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import (
55

66
. "github.com/onsi/gomega"
77

8+
hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
9+
"github.com/openshift/hypershift/support/releaseinfo"
10+
811
"github.com/blang/semver"
912
)
1013

@@ -169,3 +172,13 @@ func TestGetRHELStream(t *testing.T) {
169172
})
170173
}
171174
}
175+
176+
// TestStreamConstantsMatch ensures the API and releaseinfo stream constants
177+
// stay in sync so the cross-reference comments don't silently diverge.
178+
func TestStreamConstantsMatch(t *testing.T) {
179+
g := NewWithT(t)
180+
g.Expect(hyperv1.OSImageStreamRHEL9).To(Equal(releaseinfo.StreamRHEL9),
181+
"API constant OSImageStreamRHEL9 must match releaseinfo.StreamRHEL9")
182+
g.Expect(hyperv1.OSImageStreamRHEL10).To(Equal(releaseinfo.StreamRHEL10),
183+
"API constant OSImageStreamRHEL10 must match releaseinfo.StreamRHEL10")
184+
}

0 commit comments

Comments
 (0)