diff --git a/pkg/controller/bootimage/ami.go b/pkg/controller/bootimage/ami.go index c6a5a91d72..2b68b94deb 100644 --- a/pkg/controller/bootimage/ami.go +++ b/pkg/controller/bootimage/ami.go @@ -1922,5 +1922,45 @@ var AllowedAMIs = sets.New( "ami-0e6a4ffe9dd1dee56", "ami-0e721586ef04044d2", "ami-0e75465363d8378f0", "ami-0e79be45f2303f422", "ami-0e9a0f9e1a4c49b92", "ami-0ec2f94496976cb5f", "ami-0f186d1a862e47c8f", "ami-0f480726bfb767807", "ami-0f5633a3440704a9d", "ami-0f567edf0859e3b39", "ami-0f5b937ac530500da", "ami-0f752ff59c8b0d39f", "ami-0f7add16535539145", "ami-0f99bc65b81da18b0", "ami-0fb8045529e752a6f", - "ami-0fc49ff64e479b1f5", "ami-0fe15e383af1e71d4", "ami-0feb18654dafb55c6", "ami-0fed99e2bf80aea18", + "ami-0fc49ff64e479b1f5", "ami-0fe15e383af1e71d4", "ami-0feb18654dafb55c6", "ami-0fed99e2bf80aea18", "ami-0012f6e857cddb9a3", + "ami-0045babcefcde70ad", "ami-00468ad759fd2d7b1", "ami-0047b12f232e22bdb", "ami-004f5780eeabcc87a", "ami-005037a6054789706", + "ami-006ceba545a1fc2e4", "ami-007f29d20651b2967", "ami-009377da169f64c55", "ami-00bfd186388b8380c", "ami-00c830a23b63d4db9", + "ami-0104bef90662c2efd", "ami-013471b87ace7db65", "ami-0180fbb452a64abda", "ami-0186aa560203c6187", "ami-019714e3997bd07a7", + "ami-01973dc30b4a31e97", "ami-01b95e0b30b0ea34c", "ami-01c33df45d576ada8", "ami-01e235e15a11eb0df", "ami-01f5e666e658ad1ab", + "ami-01f61d4d53aa2a9b6", "ami-0202fd73338464188", "ami-0209332acb1e36853", "ami-021ab1158e383ad67", "ami-022be29d2d96de13c", + "ami-02432a7c4ea6e9a3c", "ami-0252011a6427df19f", "ami-02855fa6ea815d06b", "ami-028d46fe147b1cd05", "ami-029820074e2035ae4", + "ami-02c128dd1f85bbfee", "ami-02cf45d9cde4da953", "ami-02d471a336435f50a", "ami-02da66fa6254fc712", "ami-02e03c003116ad480", + "ami-02e9e72117f14091c", "ami-02eef3fb87f70be2f", "ami-030d8b87df4826acc", "ami-033b1cf48ab5ed019", "ami-034f4fd71b696a198", + "ami-035173bcb9f915eb7", "ami-035f7d85a34f9141c", "ami-036ce05c3a726a8d8", "ami-036d1e4326e2e57e0", "ami-037f5cb9b1933c006", + "ami-03877388d3d6b48fb", "ami-0388cbd1d1429c4c7", "ami-0396eaa6d72c99d19", "ami-039d3e3ccfd6e5408", "ami-03c999a079ddde054", + "ami-03ce0238d62983330", "ami-03cf2640751bbd84e", "ami-03d2f6868a810f16e", "ami-03f1bd9904faf3058", "ami-041bc3e9395d5bae7", + "ami-041c964652b0dff67", "ami-042619f83fbf591a2", "ami-0429c8240a8bad514", "ami-043a374947b137a5a", "ami-0445378debb78f68d", + "ami-044735b15a195810a", "ami-0489962ff65796c68", "ami-04946ee6336a5fdef", "ami-049e29474b06cb799", "ami-04a0abd17c9871211", + "ami-04a0b75d82d641bb3", "ami-04aaae682d4870b7c", "ami-04c82f85d00c673ec", "ami-04dad94be03c8edd9", "ami-04ee1d46e3cb6f3a4", + "ami-04f93fc50a015aaf2", "ami-050ce14fc96932898", "ami-0519ca783d0405301", "ami-052e0663607930a9d", "ami-0532c87dee2e73586", + "ami-0537af5461a58de15", "ami-055eb42c9ac922b60", "ami-0561d6e6064bb80d9", "ami-056e65e8594c0ab06", "ami-056fb8260d98c0caa", + "ami-057ecb31eeebbfb28", "ami-058733b6471456f95", "ami-0589fd0ec9af25c2a", "ami-05918a69def5e5faa", "ami-05b1fc1eeda2c2c1c", + "ami-0615fc30e2ca45e48", "ami-063a2c8fc9621d0bc", "ami-0666d4665ad983743", "ami-06752c2e4bb6e0bc7", "ami-068ef76d0b96c5962", + "ami-06a4512ffd164d0fd", "ami-06a79aa0169e4e304", "ami-06a9715660aa4d7e2", "ami-06aabe4eb1a6f5474", "ami-06b5e1e107986965c", + "ami-06c3ad3985c66cdb1", "ami-06d064935da85e2c8", "ami-070aec204c9ffc2f9", "ami-071bbf4bfda59764e", "ami-0724620758713e90a", + "ami-0789ec6c1b57d2aee", "ami-079b5123ee195034a", "ami-079bdaf173b88cb3d", "ami-07a8f5c9253f89580", "ami-07ca8313ee5def8ee", + "ami-07da482306ca26c90", "ami-07dd252d750c538b1", "ami-07e3b70729b7e899b", "ami-07e428ccbe30749f5", "ami-07e83229bad88b7da", + "ami-07f1f64194c014cbe", "ami-0806b1e81c02af6a5", "ami-081c6b6190b8a96f9", "ami-082401c298bb6d19a", "ami-083d6d8c0825b5e77", + "ami-0860c0446f0d0edff", "ami-0866e7d7620662cb8", "ami-086c4794dcbad5968", "ami-08815949a9efd3f9d", "ami-08894d33a8238ae1f", + "ami-08a8683cdafafd025", "ami-08c5a756919080ed3", "ami-08f70223a9e974fc2", "ami-090feca83f7df5af0", "ami-09116dce527366abf", + "ami-0932ffb821f6ede9a", "ami-09390eb5fa05105cb", "ami-0943da5f9c2e42bf1", "ami-094be75c86c860575", "ami-09611a0d3a8b93f5e", + "ami-096a14dab9918e653", "ami-0977a293b90a4dcd4", "ami-099b01d69f79c10de", "ami-09a227b539f0108dd", "ami-09a6deb5147f425c1", + "ami-09ca2efc70796ebb9", "ami-09f6c75068ed7d615", "ami-09fc07d00a02e1a75", "ami-0a5ec51a9dad18def", "ami-0adf76c9ca225891d", + "ami-0ae0c3359ca6aa3df", "ami-0b6ed5cedd5f504dd", "ami-0b7e179e05a0c8aed", "ami-0b7f7c4c5c1fec821", "ami-0b8633f808328467f", + "ami-0b924a152c5107772", "ami-0b9f7f6c100a4e205", "ami-0bc0b6ee7b08808e7", "ami-0bc200ec19c86b0e8", "ami-0bfa9e8ab847dc884", + "ami-0c085c5a670ed56e2", "ami-0c0e529a01661c018", "ami-0c14479e30c43c66e", "ami-0c1ea9aa8ff649700", "ami-0c20892f27a4b30ab", + "ami-0c25f24ad1d9ca99f", "ami-0c26e0c09048c67d2", "ami-0c4d8f126d6b48df5", "ami-0c5ce5542a11b1625", "ami-0c5de5918dd160bfb", + "ami-0c706f9ea60c95209", "ami-0c71ad8672b323c81", "ami-0c7b24d72d02ab879", "ami-0c8ac6d2d139ed219", "ami-0c9350869efbbadb8", + "ami-0cbf4be274bd5af48", "ami-0cdc52b7f6229c45d", "ami-0cec7d60b4e64aa10", "ami-0d16876eaa5aa6dfa", "ami-0d1905921e683afab", + "ami-0d26db932afc5b88b", "ami-0d2babc9b55094593", "ami-0d3b1aefc3285a531", "ami-0d5313a5ea3e8ffb7", "ami-0d54c1cbf3f2e8fec", + "ami-0d59eeb1aefa86643", "ami-0d78c386ff1adf973", "ami-0d7d88bd7bfc31a5c", "ami-0d8786dc679ff9a19", "ami-0d955ed29c6286ffb", + "ami-0e14bfe5873a1d8f6", "ami-0e1bf6775f2d3adc5", "ami-0e2fa97cd490f90ff", "ami-0e319599cb7cad8df", "ami-0e350eb65edaab597", + "ami-0e5fa644db548e0ca", "ami-0e8580dcb3b54bc65", "ami-0eee7e0e3b42ca954", "ami-0ef6a62b4537e52c4", "ami-0efc707d4bb86da28", + "ami-0f1df8bccf443942f", "ami-0f29e5607aaabba57", "ami-0f3f3636d4e229c66", "ami-0f650e3d802e28557", "ami-0f6fb9bca28c962d0", + "ami-0fb5741e6b20f10f1", "ami-0fc60b0d0fb8d5a8c", "ami-0fc93459d0da40cf1", "ami-0fd7aabf34efa0f19", ) diff --git a/pkg/controller/bootimage/boot_image_controller_test.go b/pkg/controller/bootimage/boot_image_controller_test.go index 786c6d730d..742fa94520 100644 --- a/pkg/controller/bootimage/boot_image_controller_test.go +++ b/pkg/controller/bootimage/boot_image_controller_test.go @@ -14,6 +14,7 @@ import ( opv1 "github.com/openshift/api/operator/v1" configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" fakemcopclient "github.com/openshift/client-go/operator/clientset/versioned/fake" + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -25,7 +26,6 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" - ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" ) func TestIsClusterStable(t *testing.T) { @@ -546,14 +546,14 @@ func TestReconcileAzureProviderSpec(t *testing.T) { fakeClient := fake.NewClientset(testSecret) tests := []struct { - name string - arch string - currentImage machinev1beta1.Image - expectedImage machinev1beta1.Image - expectPatch bool - expectSkip bool - streamData *stream.Stream // Custom stream data for specific tests - securityProfile *machinev1beta1.SecurityProfile // Custom security profile for specific tests + name string + arch string + currentImage machinev1beta1.Image + expectedImage machinev1beta1.Image + expectPatch bool + expectReconcileSkipped bool + streamData *stream.Stream // Custom stream data for specific tests + securityProfile *machinev1beta1.SecurityProfile // Custom security profile for specific tests }{ { name: "Legacy Gen1 upload image transitions to marketplace Gen1", @@ -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,7 +935,7 @@ func TestReconcileAzureProviderSpec(t *testing.T) { Version: "419.94.20250101", Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan, }, - expectSkip: true, + expectReconcileSkipped: true, securityProfile: &machinev1beta1.SecurityProfile{ Settings: machinev1beta1.SecuritySettings{ SecurityType: "ConfidentialVM", @@ -877,7 +953,7 @@ func TestReconcileAzureProviderSpec(t *testing.T) { Version: "419.94.20250101", Type: machinev1beta1.AzureImageTypeMarketplaceNoPlan, }, - expectSkip: true, + expectReconcileSkipped: true, securityProfile: &machinev1beta1.SecurityProfile{ Settings: machinev1beta1.SecuritySettings{ SecurityType: "TrustedLaunch", @@ -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/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index 1a33d49927..2b8c4506f6 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -50,7 +50,7 @@ type Bootstrap struct { // dir used to read pools and user defined machineconfigs. manifestDir string // pull secret file - pullSecretFile string + pullSecretFile string imageStreamFactory osimagestream.ImageStreamFactory inspectorFactory osimagestream.ImagesInspectorFactory } @@ -242,21 +242,15 @@ func (b *Bootstrap) Run(destDir string) error { return fmt.Errorf("error filtering pools: %w", err) } + sysCtxFactory := buildSysContextFactory(pullSecret, cconfig, cconfig.Spec.Infra, imgCfg, icspRules, idmsRules, itmsRules) + // Enable OSImageStreams if the FeatureGate is active. // Previously this also excluded ExternalTopologyMode (HyperShift) because // HyperShift did not yet write stream selection into the synthetic MCP. // Now that HyperShift writes 99_osimagestream.yaml into the MCC template // directory (openshift/hypershift#8792), the guard is no longer needed. if osimagestream.IsFeatureEnabled(fgHandler) { - osImageStream, err = b.fetchOSImageStream( - imageStream, - cconfig, - icspRules, - idmsRules, - itmsRules, - imgCfg, - pullSecret, - osImageStream) + osImageStream, err = b.fetchOSImageStream(sysCtxFactory, imageStream, cconfig, osImageStream) if err != nil { return err } @@ -369,7 +363,6 @@ func (b *Bootstrap) Run(destDir string) error { klog.Infof("Successfully created %d pre-built image component MachineConfigs for hybrid OCL.", len(preBuiltImageMCs)) } - sysCtxFactory := buildSysContextFactory(pullSecret, cconfig, imgCfg, icspRules, idmsRules, itmsRules) inspector := osimagestream.NewStreamClassInspector(b.inspectorFactory, sysCtxFactory) fpools, gconfigs, err := render.RunBootstrap(context.TODO(), pools, configs, cconfig, osImageStream, inspector) if err != nil { @@ -500,21 +493,15 @@ func filterPools(pools []*mcfgv1.MachineConfigPool) ([]*mcfgv1.MachineConfigPool } func (b *Bootstrap) fetchOSImageStream( + sysCtxFactory imageutils.SysContextFactory, imageStream *imagev1.ImageStream, cconfig *mcfgv1.ControllerConfig, - icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy, - idmsRules []*apicfgv1.ImageDigestMirrorSet, - itmsRules []*apicfgv1.ImageTagMirrorSet, - imgCfg *apicfgv1.Image, - pullSecret *corev1.Secret, existingOSImageStream *mcfgv1.OSImageStream, ) (*mcfgv1.OSImageStream, error) { ctx, cancel := context.WithTimeout(context.Background(), time.Minute) defer cancel() - sysCtxFactory := buildSysContextFactory(pullSecret, cconfig, imgCfg, icspRules, idmsRules, itmsRules) - factory := b.imageStreamFactory createOpts := osimagestream.CreateOptions{ ExistingOSImageStream: existingOSImageStream, @@ -538,6 +525,7 @@ func (b *Bootstrap) fetchOSImageStream( func buildSysContextFactory( pullSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig, + infra *apicfgv1.Infrastructure, imgCfg *apicfgv1.Image, icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy, idmsRules []*apicfgv1.ImageDigestMirrorSet, @@ -548,6 +536,12 @@ func buildSysContextFactory( WithControllerConfig(cconfig). WithSecret(pullSecret) + // In HCP the proxy config belongs to the data plane cluster and is + // unreachable from the management cluster where this code runs. + if infra != nil && infra.Status.ControlPlaneTopology == apicfgv1.ExternalTopologyMode { + builder.WithoutProxy() + } + registriesConfig, err := imageutils.GenerateRegistriesConfig(imgCfg, icspRules, idmsRules, itmsRules) if err != nil { return nil, fmt.Errorf("failed to generate registries config: %w", err) diff --git a/pkg/controller/build/imagebuilder/jobimagebuilder.go b/pkg/controller/build/imagebuilder/jobimagebuilder.go index ad65e9670c..16041cf45a 100644 --- a/pkg/controller/build/imagebuilder/jobimagebuilder.go +++ b/pkg/controller/build/imagebuilder/jobimagebuilder.go @@ -326,6 +326,34 @@ func (j *jobImageBuilder) validateBuilderType(builder buildrequest.Builder) erro return fmt.Errorf("invalid type %T from builder, expected %T", j.builder, &batchv1.Job{}) } +// buildFailedConditionsFromJob returns MachineOSBuild failed conditions populated with the +// failure reason and message from the job's Failed condition, falling back to generic values. +func buildFailedConditionsFromJob(job *batchv1.Job) []metav1.Condition { + reason := "Failed" + message := "Build Failed" + for _, cond := range job.Status.Conditions { + if cond.Type == batchv1.JobFailed && cond.Status == corev1.ConditionTrue { + if cond.Reason != "" { + reason = cond.Reason + } + if cond.Message != "" { + message = cond.Message + } + break + } + } + message = fmt.Sprintf("Job %q failed after %d attempt(s): %s", job.Name, job.Status.Failed, message) + conditions := apihelpers.MachineOSBuildFailedConditions() + for i := range conditions { + if conditions[i].Type == string(mcfgv1.MachineOSBuildFailed) { + conditions[i].Reason = reason + conditions[i].Message = message + break + } + } + return conditions +} + // Maps a given batchv1.Job to a given MachineOSBuild status. Exported so that it can be used in e2e tests. func MapJobStatusToBuildStatus(job *batchv1.Job) (mcfgv1.BuildProgress, []metav1.Condition) { // If the job is being deleted and it was not in either a successful or failed state @@ -356,7 +384,7 @@ func MapJobStatusToBuildStatus(job *batchv1.Job) (mcfgv1.BuildProgress, []metav1 return mcfgv1.MachineOSBuildSucceeded, apihelpers.MachineOSBuildSucceededConditions() } if condition.Type == batchv1.JobFailed && condition.Status == corev1.ConditionTrue { - return mcfgv1.MachineOSBuildFailed, apihelpers.MachineOSBuildFailedConditions() + return mcfgv1.MachineOSBuildFailed, buildFailedConditionsFromJob(job) } } // If we have succeeded pods but no completion condition, we're still building @@ -365,7 +393,7 @@ func MapJobStatusToBuildStatus(job *batchv1.Job) (mcfgv1.BuildProgress, []metav1 // Only return failed if there have been 4 pod failures as the backoffLimit is set to 3 if job.Status.Failed > constants.JobMaxRetries { - return mcfgv1.MachineOSBuildFailed, apihelpers.MachineOSBuildFailedConditions() + return mcfgv1.MachineOSBuildFailed, buildFailedConditionsFromJob(job) } return "", apihelpers.MachineOSBuildInitialConditions() diff --git a/pkg/controller/build/ocl_events.go b/pkg/controller/build/ocl_events.go index a26c2d9fde..8ebb1e1dd8 100644 --- a/pkg/controller/build/ocl_events.go +++ b/pkg/controller/build/ocl_events.go @@ -24,6 +24,7 @@ const ( EventJobCreated = "JobCreated" EventJobStarted = "JobStarted" EventJobCompleted = "JobCompleted" + EventJobPodFailed = "JobPodFailed" EventJobFailed = "JobFailed" EventJobDeleted = "JobDeleted" @@ -120,10 +121,23 @@ func (r *OCLEventRecorder) RecordJobCompleted(mosb *mcfgv1.MachineOSBuild, job * fmt.Sprintf("Build job completed: %s", job.Name)) } -// RecordJobFailed records when a build job fails +// RecordJobPodFailed records when a build pod fails but the job is still retrying +func (r *OCLEventRecorder) RecordJobPodFailed(mosb *mcfgv1.MachineOSBuild, job *batchv1.Job, maxRetries int32) { + r.recorder.Event(mosb, corev1.EventTypeWarning, EventJobPodFailed, + fmt.Sprintf("Build pod failed (attempt %d of %d); build job is retrying", job.Status.Failed, maxRetries+1)) +} + +// RecordJobFailed records when a build job has exhausted all retries and finally failed func (r *OCLEventRecorder) RecordJobFailed(mosb *mcfgv1.MachineOSBuild, job *batchv1.Job) { + reason := "unknown" + for _, cond := range job.Status.Conditions { + if cond.Type == batchv1.JobFailed && cond.Status == corev1.ConditionTrue && cond.Message != "" { + reason = cond.Message + break + } + } r.recorder.Event(mosb, corev1.EventTypeWarning, EventJobFailed, - fmt.Sprintf("Build job %q failed; see MachineOSBuild %q status conditions for details", job.Name, mosb.Name)) + fmt.Sprintf("Build job %q failed after %d attempt(s): %s", job.Name, job.Status.Failed, reason)) } // RecordJobDeleted records when a build job is deleted diff --git a/pkg/controller/build/ocl_events_test.go b/pkg/controller/build/ocl_events_test.go index 940af5dd2d..86d5332717 100644 --- a/pkg/controller/build/ocl_events_test.go +++ b/pkg/controller/build/ocl_events_test.go @@ -117,6 +117,7 @@ func TestJobEvents(t *testing.T) { {"JobCreated", func(r *OCLEventRecorder) { r.RecordJobCreated(mosb, job) }, "Normal", EventJobCreated}, {"JobStarted", func(r *OCLEventRecorder) { r.RecordJobStarted(mosb, job) }, "Normal", EventJobStarted}, {"JobCompleted", func(r *OCLEventRecorder) { r.RecordJobCompleted(mosb, job) }, "Normal", EventJobCompleted}, + {"JobPodFailed", func(r *OCLEventRecorder) { r.RecordJobPodFailed(mosb, job, 3) }, "Warning", EventJobPodFailed}, {"JobFailed", func(r *OCLEventRecorder) { r.RecordJobFailed(mosb, job) }, "Warning", EventJobFailed}, {"JobDeleted", func(r *OCLEventRecorder) { r.RecordJobDeleted(mosb, job.Name) }, "Normal", EventJobDeleted}, } diff --git a/pkg/controller/build/reconciler.go b/pkg/controller/build/reconciler.go index dbf150db1a..d11073359e 100644 --- a/pkg/controller/build/reconciler.go +++ b/pkg/controller/build/reconciler.go @@ -259,7 +259,11 @@ func (b *buildReconciler) UpdateJob(ctx context.Context, oldJob, curJob *batchv1 b.eventRecorder.RecordJobCompleted(mosb, curJob) } - if curJob.Status.Failed > 0 && (oldJob.Status.Failed == 0) { + if curJob.Status.Failed > oldJob.Status.Failed && curJob.Status.Failed <= constants.JobMaxRetries { + b.eventRecorder.RecordJobPodFailed(mosb, curJob, constants.JobMaxRetries) + } + + if curJob.Status.Failed > constants.JobMaxRetries && oldJob.Status.Failed <= constants.JobMaxRetries { b.eventRecorder.RecordJobFailed(mosb, curJob) } @@ -614,7 +618,6 @@ func (b *buildReconciler) startBuild(ctx context.Context, mosb *mcfgv1.MachineOS // Retrieves a deep-copy of the MachineOSConfig from the lister so that the cache is not mutated during the update. func (b *buildReconciler) getMachineOSConfigForUpdate(mosc *mcfgv1.MachineOSConfig) (*mcfgv1.MachineOSConfig, error) { out, err := b.machineOSConfigLister.Get(mosc.Name) - if err != nil { return nil, err } @@ -635,7 +638,6 @@ func (b *buildReconciler) getMachineOSBuildForJob(job *batchv1.Job) (*mcfgv1.Mac // Retrieves a deep-copy of the MachineOSBuild from the lister so that the cache is not mutated during the update. func (b *buildReconciler) getMachineOSBuildForUpdate(mosb *mcfgv1.MachineOSBuild) (*mcfgv1.MachineOSBuild, error) { out, err := b.machineOSBuildLister.Get(mosb.Name) - if err != nil { return nil, err } @@ -730,7 +732,6 @@ func (b *buildReconciler) createNewMachineOSBuildOrReuseExisting(ctx context.Con MachineOSConfig: mosc, MachineConfigPool: mcp, }) - if err != nil { return fmt.Errorf("could not instantiate new MachineOSBuild: %w", err) } @@ -1212,7 +1213,6 @@ func (b *buildReconciler) syncAll(ctx context.Context) error { return nil }) - if err != nil { return fmt.Errorf("could not sync all: %w", err) } @@ -1236,7 +1236,6 @@ func (b *buildReconciler) syncMachineOSBuilds(ctx context.Context) error { return nil }) - if err != nil { return fmt.Errorf("could not sync MachineOSBuilds: %w", err) } @@ -1249,7 +1248,6 @@ func (b *buildReconciler) syncMachineOSBuilds(ctx context.Context) error { // builder associated with it that one should be created. func (b *buildReconciler) syncMachineOSBuild(ctx context.Context, mosb *mcfgv1.MachineOSBuild) error { return b.timeObjectOperation(mosb, syncingVerb, func() error { - // It could be the case that the MCP the mosb in queue was targeting no longer is valid mcp, err := b.machineConfigPoolLister.Get(mosb.ObjectMeta.Labels[constants.TargetMachineConfigPoolLabelKey]) if err != nil { @@ -1392,7 +1390,6 @@ func (b *buildReconciler) syncMachineOSConfigs(ctx context.Context) error { return nil }) - if err != nil { return fmt.Errorf("could not sync MachineOSConfigs: %w", err) } @@ -1476,7 +1473,6 @@ func (b *buildReconciler) syncMachineConfigPools(ctx context.Context) error { return nil }) - if err != nil { return fmt.Errorf("could not sync MachineConfigPools: %w", err) } @@ -1612,7 +1608,6 @@ func (b *buildReconciler) reconcilePoolChange(ctx context.Context, mcp *mcfgv1.M return b.reuseImageForNewMOSB(ctx, mosc, oldMOSB) } return b.createNewMachineOSBuildOrReuseExisting(ctx, mosc, needsImageRebuild) - } // reuseImageForNewMOSB creates a new MOSB (for the new rendered-MC name) @@ -1637,7 +1632,6 @@ func (b *buildReconciler) reuseImageForNewMOSB(ctx context.Context, mosc *mcfgv1 MachineOSConfig: mosc, MachineConfigPool: mcp, }) - if err != nil { return err } @@ -1805,7 +1799,6 @@ func (b *buildReconciler) shouldPreventBuildDueToDegradation(mcp *mcfgv1.Machine // reconcileImageRebuild calls RequiresRebuild to see if an MC changes the kernel args, ext, or osimageurl. // if it does, we build a new image in our new MOSB func (b *buildReconciler) reconcileImageRebuild(oldMCP, curMCP *mcfgv1.MachineConfigPool) (bool, error) { - curr, err := b.machineConfigLister.Get(oldMCP.Spec.Configuration.Name) if err != nil { return false, err @@ -1982,7 +1975,6 @@ func (b *buildReconciler) seedMachineOSConfigWithExistingImage(ctx context.Conte MachineConfigPool: mcp, MachineOSConfig: mosc, }) - if err != nil { return fmt.Errorf("could not generate MachineOSBuild template for MachineOSConfig %q: %w", mosc.Name, err) } diff --git a/pkg/controller/drain/drain_controller.go b/pkg/controller/drain/drain_controller.go index 2ed90666a8..916fa97b18 100644 --- a/pkg/controller/drain/drain_controller.go +++ b/pkg/controller/drain/drain_controller.go @@ -9,11 +9,11 @@ import ( v1 "github.com/openshift/api/machineconfiguration/v1" mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned" - "github.com/openshift/client-go/machineconfiguration/clientset/versioned/scheme" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants" "github.com/openshift/machine-config-operator/pkg/helpers" "github.com/openshift/machine-config-operator/pkg/upgrademonitor" + kubescheme "k8s.io/client-go/kubernetes/scheme" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -127,7 +127,7 @@ func New( ctrl := &Controller{ client: mcfgClient, kubeClient: kubeClient, - eventRecorder: ctrlcommon.NamespacedEventRecorder(eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "machineconfigcontroller-nodecontroller"})), + eventRecorder: ctrlcommon.NamespacedEventRecorder(eventBroadcaster.NewRecorder(kubescheme.Scheme, corev1.EventSource{Component: "machineconfigcontroller-nodecontroller"})), queue: workqueue.NewTypedRateLimitingQueueWithConfig( workqueue.DefaultTypedControllerRateLimiter[string](), workqueue.TypedRateLimitingQueueConfig[string]{Name: "machineconfigcontroller-draincontroller"}), diff --git a/pkg/controller/template/render_test.go b/pkg/controller/template/render_test.go index 10e6ab739b..91067167d6 100644 --- a/pkg/controller/template/render_test.go +++ b/pkg/controller/template/render_test.go @@ -216,7 +216,6 @@ var ( "nutanix": "./test_data/controller_config_nutanix.yaml", "gcp-custom-dns": "./test_data/controller_config_gcp_custom_dns.yaml", "gcp-default-dns": "./test_data/controller_config_gcp_default_dns.yaml", - "baremetal-tnf": "./test_data/controller_config_baremetal_tnf.yaml", } ) @@ -352,84 +351,6 @@ func TestGenerateMachineConfigs(t *testing.T) { } } -func TestKubeletGracefulShutdownTNF(t *testing.T) { - cases := []struct { - name string - controllerConfig string - expectGraceful bool - }{ - { - name: "DualReplica has graceful shutdown", - controllerConfig: "./test_data/controller_config_baremetal_tnf.yaml", - expectGraceful: true, - }, - { - name: "HA does not have graceful shutdown", - controllerConfig: "./test_data/controller_config_baremetal.yaml", - expectGraceful: false, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - controllerConfig, err := controllerConfigFromFile(tc.controllerConfig) - if err != nil { - t.Fatalf("failed to load controller config: %v", err) - } - - cfgs, err := generateTemplateMachineConfigs( - &RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, - templateDir, - ) - if err != nil { - t.Fatalf("failed to generate machine configs: %v", err) - } - - found := false - for _, cfg := range cfgs { - role := cfg.Labels[mcfgv1.MachineConfigRoleLabelKey] - if role != masterRole { - continue - } - - ign, err := ctrlcommon.ParseAndConvertConfig(cfg.Spec.Config.Raw) - if err != nil { - t.Fatalf("failed to parse ignition config: %v", err) - } - - for _, file := range ign.Storage.Files { - if file.Path != "/etc/kubernetes/kubelet.conf" { - continue - } - - found = true - contents, err := ctrlcommon.DecodeIgnitionFileContents(file.Contents.Source, file.Contents.Compression) - if err != nil { - t.Fatalf("failed to decode kubelet.conf contents: %v", err) - } - - kubeletConf := string(contents) - if tc.expectGraceful { - if !strings.Contains(kubeletConf, "shutdownGracePeriod: 90s") { - t.Errorf("expected shutdownGracePeriod in kubelet.conf for DualReplica") - } - if !strings.Contains(kubeletConf, "shutdownGracePeriodCriticalPods: 60s") { - t.Errorf("expected shutdownGracePeriodCriticalPods in kubelet.conf for DualReplica") - } - } else { - if strings.Contains(kubeletConf, "shutdownGracePeriod") { - t.Errorf("unexpected shutdownGracePeriod in kubelet.conf for non-DualReplica") - } - } - } - } - if !found { - t.Fatal("kubelet.conf file not found in any master ignition config") - } - }) - } -} - func TestGetPaths(t *testing.T) { APIIntLBIP := configv1.IP("10.10.10.4") APILBIP := configv1.IP("196.78.125.4") diff --git a/pkg/controller/template/test_data/controller_config_baremetal_tnf.yaml b/pkg/controller/template/test_data/controller_config_baremetal_tnf.yaml deleted file mode 100644 index d4d7586561..0000000000 --- a/pkg/controller/template/test_data/controller_config_baremetal_tnf.yaml +++ /dev/null @@ -1,37 +0,0 @@ -apiVersion: "machineconfigurations.openshift.io/v1" -kind: "ControllerConfig" -spec: - clusterDNSIP: "10.3.0.10" - cloudProviderConfig: "" - etcdInitialCount: 2 - etcdCAData: ZHVtbXkgZXRjZC1jYQo= - rootCAData: ZHVtbXkgcm9vdC1jYQo= - pullSecret: - data: ZHVtbXkgZXRjZC1jYQo= - images: - etcd: image/etcd:1 - setupEtcdEnv: image/setupEtcdEnv:1 - infraImage: image/infraImage:1 - kubeClientAgentImage: image/kubeClientAgentImage:1 - infra: - apiVersion: config.openshift.io/v1 - kind: Infrastructure - spec: - cloudConfig: - key: config - name: cloud-provider-config - status: - apiServerInternalURI: https://api-int.my-test-cluster.installer.team.coreos.systems:6443 - apiServerURL: https://api.my-test-cluster.installer.team.coreos.systems:6443 - etcdDiscoveryDomain: my-test-cluster.installer.team.coreos.systems - infrastructureName: my-test-cluster - controlPlaneTopology: DualReplica - platformStatus: - type: "BareMetal" - baremetal: - apiServerInternalIP: 10.0.0.1 - ingressIP: 10.0.0.2 - nodeDNSIP: 10.0.0.3 - dns: - spec: - baseDomain: my-test-cluster.installer.team.coreos.systems diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index d122d341b0..3a510e3e3d 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -2921,22 +2921,26 @@ func (dn *Daemon) updateLayeredOS(config *mcfgv1.MachineConfig) error { // such that if we happen to update while the CoreDNS pod is being restarted, // the next retry should succeed if no other issues are present. backoff := wait.Backoff{ - Duration: 5 * time.Second, + Duration: 10 * time.Second, Factor: 2, - Steps: 5, + Steps: 7, + Cap: 60 * time.Second, } + var lastRebaseErr error if err := wait.ExponentialBackoff(backoff, func() (bool, error) { if err := dn.NodeUpdaterClient.RebaseLayered(newURL); err != nil { klog.Warningf("Failed to update OS to %s (will retry): %v", newURL, err) + lastRebaseErr = err return false, nil } return true, nil }); err != nil { + rebaseErr := fmt.Errorf("failed to update OS to %s after retries: %v: %w", newURL, lastRebaseErr, err) // Report ImagePulledFromRegistry condition as false (failed) if imageModeStatusReportingEnabled { mcnErr := upgrademonitor.GenerateAndApplyMachineConfigNodes( - &upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Failed to pull OS image %s from registry: %v", newURL, err)}, + &upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Failed to pull OS image %s from registry: %v", newURL, rebaseErr)}, nil, metav1.ConditionFalse, metav1.ConditionFalse, @@ -2949,7 +2953,7 @@ func (dn *Daemon) updateLayeredOS(config *mcfgv1.MachineConfig) error { klog.Errorf("Error setting ImagePulledFromRegistry condition to false: %v", mcnErr) } } - return fmt.Errorf("failed to update OS to %s after retries: %w", newURL, err) + return rebaseErr } // Report ImagePulledFromRegistry condition as true (success) diff --git a/pkg/imageutils/sys_context.go b/pkg/imageutils/sys_context.go index 53e261da22..919840e137 100644 --- a/pkg/imageutils/sys_context.go +++ b/pkg/imageutils/sys_context.go @@ -30,6 +30,7 @@ type SysContextBuilder struct { secret *corev1.Secret controllerConfig *mcfgv1.ControllerConfig registriesConfig *sysregistriesv2.V2RegistriesConf + skipProxy bool } // NewSysContextBuilder creates a new SysContextBuilder for building SysContext instances. @@ -49,6 +50,12 @@ func (b *SysContextBuilder) WithControllerConfig(cc *mcfgv1.ControllerConfig) *S return b } +// WithoutProxy disables proxy configuration even if the ControllerConfig has one. +func (b *SysContextBuilder) WithoutProxy() *SysContextBuilder { + b.skipProxy = true + return b +} + // WithRegistriesConfig adds custom container registry configuration to the SysContext. // The registries config will be written as a TOML file and used for registry lookups, // mirrors, and pull policies. @@ -157,7 +164,7 @@ func (b *SysContextBuilder) buildRegistries(sysContext *SysContext) error { // Prioritizes HTTPS proxy over HTTP proxy when both are configured. // Returns early if no controller config was provided or no proxy is configured. func (b *SysContextBuilder) buildProxy(sysContext *SysContext) error { - if b.controllerConfig == nil { + if b.controllerConfig == nil || b.skipProxy { return nil } // TODO: Improve when containers-libs is used with https://github.com/containers/container-libs/pull/583 diff --git a/pkg/imageutils/sys_context_test.go b/pkg/imageutils/sys_context_test.go index 33bfe08632..36c39c8d1d 100644 --- a/pkg/imageutils/sys_context_test.go +++ b/pkg/imageutils/sys_context_test.go @@ -320,6 +320,7 @@ func TestSysContextBuilderWithProxy(t *testing.T) { name string httpProxy string httpsProxy string + skipProxy bool expectedScheme string expectedHost string expectedUsername string @@ -376,6 +377,12 @@ func TestSysContextBuilderWithProxy(t *testing.T) { expectedUsername: "user", expectedPassword: "p@ssw0rd!", }, + { + name: "WithoutProxy skips proxy even when configured", + httpsProxy: "https://proxy.example.com:3128", + httpProxy: "http://proxy.example.com:8080", + skipProxy: true, + }, } for _, tc := range testCases { @@ -389,35 +396,41 @@ func TestSysContextBuilderWithProxy(t *testing.T) { }, } - sysCtx, err := NewSysContextBuilder(). + builder := NewSysContextBuilder(). WithSecret(secret). - WithControllerConfig(cc). - Build() + WithControllerConfig(cc) + if tc.skipProxy { + builder.WithoutProxy() + } + + sysCtx, err := builder.Build() require.NoError(t, err, "SysContextBuilder.Build should not fail") require.NotNil(t, sysCtx, "SysContext wrapper should not be nil") require.NotNil(t, sysCtx.SysContext, "Underlying SystemContext should not be nil") - // Check that proxy was set correctly - require.NotNil(t, sysCtx.SysContext.DockerProxyURL, "DockerProxyURL should not be nil") - assert.Equal(t, tc.expectedScheme, sysCtx.SysContext.DockerProxyURL.Scheme, "Proxy scheme should match") - assert.Equal(t, tc.expectedHost, sysCtx.SysContext.DockerProxyURL.Host, "Proxy host should match") + if tc.skipProxy { + assert.Nil(t, sysCtx.SysContext.DockerProxyURL, "DockerProxyURL should be nil when proxy is skipped") + } else { + require.NotNil(t, sysCtx.SysContext.DockerProxyURL, "DockerProxyURL should not be nil") + assert.Equal(t, tc.expectedScheme, sysCtx.SysContext.DockerProxyURL.Scheme, "Proxy scheme should match") + assert.Equal(t, tc.expectedHost, sysCtx.SysContext.DockerProxyURL.Host, "Proxy host should match") - // Check username and password if provided - if tc.expectedUsername != "" { - assert.NotNil(t, sysCtx.SysContext.DockerProxyURL.User, "Proxy user info should not be nil") - assert.Equal(t, tc.expectedUsername, sysCtx.SysContext.DockerProxyURL.User.Username(), "Proxy username should match") - } + if tc.expectedUsername != "" { + assert.NotNil(t, sysCtx.SysContext.DockerProxyURL.User, "Proxy user info should not be nil") + assert.Equal(t, tc.expectedUsername, sysCtx.SysContext.DockerProxyURL.User.Username(), "Proxy username should match") + } - if tc.expectedPassword != "" { - assert.NotNil(t, sysCtx.SysContext.DockerProxyURL.User, "Proxy user info should not be nil") - password, hasPassword := sysCtx.SysContext.DockerProxyURL.User.Password() - assert.True(t, hasPassword, "Proxy should have password") - assert.Equal(t, tc.expectedPassword, password, "Proxy password should match") - } + if tc.expectedPassword != "" { + assert.NotNil(t, sysCtx.SysContext.DockerProxyURL.User, "Proxy user info should not be nil") + password, hasPassword := sysCtx.SysContext.DockerProxyURL.User.Password() + assert.True(t, hasPassword, "Proxy should have password") + assert.Equal(t, tc.expectedPassword, password, "Proxy password should match") + } - if tc.expectedUsername == "" && tc.expectedPassword == "" { - if sysCtx.SysContext.DockerProxyURL.User != nil { - assert.Empty(t, sysCtx.SysContext.DockerProxyURL.User.Username(), "Proxy username should be empty") + if tc.expectedUsername == "" && tc.expectedPassword == "" { + if sysCtx.SysContext.DockerProxyURL.User != nil { + assert.Empty(t, sysCtx.SysContext.DockerProxyURL.User.Username(), "Proxy username should be empty") + } } } diff --git a/templates/common/cloud-platform-alt-dns/files/coredns.yaml b/templates/common/cloud-platform-alt-dns/files/coredns.yaml index 3cbf2213ce..09acf0ab15 100644 --- a/templates/common/cloud-platform-alt-dns/files/coredns.yaml +++ b/templates/common/cloud-platform-alt-dns/files/coredns.yaml @@ -47,7 +47,7 @@ contents: - "--out-dir" - "/etc/coredns" - "--resolvconf-path" - - "/etc/resolv.conf" + - "/var/run/NetworkManager/resolv.conf" resources: {} volumeMounts: - name: kubeconfig @@ -59,6 +59,10 @@ contents: - name: conf-dir mountPath: "/etc/coredns" mountPropagation: HostToContainer + - name: nm-resolv + mountPath: "/var/run/NetworkManager" + mountPropagation: HostToContainer + readOnly: true imagePullPolicy: IfNotPresent terminationMessagePolicy: FallbackToLogsOnError containers: @@ -86,8 +90,6 @@ contents: terminationMessagePolicy: FallbackToLogsOnError imagePullPolicy: IfNotPresent - name: coredns-monitor - securityContext: - privileged: true image: {{ .Images.baremetalRuntimeCfgImage }} command: - corednsmonitor diff --git a/templates/master/01-master-kubelet/_base/files/kubelet.yaml b/templates/master/01-master-kubelet/_base/files/kubelet.yaml index ff9ba34091..e536c6f63f 100644 --- a/templates/master/01-master-kubelet/_base/files/kubelet.yaml +++ b/templates/master/01-master-kubelet/_base/files/kubelet.yaml @@ -36,7 +36,3 @@ contents: {{- range .TLSCipherSuites }} - {{ . }} {{- end }} - {{- if eq .Infra.Status.ControlPlaneTopology "DualReplica" }} - shutdownGracePeriod: 90s - shutdownGracePeriodCriticalPods: 60s - {{- end }} 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.