Skip to content

Fixed controller logic due to clarifications #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
valeryia-hurynovich merged 1 commit into from
Mar 31, 2026
Merged

Fixed controller logic due to clarifications #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions api/v1alpha1/managedcloudprofile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ type ManagedCloudProfileSpec struct {
// MachineImageUpdates contains the source and provider information to automate machine images.
// +optional
MachineImageUpdates []MachineImageUpdate `json:"machineImageUpdates,omitempty"`
// GarbageCollection contains configuration for automated garbage collection
// +optional
GarbageCollection *GarbageCollectionConfig `json:"garbageCollection,omitempty"`
Comment on lines +23 to +25

@coderabbitai coderabbitai Bot Mar 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Regenerate the CRD for this API move.

ManagedCloudProfileSpec now exposes spec.garbageCollection, but crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml:519-611 still only defines machineImageUpdates[].garbageCollection and does not add the new top-level property. In-cluster that means the apiserver can keep validating the old shape and prune/reject the new field, so mcp.Spec.GarbageCollection never becomes effective. Please update/regenerate the CRD in the same PR, and call out a migration path if existing objects still rely on the nested field.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@api/v1alpha1/managedcloudprofile.go` around lines 23 - 25, The CRD is missing
the new top-level spec.garbageCollection field added to ManagedCloudProfileSpec;
update the CRD generation so the schema includes spec.garbageCollection
(GarbageCollection *GarbageCollectionConfig) alongside the existing
machineImageUpdates[].garbageCollection definition, regenerate the YAML, and
commit it in this PR; also provide a migration note/script to copy existing
nested machineImageUpdates[].garbageCollection values into the new
spec.garbageCollection for existing ManagedCloudProfile objects so the API
server won't prune the new field at admission time.

}

// Copy the cloud profile spec to override some validation
Expand Down Expand Up @@ -97,9 +100,6 @@ 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 {
Expand Down
10 changes: 5 additions & 5 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 12 additions & 10 deletions controllers/managedcloudprofile_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,21 +115,25 @@ func (r *Reconciler) reconcileCloudProfile(ctx context.Context, log logr.Logger,
}

func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Logger, mcp *v1alpha1.ManagedCloudProfile) error {
if mcp.Spec.GarbageCollection == nil || !mcp.Spec.GarbageCollection.Enabled {
return nil
}
if mcp.Spec.GarbageCollection.MaxAge.Duration < 0 {
return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("invalid garbage collection maxAge: %s", mcp.Spec.GarbageCollection.MaxAge.String()))
}

cutoff := time.Now().Add(-mcp.Spec.GarbageCollection.MaxAge.Duration)

for _, updates := range mcp.Spec.MachineImageUpdates {
if updates.GarbageCollection == nil || !updates.GarbageCollection.Enabled {
continue
}
if updates.GarbageCollection.MaxAge.Duration < 0 {
return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("invalid garbage collection maxAge: %s", updates.GarbageCollection.MaxAge.String()))
}
if updates.Source.OCI == nil {
continue
}
var source cloudprofilesync.Source

password, err := r.getCredential(ctx, updates.Source.OCI.Password)
if err != nil {
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,
Repository: updates.Source.OCI.Repository,
Expand All @@ -140,9 +144,8 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo
if err != nil {
return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("failed to initialize OCI source for garbage collection: %w", err))
}
source = src

versions, err := source.GetVersions(ctx)
versions, err := src.GetVersions(ctx)
if err != nil {
return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("failed to list source versions for garbage collection: %w", err))
}
Expand All @@ -152,7 +155,6 @@ func (r *Reconciler) reconcileGarbageCollection(ctx context.Context, log logr.Lo
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)
versionsToDelete := make(map[string]struct{})
for _, v := range versions {
if v.CreatedAt.IsZero() {
Expand Down
58 changes: 30 additions & 28 deletions controllers/managedcloudprofile_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,13 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() {
Insecure: true,
},
},
GarbageCollection: &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 0},
},
},
}
mcp.Spec.GarbageCollection = &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 0},
}

Expect(k8sClient.Create(ctx, &mcp)).To(Succeed())

Eventually(func(g Gomega) v1alpha1.ReconcileStatus {
Expand Down Expand Up @@ -399,12 +400,13 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() {
Insecure: true,
},
},
GarbageCollection: &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 0},
},
},
}
mcp.Spec.GarbageCollection = &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 0},
}

Expect(k8sClient.Create(ctx, &mcp)).To(Succeed())

Eventually(func(g Gomega) v1alpha1.ReconcileStatus {
Expand Down Expand Up @@ -487,12 +489,12 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() {
Insecure: true,
},
},
GarbageCollection: &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 0},
},
},
}
mcp.Spec.GarbageCollection = &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 0},
}
Expect(k8sClient.Create(ctx, &mcp)).To(Succeed())

Eventually(func(g Gomega) v1alpha1.ReconcileStatus {
Expand Down Expand Up @@ -542,12 +544,12 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() {
},
},
},
GarbageCollection: &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 3600000000000},
},
},
}
mcp.Spec.GarbageCollection = &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 3600000000000},
}
Expect(k8sClient.Create(ctx, &mcp)).To(Succeed())

Eventually(func(g Gomega) v1alpha1.ReconcileStatus {
Expand Down Expand Up @@ -581,12 +583,12 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() {
Insecure: true,
},
},
GarbageCollection: &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 3600000000000},
},
},
}
mcp.Spec.GarbageCollection = &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 3600000000000},
}
Expect(k8sClient.Create(ctx, &mcp)).To(Succeed())

Eventually(func(g Gomega) v1alpha1.ReconcileStatus {
Expand Down Expand Up @@ -629,12 +631,12 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() {
Insecure: true,
},
},
GarbageCollection: &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 3600000000000},
},
},
}
mcp.Spec.GarbageCollection = &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 3600000000000},
}
Expect(k8sClient.Create(ctx, &mcp)).To(Succeed())

Eventually(func(g Gomega) v1alpha1.ReconcileStatus {
Expand Down Expand Up @@ -785,12 +787,12 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() {
Insecure: true,
},
},
GarbageCollection: &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 0},
},
},
}
mcp.Spec.GarbageCollection = &v1alpha1.GarbageCollectionConfig{
Enabled: true,
MaxAge: metav1.Duration{Duration: 0},
}
Expect(k8sClient.Create(ctx, &mcp)).To(Succeed())

Eventually(func(g Gomega) v1alpha1.ReconcileStatus {
Expand Down
Loading