From 326309dfaa12762faec95401e2013574f7272097 Mon Sep 17 00:00:00 2001 From: Andrea Luzzardi Date: Sun, 29 May 2016 17:52:05 -0700 Subject: [PATCH] restarts: Default delay and desired=accepted. - Add a default delay of 5 seconds between restarts if not specified Otherwise we end up into a restart loop by default. - Set the new task's desired state to "ACCEPTED" rather than "READY". Ready implies pulling, which means creating a service with an invalid image leads to a restart loop. Signed-off-by: Andrea Luzzardi --- manager/orchestrator/restart.go | 11 ++++++++--- manager/orchestrator/restart_test.go | 20 ++++++++++---------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/manager/orchestrator/restart.go b/manager/orchestrator/restart.go index 4d02784e56..7db11cd149 100644 --- a/manager/orchestrator/restart.go +++ b/manager/orchestrator/restart.go @@ -14,6 +14,7 @@ import ( ) const defaultOldTaskTimeout = time.Minute +const defaultRestartDelay = 5 * time.Second type restartedInstance struct { timestamp time.Time @@ -81,12 +82,16 @@ func (r *RestartSupervisor) Restart(ctx context.Context, tx store.Tx, service *a n := store.GetNode(tx, t.NodeID) - restartTask.DesiredState = api.TaskStateReady + restartTask.DesiredState = api.TaskStateAccepted var restartDelay time.Duration // Restart delay does not applied to drained nodes - if service.Spec.Restart != nil && service.Spec.Restart.Delay != 0 && (n == nil || n.Spec.Availability != api.NodeAvailabilityDrain) { - restartDelay = service.Spec.Restart.Delay + if n == nil || n.Spec.Availability != api.NodeAvailabilityDrain { + if service.Spec.Restart != nil && service.Spec.Restart.Delay != 0 { + restartDelay = service.Spec.Restart.Delay + } else { + restartDelay = defaultRestartDelay + } } waitStop := true diff --git a/manager/orchestrator/restart_test.go b/manager/orchestrator/restart_test.go index f244f916fb..79f36d2021 100644 --- a/manager/orchestrator/restart_test.go +++ b/manager/orchestrator/restart_test.go @@ -40,6 +40,7 @@ func TestOrchestratorRestartOnAny(t *testing.T) { }, Restart: &api.RestartPolicy{ Condition: api.RestartOnAny, + Delay: 1, }, }, } @@ -137,6 +138,7 @@ func TestOrchestratorRestartOnFailure(t *testing.T) { }, Restart: &api.RestartPolicy{ Condition: api.RestartOnFailure, + Delay: 1, }, }, } @@ -173,7 +175,7 @@ func TestOrchestratorRestartOnFailure(t *testing.T) { observedTask3 := watchTaskCreate(t, watch) assert.Equal(t, observedTask3.Status.State, api.TaskStateNew) - assert.Equal(t, observedTask3.DesiredState, api.TaskStateReady) + assert.Equal(t, observedTask3.DesiredState, api.TaskStateAccepted) assert.Equal(t, observedTask3.ServiceAnnotations.Name, "name1") expectCommit(t, watch) @@ -361,7 +363,7 @@ func TestOrchestratorRestartDelay(t *testing.T) { observedTask3 := watchTaskCreate(t, watch) expectCommit(t, watch) assert.Equal(t, observedTask3.Status.State, api.TaskStateNew) - assert.Equal(t, observedTask3.DesiredState, api.TaskStateReady) + assert.Equal(t, observedTask3.DesiredState, api.TaskStateAccepted) assert.Equal(t, observedTask3.ServiceAnnotations.Name, "name1") observedTask4 := watchTaskUpdate(t, watch) @@ -370,7 +372,7 @@ func TestOrchestratorRestartDelay(t *testing.T) { // At least 100 ms should have elapsed. Only check the lower bound, // because the system may be slow and it could have taken longer. if after.Sub(before) < 100*time.Millisecond { - t.Fatal("restart delay should have elapsed") + t.Fatalf("restart delay should have elapsed. Got: %v", after.Sub(before)) } assert.Equal(t, observedTask4.Status.State, api.TaskStateNew) @@ -446,7 +448,7 @@ func TestOrchestratorRestartMaxAttempts(t *testing.T) { observedTask3 := watchTaskCreate(t, watch) expectCommit(t, watch) assert.Equal(t, observedTask3.Status.State, api.TaskStateNew) - assert.Equal(t, observedTask3.DesiredState, api.TaskStateReady) + assert.Equal(t, observedTask3.DesiredState, api.TaskStateAccepted) assert.Equal(t, observedTask3.ServiceAnnotations.Name, "name1") observedTask4 := watchTaskUpdate(t, watch) @@ -479,8 +481,7 @@ func TestOrchestratorRestartMaxAttempts(t *testing.T) { observedTask5 := watchTaskCreate(t, watch) expectCommit(t, watch) assert.Equal(t, observedTask5.Status.State, api.TaskStateNew) - assert.Equal(t, observedTask5.DesiredState, api.TaskStateReady) - assert.Equal(t, observedTask5.ServiceAnnotations.Name, "name1") + assert.Equal(t, observedTask5.DesiredState, api.TaskStateAccepted) observedTask6 := watchTaskUpdate(t, watch) // task gets started after a delay expectCommit(t, watch) @@ -578,7 +579,7 @@ func TestOrchestratorRestartWindow(t *testing.T) { observedTask3 := watchTaskCreate(t, watch) expectCommit(t, watch) assert.Equal(t, observedTask3.Status.State, api.TaskStateNew) - assert.Equal(t, observedTask3.DesiredState, api.TaskStateReady) + assert.Equal(t, observedTask3.DesiredState, api.TaskStateAccepted) assert.Equal(t, observedTask3.ServiceAnnotations.Name, "name1") observedTask4 := watchTaskUpdate(t, watch) @@ -611,7 +612,7 @@ func TestOrchestratorRestartWindow(t *testing.T) { observedTask5 := watchTaskCreate(t, watch) expectCommit(t, watch) assert.Equal(t, observedTask5.Status.State, api.TaskStateNew) - assert.Equal(t, observedTask5.DesiredState, api.TaskStateReady) + assert.Equal(t, observedTask5.DesiredState, api.TaskStateAccepted) assert.Equal(t, observedTask5.ServiceAnnotations.Name, "name1") observedTask6 := watchTaskUpdate(t, watch) // task gets started after a delay @@ -659,8 +660,7 @@ func TestOrchestratorRestartWindow(t *testing.T) { observedTask7 := watchTaskCreate(t, watch) expectCommit(t, watch) assert.Equal(t, observedTask7.Status.State, api.TaskStateNew) - assert.Equal(t, observedTask7.DesiredState, api.TaskStateReady) - assert.Equal(t, observedTask7.ServiceAnnotations.Name, "name1") + assert.Equal(t, observedTask7.DesiredState, api.TaskStateAccepted) observedTask8 := watchTaskUpdate(t, watch) after = time.Now()