Merge pull request docker-archive#327 from aluzzardi/vieux-fix_match

Fix affinity matching.

scheduler/filter/affinity.go

@@ -26,26 +26,25 @@ func (f *AffinityFilter) Filter(config *dockerclient.ContainerConfig, nodes []*c
 		for _, node := range nodes {
 			switch affinity.key {
 			case "container":
+				containers := []string{}
 				for _, container := range node.Containers() {
-					if affinity.Match(container.Id, container.Names[0]) {
-						candidates = append(candidates, node)
-						break
-					}
+					containers = append(containers, container.Id, strings.TrimPrefix(container.Names[0], "/"))
+				}
+				if affinity.Match(containers...) {
+					candidates = append(candidates, node)
 				}
 			case "image":
-			done:
+				images := []string{}
 				for _, image := range node.Images() {
-					if affinity.Match(image.Id) {
-						candidates = append(candidates, node)
-						break
-					}
+					images = append(images, image.Id)
+					images = append(images, image.RepoTags...)
 					for _, tag := range image.RepoTags {
-						if affinity.Match(tag, strings.Split(tag, ":")[0]) {
-							candidates = append(candidates, node)
-							break done
-						}
+						images = append(images, strings.Split(tag, ":")[0])
 					}
 				}
+				if affinity.Match(images...) {
+					candidates = append(candidates, node)
+				}
 			}
 		}
 		if len(candidates) == 0 {

scheduler/filter/affinity_test.go

@@ -24,8 +24,14 @@ func TestAffinityFilter(t *testing.T) {
 	nodes[0].Name = "node-0-name"
 	nodes[0].AddContainer(&cluster.Container{
 		Container: dockerclient.Container{
-			Id:    "container-0-id",
-			Names: []string{"container-0-name"},
+			Id:    "container-n0-0-id",
+			Names: []string{"/container-n0-0-name"},
+		},
+	})
+	nodes[0].AddContainer(&cluster.Container{
+		Container: dockerclient.Container{
+			Id:    "container-n0-1-id",
+			Names: []string{"/container-n0-1-name"},
 		},
 	})
 	nodes[0].AddImage(&dockerclient.Image{
@@ -37,8 +43,14 @@ func TestAffinityFilter(t *testing.T) {
 	nodes[1].Name = "node-1-name"
 	nodes[1].AddContainer(&cluster.Container{
 		Container: dockerclient.Container{
-			Id:    "container-1-id",
-			Names: []string{"container-1-name"},
+			Id:    "container-n1-0-id",
+			Names: []string{"/container-n1-0-name"},
+		},
+	})
+	nodes[1].AddContainer(&cluster.Container{
+		Container: dockerclient.Container{
+			Id:    "container-n1-1-id",
+			Names: []string{"/container-n1-1-name"},
 		},
 	})
 	nodes[1].AddImage(&dockerclient.Image{
@@ -62,7 +74,7 @@ func TestAffinityFilter(t *testing.T) {
 
 	// Set a contraint that can only be filled by a single node.
 	result, err = f.Filter(&dockerclient.ContainerConfig{
-		Env: []string{"affinity:container==container-0*"},
+		Env: []string{"affinity:container==container-n0*"},
 	}, nodes)
 	assert.NoError(t, err)
 	assert.Len(t, result, 1)
@@ -78,34 +90,50 @@ func TestAffinityFilter(t *testing.T) {
 
 	// Validate by id.
 	result, err = f.Filter(&dockerclient.ContainerConfig{
-		Env: []string{"affinity:container==container-0-id"},
+		Env: []string{"affinity:container==container-n0-0-id"},
 	}, nodes)
 	assert.NoError(t, err)
 	assert.Len(t, result, 1)
 	assert.Equal(t, result[0], nodes[0])
 
 	// Validate by id.
 	result, err = f.Filter(&dockerclient.ContainerConfig{
-		Env: []string{"affinity:container!=container-0-id"},
+		Env: []string{"affinity:container!=container-n0-0-id"},
 	}, nodes)
 	assert.NoError(t, err)
-	assert.Len(t, result, 1)
+	assert.Len(t, result, 2)
+	assert.NotContains(t, result, nodes[0])
+
+	// Validate by id.
+	result, err = f.Filter(&dockerclient.ContainerConfig{
+		Env: []string{"affinity:container!=container-n0-1-id"},
+	}, nodes)
+	assert.NoError(t, err)
+	assert.Len(t, result, 2)
 	assert.NotContains(t, result, nodes[0])
 
 	// Validate by name.
 	result, err = f.Filter(&dockerclient.ContainerConfig{
-		Env: []string{"affinity:container==container-1-name"},
+		Env: []string{"affinity:container==container-n1-0-name"},
 	}, nodes)
 	assert.NoError(t, err)
 	assert.Len(t, result, 1)
 	assert.Equal(t, result[0], nodes[1])
 
 	// Validate by name.
 	result, err = f.Filter(&dockerclient.ContainerConfig{
-		Env: []string{"affinity:container!=container-1-name"},
+		Env: []string{"affinity:container!=container-n1-0-name"},
 	}, nodes)
 	assert.NoError(t, err)
-	assert.Len(t, result, 1)
+	assert.Len(t, result, 2)
+	assert.NotContains(t, result, nodes[1])
+
+	// Validate by name.
+	result, err = f.Filter(&dockerclient.ContainerConfig{
+		Env: []string{"affinity:container!=container-n1-1-name"},
+	}, nodes)
+	assert.NoError(t, err)
+	assert.Len(t, result, 2)
 	assert.NotContains(t, result, nodes[1])
 
 	// Validate images by id
@@ -139,6 +167,33 @@ func TestAffinityFilter(t *testing.T) {
 	assert.Len(t, result, 1)
 	assert.Equal(t, result[0], nodes[1])
 
+	// Validate images by name
+	result, err = f.Filter(&dockerclient.ContainerConfig{
+		Env: []string{"affinity:image!=image-1"},
+	}, nodes)
+	assert.NoError(t, err)
+	assert.Len(t, result, 2)
+
+	// Ensure that constraints can be chained.
+	result, err = f.Filter(&dockerclient.ContainerConfig{
+		Env: []string{
+			"affinity:container!=container-n0-1-id",
+			"affinity:container!=container-n1-1-id",
+		},
+	}, nodes)
+	assert.NoError(t, err)
+	assert.Len(t, result, 1)
+	assert.Equal(t, result[0], nodes[2])
+
+	// Ensure that constraints can be chained.
+	result, err = f.Filter(&dockerclient.ContainerConfig{
+		Env: []string{
+			"affinity:container==container-n0-1-id",
+			"affinity:container==container-n1-1-id",
+		},
+	}, nodes)
+	assert.Error(t, err)
+
 	// Not support = any more
 	result, err = f.Filter(&dockerclient.ContainerConfig{
 		Env: []string{"affinity:image=image-0:tag3"},

scheduler/filter/constraint.go

@@ -30,10 +30,8 @@ func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes []
 					candidates = append(candidates, node)
 				}
 			default:
-				if label, ok := node.Labels[constraint.key]; ok {
-					if constraint.Match(label) {
-						candidates = append(candidates, node)
-					}
+				if constraint.Match(node.Labels[constraint.key]) {
+					candidates = append(candidates, node)
 				}
 			}
 		}

scheduler/filter/constraint_test.go

@@ -13,6 +13,7 @@ func testFixtures() (nodes []*cluster.Node) {
 		cluster.NewNode("node-0", 0),
 		cluster.NewNode("node-1", 0),
 		cluster.NewNode("node-2", 0),
+		cluster.NewNode("node-3", 0),
 	}
 	nodes[0].ID = "node-0-id"
 	nodes[0].Name = "node-0-name"
@@ -37,6 +38,8 @@ func testFixtures() (nodes []*cluster.Node) {
 		"group":  "2",
 		"region": "eu",
 	}
+	nodes[3].ID = "node-3-id"
+	nodes[3].Name = "node-3-name"
 	return
 }
 
@@ -132,29 +135,28 @@ func TestConstraintNotExpr(t *testing.T) {
 		Env: []string{"constraint:name!=node0"},
 	}, nodes)
 	assert.NoError(t, err)
-	assert.Len(t, result, 2)
+	assert.Len(t, result, 3)
 
 	// Check not does_not_exist. All should be found
 	result, err = f.Filter(&dockerclient.ContainerConfig{
 		Env: []string{"constraint:name!=does_not_exist"},
 	}, nodes)
 	assert.NoError(t, err)
-	assert.Len(t, result, 3)
+	assert.Len(t, result, 4)
 
 	// Check name must not start with n
 	result, err = f.Filter(&dockerclient.ContainerConfig{
 		Env: []string{"constraint:name!=n*"},
 	}, nodes)
-	assert.Error(t, err)
-	assert.Len(t, result, 0)
+	assert.NoError(t, err)
+	assert.Len(t, result, 1)
 
 	// Check not with globber pattern
 	result, err = f.Filter(&dockerclient.ContainerConfig{
 		Env: []string{"constraint:region!=us*"},
 	}, nodes)
 	assert.NoError(t, err)
-	assert.Len(t, result, 1)
-	assert.Equal(t, result[0].Labels["region"], "eu")
+	assert.Len(t, result, 2)
 }
 
 func TestConstraintRegExp(t *testing.T) {
@@ -179,21 +181,19 @@ func TestConstraintRegExp(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Len(t, result, 2)
 
-	// Check with regular expression ! and regexp /node[12]/ matches node[0]
+	// Check with regular expression ! and regexp /node[12]/ matches node[0] and node[3]
 	result, err = f.Filter(&dockerclient.ContainerConfig{
 		Env: []string{`constraint:name!=/node[12]/`},
 	}, nodes)
 	assert.NoError(t, err)
-	assert.Len(t, result, 1)
-	assert.Equal(t, result[0], nodes[0])
+	assert.Len(t, result, 2)
 
 	// Validate node pinning by ! and regexp.
 	result, err = f.Filter(&dockerclient.ContainerConfig{
 		Env: []string{"constraint:node!=/node-[01]-id/"},
 	}, nodes)
 	assert.NoError(t, err)
-	assert.Len(t, result, 1)
-	assert.Equal(t, result[0], nodes[2])
+	assert.Len(t, result, 2)
 }
 
 func TestFilterRegExpCaseInsensitive(t *testing.T) {
@@ -213,7 +213,7 @@ func TestFilterRegExpCaseInsensitive(t *testing.T) {
 		"group":  "2",
 		"region": "eu",
 	}
-	nodes = append(nodes, node3)
+	nodes[3] = node3
 
 	// Case-sensitive, so not match
 	result, err = f.Filter(&dockerclient.ContainerConfig{

scheduler/filter/expr_test.go

@@ -46,6 +46,7 @@ func TestMatch(t *testing.T) {
 	assert.False(t, e.Match("foo"))
 	assert.True(t, e.Match("bar"))
 	assert.False(t, e.Match("foo", "bar"))
+	assert.False(t, e.Match("bar", "foo"))
 
 	e = expr{operator: EQ, value: "f*o"}
 	assert.True(t, e.Match("foo"))