Merge pull request #4139 from hashicorp/b-reschedule-invalid-system-jobs

Make system jobs fail validation if they contain a reschedule stanza
This commit is contained in:
Preetha
2018-04-11 20:01:19 -05:00
committed by GitHub
4 changed files with 91 additions and 34 deletions

View File

@@ -103,6 +103,9 @@ type ReschedulePolicy struct {
}
func (r *ReschedulePolicy) Merge(rp *ReschedulePolicy) {
if rp == nil {
return
}
if rp.Interval != nil {
r.Interval = rp.Interval
}
@@ -123,6 +126,63 @@ func (r *ReschedulePolicy) Merge(rp *ReschedulePolicy) {
}
}
func (r *ReschedulePolicy) Canonicalize(jobType string) {
dp := NewDefaultReschedulePolicy(jobType)
if r.Interval == nil {
r.Interval = dp.Interval
}
if r.Attempts == nil {
r.Attempts = dp.Attempts
}
if r.Delay == nil {
r.Delay = dp.Delay
}
if r.DelayFunction == nil {
r.DelayFunction = dp.DelayFunction
}
if r.MaxDelay == nil {
r.MaxDelay = dp.MaxDelay
}
if r.Unlimited == nil {
r.Unlimited = dp.Unlimited
}
}
func NewDefaultReschedulePolicy(jobType string) *ReschedulePolicy {
var dp *ReschedulePolicy
switch jobType {
case "service":
dp = &ReschedulePolicy{
Attempts: helper.IntToPtr(structs.DefaultServiceJobReschedulePolicy.Attempts),
Interval: helper.TimeToPtr(structs.DefaultServiceJobReschedulePolicy.Interval),
Delay: helper.TimeToPtr(structs.DefaultServiceJobReschedulePolicy.Delay),
DelayFunction: helper.StringToPtr(structs.DefaultServiceJobReschedulePolicy.DelayFunction),
MaxDelay: helper.TimeToPtr(structs.DefaultServiceJobReschedulePolicy.MaxDelay),
Unlimited: helper.BoolToPtr(structs.DefaultServiceJobReschedulePolicy.Unlimited),
}
case "batch":
dp = &ReschedulePolicy{
Attempts: helper.IntToPtr(structs.DefaultBatchJobReschedulePolicy.Attempts),
Interval: helper.TimeToPtr(structs.DefaultBatchJobReschedulePolicy.Interval),
Delay: helper.TimeToPtr(structs.DefaultBatchJobReschedulePolicy.Delay),
DelayFunction: helper.StringToPtr(structs.DefaultBatchJobReschedulePolicy.DelayFunction),
MaxDelay: helper.TimeToPtr(structs.DefaultBatchJobReschedulePolicy.MaxDelay),
Unlimited: helper.BoolToPtr(structs.DefaultBatchJobReschedulePolicy.Unlimited),
}
case "system":
dp = &ReschedulePolicy{
Attempts: helper.IntToPtr(0),
Interval: helper.TimeToPtr(0),
Delay: helper.TimeToPtr(0),
DelayFunction: helper.StringToPtr(""),
MaxDelay: helper.TimeToPtr(0),
Unlimited: helper.BoolToPtr(false),
}
}
return dp
}
func (r *ReschedulePolicy) Copy() *ReschedulePolicy {
if r == nil {
return nil
@@ -408,37 +468,13 @@ func (g *TaskGroup) Canonicalize(job *Job) {
jobReschedule := job.Reschedule.Copy()
g.ReschedulePolicy = jobReschedule
}
// Merge with default reschedule policy
var defaultReschedulePolicy *ReschedulePolicy
switch *job.Type {
case "service":
defaultReschedulePolicy = &ReschedulePolicy{
Attempts: helper.IntToPtr(structs.DefaultServiceJobReschedulePolicy.Attempts),
Interval: helper.TimeToPtr(structs.DefaultServiceJobReschedulePolicy.Interval),
Delay: helper.TimeToPtr(structs.DefaultServiceJobReschedulePolicy.Delay),
DelayFunction: helper.StringToPtr(structs.DefaultServiceJobReschedulePolicy.DelayFunction),
MaxDelay: helper.TimeToPtr(structs.DefaultServiceJobReschedulePolicy.MaxDelay),
Unlimited: helper.BoolToPtr(structs.DefaultServiceJobReschedulePolicy.Unlimited),
}
case "batch":
defaultReschedulePolicy = &ReschedulePolicy{
Attempts: helper.IntToPtr(structs.DefaultBatchJobReschedulePolicy.Attempts),
Interval: helper.TimeToPtr(structs.DefaultBatchJobReschedulePolicy.Interval),
Delay: helper.TimeToPtr(structs.DefaultBatchJobReschedulePolicy.Delay),
DelayFunction: helper.StringToPtr(structs.DefaultBatchJobReschedulePolicy.DelayFunction),
MaxDelay: helper.TimeToPtr(structs.DefaultBatchJobReschedulePolicy.MaxDelay),
Unlimited: helper.BoolToPtr(structs.DefaultBatchJobReschedulePolicy.Unlimited),
}
default:
defaultReschedulePolicy = nil
// Only use default reschedule policy for non system jobs
if g.ReschedulePolicy == nil && *job.Type != "system" {
g.ReschedulePolicy = NewDefaultReschedulePolicy(*job.Type)
}
if defaultReschedulePolicy != nil && g.ReschedulePolicy != nil {
defaultReschedulePolicy.Merge(g.ReschedulePolicy)
if g.ReschedulePolicy != nil {
g.ReschedulePolicy.Canonicalize(*job.Type)
}
g.ReschedulePolicy = defaultReschedulePolicy
// Merge the migrate strategy from the job
if jm, tm := job.Migrate != nil, g.Migrate != nil; jm && tm {
jobMigrate := job.Migrate.Copy()

View File

@@ -263,10 +263,6 @@ func SystemJob() *structs.Job {
Delay: 1 * time.Minute,
Mode: structs.RestartPolicyModeDelay,
},
ReschedulePolicy: &structs.ReschedulePolicy{
Attempts: 2,
Interval: 10 * time.Minute,
},
EphemeralDisk: structs.DefaultEphemeralDisk(),
Tasks: []*structs.Task{
{

View File

@@ -3301,7 +3301,11 @@ func (tg *TaskGroup) Validate(j *Job) error {
mErr.Errors = append(mErr.Errors, fmt.Errorf("Task Group %v should have a restart policy", tg.Name))
}
if j.Type != JobTypeSystem {
if j.Type == JobTypeSystem {
if tg.ReschedulePolicy != nil {
mErr.Errors = append(mErr.Errors, fmt.Errorf("System jobs should not have a reschedule policy"))
}
} else {
if tg.ReschedulePolicy != nil {
if err := tg.ReschedulePolicy.Validate(); err != nil {
mErr.Errors = append(mErr.Errors, err)

View File

@@ -367,6 +367,7 @@ func TestJob_IsPeriodicActive(t *testing.T) {
func TestJob_SystemJob_Validate(t *testing.T) {
j := testJob()
j.Type = JobTypeSystem
j.TaskGroups[0].ReschedulePolicy = nil
j.Canonicalize()
err := j.Validate()
@@ -713,6 +714,26 @@ func TestTaskGroup_Validate(t *testing.T) {
if !strings.Contains(err.Error(), "does not allow update block") {
t.Fatalf("err: %s", err)
}
tg = &TaskGroup{
Count: -1,
RestartPolicy: &RestartPolicy{
Interval: 5 * time.Minute,
Delay: 10 * time.Second,
Attempts: 10,
Mode: RestartPolicyModeDelay,
},
ReschedulePolicy: &ReschedulePolicy{
Interval: 5 * time.Minute,
Attempts: 5,
Delay: 5 * time.Second,
},
}
j.Type = JobTypeSystem
err = tg.Validate(j)
if !strings.Contains(err.Error(), "System jobs should not have a reschedule policy") {
t.Fatalf("err: %s", err)
}
}
func TestTask_Validate(t *testing.T) {