Skip to content

Commit

Permalink
Merge pull request #52 from furiko-io/irvinlim/fix/schedule-constrain…
Browse files Browse the repository at this point in the history 
…ts-webhook
  • Loading branch information
@irvinlim
irvinlim authored Apr 16, 2022
2 parents 031b4ff + 8d93151 commit 3e131b7
Show file tree
Hide file tree
Showing 8 changed files with 337 additions and 87 deletions.
43 changes: 20 additions & 23 deletions apis/execution/v1alpha1/jobconfig_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@ type ScheduleSpec struct {
//
// +optional
Constraints *ScheduleContraints `json:"constraints,omitempty"`

// Specifies the time that the schedule was last upated. This prevents
// accidental back-scheduling.
//
// For example, if a JobConfig that was previously disabled from automatic
// scheduling is now enabled, we do not want to perform back-scheduling for
// schedules after LastScheduled prior to updating of the JobConfig.
//
// +optional
LastUpdated *metav1.Time `json:"lastUpdated,omitempty"`
}

// CronSchedule defines a Schedule based on cron format.
Expand Down Expand Up @@ -101,17 +111,17 @@ type CronSchedule struct {

// ScheduleContraints defines constraints for automatic scheduling.
type ScheduleContraints struct {
// If set, the scheduler should not perform any scheduling for timestamps before
// this.
//
// For example, if a JobConfig that was previously disabled from automatic
// scheduling is now enabled, then NotBefore will be automatically updated to
// the current time. This prevents accidental back-scheduling so that we may not
// use LastScheduleTime to compute back-schedules that are before it was
// re-enabled.
// Specifies the earliest possible time that is allowed to be scheduled. If set,
// the scheduler should not create schedules before this time.
//
// +optional
NotBefore *metav1.Time `json:"notBefore,omitempty"`

// Specifies the latest possible time that is allowed to be scheduled. If set,
// the scheduler should not create schedules after this time.
//
// +optional
NotAfter *metav1.Time `json:"notAfter,omitempty"`
}

type ConcurrencyPolicy string
Expand Down Expand Up @@ -405,23 +415,10 @@ type JobConfigStatus struct {
// during controller downtime. If the controller was down for a short period of
// time, any schedules that were missed during the downtime will be
// back-scheduled, subject to the number of schedules missed since
// LastScheduleTime.
//
// To prevent thundering herd effects, there are two controller configuration
// values that will restrict the number of back-scheduled jobs created after the
// controller comes back up:
//
// 1. MaxMissedSchedules: Defines a maximum number of jobs that the controller
// should attempt after coming back up. Having a sane value here would prevent a
// thundering herd of jobs being scheduled that would exhaust resources in the
// cluster. Defaults to 5 if not defined.
// 2. MaxDowntimeThresholdSeconds: Defines the maximum downtime threshold that
// the controller can tolerate. If the controller was intentionally shut down
// for an extended period of time, we should not attempt to back-schedule jobs
// once it was started. Defaults to 5 minutes if not defined.
// LastScheduled.
//
// +optional
LastScheduleTime *metav1.Time `json:"lastScheduleTime,omitempty"`
LastScheduled *metav1.Time `json:"lastScheduled,omitempty"`
}

type JobConfigState string
Expand Down
12 changes: 10 additions & 2 deletions apis/execution/v1alpha1/zz_generated.deepcopy.go
View file

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

14 changes: 11 additions & 3 deletions config/crd/bases/execution.furiko.io_jobconfigs.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,12 @@ spec:
constraints:
description: Specifies any constraints that should apply to this Schedule.
properties:
notAfter:
description: Specifies the latest possible time that is allowed to be scheduled. If set, the scheduler should not create schedules after this time.
format: date-time
type: string
notBefore:
description: "If set, the scheduler should not perform any scheduling for timestamps before this. \n For example, if a JobConfig that was previously disabled from automatic scheduling is now enabled, then NotBefore will be automatically updated to the current time. This prevents accidental back-scheduling so that we may not use LastScheduleTime to compute back-schedules that are before it was re-enabled."
description: Specifies the earliest possible time that is allowed to be scheduled. If set, the scheduler should not create schedules before this time.
format: date-time
type: string
type: object
Expand All @@ -190,6 +194,10 @@ spec:
disabled:
description: If true, then automatic scheduling will be disabled for the JobConfig.
type: boolean
lastUpdated:
description: "Specifies the time that the schedule was last upated. This prevents accidental back-scheduling. \n For example, if a JobConfig that was previously disabled from automatic scheduling is now enabled, we do not want to perform back-scheduling for schedules after LastScheduled prior to updating of the JobConfig."
format: date-time
type: string
type: object
template:
description: Template for creating the Job.
Expand Down Expand Up @@ -4304,8 +4312,8 @@ spec:
- namespace
type: object
type: array
lastScheduleTime:
description: "The last known schedule time for this job config, used to persist state during controller downtime. If the controller was down for a short period of time, any schedules that were missed during the downtime will be back-scheduled, subject to the number of schedules missed since LastScheduleTime. \n To prevent thundering herd effects, there are two controller configuration values that will restrict the number of back-scheduled jobs created after the controller comes back up: \n 1. MaxMissedSchedules: Defines a maximum number of jobs that the controller should attempt after coming back up. Having a sane value here would prevent a thundering herd of jobs being scheduled that would exhaust resources in the cluster. Defaults to 5 if not defined. 2. MaxDowntimeThresholdSeconds: Defines the maximum downtime threshold that the controller can tolerate. If the controller was intentionally shut down for an extended period of time, we should not attempt to back-schedule jobs once it was started. Defaults to 5 minutes if not defined."
lastScheduled:
description: The last known schedule time for this job config, used to persist state during controller downtime. If the controller was down for a short period of time, any schedules that were missed during the downtime will be back-scheduled, subject to the number of schedules missed since LastScheduled.
format: date-time
type: string
queued:
Expand Down
35 changes: 24 additions & 11 deletions pkg/execution/controllers/croncontroller/schedule.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (w *Schedule) GetNextScheduleTime(
// Did not previously compute next schedule time (or was cleared). Use the last
// schedule time in the JobConfig's status to back-schedule all missed schedules
// (up to a threshold).
lastScheduleTime := jobConfig.Status.LastScheduleTime
lastScheduleTime := jobConfig.Status.LastScheduled
if lastScheduleTime != nil && !lastScheduleTime.IsZero() {
if Clock.Since(lastScheduleTime.Time) > maxDowntimeThreshold {
newLastScheduleTime := metav1.NewTime(Clock.Now().Add(-maxDowntimeThreshold))
Expand All @@ -86,30 +86,43 @@ func (w *Schedule) GetNextScheduleTime(
fromTime = lastScheduleTime.Time
}

// Cannot schedule before schedule.NotBefore.
// Bump to LastUpdated to avoid accidental back-scheduling.
if spec := jobConfig.Spec.Schedule; spec != nil {
if lastUpdated := spec.LastUpdated; !lastUpdated.IsZero() && lastUpdated.After(fromTime) {
fromTime = lastUpdated.Time
}
}

// Cannot schedule before NotBefore.
// NOTE(irvinlim): Compute next using fromTime, so we sub nanosecond in case time falls exactly on NotBefore
if spec := jobConfig.Spec.Schedule; spec != nil && spec.Constraints != nil {
if nbf := spec.Constraints.NotBefore; !nbf.IsZero() && nbf.After(fromTime) {
fromTime = nbf.Time
if nbf := spec.Constraints.NotBefore; !nbf.IsZero() && fromTime.Before(nbf.Time) {
fromTime = nbf.Time.Add(-time.Nanosecond)
}
}

// Set timezone correctly.
fromTime = fromTime.In(timezone)

// Compute the next scheduled time.
next := expr.Next(fromTime)

// We update our internal map so it will be fetched when the time comes.
w.setNextScheduleTime(jobConfig, next)
return next
// Bump and save the next schedule time.
return w.BumpNextScheduleTime(jobConfig, fromTime, expr)
}

// BumpNextScheduleTime updates the next schedule time of a JobConfig.
func (w *Schedule) BumpNextScheduleTime(
jobConfig *execution.JobConfig, fromTime time.Time, expr *cronexpr.Expression,
) {
) time.Time {
next := expr.Next(fromTime)

// Cannot schedule after NotAfter.
if spec := jobConfig.Spec.Schedule; spec != nil && spec.Constraints != nil {
if naf := spec.Constraints.NotAfter; !naf.IsZero() && next.After(naf.Time) {
next = time.Time{}
}
}

w.setNextScheduleTime(jobConfig, next)
return next
}

// FlushNextScheduleTime flushes the next schedule time for a JobConfig, causing it to be recomputed on
Expand Down
Loading

0 comments on commit 3e131b7

Please sign in to comment.