diff --git a/api/v1alpha1/managedcloudprofile.go b/api/v1alpha1/managedcloudprofile.go index f13109e..fbe725a 100644 --- a/api/v1alpha1/managedcloudprofile.go +++ b/api/v1alpha1/managedcloudprofile.go @@ -13,6 +13,12 @@ type ManagedCloudProfileSpec struct { // CloudProfile contains the base spec of the CloudProfile. CloudProfile CloudProfileSpec `json:"cloudProfile"` + // MachineImagesPaused disables automatic machine image updates and keeps the + // existing CloudProfile machine images and provider config unchanged. Other + // updates (e.g. Kubernetes versions and base spec fields) still apply. + // +optional + MachineImagesPaused bool `json:"machineImagesPaused,omitempty"` + // MachineImageUpdates contains the source and provider information to automate machine images. // +optional MachineImageUpdates []MachineImageUpdate `json:"machineImageUpdates,omitempty"` @@ -100,9 +106,6 @@ type MachineImageUpdate struct { // ImageName is the name of the image to maintain automatically ImageName string `json:"imageName"` - // Paused disables automatic updates for this image and keeps the existing CloudProfile machine images. - // +optional - Paused bool `json:"paused,omitempty"` } // ImageFilter defines admission criteria for source images. diff --git a/controllers/cloud_profile.go b/controllers/cloud_profile.go index 74501a4..b7f6e43 100644 --- a/controllers/cloud_profile.go +++ b/controllers/cloud_profile.go @@ -6,13 +6,13 @@ import ( "context" "errors" "fmt" - "slices" gardenerv1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -42,29 +42,39 @@ func (r *Reconciler) reconcileCloudProfile(ctx context.Context, log logr.Logger, if err := controllerutil.SetControllerReference(mcp, &cloudProfile, r.Scheme()); err != nil { return err } + // When machine image updates are paused, preserve the existing machine + // images and provider config (which carries the runtime-discovered region + // image IDs) across the base-spec reset below. Other updates still apply. + machineImagesPaused := mcp.Spec.MachineImagesPaused + var storedMachineImages []gardenerv1beta1.MachineImage + var storedProviderConfig *runtime.RawExtension + if machineImagesPaused { + storedMachineImages = cloudProfile.Spec.MachineImages + storedProviderConfig = cloudProfile.Spec.ProviderConfig + } storedExpirations := collectExpirationDates(cloudProfile.Spec.MachineImages) - storedImages := collectMachineImages(cloudProfile.Spec.MachineImages) cloudProfile.Spec = CloudProfileSpecToGardener(&mcp.Spec.CloudProfile) errs := make([]error, 0) - for _, updates := range mcp.Spec.MachineImageUpdates { - if updates.Paused { - log.V(1).Info("machine image update paused, keeping existing images", "cloudProfile", cloudProfile.Name, "imageName", updates.ImageName) - if img, ok := storedImages[updates.ImageName]; ok { - // Replace any entry the MCP spec contributed for this image so the - // stored (previously reconciled) versions are kept without duplicating. - cloudProfile.Spec.MachineImages = slices.DeleteFunc(cloudProfile.Spec.MachineImages, func(m gardenerv1beta1.MachineImage) bool { - return m.Name == updates.ImageName - }) - cloudProfile.Spec.MachineImages = append(cloudProfile.Spec.MachineImages, img) - } - continue + if machineImagesPaused { + log.V(1).Info("machine image updates paused, keeping existing machine images and provider config", "cloudProfile", cloudProfile.Name) + // Restore the previously synced machine images and provider config so the + // paused reconcile does not wipe them. On first creation nothing is stored + // yet, so fall back to the base-spec values instead of clearing them. + if len(storedMachineImages) > 0 { + cloudProfile.Spec.MachineImages = storedMachineImages } - log.V(1).Info("updating machine images", "cloudProfile", cloudProfile.Name) - if updateErr := r.updateMachineImages(ctx, log, updates, &cloudProfile.Spec); updateErr != nil { - errs = append(errs, updateErr) + if storedProviderConfig != nil { + cloudProfile.Spec.ProviderConfig = storedProviderConfig + } + } else { + for _, updates := range mcp.Spec.MachineImageUpdates { + log.V(1).Info("updating machine images", "cloudProfile", cloudProfile.Name) + if updateErr := r.updateMachineImages(ctx, log, updates, &cloudProfile.Spec); updateErr != nil { + errs = append(errs, updateErr) + } } + applyExpirationDates(cloudProfile.Spec.MachineImages, storedExpirations) } - applyExpirationDates(cloudProfile.Spec.MachineImages, storedExpirations) if mcp.Spec.KubernetesUpdate != nil { log.V(1).Info("updating kubernetes versions", "cloudProfile", cloudProfile.Name) if updateErr := r.updateKubernetesVersions(ctx, *mcp.Spec.KubernetesUpdate, &cloudProfile.Spec); updateErr != nil { @@ -247,14 +257,6 @@ func expirationDateKey(imageName, version string) string { return imageName + "/" + version } -func collectMachineImages(images []gardenerv1beta1.MachineImage) map[string]gardenerv1beta1.MachineImage { - out := make(map[string]gardenerv1beta1.MachineImage, len(images)) - for _, img := range images { - out[img.Name] = *img.DeepCopy() - } - return out -} - func collectExpirationDates(images []gardenerv1beta1.MachineImage) map[string]*metav1.Time { out := make(map[string]*metav1.Time) for _, img := range images { diff --git a/controllers/garbage_collection.go b/controllers/garbage_collection.go index c827bc3..ffd7a7f 100644 --- a/controllers/garbage_collection.go +++ b/controllers/garbage_collection.go @@ -59,6 +59,11 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, mcp *v1alph if mcp.Spec.GarbageCollection == nil || !mcp.Spec.GarbageCollection.Enabled { return nil } + // Garbage collection only ever deletes machine image versions and rewrites the + // provider config mappings, so honor the machine image pause here too. + if mcp.Spec.MachineImagesPaused { + return nil + } if mcp.Spec.GarbageCollection.MaxAge.Duration < 0 { return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("invalid garbage collection maxAge: %s", mcp.Spec.GarbageCollection.MaxAge.String())) } diff --git a/controllers/managedcloudprofile_controller_test.go b/controllers/managedcloudprofile_controller_test.go index 10a3b21..8a0b23e 100644 --- a/controllers/managedcloudprofile_controller_test.go +++ b/controllers/managedcloudprofile_controller_test.go @@ -328,11 +328,44 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { Expect(k8sClient.Delete(ctx, cloudProfile)).To(Succeed()) }) - It("keeps existing images when the update is paused", func(ctx SpecContext) { + It("keeps existing images when machine image updates are paused", func(ctx SpecContext) { keptVersion := "4242.0.0" + // Simulate a previously reconciled CloudProfile whose ProviderConfig already + // carries the runtime-discovered image mappings. These live only in the + // CloudProfile (not the MCP), so a paused reconcile must preserve them. + var cloudProfile gardenerv1beta1.CloudProfile + cloudProfile.Name = "test-paused" + cloudProfile.Spec.Regions = []gardenerv1beta1.Region{{Name: "foo"}} + cloudProfile.Spec.MachineTypes = []gardenerv1beta1.MachineType{{Name: "baz"}} + cloudProfile.Spec.MachineImages = []gardenerv1beta1.MachineImage{ + { + Name: "the-image", + Versions: []gardenerv1beta1.MachineImageVersion{ + {Version: keptVersion, Architectures: []string{"amd64"}}, + }, + }, + } + var storedCfg providercfg.CloudProfileConfig + storedCfg.MachineImages = []providercfg.MachineImages{ + { + Name: "the-image", + Versions: []providercfg.MachineImageVersion{ + {Image: "repo/the-image:" + keptVersion}, + }, + }, + } + storedRaw, err := json.Marshal(storedCfg) + Expect(err).To(Succeed()) + cloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: storedRaw} + Expect(k8sClient.Create(ctx, &cloudProfile)).To(Succeed()) + var mcp v1alpha1.ManagedCloudProfile mcp.Name = "test-paused" + mcp.Spec.MachineImagesPaused = true + // The MCP itself carries no provider machineImages (mirrors production): the + // mappings are discovered at runtime, so if pause did not restore the stored + // ProviderConfig it would be wiped to an empty list. mcp.Spec.CloudProfile = baseCloudProfileSpec( gardenerv1beta1.MachineImage{ Name: "the-image", @@ -357,7 +390,6 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { }, }, ImageName: "the-image", - Paused: true, }, } Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) @@ -365,18 +397,89 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { expectReconcileStatus(ctx, &mcp, v1alpha1.SucceededReconcileStatus) expectAppliedCondition(&mcp, metav1.ConditionTrue) - cloudProfile := getCloudProfile(ctx, mcp.Name) - mi := cloudProfile.Spec.MachineImages + updated := getCloudProfile(ctx, mcp.Name) + mi := updated.Spec.MachineImages Expect(mi).To(HaveLen(1)) Expect(mi[0].Name).To(Equal("the-image")) - // The updater was skipped, so the pre-existing version is kept and the OCI - // source versions (1.0.0, 1.0.1+abc) were never fetched. + // The updater was skipped, so the base version is kept and the OCI source + // versions (1.0.0, 1.0.1+abc) were never fetched. vers := mi[0].Versions Expect(vers).To(HaveLen(1)) Expect(vers[0].Version).To(Equal(keptVersion)) + // The stored ProviderConfig mappings must survive the paused reconcile rather + // than being wiped to an empty list. + Expect(updated.Spec.ProviderConfig).ToNot(BeNil()) + var gotCfg providercfg.CloudProfileConfig + Expect(json.Unmarshal(updated.Spec.ProviderConfig.Raw, &gotCfg)).To(Succeed()) + Expect(gotCfg.MachineImages).To(HaveLen(1)) + Expect(gotCfg.MachineImages[0].Name).To(Equal("the-image")) + Expect(gotCfg.MachineImages[0].Versions).To(HaveLen(1)) + Expect(gotCfg.MachineImages[0].Versions[0].Image).To(Equal("repo/the-image:" + keptVersion)) + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) - Expect(k8sClient.Delete(ctx, cloudProfile)).To(Succeed()) + Expect(k8sClient.Delete(ctx, updated)).To(Succeed()) + }) + + It("preserves images and provider config but still applies base-spec changes when paused", func(ctx SpecContext) { + var mcp v1alpha1.ManagedCloudProfile + mcp.Name = "test-paused-preserve" + mcp.Spec.CloudProfile = baseCloudProfileSpec() + mcp.Spec.MachineImageUpdates = []v1alpha1.MachineImageUpdate{ + { + Source: v1alpha1.MachineImageUpdateSource{ + OCI: &v1alpha1.OCI{ + Registry: registryAddr, + Repository: orasRepoName("repo"), + Insecure: true, + }, + }, + Provider: v1alpha1.MachineImageUpdateProvider{ + IroncoreMetal: &v1alpha1.MachineImagesUpdateProviderIroncoreMetal{ + Registry: registryAddr, + Repository: orasRepoName("repo"), + }, + }, + ImageName: "the-image", + }, + } + Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) + expectReconcileStatus(ctx, &mcp, v1alpha1.SucceededReconcileStatus) + + // Snapshot what the first (unpaused) reconcile produced. + before := getCloudProfile(ctx, mcp.Name) + wantImages := before.Spec.MachineImages + wantProviderConfig := before.Spec.ProviderConfig + + // Pause image updates, remove the update source, and also change a base-spec + // field (add a machine type). The paused reconcile must keep the machine + // images and provider config untouched while still applying the base-spec change. + Eventually(func(g Gomega) { + var latest v1alpha1.ManagedCloudProfile + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&mcp), &latest)).To(Succeed()) + latest.Spec.MachineImagesPaused = true + latest.Spec.MachineImageUpdates = nil + usable := true + latest.Spec.CloudProfile.MachineTypes = append(latest.Spec.CloudProfile.MachineTypes, gardenerv1beta1.MachineType{ + Name: "extra", + Architecture: &amd64, + Usable: &usable, + }) + g.Expect(k8sClient.Update(ctx, &latest)).To(Succeed()) + }).Should(Succeed()) + + // Wait until the base-spec change lands, then assert images/provider config are preserved. + Eventually(func(g Gomega) { + after := getCloudProfile(ctx, mcp.Name) + g.Expect(after.Spec.MachineTypes).To(ContainElement(HaveField("Name", "extra"))) + }).Should(Succeed()) + + after := getCloudProfile(ctx, mcp.Name) + Expect(after.Spec.MachineImages).To(Equal(wantImages)) + Expect(after.Spec.ProviderConfig).To(Equal(wantProviderConfig)) + + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) + Expect(k8sClient.Delete(ctx, after)).To(Succeed()) }) It("fetches a secret for the OCI source", func(ctx SpecContext) { @@ -523,6 +626,75 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { Should(ConsistOf(newVersion)) }) + It("skips garbage collection when machine image updates are paused", func(ctx SpecContext) { + var mcp v1alpha1.ManagedCloudProfile + mcp.Name = "gc-paused-mcp" + + oldVersion := "0.1.0" + newVersion := "1.0.0" + + mcp.Spec.MachineImagesPaused = true + mcp.Spec.CloudProfile = baseCloudProfileSpec( + gardenerv1beta1.MachineImage{ + Name: "gc-image", + Versions: []gardenerv1beta1.MachineImageVersion{ + {Version: oldVersion, Architectures: []string{"amd64"}}, + {Version: newVersion, Architectures: []string{"amd64"}}, + }, + }, + ) + mcp.Spec.MachineImageUpdates = []v1alpha1.MachineImageUpdate{ + { + ImageName: "gc-image", + Source: v1alpha1.MachineImageUpdateSource{ + OCI: &v1alpha1.OCI{ + Registry: "keppel-fake", + Repository: "account/repo", + Insecure: true, + }, + }, + Provider: v1alpha1.MachineImageUpdateProvider{ + IroncoreMetal: &v1alpha1.MachineImagesUpdateProviderIroncoreMetal{ + Registry: "keppel-fake", + Repository: "account/repo", + }, + }, + }, + } + mcp.Spec.GarbageCollection = &v1alpha1.GarbageCollectionConfig{ + Enabled: true, + MaxAge: metav1.Duration{Duration: 24 * time.Hour}, + } + Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) + + reconciler := &controllers.Reconciler{ + Client: k8sClient, + OCISourceFactory: &fakeFactory{}, + RegistryProviderFunc: func(registry string) (controllers.RegistryClient, error) { + return &fakeRegistryClient{}, nil + }, + } + + _, err := reconciler.Reconcile(ctx, ctrl.Request{Name: mcp.Name}) + Expect(err).ToNot(HaveOccurred()) + + // Wait for the CloudProfile to exist with both versions, then confirm GC + // never removes the unreferenced, stale oldVersion while paused. + Eventually(func(g Gomega) []string { + var cp gardenerv1beta1.CloudProfile + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: mcp.Name}, &cp)).To(Succeed()) + return versionsByMachineImage(&cp, "gc-image") + }, 10*time.Second, 200*time.Millisecond). + Should(ConsistOf(oldVersion, newVersion)) + + Consistently(func(g Gomega) []string { + var cp gardenerv1beta1.CloudProfile + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: mcp.Name}, &cp)).To(Succeed()) + return versionsByMachineImage(&cp, "gc-image") + }, 2*time.Second, 200*time.Millisecond). + Should(ConsistOf(oldVersion, newVersion)) + }) + It("preserves old machine image versions referenced by Shoot worker pools", func(ctx SpecContext) { version := "1.0.0" diff --git a/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml b/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml index 4ee9fb7..af5db78 100644 --- a/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml +++ b/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml @@ -696,10 +696,6 @@ spec: description: ImageName is the name of the image to maintain automatically type: string - paused: - description: Paused disables automatic updates for this image - and keeps the existing CloudProfile machine images. - type: boolean provider: description: Provider contains configuration for a provider for machine images. @@ -870,6 +866,12 @@ spec: - source type: object type: array + machineImagesPaused: + description: |- + MachineImagesPaused disables automatic machine image updates and keeps the + existing CloudProfile machine images and provider config unchanged. Other + updates (e.g. Kubernetes versions and base spec fields) still apply. + type: boolean required: - cloudProfile type: object