Skip to content

Commit

Permalink
Schedule SkipImmediately
Browse files Browse the repository at this point in the history
Signed-off-by: Tiger Kaovilai <[email protected]>
  • Loading branch information
kaovilai committed Dec 1, 2023
1 parent 5df0d45 commit e536748
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 40 deletions.
16 changes: 16 additions & 0 deletions config/crd/v1/bases/velero.io_schedules.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ spec:
description: Schedule is a Cron expression defining when to run the
Backup.
type: string
skipImmediately:
default: false
description: 'SkipImmediately specifies whether to skip backup if
schedule is due immediately from `schedule.status.lastBackup` timestamp
when schedule is unpaused or if schedule is new. If true, backup
will be skipped immediately when schedule is unpaused if it is due
based on .Status.LastBackupTimestamp or schedule is new, and will
run at next schedule time. If false, backup will not be skipped
immediately when schedule is unpaused, but will run at next schedule
time. If empty, will follow server configuration (default: false).'
type: boolean
template:
description: Template is the definition of the Backup to be run on
the provided schedule
Expand Down Expand Up @@ -539,6 +550,11 @@ spec:
format: date-time
nullable: true
type: string
lastSkipped:
description: LastSkipped is the last time a Schedule was skipped
format: date-time
nullable: true
type: string
phase:
description: Phase is the current phase of the Schedule
enum:
Expand Down
2 changes: 1 addition & 1 deletion config/crd/v1/crds/crds.go

Large diffs are not rendered by default.

13 changes: 13 additions & 0 deletions pkg/apis/velero/v1/schedule_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ type ScheduleSpec struct {
// Paused specifies whether the schedule is paused or not
// +optional
Paused bool `json:"paused,omitempty"`

// SkipImmediately specifies whether to skip backup if schedule is due immediately from `schedule.status.lastBackup` timestamp when schedule is unpaused or if schedule is new.
// If true, backup will be skipped immediately when schedule is unpaused if it is due based on .Status.LastBackupTimestamp or schedule is new, and will run at next schedule time.
// If false, backup will not be skipped immediately when schedule is unpaused, but will run at next schedule time.
// If empty, will follow server configuration (default: false).
// +kubebuilder:default=false
// +optional
SkipImmediately bool `json:"skipImmediately,omitempty"`
}

// SchedulePhase is a string representation of the lifecycle phase
Expand Down Expand Up @@ -75,6 +83,11 @@ type ScheduleStatus struct {
// +nullable
LastBackup *metav1.Time `json:"lastBackup,omitempty"`

// LastSkipped is the last time a Schedule was skipped
// +optional
// +nullable
LastSkipped *metav1.Time `json:"lastSkipped,omitempty"`

// ValidationErrors is a slice of all validation errors (if
// applicable)
// +optional
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/velero/v1/zz_generated.deepcopy.go

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

31 changes: 2 additions & 29 deletions pkg/apis/velero/v2alpha1/zz_generated.deepcopy.go

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

6 changes: 6 additions & 0 deletions pkg/builder/schedule_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,9 @@ func (b *ScheduleBuilder) Template(spec velerov1api.BackupSpec) *ScheduleBuilder
b.object.Spec.Template = spec
return b
}

// SkipImmediately sets the Schedule's SkipImmediately.
func (b *ScheduleBuilder) SkipImmediately(skip bool) *ScheduleBuilder {
b.object.Spec.SkipImmediately = skip
return b
}
27 changes: 19 additions & 8 deletions pkg/controller/schedule_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,15 @@ func (c *scheduleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
}
return ctrl.Result{}, errors.Wrapf(err, "error getting schedule %s", req.String())
}

c.metrics.InitSchedule(schedule.Name)

original := schedule.DeepCopy()

if schedule.Spec.SkipImmediately {
schedule.Spec.SkipImmediately = false
schedule.Status.LastSkipped = &metav1.Time{Time: c.clock.Now()}
}

// validation - even if the item is Enabled, we can't trust it
// so re-validate
currentPhase := schedule.Status.Phase
Expand All @@ -116,7 +120,9 @@ func (c *scheduleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
}

