From f1d043cd4314cf62dab8a32e862473cd0ff54c02 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Mon, 19 Jun 2017 15:35:29 -0700 Subject: [PATCH 1/2] scheduler: Change test to exercise code path where nodes are created later Signed-off-by: Aaron Lehmann --- manager/scheduler/scheduler_test.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/manager/scheduler/scheduler_test.go b/manager/scheduler/scheduler_test.go index 6846b5b02e..82a247ff18 100644 --- a/manager/scheduler/scheduler_test.go +++ b/manager/scheduler/scheduler_test.go @@ -2764,8 +2764,6 @@ func TestSchedulerHostPort(t *testing.T) { // Add initial node and task assert.NoError(t, store.CreateTask(tx, task1)) assert.NoError(t, store.CreateTask(tx, task2)) - assert.NoError(t, store.CreateNode(tx, node1)) - assert.NoError(t, store.CreateNode(tx, node2)) return nil }) assert.NoError(t, err) @@ -2780,6 +2778,18 @@ func TestSchedulerHostPort(t *testing.T) { }() defer scheduler.Stop() + // Tasks shouldn't be scheduled because there are no nodes. + watchAssignmentFailure(t, watch) + watchAssignmentFailure(t, watch) + + err = s.Update(func(tx store.Tx) error { + // Add initial node and task + assert.NoError(t, store.CreateNode(tx, node1)) + assert.NoError(t, store.CreateNode(tx, node2)) + return nil + }) + assert.NoError(t, err) + // Tasks 1 and 2 should be assigned to different nodes. assignment1 := watchAssignment(t, watch) assignment2 := watchAssignment(t, watch) From 5b4b55a2490f19b0507b0aae28116e74720598ac Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Tue, 13 Jun 2017 17:11:34 -0700 Subject: [PATCH 2/2] scheduler: Clean up addOrUpdateNode This function previously could take an uninitialized NodeInfo structure and fill in whatever was missing. This is very error-prone, so remove this logic and change the only caller that relies on it to always pass in a properly initialized NodeInfo. Signed-off-by: Aaron Lehmann --- manager/scheduler/nodeset.go | 11 ----------- manager/scheduler/scheduler.go | 25 ++++++++++++++++--------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/manager/scheduler/nodeset.go b/manager/scheduler/nodeset.go index 7f899d8b26..b83704a18d 100644 --- a/manager/scheduler/nodeset.go +++ b/manager/scheduler/nodeset.go @@ -4,7 +4,6 @@ import ( "container/heap" "errors" "strings" - "time" "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/manager/constraint" @@ -32,16 +31,6 @@ func (ns *nodeSet) nodeInfo(nodeID string) (NodeInfo, error) { // addOrUpdateNode sets the number of tasks for a given node. It adds the node // to the set if it wasn't already tracked. func (ns *nodeSet) addOrUpdateNode(n NodeInfo) { - if n.Tasks == nil { - n.Tasks = make(map[string]*api.Task) - } - if n.ActiveTasksCountByService == nil { - n.ActiveTasksCountByService = make(map[string]int) - } - if n.recentFailures == nil { - n.recentFailures = make(map[string][]time.Time) - } - ns.nodes[n.ID] = n } diff --git a/manager/scheduler/scheduler.go b/manager/scheduler/scheduler.go index 9ff921fd5b..07d9b0458c 100644 --- a/manager/scheduler/scheduler.go +++ b/manager/scheduler/scheduler.go @@ -315,25 +315,32 @@ func (s *Scheduler) deleteTask(t *api.Task) bool { } func (s *Scheduler) createOrUpdateNode(n *api.Node) { - nodeInfo, _ := s.nodeSet.nodeInfo(n.ID) + nodeInfo, nodeInfoErr := s.nodeSet.nodeInfo(n.ID) var resources *api.Resources if n.Description != nil && n.Description.Resources != nil { resources = n.Description.Resources.Copy() // reconcile resources by looping over all tasks in this node - for _, task := range nodeInfo.Tasks { - reservations := taskReservations(task.Spec) + if nodeInfoErr == nil { + for _, task := range nodeInfo.Tasks { + reservations := taskReservations(task.Spec) - resources.MemoryBytes -= reservations.MemoryBytes - resources.NanoCPUs -= reservations.NanoCPUs + resources.MemoryBytes -= reservations.MemoryBytes + resources.NanoCPUs -= reservations.NanoCPUs - genericresource.ConsumeNodeResources(&resources.Generic, - task.AssignedGenericResources) + genericresource.ConsumeNodeResources(&resources.Generic, + task.AssignedGenericResources) + } } } else { resources = &api.Resources{} } - nodeInfo.Node = n - nodeInfo.AvailableResources = resources + + if nodeInfoErr != nil { + nodeInfo = newNodeInfo(n, nil, *resources) + } else { + nodeInfo.Node = n + nodeInfo.AvailableResources = resources + } s.nodeSet.addOrUpdateNode(nodeInfo) }