diff --git a/api/v1alpha1/rollout_types.go b/api/v1alpha1/rollout_types.go index 98b1365..c09c59c 100644 --- a/api/v1alpha1/rollout_types.go +++ b/api/v1alpha1/rollout_types.go @@ -99,10 +99,26 @@ type RolloutSpec struct { // VersionHistoryLimit defines the maximum number of entries to keep in the deployment history // +kubebuilder:validation:Minimum=1 - // +kubebuilder:default=5 + // +kubebuilder:default=10 // +optional VersionHistoryLimit *int32 `json:"versionHistoryLimit,omitempty"` + // AvailableReleasesRetentionDays defines how many days of available releases to keep based on creation timestamp + // When history is full, releases older than this retention period may be removed. + // Defaults to 7 days if not specified. + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:default=7 + // +optional + AvailableReleasesRetentionDays *int32 `json:"availableReleasesRetentionDays,omitempty"` + + // AvailableReleasesMinCount defines the minimum number of available releases to always keep + // When history is full, at least this many releases will be retained regardless of other criteria. + // Defaults to 30 if not specified. + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:default=30 + // +optional + AvailableReleasesMinCount *int32 `json:"availableReleasesMinCount,omitempty"` + // BakeTime specifies how long to wait after bake starts before marking as successful // If no errors happen within the bake time, the rollout is baked successfully. // If not specified, no bake time is enforced. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index b4d35f5..140df0d 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -461,6 +461,16 @@ func (in *RolloutSpec) DeepCopyInto(out *RolloutSpec) { *out = new(int32) **out = **in } + if in.AvailableReleasesRetentionDays != nil { + in, out := &in.AvailableReleasesRetentionDays, &out.AvailableReleasesRetentionDays + *out = new(int32) + **out = **in + } + if in.AvailableReleasesMinCount != nil { + in, out := &in.AvailableReleasesMinCount, &out.AvailableReleasesMinCount + *out = new(int32) + **out = **in + } if in.BakeTime != nil { in, out := &in.BakeTime, &out.BakeTime *out = new(v1.Duration) diff --git a/config/crd/bases/kuberik.com_rollouts.yaml b/config/crd/bases/kuberik.com_rollouts.yaml index 2c1c21a..ef6884e 100644 --- a/config/crd/bases/kuberik.com_rollouts.yaml +++ b/config/crd/bases/kuberik.com_rollouts.yaml @@ -67,6 +67,24 @@ spec: spec: description: RolloutSpec defines the desired state of Rollout. properties: + availableReleasesMinCount: + default: 30 + description: |- + AvailableReleasesMinCount defines the minimum number of available releases to always keep + When history is full, at least this many releases will be retained regardless of other criteria. + Defaults to 30 if not specified. + format: int32 + minimum: 1 + type: integer + availableReleasesRetentionDays: + default: 7 + description: |- + AvailableReleasesRetentionDays defines how many days of available releases to keep based on creation timestamp + When history is full, releases older than this retention period may be removed. + Defaults to 7 days if not specified. + format: int32 + minimum: 1 + type: integer bakeTime: description: |- BakeTime specifies how long to wait after bake starts before marking as successful @@ -198,7 +216,7 @@ spec: type: object x-kubernetes-map-type: atomic versionHistoryLimit: - default: 5 + default: 10 description: VersionHistoryLimit defines the maximum number of entries to keep in the deployment history format: int32 diff --git a/internal/controller/rollout_controller.go b/internal/controller/rollout_controller.go index 45db25b..576d1dc 100644 --- a/internal/controller/rollout_controller.go +++ b/internal/controller/rollout_controller.go @@ -1210,12 +1210,16 @@ func (r *RolloutReconciler) deployRelease(ctx context.Context, rollout *rolloutv BakeEndTime: nil, // Will be set when bake completes (succeeds, fails, or times out) }}, rollout.Status.History...) // Limit history size if specified - versionHistoryLimit := int32(5) // default value + versionHistoryLimit := int32(10) // default value if rollout.Spec.VersionHistoryLimit != nil { versionHistoryLimit = *rollout.Spec.VersionHistoryLimit } if int32(len(rollout.Status.History)) > versionHistoryLimit { rollout.Status.History = rollout.Status.History[:versionHistoryLimit] + // If history is full, remove available versions based on retention criteria + if len(rollout.Status.History) > 0 { + r.cleanupOldAvailableReleases(rollout, log) + } } // Update the condition message to reflect if gates were bypassed @@ -1317,6 +1321,116 @@ func (r *RolloutReconciler) deployRelease(ctx context.Context, rollout *rolloutv return nil } +// cleanupOldAvailableReleases removes available releases based on multiple retention criteria. +// This is called when history is full to prevent AvailableReleases from growing unbounded. +// It keeps the maximum number of releases that satisfy at least one of: +// 1. Everything up to and including the history entry that's furthest down in AvailableReleases +// 2. All releases created within the retention period (configurable via AvailableReleasesRetentionDays, default: 7 days) +// 3. At least a minimum number of releases (configurable via AvailableReleasesMinCount, default: 30) +// We keep the maximum number that satisfies any of these criteria. +func (r *RolloutReconciler) cleanupOldAvailableReleases(rollout *rolloutv1alpha1.Rollout, log logr.Logger) { + if len(rollout.Status.History) == 0 || len(rollout.Status.AvailableReleases) == 0 { + return + } + + // Get configurable retention values with defaults + retentionDays := int32(7) // default value + if rollout.Spec.AvailableReleasesRetentionDays != nil { + retentionDays = *rollout.Spec.AvailableReleasesRetentionDays + } + + minReleases := int32(30) // default value + if rollout.Spec.AvailableReleasesMinCount != nil { + minReleases = *rollout.Spec.AvailableReleasesMinCount + } + + cutoffTime := r.now().Add(-time.Duration(retentionDays) * 24 * time.Hour) + + updatedReleases := CalculateAvailableReleasesToKeep( + rollout.Status.AvailableReleases, + rollout.Status.History, + cutoffTime, + int(minReleases), + ) + + if len(updatedReleases) < len(rollout.Status.AvailableReleases) { + removedCount := len(rollout.Status.AvailableReleases) - len(updatedReleases) + rollout.Status.AvailableReleases = updatedReleases + log.Info("Cleaned up old available releases", + "removed", removedCount, + "remaining", len(updatedReleases)) + } +} + +// CalculateAvailableReleasesToKeep determines which available releases should be retained based on multiple criteria. +// It returns a slice of VersionInfo that should be kept. +// releases: the current list of available releases (sorted oldest to newest) +// history: the current deployment history +// cutoffTime: releases older than this may be removed (unless kept by other criteria) +// minReleases: keep at least this many newest releases +func CalculateAvailableReleasesToKeep( + releases []rolloutv1alpha1.VersionInfo, + history []rolloutv1alpha1.DeploymentHistoryEntry, + cutoffTime time.Time, + minReleases int, +) []rolloutv1alpha1.VersionInfo { + if len(releases) == 0 { + return nil + } + + // Criterion 1: Keep everything from the lowest history entry index to the end + minHistoryIndex := len(releases) + for _, historyEntry := range history { + targetTag := historyEntry.Version.Tag + for i := 0; i < len(releases); i++ { + if releases[i].Tag == targetTag { + if i < minHistoryIndex { + minHistoryIndex = i + } + break + } + } + } + criterion1KeepFromEnd := 0 + if minHistoryIndex < len(releases) { + criterion1KeepFromEnd = len(releases) - minHistoryIndex + } + + // Criterion 2: Keep releases within the retention period (newer than cutoffTime) + retentionTimeIndex := 0 + for i := len(releases) - 1; i >= 0; i-- { + if releases[i].Created != nil && releases[i].Created.Time.Before(cutoffTime) { + // This release is too old, so we keep everything after it (i+1 onwards) + retentionTimeIndex = i + 1 + break + } + } + criterion2KeepFromEnd := len(releases) - retentionTimeIndex + + // Criterion 3: Keep at least a minimum number of releases + criterion3KeepFromEnd := minReleases + if criterion3KeepFromEnd > len(releases) { + criterion3KeepFromEnd = len(releases) + } + + // Keep the maximum number of releases that satisfy any of the criteria + keepFromEnd := criterion1KeepFromEnd + if criterion2KeepFromEnd > keepFromEnd { + keepFromEnd = criterion2KeepFromEnd + } + if criterion3KeepFromEnd > keepFromEnd { + keepFromEnd = criterion3KeepFromEnd + } + + // Ensure we don't exceed the actual list length + if keepFromEnd >= len(releases) { + return releases + } + + // Return the newest keepFromEnd releases + return releases[len(releases)-keepFromEnd:] +} + // patchOCIRepositories finds OCIRepositories with rollout annotation and patches their tag. func (r *RolloutReconciler) patchOCIRepositories(ctx context.Context, rollout *rolloutv1alpha1.Rollout, wantedRelease string) error { log := logf.FromContext(ctx) diff --git a/internal/controller/rollout_controller_test.go b/internal/controller/rollout_controller_test.go index d72a0e7..ad56a7f 100644 --- a/internal/controller/rollout_controller_test.go +++ b/internal/controller/rollout_controller_test.go @@ -18,6 +18,7 @@ package controller import ( "context" + "fmt" "time" "github.com/docker/cli/cli/config/configfile" @@ -455,6 +456,46 @@ var _ = Describe("Rollout Controller", func() { Expect(updatedRollout.Status.History[2].Version.Tag).To(Equal("0.2.0")) }) + It("should use default history limit of 10 when not specified", func() { + By("Ensuring VersionHistoryLimit uses default") + rollout := &rolloutv1alpha1.Rollout{} + err := k8sClient.Get(ctx, typeNamespacedName, rollout) + Expect(err).NotTo(HaveOccurred()) + // Kubebuilder defaults may be applied, but we want to test the default behavior + // So we'll just proceed without setting it explicitly + + By("Reconciling the resources") + controllerReconciler := &RolloutReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + // Deploy 11 versions to exceed the default limit of 10 + for i := 1; i <= 11; i++ { + version := fmt.Sprintf("0.%d.0", i) + imagePolicy.Status.LatestRef = &imagev1beta2.ImageRef{ + Tag: version, + } + Expect(k8sClient.Status().Update(ctx, imagePolicy)).To(Succeed()) + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + } + + By("Verifying that history is limited to default of 10") + updatedRollout := &rolloutv1alpha1.Rollout{} + err = k8sClient.Get(ctx, typeNamespacedName, updatedRollout) + Expect(err).NotTo(HaveOccurred()) + + Expect(updatedRollout.Status.History).To(HaveLen(10), "History should be limited to default of 10 entries") + // Verify the most recent versions are present (0.11.0 through 0.2.0) + Expect(updatedRollout.Status.History[0].Version.Tag).To(Equal("0.11.0")) + Expect(updatedRollout.Status.History[9].Version.Tag).To(Equal("0.2.0")) + // Verify 0.1.0 was removed (it's the oldest) + Expect(updatedRollout.Status.History).To(Not(ContainElement(HaveField("Version.Tag", "0.1.0")))) + }) + It("should respect the wanted version override", func() { By("Setting available releases") rollout.Status.AvailableReleases = []rolloutv1alpha1.VersionInfo{ diff --git a/internal/controller/rollout_history_test.go b/internal/controller/rollout_history_test.go new file mode 100644 index 0000000..229ed0d --- /dev/null +++ b/internal/controller/rollout_history_test.go @@ -0,0 +1,179 @@ +package controller + +import ( + "time" + + rolloutv1alpha1 "github.com/kuberik/rollout-controller/api/v1alpha1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("Rollout Available Releases Cleanup", func() { + Context("CalculateAvailableReleasesToKeep", func() { + var ( + now time.Time + cutoff time.Time + releases []rolloutv1alpha1.VersionInfo + history []rolloutv1alpha1.DeploymentHistoryEntry + minReleases int + ) + + BeforeEach(func() { + now = time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC) + cutoff = now.Add(-7 * 24 * time.Hour) + minReleases = 2 + + releases = []rolloutv1alpha1.VersionInfo{ + {Tag: "0.1.0", Created: &metav1.Time{Time: now.Add(-10 * 24 * time.Hour)}}, // Old + {Tag: "0.2.0", Created: &metav1.Time{Time: now.Add(-8 * 24 * time.Hour)}}, // Old + {Tag: "0.3.0", Created: &metav1.Time{Time: now.Add(-2 * 24 * time.Hour)}}, // Recent + {Tag: "0.4.0", Created: &metav1.Time{Time: now}}, // Newest + } + + history = []rolloutv1alpha1.DeploymentHistoryEntry{ + {Version: rolloutv1alpha1.VersionInfo{Tag: "0.4.0"}}, + {Version: rolloutv1alpha1.VersionInfo{Tag: "0.3.0"}}, + } + }) + + It("should keep releases in history + recent releases + min releases", func() { + // Criteria evaluation: + // 1. History: 0.4.0 (idx 3), 0.3.0 (idx 2). Min index 2. Keep [2, 3] (2 from end) + // 2. Retention: 0.3.0 and 0.4.0 are recent. Retention index 2. Keep [2, 3] (2 from end) + // 3. MinReleases: 2. Keep [2, 3] (2 from end) + // Max = 2. + + result := CalculateAvailableReleasesToKeep(releases, history, cutoff, minReleases) + Expect(result).To(HaveLen(2)) + Expect(result[0].Tag).To(Equal("0.3.0")) + Expect(result[1].Tag).To(Equal("0.4.0")) + }) + + It("should keep more if history oldest entry is older", func() { + history = append(history, rolloutv1alpha1.DeploymentHistoryEntry{ + Version: rolloutv1alpha1.VersionInfo{Tag: "0.2.0"}, + }) + // Criteria evaluation: + // 1. History: 0.4.0 (idx 3), 0.3.0 (idx 2), 0.2.0 (idx 1). Min index 1. Keep [1, 2, 3] (3 from end) + // 2. Retention: keeps 2 (0.3, 0.4) + // 3. Min: keeps 2 + // Max = 3. + + result := CalculateAvailableReleasesToKeep(releases, history, cutoff, minReleases) + Expect(result).To(HaveLen(3)) + Expect(result[0].Tag).To(Equal("0.2.0")) + }) + + It("should keep all if minReleases is large", func() { + minReleases = 10 + result := CalculateAvailableReleasesToKeep(releases, history, cutoff, minReleases) + Expect(result).To(HaveLen(4)) + }) + + It("should keep none if all empty (edge case)", func() { + result := CalculateAvailableReleasesToKeep(nil, history, cutoff, minReleases) + Expect(result).To(BeNil()) + }) + + It("should skip missing timestamps when searching for the newest old release", func() { + // [nil (idx 0), old (idx 1), recent (idx 2), newest (idx 3)] + releases[0].Created = nil + // 1. History: keeps [2, 3] (2 from end) + // 2. Retention: skips idx 0 (nil). Finds idx 1 is OLD. retentionTimeIndex = 1+1 = 2. Keep 4-2 = 2. + // 3. Min: 2. + // Max = 2. + + result := CalculateAvailableReleasesToKeep(releases, history, cutoff, minReleases) + Expect(result).To(HaveLen(2)) + Expect(result[0].Tag).To(Equal("0.3.0")) + }) + + It("should ignore tags in history that are not in releases", func() { + history = append(history, rolloutv1alpha1.DeploymentHistoryEntry{ + Version: rolloutv1alpha1.VersionInfo{Tag: "non-existent"}, + }) + // non-existent tag should be ignored. Criteria evaluation stays same. + result := CalculateAvailableReleasesToKeep(releases, history, cutoff, minReleases) + Expect(result).To(HaveLen(2)) + Expect(result[0].Tag).To(Equal("0.3.0")) + }) + + It("should keep only history when all releases are old and minReleases is 0", func() { + minReleases = 0 + // All releases in 'releases' are older than cutoff, except 0.3.0 and 0.4.0. + // Let's make all old for this test. + for i := range releases { + releases[i].Created = &metav1.Time{Time: cutoff.Add(-time.Hour)} + } + // But history has 0.4.0 and 0.3.0. + result := CalculateAvailableReleasesToKeep(releases, history, cutoff, minReleases) + Expect(result).To(HaveLen(2)) // Should keep 0.3.0 and 0.4.0 because they are in history + Expect(result[0].Tag).To(Equal("0.3.0")) + Expect(result[1].Tag).To(Equal("0.4.0")) + + // If history is empty and minReleases is 0 and all are old, keep nothing. + resultEmpty := CalculateAvailableReleasesToKeep(releases, nil, cutoff, 0) + Expect(resultEmpty).To(BeEmpty()) + }) + + It("should keep all releases when all are recent", func() { + for i := range releases { + releases[i].Created = &metav1.Time{Time: now} + } + result := CalculateAvailableReleasesToKeep(releases, nil, cutoff, 0) + Expect(result).To(HaveLen(4)) + }) + + It("should keep only minReleases when history is empty and all are old", func() { + minReleases = 1 + for i := range releases { + releases[i].Created = &metav1.Time{Time: cutoff.Add(-time.Hour)} + } + result := CalculateAvailableReleasesToKeep(releases, nil, cutoff, minReleases) + Expect(result).To(HaveLen(1)) + Expect(result[0].Tag).To(Equal("0.4.0")) + }) + + It("should handle multiple duplicate tags in history correctly", func() { + history = []rolloutv1alpha1.DeploymentHistoryEntry{ + {Version: rolloutv1alpha1.VersionInfo{Tag: "0.2.0"}}, + {Version: rolloutv1alpha1.VersionInfo{Tag: "0.2.0"}}, + {Version: rolloutv1alpha1.VersionInfo{Tag: "0.1.0"}}, + } + // Min index is 0 (0.1.0). Keep all. + result := CalculateAvailableReleasesToKeep(releases, history, cutoff, 0) + Expect(result).To(HaveLen(4)) + }) + + It("should handle mixed nil and old timestamps correctly", func() { + // [old, nil, recent, recent] + releases[1].Created = nil + // Criterion 2 (Retention): + // i = 3 (recent) + // i = 2 (recent) + // i = 1 (nil) -> skip + // i = 0 (old) -> matches. retentionTimeIndex = 0 + 1 = 1. Keep 4-1 = 3. + + result := CalculateAvailableReleasesToKeep(releases, nil, cutoff, 0) + Expect(result).To(HaveLen(3)) + Expect(result[0].Tag).To(Equal("0.2.0")) + }) + + It("should keep releases based on time retention when it exceeds minReleases and history", func() { + // Make 0.2.0 recent enough to be kept by retention + releases[1].Created = &metav1.Time{Time: now.Add(-6 * 24 * time.Hour)} // 6 days old < 7 days cutoff + + // [old, recent, recent, newest] + // MinReleases = 1 (would keep 1) + // History = empty (would keep 0) + // Retention: keeps 3 (0.2.0, 0.3.0, 0.4.0) + minReleases = 1 + result := CalculateAvailableReleasesToKeep(releases, nil, cutoff, minReleases) + Expect(result).To(HaveLen(3)) + Expect(result[0].Tag).To(Equal("0.2.0")) + Expect(result[1].Tag).To(Equal("0.3.0")) + Expect(result[2].Tag).To(Equal("0.4.0")) + }) + }) +})