From 9de6ccdee05c0dbf900ccc7280de6994a94ef9b5 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 8 Mar 2017 11:02:17 -0800 Subject: [PATCH] allocator: Avoid allocating tasks that are no longer running The allocator currently uses taskDead to check if a task should not be allocated. However, this checks that both the desired state and actual state are past "running". In the case of attachment tasks, there is no orchestration, and the desired state is always "running". This means allocation can be reattempted on a failed attachment task indefinitely. Change the allocator to only try to allocate tasks where both the desired and actual state are "running" or below. Signed-off-by: Aaron Lehmann --- manager/allocator/network.go | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/manager/allocator/network.go b/manager/allocator/network.go index f46ff606e6..13e0235aac 100644 --- a/manager/allocator/network.go +++ b/manager/allocator/network.go @@ -281,7 +281,7 @@ func (a *Allocator) doNetworkInit(ctx context.Context) (err error) { } for _, t := range tasks { - if taskDead(t) { + if t.Status.State > api.TaskStateRunning { continue } @@ -486,17 +486,6 @@ func (a *Allocator) doNodeAlloc(ctx context.Context, ev events.Event) { } } -// taskRunning checks whether a task is either actively running, or in the -// process of starting up. -func taskRunning(t *api.Task) bool { - return t.DesiredState <= api.TaskStateRunning && t.Status.State <= api.TaskStateRunning -} - -// taskDead checks whether a task is not actively running as far as allocator purposes are concerned. -func taskDead(t *api.Task) bool { - return t.DesiredState > api.TaskStateRunning && t.Status.State > api.TaskStateRunning -} - // taskReadyForNetworkVote checks if the task is ready for a network // vote to move it to PENDING state. func taskReadyForNetworkVote(t *api.Task, s *api.Service, nc *networkContext) bool { @@ -599,10 +588,9 @@ func (a *Allocator) doTaskAlloc(ctx context.Context, ev events.Event) { nc := a.netCtx - // If the task has stopped running or it's being deleted then - // we should free the network resources associated with the - // task right away. - if taskDead(t) || isDelete { + // If the task has stopped running then we should free the network + // resources associated with the task right away. + if t.Status.State > api.TaskStateRunning || isDelete { if nc.nwkAllocator.IsTaskAllocated(t) { if err := nc.nwkAllocator.DeallocateTask(t); err != nil { log.G(ctx).WithError(err).Errorf("Failed freeing network resources for task %s", t.ID) @@ -637,7 +625,7 @@ func (a *Allocator) doTaskAlloc(ctx context.Context, ev events.Event) { // available in store. But we still need to // cleanup network resources associated with // the task. - if taskRunning(t) && !isDelete { + if t.Status.State <= api.TaskStateRunning && !isDelete { log.G(ctx).Errorf("Event %T: Failed to get service %s for task %s state %s: could not find service %s", ev, t.ServiceID, t.ID, t.Status.State, t.ServiceID) return }