From c6cd7b523ba48c14c8da2d596de5fe01de6e10fc Mon Sep 17 00:00:00 2001 From: Jasmine Dahilig Date: Thu, 9 Jan 2020 11:43:05 -0800 Subject: [PATCH] clean up restart conditions and restart tests for task lifecycle --- .../taskrunner/restarts/restarts.go | 11 +- .../taskrunner/restarts/restarts_test.go | 156 ++++++------------ 2 files changed, 52 insertions(+), 115 deletions(-) diff --git a/client/allocrunner/taskrunner/restarts/restarts.go b/client/allocrunner/taskrunner/restarts/restarts.go index 87563ade9..2449a90e3 100644 --- a/client/allocrunner/taskrunner/restarts/restarts.go +++ b/client/allocrunner/taskrunner/restarts/restarts.go @@ -21,14 +21,11 @@ const ( ) func NewRestartTracker(policy *structs.RestartPolicy, jobType string, tlc *structs.TaskLifecycleConfig) *RestartTracker { - onSuccess := true - if tlc != nil && tlc.BlockUntil == structs.TaskLifecycleBlockUntilCompleted { - onSuccess = false - } else if tlc != nil && tlc.BlockUntil == structs.TaskLifecycleBlockUntilRunning { - onSuccess = true - } else if jobType == structs.JobTypeBatch { - onSuccess = false + onSuccess := jobType == structs.JobTypeService + if tlc != nil && tlc.Hook == structs.TaskLifecycleHookPrestart { + onSuccess = tlc.BlockUntil != structs.TaskLifecycleBlockUntilCompleted } + return &RestartTracker{ startTime: time.Now(), onSuccess: onSuccess, diff --git a/client/allocrunner/taskrunner/restarts/restarts_test.go b/client/allocrunner/taskrunner/restarts/restarts_test.go index cf97f5b79..fe877e418 100644 --- a/client/allocrunner/taskrunner/restarts/restarts_test.go +++ b/client/allocrunner/taskrunner/restarts/restarts_test.go @@ -203,147 +203,87 @@ func TestClient_RestartTracker_StartError_Recoverable_Delay(t *testing.T) { func TestClient_RestartTracker_Lifecycle(t *testing.T) { t.Parallel() - tc := []struct { - name string - tlc *structs.TaskLifecycleConfig - jt string - rp *structs.RestartPolicy - exit int - shouldRestart bool + testCase := []struct { + name string + taskLifecycleConfig *structs.TaskLifecycleConfig + jobType string + shouldRestartOnSuccess bool + shouldRestartOnFailure bool }{ { - name: "exit code 0 - service job no lifecycle", - tlc: nil, - jt: structs.JobTypeService, - rp: testPolicy(true, structs.JobTypeService), - exit: 0, - shouldRestart: true, + name: "service job no lifecycle", + taskLifecycleConfig: nil, + jobType: structs.JobTypeService, + shouldRestartOnSuccess: true, + shouldRestartOnFailure: true, }, { - name: "exit code 0 - batch job no lifecycle", - tlc: nil, - jt: structs.JobTypeBatch, - rp: testPolicy(true, structs.JobTypeBatch), - exit: 0, - shouldRestart: false, + name: "batch job no lifecycle", + taskLifecycleConfig: nil, + jobType: structs.JobTypeBatch, + shouldRestartOnSuccess: false, + shouldRestartOnFailure: true, }, { - name: "exit code 0 - service job w/ lifecycle completed", - tlc: &structs.TaskLifecycleConfig{ + name: "service job w/ lifecycle completed", + taskLifecycleConfig: &structs.TaskLifecycleConfig{ Hook: structs.TaskLifecycleHookPrestart, BlockUntil: structs.TaskLifecycleBlockUntilCompleted, }, - jt: structs.JobTypeService, - rp: testPolicy(true, structs.JobTypeService), - exit: 0, - shouldRestart: false, + jobType: structs.JobTypeService, + shouldRestartOnSuccess: false, + shouldRestartOnFailure: true, }, { - name: "exit code 0 - service job w/ lifecycle running", - tlc: &structs.TaskLifecycleConfig{ + name: "service job w/ lifecycle running", + taskLifecycleConfig: &structs.TaskLifecycleConfig{ Hook: structs.TaskLifecycleHookPrestart, BlockUntil: structs.TaskLifecycleBlockUntilRunning, }, - jt: structs.JobTypeService, - rp: testPolicy(true, structs.JobTypeService), - exit: 0, - shouldRestart: true, + jobType: structs.JobTypeService, + shouldRestartOnSuccess: true, + shouldRestartOnFailure: true, }, { - name: "exit code 0 - batch job w/ lifecycle completed", - tlc: &structs.TaskLifecycleConfig{ + name: "batch job lifecycle completed", + taskLifecycleConfig: &structs.TaskLifecycleConfig{ Hook: structs.TaskLifecycleHookPrestart, BlockUntil: structs.TaskLifecycleBlockUntilCompleted, }, - jt: structs.JobTypeService, - rp: testPolicy(true, structs.JobTypeService), - exit: 0, - shouldRestart: false, + jobType: structs.JobTypeService, + shouldRestartOnSuccess: false, + shouldRestartOnFailure: true, }, { - name: "exit code 0 - batch job w/ lifecycle running", - tlc: &structs.TaskLifecycleConfig{ + name: "batch job lifecycle running", + taskLifecycleConfig: &structs.TaskLifecycleConfig{ Hook: structs.TaskLifecycleHookPrestart, BlockUntil: structs.TaskLifecycleBlockUntilRunning, }, - jt: structs.JobTypeBatch, - rp: testPolicy(true, structs.JobTypeBatch), - exit: 0, - shouldRestart: true, - }, - { - name: "exit code 127 - service job no lifecycle", - tlc: nil, - jt: structs.JobTypeService, - rp: testPolicy(true, structs.JobTypeService), - exit: 127, - shouldRestart: true, - }, - { - name: "exit code 127 - batch job no lifecycle", - tlc: nil, - jt: structs.JobTypeBatch, - rp: testPolicy(true, structs.JobTypeBatch), - exit: 127, - shouldRestart: true, - }, - { - name: "exit code 127 - service job w/ lifecycle completed", - tlc: &structs.TaskLifecycleConfig{ - Hook: structs.TaskLifecycleHookPrestart, - BlockUntil: structs.TaskLifecycleBlockUntilCompleted, - }, - jt: structs.JobTypeService, - rp: testPolicy(true, structs.JobTypeService), - exit: 127, - shouldRestart: true, - }, - { - name: "exit code 127 - service job w/ lifecycle running", - tlc: &structs.TaskLifecycleConfig{ - Hook: structs.TaskLifecycleHookPrestart, - BlockUntil: structs.TaskLifecycleBlockUntilCompleted, - }, - jt: structs.JobTypeService, - rp: testPolicy(true, structs.JobTypeService), - exit: 127, - shouldRestart: true, - }, - { - name: "exit code 127 - batch job w/ lifecycle completed", - tlc: &structs.TaskLifecycleConfig{ - Hook: structs.TaskLifecycleHookPrestart, - BlockUntil: structs.TaskLifecycleBlockUntilCompleted, - }, - jt: structs.JobTypeService, - rp: testPolicy(true, structs.JobTypeService), - exit: 127, - shouldRestart: true, - }, - { - name: "exit code 127 - batch job w/ lifecycle running", - tlc: &structs.TaskLifecycleConfig{ - Hook: structs.TaskLifecycleHookPrestart, - BlockUntil: structs.TaskLifecycleBlockUntilCompleted, - }, - jt: structs.JobTypeBatch, - rp: testPolicy(true, structs.JobTypeBatch), - exit: 127, - shouldRestart: true, + jobType: structs.JobTypeBatch, + shouldRestartOnSuccess: true, + shouldRestartOnFailure: true, }, } - for _, tc := range tc { - t.Run(tc.name, func(t *testing.T) { - rt := NewRestartTracker(tc.rp, tc.jt, tc.tlc) + for _, testCase := range testCase { + t.Run(testCase.name, func(t *testing.T) { + restartPolicy := testPolicy(true, testCase.jobType) + restartTracker := NewRestartTracker(restartPolicy, testCase.jobType, testCase.taskLifecycleConfig) - state, _ := rt.SetExitResult(testExitResult(tc.exit)).GetState() - if !tc.shouldRestart { + state, _ := restartTracker.SetExitResult(testExitResult(0)).GetState() + if !testCase.shouldRestartOnSuccess { require.Equal(t, structs.TaskTerminated, state) } else { require.Equal(t, structs.TaskRestarting, state) } + state, _ = restartTracker.SetExitResult(testExitResult(127)).GetState() + if !testCase.shouldRestartOnFailure { + require.Equal(t, structs.TaskTerminated, state) + } else { + require.Equal(t, structs.TaskRestarting, state) + } }) } }