From e546965cf31f55859a54e12a2a13866be48611bd Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Wed, 21 Jan 2015 00:02:27 +0000 Subject: [PATCH] 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) -}