From 37e10e9656376bfc950240d0cc4cb696476a0d22 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Sun, 11 Jan 2015 02:39:47 +0700 Subject: [PATCH 01/15] support not in constraints expressions Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/constraint.go | 16 +++++++-- scheduler/filter/constraint_test.go | 54 ++++++++++++++++++++++------- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/scheduler/filter/constraint.go b/scheduler/filter/constraint.go index 2671b2390b..f2192d45ad 100644 --- a/scheduler/filter/constraint.go +++ b/scheduler/filter/constraint.go @@ -16,6 +16,12 @@ func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes [] constraints := extractEnv("constraint", config.Env) for k, v := range constraints { log.Debugf("matching constraint: %s=%s", k, v) + negate := false + if strings.HasPrefix(v, "!") { + log.Debugf("negate detected") + v = strings.TrimPrefix(v, "!") + negate = true + } candidates := []*cluster.Node{} for _, node := range nodes { switch k { @@ -27,8 +33,14 @@ func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes [] default: // By default match the node labels. if label, ok := node.Labels[k]; ok { - if match(v, label) { - candidates = append(candidates, node) + if negate { + if f.match(v, label) == false { + candidates = append(candidates, node) + } + } else { + if f.match(v, label) { + candidates = append(candidates, node) + } } } } diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index 134dd94745..85116f77cb 100644 --- a/scheduler/filter/constraint_test.go +++ b/scheduler/filter/constraint_test.go @@ -8,18 +8,12 @@ import ( "github.com/stretchr/testify/assert" ) -func TestConstraintFilter(t *testing.T) { - var ( - f = ConstraintFilter{} - nodes = []*cluster.Node{ - cluster.NewNode("node-0", 0), - cluster.NewNode("node-1", 0), - cluster.NewNode("node-2", 0), - } - result []*cluster.Node - err error - ) - +func testFixtures() (nodes []*cluster.Node) { + nodes = []*cluster.Node{ + cluster.NewNode("node-0", 0), + cluster.NewNode("node-1", 0), + cluster.NewNode("node-2", 0), + } nodes[0].ID = "node-0-id" nodes[0].Name = "node-0-name" nodes[0].Labels = map[string]string{ @@ -43,6 +37,17 @@ func TestConstraintFilter(t *testing.T) { "group": "2", "region": "eu", } + return +} + +/* +func TestConstrainteFilter(t *testing.T) { + var ( + f = ConstraintFilter{} + nodes = testFixtures() + result []*cluster.Node + err error + ) // Without constraints we should get the unfiltered list of nodes back. result, err = f.Filter(&dockerclient.ContainerConfig{}, nodes) @@ -108,3 +113,28 @@ func TestConstraintFilter(t *testing.T) { assert.NoError(t, err) assert.Len(t, result, 2) } +*/ + +func TestConstraintNotExpression(t *testing.T) { + var ( + f = ConstraintFilter{} + nodes = testFixtures() + result []*cluster.Node + err error + ) + + // Check not (!) expression + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{"constraint:name=!node0"}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 2) + + // 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") +} From 54c7c12d057357408752e3f15ef6e92bcc577646 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Sun, 11 Jan 2015 13:54:01 +0700 Subject: [PATCH 02/15] improve regexp matching Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/constraint.go | 56 +++++++++++++++++++++++----- scheduler/filter/constraint_test.go | 57 +++++++++++++++++++++++++++-- 2 files changed, 101 insertions(+), 12 deletions(-) diff --git a/scheduler/filter/constraint.go b/scheduler/filter/constraint.go index f2192d45ad..b06645ac8b 100644 --- a/scheduler/filter/constraint.go +++ b/scheduler/filter/constraint.go @@ -12,41 +12,79 @@ import ( type ConstraintFilter struct { } +func (f *ConstraintFilter) extractConstraints(env []string) map[string]string { + constraints := make(map[string]string) + for _, e := range env { + if strings.HasPrefix(e, "constraint:") { + constraint := strings.TrimPrefix(e, "constraint:") + parts := strings.SplitN(constraint, "=", 2) + constraints[strings.ToLower(parts[0])] = strings.ToLower(parts[1]) + } + } + return constraints +} + +// Create the regex for globbing (ex: ub*t* -> ^ub.*t.*$) +// and match. +func (f *ConstraintFilter) match(pattern, s string, useRegex bool) bool { + regex := pattern + if !useRegex { + regex = "^" + strings.Replace(pattern, "*", ".*", -1) + "$" + } + matched, err := regexp.MatchString(regex, strings.ToLower(s)) + if err != nil { + log.Error(err) + } + return matched +} + func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes []*cluster.Node) ([]*cluster.Node, error) { constraints := extractEnv("constraint", config.Env) for k, v := range constraints { + + v0 := v + log.Debugf("matching constraint: %s=%s", k, v) + negate := false + useRegex := false + if strings.HasPrefix(v, "!") { log.Debugf("negate detected") v = strings.TrimPrefix(v, "!") negate = true } + if strings.HasPrefix(v, "/") && strings.HasSuffix(v, "/") { + log.Infof("regex detected") + v = strings.TrimPrefix(strings.TrimSuffix(v, "/"), "/") + useRegex = true + } + candidates := []*cluster.Node{} for _, node := range nodes { switch k { case "node": // "node" label is a special case pinning a container to a specific node. +<<<<<<< HEAD if match(v, node.ID) || match(v, node.Name) { +======= + matchResult := f.match(v, node.ID, useRegex) || f.match(v, node.Name, useRegex) + if (negate && !matchResult) || (!negate && matchResult) { +>>>>>>> improve regexp matching candidates = append(candidates, node) } default: // By default match the node labels. if label, ok := node.Labels[k]; ok { - if negate { - if f.match(v, label) == false { - candidates = append(candidates, node) - } - } else { - if f.match(v, label) { - candidates = append(candidates, node) - } + matchResult := f.match(v, label, useRegex) + if (negate && !matchResult) || (!negate && matchResult) { + candidates = append(candidates, node) } } } } if len(candidates) == 0 { - return nil, fmt.Errorf("unable to find a node that satisfies %s == %s", k, v) + return nil, fmt.Errorf("unable to find a node that satisfies %s = %s", k, v0) } nodes = candidates } diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index 85116f77cb..7f7e4c9168 100644 --- a/scheduler/filter/constraint_test.go +++ b/scheduler/filter/constraint_test.go @@ -40,7 +40,6 @@ func testFixtures() (nodes []*cluster.Node) { return } -/* func TestConstrainteFilter(t *testing.T) { var ( f = ConstraintFilter{} @@ -113,9 +112,8 @@ func TestConstrainteFilter(t *testing.T) { assert.NoError(t, err) assert.Len(t, result, 2) } -*/ -func TestConstraintNotExpression(t *testing.T) { +func TestConstraintNotExpr(t *testing.T) { var ( f = ConstraintFilter{} nodes = testFixtures() @@ -130,6 +128,20 @@ func TestConstraintNotExpression(t *testing.T) { assert.NoError(t, err) assert.Len(t, result, 2) + // 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) + + // 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) + // Check not with globber pattern result, err = f.Filter(&dockerclient.ContainerConfig{ Env: []string{"constraint:region=!us*"}, @@ -138,3 +150,42 @@ func TestConstraintNotExpression(t *testing.T) { assert.Len(t, result, 1) assert.Equal(t, result[0].Labels["region"], "eu") } + +func TestConstraintRegExp(t *testing.T) { + var ( + f = ConstraintFilter{} + nodes = testFixtures() + result []*cluster.Node + err error + ) + + // Check with regular expression /node\d/ matches node{0..2} + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{`constraint:name=/node\d/`}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 3) + + // Check with regular expression /node\d/ matches node{0..2} + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{`constraint:name=/node[12]/`}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 2) + + // Check with regular expression ! and regexp /node[12]/ matches node[0] + 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]) + + // 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]) +} From 564cca1a22eac5d26eb7211a07c2ebed0ebfce3d Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Sun, 11 Jan 2015 14:40:04 +0700 Subject: [PATCH 03/15] explain expression syntax in README Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/scheduler/filter/README.md b/scheduler/filter/README.md index 0f674d9a98..fbcd27dd52 100644 --- a/scheduler/filter/README.md +++ b/scheduler/filter/README.md @@ -147,6 +147,18 @@ CONTAINER ID IMAGE COMMAND CREATED As you can see here, the containers were only scheduled on nodes with the redis imagealreayd pulled. +#### Constraint Expression Syntax + +Additionally, you can use a not (`!`) to negate and a regular expression in the form of `/regexp/` for specifying a constraint. +For example, + +* `name=node1` will match nodes named with `node1`. +* `name=!node1` will match all nodes, except `node1`. +* `region=!us*` will match all nodes outside the regions prefixed with `us`. +* `name=/node[12]/` will match nodes named `node1` and `node2`. +* `name=/node\d/` will match all nodes named with `node` + 1 digit +* `node=!/node-[01]-id/` will match all nodes, except those with ids `node-0-id` and `node-1-id` + ## Port Filter With this filter, `ports` are considered as a unique resource. From 790b1ea45db96f510fbd97dbe10a10d5ce8dc7cc Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Sun, 11 Jan 2015 15:34:07 +0700 Subject: [PATCH 04/15] cleanup and add comments Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/constraint.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scheduler/filter/constraint.go b/scheduler/filter/constraint.go index b06645ac8b..ef264b2ff6 100644 --- a/scheduler/filter/constraint.go +++ b/scheduler/filter/constraint.go @@ -41,21 +41,21 @@ func (f *ConstraintFilter) match(pattern, s string, useRegex bool) bool { func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes []*cluster.Node) ([]*cluster.Node, error) { constraints := extractEnv("constraint", config.Env) for k, v := range constraints { - - v0 := v - log.Debugf("matching constraint: %s=%s", k, v) - negate := false - useRegex := false + // keep the original for display in case of error + v0 := v + negate := false if strings.HasPrefix(v, "!") { log.Debugf("negate detected") v = strings.TrimPrefix(v, "!") negate = true } + + useRegex := false if strings.HasPrefix(v, "/") && strings.HasSuffix(v, "/") { - log.Infof("regex detected") + log.Debugf("regex detected") v = strings.TrimPrefix(strings.TrimSuffix(v, "/"), "/") useRegex = true } From b4a7abdc83c87c2f6296389e6294ed79b5e8835c Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Tue, 13 Jan 2015 14:09:11 +0700 Subject: [PATCH 05/15] add a testcase for escape regexp Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/README.md | 13 ++++++----- scheduler/filter/constraint_test.go | 35 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/scheduler/filter/README.md b/scheduler/filter/README.md index fbcd27dd52..9e0b7bf4de 100644 --- a/scheduler/filter/README.md +++ b/scheduler/filter/README.md @@ -152,12 +152,13 @@ As you can see here, the containers were only scheduled on nodes with the redis Additionally, you can use a not (`!`) to negate and a regular expression in the form of `/regexp/` for specifying a constraint. For example, -* `name=node1` will match nodes named with `node1`. -* `name=!node1` will match all nodes, except `node1`. -* `region=!us*` will match all nodes outside the regions prefixed with `us`. -* `name=/node[12]/` will match nodes named `node1` and `node2`. -* `name=/node\d/` will match all nodes named with `node` + 1 digit -* `node=!/node-[01]-id/` will match all nodes, except those with ids `node-0-id` and `node-1-id` +* `constraint:name=node1` will match nodes named with `node1`. +* `constraint:name=!node1` will match all nodes, except `node1`. +* `constraint:region=!us*` will match all nodes outside the regions prefixed with `us`. +* `constraint:name=/node[12]/` will match nodes named `node1` and `node2`. +* `constraint:name=/node\d/` will match all nodes named with `node` + 1 digit. +* `constraint:node=!/node-[01]-id/` will match all nodes, except those with ids `node-0-id` and `node-1-id`. +* `constraint:name=!/foo\[bar\]/` will match all nodes, except those with name `foo[bar]`. You can see the use of escape characters here. ## Port Filter diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index 7f7e4c9168..edc00ffba2 100644 --- a/scheduler/filter/constraint_test.go +++ b/scheduler/filter/constraint_test.go @@ -189,3 +189,38 @@ func TestConstraintRegExp(t *testing.T) { assert.Len(t, result, 1) assert.Equal(t, result[0], nodes[2]) } + +func TestFilterRegExpWithEscape(t *testing.T) { + var ( + f = ConstraintFilter{} + nodes = testFixtures() + result []*cluster.Node + err error + ) + + // Prepare node with a strange name + node3 := cluster.NewNode("node-3") + node3.ID = "node-3-id" + node3.Name = "node-3-name" + node3.Labels = map[string]string{ + "name": "foo[bar]", + "group": "2", + "region": "eu", + } + nodes = append(nodes, node3) + + // Test filter with a strange name + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{`constraint:name=/foo\[bar\]/`}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 1) + assert.Equal(t, result[0], nodes[3]) + + // Test ! filter with a strange name + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{`constraint:name=!/foo\[bar\]/`}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 3) +} From 5b973d00577070bae82ba6a4d244bb5e1960e5a3 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Wed, 14 Jan 2015 11:24:41 +0700 Subject: [PATCH 06/15] improve to allow optional matching with case-insensitivity Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/constraint.go | 2 +- scheduler/filter/constraint_test.go | 43 +++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/scheduler/filter/constraint.go b/scheduler/filter/constraint.go index ef264b2ff6..bfe8d42b71 100644 --- a/scheduler/filter/constraint.go +++ b/scheduler/filter/constraint.go @@ -31,7 +31,7 @@ func (f *ConstraintFilter) match(pattern, s string, useRegex bool) bool { if !useRegex { regex = "^" + strings.Replace(pattern, "*", ".*", -1) + "$" } - matched, err := regexp.MatchString(regex, strings.ToLower(s)) + matched, err := regexp.MatchString(regex, s) if err != nil { log.Error(err) } diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index edc00ffba2..c0c8baf840 100644 --- a/scheduler/filter/constraint_test.go +++ b/scheduler/filter/constraint_test.go @@ -224,3 +224,46 @@ func TestFilterRegExpWithEscape(t *testing.T) { assert.NoError(t, err) assert.Len(t, result, 3) } + +func TestFilterRegExpCaseInsensitive(t *testing.T) { + var ( + f = ConstraintFilter{} + nodes = testFixtures() + result []*cluster.Node + err error + ) + + // Prepare node with a strange name + node3 := cluster.NewNode("node-3") + node3.ID = "node-3-id" + node3.Name = "node-3-name" + node3.Labels = map[string]string{ + "name": "aBcDeF", + "group": "2", + "region": "eu", + } + nodes = append(nodes, node3) + + // Case-sensitive, so not match + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{`constraint:name=/abcdef/`}, + }, nodes) + assert.Error(t, err) + assert.Len(t, result, 0) + + // Match with case-insensitive + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{`constraint:name=/(?i)abcdef/`}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 1) + assert.Equal(t, result[0], nodes[3]) + assert.Equal(t, result[0].Labels["name"], "aBcDeF") + + // Test ! filter combined with case insensitive + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{`constraint:name=!/(?i)abc*/`}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 3) +} From 25231f3bb34495a0519a97c44c533f73c07975b0 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Wed, 14 Jan 2015 20:56:06 +0700 Subject: [PATCH 07/15] fix test failed per rebase #228 Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/constraint_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index c0c8baf840..786c48b062 100644 --- a/scheduler/filter/constraint_test.go +++ b/scheduler/filter/constraint_test.go @@ -199,7 +199,7 @@ func TestFilterRegExpWithEscape(t *testing.T) { ) // Prepare node with a strange name - node3 := cluster.NewNode("node-3") + node3 := cluster.NewNode("node-3", 0) node3.ID = "node-3-id" node3.Name = "node-3-name" node3.Labels = map[string]string{ @@ -234,7 +234,7 @@ func TestFilterRegExpCaseInsensitive(t *testing.T) { ) // Prepare node with a strange name - node3 := cluster.NewNode("node-3") + node3 := cluster.NewNode("node-3", 0) node3.ID = "node-3-id" node3.Name = "node-3-name" node3.Labels = map[string]string{ From d335f597575c69eee76d1024b3d1d16d0f3291bc Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Wed, 14 Jan 2015 22:08:11 +0700 Subject: [PATCH 08/15] implement alternative negation syntax Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/constraint.go | 9 +++++-- scheduler/filter/constraint_test.go | 38 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/scheduler/filter/constraint.go b/scheduler/filter/constraint.go index bfe8d42b71..9c7b370629 100644 --- a/scheduler/filter/constraint.go +++ b/scheduler/filter/constraint.go @@ -45,12 +45,17 @@ func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes [] // keep the original for display in case of error v0 := v + k0 := k negate := false if strings.HasPrefix(v, "!") { - log.Debugf("negate detected") + log.Debugf("negate detected in value") v = strings.TrimPrefix(v, "!") negate = true + } else if strings.HasSuffix(k, "!") { + log.Debugf("negate detected in key") + k = strings.TrimSuffix(k, "!") + negate = true } useRegex := false @@ -84,7 +89,7 @@ func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes [] } } if len(candidates) == 0 { - return nil, fmt.Errorf("unable to find a node that satisfies %s = %s", k, v0) + return nil, fmt.Errorf("unable to find a node that satisfies %s=%s", k0, v0) } nodes = candidates } diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index 786c48b062..9fc1fb830f 100644 --- a/scheduler/filter/constraint_test.go +++ b/scheduler/filter/constraint_test.go @@ -151,6 +151,44 @@ func TestConstraintNotExpr(t *testing.T) { assert.Equal(t, result[0].Labels["region"], "eu") } +func TestConstraintAlternativeNotExpr(t *testing.T) { + var ( + f = ConstraintFilter{} + nodes = testFixtures() + result []*cluster.Node + err error + ) + + // Check not (!) expression + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{"constraint:name!=node0"}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 2) + + // 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) + + // 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) + + // 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") +} + func TestConstraintRegExp(t *testing.T) { var ( f = ConstraintFilter{} From 7ee722b2d773f4732525b3026177e76b3752ce55 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Wed, 14 Jan 2015 22:26:42 +0700 Subject: [PATCH 09/15] support relative comparisons in constraints Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/constraint.go | 34 +++++++++++++++---- scheduler/filter/constraint_test.go | 51 +++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/scheduler/filter/constraint.go b/scheduler/filter/constraint.go index 9c7b370629..664f987ba3 100644 --- a/scheduler/filter/constraint.go +++ b/scheduler/filter/constraint.go @@ -58,6 +58,16 @@ func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes [] negate = true } + gte := strings.HasSuffix(k, ">") + lte := strings.HasSuffix(k, "<") + if gte { + log.Debugf("gt (>) detected in key") + k = strings.TrimSuffix(k, ">") + } else if lte { + log.Debugf("lt (<) detected in key") + k = strings.TrimSuffix(k, "<") + } + useRegex := false if strings.HasPrefix(v, "/") && strings.HasSuffix(v, "/") { log.Debugf("regex detected") @@ -70,20 +80,32 @@ func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes [] switch k { case "node": // "node" label is a special case pinning a container to a specific node. -<<<<<<< HEAD - if match(v, node.ID) || match(v, node.Name) { -======= matchResult := f.match(v, node.ID, useRegex) || f.match(v, node.Name, useRegex) if (negate && !matchResult) || (!negate && matchResult) { ->>>>>>> improve regexp matching + + if gte && node.ID >= v { candidates = append(candidates, node) + } else if lte && node.ID <= v { + candidates = append(candidates, node) + } else { + // "node" label is a special case pinning a container to a specific node. + matchResult := f.match(v, node.ID, useRegex) || f.match(v, node.Name, useRegex) + if (negate && !matchResult) || (!negate && matchResult) { + candidates = append(candidates, node) + } } default: // By default match the node labels. if label, ok := node.Labels[k]; ok { - matchResult := f.match(v, label, useRegex) - if (negate && !matchResult) || (!negate && matchResult) { + if gte && label >= v { candidates = append(candidates, node) + } else if lte && label <= v { + candidates = append(candidates, node) + } else { + matchResult := f.match(v, label, useRegex) + if (negate && !matchResult) || (!negate && matchResult) { + candidates = append(candidates, node) + } } } } diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index 9fc1fb830f..e39ec69752 100644 --- a/scheduler/filter/constraint_test.go +++ b/scheduler/filter/constraint_test.go @@ -305,3 +305,54 @@ func TestFilterRegExpCaseInsensitive(t *testing.T) { assert.NoError(t, err) assert.Len(t, result, 3) } + +func TestFilterWithRelativeComparisons(t *testing.T) { + var ( + f = ConstraintFilter{} + nodes = testFixtures() + result []*cluster.Node + err error + ) + + // Prepare node with a strange name + node3 := cluster.NewNode("node-3", 0) + node3.ID = "node-3-id" + node3.Name = "node-3-name" + node3.Labels = map[string]string{ + "name": "aBcDeF", + "group": "4", + "kernel": "3.1", + "region": "eu", + } + nodes = append(nodes, node3) + + // Check with less than or equal + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{`constraint:group<=3`}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 3) + + // Check with greater than or equal + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{`constraint:group>=4`}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 1) + + // Another gte check with a complex string + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{`constraint:kernel>=3.0`}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 1) + assert.Equal(t, result[0], nodes[3]) + assert.Equal(t, result[0].Labels["kernel"], "3.1") + + // Check with greater than or equal. This should match node-3-id. + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{`constraint:node>=node-3`}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 1) +} From d77c9596b1226f54e727722e9e0e70439ed86201 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Thu, 15 Jan 2015 17:14:08 +0700 Subject: [PATCH 10/15] add double equals comparison Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/constraint.go | 8 ++++ scheduler/filter/constraint_test.go | 32 ++++++++++++++ scheduler/filter/utils.go | 68 +++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+) diff --git a/scheduler/filter/constraint.go b/scheduler/filter/constraint.go index 664f987ba3..2e9c103679 100644 --- a/scheduler/filter/constraint.go +++ b/scheduler/filter/constraint.go @@ -47,6 +47,14 @@ func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes [] v0 := v k0 := k + // support case of constraint:k==v + // try to check = on both sides, "k= : = : v" and "k : = : =v", to make sure it works everywhere + if strings.HasSuffix(k, "=") { + k = strings.TrimSuffix(k, "=") + } else if strings.HasPrefix(v, "=") { + v = strings.TrimPrefix(v, "=") + } + negate := false if strings.HasPrefix(v, "!") { log.Debugf("negate detected in value") diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index e39ec69752..fce535da77 100644 --- a/scheduler/filter/constraint_test.go +++ b/scheduler/filter/constraint_test.go @@ -356,3 +356,35 @@ func TestFilterWithRelativeComparisons(t *testing.T) { assert.NoError(t, err) assert.Len(t, result, 1) } + +func TestFilterWithDoubleEquals(t *testing.T) { + var ( + f = ConstraintFilter{} + nodes = testFixtures() + result []*cluster.Node + err error + ) + + // Check == comparison + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{"constraint:name==node0"}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 1) + + // Test == with glob + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{"constraint:region==us*"}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 2) + + // Validate node name with == + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{"constraint:node==node-1-name"}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 1) + assert.Equal(t, result[0], nodes[1]) + +} diff --git a/scheduler/filter/utils.go b/scheduler/filter/utils.go index 31a3fdf340..280a9658fe 100644 --- a/scheduler/filter/utils.go +++ b/scheduler/filter/utils.go @@ -7,6 +7,63 @@ import ( log "github.com/Sirupsen/logrus" ) +<<<<<<< HEAD +======= +type comparison int + +const ( + equ = comparison(iota) + neg + gte + lte +) + +func parse(k, v string) (string, string, comparison, bool) { + // default comparison mode + mode := equ + + // support case of constraint:k==v + // with "=", it's possible for an entry "k==v" to be split as: + // 1. "k=" as key and "v" as value + // 2. "k" as key and "=v" as value + // Just to make sure it cover these cases. + if strings.HasSuffix(k, "=") { + k = strings.TrimSuffix(k, "=") + } else if strings.HasPrefix(v, "=") { + v = strings.TrimPrefix(v, "=") + } + + if strings.HasPrefix(v, "!") { + log.Debugf("negate detected in value") + v = strings.TrimPrefix(v, "!") + mode = neg + } else if strings.HasSuffix(k, "!") { + log.Debugf("negate detected in key") + k = strings.TrimSuffix(k, "!") + mode = neg + } else { + if strings.HasSuffix(k, ">") { + log.Debugf("gt (>) detected in key") + k = strings.TrimSuffix(k, ">") + mode = gte + } else if strings.HasSuffix(k, "<") { + log.Debugf("lt (<) detected in key") + k = strings.TrimSuffix(k, "<") + mode = lte + } + } + + useRegex := false + if strings.HasPrefix(v, "/") && strings.HasSuffix(v, "/") { + log.Debugf("regex detected") + v = strings.TrimPrefix(strings.TrimSuffix(v, "/"), "/") + useRegex = true + } + + return k, v, mode, useRegex +} + +>>>>>>> add double equals comparison func extractEnv(key string, env []string) map[string]string { values := make(map[string]string) for _, e := range env { @@ -23,10 +80,21 @@ func extractEnv(key string, env []string) map[string]string { return values } +<<<<<<< HEAD // Create the regex for globbing (ex: ub*t* -> ^ub.*t.*$) and match. func match(pattern, s string) bool { regex := "^" + strings.Replace(pattern, "*", ".*", -1) + "$" matched, err := regexp.MatchString(regex, strings.ToLower(s)) +======= +// Create the regex for globbing (ex: ub*t* -> ^ub.*t.*$) +// and match. +func match(pattern, s string, useRegex bool) bool { + regex := pattern + if !useRegex { + regex = "^" + strings.Replace(pattern, "*", ".*", -1) + "$" + } + matched, err := regexp.MatchString(regex, s) +>>>>>>> add double equals comparison if err != nil { log.Error(err) } From 45b7f263941e7221c089fb2fb940f192061dab19 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Thu, 15 Jan 2015 17:42:01 +0700 Subject: [PATCH 11/15] refactor matching into filter/utils.go Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/README.md | 9 +++- scheduler/filter/constraint.go | 82 ++++------------------------------ scheduler/filter/utils.go | 14 +----- 3 files changed, 19 insertions(+), 86 deletions(-) diff --git a/scheduler/filter/README.md b/scheduler/filter/README.md index 9e0b7bf4de..26a4ca0560 100644 --- a/scheduler/filter/README.md +++ b/scheduler/filter/README.md @@ -150,15 +150,22 @@ As you can see here, the containers were only scheduled on nodes with the redis #### Constraint Expression Syntax Additionally, you can use a not (`!`) to negate and a regular expression in the form of `/regexp/` for specifying a constraint. +Relative comparisons, `>=` and `<=` are also supported, but they are limited to `string` comparison only. + For example, * `constraint:name=node1` will match nodes named with `node1`. -* `constraint:name=!node1` will match all nodes, except `node1`. +* `constraint:name==node1` will also match nodes named with `node1`. Note that `==` also allowed. +* `constraint:name!=node1` will match all nodes, except `node1`. +* `constraint:name=!node1` will match all nodes, except `node1` (alternative syntax). * `constraint:region=!us*` will match all nodes outside the regions prefixed with `us`. * `constraint:name=/node[12]/` will match nodes named `node1` and `node2`. * `constraint:name=/node\d/` will match all nodes named with `node` + 1 digit. * `constraint:node=!/node-[01]-id/` will match all nodes, except those with ids `node-0-id` and `node-1-id`. * `constraint:name=!/foo\[bar\]/` will match all nodes, except those with name `foo[bar]`. You can see the use of escape characters here. +* `constraint:name=/(?i)node1/` will match all nodes named with `node1` case-insensitive. So 'NoDe1' or 'NODE1' will also matched. +* `constraint:kernel>=3.0` will match all nodes with label `kernel` greater than or equal to "3.0". This is the string, not numeric, comparison. +* `constraint:group<=3` will match all nodes with `group` less than or equal to "3". This is also the string, not numeric, comparison. ## Port Filter diff --git a/scheduler/filter/constraint.go b/scheduler/filter/constraint.go index 2e9c103679..98ef32f155 100644 --- a/scheduler/filter/constraint.go +++ b/scheduler/filter/constraint.go @@ -12,32 +12,6 @@ import ( type ConstraintFilter struct { } -func (f *ConstraintFilter) extractConstraints(env []string) map[string]string { - constraints := make(map[string]string) - for _, e := range env { - if strings.HasPrefix(e, "constraint:") { - constraint := strings.TrimPrefix(e, "constraint:") - parts := strings.SplitN(constraint, "=", 2) - constraints[strings.ToLower(parts[0])] = strings.ToLower(parts[1]) - } - } - return constraints -} - -// Create the regex for globbing (ex: ub*t* -> ^ub.*t.*$) -// and match. -func (f *ConstraintFilter) match(pattern, s string, useRegex bool) bool { - regex := pattern - if !useRegex { - regex = "^" + strings.Replace(pattern, "*", ".*", -1) + "$" - } - matched, err := regexp.MatchString(regex, s) - if err != nil { - log.Error(err) - } - return matched -} - func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes []*cluster.Node) ([]*cluster.Node, error) { constraints := extractEnv("constraint", config.Env) for k, v := range constraints { @@ -47,71 +21,33 @@ func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes [] v0 := v k0 := k - // support case of constraint:k==v - // try to check = on both sides, "k= : = : v" and "k : = : =v", to make sure it works everywhere - if strings.HasSuffix(k, "=") { - k = strings.TrimSuffix(k, "=") - } else if strings.HasPrefix(v, "=") { - v = strings.TrimPrefix(v, "=") - } - - negate := false - if strings.HasPrefix(v, "!") { - log.Debugf("negate detected in value") - v = strings.TrimPrefix(v, "!") - negate = true - } else if strings.HasSuffix(k, "!") { - log.Debugf("negate detected in key") - k = strings.TrimSuffix(k, "!") - negate = true - } - - gte := strings.HasSuffix(k, ">") - lte := strings.HasSuffix(k, "<") - if gte { - log.Debugf("gt (>) detected in key") - k = strings.TrimSuffix(k, ">") - } else if lte { - log.Debugf("lt (<) detected in key") - k = strings.TrimSuffix(k, "<") - } - - useRegex := false - if strings.HasPrefix(v, "/") && strings.HasSuffix(v, "/") { - log.Debugf("regex detected") - v = strings.TrimPrefix(strings.TrimSuffix(v, "/"), "/") - useRegex = true - } + k, v, mode, useRegex := parse(k, v) candidates := []*cluster.Node{} for _, node := range nodes { switch k { case "node": - // "node" label is a special case pinning a container to a specific node. - matchResult := f.match(v, node.ID, useRegex) || f.match(v, node.Name, useRegex) - if (negate && !matchResult) || (!negate && matchResult) { - - if gte && node.ID >= v { + if mode == gte && node.ID >= v { candidates = append(candidates, node) - } else if lte && node.ID <= v { + } else if mode == lte && node.ID <= v { candidates = append(candidates, node) } else { // "node" label is a special case pinning a container to a specific node. - matchResult := f.match(v, node.ID, useRegex) || f.match(v, node.Name, useRegex) - if (negate && !matchResult) || (!negate && matchResult) { + matchResult := match(v, node.ID, useRegex) || match(v, node.Name, useRegex) + if (mode == neg && !matchResult) || (mode == equ && matchResult) { candidates = append(candidates, node) } } default: // By default match the node labels. if label, ok := node.Labels[k]; ok { - if gte && label >= v { + if mode == gte && label >= v { candidates = append(candidates, node) - } else if lte && label <= v { + } else if mode == lte && label <= v { candidates = append(candidates, node) } else { - matchResult := f.match(v, label, useRegex) - if (negate && !matchResult) || (!negate && matchResult) { + matchResult := match(v, label, useRegex) + if (mode == neg && !matchResult) || (mode == equ && matchResult) { candidates = append(candidates, node) } } diff --git a/scheduler/filter/utils.go b/scheduler/filter/utils.go index 280a9658fe..8e9cec99bd 100644 --- a/scheduler/filter/utils.go +++ b/scheduler/filter/utils.go @@ -7,8 +7,6 @@ import ( log "github.com/Sirupsen/logrus" ) -<<<<<<< HEAD -======= type comparison int const ( @@ -63,7 +61,6 @@ func parse(k, v string) (string, string, comparison, bool) { return k, v, mode, useRegex } ->>>>>>> add double equals comparison func extractEnv(key string, env []string) map[string]string { values := make(map[string]string) for _, e := range env { @@ -71,7 +68,7 @@ func extractEnv(key string, env []string) map[string]string { value := strings.TrimPrefix(e, key+":") parts := strings.SplitN(value, "=", 2) if len(parts) == 2 { - values[strings.ToLower(parts[0])] = strings.ToLower(parts[1]) + values[strings.ToLower(parts[0])] = parts[1] } else { values[strings.ToLower(parts[0])] = "" } @@ -80,21 +77,14 @@ func extractEnv(key string, env []string) map[string]string { return values } -<<<<<<< HEAD // Create the regex for globbing (ex: ub*t* -> ^ub.*t.*$) and match. -func match(pattern, s string) bool { - regex := "^" + strings.Replace(pattern, "*", ".*", -1) + "$" - matched, err := regexp.MatchString(regex, strings.ToLower(s)) -======= -// Create the regex for globbing (ex: ub*t* -> ^ub.*t.*$) -// and match. +// If useRegex is true, the pattern will be used directly func match(pattern, s string, useRegex bool) bool { regex := pattern if !useRegex { regex = "^" + strings.Replace(pattern, "*", ".*", -1) + "$" } matched, err := regexp.MatchString(regex, s) ->>>>>>> add double equals comparison if err != nil { log.Error(err) } From e4db376a6d461b8b0dd7fcd3ea9c658ab433ea05 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Sat, 17 Jan 2015 01:41:15 +0700 Subject: [PATCH 12/15] support affinity and validate key and value properly Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/README.md | 11 ++++++-- scheduler/filter/affinity.go | 12 ++++++--- scheduler/filter/constraint.go | 5 +++- scheduler/filter/constraint_test.go | 41 +++++------------------------ scheduler/filter/utils.go | 30 +++++++++++++++++++-- scheduler/filter/utils_test.go | 38 ++++++++++++++++++++++++++ 6 files changed, 93 insertions(+), 44 deletions(-) create mode 100644 scheduler/filter/utils_test.go diff --git a/scheduler/filter/README.md b/scheduler/filter/README.md index 26a4ca0560..ecb332d54e 100644 --- a/scheduler/filter/README.md +++ b/scheduler/filter/README.md @@ -149,11 +149,18 @@ As you can see here, the containers were only scheduled on nodes with the redis #### Constraint Expression Syntax -Additionally, you can use a not (`!`) to negate and a regular expression in the form of `/regexp/` for specifying a constraint. +As previously mentioned, a constraint consists of a `key` and a `value`. +A `key` must conform the alpha-numeric pattern, with the leading alphabet or underscore. + +A `value` must be one of the following: +* An alpha-numeric string, dots, hyphens, and underscores. +* A globbing pattern, i.e., `abc*`. +* A regular expression in the form of `/regexp/`. We support the Go's regular expression syntax. + +You can use a not (`!`) to negate and a regular expression in the form of `/regexp/` for specifying a constraint. Relative comparisons, `>=` and `<=` are also supported, but they are limited to `string` comparison only. For example, - * `constraint:name=node1` will match nodes named with `node1`. * `constraint:name==node1` will also match nodes named with `node1`. Note that `==` also allowed. * `constraint:name!=node1` will match all nodes, except `node1`. diff --git a/scheduler/filter/affinity.go b/scheduler/filter/affinity.go index 54f84840c3..894c4baeac 100644 --- a/scheduler/filter/affinity.go +++ b/scheduler/filter/affinity.go @@ -13,7 +13,11 @@ type AffinityFilter struct { } func (f *AffinityFilter) Filter(config *dockerclient.ContainerConfig, nodes []*cluster.Node) ([]*cluster.Node, error) { - affinities := extractEnv("affinity", config.Env) + affinities, err := extractEnv("affinity", config.Env) + if err != nil { + return nil, err + } + for k, v := range affinities { log.Debugf("matching affinity: %s=%s", k, v) candidates := []*cluster.Node{} @@ -21,7 +25,7 @@ func (f *AffinityFilter) Filter(config *dockerclient.ContainerConfig, nodes []*c switch k { case "container": for _, container := range node.Containers() { - if match(v, container.Id) || match(v, container.Names[0]) { + if match(v, container.Id, false) || match(v, container.Names[0], false) { candidates = append(candidates, node) break } @@ -29,12 +33,12 @@ func (f *AffinityFilter) Filter(config *dockerclient.ContainerConfig, nodes []*c case "image": done: for _, image := range node.Images() { - if match(v, image.Id) { + if match(v, image.Id, false) { candidates = append(candidates, node) break } for _, t := range image.RepoTags { - if match(v, t) { + if match(v, t, false) { candidates = append(candidates, node) break done } diff --git a/scheduler/filter/constraint.go b/scheduler/filter/constraint.go index 98ef32f155..df2de93cef 100644 --- a/scheduler/filter/constraint.go +++ b/scheduler/filter/constraint.go @@ -13,7 +13,10 @@ type ConstraintFilter struct { } func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes []*cluster.Node) ([]*cluster.Node, error) { - constraints := extractEnv("constraint", config.Env) + constraints, err := extractEnv("constraint", config.Env) + if err != nil { + return nil, err + } for k, v := range constraints { log.Debugf("matching constraint: %s=%s", k, v) diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index fce535da77..c0ad9ca1c1 100644 --- a/scheduler/filter/constraint_test.go +++ b/scheduler/filter/constraint_test.go @@ -111,6 +111,12 @@ func TestConstrainteFilter(t *testing.T) { }, nodes) assert.NoError(t, err) assert.Len(t, result, 2) + + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{"constraint:region=*us*"}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 2) } func TestConstraintNotExpr(t *testing.T) { @@ -228,41 +234,6 @@ func TestConstraintRegExp(t *testing.T) { assert.Equal(t, result[0], nodes[2]) } -func TestFilterRegExpWithEscape(t *testing.T) { - var ( - f = ConstraintFilter{} - nodes = testFixtures() - result []*cluster.Node - err error - ) - - // Prepare node with a strange name - node3 := cluster.NewNode("node-3", 0) - node3.ID = "node-3-id" - node3.Name = "node-3-name" - node3.Labels = map[string]string{ - "name": "foo[bar]", - "group": "2", - "region": "eu", - } - nodes = append(nodes, node3) - - // Test filter with a strange name - result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{`constraint:name=/foo\[bar\]/`}, - }, nodes) - assert.NoError(t, err) - assert.Len(t, result, 1) - assert.Equal(t, result[0], nodes[3]) - - // Test ! filter with a strange name - result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{`constraint:name=!/foo\[bar\]/`}, - }, nodes) - assert.NoError(t, err) - assert.Len(t, result, 3) -} - func TestFilterRegExpCaseInsensitive(t *testing.T) { var ( f = ConstraintFilter{} diff --git a/scheduler/filter/utils.go b/scheduler/filter/utils.go index 8e9cec99bd..6daaa004ab 100644 --- a/scheduler/filter/utils.go +++ b/scheduler/filter/utils.go @@ -1,6 +1,7 @@ package filter import ( + "fmt" "regexp" "strings" @@ -61,20 +62,45 @@ func parse(k, v string) (string, string, comparison, bool) { return k, v, mode, useRegex } -func extractEnv(key string, env []string) map[string]string { +func extractEnv(key string, env []string) (map[string]string, error) { values := make(map[string]string) for _, e := range env { if strings.HasPrefix(e, key+":") { value := strings.TrimPrefix(e, key+":") parts := strings.SplitN(value, "=", 2) + + // validate key + // allow alpha-numeric + // but also allow !,>,< operators as suffix + matched, err := regexp.MatchString(`^(?i)[a-z_][a-z0-9\-_]+[> ^ub.*t.*$) and match. diff --git a/scheduler/filter/utils_test.go b/scheduler/filter/utils_test.go new file mode 100644 index 0000000000..fe01c92760 --- /dev/null +++ b/scheduler/filter/utils_test.go @@ -0,0 +1,38 @@ +package filter + +import ( + "github.com/stretchr/testify/assert" + "testing" +) + +func TestValidatingEnvExtraction(t *testing.T) { + err := error(nil) + + // Cannot use the leading digit for key + _, err = extractEnv("constraint", []string{"constraint:1node"}) + assert.Error(t, err) + + // Cannot use space in key + _, err = extractEnv("constraint", []string{"constraint:node ==node1"}) + assert.Error(t, err) + + // Cannot use dot in key + _, err = extractEnv("constraint", []string{"constraint:no.de==node1"}) + assert.Error(t, err) + + // Cannot use * in key + _, err = extractEnv("constraint", []string{"constraint:no*de==node1"}) + assert.Error(t, err) + + // Allow leading underscore + _, err = extractEnv("constraint", []string{"constraint:_node==_node1"}) + assert.NoError(t, err) + + // Allow globbing + _, err = extractEnv("constraint", []string{"constraint:node==*node*"}) + assert.NoError(t, err) + + // Allow regexp in value + _, err = extractEnv("constraint", []string{"constraint:node==/(?i)^[a-b]+c*$/"}) + assert.NoError(t, err) +} From 3c147df81afacf7e736e061406400112f2aec1ec Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Tue, 20 Jan 2015 13:31:42 +0700 Subject: [PATCH 13/15] remove unsupported operators Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/README.md | 20 ++-- scheduler/filter/affinity.go | 19 ++-- scheduler/filter/affinity_test.go | 51 ++++++++-- scheduler/filter/constraint.go | 38 +++----- scheduler/filter/constraint_test.go | 94 ++++++++---------- scheduler/filter/utils.go | 146 ++++++++++++---------------- 6 files changed, 180 insertions(+), 188 deletions(-) diff --git a/scheduler/filter/README.md b/scheduler/filter/README.md index ecb332d54e..4aec6529d1 100644 --- a/scheduler/filter/README.md +++ b/scheduler/filter/README.md @@ -45,7 +45,7 @@ Once the nodes are registered with the cluster, the master pulls their respectiv Let's start a MySQL server and make sure it gets good I/O performance by selecting nodes with flash drives: ``` -$ docker run -d -P -e constraint:storage=ssd --name db mysql +$ docker run -d -P -e constraint:storage==ssd --name db mysql f8b693db9cd6 $ docker ps @@ -59,7 +59,7 @@ In this case, the master selected all nodes that met the `storage=ssd` constrain Now we want to run an `nginx` frontend in our cluster. However, we don't want *flash* drives since we'll mostly write logs to disk. ``` -$ docker run -d -P -e constraint:storage=disk --name frontend nginx +$ docker run -d -P -e constraint:storage==disk --name frontend nginx f8b693db9cd6 $ docker ps @@ -161,16 +161,14 @@ You can use a not (`!`) to negate and a regular expression in the form of `/rege Relative comparisons, `>=` and `<=` are also supported, but they are limited to `string` comparison only. For example, -* `constraint:name=node1` will match nodes named with `node1`. -* `constraint:name==node1` will also match nodes named with `node1`. Note that `==` also allowed. +* `constraint:name==node1` will match nodes named with `node1`. * `constraint:name!=node1` will match all nodes, except `node1`. -* `constraint:name=!node1` will match all nodes, except `node1` (alternative syntax). -* `constraint:region=!us*` will match all nodes outside the regions prefixed with `us`. -* `constraint:name=/node[12]/` will match nodes named `node1` and `node2`. -* `constraint:name=/node\d/` will match all nodes named with `node` + 1 digit. -* `constraint:node=!/node-[01]-id/` will match all nodes, except those with ids `node-0-id` and `node-1-id`. -* `constraint:name=!/foo\[bar\]/` will match all nodes, except those with name `foo[bar]`. You can see the use of escape characters here. -* `constraint:name=/(?i)node1/` will match all nodes named with `node1` case-insensitive. So 'NoDe1' or 'NODE1' will also matched. +* `constraint:region!=us*` will match all nodes outside the regions prefixed with `us`. +* `constraint:name==/node[12]/` will match nodes named `node1` and `node2`. +* `constraint:name==/node\d/` will match all nodes named with `node` + 1 digit. +* `constraint:node!=/node-[01]-id/` will match all nodes, except those with ids `node-0-id` and `node-1-id`. +* `constraint:name!=/foo\[bar\]/` will match all nodes, except those with name `foo[bar]`. You can see the use of escape characters here. +* `constraint:name==/(?i)node1/` will match all nodes named with `node1` case-insensitive. So 'NoDe1' or 'NODE1' will also matched. * `constraint:kernel>=3.0` will match all nodes with label `kernel` greater than or equal to "3.0". This is the string, not numeric, comparison. * `constraint:group<=3` will match all nodes with `group` less than or equal to "3". This is also the string, not numeric, comparison. diff --git a/scheduler/filter/affinity.go b/scheduler/filter/affinity.go index 894c4baeac..80ab1635a9 100644 --- a/scheduler/filter/affinity.go +++ b/scheduler/filter/affinity.go @@ -19,13 +19,20 @@ func (f *AffinityFilter) Filter(config *dockerclient.ContainerConfig, nodes []*c } for k, v := range affinities { - log.Debugf("matching affinity: %s=%s", k, v) + log.Debugf("matching affinity: %s%s%s", k, v[0], v[1]) + candidates := []*cluster.Node{} for _, node := range nodes { switch k { case "container": for _, container := range node.Containers() { - if match(v, container.Id, false) || match(v, container.Names[0], false) { + matchResult := false + if v[0] != "!=" { + matchResult = match(v, container.Id) || match(v, container.Names[0]) + } else if v[0] == "!=" { + matchResult = match(v, container.Id) && match(v, container.Names[0]) + } + if matchResult { candidates = append(candidates, node) break } @@ -33,12 +40,12 @@ func (f *AffinityFilter) Filter(config *dockerclient.ContainerConfig, nodes []*c case "image": done: for _, image := range node.Images() { - if match(v, image.Id, false) { + if match(v, image.Id) { candidates = append(candidates, node) break } - for _, t := range image.RepoTags { - if match(v, t, false) { + for _, tag := range image.RepoTags { + if match(v, tag) { candidates = append(candidates, node) break done } @@ -47,7 +54,7 @@ func (f *AffinityFilter) Filter(config *dockerclient.ContainerConfig, nodes []*c } } if len(candidates) == 0 { - return nil, fmt.Errorf("unable to find a node that satisfies %s == %s", k, v) + return nil, fmt.Errorf("unable to find a node that satisfies %s%s%s", k, v[0], v[1]) } nodes = candidates } diff --git a/scheduler/filter/affinity_test.go b/scheduler/filter/affinity_test.go index ff997c9d8e..64f54ce7b8 100644 --- a/scheduler/filter/affinity_test.go +++ b/scheduler/filter/affinity_test.go @@ -56,13 +56,13 @@ func TestAffinityFilter(t *testing.T) { // Set a constraint that cannot be fullfilled and expect an error back. result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"affinity:container=does_not_exsits"}, + Env: []string{"affinity:container==does_not_exsits"}, }, nodes) assert.Error(t, err) // 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-0*"}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 1) @@ -70,7 +70,7 @@ func TestAffinityFilter(t *testing.T) { // This constraint can only be fullfilled by a subset of nodes. result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"affinity:container=container-*"}, + Env: []string{"affinity:container==container-*"}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 2) @@ -78,23 +78,39 @@ 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-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"}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 1) + 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-1-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"}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 1) + assert.NotContains(t, result, nodes[1]) + // Validate images by id result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"affinity:image=image-0-id"}, + Env: []string{"affinity:image==image-0-id"}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 1) @@ -102,9 +118,30 @@ func TestAffinityFilter(t *testing.T) { // Validate images by name result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"affinity:image=image-0:tag3"}, + Env: []string{"affinity:image==image-0:tag3"}, }, nodes) assert.NoError(t, err) 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-0:tag3"}, + }, nodes) + assert.NoError(t, err) + assert.Len(t, result, 2) + + // Not support = any more + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{"affinity:image=image-0:tag3"}, + }, nodes) + assert.Error(t, err) + assert.Len(t, result, 0) + + // Not support =! any more + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{"affinity:image=!image-0:tag3"}, + }, nodes) + assert.Error(t, err) + assert.Len(t, result, 0) } diff --git a/scheduler/filter/constraint.go b/scheduler/filter/constraint.go index df2de93cef..8d23ea24d3 100644 --- a/scheduler/filter/constraint.go +++ b/scheduler/filter/constraint.go @@ -17,48 +17,34 @@ func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes [] if err != nil { return nil, err } + for k, v := range constraints { - log.Debugf("matching constraint: %s=%s", k, v) - - // keep the original for display in case of error - v0 := v - k0 := k - - k, v, mode, useRegex := parse(k, v) + log.Debugf("matching constraint: %s %s %s", k, v[0], v[1]) candidates := []*cluster.Node{} for _, node := range nodes { switch k { case "node": - if mode == gte && node.ID >= v { + // "node" label is a special case pinning a container to a specific node. + matchResult := false + if v[0] != "!=" { + matchResult = match(v, node.ID) || match(v, node.Name) + } else if v[0] == "!=" { + matchResult = match(v, node.ID) && match(v, node.Name) + } + if matchResult { candidates = append(candidates, node) - } else if mode == lte && node.ID <= v { - candidates = append(candidates, node) - } else { - // "node" label is a special case pinning a container to a specific node. - matchResult := match(v, node.ID, useRegex) || match(v, node.Name, useRegex) - if (mode == neg && !matchResult) || (mode == equ && matchResult) { - candidates = append(candidates, node) - } } default: - // By default match the node labels. if label, ok := node.Labels[k]; ok { - if mode == gte && label >= v { + if match(v, label) { candidates = append(candidates, node) - } else if mode == lte && label <= v { - candidates = append(candidates, node) - } else { - matchResult := match(v, label, useRegex) - if (mode == neg && !matchResult) || (mode == equ && matchResult) { - candidates = append(candidates, node) - } } } } } if len(candidates) == 0 { - return nil, fmt.Errorf("unable to find a node that satisfies %s=%s", k0, v0) + return nil, fmt.Errorf("unable to find a node that satisfies %s%s%s", k, v[0], v[1]) } nodes = candidates } diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index c0ad9ca1c1..85c916974a 100644 --- a/scheduler/filter/constraint_test.go +++ b/scheduler/filter/constraint_test.go @@ -55,13 +55,13 @@ func TestConstrainteFilter(t *testing.T) { // Set a constraint that cannot be fullfilled and expect an error back. result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"constraint:does_not_exist=true"}, + Env: []string{"constraint:does_not_exist==true"}, }, nodes) assert.Error(t, err) // Set a contraint that can only be filled by a single node. result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"constraint:name=node1"}, + Env: []string{"constraint:name==node1"}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 1) @@ -69,7 +69,7 @@ func TestConstrainteFilter(t *testing.T) { // This constraint can only be fullfilled by a subset of nodes. result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"constraint:group=1"}, + Env: []string{"constraint:group==1"}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 2) @@ -77,7 +77,7 @@ func TestConstrainteFilter(t *testing.T) { // Validate node pinning by id. result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"constraint:node=node-2-id"}, + Env: []string{"constraint:node==node-2-id"}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 1) @@ -85,7 +85,7 @@ func TestConstrainteFilter(t *testing.T) { // Validate node pinning by name. result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"constraint:node=node-1-name"}, + Env: []string{"constraint:node==node-1-name"}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 1) @@ -93,7 +93,7 @@ func TestConstrainteFilter(t *testing.T) { // Make sure constraints are evaluated as logical ANDs. result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"constraint:name=node0", "constraint:group=1"}, + Env: []string{"constraint:name==node0", "constraint:group==1"}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 1) @@ -101,19 +101,19 @@ func TestConstrainteFilter(t *testing.T) { // Check matching result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"constraint:region=us"}, + Env: []string{"constraint:region==us"}, }, nodes) assert.Error(t, err) assert.Len(t, result, 0) result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"constraint:region=us*"}, + Env: []string{"constraint:region==us*"}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 2) result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"constraint:region=*us*"}, + Env: []string{"constraint:region==*us*"}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 2) @@ -127,44 +127,6 @@ func TestConstraintNotExpr(t *testing.T) { err error ) - // Check not (!) expression - result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"constraint:name=!node0"}, - }, nodes) - assert.NoError(t, err) - assert.Len(t, result, 2) - - // 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) - - // 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) - - // 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") -} - -func TestConstraintAlternativeNotExpr(t *testing.T) { - var ( - f = ConstraintFilter{} - nodes = testFixtures() - result []*cluster.Node - err error - ) - // Check not (!) expression result, err = f.Filter(&dockerclient.ContainerConfig{ Env: []string{"constraint:name!=node0"}, @@ -205,21 +167,21 @@ func TestConstraintRegExp(t *testing.T) { // Check with regular expression /node\d/ matches node{0..2} result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{`constraint:name=/node\d/`}, + Env: []string{`constraint:name==/node\d/`}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 3) // Check with regular expression /node\d/ matches node{0..2} result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{`constraint:name=/node[12]/`}, + Env: []string{`constraint:name==/node[12]/`}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 2) // Check with regular expression ! and regexp /node[12]/ matches node[0] result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{`constraint:name=!/node[12]/`}, + Env: []string{`constraint:name!=/node[12]/`}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 1) @@ -227,7 +189,7 @@ func TestConstraintRegExp(t *testing.T) { // Validate node pinning by ! and regexp. result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{"constraint:node=!/node-[01]-id/"}, + Env: []string{"constraint:node!=/node-[01]-id/"}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 1) @@ -255,14 +217,14 @@ func TestFilterRegExpCaseInsensitive(t *testing.T) { // Case-sensitive, so not match result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{`constraint:name=/abcdef/`}, + Env: []string{`constraint:name==/abcdef/`}, }, nodes) assert.Error(t, err) assert.Len(t, result, 0) // Match with case-insensitive result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{`constraint:name=/(?i)abcdef/`}, + Env: []string{`constraint:name==/(?i)abcdef/`}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 1) @@ -271,7 +233,7 @@ func TestFilterRegExpCaseInsensitive(t *testing.T) { // Test ! filter combined with case insensitive result, err = f.Filter(&dockerclient.ContainerConfig{ - Env: []string{`constraint:name=!/(?i)abc*/`}, + Env: []string{`constraint:name!=/(?i)abc*/`}, }, nodes) assert.NoError(t, err) assert.Len(t, result, 3) @@ -328,7 +290,7 @@ func TestFilterWithRelativeComparisons(t *testing.T) { assert.Len(t, result, 1) } -func TestFilterWithDoubleEquals(t *testing.T) { +func TestFilterEquals(t *testing.T) { var ( f = ConstraintFilter{} nodes = testFixtures() @@ -357,5 +319,25 @@ func TestFilterWithDoubleEquals(t *testing.T) { assert.NoError(t, err) assert.Len(t, result, 1) assert.Equal(t, result[0], nodes[1]) - +} + +func TestUnsupportedOperators(t *testing.T) { + var ( + f = ConstraintFilter{} + nodes = testFixtures() + result []*cluster.Node + err error + ) + + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{"constraint:name=node0"}, + }, nodes) + assert.Error(t, err) + assert.Len(t, result, 0) + + result, err = f.Filter(&dockerclient.ContainerConfig{ + Env: []string{"constraint:name=!node0"}, + }, nodes) + assert.Error(t, err) + assert.Len(t, result, 0) } diff --git a/scheduler/filter/utils.go b/scheduler/filter/utils.go index 6daaa004ab..7591102c12 100644 --- a/scheduler/filter/utils.go +++ b/scheduler/filter/utils.go @@ -8,95 +8,54 @@ import ( log "github.com/Sirupsen/logrus" ) -type comparison int +type opWithValue []string -const ( - equ = comparison(iota) - neg - gte - lte -) - -func parse(k, v string) (string, string, comparison, bool) { - // default comparison mode - mode := equ - - // support case of constraint:k==v - // with "=", it's possible for an entry "k==v" to be split as: - // 1. "k=" as key and "v" as value - // 2. "k" as key and "=v" as value - // Just to make sure it cover these cases. - if strings.HasSuffix(k, "=") { - k = strings.TrimSuffix(k, "=") - } else if strings.HasPrefix(v, "=") { - v = strings.TrimPrefix(v, "=") - } - - if strings.HasPrefix(v, "!") { - log.Debugf("negate detected in value") - v = strings.TrimPrefix(v, "!") - mode = neg - } else if strings.HasSuffix(k, "!") { - log.Debugf("negate detected in key") - k = strings.TrimSuffix(k, "!") - mode = neg - } else { - if strings.HasSuffix(k, ">") { - log.Debugf("gt (>) detected in key") - k = strings.TrimSuffix(k, ">") - mode = gte - } else if strings.HasSuffix(k, "<") { - log.Debugf("lt (<) detected in key") - k = strings.TrimSuffix(k, "<") - mode = lte - } - } - - useRegex := false - if strings.HasPrefix(v, "/") && strings.HasSuffix(v, "/") { - log.Debugf("regex detected") - v = strings.TrimPrefix(strings.TrimSuffix(v, "/"), "/") - useRegex = true - } - - return k, v, mode, useRegex -} - -func extractEnv(key string, env []string) (map[string]string, error) { - values := make(map[string]string) +func extractEnv(key string, env []string) (map[string]opWithValue, error) { + ops := []string{"==", "!=", ">=", "<="} + values := make(map[string]opWithValue) for _, e := range env { if strings.HasPrefix(e, key+":") { - value := strings.TrimPrefix(e, key+":") - parts := strings.SplitN(value, "=", 2) + entry := strings.TrimPrefix(e, key+":") + found := false + for _, op := range ops { + if strings.Contains(entry, op) { + // split with the op + parts := strings.SplitN(entry, op, 2) - // validate key - // allow alpha-numeric - // but also allow !,>,< operators as suffix - matched, err := regexp.MatchString(`^(?i)[a-z_][a-z0-9\-_]+[>=, <= is expected") } } } @@ -105,8 +64,15 @@ func extractEnv(key string, env []string) (map[string]string, error) { // Create the regex for globbing (ex: ub*t* -> ^ub.*t.*$) and match. // If useRegex is true, the pattern will be used directly -func match(pattern, s string, useRegex bool) bool { +func internalMatch(pattern, s string) bool { regex := pattern + useRegex := false + if strings.HasPrefix(pattern, "/") && strings.HasSuffix(pattern, "/") { + log.Debugf("regex detected") + regex = strings.TrimPrefix(strings.TrimSuffix(pattern, "/"), "/") + useRegex = true + } + if !useRegex { regex = "^" + strings.Replace(pattern, "*", ".*", -1) + "$" } @@ -116,3 +82,19 @@ func match(pattern, s string, useRegex bool) bool { } return matched } + +func match(val opWithValue, what string) bool { + op, v := val[0], val[1] + if op == ">=" && what >= v { + return true + } else if op == "<=" && what <= v { + return true + } else { + matchResult := internalMatch(v, what) + if (op == "!=" && !matchResult) || (op == "==" && matchResult) { + return true + } + } + + return false +} From bf852ec2433897bea1bb0cf0298f3b1e18e8df37 Mon Sep 17 00:00:00 2001 From: Chanwit Kaewkasi Date: Tue, 20 Jan 2015 15:42:21 +0700 Subject: [PATCH 14/15] update expression syntax in doc Signed-off-by: Chanwit Kaewkasi --- scheduler/filter/README.md | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/scheduler/filter/README.md b/scheduler/filter/README.md index 4aec6529d1..3bf37e90ff 100644 --- a/scheduler/filter/README.md +++ b/scheduler/filter/README.md @@ -95,11 +95,11 @@ CONTAINER ID IMAGE COMMAND CREATED 87c4376856a8 nginx:latest "nginx" Less than a second ago running 192.168.0.42:80->80/tcp node-1 front ``` -Using `-e affinity:container=front` will schedule a container next to the container `front`. -You can also use IDs instead of name: `-e affinity:container=87c4376856a8` +Using `-e affinity:container==front` will schedule a container next to the container `front`. +You can also use IDs instead of name: `-e affinity:container==87c4376856a8` ``` -$ docker run -d --name logger -e affinity:container=front logger +$ docker run -d --name logger -e affinity:container==front logger 87c4376856a8 $ docker ps @@ -124,14 +124,14 @@ Here only `node-1` and `node-3` have the `redis` image. Using `-e affinity:image schedule container only on these 2 nodes. You can also use the image ID instead of it's name. ``` -$ docker run -d --name redis1 -e affinity:image=redis redis -$ docker run -d --name redis2 -e affinity:image=redis redis -$ docker run -d --name redis3 -e affinity:image=redis redis -$ docker run -d --name redis4 -e affinity:image=redis redis -$ docker run -d --name redis5 -e affinity:image=redis redis -$ docker run -d --name redis6 -e affinity:image=redis redis -$ docker run -d --name redis7 -e affinity:image=redis redis -$ docker run -d --name redis8 -e affinity:image=redis redis +$ docker run -d --name redis1 -e affinity:image==redis redis +$ docker run -d --name redis2 -e affinity:image==redis redis +$ docker run -d --name redis3 -e affinity:image==redis redis +$ docker run -d --name redis4 -e affinity:image==redis redis +$ docker run -d --name redis5 -e affinity:image==redis redis +$ docker run -d --name redis6 -e affinity:image==redis redis +$ docker run -d --name redis7 -e affinity:image==redis redis +$ docker run -d --name redis8 -e affinity:image==redis redis $ docker ps CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NODE NAMES @@ -147,9 +147,9 @@ CONTAINER ID IMAGE COMMAND CREATED As you can see here, the containers were only scheduled on nodes with the redis imagealreayd pulled. -#### Constraint Expression Syntax +#### Expression Syntax -As previously mentioned, a constraint consists of a `key` and a `value`. +An affinity or a constraint expression consists of a `key` and a `value`. A `key` must conform the alpha-numeric pattern, with the leading alphabet or underscore. A `value` must be one of the following: @@ -157,8 +157,8 @@ A `value` must be one of the following: * A globbing pattern, i.e., `abc*`. * A regular expression in the form of `/regexp/`. We support the Go's regular expression syntax. -You can use a not (`!`) to negate and a regular expression in the form of `/regexp/` for specifying a constraint. -Relative comparisons, `>=` and `<=` are also supported, but they are limited to `string` comparison only. +Current `swarm` supports affinity/constraint operators as the following: `==`, `!=`, `>=` and `<=`. +Relative comparisons, `>=` and `<=` are supported, but limited to `string` comparison only. For example, * `constraint:name==node1` will match nodes named with `node1`. From e546965cf31f55859a54e12a2a13866be48611bd Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Wed, 21 Jan 2015 00:02:27 +0000 Subject: [PATCH 15/15] refactor and remove <= and >= Signed-off-by: Victor Vieux --- contrib/demo.sh | 8 +-- scheduler/filter/README.md | 5 +- scheduler/filter/affinity.go | 22 +++--- scheduler/filter/constraint.go | 22 +++--- scheduler/filter/constraint_test.go | 1 + scheduler/filter/expr.go | 106 ++++++++++++++++++++++++++++ scheduler/filter/expr_test.go | 59 ++++++++++++++++ scheduler/filter/utils.go | 100 -------------------------- scheduler/filter/utils_test.go | 38 ---------- 9 files changed, 187 insertions(+), 174 deletions(-) create mode 100644 scheduler/filter/expr.go create mode 100644 scheduler/filter/expr_test.go delete mode 100644 scheduler/filter/utils.go delete mode 100644 scheduler/filter/utils_test.go diff --git a/contrib/demo.sh b/contrib/demo.sh index 0d04da2e03..04727330b0 100644 --- a/contrib/demo.sh +++ b/contrib/demo.sh @@ -43,16 +43,16 @@ docker run -d -p 80:80 nginx # clean up cluster docker rm -f `docker ps -aq` -docker run -d -e "constraint:operatingsystem=fedora*" redis +docker run -d -e "constraint:operatingsystem==fedora*" redis docker ps -docker run -d -e constraint:storagedriver=devicemapper redis +docker run -d -e constraint:storagedriver==devicemapper redis docker ps -docker run -d -e constraint:storagedriver=aufs redis +docker run -d -e constraint:storagedriver==aufs redis docker ps -docker run -d -e constraint:node=fedora-1 redis +docker run -d -e constraint:node==fedora-1 redis docker ps # clean up cluster diff --git a/scheduler/filter/README.md b/scheduler/filter/README.md index 3bf37e90ff..fad98b44f5 100644 --- a/scheduler/filter/README.md +++ b/scheduler/filter/README.md @@ -157,8 +157,7 @@ A `value` must be one of the following: * A globbing pattern, i.e., `abc*`. * A regular expression in the form of `/regexp/`. We support the Go's regular expression syntax. -Current `swarm` supports affinity/constraint operators as the following: `==`, `!=`, `>=` and `<=`. -Relative comparisons, `>=` and `<=` are supported, but limited to `string` comparison only. +Current `swarm` supports affinity/constraint operators as the following: `==` and `!=`. For example, * `constraint:name==node1` will match nodes named with `node1`. @@ -169,8 +168,6 @@ For example, * `constraint:node!=/node-[01]-id/` will match all nodes, except those with ids `node-0-id` and `node-1-id`. * `constraint:name!=/foo\[bar\]/` will match all nodes, except those with name `foo[bar]`. You can see the use of escape characters here. * `constraint:name==/(?i)node1/` will match all nodes named with `node1` case-insensitive. So 'NoDe1' or 'NODE1' will also matched. -* `constraint:kernel>=3.0` will match all nodes with label `kernel` greater than or equal to "3.0". This is the string, not numeric, comparison. -* `constraint:group<=3` will match all nodes with `group` less than or equal to "3". This is also the string, not numeric, comparison. ## Port Filter diff --git a/scheduler/filter/affinity.go b/scheduler/filter/affinity.go index 80ab1635a9..9722a31752 100644 --- a/scheduler/filter/affinity.go +++ b/scheduler/filter/affinity.go @@ -13,26 +13,20 @@ type AffinityFilter struct { } func (f *AffinityFilter) Filter(config *dockerclient.ContainerConfig, nodes []*cluster.Node) ([]*cluster.Node, error) { - affinities, err := extractEnv("affinity", config.Env) + affinities, err := parseExprs("affinity", config.Env) if err != nil { return nil, err } - for k, v := range affinities { - log.Debugf("matching affinity: %s%s%s", k, v[0], v[1]) + for _, affinity := range affinities { + log.Debugf("matching affinity: %s%s%s", affinity.key, OPERATORS[affinity.operator], affinity.value) candidates := []*cluster.Node{} for _, node := range nodes { - switch k { + switch affinity.key { case "container": for _, container := range node.Containers() { - matchResult := false - if v[0] != "!=" { - matchResult = match(v, container.Id) || match(v, container.Names[0]) - } else if v[0] == "!=" { - matchResult = match(v, container.Id) && match(v, container.Names[0]) - } - if matchResult { + if affinity.Match(container.Id, container.Names[0]) { candidates = append(candidates, node) break } @@ -40,12 +34,12 @@ func (f *AffinityFilter) Filter(config *dockerclient.ContainerConfig, nodes []*c case "image": done: for _, image := range node.Images() { - if match(v, image.Id) { + if affinity.Match(image.Id) { candidates = append(candidates, node) break } for _, tag := range image.RepoTags { - if match(v, tag) { + if affinity.Match(tag) { candidates = append(candidates, node) break done } @@ -54,7 +48,7 @@ func (f *AffinityFilter) Filter(config *dockerclient.ContainerConfig, nodes []*c } } if len(candidates) == 0 { - return nil, fmt.Errorf("unable to find a node that satisfies %s%s%s", k, v[0], v[1]) + return nil, fmt.Errorf("unable to find a node that satisfies %s%s%s", affinity.key, OPERATORS[affinity.operator], affinity.value) } nodes = candidates } diff --git a/scheduler/filter/constraint.go b/scheduler/filter/constraint.go index 8d23ea24d3..e994a98948 100644 --- a/scheduler/filter/constraint.go +++ b/scheduler/filter/constraint.go @@ -13,38 +13,32 @@ type ConstraintFilter struct { } func (f *ConstraintFilter) Filter(config *dockerclient.ContainerConfig, nodes []*cluster.Node) ([]*cluster.Node, error) { - constraints, err := extractEnv("constraint", config.Env) + constraints, err := parseExprs("constraint", config.Env) if err != nil { return nil, err } - for k, v := range constraints { - log.Debugf("matching constraint: %s %s %s", k, v[0], v[1]) + for _, constraint := range constraints { + log.Debugf("matching constraint: %s %s %s", constraint.key, OPERATORS[constraint.operator], constraint.value) candidates := []*cluster.Node{} for _, node := range nodes { - switch k { + switch constraint.key { case "node": // "node" label is a special case pinning a container to a specific node. - matchResult := false - if v[0] != "!=" { - matchResult = match(v, node.ID) || match(v, node.Name) - } else if v[0] == "!=" { - matchResult = match(v, node.ID) && match(v, node.Name) - } - if matchResult { + if constraint.Match(node.ID, node.Name) { candidates = append(candidates, node) } default: - if label, ok := node.Labels[k]; ok { - if match(v, label) { + if label, ok := node.Labels[constraint.key]; ok { + if constraint.Match(label) { candidates = append(candidates, node) } } } } if len(candidates) == 0 { - return nil, fmt.Errorf("unable to find a node that satisfies %s%s%s", k, v[0], v[1]) + return nil, fmt.Errorf("unable to find a node that satisfies %s%s%s", constraint.key, OPERATORS[constraint.operator], constraint.value) } nodes = candidates } diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index 85c916974a..016aeeaada 100644 --- a/scheduler/filter/constraint_test.go +++ b/scheduler/filter/constraint_test.go @@ -240,6 +240,7 @@ func TestFilterRegExpCaseInsensitive(t *testing.T) { } func TestFilterWithRelativeComparisons(t *testing.T) { + t.Skip() var ( f = ConstraintFilter{} nodes = testFixtures() diff --git a/scheduler/filter/expr.go b/scheduler/filter/expr.go new file mode 100644 index 0000000000..cbb526e121 --- /dev/null +++ b/scheduler/filter/expr.go @@ -0,0 +1,106 @@ +package filter + +import ( + "fmt" + "regexp" + "strings" + + log "github.com/Sirupsen/logrus" +) + +const ( + EQ = iota + NOTEQ +) + +var OPERATORS = []string{"==", "!="} + +type expr struct { + key string + operator int + value string +} + +func parseExprs(key string, env []string) ([]expr, error) { + exprs := []expr{} + for _, e := range env { + if strings.HasPrefix(e, key+":") { + entry := strings.TrimPrefix(e, key+":") + found := false + for i, op := range OPERATORS { + if strings.Contains(entry, op) { + // split with the op + parts := strings.SplitN(entry, op, 2) + + // validate key + // allow alpha-numeric + matched, err := regexp.MatchString(`^(?i)[a-z_][a-z0-9\-_]+$`, parts[0]) + if err != nil { + return nil, err + } + if matched == false { + return nil, fmt.Errorf("Key '%s' is invalid", parts[0]) + } + + if len(parts) == 2 { + + // validate value + // allow leading = in case of using == + // allow * for globbing + // allow regexp + matched, err := regexp.MatchString(`^(?i)[=!\/]?[a-z0-9:\-_\.\*/\(\)\?\+\[\]\\\^\$]+$`, parts[1]) + if err != nil { + return nil, err + } + if matched == false { + return nil, fmt.Errorf("Value '%s' is invalid", parts[1]) + } + exprs = append(exprs, expr{key: strings.ToLower(parts[0]), operator: i, value: parts[1]}) + } else { + exprs = append(exprs, expr{key: strings.ToLower(parts[0]), operator: i}) + } + + found = true + break // found an op, move to next entry + } + } + if !found { + return nil, fmt.Errorf("One of operator ==, != is expected") + } + } + } + return exprs, nil +} + +func (e *expr) Match(whats ...string) bool { + var ( + pattern string + match bool + err error + ) + + if e.value[0] == '/' && e.value[len(e.value)-1] == '/' { + // regexp + pattern = e.value[1 : len(e.value)-1] + } else { + // simple match, create the regex for globbing (ex: ub*t* -> ^ub.*t.*$) and match. + pattern = "^" + strings.Replace(e.value, "*", ".*", -1) + "$" + } + + for _, what := range whats { + if match, err = regexp.MatchString(pattern, what); match { + break + } else if err != nil { + log.Error(err) + } + } + + switch e.operator { + case EQ: + return match + case NOTEQ: + return !match + } + + return false +} diff --git a/scheduler/filter/expr_test.go b/scheduler/filter/expr_test.go new file mode 100644 index 0000000000..1f7a357577 --- /dev/null +++ b/scheduler/filter/expr_test.go @@ -0,0 +1,59 @@ +package filter + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseExprs(t *testing.T) { + // Cannot use the leading digit for key + _, err := parseExprs("constraint", []string{"constraint:1node"}) + assert.Error(t, err) + + // Cannot use space in key + _, err = parseExprs("constraint", []string{"constraint:node ==node1"}) + assert.Error(t, err) + + // Cannot use dot in key + _, err = parseExprs("constraint", []string{"constraint:no.de==node1"}) + assert.Error(t, err) + + // Cannot use * in key + _, err = parseExprs("constraint", []string{"constraint:no*de==node1"}) + assert.Error(t, err) + + // Allow leading underscore + _, err = parseExprs("constraint", []string{"constraint:_node==_node1"}) + assert.NoError(t, err) + + // Allow globbing + _, err = parseExprs("constraint", []string{"constraint:node==*node*"}) + assert.NoError(t, err) + + // Allow regexp in value + _, err = parseExprs("constraint", []string{"constraint:node==/(?i)^[a-b]+c*$/"}) + assert.NoError(t, err) +} + +func TestMatch(t *testing.T) { + e := expr{operator: EQ, value: "foo"} + assert.True(t, e.Match("foo")) + assert.False(t, e.Match("bar")) + assert.True(t, e.Match("foo", "bar")) + + e = expr{operator: NOTEQ, value: "foo"} + assert.False(t, e.Match("foo")) + assert.True(t, e.Match("bar")) + assert.False(t, e.Match("foo", "bar")) + + e = expr{operator: EQ, value: "f*o"} + assert.True(t, e.Match("foo")) + assert.True(t, e.Match("fuo")) + assert.True(t, e.Match("foo", "fuo", "bar")) + + e = expr{operator: NOTEQ, value: "f*o"} + assert.False(t, e.Match("foo")) + assert.False(t, e.Match("fuo")) + assert.False(t, e.Match("foo", "fuo", "bar")) +} diff --git a/scheduler/filter/utils.go b/scheduler/filter/utils.go deleted file mode 100644 index 7591102c12..0000000000 --- a/scheduler/filter/utils.go +++ /dev/null @@ -1,100 +0,0 @@ -package filter - -import ( - "fmt" - "regexp" - "strings" - - log "github.com/Sirupsen/logrus" -) - -type opWithValue []string - -func extractEnv(key string, env []string) (map[string]opWithValue, error) { - ops := []string{"==", "!=", ">=", "<="} - values := make(map[string]opWithValue) - for _, e := range env { - if strings.HasPrefix(e, key+":") { - entry := strings.TrimPrefix(e, key+":") - found := false - for _, op := range ops { - if strings.Contains(entry, op) { - // split with the op - parts := strings.SplitN(entry, op, 2) - - // validate key - // allow alpha-numeric - matched, err := regexp.MatchString(`^(?i)[a-z_][a-z0-9\-_]+$`, parts[0]) - if err != nil { - return nil, err - } - if matched == false { - return nil, fmt.Errorf("Key '%s' is invalid", parts[0]) - } - - if len(parts) == 2 { - - // validate value - // allow leading = in case of using == - // allow * for globbing - // allow regexp - matched, err := regexp.MatchString(`^(?i)[=!\/]?[a-z0-9:\-_\.\*/\(\)\?\+\[\]\\\^\$]+$`, parts[1]) - if err != nil { - return nil, err - } - if matched == false { - return nil, fmt.Errorf("Value '%s' is invalid", parts[1]) - } - values[strings.ToLower(parts[0])] = opWithValue{op, parts[1]} - } else { - values[strings.ToLower(parts[0])] = opWithValue{op, ""} - } - - found = true - break // found an op, move to next entry - } - } - if !found { - return nil, fmt.Errorf("One of operator ==, !=, >=, <= is expected") - } - } - } - return values, nil -} - -// Create the regex for globbing (ex: ub*t* -> ^ub.*t.*$) and match. -// If useRegex is true, the pattern will be used directly -func internalMatch(pattern, s string) bool { - regex := pattern - useRegex := false - if strings.HasPrefix(pattern, "/") && strings.HasSuffix(pattern, "/") { - log.Debugf("regex detected") - regex = strings.TrimPrefix(strings.TrimSuffix(pattern, "/"), "/") - useRegex = true - } - - if !useRegex { - regex = "^" + strings.Replace(pattern, "*", ".*", -1) + "$" - } - matched, err := regexp.MatchString(regex, s) - if err != nil { - log.Error(err) - } - return matched -} - -func match(val opWithValue, what string) bool { - op, v := val[0], val[1] - if op == ">=" && what >= v { - return true - } else if op == "<=" && what <= v { - return true - } else { - matchResult := internalMatch(v, what) - if (op == "!=" && !matchResult) || (op == "==" && matchResult) { - return true - } - } - - return false -} diff --git a/scheduler/filter/utils_test.go b/scheduler/filter/utils_test.go deleted file mode 100644 index fe01c92760..0000000000 --- a/scheduler/filter/utils_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package filter - -import ( - "github.com/stretchr/testify/assert" - "testing" -) - -func TestValidatingEnvExtraction(t *testing.T) { - err := error(nil) - - // Cannot use the leading digit for key - _, err = extractEnv("constraint", []string{"constraint:1node"}) - assert.Error(t, err) - - // Cannot use space in key - _, err = extractEnv("constraint", []string{"constraint:node ==node1"}) - assert.Error(t, err) - - // Cannot use dot in key - _, err = extractEnv("constraint", []string{"constraint:no.de==node1"}) - assert.Error(t, err) - - // Cannot use * in key - _, err = extractEnv("constraint", []string{"constraint:no*de==node1"}) - assert.Error(t, err) - - // Allow leading underscore - _, err = extractEnv("constraint", []string{"constraint:_node==_node1"}) - assert.NoError(t, err) - - // Allow globbing - _, err = extractEnv("constraint", []string{"constraint:node==*node*"}) - assert.NoError(t, err) - - // Allow regexp in value - _, err = extractEnv("constraint", []string{"constraint:node==/(?i)^[a-b]+c*$/"}) - assert.NoError(t, err) -}