diff --git a/cluster/node.go b/cluster/node.go index a8816e6d4b..1345660252 100644 --- a/cluster/node.go +++ b/cluster/node.go @@ -2,6 +2,7 @@ package cluster import ( "crypto/tls" + "errors" "fmt" "net" "strings" @@ -307,7 +308,8 @@ func (n *Node) ListImages() ([]string, error) { return out, nil } -func (n *Node) Remove(container *Container, force bool) error { +// Destroy and remove a container from the node. +func (n *Node) Destroy(container *Container, force bool) error { if err := n.client.RemoveContainer(container.Id, force); err != nil { return err } @@ -371,11 +373,33 @@ func (n *Node) handler(ev *dockerclient.Event, args ...interface{}) { n.eventHandler.Handle(event) } -// Used only on tests -func (n *Node) AddContainer(container *Container) { +// Inject a container into the internal state. +func (n *Node) AddContainer(container *Container) error { + n.Lock() + defer n.Unlock() + + if _, ok := n.containers[container.Id]; ok { + return errors.New("container already exists") + } n.containers[container.Id] = container + return nil } -func (n *Node) CleanupContainers() { - n.containers = make(map[string]*Container) +// Remove a container from the internal test. +func (n *Node) RemoveContainer(container *Container) error { + n.Lock() + defer n.Unlock() + + if _, ok := n.containers[container.Id]; !ok { + return errors.New("container not found") + } + delete(n.containers, container.Id) + return nil +} + +// Wipes the internal container state. +func (n *Node) CleanupContainers() { + n.Lock() + n.containers = make(map[string]*Container) + n.Unlock() } diff --git a/scheduler/filter/port_test.go b/scheduler/filter/port_test.go index cbcb3bb810..eeead7a901 100644 --- a/scheduler/filter/port_test.go +++ b/scheduler/filter/port_test.go @@ -59,7 +59,7 @@ func TestPortFilterNoConflicts(t *testing.T) { // Add a container taking a different (4242) port. container := &cluster.Container{Container: dockerclient.Container{Id: "c1"}, Info: dockerclient.ContainerInfo{}} container.Info.NetworkSettings.Ports = makeBinding("", "4242") - nodes[0].AddContainer(container) + assert.NoError(t, nodes[0].AddContainer(container)) // Since no node is using port 80, there should be no filter result, err = p.Filter(config, nodes) @@ -82,7 +82,7 @@ func TestPortFilterSimple(t *testing.T) { // Add a container taking away port 80 to nodes[0]. container := &cluster.Container{Container: dockerclient.Container{Id: "c1"}, Info: dockerclient.ContainerInfo{}} container.Info.NetworkSettings.Ports = makeBinding("", "80") - nodes[0].AddContainer(container) + assert.NoError(t, nodes[0].AddContainer(container)) // Request port 80. config := &dockerclient.ContainerConfig{ @@ -112,7 +112,7 @@ func TestPortFilterDifferentInterfaces(t *testing.T) { // Add a container taking away port 80 on every interface to nodes[0]. container := &cluster.Container{Container: dockerclient.Container{Id: "c1"}, Info: dockerclient.ContainerInfo{}} container.Info.NetworkSettings.Ports = makeBinding("", "80") - nodes[0].AddContainer(container) + assert.NoError(t, nodes[0].AddContainer(container)) // Request port 80 for the local interface. config := &dockerclient.ContainerConfig{ @@ -131,7 +131,7 @@ func TestPortFilterDifferentInterfaces(t *testing.T) { // nodes[1]. container = &cluster.Container{Container: dockerclient.Container{Id: "c1"}, Info: dockerclient.ContainerInfo{}} container.Info.NetworkSettings.Ports = makeBinding("127.0.0.1", "4242") - nodes[1].AddContainer(container) + assert.NoError(t, nodes[1].AddContainer(container)) // Request port 4242 on the same interface. config = &dockerclient.ContainerConfig{ diff --git a/scheduler/scheduler.go b/scheduler/scheduler.go index 510fdbc0a3..dcb6625942 100644 --- a/scheduler/scheduler.go +++ b/scheduler/scheduler.go @@ -61,5 +61,5 @@ func (s *Scheduler) RemoveContainer(container *cluster.Container, force bool) er s.Lock() defer s.Unlock() - return container.Node().Remove(container, force) + return container.Node().Destroy(container, force) } diff --git a/scheduler/strategy/binpacking_test.go b/scheduler/strategy/binpacking_test.go index 413f962ffe..79579654a8 100644 --- a/scheduler/strategy/binpacking_test.go +++ b/scheduler/strategy/binpacking_test.go @@ -37,7 +37,7 @@ func TestPlaceContainerMemory(t *testing.T) { config := createConfig(1, 0) node1, err := s.PlaceContainer(config, nodes) assert.NoError(t, err) - node1.AddContainer(createContainer("c1", config)) + assert.NoError(t, node1.AddContainer(createContainer("c1", config))) assert.Equal(t, node1.ReservedMemory(), 1024*1024*1024) @@ -45,7 +45,7 @@ func TestPlaceContainerMemory(t *testing.T) { config = createConfig(1, 1) node2, err := s.PlaceContainer(config, nodes) assert.NoError(t, err) - node2.AddContainer(createContainer("c2", config)) + assert.NoError(t, node2.AddContainer(createContainer("c2", config))) assert.Equal(t, node2.ReservedMemory(), 2*1024*1024*1024) @@ -66,7 +66,7 @@ func TestPlaceContainerCPU(t *testing.T) { config := createConfig(0, 1) node1, err := s.PlaceContainer(config, nodes) assert.NoError(t, err) - node1.AddContainer(createContainer("c1", config)) + assert.NoError(t, node1.AddContainer(createContainer("c1", config))) assert.Equal(t, node1.ReservedCpus(), 1) @@ -74,8 +74,7 @@ func TestPlaceContainerCPU(t *testing.T) { config = createConfig(0, 1) node2, err := s.PlaceContainer(config, nodes) assert.NoError(t, err) - node2.AddContainer(createContainer("c2", config)) - + assert.NoError(t, node2.AddContainer(createContainer("c2", config))) assert.Equal(t, node2.ReservedCpus(), 2) // check that both containers ended on the same node @@ -95,7 +94,7 @@ func TestPlaceContainerHuge(t *testing.T) { for i := 0; i < 100; i++ { node, err := s.PlaceContainer(createConfig(0, 1), nodes) assert.NoError(t, err) - node.AddContainer(createContainer(fmt.Sprintf("c%d", i), createConfig(0, 100))) + assert.NoError(t, node.AddContainer(createContainer(fmt.Sprintf("c%d", i), createConfig(0, 100)))) } // try to add another container 1CPU @@ -103,10 +102,10 @@ func TestPlaceContainerHuge(t *testing.T) { assert.Error(t, err) // add 100 container 1G - for i := 0; i < 100; i++ { + for i := 100; i < 200; i++ { node, err := s.PlaceContainer(createConfig(1, 0), nodes) assert.NoError(t, err) - node.AddContainer(createContainer(fmt.Sprintf("c%d", i), createConfig(1, 0))) + assert.NoError(t, node.AddContainer(createContainer(fmt.Sprintf("c%d", i), createConfig(1, 0)))) } // try to add another container 1G @@ -166,13 +165,13 @@ func TestPlaceContainerDemo(t *testing.T) { config = createConfig(1, 0) node1, err := s.PlaceContainer(config, nodes) assert.NoError(t, err) - node1.AddContainer(createContainer("c1", config)) + assert.NoError(t, node1.AddContainer(createContainer("c1", config))) // add another container 1G config = createConfig(1, 0) node1bis, err := s.PlaceContainer(config, nodes) assert.NoError(t, err) - node1bis.AddContainer(createContainer("c2", config)) + assert.NoError(t, node1bis.AddContainer(createContainer("c2", config))) // check that both containers ended on the same node assert.Equal(t, node1.ID, node1bis.ID, "") @@ -182,7 +181,7 @@ func TestPlaceContainerDemo(t *testing.T) { config = createConfig(2, 0) node2, err := s.PlaceContainer(config, nodes) assert.NoError(t, err) - node2.AddContainer(createContainer("c3", config)) + assert.NoError(t, node2.AddContainer(createContainer("c3", config))) // check that it ends up on another node assert.NotEqual(t, node1.ID, node2.ID, "") @@ -191,7 +190,7 @@ func TestPlaceContainerDemo(t *testing.T) { config = createConfig(1, 0) node3, err := s.PlaceContainer(config, nodes) assert.NoError(t, err) - node3.AddContainer(createContainer("c4", config)) + assert.NoError(t, node3.AddContainer(createContainer("c4", config))) // check that it ends up on another node assert.NotEqual(t, node1.ID, node3.ID, "") @@ -201,7 +200,7 @@ func TestPlaceContainerDemo(t *testing.T) { config = createConfig(1, 0) node3bis, err := s.PlaceContainer(config, nodes) assert.NoError(t, err) - node3bis.AddContainer(createContainer("c5", config)) + assert.NoError(t, node3bis.AddContainer(createContainer("c5", config))) // check that it ends up on the same node assert.Equal(t, node3.ID, node3bis.ID, "") @@ -220,7 +219,7 @@ func TestPlaceContainerDemo(t *testing.T) { config = createConfig(1, 0) node2bis, err := s.PlaceContainer(config, nodes) assert.NoError(t, err) - node2bis.AddContainer(createContainer("c6", config)) + assert.NoError(t, node2bis.AddContainer(createContainer("c6", config))) // check it ends up on `node3` assert.Equal(t, node2.ID, node2bis.ID, "")