From c1a6db9bc61c7454698e39360c5cd71c36b575ac Mon Sep 17 00:00:00 2001 From: C5412626 Date: Wed, 4 Mar 2026 11:35:15 +0100 Subject: [PATCH 01/14] implemented an automated cleanup of old and unused images --- api/v1alpha1/managedcloudprofile.go | 13 +++ api/v1alpha1/zz_generated.deepcopy.go | 21 ++++ cloudprofilesync/source.go | 13 +++ controllers/managedcloudprofile_controller.go | 95 +++++++++++++++++++ ...c.cobaltcore.dev_managedcloudprofiles.yaml | 16 +++- 5 files changed, 157 insertions(+), 1 deletion(-) diff --git a/api/v1alpha1/managedcloudprofile.go b/api/v1alpha1/managedcloudprofile.go index d1672a5..1622679 100644 --- a/api/v1alpha1/managedcloudprofile.go +++ b/api/v1alpha1/managedcloudprofile.go @@ -97,6 +97,19 @@ type MachineImageUpdate struct { // ImagesName is the name of the image to maintain automatically ImageName string `json:"imageName"` + // GarbageCollection contains configuration for automated garbage collection + // +optional + GarbageCollection *GarbageCollectionConfig `json:"garbageCollection,omitempty"` +} + +type GarbageCollectionConfig struct { + // Enabled toggles garbage collection for this image. + // +optional + Enabled bool `json:"enabled,omitempty"` + // MaxAge defines the maximum age for images to keep. Images older than + // now - MaxAge are eligible for deletion. + // +optional + MaxAge metav1.Duration `json:"maxAge,omitempty"` } type MachineImageUpdateSource struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 5a4e2a9..f73ee13 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -86,11 +86,32 @@ func (in *CloudProfileSpec) DeepCopy() *CloudProfileSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GarbageCollectionConfig) DeepCopyInto(out *GarbageCollectionConfig) { + *out = *in + out.MaxAge = in.MaxAge +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GarbageCollectionConfig. +func (in *GarbageCollectionConfig) DeepCopy() *GarbageCollectionConfig { + if in == nil { + return nil + } + out := new(GarbageCollectionConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachineImageUpdate) DeepCopyInto(out *MachineImageUpdate) { *out = *in in.Source.DeepCopyInto(&out.Source) in.Provider.DeepCopyInto(&out.Provider) + if in.GarbageCollection != nil { + in, out := &in.GarbageCollection, &out.GarbageCollection + *out = new(GarbageCollectionConfig) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineImageUpdate. diff --git a/cloudprofilesync/source.go b/cloudprofilesync/source.go index f9af796..cab9ade 100644 --- a/cloudprofilesync/source.go +++ b/cloudprofilesync/source.go @@ -8,6 +8,7 @@ import ( "encoding/json" "errors" "strings" + "time" "golang.org/x/sync/semaphore" "oras.land/oras-go/v2/registry/remote" @@ -23,6 +24,7 @@ type Result[T any] struct { type SourceImage struct { Version string Architectures []string + CreatedAt time.Time } type Source interface { @@ -104,10 +106,21 @@ func (o *OCI) GetVersions(ctx context.Context) ([]SourceImage, error) { out <- Result[SourceImage]{err: errors.New("architecture annotation not found in descriptor")} return } + created := time.Time{} + if s, ok := manifest.Annotations["org.opencontainers.image.created"]; ok { + if t, err := time.Parse(time.RFC3339, s); err == nil { + created = t + } + } else if s, ok := manifest.Annotations["created"]; ok { + if t, err := time.Parse(time.RFC3339, s); err == nil { + created = t + } + } out <- Result[SourceImage]{ value: SourceImage{ Version: strings.ReplaceAll(tag, "_", "+"), // Follow the helm convention Architectures: []string{arch}, + CreatedAt: created, }, } }() diff --git a/controllers/managedcloudprofile_controller.go b/controllers/managedcloudprofile_controller.go index 2c46a64..f0ad89b 100644 --- a/controllers/managedcloudprofile_controller.go +++ b/controllers/managedcloudprofile_controller.go @@ -5,13 +5,16 @@ package controllers import ( "context" + "encoding/json" "errors" "fmt" "slices" + "strings" "time" gardenerv1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" "github.com/go-logr/logr" + providercfg "github.com/ironcore-dev/gardener-extension-provider-ironcore-metal/pkg/apis/metal/v1alpha1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -84,9 +87,101 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } } log.Info("reconciled ManagedCloudProfile") + for _, updates := range mcp.Spec.MachineImageUpdates { + if updates.GarbageCollection == nil || !updates.GarbageCollection.Enabled { + continue + } + var source cloudprofilesync.Source + switch { + case updates.Source.OCI != nil: + password, err := r.getCredential(ctx, updates.Source.OCI.Password) + if err != nil { + return ctrl.Result{}, err + } + oci, err := cloudprofilesync.NewOCI(cloudprofilesync.OCIParams{ + Registry: updates.Source.OCI.Registry, + Repository: updates.Source.OCI.Repository, + Username: updates.Source.OCI.Username, + Password: string(password), + Parallel: 1, + }, updates.Source.OCI.Insecure) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to initialize oci source for garbage collection: %w", err) + } + source = oci + default: + continue + } + + versions, err := source.GetVersions(ctx) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to list source versions for garbage collection: %w", err) + } + cutoff := time.Now().Add(-updates.GarbageCollection.MaxAge.Duration) + for _, v := range versions { + if v.CreatedAt.IsZero() { + continue + } + if v.CreatedAt.Before(cutoff) { + if err := r.deleteVersion(ctx, mcp.Name, updates.ImageName, v.Version); err != nil { + if apierrors.IsInvalid(err) { + log.V(1).Info("garbage collection validation failed, skipping", "image", updates.ImageName, "version", v.Version) + continue + } + return ctrl.Result{}, fmt.Errorf("failed to delete image version: %w", err) + } + log.Info("deleted image version from CloudProfile", "image", updates.ImageName, "version", v.Version) + } + } + } + return ctrl.Result{RequeueAfter: 5 * time.Minute}, nil } +func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageName, version string) error { + var cp gardenerv1beta1.CloudProfile + if err := r.Get(ctx, types.NamespacedName{Name: cloudProfileName}, &cp); err != nil { + return err + } + for i := range cp.Spec.MachineImages { + if cp.Spec.MachineImages[i].Name != imageName { + continue + } + newVersions := make([]gardenerv1beta1.MachineImageVersion, 0, len(cp.Spec.MachineImages[i].Versions)) + for _, mv := range cp.Spec.MachineImages[i].Versions { + if mv.Version == version { + continue + } + newVersions = append(newVersions, mv) + } + cp.Spec.MachineImages[i].Versions = newVersions + } + if cp.Spec.ProviderConfig != nil { + var cfg providercfg.CloudProfileConfig + if err := json.Unmarshal(cp.Spec.ProviderConfig.Raw, &cfg); err == nil { + for i := range cfg.MachineImages { + if cfg.MachineImages[i].Name != imageName { + continue + } + new := make([]providercfg.MachineImageVersion, 0, len(cfg.MachineImages[i].Versions)) + for _, mv := range cfg.MachineImages[i].Versions { + if strings.HasSuffix(mv.Image, ":"+version) { + continue + } + new = append(new, mv) + } + cfg.MachineImages[i].Versions = new + } + raw, err := json.Marshal(cfg) + if err == nil { + cp.Spec.ProviderConfig.Raw = raw + } + } + } + + return r.Update(ctx, &cp) +} + func (r *Reconciler) updateMachineImages(ctx context.Context, log logr.Logger, update v1alpha1.MachineImageUpdate, cpSpec *gardenerv1beta1.CloudProfileSpec) error { var source cloudprofilesync.Source switch { diff --git a/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml b/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml index 7cc0252..b01807a 100644 --- a/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml +++ b/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.18.0 + controller-gen.kubebuilder.io/version: v0.20.0 name: managedcloudprofiles.cloudprofilesync.cobaltcore.dev spec: group: cloudprofilesync.cobaltcore.dev @@ -521,6 +521,20 @@ spec: information to automate machine images. items: properties: + garbageCollection: + description: GarbageCollection contains configuration for automated + garbage collection + properties: + enabled: + description: Enabled toggles garbage collection for this + image. + type: boolean + maxAge: + description: |- + MaxAge defines the maximum age for images to keep. Images older than + now MaxAge are eligible for deletion. + type: string + type: object imageName: description: ImagesName is the name of the image to maintain automatically From 4c020fac9739a503e6632fcfd98c342549dafd45 Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Thu, 5 Mar 2026 14:45:05 +0100 Subject: [PATCH 02/14] fixed linter issues --- .github/workflows/checks.yaml | 2 +- .github/workflows/ci.yaml | 4 ++-- .github/workflows/codeql.yaml | 2 +- cloudprofilesync/source.go | 2 +- controllers/managedcloudprofile_controller.go | 8 ++++---- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 07e2d39..5cfabb0 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -30,7 +30,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.5 + go-version: 1.25.7 - name: Run golangci-lint uses: golangci/golangci-lint-action@v9 with: diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 9da8f1d..4f96519 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -32,7 +32,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.5 + go-version: 1.25.7 - name: Build all binaries run: make build-all code_coverage: @@ -65,7 +65,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.5 + go-version: 1.25.7 - name: Run tests and generate coverage report run: make build/cover.out - name: Archive code coverage results diff --git a/.github/workflows/codeql.yaml b/.github/workflows/codeql.yaml index f84b5c5..6081cf9 100644 --- a/.github/workflows/codeql.yaml +++ b/.github/workflows/codeql.yaml @@ -32,7 +32,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.5 + go-version: 1.25.7 - name: Initialize CodeQL uses: github/codeql-action/init@v4 with: diff --git a/cloudprofilesync/source.go b/cloudprofilesync/source.go index cab9ade..5d8f9d3 100644 --- a/cloudprofilesync/source.go +++ b/cloudprofilesync/source.go @@ -40,7 +40,7 @@ type OCIParams struct { Registry string `json:"registry"` Repository string `json:"repository"` Username string `json:"username"` - Password string `json:"password"` + Password string `json:"password"` //nolint:gosec,nolintlint Parallel int64 `json:"parallel"` } diff --git a/controllers/managedcloudprofile_controller.go b/controllers/managedcloudprofile_controller.go index f0ad89b..1305871 100644 --- a/controllers/managedcloudprofile_controller.go +++ b/controllers/managedcloudprofile_controller.go @@ -106,7 +106,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu Parallel: 1, }, updates.Source.OCI.Insecure) if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to initialize oci source for garbage collection: %w", err) + return ctrl.Result{}, fmt.Errorf("failed to initialize OCI source for garbage collection: %w", err) } source = oci default: @@ -163,14 +163,14 @@ func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageN if cfg.MachineImages[i].Name != imageName { continue } - new := make([]providercfg.MachineImageVersion, 0, len(cfg.MachineImages[i].Versions)) + filtered := make([]providercfg.MachineImageVersion, 0, len(cfg.MachineImages[i].Versions)) for _, mv := range cfg.MachineImages[i].Versions { if strings.HasSuffix(mv.Image, ":"+version) { continue } - new = append(new, mv) + filtered = append(filtered, mv) } - cfg.MachineImages[i].Versions = new + cfg.MachineImages[i].Versions = filtered } raw, err := json.Marshal(cfg) if err == nil { From 2700a2f83e385e595791df539b1a0900989dbfa6 Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Mon, 9 Mar 2026 15:12:58 +0100 Subject: [PATCH 03/14] added protection for referenced images --- controllers/managedcloudprofile_controller.go | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/controllers/managedcloudprofile_controller.go b/controllers/managedcloudprofile_controller.go index 1305871..df74f01 100644 --- a/controllers/managedcloudprofile_controller.go +++ b/controllers/managedcloudprofile_controller.go @@ -117,11 +117,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if err != nil { return ctrl.Result{}, fmt.Errorf("failed to list source versions for garbage collection: %w", err) } + + // Get currently referenced versions in CloudProfile + referencedVersions := r.getReferencedVersions(ctx, mcp.Name, updates.ImageName) + cutoff := time.Now().Add(-updates.GarbageCollection.MaxAge.Duration) for _, v := range versions { if v.CreatedAt.IsZero() { continue } + // Skip deletion if version is currently referenced + if _, isReferenced := referencedVersions[v.Version]; isReferenced { + continue + } if v.CreatedAt.Before(cutoff) { if err := r.deleteVersion(ctx, mcp.Name, updates.ImageName, v.Version); err != nil { if apierrors.IsInvalid(err) { @@ -182,6 +190,47 @@ func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageN return r.Update(ctx, &cp) } +// getReferencedVersions returns a map of currently referenced image versions in the CloudProfile +func (r *Reconciler) getReferencedVersions(ctx context.Context, cloudProfileName, imageName string) map[string]bool { + referenced := make(map[string]bool) + + var cp gardenerv1beta1.CloudProfile + if err := r.Get(ctx, types.NamespacedName{Name: cloudProfileName}, &cp); err != nil { + return referenced + } + + // Check machine images in spec + for _, img := range cp.Spec.MachineImages { + if img.Name != imageName { + continue + } + for _, v := range img.Versions { + referenced[v.Version] = true + } + } + + // Check provider config + if cp.Spec.ProviderConfig != nil { + var cfg providercfg.CloudProfileConfig + if err := json.Unmarshal(cp.Spec.ProviderConfig.Raw, &cfg); err == nil { + for _, img := range cfg.MachineImages { + if img.Name != imageName { + continue + } + for _, v := range img.Versions { + // Extract version from image URI (e.g., "registry/repo:version" -> "version") + if idx := strings.LastIndex(v.Image, ":"); idx != -1 { + version := v.Image[idx+1:] + referenced[version] = true + } + } + } + } + } + + return referenced +} + func (r *Reconciler) updateMachineImages(ctx context.Context, log logr.Logger, update v1alpha1.MachineImageUpdate, cpSpec *gardenerv1beta1.CloudProfileSpec) error { var source cloudprofilesync.Source switch { From add9e69404534b378335588c02d43c22a0ccb78f Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Tue, 10 Mar 2026 09:37:30 +0100 Subject: [PATCH 04/14] fixed receiving of referenced versions --- controllers/managedcloudprofile_controller.go | 37 +++++-------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/controllers/managedcloudprofile_controller.go b/controllers/managedcloudprofile_controller.go index df74f01..4187c72 100644 --- a/controllers/managedcloudprofile_controller.go +++ b/controllers/managedcloudprofile_controller.go @@ -118,7 +118,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, fmt.Errorf("failed to list source versions for garbage collection: %w", err) } - // Get currently referenced versions in CloudProfile referencedVersions := r.getReferencedVersions(ctx, mcp.Name, updates.ImageName) cutoff := time.Now().Add(-updates.GarbageCollection.MaxAge.Duration) @@ -126,7 +125,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if v.CreatedAt.IsZero() { continue } - // Skip deletion if version is currently referenced if _, isReferenced := referencedVersions[v.Version]; isReferenced { continue } @@ -190,40 +188,25 @@ func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageN return r.Update(ctx, &cp) } -// getReferencedVersions returns a map of currently referenced image versions in the CloudProfile func (r *Reconciler) getReferencedVersions(ctx context.Context, cloudProfileName, imageName string) map[string]bool { referenced := make(map[string]bool) - var cp gardenerv1beta1.CloudProfile - if err := r.Get(ctx, types.NamespacedName{Name: cloudProfileName}, &cp); err != nil { + shootList := &gardenerv1beta1.ShootList{} + if err := r.List(ctx, shootList, client.InNamespace(metav1.NamespaceAll)); err != nil { return referenced } - // Check machine images in spec - for _, img := range cp.Spec.MachineImages { - if img.Name != imageName { + for _, shoot := range shootList.Items { + if shoot.Spec.CloudProfile == nil || shoot.Spec.CloudProfile.Name != cloudProfileName { continue } - for _, v := range img.Versions { - referenced[v.Version] = true - } - } - // Check provider config - if cp.Spec.ProviderConfig != nil { - var cfg providercfg.CloudProfileConfig - if err := json.Unmarshal(cp.Spec.ProviderConfig.Raw, &cfg); err == nil { - for _, img := range cfg.MachineImages { - if img.Name != imageName { - continue - } - for _, v := range img.Versions { - // Extract version from image URI (e.g., "registry/repo:version" -> "version") - if idx := strings.LastIndex(v.Image, ":"); idx != -1 { - version := v.Image[idx+1:] - referenced[version] = true - } - } + for _, worker := range shoot.Spec.Provider.Workers { + if worker.Machine.Image == nil || worker.Machine.Image.Name != imageName { + continue + } + if worker.Machine.Image.Version != nil { + referenced[*worker.Machine.Image.Version] = true } } } From aa7247ebcc2a8837ac6e0adc4ae051e308b30d4d Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Tue, 10 Mar 2026 12:16:04 +0100 Subject: [PATCH 05/14] resolved vulnerabilities caused by go version --- .github/workflows/checks.yaml | 3 ++- .github/workflows/ci.yaml | 4 ++-- .github/workflows/codeql.yaml | 2 +- Dockerfile | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 5cfabb0..55649eb 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -30,11 +30,12 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.7 + go-version: 1.25.8 - name: Run golangci-lint uses: golangci/golangci-lint-action@v9 with: version: latest + args: --timeout=15m - name: Delete pre-installed shellcheck run: sudo rm -f $(which shellcheck) - name: Run shellcheck diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 4f96519..2a244b1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -32,7 +32,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.7 + go-version: 1.25.8 - name: Build all binaries run: make build-all code_coverage: @@ -65,7 +65,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.7 + go-version: 1.25.8 - name: Run tests and generate coverage report run: make build/cover.out - name: Archive code coverage results diff --git a/.github/workflows/codeql.yaml b/.github/workflows/codeql.yaml index 6081cf9..8f42fe5 100644 --- a/.github/workflows/codeql.yaml +++ b/.github/workflows/codeql.yaml @@ -32,7 +32,7 @@ jobs: uses: actions/setup-go@v6 with: check-latest: true - go-version: 1.25.7 + go-version: 1.25.8 - name: Initialize CodeQL uses: github/codeql-action/init@v4 with: diff --git a/Dockerfile b/Dockerfile index ed23100..9879b2d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 # Build the manager binary -FROM golang:1.25-alpine AS builder +FROM golang:1.25.8-alpine AS builder WORKDIR /workspace ENV GOTOOLCHAIN=local From 4f213989eaaa0d542971672f71842ca2ae4ca82b Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Wed, 11 Mar 2026 16:24:15 +0100 Subject: [PATCH 06/14] added additional tests for new functionality --- controllers/managedcloudprofile_controller.go | 133 ++++-- .../managedcloudprofile_controller_test.go | 417 +++++++++++++++++- controllers/suite_test.go | 1 + 3 files changed, 510 insertions(+), 41 deletions(-) diff --git a/controllers/managedcloudprofile_controller.go b/controllers/managedcloudprofile_controller.go index 4187c72..13539c8 100644 --- a/controllers/managedcloudprofile_controller.go +++ b/controllers/managedcloudprofile_controller.go @@ -31,6 +31,12 @@ const ( CloudProfileAppliedConditionType string = "CloudProfileApplied" ) +// OCIFactory provides a hook for constructing OCI sources. It defaults to +// cloudprofilesync.NewOCI but can be overridden in tests to simulate errors +var OCIFactory = func(params cloudprofilesync.OCIParams, insecure bool) (cloudprofilesync.Source, error) { + return cloudprofilesync.NewOCI(params, insecure) +} + type Reconciler struct { client.Client } @@ -53,6 +59,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu cloudProfile.Spec = CloudProfileSpecToGardener(&mcp.Spec.CloudProfile) errs := make([]error, 0) for _, updates := range mcp.Spec.MachineImageUpdates { + // only call updater when an explicit source is provided + if updates.Source.OCI == nil { + continue + } errs = append(errs, r.updateMachineImages(ctx, log, updates, &cloudProfile.Spec)) } gardenerv1beta1.SetObjectDefaults_CloudProfile(&cloudProfile) @@ -98,7 +108,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if err != nil { return ctrl.Result{}, err } - oci, err := cloudprofilesync.NewOCI(cloudprofilesync.OCIParams{ + src, err := OCIFactory(cloudprofilesync.OCIParams{ Registry: updates.Source.OCI.Registry, Repository: updates.Source.OCI.Repository, Username: updates.Source.OCI.Username, @@ -106,21 +116,40 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu Parallel: 1, }, updates.Source.OCI.Insecure) if err != nil { + if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ + Type: CloudProfileAppliedConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: mcp.Generation, + Reason: "GarbageCollectionFailed", + Message: fmt.Sprintf("failed to initialize OCI source for garbage collection: %s", err), + }); patchErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err) + } return ctrl.Result{}, fmt.Errorf("failed to initialize OCI source for garbage collection: %w", err) } - source = oci + source = src default: continue } versions, err := source.GetVersions(ctx) if err != nil { + if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ + Type: CloudProfileAppliedConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: mcp.Generation, + Reason: "GarbageCollectionFailed", + Message: fmt.Sprintf("failed to list source versions for garbage collection: %s", err), + }); patchErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err) + } return ctrl.Result{}, fmt.Errorf("failed to list source versions for garbage collection: %w", err) } referencedVersions := r.getReferencedVersions(ctx, mcp.Name, updates.ImageName) cutoff := time.Now().Add(-updates.GarbageCollection.MaxAge.Duration) + versionsToDelete := make([]string, 0) for _, v := range versions { if v.CreatedAt.IsZero() { continue @@ -129,14 +158,29 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu continue } if v.CreatedAt.Before(cutoff) { - if err := r.deleteVersion(ctx, mcp.Name, updates.ImageName, v.Version); err != nil { - if apierrors.IsInvalid(err) { - log.V(1).Info("garbage collection validation failed, skipping", "image", updates.ImageName, "version", v.Version) - continue - } - return ctrl.Result{}, fmt.Errorf("failed to delete image version: %w", err) + versionsToDelete = append(versionsToDelete, v.Version) + } + } + + if len(versionsToDelete) > 0 { + if err := r.deleteVersion(ctx, mcp.Name, updates.ImageName, versionsToDelete); err != nil { + if apierrors.IsInvalid(err) { + log.V(1).Info("garbage collection validation failed, skipping", "image", updates.ImageName) + continue } - log.Info("deleted image version from CloudProfile", "image", updates.ImageName, "version", v.Version) + if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ + Type: CloudProfileAppliedConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: mcp.Generation, + Reason: "GarbageCollectionFailed", + Message: fmt.Sprintf("failed to list source versions for garbage collection: %s", err), + }); patchErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err) + } + return ctrl.Result{}, fmt.Errorf("failed to delete image versions: %w", err) + } + for _, v := range versionsToDelete { + log.Info("deleted image version from CloudProfile", "image", updates.ImageName, "version", v) } } } @@ -144,21 +188,26 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{RequeueAfter: 5 * time.Minute}, nil } -func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageName, version string) error { +func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageName string, versions []string) error { var cp gardenerv1beta1.CloudProfile if err := r.Get(ctx, types.NamespacedName{Name: cloudProfileName}, &cp); err != nil { return err } + + versionsSet := make(map[string]bool) + for _, v := range versions { + versionsSet[v] = true + } + for i := range cp.Spec.MachineImages { if cp.Spec.MachineImages[i].Name != imageName { continue } newVersions := make([]gardenerv1beta1.MachineImageVersion, 0, len(cp.Spec.MachineImages[i].Versions)) for _, mv := range cp.Spec.MachineImages[i].Versions { - if mv.Version == version { - continue + if !versionsSet[mv.Version] { + newVersions = append(newVersions, mv) } - newVersions = append(newVersions, mv) } cp.Spec.MachineImages[i].Versions = newVersions } @@ -171,10 +220,16 @@ func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageN } filtered := make([]providercfg.MachineImageVersion, 0, len(cfg.MachineImages[i].Versions)) for _, mv := range cfg.MachineImages[i].Versions { - if strings.HasSuffix(mv.Image, ":"+version) { - continue + found := false + for _, version := range versions { + if strings.HasSuffix(mv.Image, ":"+version) { + found = true + break + } + } + if !found { + filtered = append(filtered, mv) } - filtered = append(filtered, mv) } cfg.MachineImages[i].Versions = filtered } @@ -191,22 +246,40 @@ func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageN func (r *Reconciler) getReferencedVersions(ctx context.Context, cloudProfileName, imageName string) map[string]bool { referenced := make(map[string]bool) - shootList := &gardenerv1beta1.ShootList{} - if err := r.List(ctx, shootList, client.InNamespace(metav1.NamespaceAll)); err != nil { - return referenced - } - - for _, shoot := range shootList.Items { - if shoot.Spec.CloudProfile == nil || shoot.Spec.CloudProfile.Name != cloudProfileName { - continue + var cp gardenerv1beta1.CloudProfile + if err := r.Get(ctx, types.NamespacedName{Name: cloudProfileName}, &cp); err == nil { + if cp.Spec.ProviderConfig != nil { + var cfg providercfg.CloudProfileConfig + if err := json.Unmarshal(cp.Spec.ProviderConfig.Raw, &cfg); err == nil { + for _, img := range cfg.MachineImages { + if img.Name != imageName { + continue + } + for _, v := range img.Versions { + if idx := strings.LastIndex(v.Image, ":"); idx != -1 { + version := v.Image[idx+1:] + referenced[version] = true + } + } + } + } } + } - for _, worker := range shoot.Spec.Provider.Workers { - if worker.Machine.Image == nil || worker.Machine.Image.Name != imageName { + shootList := &gardenerv1beta1.ShootList{} + if err := r.List(ctx, shootList, client.InNamespace(metav1.NamespaceAll)); err == nil { + for _, shoot := range shootList.Items { + if shoot.Spec.CloudProfile == nil || shoot.Spec.CloudProfile.Name != cloudProfileName { continue } - if worker.Machine.Image.Version != nil { - referenced[*worker.Machine.Image.Version] = true + + for _, worker := range shoot.Spec.Provider.Workers { + if worker.Machine.Image == nil || worker.Machine.Image.Name != imageName { + continue + } + if worker.Machine.Image.Version != nil { + referenced[*worker.Machine.Image.Version] = true + } } } } @@ -222,7 +295,7 @@ func (r *Reconciler) updateMachineImages(ctx context.Context, log logr.Logger, u if err != nil { return err } - oci, err := cloudprofilesync.NewOCI(cloudprofilesync.OCIParams{ + src, err := OCIFactory(cloudprofilesync.OCIParams{ Registry: update.Source.OCI.Registry, Repository: update.Source.OCI.Repository, Username: update.Source.OCI.Username, @@ -232,7 +305,7 @@ func (r *Reconciler) updateMachineImages(ctx context.Context, log logr.Logger, u if err != nil { return fmt.Errorf("failed to initialize oci source: %w", err) } - source = oci + source = src default: return errors.New("no machine images source configured") } diff --git a/controllers/managedcloudprofile_controller_test.go b/controllers/managedcloudprofile_controller_test.go index 79a29e0..9797ceb 100644 --- a/controllers/managedcloudprofile_controller_test.go +++ b/controllers/managedcloudprofile_controller_test.go @@ -4,35 +4,78 @@ package controllers_test import ( - gardenerv1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" + "context" + "encoding/json" + "errors" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + gardenerv1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + providercfg "github.com/ironcore-dev/gardener-extension-provider-ironcore-metal/pkg/apis/metal/v1alpha1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/cobaltcore-dev/cloud-profile-sync/api/v1alpha1" + "github.com/cobaltcore-dev/cloud-profile-sync/cloudprofilesync" "github.com/cobaltcore-dev/cloud-profile-sync/controllers" ) +// fakeSource used to simulate GC list failures in tests +type fakeSource struct{} + +func (f *fakeSource) GetVersions(ctx context.Context) ([]cloudprofilesync.SourceImage, error) { + return nil, errors.New("simulated list error") +} + var _ = Describe("The ManagedCloudProfile reconciler", func() { AfterEach(func(ctx SpecContext) { var mcpList v1alpha1.ManagedCloudProfileList + Expect(k8sClient.List(ctx, &mcpList)).To(Succeed()) + for _, mcp := range mcpList.Items { + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) + } + + var cpList gardenerv1beta1.CloudProfileList + Expect(k8sClient.List(ctx, &cpList)).To(Succeed()) + for _, cp := range cpList.Items { + Expect(k8sClient.Delete(ctx, &cp)).To(Succeed()) + } + + var secList corev1.SecretList + Expect(k8sClient.List(ctx, &secList)).To(Succeed()) + for _, sec := range secList.Items { + if sec.Namespace == metav1.NamespaceDefault && sec.Name == "oci" { + Expect(k8sClient.Delete(ctx, &sec)).To(Succeed()) + } + } + Eventually(func(g Gomega) int { - g.Expect(k8sClient.List(ctx, &mcpList)).To(Succeed()) - return len(mcpList.Items) + var updated v1alpha1.ManagedCloudProfileList + g.Expect(k8sClient.List(ctx, &updated)).To(Succeed()) + return len(updated.Items) }).Should(Equal(0)) - var cloudprofiles gardenerv1beta1.CloudProfileList + Eventually(func(g Gomega) int { - g.Expect(k8sClient.List(ctx, &cloudprofiles)).To(Succeed()) - return len(cloudprofiles.Items) + var updated gardenerv1beta1.CloudProfileList + g.Expect(k8sClient.List(ctx, &updated)).To(Succeed()) + return len(updated.Items) }).Should(Equal(0)) - var secrets corev1.SecretList + Eventually(func(g Gomega) int { - g.Expect(k8sClient.List(ctx, &secrets)).To(Succeed()) - return len(secrets.Items) + var updated corev1.SecretList + g.Expect(k8sClient.List(ctx, &updated)).To(Succeed()) + count := 0 + for _, sec := range updated.Items { + if sec.Namespace == metav1.NamespaceDefault && sec.Name == "oci" { + count++ + } + } + return count }).Should(Equal(0)) }) @@ -214,4 +257,356 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { Expect(k8sClient.Delete(ctx, &secret)).To(Succeed()) }) + It("deletes old machine image versions not referenced by any Shoot", func(ctx SpecContext) { + var mcp v1alpha1.ManagedCloudProfile + mcp.Name = "test-gc" + mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ + Regions: []gardenerv1beta1.Region{{Name: "foo"}}, + MachineTypes: []gardenerv1beta1.MachineType{{Name: "baz"}}, + } + mcp.Spec.MachineImageUpdates = []v1alpha1.MachineImageUpdate{ + { + ImageName: "gc-image", + Source: v1alpha1.MachineImageUpdateSource{ + OCI: &v1alpha1.MachineImageUpdateSourceOCI{ + Registry: registryAddr, + Repository: "repo", + Insecure: true, + }, + }, + GarbageCollection: &v1alpha1.GarbageCollectionConfig{ + Enabled: true, + MaxAge: metav1.Duration{Duration: 0}, + }, + }, + } + Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) + + Eventually(func(g Gomega) v1alpha1.ReconcileStatus { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&mcp), &mcp)).To(Succeed()) + return mcp.Status.Status + }).Should(Equal(v1alpha1.SucceededReconcileStatus)) + + var cloudProfile gardenerv1beta1.CloudProfile + cloudProfile.Name = mcp.Name + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&cloudProfile), &cloudProfile)).To(Succeed()) + + Eventually(func(g Gomega) int { + freshProfile := gardenerv1beta1.CloudProfile{} + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&cloudProfile), &freshProfile)).To(Succeed()) + if len(freshProfile.Spec.MachineImages) == 0 { + return 0 + } + for _, img := range freshProfile.Spec.MachineImages { + if img.Name == "gc-image" { + return len(img.Versions) + } + } + return 0 + }, "10s").Should(Equal(0)) + + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) + }) + + It("preserves old machine image versions referenced by Shoot worker pools", func(ctx SpecContext) { + var cloudProfile gardenerv1beta1.CloudProfile + cloudProfile.Name = "test-gc-preserve" + cloudProfile.Spec.Regions = []gardenerv1beta1.Region{{Name: "foo"}} + cloudProfile.Spec.MachineTypes = []gardenerv1beta1.MachineType{{Name: "baz"}} + cloudProfile.Spec.MachineImages = []gardenerv1beta1.MachineImage{ + { + Name: "preserve-image", + Versions: []gardenerv1beta1.MachineImageVersion{ + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.0"}, Architectures: []string{"amd64"}}, + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "2.0.0"}, Architectures: []string{"amd64"}}, + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "3.0.0"}, Architectures: []string{"amd64"}}, + }, + }, + } + + var cfg providercfg.CloudProfileConfig + cfg.MachineImages = []providercfg.MachineImages{ + { + Name: "preserve-image", + Versions: []providercfg.MachineImageVersion{ + {Image: "repo/preserve-image:1.0.0"}, + }, + }, + } + raw, err := json.Marshal(cfg) + Expect(err).To(Succeed()) + cloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: raw} + Expect(k8sClient.Create(ctx, &cloudProfile)).To(Succeed()) + + var mcp v1alpha1.ManagedCloudProfile + mcp.Name = "test-gc-preserve" + mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ + Regions: []gardenerv1beta1.Region{{Name: "foo"}}, + MachineImages: []gardenerv1beta1.MachineImage{ + { + Name: "preserve-image", + Versions: []gardenerv1beta1.MachineImageVersion{ + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.0"}, Architectures: []string{"amd64"}}, + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "2.0.0"}, Architectures: []string{"amd64"}}, + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "3.0.0"}, Architectures: []string{"amd64"}}, + }, + }, + }, + MachineTypes: []gardenerv1beta1.MachineType{{Name: "baz"}}, + } + mcp.Spec.MachineImageUpdates = []v1alpha1.MachineImageUpdate{ + { + ImageName: "preserve-image", + Source: v1alpha1.MachineImageUpdateSource{ + OCI: &v1alpha1.MachineImageUpdateSourceOCI{ + Registry: registryAddr, + Repository: "repo", + Insecure: true, + }, + }, + GarbageCollection: &v1alpha1.GarbageCollectionConfig{ + Enabled: true, + MaxAge: metav1.Duration{Duration: 0}, + }, + }, + } + Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) + + Eventually(func(g Gomega) v1alpha1.ReconcileStatus { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&mcp), &mcp)).To(Succeed()) + return mcp.Status.Status + }).Should(Equal(v1alpha1.SucceededReconcileStatus)) + + Eventually(func(g Gomega) []string { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&cloudProfile), &cloudProfile)).To(Succeed()) + if len(cloudProfile.Spec.MachineImages) == 0 { + return []string{} + } + versions := []string{} + for _, v := range cloudProfile.Spec.MachineImages[0].Versions { + versions = append(versions, v.Version) + } + return versions + }).Should(ContainElement("1.0.0")) + + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) + Expect(k8sClient.Delete(ctx, &cloudProfile)).To(Succeed()) + }) + + It("handles missing credential for GC OCI source", func(ctx SpecContext) { + var mcp v1alpha1.ManagedCloudProfile + mcp.Name = "test-gc-cred-error" + mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ + Regions: []gardenerv1beta1.Region{{Name: "foo"}}, + MachineTypes: []gardenerv1beta1.MachineType{{Name: "baz"}}, + } + mcp.Spec.MachineImageUpdates = []v1alpha1.MachineImageUpdate{ + { + ImageName: "test-image", + Source: v1alpha1.MachineImageUpdateSource{ + OCI: &v1alpha1.MachineImageUpdateSourceOCI{ + Registry: registryAddr, + Repository: "repo", + Insecure: true, + Password: v1alpha1.SecretReference{ + Name: "nonexistent-secret", + Namespace: metav1.NamespaceDefault, + Key: "password", + }, + }, + }, + GarbageCollection: &v1alpha1.GarbageCollectionConfig{ + Enabled: true, + MaxAge: metav1.Duration{Duration: 3600000000000}, + }, + }, + } + Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) + + Eventually(func(g Gomega) v1alpha1.ReconcileStatus { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&mcp), &mcp)).To(Succeed()) + return mcp.Status.Status + }).Should(Equal(v1alpha1.FailedReconcileStatus)) + + Expect(mcp.Status.Conditions).To(ContainElement(SatisfyAll( + HaveField("Type", controllers.CloudProfileAppliedConditionType), + HaveField("Status", metav1.ConditionFalse), + HaveField("Message", ContainSubstring("failed to get secret")), + ))) + + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) + }) + + It("handles invalid OCI registry for GC", func(ctx SpecContext) { + var mcp v1alpha1.ManagedCloudProfile + mcp.Name = "test-gc-invalid-registry" + mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ + Regions: []gardenerv1beta1.Region{{Name: "foo"}}, + MachineTypes: []gardenerv1beta1.MachineType{{Name: "baz"}}, + } + mcp.Spec.MachineImageUpdates = []v1alpha1.MachineImageUpdate{ + { + ImageName: "test-image", + Source: v1alpha1.MachineImageUpdateSource{ + OCI: &v1alpha1.MachineImageUpdateSourceOCI{ + Registry: "invalid://registry", + Repository: "repo", + Insecure: true, + }, + }, + GarbageCollection: &v1alpha1.GarbageCollectionConfig{ + Enabled: true, + MaxAge: metav1.Duration{Duration: 3600000000000}, + }, + }, + } + Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) + + Eventually(func(g Gomega) v1alpha1.ReconcileStatus { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&mcp), &mcp)).To(Succeed()) + return mcp.Status.Status + }).Should(Equal(v1alpha1.FailedReconcileStatus)) + + Expect(mcp.Status.Conditions).To(ContainElement(SatisfyAll( + HaveField("Type", controllers.CloudProfileAppliedConditionType), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", "ApplyFailed"), + HaveField("Message", ContainSubstring("failed to initialize oci source")), + ))) + + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) + }) + + It("reports failure when GC version listing errors occur", func(ctx SpecContext) { + old := controllers.OCIFactory + defer func() { controllers.OCIFactory = old }() + controllers.OCIFactory = func(params cloudprofilesync.OCIParams, insecure bool) (cloudprofilesync.Source, error) { + return &fakeSource{}, nil + } + + var mcp v1alpha1.ManagedCloudProfile + mcp.Name = "test-gc-list-error" + mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ + Regions: []gardenerv1beta1.Region{{Name: "foo"}}, + MachineTypes: []gardenerv1beta1.MachineType{{Name: "baz"}}, + } + mcp.Spec.MachineImageUpdates = []v1alpha1.MachineImageUpdate{ + { + ImageName: "test-image", + Source: v1alpha1.MachineImageUpdateSource{ + OCI: &v1alpha1.MachineImageUpdateSourceOCI{ + Registry: registryAddr, + Repository: "repo", + Insecure: true, + }, + }, + GarbageCollection: &v1alpha1.GarbageCollectionConfig{ + Enabled: true, + MaxAge: metav1.Duration{Duration: 3600000000000}, + }, + }, + } + Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) + + Eventually(func(g Gomega) v1alpha1.ReconcileStatus { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&mcp), &mcp)).To(Succeed()) + return mcp.Status.Status + }).Should(Equal(v1alpha1.FailedReconcileStatus)) + + Expect(mcp.Status.Conditions).To(ContainElement(SatisfyAll( + HaveField("Type", controllers.CloudProfileAppliedConditionType), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", "ApplyFailed"), + HaveField("Message", ContainSubstring("failed to retrieve images version")), + ))) + + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) + }) + + It("skips GC when no source is configured", func(ctx SpecContext) { + var mcp v1alpha1.ManagedCloudProfile + mcp.Name = "test-gc-no-source" + mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ + Regions: []gardenerv1beta1.Region{{Name: "foo"}}, + MachineImages: []gardenerv1beta1.MachineImage{ + { + Name: "test-image", + Versions: []gardenerv1beta1.MachineImageVersion{ + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.0"}, Architectures: []string{"amd64"}}, + }, + }, + }, + MachineTypes: []gardenerv1beta1.MachineType{{Name: "baz"}}, + } + mcp.Spec.MachineImageUpdates = []v1alpha1.MachineImageUpdate{ + { + ImageName: "test-image", + GarbageCollection: &v1alpha1.GarbageCollectionConfig{ + Enabled: true, + MaxAge: metav1.Duration{Duration: 0}, + }, + }, + } + Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) + + var cp gardenerv1beta1.CloudProfile + Eventually(func(g Gomega) error { + return k8sClient.Get(ctx, client.ObjectKey{Name: mcp.Name}, &cp) + }).Should(Succeed()) + + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: mcp.Name}, &cp)).To(Succeed()) + Expect(cp.Spec.MachineImages).To(HaveLen(1)) + Expect(cp.Spec.MachineImages[0].Versions).To(HaveLen(1)) + + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) + }) + + It("reports failure when CloudProfile is already owned by another controller", func(ctx SpecContext) { + var cloudProfile gardenerv1beta1.CloudProfile + cloudProfile.Name = "test-owned" + cloudProfile.Spec = gardenerv1beta1.CloudProfileSpec{ + Regions: []gardenerv1beta1.Region{{Name: "existing-region"}}, + MachineImages: []gardenerv1beta1.MachineImage{ + { + Name: "existing-image", + Versions: []gardenerv1beta1.MachineImageVersion{ + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.0"}, Architectures: []string{"amd64"}}, + }, + }, + }, + MachineTypes: []gardenerv1beta1.MachineType{ + {Name: "existing-type", Architecture: ptr.To("amd64"), Usable: ptr.To(true)}, + }, + } + cloudProfile.OwnerReferences = []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Dummy", + Name: "dummy-owner", + UID: "dummy-uid", + }, + } + Expect(k8sClient.Create(ctx, &cloudProfile)).To(Succeed()) + + var mcp v1alpha1.ManagedCloudProfile + mcp.Name = "test-owned" + mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ + Regions: []gardenerv1beta1.Region{{Name: "foo"}}, + MachineTypes: []gardenerv1beta1.MachineType{{Name: "baz"}}, + } + Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) + + Eventually(func(g Gomega) v1alpha1.ReconcileStatus { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&mcp), &mcp)).To(Succeed()) + return mcp.Status.Status + }).Should(Equal(v1alpha1.FailedReconcileStatus)) + Expect(mcp.Status.Conditions).To(ContainElement(SatisfyAll( + HaveField("Type", controllers.CloudProfileAppliedConditionType), + HaveField("Status", metav1.ConditionFalse), + ))) + + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) + Expect(k8sClient.Delete(ctx, &cloudProfile)).To(Succeed()) + }) + }) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index c0451b3..f8d1fc9 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -129,6 +129,7 @@ var _ = BeforeSuite(func(ctx SpecContext) { }, Annotations: map[string]string{ "architecture": "amd64", + "created": time.Now().Add(-24 * time.Hour).Format(time.RFC3339), }, } indexBlob, err := json.Marshal(index) From e5daea6d9abeef8745113a3d41ba89d8c6d8fef3 Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Fri, 13 Mar 2026 14:19:04 +0100 Subject: [PATCH 07/14] added more tests --- .../managedcloudprofile_controller_test.go | 184 ++++++++++++++++++ ...c.cobaltcore.dev_managedcloudprofiles.yaml | 2 +- crd/core.gardener.cloud_shoots.yaml | 75 +++++++ 3 files changed, 260 insertions(+), 1 deletion(-) create mode 100644 crd/core.gardener.cloud_shoots.yaml diff --git a/controllers/managedcloudprofile_controller_test.go b/controllers/managedcloudprofile_controller_test.go index 9797ceb..07cdbf1 100644 --- a/controllers/managedcloudprofile_controller_test.go +++ b/controllers/managedcloudprofile_controller_test.go @@ -393,6 +393,97 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { Expect(k8sClient.Delete(ctx, &cloudProfile)).To(Succeed()) }) + It("preserves machine image versions referenced by Shoot workers", func(ctx SpecContext) { + var cloudProfile gardenerv1beta1.CloudProfile + cloudProfile.Name = "test-gc-shoot-preserve" + cloudProfile.Spec.Regions = []gardenerv1beta1.Region{{Name: "foo"}} + cloudProfile.Spec.MachineTypes = []gardenerv1beta1.MachineType{{Name: "baz"}} + cloudProfile.Spec.MachineImages = []gardenerv1beta1.MachineImage{ + { + Name: "shoot-preserve-image", + Versions: []gardenerv1beta1.MachineImageVersion{ + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.0"}, Architectures: []string{"amd64"}}, + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.1+abc"}, Architectures: []string{"amd64"}}, + }, + }, + } + Expect(k8sClient.Create(ctx, &cloudProfile)).To(Succeed()) + + var shoot gardenerv1beta1.Shoot + shoot.Name = "test-shoot" + shoot.Namespace = metav1.NamespaceDefault + shoot.Spec.CloudProfile = &gardenerv1beta1.CloudProfileReference{Name: cloudProfile.Name} + shoot.Spec.Provider.Workers = []gardenerv1beta1.Worker{ + { + Name: "worker1", + Machine: gardenerv1beta1.Machine{ + Image: &gardenerv1beta1.ShootMachineImage{ + Name: "shoot-preserve-image", + Version: ptr.To("1.0.0"), + }, + }, + }, + } + Expect(k8sClient.Create(ctx, &shoot)).To(Succeed()) + + var mcp v1alpha1.ManagedCloudProfile + mcp.Name = "test-gc-shoot-preserve" + mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ + Regions: []gardenerv1beta1.Region{{Name: "foo"}}, + MachineImages: []gardenerv1beta1.MachineImage{ + { + Name: "shoot-preserve-image", + Versions: []gardenerv1beta1.MachineImageVersion{ + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.0"}, Architectures: []string{"amd64"}}, + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.1+abc"}, Architectures: []string{"amd64"}}, + }, + }, + }, + MachineTypes: []gardenerv1beta1.MachineType{{Name: "baz"}}, + } + mcp.Spec.MachineImageUpdates = []v1alpha1.MachineImageUpdate{ + { + ImageName: "shoot-preserve-image", + Source: v1alpha1.MachineImageUpdateSource{ + OCI: &v1alpha1.MachineImageUpdateSourceOCI{ + Registry: registryAddr, + Repository: "repo", + Insecure: true, + }, + }, + GarbageCollection: &v1alpha1.GarbageCollectionConfig{ + Enabled: true, + MaxAge: metav1.Duration{Duration: 0}, + }, + }, + } + Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) + + Eventually(func(g Gomega) v1alpha1.ReconcileStatus { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&mcp), &mcp)).To(Succeed()) + return mcp.Status.Status + }).Should(Equal(v1alpha1.SucceededReconcileStatus)) + + Eventually(func(g Gomega) []string { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&cloudProfile), &cloudProfile)).To(Succeed()) + if len(cloudProfile.Spec.MachineImages) == 0 { + return []string{} + } + versions := []string{} + for _, v := range cloudProfile.Spec.MachineImages[0].Versions { + versions = append(versions, v.Version) + } + return versions + }).Should(And( + ContainElement("1.0.0"), + Not(ContainElement("1.0.1+abc")), + )) + + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) + Expect(k8sClient.Delete(ctx, &cloudProfile)).To(Succeed()) + Expect(k8sClient.Delete(ctx, &shoot)).To(Succeed()) + }) + It("handles missing credential for GC OCI source", func(ctx SpecContext) { var mcp v1alpha1.ManagedCloudProfile mcp.Name = "test-gc-cred-error" @@ -609,4 +700,97 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { Expect(k8sClient.Delete(ctx, &cloudProfile)).To(Succeed()) }) + It("updates ProviderConfig when garbage collecting machine image versions", func(ctx SpecContext) { + var cloudProfile gardenerv1beta1.CloudProfile + cloudProfile.Name = "test-gc-provider-config" + cloudProfile.Spec.Regions = []gardenerv1beta1.Region{{Name: "foo"}} + cloudProfile.Spec.MachineTypes = []gardenerv1beta1.MachineType{{Name: "baz"}} + cloudProfile.Spec.MachineImages = []gardenerv1beta1.MachineImage{ + { + Name: "provider-config-image", + Versions: []gardenerv1beta1.MachineImageVersion{ + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.0"}, Architectures: []string{"amd64"}}, + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.1+abc"}, Architectures: []string{"amd64"}}, + }, + }, + } + + var cfg providercfg.CloudProfileConfig + cfg.MachineImages = []providercfg.MachineImages{ + { + Name: "provider-config-image", + Versions: []providercfg.MachineImageVersion{ + {Image: "repo/provider-config-image:1.0.0"}, + {Image: "repo/provider-config-image:1.0.1+abc"}, + }, + }, + } + raw, err := json.Marshal(cfg) + Expect(err).To(Succeed()) + cloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: raw} + Expect(k8sClient.Create(ctx, &cloudProfile)).To(Succeed()) + + var mcp v1alpha1.ManagedCloudProfile + mcp.Name = "test-gc-provider-config" + mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ + Regions: []gardenerv1beta1.Region{{Name: "foo"}}, + MachineImages: []gardenerv1beta1.MachineImage{ + { + Name: "provider-config-image", + Versions: []gardenerv1beta1.MachineImageVersion{ + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.0"}, Architectures: []string{"amd64"}}, + {ExpirableVersion: gardenerv1beta1.ExpirableVersion{Version: "1.0.1+abc"}, Architectures: []string{"amd64"}}, + }, + }, + }, + MachineTypes: []gardenerv1beta1.MachineType{{Name: "baz"}}, + } + mcp.Spec.MachineImageUpdates = []v1alpha1.MachineImageUpdate{ + { + ImageName: "provider-config-image", + Source: v1alpha1.MachineImageUpdateSource{ + OCI: &v1alpha1.MachineImageUpdateSourceOCI{ + Registry: registryAddr, + Repository: "repo", + Insecure: true, + }, + }, + GarbageCollection: &v1alpha1.GarbageCollectionConfig{ + Enabled: true, + MaxAge: metav1.Duration{Duration: 0}, + }, + }, + } + Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) + + Eventually(func(g Gomega) v1alpha1.ReconcileStatus { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&mcp), &mcp)).To(Succeed()) + return mcp.Status.Status + }).Should(Equal(v1alpha1.SucceededReconcileStatus)) + + Eventually(func(g Gomega) []string { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&cloudProfile), &cloudProfile)).To(Succeed()) + if cloudProfile.Spec.ProviderConfig == nil { + return []string{} + } + var updatedCfg providercfg.CloudProfileConfig + if err := json.Unmarshal(cloudProfile.Spec.ProviderConfig.Raw, &updatedCfg); err != nil { + return []string{} + } + for _, img := range updatedCfg.MachineImages { + if img.Name == "provider-config-image" { + images := make([]string, len(img.Versions)) + for i, v := range img.Versions { + images[i] = v.Image + } + return images + } + } + return []string{} + }).Should(BeEmpty()) + + Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) + Expect(k8sClient.Delete(ctx, &cloudProfile)).To(Succeed()) + }) + }) diff --git a/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml b/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml index b01807a..9b26191 100644 --- a/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml +++ b/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml @@ -532,7 +532,7 @@ spec: maxAge: description: |- MaxAge defines the maximum age for images to keep. Images older than - now MaxAge are eligible for deletion. + now - MaxAge are eligible for deletion. type: string type: object imageName: diff --git a/crd/core.gardener.cloud_shoots.yaml b/crd/core.gardener.cloud_shoots.yaml new file mode 100644 index 0000000..da58e2b --- /dev/null +++ b/crd/core.gardener.cloud_shoots.yaml @@ -0,0 +1,75 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: shoots.core.gardener.cloud +spec: + group: core.gardener.cloud + versions: + - name: v1beta1 + served: true + storage: true + schema: + openAPIV3Schema: + description: Shoot is the schema for the shoots API. + type: object + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: Spec defines the desired state of the Shoot. + type: object + properties: + cloudProfile: + description: Reference to the CloudProfile used by the Shoot. + type: object + properties: + name: + description: Name of the CloudProfile. + type: string + provider: + description: Provider-specific configuration. + type: object + properties: + workers: + description: Worker pools for the Shoot. + type: array + items: + type: object + properties: + machine: + description: Machine configuration for a worker pool. + type: object + properties: + image: + description: Image configuration for worker nodes. + type: object + properties: + name: + description: Machine image name. + type: string + version: + description: Machine image version. + type: string + status: + description: Status contains the current status of the Shoot. + type: object + scope: Namespaced + names: + plural: shoots + singular: shoot + kind: Shoot \ No newline at end of file From 132d34e0f30782caec2acab15ed0d7b3f676738b Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Fri, 13 Mar 2026 15:09:13 +0100 Subject: [PATCH 08/14] fixed comments and improved error handling --- controllers/managedcloudprofile_controller.go | 119 ++++++++++-------- .../managedcloudprofile_controller_test.go | 26 +++- 2 files changed, 89 insertions(+), 56 deletions(-) diff --git a/controllers/managedcloudprofile_controller.go b/controllers/managedcloudprofile_controller.go index 13539c8..3040f59 100644 --- a/controllers/managedcloudprofile_controller.go +++ b/controllers/managedcloudprofile_controller.go @@ -106,6 +106,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu case updates.Source.OCI != nil: password, err := r.getCredential(ctx, updates.Source.OCI.Password) if err != nil { + if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ + Type: CloudProfileAppliedConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: mcp.Generation, + Reason: "GarbageCollectionFailed", + Message: fmt.Sprintf("failed to get credential for garbage collection: %s", err), + }); patchErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err) + } return ctrl.Result{}, err } src, err := OCIFactory(cloudprofilesync.OCIParams{ @@ -146,7 +155,24 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, fmt.Errorf("failed to list source versions for garbage collection: %w", err) } - referencedVersions := r.getReferencedVersions(ctx, mcp.Name, updates.ImageName) + referencedVersions, err := r.getReferencedVersions(ctx, mcp.Name, updates.ImageName) + if err != nil { + if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ + Type: CloudProfileAppliedConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: mcp.Generation, + Reason: "GarbageCollectionFailed", + Message: fmt.Sprintf("failed to determine referenced versions for garbage collection: %s", err), + }); patchErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err) + } + return ctrl.Result{}, fmt.Errorf("failed to determine referenced versions for garbage collection: %w", err) + } + + if updates.GarbageCollection.MaxAge.Duration < 0 { + log.Info("skipping garbage collection due to negative MaxAge", "image", updates.ImageName, "maxAge", updates.GarbageCollection.MaxAge.Duration) + continue + } cutoff := time.Now().Add(-updates.GarbageCollection.MaxAge.Duration) versionsToDelete := make([]string, 0) @@ -173,7 +199,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu Status: metav1.ConditionFalse, ObservedGeneration: mcp.Generation, Reason: "GarbageCollectionFailed", - Message: fmt.Sprintf("failed to list source versions for garbage collection: %s", err), + Message: fmt.Sprintf("failed to delete image versions during garbage collection: %s", err), }); patchErr != nil { return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err) } @@ -213,78 +239,61 @@ func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageN } if cp.Spec.ProviderConfig != nil { var cfg providercfg.CloudProfileConfig - if err := json.Unmarshal(cp.Spec.ProviderConfig.Raw, &cfg); err == nil { - for i := range cfg.MachineImages { - if cfg.MachineImages[i].Name != imageName { - continue - } - filtered := make([]providercfg.MachineImageVersion, 0, len(cfg.MachineImages[i].Versions)) - for _, mv := range cfg.MachineImages[i].Versions { - found := false - for _, version := range versions { - if strings.HasSuffix(mv.Image, ":"+version) { - found = true - break - } - } - if !found { - filtered = append(filtered, mv) + if err := json.Unmarshal(cp.Spec.ProviderConfig.Raw, &cfg); err != nil { + return fmt.Errorf("failed to unmarshal ProviderConfig: %w", err) + } + for i := range cfg.MachineImages { + if cfg.MachineImages[i].Name != imageName { + continue + } + filtered := make([]providercfg.MachineImageVersion, 0, len(cfg.MachineImages[i].Versions)) + for _, mv := range cfg.MachineImages[i].Versions { + found := false + for _, version := range versions { + if strings.HasSuffix(mv.Image, ":"+version) { + found = true + break } } - cfg.MachineImages[i].Versions = filtered - } - raw, err := json.Marshal(cfg) - if err == nil { - cp.Spec.ProviderConfig.Raw = raw + if !found { + filtered = append(filtered, mv) + } } + cfg.MachineImages[i].Versions = filtered } + raw, err := json.Marshal(cfg) + if err != nil { + return fmt.Errorf("failed to marshal ProviderConfig: %w", err) + } + cp.Spec.ProviderConfig.Raw = raw } return r.Update(ctx, &cp) } -func (r *Reconciler) getReferencedVersions(ctx context.Context, cloudProfileName, imageName string) map[string]bool { +func (r *Reconciler) getReferencedVersions(ctx context.Context, cloudProfileName, imageName string) (map[string]bool, error) { referenced := make(map[string]bool) - var cp gardenerv1beta1.CloudProfile - if err := r.Get(ctx, types.NamespacedName{Name: cloudProfileName}, &cp); err == nil { - if cp.Spec.ProviderConfig != nil { - var cfg providercfg.CloudProfileConfig - if err := json.Unmarshal(cp.Spec.ProviderConfig.Raw, &cfg); err == nil { - for _, img := range cfg.MachineImages { - if img.Name != imageName { - continue - } - for _, v := range img.Versions { - if idx := strings.LastIndex(v.Image, ":"); idx != -1 { - version := v.Image[idx+1:] - referenced[version] = true - } - } - } - } - } + shootList := &gardenerv1beta1.ShootList{} + if err := r.List(ctx, shootList, client.InNamespace(metav1.NamespaceAll)); err != nil { + return nil, fmt.Errorf("failed to list Shoots: %w", err) } + for _, shoot := range shootList.Items { + if shoot.Spec.CloudProfile == nil || shoot.Spec.CloudProfile.Name != cloudProfileName { + continue + } - shootList := &gardenerv1beta1.ShootList{} - if err := r.List(ctx, shootList, client.InNamespace(metav1.NamespaceAll)); err == nil { - for _, shoot := range shootList.Items { - if shoot.Spec.CloudProfile == nil || shoot.Spec.CloudProfile.Name != cloudProfileName { + for _, worker := range shoot.Spec.Provider.Workers { + if worker.Machine.Image == nil || worker.Machine.Image.Name != imageName { continue } - - for _, worker := range shoot.Spec.Provider.Workers { - if worker.Machine.Image == nil || worker.Machine.Image.Name != imageName { - continue - } - if worker.Machine.Image.Version != nil { - referenced[*worker.Machine.Image.Version] = true - } + if worker.Machine.Image.Version != nil { + referenced[*worker.Machine.Image.Version] = true } } } - return referenced + return referenced, nil } func (r *Reconciler) updateMachineImages(ctx context.Context, log logr.Logger, update v1alpha1.MachineImageUpdate, cpSpec *gardenerv1beta1.CloudProfileSpec) error { diff --git a/controllers/managedcloudprofile_controller_test.go b/controllers/managedcloudprofile_controller_test.go index 07cdbf1..3b2e297 100644 --- a/controllers/managedcloudprofile_controller_test.go +++ b/controllers/managedcloudprofile_controller_test.go @@ -46,6 +46,12 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { Expect(k8sClient.Delete(ctx, &cp)).To(Succeed()) } + var shootList gardenerv1beta1.ShootList + Expect(k8sClient.List(ctx, &shootList)).To(Succeed()) + for _, shoot := range shootList.Items { + Expect(k8sClient.Delete(ctx, &shoot)).To(Succeed()) + } + var secList corev1.SecretList Expect(k8sClient.List(ctx, &secList)).To(Succeed()) for _, sec := range secList.Items { @@ -61,7 +67,7 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { }).Should(Equal(0)) Eventually(func(g Gomega) int { - var updated gardenerv1beta1.CloudProfileList + var updated gardenerv1beta1.ShootList g.Expect(k8sClient.List(ctx, &updated)).To(Succeed()) return len(updated.Items) }).Should(Equal(0)) @@ -338,6 +344,23 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { cloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: raw} Expect(k8sClient.Create(ctx, &cloudProfile)).To(Succeed()) + var shoot gardenerv1beta1.Shoot + shoot.Name = "test-shoot-preserve" + shoot.Namespace = metav1.NamespaceDefault + shoot.Spec.CloudProfile = &gardenerv1beta1.CloudProfileReference{Name: cloudProfile.Name} + shoot.Spec.Provider.Workers = []gardenerv1beta1.Worker{ + { + Name: "worker1", + Machine: gardenerv1beta1.Machine{ + Image: &gardenerv1beta1.ShootMachineImage{ + Name: "preserve-image", + Version: ptr.To("1.0.0"), + }, + }, + }, + } + Expect(k8sClient.Create(ctx, &shoot)).To(Succeed()) + var mcp v1alpha1.ManagedCloudProfile mcp.Name = "test-gc-preserve" mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ @@ -391,6 +414,7 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed()) Expect(k8sClient.Delete(ctx, &cloudProfile)).To(Succeed()) + Expect(k8sClient.Delete(ctx, &shoot)).To(Succeed()) }) It("preserves machine image versions referenced by Shoot workers", func(ctx SpecContext) { From 5308edd7fb03f54cdad00a87838df228dfdf5c47 Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Wed, 18 Mar 2026 14:22:19 +0100 Subject: [PATCH 09/14] fixed comments --- controllers/managedcloudprofile_controller.go | 204 ++++++++---------- .../managedcloudprofile_controller_test.go | 9 - 2 files changed, 91 insertions(+), 122 deletions(-) diff --git a/controllers/managedcloudprofile_controller.go b/controllers/managedcloudprofile_controller.go index 3040f59..c050103 100644 --- a/controllers/managedcloudprofile_controller.go +++ b/controllers/managedcloudprofile_controller.go @@ -48,28 +48,33 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, client.IgnoreNotFound(err) } + if result, err := r.reconcileCloudProfile(ctx, log, &mcp); err != nil || !result.IsZero() { + return result, err + } + + log.Info("reconciled ManagedCloudProfile") + return r.reconcileGarbageCollection(ctx, log, &mcp) +} + +func (r *Reconciler) reconcileCloudProfile(ctx context.Context, log logr.Logger, mcp *v1alpha1.ManagedCloudProfile) (ctrl.Result, error) { var cloudProfile gardenerv1beta1.CloudProfile cloudProfile.Name = mcp.Name op, err := controllerutil.CreateOrPatch(ctx, r.Client, &cloudProfile, func() error { - err := controllerutil.SetControllerReference(&mcp, &cloudProfile, r.Scheme()) + err := controllerutil.SetControllerReference(mcp, &cloudProfile, r.Scheme()) if err != nil { return err } cloudProfile.Spec = CloudProfileSpecToGardener(&mcp.Spec.CloudProfile) errs := make([]error, 0) for _, updates := range mcp.Spec.MachineImageUpdates { - // only call updater when an explicit source is provided - if updates.Source.OCI == nil { - continue - } errs = append(errs, r.updateMachineImages(ctx, log, updates, &cloudProfile.Spec)) } gardenerv1beta1.SetObjectDefaults_CloudProfile(&cloudProfile) return errors.Join(errs...) }) if err != nil { - if err := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ + if err := r.patchStatusAndCondition(ctx, mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ Type: CloudProfileAppliedConditionType, Status: metav1.ConditionFalse, ObservedGeneration: mcp.Generation, @@ -86,7 +91,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } log.Info("applied cloud profile", "op", op) if op != controllerutil.OperationResultNone { - if err := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.SucceededReconcileStatus, metav1.Condition{ + if err := r.patchStatusAndCondition(ctx, mcp, v1alpha1.SucceededReconcileStatus, metav1.Condition{ Type: CloudProfileAppliedConditionType, Status: metav1.ConditionTrue, ObservedGeneration: mcp.Generation, @@ -96,86 +101,46 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w", err) } } - log.Info("reconciled ManagedCloudProfile") + return ctrl.Result{}, nil +} + +func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Logger, mcp *v1alpha1.ManagedCloudProfile) (ctrl.Result, error) { for _, updates := range mcp.Spec.MachineImageUpdates { if updates.GarbageCollection == nil || !updates.GarbageCollection.Enabled { continue } - var source cloudprofilesync.Source - switch { - case updates.Source.OCI != nil: - password, err := r.getCredential(ctx, updates.Source.OCI.Password) - if err != nil { - if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ - Type: CloudProfileAppliedConditionType, - Status: metav1.ConditionFalse, - ObservedGeneration: mcp.Generation, - Reason: "GarbageCollectionFailed", - Message: fmt.Sprintf("failed to get credential for garbage collection: %s", err), - }); patchErr != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err) - } - return ctrl.Result{}, err - } - src, err := OCIFactory(cloudprofilesync.OCIParams{ - Registry: updates.Source.OCI.Registry, - Repository: updates.Source.OCI.Repository, - Username: updates.Source.OCI.Username, - Password: string(password), - Parallel: 1, - }, updates.Source.OCI.Insecure) - if err != nil { - if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ - Type: CloudProfileAppliedConditionType, - Status: metav1.ConditionFalse, - ObservedGeneration: mcp.Generation, - Reason: "GarbageCollectionFailed", - Message: fmt.Sprintf("failed to initialize OCI source for garbage collection: %s", err), - }); patchErr != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err) - } - return ctrl.Result{}, fmt.Errorf("failed to initialize OCI source for garbage collection: %w", err) - } - source = src - default: + if updates.Source.OCI == nil { continue } + var source cloudprofilesync.Source + password, err := r.getCredential(ctx, updates.Source.OCI.Password) + if err != nil { + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", fmt.Sprintf("failed to get credential for garbage collection: %s", err), err) + } + src, err := OCIFactory(cloudprofilesync.OCIParams{ + Registry: updates.Source.OCI.Registry, + Repository: updates.Source.OCI.Repository, + Username: updates.Source.OCI.Username, + Password: string(password), + Parallel: 1, + }, updates.Source.OCI.Insecure) + if err != nil { + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", fmt.Sprintf("failed to initialize OCI source for garbage collection: %s", err), fmt.Errorf("failed to initialize OCI source for garbage collection: %w", err)) + } + source = src versions, err := source.GetVersions(ctx) if err != nil { - if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ - Type: CloudProfileAppliedConditionType, - Status: metav1.ConditionFalse, - ObservedGeneration: mcp.Generation, - Reason: "GarbageCollectionFailed", - Message: fmt.Sprintf("failed to list source versions for garbage collection: %s", err), - }); patchErr != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err) - } - return ctrl.Result{}, fmt.Errorf("failed to list source versions for garbage collection: %w", err) + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", fmt.Sprintf("failed to list source versions for garbage collection: %s", err), fmt.Errorf("failed to list source versions for garbage collection: %w", err)) } referencedVersions, err := r.getReferencedVersions(ctx, mcp.Name, updates.ImageName) if err != nil { - if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ - Type: CloudProfileAppliedConditionType, - Status: metav1.ConditionFalse, - ObservedGeneration: mcp.Generation, - Reason: "GarbageCollectionFailed", - Message: fmt.Sprintf("failed to determine referenced versions for garbage collection: %s", err), - }); patchErr != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err) - } - return ctrl.Result{}, fmt.Errorf("failed to determine referenced versions for garbage collection: %w", err) - } - - if updates.GarbageCollection.MaxAge.Duration < 0 { - log.Info("skipping garbage collection due to negative MaxAge", "image", updates.ImageName, "maxAge", updates.GarbageCollection.MaxAge.Duration) - continue + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", fmt.Sprintf("failed to determine referenced versions for garbage collection: %s", err), fmt.Errorf("failed to determine referenced versions for garbage collection: %w", err)) } cutoff := time.Now().Add(-updates.GarbageCollection.MaxAge.Duration) - versionsToDelete := make([]string, 0) + versionsToDelete := make(map[string]struct{}) for _, v := range versions { if v.CreatedAt.IsZero() { continue @@ -184,28 +149,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu continue } if v.CreatedAt.Before(cutoff) { - versionsToDelete = append(versionsToDelete, v.Version) + versionsToDelete[v.Version] = struct{}{} } } if len(versionsToDelete) > 0 { - if err := r.deleteVersion(ctx, mcp.Name, updates.ImageName, versionsToDelete); err != nil { + if err := r.deleteVersions(ctx, mcp.Name, updates.ImageName, versionsToDelete); err != nil { if apierrors.IsInvalid(err) { log.V(1).Info("garbage collection validation failed, skipping", "image", updates.ImageName) continue } - if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ - Type: CloudProfileAppliedConditionType, - Status: metav1.ConditionFalse, - ObservedGeneration: mcp.Generation, - Reason: "GarbageCollectionFailed", - Message: fmt.Sprintf("failed to delete image versions during garbage collection: %s", err), - }); patchErr != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err) - } - return ctrl.Result{}, fmt.Errorf("failed to delete image versions: %w", err) + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", fmt.Sprintf("failed to delete image versions during garbage collection: %s", err), fmt.Errorf("failed to delete image versions: %w", err)) } - for _, v := range versionsToDelete { + for v := range versionsToDelete { log.Info("deleted image version from CloudProfile", "image", updates.ImageName, "version", v) } } @@ -214,28 +170,20 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{RequeueAfter: 5 * time.Minute}, nil } -func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageName string, versions []string) error { +func (r *Reconciler) deleteVersions(ctx context.Context, cloudProfileName, imageName string, versionsToDelete map[string]struct{}) error { var cp gardenerv1beta1.CloudProfile if err := r.Get(ctx, types.NamespacedName{Name: cloudProfileName}, &cp); err != nil { return err } - versionsSet := make(map[string]bool) - for _, v := range versions { - versionsSet[v] = true - } - for i := range cp.Spec.MachineImages { if cp.Spec.MachineImages[i].Name != imageName { continue } - newVersions := make([]gardenerv1beta1.MachineImageVersion, 0, len(cp.Spec.MachineImages[i].Versions)) - for _, mv := range cp.Spec.MachineImages[i].Versions { - if !versionsSet[mv.Version] { - newVersions = append(newVersions, mv) - } - } - cp.Spec.MachineImages[i].Versions = newVersions + cp.Spec.MachineImages[i].Versions = slices.DeleteFunc(cp.Spec.MachineImages[i].Versions, func(mv gardenerv1beta1.MachineImageVersion) bool { + _, exists := versionsToDelete[mv.Version] + return exists + }) } if cp.Spec.ProviderConfig != nil { var cfg providercfg.CloudProfileConfig @@ -246,20 +194,15 @@ func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageN if cfg.MachineImages[i].Name != imageName { continue } - filtered := make([]providercfg.MachineImageVersion, 0, len(cfg.MachineImages[i].Versions)) - for _, mv := range cfg.MachineImages[i].Versions { - found := false - for _, version := range versions { - if strings.HasSuffix(mv.Image, ":"+version) { - found = true - break - } + cfg.MachineImages[i].Versions = slices.DeleteFunc(cfg.MachineImages[i].Versions, func(mv providercfg.MachineImageVersion) bool { + idx := strings.LastIndex(mv.Image, ":") + if idx == -1 { + return false } - if !found { - filtered = append(filtered, mv) - } - } - cfg.MachineImages[i].Versions = filtered + version := mv.Image[idx+1:] + _, exists := versionsToDelete[version] + return exists + }) } raw, err := json.Marshal(cfg) if err != nil { @@ -271,8 +214,30 @@ func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageN return r.Update(ctx, &cp) } -func (r *Reconciler) getReferencedVersions(ctx context.Context, cloudProfileName, imageName string) (map[string]bool, error) { - referenced := make(map[string]bool) +func (r *Reconciler) getReferencedVersions(ctx context.Context, cloudProfileName, imageName string) (map[string]struct{}, error) { + referenced := make(map[string]struct{}) + + var cp gardenerv1beta1.CloudProfile + if err := r.Get(ctx, types.NamespacedName{Name: cloudProfileName}, &cp); err != nil { + return nil, fmt.Errorf("failed to get CloudProfile: %w", err) + } + if cp.Spec.ProviderConfig != nil { + var cfg providercfg.CloudProfileConfig + if err := json.Unmarshal(cp.Spec.ProviderConfig.Raw, &cfg); err != nil { + return nil, fmt.Errorf("failed to unmarshal ProviderConfig: %w", err) + } + for _, img := range cfg.MachineImages { + if img.Name != imageName { + continue + } + for _, v := range img.Versions { + if idx := strings.LastIndex(v.Image, ":"); idx != -1 { + version := v.Image[idx+1:] + referenced[version] = struct{}{} + } + } + } + } shootList := &gardenerv1beta1.ShootList{} if err := r.List(ctx, shootList, client.InNamespace(metav1.NamespaceAll)); err != nil { @@ -288,7 +253,7 @@ func (r *Reconciler) getReferencedVersions(ctx context.Context, cloudProfileName continue } if worker.Machine.Image.Version != nil { - referenced[*worker.Machine.Image.Version] = true + referenced[*worker.Machine.Image.Version] = struct{}{} } } } @@ -400,6 +365,19 @@ func CloudProfileSpecToGardener(spec *v1alpha1.CloudProfileSpec) gardenerv1beta1 } } +func (r *Reconciler) failWithStatusUpdate(ctx context.Context, mcp *v1alpha1.ManagedCloudProfile, reason, message string, returnErr error) error { + if patchErr := r.patchStatusAndCondition(ctx, mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ + Type: CloudProfileAppliedConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: mcp.Generation, + Reason: reason, + Message: message, + }); patchErr != nil { + return fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, returnErr) + } + return returnErr +} + // SetupWithManager attaches the controller to the given manager. func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/controllers/managedcloudprofile_controller_test.go b/controllers/managedcloudprofile_controller_test.go index 3b2e297..1590eef 100644 --- a/controllers/managedcloudprofile_controller_test.go +++ b/controllers/managedcloudprofile_controller_test.go @@ -653,15 +653,6 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { }, MachineTypes: []gardenerv1beta1.MachineType{{Name: "baz"}}, } - mcp.Spec.MachineImageUpdates = []v1alpha1.MachineImageUpdate{ - { - ImageName: "test-image", - GarbageCollection: &v1alpha1.GarbageCollectionConfig{ - Enabled: true, - MaxAge: metav1.Duration{Duration: 0}, - }, - }, - } Expect(k8sClient.Create(ctx, &mcp)).To(Succeed()) var cp gardenerv1beta1.CloudProfile From dd44fc9caee29daf86fb39d066bf2bbc159dba87 Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Thu, 19 Mar 2026 12:04:26 +0100 Subject: [PATCH 10/14] fixed comments and recommendations --- api/v1alpha1/managedcloudprofile.go | 1 + controllers/managedcloudprofile_controller.go | 23 +++++++++++++++---- .../managedcloudprofile_controller_test.go | 19 +++++++++++---- controllers/suite_test.go | 6 +++-- 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/api/v1alpha1/managedcloudprofile.go b/api/v1alpha1/managedcloudprofile.go index 1622679..b01264b 100644 --- a/api/v1alpha1/managedcloudprofile.go +++ b/api/v1alpha1/managedcloudprofile.go @@ -109,6 +109,7 @@ type GarbageCollectionConfig struct { // MaxAge defines the maximum age for images to keep. Images older than // now - MaxAge are eligible for deletion. // +optional + // +kubebuilder:validation:XValidation:rule="duration(self) >= duration('0s')",message="maxAge must not be negative" MaxAge metav1.Duration `json:"maxAge,omitempty"` } diff --git a/controllers/managedcloudprofile_controller.go b/controllers/managedcloudprofile_controller.go index c050103..307b241 100644 --- a/controllers/managedcloudprofile_controller.go +++ b/controllers/managedcloudprofile_controller.go @@ -31,14 +31,21 @@ const ( CloudProfileAppliedConditionType string = "CloudProfileApplied" ) -// OCIFactory provides a hook for constructing OCI sources. It defaults to -// cloudprofilesync.NewOCI but can be overridden in tests to simulate errors -var OCIFactory = func(params cloudprofilesync.OCIParams, insecure bool) (cloudprofilesync.Source, error) { +// OCISourceFactory defines an interface for creating OCI sources. +type OCISourceFactory interface { + Create(params cloudprofilesync.OCIParams, insecure bool) (cloudprofilesync.Source, error) +} + +// DefaultOCISourceFactory is the default implementation of OCISourceFactory. +type DefaultOCISourceFactory struct{} + +func (f *DefaultOCISourceFactory) Create(params cloudprofilesync.OCIParams, insecure bool) (cloudprofilesync.Source, error) { return cloudprofilesync.NewOCI(params, insecure) } type Reconciler struct { client.Client + OCISourceFactory OCISourceFactory } func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -109,6 +116,9 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo if updates.GarbageCollection == nil || !updates.GarbageCollection.Enabled { continue } + if updates.GarbageCollection.MaxAge.Duration < 0 { + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", "garbage collection maxAge must not be negative", fmt.Errorf("invalid garbage collection maxAge: %s", updates.GarbageCollection.MaxAge.String())) + } if updates.Source.OCI == nil { continue } @@ -117,7 +127,7 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo if err != nil { return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", fmt.Sprintf("failed to get credential for garbage collection: %s", err), err) } - src, err := OCIFactory(cloudprofilesync.OCIParams{ + src, err := r.OCISourceFactory.Create(cloudprofilesync.OCIParams{ Registry: updates.Source.OCI.Registry, Repository: updates.Source.OCI.Repository, Username: updates.Source.OCI.Username, @@ -269,7 +279,7 @@ func (r *Reconciler) updateMachineImages(ctx context.Context, log logr.Logger, u if err != nil { return err } - src, err := OCIFactory(cloudprofilesync.OCIParams{ + src, err := r.OCISourceFactory.Create(cloudprofilesync.OCIParams{ Registry: update.Source.OCI.Registry, Repository: update.Source.OCI.Repository, Username: update.Source.OCI.Username, @@ -380,6 +390,9 @@ func (r *Reconciler) failWithStatusUpdate(ctx context.Context, mcp *v1alpha1.Man // SetupWithManager attaches the controller to the given manager. func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { + if r.OCISourceFactory == nil { + r.OCISourceFactory = &DefaultOCISourceFactory{} + } return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.ManagedCloudProfile{}). Owns(&gardenerv1beta1.CloudProfile{}). diff --git a/controllers/managedcloudprofile_controller_test.go b/controllers/managedcloudprofile_controller_test.go index 1590eef..a33dc5d 100644 --- a/controllers/managedcloudprofile_controller_test.go +++ b/controllers/managedcloudprofile_controller_test.go @@ -31,6 +31,15 @@ func (f *fakeSource) GetVersions(ctx context.Context) ([]cloudprofilesync.Source return nil, errors.New("simulated list error") } +// mockOCIFactory implements controllers.OCISourceFactory for testing +type mockOCIFactory struct { + createFunc func(params cloudprofilesync.OCIParams, insecure bool) (cloudprofilesync.Source, error) +} + +func (m *mockOCIFactory) Create(params cloudprofilesync.OCIParams, insecure bool) (cloudprofilesync.Source, error) { + return m.createFunc(params, insecure) +} + var _ = Describe("The ManagedCloudProfile reconciler", func() { AfterEach(func(ctx SpecContext) { @@ -593,10 +602,12 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { }) It("reports failure when GC version listing errors occur", func(ctx SpecContext) { - old := controllers.OCIFactory - defer func() { controllers.OCIFactory = old }() - controllers.OCIFactory = func(params cloudprofilesync.OCIParams, insecure bool) (cloudprofilesync.Source, error) { - return &fakeSource{}, nil + old := reconciler.OCISourceFactory + defer func() { reconciler.OCISourceFactory = old }() + reconciler.OCISourceFactory = &mockOCIFactory{ + createFunc: func(params cloudprofilesync.OCIParams, insecure bool) (cloudprofilesync.Source, error) { + return &fakeSource{}, nil + }, } var mcp v1alpha1.ManagedCloudProfile diff --git a/controllers/suite_test.go b/controllers/suite_test.go index f8d1fc9..c9af20f 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -45,6 +45,7 @@ var ( testEnv *envtest.Environment reg *registry.Registry stop context.CancelFunc + reconciler *controllers.Reconciler ) func TestControllers(t *testing.T) { @@ -77,9 +78,10 @@ var _ = BeforeSuite(func(ctx SpecContext) { }) Expect(err).To(Succeed()) - err = (&controllers.Reconciler{ + reconciler = &controllers.Reconciler{ Client: k8sManager.GetClient(), - }).SetupWithManager(k8sManager) + } + err = reconciler.SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) stopCtx, cancel := context.WithCancel(ctrl.SetupSignalHandler()) From 21d98b2e98cb4a76bad3383052348fe278e44867 Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Thu, 19 Mar 2026 15:04:01 +0100 Subject: [PATCH 11/14] fixed linter issues --- .github/workflows/checks.yaml | 1 - cloudprofilesync/provider_test.go | 8 +++-- controllers/managedcloudprofile_controller.go | 32 +++++++++---------- .../managedcloudprofile_controller_test.go | 20 +++++++----- 4 files changed, 33 insertions(+), 28 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 1be7ffc..ba3ea53 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -34,7 +34,6 @@ jobs: uses: golangci/golangci-lint-action@v9 with: version: latest - args: --timeout=15m - name: Delete pre-installed shellcheck run: sudo rm -f $(which shellcheck) - name: Run shellcheck diff --git a/cloudprofilesync/provider_test.go b/cloudprofilesync/provider_test.go index 8c9cad3..f566924 100644 --- a/cloudprofilesync/provider_test.go +++ b/cloudprofilesync/provider_test.go @@ -10,7 +10,6 @@ import ( "github.com/ironcore-dev/gardener-extension-provider-ironcore-metal/pkg/apis/metal/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "k8s.io/utils/ptr" "github.com/cobaltcore-dev/cloud-profile-sync/cloudprofilesync" ) @@ -51,16 +50,19 @@ var _ = Describe("IroncoreProvider", func() { Expect(json.Unmarshal(cpSpec.ProviderConfig.Raw, &providerConfig)).To(Succeed()) Expect(providerConfig.MachineImages).To(HaveLen(1)) Expect(providerConfig.MachineImages[0].Name).To(Equal("test")) + + amd64 := "amd64" + arm64 := "arm64" Expect(providerConfig.MachineImages[0].Versions).To(ConsistOf([]v1alpha1.MachineImageVersion{ { Version: "v1.0.0", Image: "registry.io/repo:v1.0.0", - Architecture: ptr.To("amd64"), + Architecture: &amd64, }, { Version: "v1.0.0", Image: "registry.io/repo:v1.0.0", - Architecture: ptr.To("arm64"), + Architecture: &arm64, }, })) }) diff --git a/controllers/managedcloudprofile_controller.go b/controllers/managedcloudprofile_controller.go index 307b241..8563fd2 100644 --- a/controllers/managedcloudprofile_controller.go +++ b/controllers/managedcloudprofile_controller.go @@ -55,15 +55,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, client.IgnoreNotFound(err) } - if result, err := r.reconcileCloudProfile(ctx, log, &mcp); err != nil || !result.IsZero() { - return result, err + if err := r.reconcileCloudProfile(ctx, log, &mcp); err != nil { + return ctrl.Result{}, err } log.Info("reconciled ManagedCloudProfile") return r.reconcileGarbageCollection(ctx, log, &mcp) } -func (r *Reconciler) reconcileCloudProfile(ctx context.Context, log logr.Logger, mcp *v1alpha1.ManagedCloudProfile) (ctrl.Result, error) { +func (r *Reconciler) reconcileCloudProfile(ctx context.Context, log logr.Logger, mcp *v1alpha1.ManagedCloudProfile) error { var cloudProfile gardenerv1beta1.CloudProfile cloudProfile.Name = mcp.Name @@ -88,13 +88,13 @@ func (r *Reconciler) reconcileCloudProfile(ctx context.Context, log logr.Logger, Reason: "ApplyFailed", Message: fmt.Sprintf("Failed to apply CloudProfile: %s", err), }); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w", err) + return fmt.Errorf("failed to patch ManagedCloudProfile status: %w", err) } if apierrors.IsInvalid(err) { log.Error(err, "tried to apply invalid CloudProfile") - return ctrl.Result{}, nil + return nil } - return ctrl.Result{}, fmt.Errorf("failed to create or patch CloudProfile: %w", err) + return fmt.Errorf("failed to create or patch CloudProfile: %w", err) } log.Info("applied cloud profile", "op", op) if op != controllerutil.OperationResultNone { @@ -105,10 +105,10 @@ func (r *Reconciler) reconcileCloudProfile(ctx context.Context, log logr.Logger, Reason: "Applied", Message: "Generated CloudProfile applied successfully", }); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w", err) + return fmt.Errorf("failed to patch ManagedCloudProfile status: %w", err) } } - return ctrl.Result{}, nil + return nil } func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Logger, mcp *v1alpha1.ManagedCloudProfile) (ctrl.Result, error) { @@ -117,7 +117,7 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo continue } if updates.GarbageCollection.MaxAge.Duration < 0 { - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", "garbage collection maxAge must not be negative", fmt.Errorf("invalid garbage collection maxAge: %s", updates.GarbageCollection.MaxAge.String())) + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "garbage collection maxAge must not be negative", fmt.Errorf("invalid garbage collection maxAge: %s", updates.GarbageCollection.MaxAge.String())) } if updates.Source.OCI == nil { continue @@ -125,7 +125,7 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo var source cloudprofilesync.Source password, err := r.getCredential(ctx, updates.Source.OCI.Password) if err != nil { - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", fmt.Sprintf("failed to get credential for garbage collection: %s", err), err) + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, fmt.Sprintf("failed to get credential for garbage collection: %s", err), err) } src, err := r.OCISourceFactory.Create(cloudprofilesync.OCIParams{ Registry: updates.Source.OCI.Registry, @@ -135,18 +135,18 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo Parallel: 1, }, updates.Source.OCI.Insecure) if err != nil { - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", fmt.Sprintf("failed to initialize OCI source for garbage collection: %s", err), fmt.Errorf("failed to initialize OCI source for garbage collection: %w", err)) + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, fmt.Sprintf("failed to initialize OCI source for garbage collection: %s", err), fmt.Errorf("failed to initialize OCI source for garbage collection: %w", err)) } source = src versions, err := source.GetVersions(ctx) if err != nil { - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", fmt.Sprintf("failed to list source versions for garbage collection: %s", err), fmt.Errorf("failed to list source versions for garbage collection: %w", err)) + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, fmt.Sprintf("failed to list source versions for garbage collection: %s", err), fmt.Errorf("failed to list source versions for garbage collection: %w", err)) } referencedVersions, err := r.getReferencedVersions(ctx, mcp.Name, updates.ImageName) if err != nil { - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", fmt.Sprintf("failed to determine referenced versions for garbage collection: %s", err), fmt.Errorf("failed to determine referenced versions for garbage collection: %w", err)) + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, fmt.Sprintf("failed to determine referenced versions for garbage collection: %s", err), fmt.Errorf("failed to determine referenced versions for garbage collection: %w", err)) } cutoff := time.Now().Add(-updates.GarbageCollection.MaxAge.Duration) @@ -169,7 +169,7 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo log.V(1).Info("garbage collection validation failed, skipping", "image", updates.ImageName) continue } - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "GarbageCollectionFailed", fmt.Sprintf("failed to delete image versions during garbage collection: %s", err), fmt.Errorf("failed to delete image versions: %w", err)) + return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, fmt.Sprintf("failed to delete image versions during garbage collection: %s", err), fmt.Errorf("failed to delete image versions: %w", err)) } for v := range versionsToDelete { log.Info("deleted image version from CloudProfile", "image", updates.ImageName, "version", v) @@ -375,12 +375,12 @@ func CloudProfileSpecToGardener(spec *v1alpha1.CloudProfileSpec) gardenerv1beta1 } } -func (r *Reconciler) failWithStatusUpdate(ctx context.Context, mcp *v1alpha1.ManagedCloudProfile, reason, message string, returnErr error) error { +func (r *Reconciler) failWithStatusUpdate(ctx context.Context, mcp *v1alpha1.ManagedCloudProfile, message string, returnErr error) error { if patchErr := r.patchStatusAndCondition(ctx, mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ Type: CloudProfileAppliedConditionType, Status: metav1.ConditionFalse, ObservedGeneration: mcp.Generation, - Reason: reason, + Reason: "GarbageCollectionFailed", Message: message, }); patchErr != nil { return fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, returnErr) diff --git a/controllers/managedcloudprofile_controller_test.go b/controllers/managedcloudprofile_controller_test.go index a33dc5d..f2911d3 100644 --- a/controllers/managedcloudprofile_controller_test.go +++ b/controllers/managedcloudprofile_controller_test.go @@ -41,7 +41,8 @@ func (m *mockOCIFactory) Create(params cloudprofilesync.OCIParams, insecure bool } var _ = Describe("The ManagedCloudProfile reconciler", func() { - + amd64 := "amd64" + version := "1.0.0" AfterEach(func(ctx SpecContext) { var mcpList v1alpha1.ManagedCloudProfileList Expect(k8sClient.List(ctx, &mcpList)).To(Succeed()) @@ -97,6 +98,7 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { It("should copy the spec of a ManagedCloudProfile to the respective CloudProfile", func(ctx SpecContext) { var mcp v1alpha1.ManagedCloudProfile mcp.Name = "test-mcp" + usable := true mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ Regions: []gardenerv1beta1.Region{{Name: "foo"}}, MachineImages: []gardenerv1beta1.MachineImage{ @@ -115,8 +117,8 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { MachineTypes: []gardenerv1beta1.MachineType{ { Name: "baz", - Architecture: ptr.To("amd64"), - Usable: ptr.To(true), + Architecture: &amd64, + Usable: &usable, }, }, } @@ -165,13 +167,14 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { It("invokes the image updater based on an image source", func(ctx SpecContext) { var mcp v1alpha1.ManagedCloudProfile mcp.Name = "test-oci" + usable := true mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{ Regions: []gardenerv1beta1.Region{{Name: "foo"}}, MachineTypes: []gardenerv1beta1.MachineType{ { Name: "baz", - Architecture: ptr.To("amd64"), - Usable: ptr.To(true), + Architecture: &amd64, + Usable: &usable, }, }, } @@ -363,7 +366,7 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { Machine: gardenerv1beta1.Machine{ Image: &gardenerv1beta1.ShootMachineImage{ Name: "preserve-image", - Version: ptr.To("1.0.0"), + Version: &version, }, }, }, @@ -452,7 +455,7 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { Machine: gardenerv1beta1.Machine{ Image: &gardenerv1beta1.ShootMachineImage{ Name: "shoot-preserve-image", - Version: ptr.To("1.0.0"), + Version: &version, }, }, }, @@ -681,6 +684,7 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { It("reports failure when CloudProfile is already owned by another controller", func(ctx SpecContext) { var cloudProfile gardenerv1beta1.CloudProfile cloudProfile.Name = "test-owned" + usable := true cloudProfile.Spec = gardenerv1beta1.CloudProfileSpec{ Regions: []gardenerv1beta1.Region{{Name: "existing-region"}}, MachineImages: []gardenerv1beta1.MachineImage{ @@ -692,7 +696,7 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() { }, }, MachineTypes: []gardenerv1beta1.MachineType{ - {Name: "existing-type", Architecture: ptr.To("amd64"), Usable: ptr.To(true)}, + {Name: "existing-type", Architecture: &amd64, Usable: &usable}, }, } cloudProfile.OwnerReferences = []metav1.OwnerReference{ From ca3364acab4bd20e6b843e78b26538dd58b53c8e Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Thu, 19 Mar 2026 15:17:52 +0100 Subject: [PATCH 12/14] fixed spelling errors --- cloudprofilesync/imageupdater.go | 2 +- controllers/managedcloudprofile_controller.go | 26 +++++++++---------- ...c.cobaltcore.dev_managedcloudprofiles.yaml | 6 ++--- crd/core.gardener.cloud_cloudprofiles.yaml | 4 +-- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/cloudprofilesync/imageupdater.go b/cloudprofilesync/imageupdater.go index 59d85b4..a93e206 100644 --- a/cloudprofilesync/imageupdater.go +++ b/cloudprofilesync/imageupdater.go @@ -47,7 +47,7 @@ func (iu *ImageUpdater) Update(ctx context.Context, cpSpec *gardenerv1beta1.Clou iu.Log.Info("checked source", "image", iu.ImageName) sourceImages = filterImages(iu.Log, sourceImages) // Images from a source arrive in no guaranteed order. A changed order - // in the source images may lead to a chenged order in the CloudProfile, + // in the source images may lead to a changed order in the CloudProfile, // causing unnecesscary reconciliations. slices.SortFunc(sourceImages, func(a, b SourceImage) int { return cmp.Compare(a.Version, b.Version) diff --git a/controllers/managedcloudprofile_controller.go b/controllers/managedcloudprofile_controller.go index 8563fd2..e1193dd 100644 --- a/controllers/managedcloudprofile_controller.go +++ b/controllers/managedcloudprofile_controller.go @@ -358,20 +358,20 @@ func applyCondition(conditions []metav1.Condition, cond metav1.Condition) []meta } func CloudProfileSpecToGardener(spec *v1alpha1.CloudProfileSpec) gardenerv1beta1.CloudProfileSpec { - cpy := spec.DeepCopy() + cpu := spec.DeepCopy() return gardenerv1beta1.CloudProfileSpec{ - CABundle: cpy.CABundle, - Kubernetes: cpy.Kubernetes, - MachineImages: cpy.MachineImages, - MachineTypes: cpy.MachineTypes, - ProviderConfig: cpy.ProviderConfig, - Regions: cpy.Regions, - SeedSelector: cpy.SeedSelector, - Type: cpy.Type, - VolumeTypes: cpy.VolumeTypes, - Bastion: cpy.Bastion, - Limits: cpy.Limits, - MachineCapabilities: cpy.MachineCapabilities, + CABundle: cpu.CABundle, + Kubernetes: cpu.Kubernetes, + MachineImages: cpu.MachineImages, + MachineTypes: cpu.MachineTypes, + ProviderConfig: cpu.ProviderConfig, + Regions: cpu.Regions, + SeedSelector: cpu.SeedSelector, + Type: cpu.Type, + VolumeTypes: cpu.VolumeTypes, + Bastion: cpu.Bastion, + Limits: cpu.Limits, + MachineCapabilities: cpu.MachineCapabilities, } } diff --git a/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml b/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml index 9b26191..7546e34 100644 --- a/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml +++ b/crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml @@ -431,7 +431,7 @@ spec: properties: matchExpressions: description: matchExpressions is a list of label selector - requirements. The requirements are ANDed. + requirements. The requirements are ANDead. items: description: |- A label selector requirement is a selector that contains values, a key, and an operator that @@ -468,7 +468,7 @@ spec: description: |- matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. + operator is "In", and the values array contains only "value". The requirements are ANDead. type: object providerTypes: description: Providers is optional and can be used by restricting @@ -532,7 +532,7 @@ spec: maxAge: description: |- MaxAge defines the maximum age for images to keep. Images older than - now - MaxAge are eligible for deletion. + now MaxAge are eligible for deletion. type: string type: object imageName: diff --git a/crd/core.gardener.cloud_cloudprofiles.yaml b/crd/core.gardener.cloud_cloudprofiles.yaml index 54f93b8..c68e8c6 100644 --- a/crd/core.gardener.cloud_cloudprofiles.yaml +++ b/crd/core.gardener.cloud_cloudprofiles.yaml @@ -414,7 +414,7 @@ spec: properties: matchExpressions: description: matchExpressions is a list of label selector requirements. - The requirements are ANDed. + The requirements are ANDead. items: description: |- A label selector requirement is a selector that contains values, a key, and an operator that @@ -451,7 +451,7 @@ spec: description: |- matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. + operator is "In", and the values array contains only "value". The requirements are ANDead. type: object providerTypes: description: Providers is optional and can be used by restricting From 234bc8cd1b1e56dbf8cc8c2e284ffcf4cf3d1329 Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Thu, 19 Mar 2026 15:41:33 +0100 Subject: [PATCH 13/14] fixed reuse check --- .gitignore | 2 +- LICENSES/CC0-1.0.txt | 121 +++++++++++++++++++++++++++++++++++++++++++ REUSE.toml | 8 +++ 3 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 LICENSES/CC0-1.0.txt diff --git a/.gitignore b/.gitignore index ed86553..17f74a6 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,2 @@ .DS_Store -build/ +build/.claude/ \ No newline at end of file diff --git a/LICENSES/CC0-1.0.txt b/LICENSES/CC0-1.0.txt new file mode 100644 index 0000000..0e259d4 --- /dev/null +++ b/LICENSES/CC0-1.0.txt @@ -0,0 +1,121 @@ +Creative Commons Legal Code + +CC0 1.0 Universal + + CREATIVE COMMONS CORPORATION IS NOT A LAW FIRM AND DOES NOT PROVIDE + LEGAL SERVICES. DISTRIBUTION OF THIS DOCUMENT DOES NOT CREATE AN + ATTORNEY-CLIENT RELATIONSHIP. CREATIVE COMMONS PROVIDES THIS + INFORMATION ON AN "AS-IS" BASIS. CREATIVE COMMONS MAKES NO WARRANTIES + REGARDING THE USE OF THIS DOCUMENT OR THE INFORMATION OR WORKS + PROVIDED HEREUNDER, AND DISCLAIMS LIABILITY FOR DAMAGES RESULTING FROM + THE USE OF THIS DOCUMENT OR THE INFORMATION OR WORKS PROVIDED + HEREUNDER. + +Statement of Purpose + +The laws of most jurisdictions throughout the world automatically confer +exclusive Copyright and Related Rights (defined below) upon the creator +and subsequent owner(s) (each and all, an "owner") of an original work of +authorship and/or a database (each, a "Work"). + +Certain owners wish to permanently relinquish those rights to a Work for +the purpose of contributing to a commons of creative, cultural and +scientific works ("Commons") that the public can reliably and without fear +of later claims of infringement build upon, modify, incorporate in other +works, reuse and redistribute as freely as possible in any form whatsoever +and for any purposes, including without limitation commercial purposes. +These owners may contribute to the Commons to promote the ideal of a free +culture and the further production of creative, cultural and scientific +works, or to gain reputation or greater distribution for their Work in +part through the use and efforts of others. + +For these and/or other purposes and motivations, and without any +expectation of additional consideration or compensation, the person +associating CC0 with a Work (the "Affirmer"), to the extent that he or she +is an owner of Copyright and Related Rights in the Work, voluntarily +elects to apply CC0 to the Work and publicly distribute the Work under its +terms, with knowledge of his or her Copyright and Related Rights in the +Work and the meaning and intended legal effect of CC0 on those rights. + +1. Copyright and Related Rights. A Work made available under CC0 may be +protected by copyright and related or neighboring rights ("Copyright and +Related Rights"). Copyright and Related Rights include, but are not +limited to, the following: + + i. the right to reproduce, adapt, distribute, perform, display, + communicate, and translate a Work; + ii. moral rights retained by the original author(s) and/or performer(s); +iii. publicity and privacy rights pertaining to a person's image or + likeness depicted in a Work; + iv. rights protecting against unfair competition in regards to a Work, + subject to the limitations in paragraph 4(a), below; + v. rights protecting the extraction, dissemination, use and reuse of data + in a Work; + vi. database rights (such as those arising under Directive 96/9/EC of the + European Parliament and of the Council of 11 March 1996 on the legal + protection of databases, and under any national implementation + thereof, including any amended or successor version of such + directive); and +vii. other similar, equivalent or corresponding rights throughout the + world based on applicable law or treaty, and any national + implementations thereof. + +2. Waiver. To the greatest extent permitted by, but not in contravention +of, applicable law, Affirmer hereby overtly, fully, permanently, +irrevocably and unconditionally waives, abandons, and surrenders all of +Affirmer's Copyright and Related Rights and associated claims and causes +of action, whether now known or unknown (including existing as well as +future claims and causes of action), in the Work (i) in all territories +worldwide, (ii) for the maximum duration provided by applicable law or +treaty (including future time extensions), (iii) in any current or future +medium and for any number of copies, and (iv) for any purpose whatsoever, +including without limitation commercial, advertising or promotional +purposes (the "Waiver"). Affirmer makes the Waiver for the benefit of each +member of the public at large and to the detriment of Affirmer's heirs and +successors, fully intending that such Waiver shall not be subject to +revocation, rescission, cancellation, termination, or any other legal or +equitable action to disrupt the quiet enjoyment of the Work by the public +as contemplated by Affirmer's express Statement of Purpose. + +3. Public License Fallback. Should any part of the Waiver for any reason +be judged legally invalid or ineffective under applicable law, then the +Waiver shall be preserved to the maximum extent permitted taking into +account Affirmer's express Statement of Purpose. In addition, to the +extent the Waiver is so judged Affirmer hereby grants to each affected +person a royalty-free, non transferable, non sublicensable, non exclusive, +irrevocable and unconditional license to exercise Affirmer's Copyright and +Related Rights in the Work (i) in all territories worldwide, (ii) for the +maximum duration provided by applicable law or treaty (including future +time extensions), (iii) in any current or future medium and for any number +of copies, and (iv) for any purpose whatsoever, including without +limitation commercial, advertising or promotional purposes (the +"License"). The License shall be deemed effective as of the date CC0 was +applied by Affirmer to the Work. Should any part of the License for any +reason be judged legally invalid or ineffective under applicable law, such +partial invalidity or ineffectiveness shall not invalidate the remainder +of the License, and in such case Affirmer hereby affirms that he or she +will not (i) exercise any of his or her remaining Copyright and Related +Rights in the Work or (ii) assert any associated claims and causes of +action with respect to the Work, in either case contrary to Affirmer's +express Statement of Purpose. + +4. Limitations and Disclaimers. + + a. No trademark or patent rights held by Affirmer are waived, abandoned, + surrendered, licensed or otherwise affected by this document. + b. Affirmer offers the Work as-is and makes no representations or + warranties of any kind concerning the Work, express, implied, + statutory or otherwise, including without limitation warranties of + title, merchantability, fitness for a particular purpose, non + infringement, or the absence of latent or other defects, accuracy, or + the present or absence of errors, whether or not discoverable, all to + the greatest extent permissible under applicable law. + c. Affirmer disclaims responsibility for clearing rights of other persons + that may apply to the Work or any use thereof, including without + limitation any person's Copyright and Related Rights in the Work. + Further, Affirmer disclaims responsibility for obtaining any necessary + consents, permissions or other rights required for any use of the + Work. + d. Affirmer understands and acknowledges that Creative Commons is not a + party to this document and has no duty or obligation with respect to + this CC0 or use of the Work. diff --git a/REUSE.toml b/REUSE.toml index 36a6efc..cd9f98d 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -32,3 +32,11 @@ path = [ ] SPDX-FileCopyrightText = "2020 The Kubernetes Authors" SPDX-License-Identifier = "Apache-2.0" + +[[annotations]] +path = [ + "build/*", + ".claude/*", +] +SPDX-FileCopyrightText = "NOASSERTION" +SPDX-License-Identifier = "CC0-1.0" From 1d59c2d5d2bb4a08d4eed8adefd4352ef00c5833 Mon Sep 17 00:00:00 2001 From: valeryia-hurynovich Date: Fri, 20 Mar 2026 15:52:07 +0100 Subject: [PATCH 14/14] fixed error handling and reconcile func --- controllers/managedcloudprofile_controller.go | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/controllers/managedcloudprofile_controller.go b/controllers/managedcloudprofile_controller.go index e1193dd..b6249a3 100644 --- a/controllers/managedcloudprofile_controller.go +++ b/controllers/managedcloudprofile_controller.go @@ -60,7 +60,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } log.Info("reconciled ManagedCloudProfile") - return r.reconcileGarbageCollection(ctx, log, &mcp) + if err := r.reconcileGarbageCollection(ctx, log, &mcp); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: 5 * time.Minute}, nil } func (r *Reconciler) reconcileCloudProfile(ctx context.Context, log logr.Logger, mcp *v1alpha1.ManagedCloudProfile) error { @@ -111,13 +114,13 @@ func (r *Reconciler) reconcileCloudProfile(ctx context.Context, log logr.Logger, return nil } -func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Logger, mcp *v1alpha1.ManagedCloudProfile) (ctrl.Result, error) { +func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Logger, mcp *v1alpha1.ManagedCloudProfile) error { for _, updates := range mcp.Spec.MachineImageUpdates { if updates.GarbageCollection == nil || !updates.GarbageCollection.Enabled { continue } if updates.GarbageCollection.MaxAge.Duration < 0 { - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, "garbage collection maxAge must not be negative", fmt.Errorf("invalid garbage collection maxAge: %s", updates.GarbageCollection.MaxAge.String())) + return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("invalid garbage collection maxAge: %s", updates.GarbageCollection.MaxAge.String())) } if updates.Source.OCI == nil { continue @@ -125,7 +128,7 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo var source cloudprofilesync.Source password, err := r.getCredential(ctx, updates.Source.OCI.Password) if err != nil { - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, fmt.Sprintf("failed to get credential for garbage collection: %s", err), err) + return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("failed to get credential for garbage collection: %w", err)) } src, err := r.OCISourceFactory.Create(cloudprofilesync.OCIParams{ Registry: updates.Source.OCI.Registry, @@ -135,18 +138,18 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo Parallel: 1, }, updates.Source.OCI.Insecure) if err != nil { - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, fmt.Sprintf("failed to initialize OCI source for garbage collection: %s", err), fmt.Errorf("failed to initialize OCI source for garbage collection: %w", err)) + return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("failed to initialize OCI source for garbage collection: %w", err)) } source = src versions, err := source.GetVersions(ctx) if err != nil { - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, fmt.Sprintf("failed to list source versions for garbage collection: %s", err), fmt.Errorf("failed to list source versions for garbage collection: %w", err)) + return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("failed to list source versions for garbage collection: %w", err)) } referencedVersions, err := r.getReferencedVersions(ctx, mcp.Name, updates.ImageName) if err != nil { - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, fmt.Sprintf("failed to determine referenced versions for garbage collection: %s", err), fmt.Errorf("failed to determine referenced versions for garbage collection: %w", err)) + return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("failed to determine referenced versions for garbage collection: %w", err)) } cutoff := time.Now().Add(-updates.GarbageCollection.MaxAge.Duration) @@ -169,7 +172,7 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo log.V(1).Info("garbage collection validation failed, skipping", "image", updates.ImageName) continue } - return ctrl.Result{}, r.failWithStatusUpdate(ctx, mcp, fmt.Sprintf("failed to delete image versions during garbage collection: %s", err), fmt.Errorf("failed to delete image versions: %w", err)) + return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("failed to delete image versions: %w", err)) } for v := range versionsToDelete { log.Info("deleted image version from CloudProfile", "image", updates.ImageName, "version", v) @@ -177,7 +180,7 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo } } - return ctrl.Result{RequeueAfter: 5 * time.Minute}, nil + return nil } func (r *Reconciler) deleteVersions(ctx context.Context, cloudProfileName, imageName string, versionsToDelete map[string]struct{}) error { @@ -375,13 +378,13 @@ func CloudProfileSpecToGardener(spec *v1alpha1.CloudProfileSpec) gardenerv1beta1 } } -func (r *Reconciler) failWithStatusUpdate(ctx context.Context, mcp *v1alpha1.ManagedCloudProfile, message string, returnErr error) error { +func (r *Reconciler) failWithStatusUpdate(ctx context.Context, mcp *v1alpha1.ManagedCloudProfile, returnErr error) error { if patchErr := r.patchStatusAndCondition(ctx, mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{ Type: CloudProfileAppliedConditionType, Status: metav1.ConditionFalse, ObservedGeneration: mcp.Generation, Reason: "GarbageCollectionFailed", - Message: message, + Message: returnErr.Error(), }); patchErr != nil { return fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, returnErr) }