From e5b86d405e259a793c13c19a3987bd90f8c2356e Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Mon, 19 Mar 2018 12:16:13 -0500 Subject: [PATCH] Fix incorrect initialization of reschedule policy for system jobs. --- api/tasks.go | 7 +- command/agent/job_endpoint.go | 16 +- command/agent/job_endpoint_test.go | 236 +++++++++++++++++++++++++++++ 3 files changed, 247 insertions(+), 12 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 5bf957800..448084156 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -359,13 +359,10 @@ func (g *TaskGroup) Canonicalize(job *Job) { Unlimited: helper.BoolToPtr(structs.DefaultBatchJobReschedulePolicy.Unlimited), } default: - defaultReschedulePolicy = &ReschedulePolicy{ - Attempts: helper.IntToPtr(0), - Interval: helper.TimeToPtr(0 * time.Second), - } + defaultReschedulePolicy = nil } - if g.ReschedulePolicy != nil { + if defaultReschedulePolicy!=nil && g.ReschedulePolicy != nil { defaultReschedulePolicy.Merge(g.ReschedulePolicy) } g.ReschedulePolicy = defaultReschedulePolicy diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 2012513a7..840fb1fee 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -638,13 +638,15 @@ func ApiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup) { Mode: *taskGroup.RestartPolicy.Mode, } - tg.ReschedulePolicy = &structs.ReschedulePolicy{ - Attempts: *taskGroup.ReschedulePolicy.Attempts, - Interval: *taskGroup.ReschedulePolicy.Interval, - Delay: *taskGroup.ReschedulePolicy.Delay, - DelayFunction: *taskGroup.ReschedulePolicy.DelayFunction, - MaxDelay: *taskGroup.ReschedulePolicy.MaxDelay, - Unlimited: *taskGroup.ReschedulePolicy.Unlimited, + if taskGroup.ReschedulePolicy != nil { + tg.ReschedulePolicy = &structs.ReschedulePolicy{ + Attempts: *taskGroup.ReschedulePolicy.Attempts, + Interval: *taskGroup.ReschedulePolicy.Interval, + Delay: *taskGroup.ReschedulePolicy.Delay, + DelayFunction: *taskGroup.ReschedulePolicy.DelayFunction, + MaxDelay: *taskGroup.ReschedulePolicy.MaxDelay, + Unlimited: *taskGroup.ReschedulePolicy.Unlimited, + } } tg.EphemeralDisk = &structs.EphemeralDisk{ diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index dc50dd77e..f59acaaf2 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -1548,4 +1548,240 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { if diff := pretty.Diff(expected, structsJob); len(diff) > 0 { t.Fatalf("bad:\n%s", strings.Join(diff, "\n")) } + + systemAPIJob := &api.Job{ + Stop: helper.BoolToPtr(true), + Region: helper.StringToPtr("global"), + Namespace: helper.StringToPtr("foo"), + ID: helper.StringToPtr("foo"), + ParentID: helper.StringToPtr("lol"), + Name: helper.StringToPtr("name"), + Type: helper.StringToPtr("system"), + Priority: helper.IntToPtr(50), + AllAtOnce: helper.BoolToPtr(true), + Datacenters: []string{"dc1", "dc2"}, + Constraints: []*api.Constraint{ + { + LTarget: "a", + RTarget: "b", + Operand: "c", + }, + }, + TaskGroups: []*api.TaskGroup{ + { + Name: helper.StringToPtr("group1"), + Count: helper.IntToPtr(5), + Constraints: []*api.Constraint{ + { + LTarget: "x", + RTarget: "y", + Operand: "z", + }, + }, + RestartPolicy: &api.RestartPolicy{ + Interval: helper.TimeToPtr(1 * time.Second), + Attempts: helper.IntToPtr(5), + Delay: helper.TimeToPtr(10 * time.Second), + Mode: helper.StringToPtr("delay"), + }, + EphemeralDisk: &api.EphemeralDisk{ + SizeMB: helper.IntToPtr(100), + Sticky: helper.BoolToPtr(true), + Migrate: helper.BoolToPtr(true), + }, + Meta: map[string]string{ + "key": "value", + }, + Tasks: []*api.Task{ + { + Name: "task1", + Leader: true, + Driver: "docker", + User: "mary", + Config: map[string]interface{}{ + "lol": "code", + }, + Env: map[string]string{ + "hello": "world", + }, + Constraints: []*api.Constraint{ + { + LTarget: "x", + RTarget: "y", + Operand: "z", + }, + }, + Resources: &api.Resources{ + CPU: helper.IntToPtr(100), + MemoryMB: helper.IntToPtr(10), + Networks: []*api.NetworkResource{ + { + IP: "10.10.11.1", + MBits: helper.IntToPtr(10), + ReservedPorts: []api.Port{ + { + Label: "http", + Value: 80, + }, + }, + DynamicPorts: []api.Port{ + { + Label: "ssh", + Value: 2000, + }, + }, + }, + }, + }, + Meta: map[string]string{ + "lol": "code", + }, + KillTimeout: helper.TimeToPtr(10 * time.Second), + KillSignal: "SIGQUIT", + LogConfig: &api.LogConfig{ + MaxFiles: helper.IntToPtr(10), + MaxFileSizeMB: helper.IntToPtr(100), + }, + Artifacts: []*api.TaskArtifact{ + { + GetterSource: helper.StringToPtr("source"), + GetterOptions: map[string]string{ + "a": "b", + }, + GetterMode: helper.StringToPtr("dir"), + RelativeDest: helper.StringToPtr("dest"), + }, + }, + DispatchPayload: &api.DispatchPayloadConfig{ + File: "fileA", + }, + }, + }, + }, + }, + Status: helper.StringToPtr("status"), + StatusDescription: helper.StringToPtr("status_desc"), + Version: helper.Uint64ToPtr(10), + CreateIndex: helper.Uint64ToPtr(1), + ModifyIndex: helper.Uint64ToPtr(3), + JobModifyIndex: helper.Uint64ToPtr(5), + } + + expectedSystemJob := &structs.Job{ + Stop: true, + Region: "global", + Namespace: "foo", + ID: "foo", + ParentID: "lol", + Name: "name", + Type: "system", + Priority: 50, + AllAtOnce: true, + Datacenters: []string{"dc1", "dc2"}, + Constraints: []*structs.Constraint{ + { + LTarget: "a", + RTarget: "b", + Operand: "c", + }, + }, + TaskGroups: []*structs.TaskGroup{ + { + Name: "group1", + Count: 5, + Constraints: []*structs.Constraint{ + { + LTarget: "x", + RTarget: "y", + Operand: "z", + }, + }, + RestartPolicy: &structs.RestartPolicy{ + Interval: 1 * time.Second, + Attempts: 5, + Delay: 10 * time.Second, + Mode: "delay", + }, + EphemeralDisk: &structs.EphemeralDisk{ + SizeMB: 100, + Sticky: true, + Migrate: true, + }, + Meta: map[string]string{ + "key": "value", + }, + Tasks: []*structs.Task{ + { + Name: "task1", + Driver: "docker", + Leader: true, + User: "mary", + Config: map[string]interface{}{ + "lol": "code", + }, + Constraints: []*structs.Constraint{ + { + LTarget: "x", + RTarget: "y", + Operand: "z", + }, + }, + Env: map[string]string{ + "hello": "world", + }, + Resources: &structs.Resources{ + CPU: 100, + MemoryMB: 10, + Networks: []*structs.NetworkResource{ + { + IP: "10.10.11.1", + MBits: 10, + ReservedPorts: []structs.Port{ + { + Label: "http", + Value: 80, + }, + }, + DynamicPorts: []structs.Port{ + { + Label: "ssh", + Value: 2000, + }, + }, + }, + }, + }, + Meta: map[string]string{ + "lol": "code", + }, + KillTimeout: 10 * time.Second, + KillSignal: "SIGQUIT", + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 100, + }, + Artifacts: []*structs.TaskArtifact{ + { + GetterSource: "source", + GetterOptions: map[string]string{ + "a": "b", + }, + GetterMode: "dir", + RelativeDest: "dest", + }, + }, + DispatchPayload: &structs.DispatchPayloadConfig{ + File: "fileA", + }, + }, + }, + }, + }, + } + + systemStructsJob := ApiJobToStructJob(systemAPIJob) + + if diff := pretty.Diff(expectedSystemJob, systemStructsJob); len(diff) > 0 { + t.Fatalf("bad:\n%s", strings.Join(diff, "\n")) + } }