diff --git a/pkg/controller/bootimage/boot_image_controller_test.go b/pkg/controller/bootimage/boot_image_controller_test.go index 786c6d730d..7c09ad8c5c 100644 --- a/pkg/controller/bootimage/boot_image_controller_test.go +++ b/pkg/controller/bootimage/boot_image_controller_test.go @@ -551,7 +551,7 @@ func TestReconcileAzureProviderSpec(t *testing.T) { currentImage machinev1beta1.Image expectedImage machinev1beta1.Image expectPatch bool - expectSkip bool + expectReconcileSkipped bool streamData *stream.Stream // Custom stream data for specific tests securityProfile *machinev1beta1.SecurityProfile // Custom security profile for specific tests }{ @@ -682,7 +682,6 @@ func TestReconcileAzureProviderSpec(t *testing.T) { Version: "419.94.20250101", Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan, }, - expectSkip: true, }, { name: "Skip unsupported architecture s390x", @@ -695,7 +694,6 @@ func TestReconcileAzureProviderSpec(t *testing.T) { Version: "419.94.20250101", Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan, }, - expectSkip: true, }, { name: "Paid OCP Gen1 image updates to newer version", @@ -813,7 +811,7 @@ func TestReconcileAzureProviderSpec(t *testing.T) { Version: "419.94.20250101", Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan, }, - expectSkip: true, + expectReconcileSkipped: true, streamData: &stream.Stream{ Architectures: map[string]stream.Arch{ "x86_64": { @@ -835,7 +833,7 @@ func TestReconcileAzureProviderSpec(t *testing.T) { Version: "419.94.20250101", Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan, }, - expectSkip: true, + expectReconcileSkipped: true, streamData: &stream.Stream{ Architectures: map[string]stream.Arch{ "x86_64": { @@ -848,6 +846,84 @@ func TestReconcileAzureProviderSpec(t *testing.T) { }, }, }, + { + name: "Skip when Gen1 Azure marketplace image is unavailable (Gen1 removal)", + arch: "x86_64", + currentImage: machinev1beta1.Image{ + Offer: "aro4", + Publisher: "azureopenshift", + ResourceID: "", + SKU: "aro_418", + Version: "418.94.20241201", + Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan, + }, + expectReconcileSkipped: true, + streamData: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + RHELCoreOSExtensions: &rhcos.Extensions{ + Marketplace: &rhcos.Marketplace{ + Azure: &rhcos.AzureMarketplace{ + NoPurchasePlan: &rhcos.AzureMarketplaceImages{ + // Gen1 intentionally omitted, mirroring the stream once + // Gen1 Azure images are removed upstream (CORS-4441). + Gen2: &rhcos.AzureMarketplaceImage{ + Offer: "aro4", + Publisher: "azureopenshift", + SKU: "aro_50-x64", + Version: "50.0.20260601", + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "Post-Gen1-removal Gen2 SKU ('gen2' suffix) still updates", + arch: "x86_64", + currentImage: machinev1beta1.Image{ + Offer: "aro4", + Publisher: "azureopenshift", + ResourceID: "", + SKU: "aro_5-0_x86_gen2", + Version: "50.0.20260601", + Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan, + }, + expectedImage: machinev1beta1.Image{ + Offer: "aro4", + Publisher: "azureopenshift", + ResourceID: "", + SKU: "aro_5-0_x86_gen2", + Version: "50.0.20260701", + Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan, + }, + expectPatch: true, + streamData: &stream.Stream{ + Architectures: map[string]stream.Arch{ + "x86_64": { + RHELCoreOSExtensions: &rhcos.Extensions{ + Marketplace: &rhcos.Marketplace{ + Azure: &rhcos.AzureMarketplace{ + NoPurchasePlan: &rhcos.AzureMarketplaceImages{ + // Gen1 intentionally omitted, mirroring the stream once + // Gen1 Azure images are removed upstream (CORS-4441). + Gen2: &rhcos.AzureMarketplaceImage{ + Offer: "aro4", + Publisher: "azureopenshift", + SKU: "aro_5-0_x86_gen2", + Version: "50.0.20260701", + }, + }, + }, + }, + }, + }, + }, + }, + }, { name: "Skip machineset with ConfidentialVM SecurityType", arch: "x86_64", @@ -859,12 +935,12 @@ func TestReconcileAzureProviderSpec(t *testing.T) { Version: "419.94.20250101", Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan, }, - expectSkip: true, securityProfile: &machinev1beta1.SecurityProfile{ Settings: machinev1beta1.SecuritySettings{ SecurityType: "ConfidentialVM", }, }, + expectReconcileSkipped: true, }, { name: "Skip machineset with TrustedLaunch SecurityType", @@ -877,12 +953,12 @@ func TestReconcileAzureProviderSpec(t *testing.T) { Version: "419.94.20250101", Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan, }, - expectSkip: true, securityProfile: &machinev1beta1.SecurityProfile{ Settings: machinev1beta1.SecuritySettings{ SecurityType: "TrustedLaunch", }, }, + expectReconcileSkipped: true, }, { name: "Process machineset with SecurityProfile but empty SecurityType", @@ -954,7 +1030,7 @@ func TestReconcileAzureProviderSpec(t *testing.T) { testStreamData = tt.streamData } - patchRequired, _, updatedProviderSpec, _, err := reconcileAzureProviderSpec( + patchRequired, reconcileSkipped, updatedProviderSpec, _, err := reconcileAzureProviderSpec( testStreamData, tt.arch, infra, @@ -965,11 +1041,7 @@ func TestReconcileAzureProviderSpec(t *testing.T) { require.NoError(t, err) - if tt.expectSkip { - assert.False(t, patchRequired, "Expected no patch for skipped case") - return - } - + assert.Equal(t, tt.expectReconcileSkipped, reconcileSkipped, "Reconcile skipped mismatch") assert.Equal(t, tt.expectPatch, patchRequired, "Patch required mismatch") if tt.expectPatch { diff --git a/pkg/controller/bootimage/platform_helpers.go b/pkg/controller/bootimage/platform_helpers.go index b13385cd42..9b3a95244a 100644 --- a/pkg/controller/bootimage/platform_helpers.go +++ b/pkg/controller/bootimage/platform_helpers.go @@ -286,7 +286,7 @@ func reconcileAzureProviderSpec(streamData *stream.Stream, arch string, _ *oscon if providerSpec.SecurityProfile != nil && providerSpec.SecurityProfile.Settings.SecurityType != "" { klog.Infof("Skipping update for %s, machinesets/controlplanemachinesets with a SecurityType defined(%s in this case) is not currently supported for Azure", machineSetName, providerSpec.SecurityProfile.Settings.SecurityType) - return false, false, nil, "", nil + return false, true, nil, "", nil } currentImage := providerSpec.Image @@ -326,7 +326,8 @@ func reconcileAzureProviderSpec(streamData *stream.Stream, arch string, _ *oscon // Uploaded images(legacy) have a "gen2" in the resourceID field to indicate hyperGenV2 // // Unpaid marketplace images: - // - have a "v2" in the SKU field to indicate hyperGenV2 + // - have a "v2" in the SKU field to indicate hyperGenV2 (e.g. "aro_422-v2") + // - have a "gen2" in the SKU field to indicate hyperGenV2 (5.0+, e.g. "aro_5-0_x86_gen2") // - aarch64 machinesets can only use hyperGenV2 images // // Paid marketplace images(MCO-1790): @@ -337,16 +338,20 @@ func reconcileAzureProviderSpec(streamData *stream.Stream, arch string, _ *oscon case usesLegacyImageUpload: usesHyperVGen2 = strings.Contains(currentImage.ResourceID, "gen2") case providerSpec.Image.Type == machinev1beta1.AzureImageTypeMarketplaceNoPlan: - usesHyperVGen2 = strings.Contains(currentImage.SKU, "v2") || arch == "aarch64" + usesHyperVGen2 = strings.Contains(currentImage.SKU, "v2") || strings.Contains(currentImage.SKU, "gen2") || arch == "aarch64" default: usesHyperVGen2 = !strings.Contains(currentImage.SKU, "gen1") } // Determine target image from RHCOS stream - targetImage, err := getTargetImageFromStream(streamArch, azureVariant, usesHyperVGen2, arch) + targetImage, reconcileSkipped, err := getTargetImageFromStream(streamArch, azureVariant, usesHyperVGen2, arch) if err != nil { return false, false, nil, "", err } + if reconcileSkipped { + klog.Infof("Skipping machineset %s, no Gen1 Azure marketplace image available for architecture %s", machineSetName, arch) + return false, true, nil, "", nil + } // If the current image matches, nothing to do here // Q: Should we enhance this to do version comparisons? @@ -415,8 +420,11 @@ func determineAzureVariant(usesLegacyImageUpload bool, currentImage machinev1bet return "", fmt.Errorf("could not determine azure marketplace variant, cannot update boot images") } -// getTargetImageFromStream determines the correct Azure marketplace image based on architecture and variant -func getTargetImageFromStream(streamArch *stream.Arch, variant AzureVariant, usesHyperVGen2 bool, arch string) (machinev1beta1.Image, error) { +// getTargetImageFromStream determines the correct Azure marketplace image based on architecture and variant. +// Returns reconcileSkipped=true (with no error) when a Gen1 image is requested but the stream no longer +// publishes one, e.g. once Gen1 Azure images are removed upstream (see CORS-4441): the boot image update is +// skipped for this MachineSet rather than treated as an error, so skew enforcement can flag it as out of date. +func getTargetImageFromStream(streamArch *stream.Arch, variant AzureVariant, usesHyperVGen2 bool, arch string) (machinev1beta1.Image, bool, error) { marketplace := streamArch.RHELCoreOSExtensions.Marketplace.Azure var imageSet *rhcos.AzureMarketplaceImages @@ -438,11 +446,11 @@ func getTargetImageFromStream(streamArch *stream.Arch, variant AzureVariant, use case AzureVariantOKEEMEA: imageSet = marketplace.OKEEMEA default: - return machinev1beta1.Image{}, fmt.Errorf("unsupported Azure variant") + return machinev1beta1.Image{}, false, fmt.Errorf("unsupported Azure variant") } if imageSet == nil { - return machinev1beta1.Image{}, fmt.Errorf("no Azure marketplace images available for variant %s", variant) + return machinev1beta1.Image{}, false, fmt.Errorf("no Azure marketplace images available for variant %s", variant) } var streamImage *rhcos.AzureMarketplaceImage @@ -450,12 +458,12 @@ func getTargetImageFromStream(streamArch *stream.Arch, variant AzureVariant, use // arm64 only uses hyperGenV2 if usesHyperVGen2 { if imageSet.Gen2 == nil { - return machinev1beta1.Image{}, fmt.Errorf("no Gen2 Azure marketplace image available for architecture %s", arch) + return machinev1beta1.Image{}, false, fmt.Errorf("no Gen2 Azure marketplace image available for architecture %s", arch) } streamImage = imageSet.Gen2 } else { if imageSet.Gen1 == nil { - return machinev1beta1.Image{}, fmt.Errorf("no Gen1 Azure marketplace image available for architecture %s", arch) + return machinev1beta1.Image{}, true, nil } streamImage = imageSet.Gen1 } @@ -464,5 +472,5 @@ func getTargetImageFromStream(streamArch *stream.Arch, variant AzureVariant, use // Convert stream image to Azure machine image targetImage := getAzureImageFromStreamImage(*streamImage, isPaidImage) - return targetImage, nil + return targetImage, false, nil } diff --git a/test/extended-priv/mco_bootimages.go b/test/extended-priv/mco_bootimages.go index a8df8b13cd..c1dbb219ce 100644 --- a/test/extended-priv/mco_bootimages.go +++ b/test/extended-priv/mco_bootimages.go @@ -71,7 +71,7 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati duplicatedMachinesetName = fmt.Sprintf("cloned-tc-%s", GetCurrentTestPolarionIDNumber()) firstMachineSet = NewMachineSetList(oc.AsAdmin(), MachineAPINamespace).GetAllOrFail()[0] backdatedImageName = getBackdatedBootImage(oc.AsAdmin(), firstMachineSet) - fakeImageNameNoUpdate = "fake-noupdate-image-81403" + fakeImageNameNoUpdate = getFakeNoUpdateBootImage(oc.AsAdmin(), "81403") ) exutil.By("Duplicate machineset for testing") @@ -142,7 +142,7 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati var ( machineSet = NewMachineSetList(oc.AsAdmin(), MachineAPINamespace).GetAllOrFail()[0] backdatedImageName = getBackdatedBootImage(oc.AsAdmin(), machineSet) - fakeImageNameNoUpdate = "fake-noupdate-image-74240" + fakeImageNameNoUpdate = getFakeNoUpdateBootImage(oc.AsAdmin(), "74240") clonedMSName = "cloned-tc-74240" clonedWrongBootImageMSName = "cloned-tc-74240-wrong-boot-image" clonedOwnedMSName = "cloned-tc-74240-owned" @@ -248,7 +248,7 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati var ( machineSet = NewMachineSetList(oc.AsAdmin(), MachineAPINamespace).GetAllOrFail()[0] backdatedImageName = getBackdatedBootImage(oc.AsAdmin(), machineSet) - fakeImageNameNoUpdate = "fake-noupdate-image-74239" + fakeImageNameNoUpdate = getFakeNoUpdateBootImage(oc.AsAdmin(), "74239") clonedMSLabelName = "cloned-tc-74239-label" clonedMSNoLabelName = "cloned-tc-74239-no-label" clonedMSLabelOwnedName = "cloned-tc-74239-label-owned" @@ -1109,12 +1109,34 @@ func CheckCurrentOSImageIsNotUpdated(bir BootImageResource, fakeImageName string return release }, "15s", "5s").ShouldNot(o.Equal(currentCoreOsBootImage), "%s was updated but it should NOT have been", bir) + case AzurePlatform: + // Compare by resourceID only to avoid field-ordering sensitivity in the full Image JSON. + expectedResourceID := gjson.Get(fakeImageName, "resourceID").String() + o.Consistently(func() (string, error) { + img, err := bir.GetCoreOsBootImage() + if err != nil { + return "", err + } + return gjson.Get(img, "resourceID").String(), nil + }, "15s", "5s").Should(o.Equal(expectedResourceID), + "%s was updated but it should NOT have been", bir) default: o.Consistently(bir.GetCoreOsBootImage, "15s", "5s").Should(o.Equal(fakeImageName), "%s was updated but it should NOT have been", bir) } } +// getFakeNoUpdateBootImage returns a platform-appropriate fake boot image value that will not be +// recognised as a valid managed image by MCO, so the resource carrying it is expected to stay +// unchanged. On Azure the image field is a struct, so a plain string would be rejected by the +// MachineSet admission webhook; we wrap it in a minimal Image JSON object instead. +func getFakeNoUpdateBootImage(oc *exutil.CLI, id string) string { + if exutil.CheckPlatform(oc) == AzurePlatform { + return fmt.Sprintf(`{"offer":"","publisher":"","resourceID":"fake-noupdate-image-%s","sku":"","version":""}`, id) + } + return "fake-noupdate-image-" + id +} + // setArchitectureAndCheckStatus sets the capacity labels annotation on the cloned machineset and checks the status. // If archValue already contains "kubernetes.io/arch=", it is used as the raw annotation value. // Otherwise, "kubernetes.io/arch=" is prepended automatically.