// update status if it's changed
if currentPhase != schedule.Status.Phase {
if currentPhase != schedule.Status.Phase ||
original.Status.LastSkipped != schedule.Status.LastSkipped ||
original.Status.LastBackup != schedule.Status.LastBackup {
if err := c.Patch(ctx, schedule, client.MergeFrom(original)); err != nil {
return ctrl.Result{}, errors.Wrapf(err, "error updating phase of schedule %s to %s", req.String(), schedule.Status.Phase)
}
Expand All @@ -131,13 +137,15 @@ func (c *scheduleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
// If there are backup created by this schedule still in New or InProgress state,
// skip current backup creation to avoid running overlap backups.
// As the schedule must be validated before checking whether it's due, we cannot put the checking log in Predicate
if c.ifDue(schedule, cronSchedule) && !c.checkIfBackupInNewOrProgress(schedule) {
due, nextRunTime := c.ifDue(schedule, cronSchedule)
durationTillNextRun := nextRunTime.Sub(c.clock.Now())
if due && !c.checkIfBackupInNewOrProgress(schedule) {
if err := c.submitBackup(ctx, schedule); err != nil {
return ctrl.Result{}, errors.Wrapf(err, "error submit backup for schedule %s", req.String())
return ctrl.Result{RequeueAfter: durationTillNextRun}, errors.Wrapf(err, "error submit backup for schedule %s", req.String())
}
}

return ctrl.Result{}, nil
return ctrl.Result{RequeueAfter: durationTillNextRun}, nil
}

func parseCronSchedule(itm *velerov1.Schedule, logger logrus.FieldLogger) (cron.Schedule, []string) {
Expand Down Expand Up @@ -207,16 +215,16 @@ func (c *scheduleReconciler) checkIfBackupInNewOrProgress(schedule *velerov1.Sch
}

// ifDue check whether schedule is due to create a new backup.
func (c *scheduleReconciler) ifDue(schedule *velerov1.Schedule, cronSchedule cron.Schedule) bool {
func (c *scheduleReconciler) ifDue(schedule *velerov1.Schedule, cronSchedule cron.Schedule) (bool, time.Time) {
isDue, nextRunTime := getNextRunTime(schedule, cronSchedule, c.clock.Now())
log := c.logger.WithField("schedule", kube.NamespaceAndName(schedule))

if !isDue {
log.WithField("nextRunTime", nextRunTime).Debug("Schedule is not due, skipping")
return false
return false, nextRunTime
}

return true
return true, nextRunTime
}

// submitBackup create a backup from schedule.
Expand Down Expand Up @@ -248,6 +256,9 @@ func getNextRunTime(schedule *velerov1.Schedule, cronSchedule cron.Schedule, asO
} else {
lastBackupTime = schedule.CreationTimestamp.Time
}
if schedule.Status.LastSkipped != nil && schedule.Status.LastSkipped.After(lastBackupTime) {
lastBackupTime = schedule.Status.LastSkipped.Time
}

nextRunTime := cronSchedule.Next(lastBackupTime)

Expand Down
26 changes: 24 additions & 2 deletions pkg/controller/schedule_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
velerotest "github.com/vmware-tanzu/velero/pkg/test"
)

// Test reconcile function of schedule controller. Pause is not covered as event filter will not allow it through
func TestReconcileOfSchedule(t *testing.T) {
require.Nil(t, velerov1.AddToScheme(scheme.Scheme))

Expand All @@ -52,6 +53,7 @@ func TestReconcileOfSchedule(t *testing.T) {
expectedValidationErrors []string
expectedBackupCreate *velerov1.Backup
expectedLastBackup string
expectedLastSkipped string
backup *velerov1.Backup
}{
{
Expand Down Expand Up @@ -88,6 +90,13 @@ func TestReconcileOfSchedule(t *testing.T) {
expectedBackupCreate: builder.ForBackup("ns", "name-20170101120000").ObjectMeta(builder.WithLabels(velerov1.ScheduleNameLabel, "name")).Result(),
expectedLastBackup: "2017-01-01 12:00:00",
},
{
name: "schedule with phase New and SkipImmediately gets validated and does not trigger a backup",
schedule: newScheduleBuilder(velerov1.SchedulePhaseNew).CronSchedule("@every 5m").SkipImmediately(true).Result(),
fakeClockTime: "2017-01-01 12:00:00",
expectedPhase: string(velerov1.SchedulePhaseEnabled),
expectedLastSkipped: "2017-01-01 12:00:00",
},
{
name: "schedule with phase Enabled gets re-validated and triggers a backup if valid",
schedule: newScheduleBuilder(velerov1.SchedulePhaseEnabled).CronSchedule("@every 5m").Result(),
Expand All @@ -103,6 +112,13 @@ func TestReconcileOfSchedule(t *testing.T) {
expectedBackupCreate: builder.ForBackup("ns", "name-20170101120000").ObjectMeta(builder.WithLabels(velerov1.ScheduleNameLabel, "name")).Result(),
expectedLastBackup: "2017-01-01 12:00:00",
},
{
name: "schedule that's already run but has SkippedImmediately do not get LastBackup updated",
schedule: newScheduleBuilder(velerov1.SchedulePhaseEnabled).CronSchedule("@every 5m").LastBackupTime("2000-01-01 00:00:00").SkipImmediately(true).Result(),
fakeClockTime: "2017-01-01 12:00:00",
expectedLastBackup: "2000-01-01 00:00:00",
expectedLastSkipped: "2017-01-01 12:00:00",
},
{
name: "schedule already has backup in New state.",
schedule: newScheduleBuilder(velerov1.SchedulePhaseEnabled).CronSchedule("@every 5m").LastBackupTime("2000-01-01 00:00:00").Result(),
Expand All @@ -119,8 +135,8 @@ func TestReconcileOfSchedule(t *testing.T) {
testTime time.Time
err error
)

reconciler := NewScheduleReconciler("namespace", logger, client, metrics.NewServerMetrics())
name := t.Name()
reconciler := NewScheduleReconciler("namespace"+name, logger, client, metrics.NewServerMetrics())

if test.fakeClockTime != "" {
testTime, err = time.Parse("2006-01-02 15:04:05", test.fakeClockTime)
Expand Down Expand Up @@ -151,8 +167,14 @@ func TestReconcileOfSchedule(t *testing.T) {
}
if len(test.expectedLastBackup) > 0 {
require.Nil(t, err)
require.NotNil(t, schedule.Status.LastBackup)
assert.Equal(t, parseTime(test.expectedLastBackup).Unix(), schedule.Status.LastBackup.Unix())
}
if len(test.expectedLastSkipped) > 0 {
require.Nil(t, err)
require.NotNil(t, schedule.Status.LastSkipped)
assert.Equal(t, parseTime(test.expectedLastSkipped).Unix(), schedule.Status.LastSkipped.Unix())
}

backups := &velerov1.BackupList{}
require.Nil(t, client.List(ctx, backups))
Expand Down

0 comments on commit e536748

Please sign in to comment.