-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restart unhealthy tasks #3105
Restart unhealthy tasks #3105
Changes from all commits
a720bb5
bd1a342
1608e59
ebbf87f
555d1e2
c2d895d
78c72f8
7e103f6
850d991
3db835c
526528c
568b963
9fb2865
092057a
8b8c164
237c096
f8e872c
40ed262
5cd1d57
10dc1c7
3c0a42b
6f72270
a508bb9
5141c95
1564e1c
8014762
cde908e
fa836d8
924813d
6bcf019
10ae18c
967825d
3d7446d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,7 +49,7 @@ var ( | |
// TaskHooks is an interface which provides hooks into the tasks life-cycle | ||
type TaskHooks interface { | ||
// Restart is used to restart the task | ||
Restart(source, reason string) | ||
Restart(source, reason string, failure bool) | ||
|
||
// Signal is used to signal the task | ||
Signal(source, reason string, s os.Signal) error | ||
|
@@ -439,7 +439,8 @@ func (tm *TaskTemplateManager) handleTemplateRerenders(allRenderedTime time.Time | |
} | ||
|
||
if restart { | ||
tm.config.Hooks.Restart(consulTemplateSourceName, "template with change_mode restart re-rendered") | ||
const failure = false | ||
tm.config.Hooks.Restart(consulTemplateSourceName, "template with change_mode restart re-rendered", failure) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My little const pattern is pretty funky, and I'd be happy to remove it. I do it because I hate seeing method calls with literal booleans in them and having no idea what those booleans do without looking at the method signature and/or docs. |
||
} else if len(signals) != 0 { | ||
var mErr multierror.Error | ||
for signal := range signals { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,11 +99,24 @@ func TestClient_RestartTracker_RestartTriggered(t *testing.T) { | |
p := testPolicy(true, structs.RestartPolicyModeFail) | ||
p.Attempts = 0 | ||
rt := newRestartTracker(p, structs.JobTypeService) | ||
if state, when := rt.SetRestartTriggered().GetState(); state != structs.TaskRestarting && when != 0 { | ||
if state, when := rt.SetRestartTriggered(false).GetState(); state != structs.TaskRestarting && when != 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unit test the new case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added cde908e |
||
t.Fatalf("expect restart immediately, got %v %v", state, when) | ||
} | ||
} | ||
|
||
func TestClient_RestartTracker_RestartTriggered_Failure(t *testing.T) { | ||
t.Parallel() | ||
p := testPolicy(true, structs.RestartPolicyModeFail) | ||
p.Attempts = 1 | ||
rt := newRestartTracker(p, structs.JobTypeService) | ||
if state, when := rt.SetRestartTriggered(true).GetState(); state != structs.TaskRestarting || when == 0 { | ||
t.Fatalf("expect restart got %v %v", state, when) | ||
} | ||
if state, when := rt.SetRestartTriggered(true).GetState(); state != structs.TaskNotRestarting || when != 0 { | ||
t.Fatalf("expect failed got %v %v", state, when) | ||
} | ||
} | ||
|
||
func TestClient_RestartTracker_StartError_Recoverable_Fail(t *testing.T) { | ||
t.Parallel() | ||
p := testPolicy(true, structs.RestartPolicyModeFail) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would advice against adding anything else at this point. I would convert to a config struct. No action required in this PR but its getting a bit much
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking the same thing!