Merge of #949

mergify[bot] · web-flow · commit 8992271e86ed · 2026-03-11T10:08:27.000Z
diff --git a/api/csiaddons/v1alpha1/encryptionkeyrotationcronjob_types.go b/api/csiaddons/v1alpha1/encryptionkeyrotationcronjob_types.go
@@ -36,6 +36,9 @@ type EncryptionKeyRotationJobTemplateSpec struct {
 // EncryptionKeyRotationCronJobSpec defines the desired state of EncryptionKeyRotationCronJob
 type EncryptionKeyRotationCronJobSpec struct {
 	// The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+	// A deterministic, UID-based stagger offset is applied to spread
+	// execution across the "cronjob-stagger-window" (default: 2 hours,
+	// set to 0 to disable) configured in the csi-addons-config ConfigMap.
 	// +kubebuilder:validation:Required
 	// +kubebuilder:validation:Pattern:=.+
 	Schedule string `json:"schedule"`
diff --git a/api/csiaddons/v1alpha1/reclaimspacecronjob_types.go b/api/csiaddons/v1alpha1/reclaimspacecronjob_types.go
@@ -36,6 +36,9 @@ type ReclaimSpaceJobTemplateSpec struct {
 // ReclaimSpaceCronJobSpec defines the desired state of ReclaimSpaceJob
 type ReclaimSpaceCronJobSpec struct {
 	// The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+	// A deterministic, UID-based stagger offset is applied to spread
+	// execution across the "cronjob-stagger-window" (default: 2 hours,
+	// set to 0 to disable) configured in the csi-addons-config ConfigMap.
 	// +kubebuilder:validation:Required
 	// +kubebuilder:validation:Pattern:=.+
 	Schedule string `json:"schedule"`
diff --git a/cmd/manager/main.go b/cmd/manager/main.go
@@ -107,6 +107,7 @@ func main() {
 	flag.BoolVar(&enableAuth, "enable-auth", true, "Enables TLS and adds bearer token to the headers (enabled by default)")
 	flag.IntVar(&cfg.MaxGroupPVC, "max-group-pvc", cfg.MaxGroupPVC, "Maximum number of PVCs allowed in a volume group")
 	flag.IntVar(&cfg.CSIAddonsNodeRetryDelay, util.CsiaddonsNodeRetryDelayKey, cfg.CSIAddonsNodeRetryDelay, "Duration, in seconds, the CSIAddonsNode reconciler must wait before retrying the connection to csi-addons sidecar")
+	flag.IntVar(&cfg.CronJobStaggerWindow, util.CronJobStaggerWindowKey, cfg.CronJobStaggerWindow, "Duration, in hours, that the CronJobs for KeyRotation and ReclaimSpace are staggered within. Defaults to 2 hours, set as 0 to disable.")
 	opts := zap.Options{
 		Development: true,
 		TimeEncoder: zapcore.ISO8601TimeEncoder,
@@ -234,8 +235,9 @@ func main() {
 		os.Exit(1)
 	}
 	if err = (&controllers.ReclaimSpaceCronJobReconciler{
-		Client: mgr.GetClient(),
-		Scheme: mgr.GetScheme(),
+		Client:        mgr.GetClient(),
+		Scheme:        mgr.GetScheme(),
+		StaggerWindow: cfg.CronJobStaggerWindow,
 	}).SetupWithManager(mgr, ctrlOptions); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "ReclaimSpaceCronJob")
 		os.Exit(1)
@@ -299,8 +301,9 @@ func main() {
 		os.Exit(1)
 	}
 	if err = (&controllers.EncryptionKeyRotationCronJobReconciler{
-		Client: mgr.GetClient(),
-		Scheme: mgr.GetScheme(),
+		Client:        mgr.GetClient(),
+		Scheme:        mgr.GetScheme(),
+		StaggerWindow: cfg.CronJobStaggerWindow,
 	}).SetupWithManager(mgr, ctrlOptions); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "EncryptionKeyRotationCronJob")
 		os.Exit(1)
diff --git a/config/crd/bases/csiaddons.openshift.io_encryptionkeyrotationcronjobs.yaml b/config/crd/bases/csiaddons.openshift.io_encryptionkeyrotationcronjobs.yaml
@@ -146,7 +146,11 @@ spec:
                 - spec
                 type: object
               schedule:
-                description: The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+                description: |-
+                  The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+                  A deterministic, UID-based stagger offset is applied to spread
+                  execution across the "cronjob-stagger-window" (default: 2 hours,
+                  set to 0 to disable) configured in the csi-addons-config ConfigMap.
                 pattern: .+
                 type: string
               startingDeadlineSeconds:
diff --git a/config/crd/bases/csiaddons.openshift.io_reclaimspacecronjobs.yaml b/config/crd/bases/csiaddons.openshift.io_reclaimspacecronjobs.yaml
@@ -145,7 +145,11 @@ spec:
                 - spec
                 type: object
               schedule:
-                description: The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+                description: |-
+                  The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+                  A deterministic, UID-based stagger offset is applied to spread
+                  execution across the "cronjob-stagger-window" (default: 2 hours,
+                  set to 0 to disable) configured in the csi-addons-config ConfigMap.
                 pattern: .+
                 type: string
               startingDeadlineSeconds:
diff --git a/deploy/controller/crds.yaml b/deploy/controller/crds.yaml
@@ -304,7 +304,11 @@ spec:
                 - spec
                 type: object
               schedule:
-                description: The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+                description: |-
+                  The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+                  A deterministic, UID-based stagger offset is applied to spread
+                  execution across the "cronjob-stagger-window" (default: 2 hours,
+                  set to 0 to disable) configured in the csi-addons-config ConfigMap.
                 pattern: .+
                 type: string
               startingDeadlineSeconds:
@@ -1004,7 +1008,11 @@ spec:
                 - spec
                 type: object
               schedule:
-                description: The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+                description: |-
+                  The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+                  A deterministic, UID-based stagger offset is applied to spread
+                  execution across the "cronjob-stagger-window" (default: 2 hours,
+                  set to 0 to disable) configured in the csi-addons-config ConfigMap.
                 pattern: .+
                 type: string
               startingDeadlineSeconds:
diff --git a/deploy/controller/csi-addons-config.yaml b/deploy/controller/csi-addons-config.yaml
@@ -10,3 +10,4 @@ data:
   "max-concurrent-reconciles": "100"
   "max-group-pvcs": "100"
   "csi-addons-node-retry-delay": "5"
+  "cronjob-stagger-window": "2"
diff --git a/docs/csi-addons-config.md b/docs/csi-addons-config.md
@@ -4,13 +4,13 @@ CSI-Addons Operator can consume configuration from a ConfigMap named `csi-addons
 in the same namespace as the operator. This enables configuration of the operator to persist across
 upgrades. The ConfigMap can support the following configuration options:
 
-| Option                        | Default value | Description                                                                                         |
-| ----------------------------- | ------------- | --------------------------------------------------------------------------------------------------- |
-| `reclaim-space-timeout`       | `"3m"`        | Timeout for reclaimspace operation                                                                  |
-| `max-concurrent-reconciles`   | `"100"`       | Maximum number of concurrent reconciles                                                             |
-| `max-group-pvcs`              | `"100"`       | Maximum number of PVCs allowed in a volume group                                                    |
-| `csi-addons-node-retry-delay` | `"5"`         | Duration, in seconds, that csi-addons reconcile must wait before retrying connection to the sidecar |
-| `schedule-precedence`         | `"pvc"`       | The order in which the schedule annotation should be read                                           |
+| Option                        | Default value | Description                                                                                                              |
+| ----------------------------- | ------------- | ------------------------------------------------------------------------------------------------------------------------ |
+| `max-concurrent-reconciles`   | `"100"`       | Maximum number of concurrent reconciles                                                                                  |
+| `max-group-pvcs`              | `"100"`       | Maximum number of PVCs allowed in a volume group                                                                         |
+| `csi-addons-node-retry-delay` | `"5"`         | Duration, in seconds, that csi-addons reconcile must wait before retrying connection to the sidecar                      |
+| `schedule-precedence`         | `"pvc"`       | The order in which the schedule annotation should be read                                                                |
+| `cronjob-stagger-window`      | `"2"`         | Maximum stagger window, in hours, for key rotation and reclaim space CronJob schedules. Set to `0` to disable staggering |
 
 [`csi-addons-config` ConfigMap](../deploy/controller/csi-addons-config.yaml) is provided as an example.
 
diff --git a/internal/controller/csiaddons/encryptionkeyrotationcronjob_controller.go b/internal/controller/csiaddons/encryptionkeyrotationcronjob_controller.go
@@ -40,7 +40,8 @@ import (
 // EncryptionKeyRotationCronJobReconciler reconciles a EncryptionKeyRotationCronJob object
 type EncryptionKeyRotationCronJobReconciler struct {
 	client.Client
-	Scheme *runtime.Scheme
+	Scheme        *runtime.Scheme
+	StaggerWindow int
 }
 
 //+kubebuilder:rbac:groups=csiaddons.openshift.io,resources=encryptionkeyrotationcronjobs,verbs=get;list;watch;create;update;patch;delete
@@ -123,7 +124,7 @@ func (r *EncryptionKeyRotationCronJobReconciler) Reconcile(ctx context.Context,
 		return ctrl.Result{}, nil
 	}
 
-	missedRun, nextRun, err := getNextScheduleForKeyRotation(krcJob, time.Now())
+	missedRun, nextRun, err := getNextScheduleForKeyRotation(krcJob, time.Now(), r.StaggerWindow)
 	if err != nil {
 		logger.Error(err, "failed to get next schedule for jobs", "schedule", krcJob.Spec.Schedule)
 
@@ -293,7 +294,8 @@ func (r *EncryptionKeyRotationCronJobReconciler) deleteOldEncryptionKeyRotationJ
 // An error is returned if start is missed more than 100 times
 func getNextScheduleForKeyRotation(
 	krcJob *csiaddonsv1alpha1.EncryptionKeyRotationCronJob,
-	now time.Time) (time.Time, time.Time, error) {
+	now time.Time,
+	staggerWindow int) (time.Time, time.Time, error) {
 	sched, err := cron.ParseStandard(krcJob.Spec.Schedule)
 	if err != nil {
 		return time.Time{}, time.Time{}, fmt.Errorf("unparsable schedule %q: %v", krcJob.Spec.Schedule, err)
@@ -313,8 +315,11 @@ func getNextScheduleForKeyRotation(
 			earliestTime = schedulingDeadline
 		}
 	}
+
+	rawNext := sched.Next(now)
+	staggeredNext := utils.GetStaggeredNext(krcJob.UID, rawNext, sched, staggerWindow)
 	if earliestTime.After(now) {
-		return time.Time{}, sched.Next(now), nil
+		return time.Time{}, staggeredNext, nil
 	}
 
 	starts := 0
@@ -333,7 +338,7 @@ func getNextScheduleForKeyRotation(
 					" delete and recreate encryptionkeyrotationjob")
 		}
 	}
-	return lastMissed, sched.Next(now), nil
+	return lastMissed, staggeredNext, nil
 }
 
 // constructEncryptionKeyRotationJob forms an EncryptionKeyRotationJob for a given
diff --git a/internal/controller/csiaddons/reclaimspacecronjob_controller.go b/internal/controller/csiaddons/reclaimspacecronjob_controller.go
@@ -40,7 +40,8 @@ import (
 // ReclaimSpaceCronJobReconciler reconciles a ReclaimSpaceCronJob object
 type ReclaimSpaceCronJobReconciler struct {
 	client.Client
-	Scheme *runtime.Scheme
+	Scheme        *runtime.Scheme
+	StaggerWindow int
 }
 
 var (
@@ -129,7 +130,7 @@ func (r *ReclaimSpaceCronJobReconciler) Reconcile(ctx context.Context, req ctrl.
 	}
 
 	// figure out the next times that we need to create jobs at (or anything we missed).
-	missedRun, nextRun, err := getNextSchedule(rsCronJob, time.Now())
+	missedRun, nextRun, err := getNextSchedule(rsCronJob, time.Now(), r.StaggerWindow)
 	if err != nil {
 		logger.Error(err, "Failed to Parse out CronJob schedule", "schedule", rsCronJob.Spec.Schedule)
 		// invalid schedule, do not requeue.
@@ -356,7 +357,8 @@ func getScheduledTimeForRSJob(rsJob *csiaddonsv1alpha1.ReclaimSpaceJob) (*time.T
 // This function returns error if there are more than 100 missed start times.
 func getNextSchedule(
 	rsCronJob *csiaddonsv1alpha1.ReclaimSpaceCronJob,
-	now time.Time) (time.Time, time.Time, error) {
+	now time.Time,
+	staggerWindow int) (time.Time, time.Time, error) {
 	sched, err := cron.ParseStandard(rsCronJob.Spec.Schedule)
 	if err != nil {
 		return time.Time{}, time.Time{}, fmt.Errorf("unparsable schedule %q: %v", rsCronJob.Spec.Schedule, err)
@@ -376,8 +378,11 @@ func getNextSchedule(
 			earliestTime = schedulingDeadline
 		}
 	}
+
+	rawNext := sched.Next(now)
+	staggeredNext := utils.GetStaggeredNext(rsCronJob.UID, rawNext, sched, staggerWindow)
 	if earliestTime.After(now) {
-		return time.Time{}, sched.Next(now), nil
+		return time.Time{}, staggeredNext, nil
 	}
 
 	starts := 0
@@ -396,5 +401,5 @@ func getNextSchedule(
 					" delete and recreate reclaimspacecronjob")
 		}
 	}
-	return lastMissed, sched.Next(now), nil
+	return lastMissed, staggeredNext, nil
 }
diff --git a/internal/controller/csiaddons/reclaimspacecronjob_controller_test.go b/internal/controller/csiaddons/reclaimspacecronjob_controller_test.go
@@ -22,9 +22,12 @@ import (
 	"time"
 
 	csiaddonsv1alpha1 "github.com/csi-addons/kubernetes-csi-addons/api/csiaddons/v1alpha1"
+	"github.com/csi-addons/kubernetes-csi-addons/internal/controller/utils"
 
+	"github.com/robfig/cron/v3"
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 )
 
@@ -113,6 +116,13 @@ func TestGetScheduledTimeForRSJob(t *testing.T) {
 func TestGetNextSchedule(t *testing.T) {
 	now := time.Now()
 	expectedLastMissed := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, time.Local)
+	mustParse := func(sched string) cron.Schedule {
+		s, err := cron.ParseStandard(sched)
+		if err != nil {
+			t.Fatalf("Failed to parse cron spec %q: %v", sched, err)
+		}
+		return s
+	}
 	type args struct {
 		rsCronJob *csiaddonsv1alpha1.ReclaimSpaceCronJob
 		now       time.Time
@@ -128,6 +138,9 @@ func TestGetNextSchedule(t *testing.T) {
 			name: "Valid schedule, no deadline, last schedule time exists",
 			args: args{
 				rsCronJob: &csiaddonsv1alpha1.ReclaimSpaceCronJob{
+					ObjectMeta: metav1.ObjectMeta{
+						UID: types.UID("valid-sched-1"),
+					},
 					Spec: csiaddonsv1alpha1.ReclaimSpaceCronJobSpec{
 						Schedule: "0 0 * * *",
 					},
@@ -145,6 +158,9 @@ func TestGetNextSchedule(t *testing.T) {
 			name: "Valid schedule, deadline not exceeded, last schedule time exists",
 			args: args{
 				rsCronJob: &csiaddonsv1alpha1.ReclaimSpaceCronJob{
+					ObjectMeta: metav1.ObjectMeta{
+						UID: types.UID("valid-sched-2"),
+					},
 					Spec: csiaddonsv1alpha1.ReclaimSpaceCronJobSpec{
 						Schedule:                "0 0 * * *",
 						StartingDeadlineSeconds: ptr.To(int64(3600)),
@@ -163,6 +179,9 @@ func TestGetNextSchedule(t *testing.T) {
 			name: "Valid schedule, deadline exceeded, last schedule time exists, missed schedules < 100",
 			args: args{
 				rsCronJob: &csiaddonsv1alpha1.ReclaimSpaceCronJob{
+					ObjectMeta: metav1.ObjectMeta{
+						UID: types.UID("valid-sched-3"),
+					},
 					Spec: csiaddonsv1alpha1.ReclaimSpaceCronJobSpec{
 						Schedule:                "*/1 * * * *",
 						StartingDeadlineSeconds: ptr.To(int64(6000)),
@@ -181,6 +200,9 @@ func TestGetNextSchedule(t *testing.T) {
 			name: "Valid schedule, deadline exceeded, last schedule time exists, missed schedules > 100",
 			args: args{
 				rsCronJob: &csiaddonsv1alpha1.ReclaimSpaceCronJob{
+					ObjectMeta: metav1.ObjectMeta{
+						UID: types.UID("valid-sched-4"),
+					},
 					Spec: csiaddonsv1alpha1.ReclaimSpaceCronJobSpec{
 						Schedule:                "*/1 * * * *",
 						StartingDeadlineSeconds: ptr.To(int64(6060)),
@@ -198,16 +220,24 @@ func TestGetNextSchedule(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			gotLastMissed, gotNextSchedule, err := getNextSchedule(tt.args.rsCronJob, tt.args.now)
+			gotLastMissed, gotNextSchedule, err := getNextSchedule(tt.args.rsCronJob, tt.args.now, 2)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("getNextSchedule() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
+
+			if tt.wantErr {
+				return
+			}
+
 			if !gotLastMissed.Equal(tt.lastMissed) && !gotLastMissed.Equal(time.Time{}) {
 				t.Errorf("getNextSchedule() got last missed = %v, want %v", gotLastMissed, tt.lastMissed)
 			}
-			if !gotNextSchedule.Equal(tt.nextSchedule) {
-				t.Errorf("getNextSchedule() got next schedule = %v, want %v", gotNextSchedule, tt.nextSchedule)
+
+			sched := mustParse(tt.args.rsCronJob.Spec.Schedule)
+			staggered := utils.GetStaggeredNext(tt.args.rsCronJob.UID, tt.nextSchedule, sched, 2)
+			if !gotNextSchedule.Equal(staggered) {
+				t.Errorf("getNextSchedule() got next schedule = %v, want %v", gotNextSchedule, staggered)
 			}
 		})
 	}
diff --git a/internal/controller/utils/reclaimspace_keyrotation.go b/internal/controller/utils/reclaimspace_keyrotation.go
@@ -99,17 +99,6 @@ func SetSpec(obj client.Object, schedule, pvcName string) {
 	}
 }
 
-func GetSchedule(obj client.Object) string {
-	switch v := obj.(type) {
-	case *csiaddonsv1alpha1.EncryptionKeyRotationCronJob:
-		return v.Spec.Schedule
-	case *csiaddonsv1alpha1.ReclaimSpaceCronJob:
-		return v.Spec.Schedule
-	default:
-		return ""
-	}
-}
-
 // ExtractOwnerNameFromPVCObj extracts owner.Name from the object if it is
 // of type `T` and has a PVC as its owner.
 func ExtractOwnerNameFromPVCObj[T client.Object](rawObj client.Object) []string {
diff --git a/internal/controller/utils/schedule.go b/internal/controller/utils/schedule.go
diff --git a/internal/controller/utils/schedule_test.go b/internal/controller/utils/schedule_test.go
diff --git a/internal/util/config.go b/internal/util/config.go
diff --git a/internal/util/config_test.go b/internal/util/config_test.go
diff --git a/test/e2e/encryptionkeyrotation/encryptionkeyrotation_test.go b/test/e2e/encryptionkeyrotation/encryptionkeyrotation_test.go
diff --git a/test/e2e/reclaimspace/reclaimspace_test.go b/test/e2e/reclaimspace/reclaimspace_test.go

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,8 @@ import (`
`40`	`40`	`// EncryptionKeyRotationCronJobReconciler reconciles a EncryptionKeyRotationCronJob object`
`41`	`41`	`type EncryptionKeyRotationCronJobReconciler struct {`
`42`	`42`	`client.Client`
`43`		`- Scheme *runtime.Scheme`
	`43`	`+ Scheme *runtime.Scheme`
	`44`	`+ StaggerWindow int`
`44`	`45`	`}`
`45`	`46`
`46`	`47`	`//+kubebuilder:rbac:groups=csiaddons.openshift.io,resources=encryptionkeyrotationcronjobs,verbs=get;list;watch;create;update;patch;delete`
`@@ -123,7 +124,7 @@ func (r *EncryptionKeyRotationCronJobReconciler) Reconcile(ctx context.Context,`
`123`	`124`	`return ctrl.Result{}, nil`
`124`	`125`	`}`
`125`	`126`
`126`		`- missedRun, nextRun, err := getNextScheduleForKeyRotation(krcJob, time.Now())`
	`127`	`+ missedRun, nextRun, err := getNextScheduleForKeyRotation(krcJob, time.Now(), r.StaggerWindow)`
`127`	`128`	`if err != nil {`
`128`	`129`	`logger.Error(err, "failed to get next schedule for jobs", "schedule", krcJob.Spec.Schedule)`
`129`	`130`
`@@ -293,7 +294,8 @@ func (r *EncryptionKeyRotationCronJobReconciler) deleteOldEncryptionKeyRotationJ`
`293`	`294`	`// An error is returned if start is missed more than 100 times`
`294`	`295`	`func getNextScheduleForKeyRotation(`
`295`	`296`	`krcJob *csiaddonsv1alpha1.EncryptionKeyRotationCronJob,`
`296`		`- now time.Time) (time.Time, time.Time, error) {`
	`297`	`+ now time.Time,`
	`298`	`+ staggerWindow int) (time.Time, time.Time, error) {`
`297`	`299`	`sched, err := cron.ParseStandard(krcJob.Spec.Schedule)`
`298`	`300`	`if err != nil {`
`299`	`301`	`return time.Time{}, time.Time{}, fmt.Errorf("unparsable schedule %q: %v", krcJob.Spec.Schedule, err)`
`@@ -313,8 +315,11 @@ func getNextScheduleForKeyRotation(`
`313`	`315`	`earliestTime = schedulingDeadline`
`314`	`316`	`}`
`315`	`317`	`}`
	`318`	`+`
	`319`	`+ rawNext := sched.Next(now)`
	`320`	`+ staggeredNext := utils.GetStaggeredNext(krcJob.UID, rawNext, sched, staggerWindow)`
`316`	`321`	`if earliestTime.After(now) {`
`317`		`- return time.Time{}, sched.Next(now), nil`
	`322`	`+ return time.Time{}, staggeredNext, nil`
`318`	`323`	`}`
`319`	`324`
`320`	`325`	`starts := 0`
`@@ -333,7 +338,7 @@ func getNextScheduleForKeyRotation(`
`333`	`338`	`" delete and recreate encryptionkeyrotationjob")`
`334`	`339`	`}`
`335`	`340`	`}`
`336`		`- return lastMissed, sched.Next(now), nil`
	`341`	`+ return lastMissed, staggeredNext, nil`
`337`	`342`	`}`
`338`	`343`
`339`	`344`	`// constructEncryptionKeyRotationJob forms an EncryptionKeyRotationJob for a given`