Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 85 additions & 13 deletions pkg/controller/bootimage/boot_image_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}{
Expand Down Expand Up @@ -682,7 +682,6 @@ func TestReconcileAzureProviderSpec(t *testing.T) {
Version: "419.94.20250101",
Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan,
},
expectSkip: true,
},
{
name: "Skip unsupported architecture s390x",
Expand All @@ -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",
Expand Down Expand Up @@ -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": {
Expand All @@ -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": {
Expand All @@ -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",
},
},
},
},
},
},
},
},
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
{
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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
30 changes: 19 additions & 11 deletions pkg/controller/bootimage/platform_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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?
Expand Down Expand Up @@ -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
Expand All @@ -438,24 +446,24 @@ 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

// 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
}
Expand All @@ -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
}