Updated to fix bug on greedy match when looking up Docker containers

Author: nicholasjackson

ChangeLog.md

@@ -2,6 +2,11 @@
 
 ## version 0.0.16
 
+### Bugfixes
+* By default when looking up the id of a Docker container Docker does a greedy match, this caused issues where we would grab an incorrect id on destroy. Changed to use a regex.
+
+## version 0.0.16
+
 ### Bugfixes
 * Fixes a bug where local folders were being created for types other than `bind`
 * Fix a bug where Volumes for Nomad Clusters were not changed to absolute paths

functional_tests/test_fixtures/nomad/app_config/example2.nomad

@@ -18,7 +18,7 @@ job "example_2" {
     healthy_deadline = "5m"
   }
 
-  group "cache" {
+  group "consul" {
     count = 1
 
     restart {
@@ -33,16 +33,16 @@ job "example_2" {
       size = 200
     }
 
-    task "redis" {
+    task "consul" {
       # The "driver" parameter specifies the task driver that should be used to
       # run the task.
       driver = "docker"
 
       config {
-        image = "redis:3.2"
+        image = "consul:1.7.1"
 
         port_map {
-          db = 6380
+          http = 8500
         }
       }
 
@@ -52,20 +52,7 @@ job "example_2" {
 
         network {
           mbits = 10
-          port  "db"  {}
-        }
-      }
-      
-      service {
-        name = "redis-cache"
-        tags = ["global", "cache"]
-        port = "db"
-
-        check {
-          name     = "alive"
-          type     = "tcp"
-          interval = "10s"
-          timeout  = "2s"
+          port  "http"  {}
         }
       }
     }

pkg/clients/docker_tasks.go

@@ -233,7 +233,8 @@ func (d *DockerTasks) FindContainerIDs(containerName string, typeName config.Res
 	fullName := utils.FQDN(containerName, string(typeName))
 
 	args := filters.NewArgs()
-	args.Add("name", fullName)
+	// By default Docker will wildcard searches, use regex to return the absolute
+	args.Add("name", fmt.Sprintf("^/%s$", fullName))
 
 	opts := types.ContainerListOptions{Filters: args, All: true}
 

pkg/clients/docker_tasks_find_ids_test.go

@@ -31,7 +31,7 @@ func TestFindContainerIDsReturnsID(t *testing.T) {
 
 	// ensure that the FQDN was passed as an argument
 	args := getCalls(&md.Mock, "ContainerList")[0].Arguments[1].(types.ContainerListOptions)
-	assert.Equal(t, "test.cloud.shipyard.run", args.Filters.Get("name")[0])
+	assert.Equal(t, "^/test.cloud.shipyard.run$", args.Filters.Get("name")[0])
 
 	// ensure that the id has been returned
 	assert.Len(t, ids, 2)

pkg/config/parser.go

@@ -124,7 +124,8 @@ func parseYardMarkdown(file string, c *Config) error {
 
 	fr, body, err := m.Parse(f)
 	if err != nil && err != front.ErrIsEmpty {
-		return err
+		fmt.Println("Error parsing README.md", err)
+		return nil
 	}
 
 	bp := &Blueprint{}

pkg/providers/cluster_k3s.go

@@ -66,7 +66,7 @@ func (c *K8sCluster) createK3s() error {
 	c.log.Info("Creating Cluster", "ref", c.config.Name)
 
 	// check the cluster does not already exist
-	ids, err := c.client.FindContainerIDs(c.config.Name, c.config.Type)
+	ids, err := c.client.FindContainerIDs(fmt.Sprintf("server.%s", c.config.Name), c.config.Type)
 	if err != nil {
 		return err
 	}
@@ -295,7 +295,7 @@ func (c *K8sCluster) ImportLocalDockerImages(name string, id string, images []co
 func (c *K8sCluster) destroyK3s() error {
 	c.log.Info("Destroy Cluster", "ref", c.config.Name)
 
-	ids, err := c.client.FindContainerIDs(c.config.Name, c.config.Type)
+	ids, err := c.client.FindContainerIDs(fmt.Sprintf("server.%s", c.config.Name), c.config.Type)
 	if err != nil {
 		return err
 	}

pkg/providers/cluster_k3s_test.go

@@ -80,7 +80,7 @@ func TestClusterK3ErrorsWhenUnableToLookupIDs(t *testing.T) {
 
 func TestClusterK3ErrorsWhenClusterExists(t *testing.T) {
 	md := &mocks.MockContainerTasks{}
-	md.On("FindContainerIDs", mock.Anything, mock.Anything).Return([]string{"abc"}, nil)
+	md.On("FindContainerIDs", "server."+clusterConfig.Name, mock.Anything).Return([]string{"abc"}, nil)
 
 	mk := &mocks.MockKubernetes{}
 	p := NewK8sCluster(clusterConfig, md, mk, nil, hclog.NewNullLogger())
@@ -340,7 +340,7 @@ func TestClusterK3sDestroyGetsIDr(t *testing.T) {
 
 	err := p.Destroy()
 	assert.NoError(t, err)
-	md.AssertCalled(t, "FindContainerIDs", clusterConfig.Name, clusterConfig.Type)
+	md.AssertCalled(t, "FindContainerIDs", "server."+clusterConfig.Name, clusterConfig.Type)
 }
 
 func TestClusterK3sDestroyWithFindIDErrorReturnsError(t *testing.T) {

pkg/providers/cluster_nomad.go

@@ -45,7 +45,7 @@ func (c *NomadCluster) createNomad() error {
 	c.log.Info("Creating Cluster", "ref", c.config.Name)
 
 	// check the cluster does not already exist
-	ids, err := c.client.FindContainerIDs(c.config.Name, c.config.Type)
+	ids, err := c.client.FindContainerIDs(fmt.Sprintf("server.%s", c.config.Name), c.config.Type)
 	if len(ids) > 0 {
 		return ErrorClusterExists
 	}
@@ -171,19 +171,22 @@ func (c *NomadCluster) ImportLocalDockerImages(name string, id string, images []
 }
 
 func (c *NomadCluster) destroyNomad() error {
-	c.log.Info("Destroy Cluster", "ref", c.config.Name)
+	c.log.Info("Destroy Nomad Cluster", "ref", c.config.Name)
 
-	ids, err := c.client.FindContainerIDs(c.config.Name, c.config.Type)
+	// FindContainerIDs works on absolute addresses, we need to append the server
+
+	ids, err := c.client.FindContainerIDs(fmt.Sprintf("server.%s", c.config.Name), c.config.Type)
 	if err != nil {
 		return err
 	}
 
 	for _, i := range ids {
 		// remove from the networks
 		for _, n := range c.config.Networks {
+			c.log.Debug("Detaching container from network", "ref", c.config.Name, "id", i, "network", n.Name)
 			err := c.client.DetachNetwork(n.Name, i)
 			if err != nil {
-				return err
+				c.log.Error("Unable to detach network", "ref", c.config.Name, "network", n.Name, "error", err)
 			}
 		}
 

pkg/providers/cluster_nomad_test.go

@@ -70,7 +70,7 @@ func TestClusterNomadErrorsWhenUnableToLookupIDs(t *testing.T) {
 
 func TestClusterNomadErrorsWhenClusterExists(t *testing.T) {
 	md := &mocks.MockContainerTasks{}
-	md.On("FindContainerIDs", mock.Anything, mock.Anything).Return([]string{"abc"}, nil)
+	md.On("FindContainerIDs", "server."+clusterNomadConfig.Name, mock.Anything).Return([]string{"abc"}, nil)
 
 	p := NewNomadCluster(clusterNomadConfig, md, nil, hclog.NewNullLogger())
 
@@ -260,7 +260,7 @@ func TestClusterNomadDestroyGetsIDr(t *testing.T) {
 
 	err := p.Destroy()
 	assert.NoError(t, err)
-	md.AssertCalled(t, "FindContainerIDs", clusterNomadConfig.Name, clusterNomadConfig.Type)
+	md.AssertCalled(t, "FindContainerIDs", "server."+clusterNomadConfig.Name, clusterNomadConfig.Type)
 }
 
 func TestClusterNomadDestroyWithFindIDErrorReturnsError(t *testing.T) {

pkg/providers/ingress.go

@@ -180,7 +180,7 @@ func (i *Ingress) Create() error {
 
 // Destroy the ingress
 func (i *Ingress) Destroy() error {
-	i.log.Info("Destroy Ingress", "ref", i.config.Name)
+	i.log.Info("Destroy Ingress", "ref", i.config.Name, "type", i.config.Type)
 
 	ids, err := i.client.FindContainerIDs(i.config.Name, i.config.Type)
 	if err != nil {
@@ -189,9 +189,10 @@ func (i *Ingress) Destroy() error {
 
 	for _, id := range ids {
 		for _, n := range i.config.Networks {
+			i.log.Debug("Detaching container from network", "ref", i.config.Name, "id", id, "network", n.Name)
 			err := i.client.DetachNetwork(n.Name, id)
 			if err != nil {
-				i.log.Error("Unable to detach network", "ref", i.config.Name, "network", n.Name)
+				i.log.Error("Unable to detach network", "ref", i.config.Name, "network", n.Name, "error", err)
 			}
 		}