From 54dea9fe87a7d8f4b7474395d09a8a11b3892d85 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sun, 11 Oct 2015 15:12:39 -0400 Subject: [PATCH 1/9] scheduler: adding version constraint logic --- scheduler/feasible.go | 39 ++++++++++++++++++++++++++++++++++++++ scheduler/feasible_test.go | 39 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index d11a2034e278..b015bd24b32e 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -5,6 +5,7 @@ import ( "reflect" "strings" + "github.com/hashicorp/go-version" "github.com/hashicorp/nomad/nomad/structs" ) @@ -253,7 +254,45 @@ func checkConstraint(operand string, lVal, rVal interface{}) bool { case "contains": // TODO: Implement return false + case "version": + return checkVersionConstraint(lVal, rVal) default: return false } } + +// checkVersionConstraint is used to compare a version on the +// left hand side with a set of constraints on the right hand side +func checkVersionConstraint(lVal, rVal interface{}) bool { + // Parse the version + var versionStr string + switch v := lVal.(type) { + case string: + versionStr = v + case int: + versionStr = fmt.Sprintf("%d", v) + default: + return false + } + + // Parse the verison + vers, err := version.NewVersion(versionStr) + if err != nil { + return false + } + + // Constraint must be a string + constraintStr, ok := rVal.(string) + if !ok { + return false + } + + // Parse the constraints + constraints, err := version.NewConstraint(constraintStr) + if err != nil { + return false + } + + // Check the constraints against the version + return constraints.Check(vers) +} diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index e651dec6fb00..071df55ddf88 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -244,6 +244,11 @@ func TestCheckConstraint(t *testing.T) { lVal: "foo", rVal: "bar", result: true, }, + { + op: "version", + lVal: "1.2.3", rVal: "~> 1.0", + result: true, + }, } for _, tc := range cases { @@ -253,6 +258,40 @@ func TestCheckConstraint(t *testing.T) { } } +func TestCheckVersionConstraint(t *testing.T) { + type tcase struct { + lVal, rVal interface{} + result bool + } + cases := []tcase{ + { + lVal: "1.2.3", rVal: "~> 1.0", + result: true, + }, + { + lVal: "1.2.3", rVal: ">= 1.0, < 1.4", + result: true, + }, + { + lVal: "2.0.1", rVal: "~> 1.0", + result: false, + }, + { + lVal: "1.4", rVal: ">= 1.0, < 1.4", + result: false, + }, + { + lVal: 1, rVal: "~> 1.0", + result: true, + }, + } + for _, tc := range cases { + if res := checkVersionConstraint(tc.lVal, tc.rVal); res != tc.result { + t.Fatalf("TC: %#v, Result: %v", tc, res) + } + } +} + func collectFeasible(iter FeasibleIterator) (out []*structs.Node) { for { next := iter.Next() From a5342d61b7c554bbc00fc53ab0ac216e75e92940 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sun, 11 Oct 2015 15:20:58 -0400 Subject: [PATCH 2/9] jobspec: adding sugar for version constraint --- jobspec/parse.go | 7 +++++++ jobspec/parse_test.go | 20 ++++++++++++++++++++ jobspec/test-fixtures/version-constraint.hcl | 6 ++++++ 3 files changed, 33 insertions(+) create mode 100644 jobspec/test-fixtures/version-constraint.hcl diff --git a/jobspec/parse.go b/jobspec/parse.go index 1231b5ec059e..20086d86e481 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -242,6 +242,13 @@ func parseConstraints(result *[]*structs.Constraint, obj *hclobj.Object) error { m["hard"] = true } + // If "version" is provided, set the operand + // to "version" and the value to the "RTarget" + if constraint, ok := m["version"]; ok { + m["Operand"] = "version" + m["RTarget"] = constraint + } + // Build the constraint var c structs.Constraint if err := mapstructure.WeakDecode(m, &c); err != nil { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index dea59ec266ed..4336b9aac044 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -152,6 +152,26 @@ func TestParse(t *testing.T) { false, }, + { + "version-constraint.hcl", + &structs.Job{ + ID: "foo", + Name: "foo", + Priority: 50, + Region: "global", + Type: "service", + Constraints: []*structs.Constraint{ + &structs.Constraint{ + Hard: true, + LTarget: "$attr.kernel.version", + RTarget: "~> 3.2", + Operand: "version", + }, + }, + }, + false, + }, + { "specify-job.hcl", &structs.Job{ diff --git a/jobspec/test-fixtures/version-constraint.hcl b/jobspec/test-fixtures/version-constraint.hcl new file mode 100644 index 000000000000..3ba7552729db --- /dev/null +++ b/jobspec/test-fixtures/version-constraint.hcl @@ -0,0 +1,6 @@ +job "foo" { + constraint { + attribute = "$attr.kernel.version" + version = "~> 3.2" + } +} From 2aad0838fa20c5cf723395425c6b1e67ae156344 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sun, 11 Oct 2015 15:24:16 -0400 Subject: [PATCH 3/9] website: update the jobspec --- website/source/docs/jobspec/index.html.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/website/source/docs/jobspec/index.html.md b/website/source/docs/jobspec/index.html.md index 9ed167bef653..d5af9703699e 100644 --- a/website/source/docs/jobspec/index.html.md +++ b/website/source/docs/jobspec/index.html.md @@ -223,6 +223,13 @@ The `constraint` object supports the following keys: * `value` - Specifies the value to compare the attribute against. This can be a literal value or another attribute. +* `version` - Specifies a version constraint against the attribute. + This sets the operator to "version" and the `value` to what is + specified. This supports a comma seperated list of constraints, + including the pessimistic operator. See the + [go-version](https://github.com/hashicorp/go-version) repository + for examples. + Below is a table documenting the variables that can be interpreted: From 0a2e8742451cef5343b1edd2cb367c93fcda50bd Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sun, 11 Oct 2015 15:35:13 -0400 Subject: [PATCH 4/9] scheduler: adding regexp constraints --- scheduler/feasible.go | 31 +++++++++++++++++++++++++++--- scheduler/feasible_test.go | 39 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index b015bd24b32e..5ecfbedabaf1 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -3,6 +3,7 @@ package scheduler import ( "fmt" "reflect" + "regexp" "strings" "github.com/hashicorp/go-version" @@ -251,11 +252,10 @@ func checkConstraint(operand string, lVal, rVal interface{}) bool { case "<", "<=", ">", ">=": // TODO: Implement return false - case "contains": - // TODO: Implement - return false case "version": return checkVersionConstraint(lVal, rVal) + case "regexp": + return checkRegexpConstraint(lVal, rVal) default: return false } @@ -296,3 +296,28 @@ func checkVersionConstraint(lVal, rVal interface{}) bool { // Check the constraints against the version return constraints.Check(vers) } + +// checkRegexpConstraint is used to compare a value on the +// left hand side with a regexp on the right hand side +func checkRegexpConstraint(lVal, rVal interface{}) bool { + // Ensure left-hand is string + lStr, ok := lVal.(string) + if !ok { + return false + } + + // Regexp must be a string + regexpStr, ok := rVal.(string) + if !ok { + return false + } + + // Parse the regexp + re, err := regexp.Compile(regexpStr) + if err != nil { + return false + } + + // Look for a match + return re.MatchString(lStr) +} diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index 071df55ddf88..8ee3c6dc5845 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -249,6 +249,11 @@ func TestCheckConstraint(t *testing.T) { lVal: "1.2.3", rVal: "~> 1.0", result: true, }, + { + op: "regexp", + lVal: "foobarbaz", rVal: "[\\w]+", + result: true, + }, } for _, tc := range cases { @@ -292,6 +297,40 @@ func TestCheckVersionConstraint(t *testing.T) { } } +func TestCheckRegexpConstraint(t *testing.T) { + type tcase struct { + lVal, rVal interface{} + result bool + } + cases := []tcase{ + { + lVal: "foobar", rVal: "bar", + result: true, + }, + { + lVal: "foobar", rVal: "^foo", + result: true, + }, + { + lVal: "foobar", rVal: "^bar", + result: false, + }, + { + lVal: "zipzap", rVal: "foo", + result: false, + }, + { + lVal: 1, rVal: "foo", + result: false, + }, + } + for _, tc := range cases { + if res := checkRegexpConstraint(tc.lVal, tc.rVal); res != tc.result { + t.Fatalf("TC: %#v, Result: %v", tc, res) + } + } +} + func collectFeasible(iter FeasibleIterator) (out []*structs.Node) { for { next := iter.Next() From e2093e41891aedf7f0f6047a33e337f11207dae7 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sun, 11 Oct 2015 15:37:50 -0400 Subject: [PATCH 5/9] jobspec: adding sugar for regexp constraint --- jobspec/parse.go | 7 +++++++ jobspec/parse_test.go | 20 ++++++++++++++++++++ jobspec/test-fixtures/regexp-constraint.hcl | 6 ++++++ 3 files changed, 33 insertions(+) create mode 100644 jobspec/test-fixtures/regexp-constraint.hcl diff --git a/jobspec/parse.go b/jobspec/parse.go index 20086d86e481..b7ca31c54595 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -249,6 +249,13 @@ func parseConstraints(result *[]*structs.Constraint, obj *hclobj.Object) error { m["RTarget"] = constraint } + // If "regexp" is provided, set the operand + // to "regexp" and the value to the "RTarget" + if constraint, ok := m["regexp"]; ok { + m["Operand"] = "regexp" + m["RTarget"] = constraint + } + // Build the constraint var c structs.Constraint if err := mapstructure.WeakDecode(m, &c); err != nil { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 4336b9aac044..60c5f1f0946f 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -172,6 +172,26 @@ func TestParse(t *testing.T) { false, }, + { + "regexp-constraint.hcl", + &structs.Job{ + ID: "foo", + Name: "foo", + Priority: 50, + Region: "global", + Type: "service", + Constraints: []*structs.Constraint{ + &structs.Constraint{ + Hard: true, + LTarget: "$attr.kernel.version", + RTarget: "[0-9.]+", + Operand: "regexp", + }, + }, + }, + false, + }, + { "specify-job.hcl", &structs.Job{ diff --git a/jobspec/test-fixtures/regexp-constraint.hcl b/jobspec/test-fixtures/regexp-constraint.hcl new file mode 100644 index 000000000000..dfdb4ce2064a --- /dev/null +++ b/jobspec/test-fixtures/regexp-constraint.hcl @@ -0,0 +1,6 @@ +job "foo" { + constraint { + attribute = "$attr.kernel.version" + regexp = "[0-9.]+" + } +} From 982edafa0190067bdd7462692487c27351555030 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sun, 11 Oct 2015 15:38:38 -0400 Subject: [PATCH 6/9] website: document the regexp constraint --- website/source/docs/jobspec/index.html.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/website/source/docs/jobspec/index.html.md b/website/source/docs/jobspec/index.html.md index d5af9703699e..b8dd7119a659 100644 --- a/website/source/docs/jobspec/index.html.md +++ b/website/source/docs/jobspec/index.html.md @@ -230,6 +230,10 @@ The `constraint` object supports the following keys: [go-version](https://github.com/hashicorp/go-version) repository for examples. +* `regexp` - Specifies a regular expression constraint against + the attribute. This sets the operator to "regexp" and the `value` + to the regular expression. + Below is a table documenting the variables that can be interpreted:
From a3dda996fabc2e15322da3c622be9c5c1b06dda4 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sun, 11 Oct 2015 15:50:16 -0400 Subject: [PATCH 7/9] nomad: additional constraint validation --- nomad/structs/structs.go | 40 +++++++++++++++++++++++++++++++++++ nomad/structs/structs_test.go | 37 ++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 098c57b32c74..09cef210a131 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -4,11 +4,13 @@ import ( "bytes" "errors" "fmt" + "regexp" "strings" "time" "github.com/hashicorp/go-msgpack/codec" "github.com/hashicorp/go-multierror" + "github.com/hashicorp/go-version" ) var ( @@ -809,6 +811,12 @@ func (j *Job) Validate() error { if len(j.TaskGroups) == 0 { mErr.Errors = append(mErr.Errors, errors.New("Missing job task groups")) } + for idx, constr := range j.Constraints { + if err := constr.Validate(); err != nil { + outer := fmt.Errorf("Constraint %d validation failed: %s", idx+1, err) + mErr.Errors = append(mErr.Errors, outer) + } + } // Check for duplicate task groups taskGroups := make(map[string]int) @@ -918,6 +926,12 @@ func (tg *TaskGroup) Validate() error { if len(tg.Tasks) == 0 { mErr.Errors = append(mErr.Errors, errors.New("Missing tasks for task group")) } + for idx, constr := range tg.Constraints { + if err := constr.Validate(); err != nil { + outer := fmt.Errorf("Constraint %d validation failed: %s", idx+1, err) + mErr.Errors = append(mErr.Errors, outer) + } + } // Check for duplicate tasks tasks := make(map[string]int) @@ -997,6 +1011,12 @@ func (t *Task) Validate() error { if t.Resources == nil { mErr.Errors = append(mErr.Errors, errors.New("Missing task resources")) } + for idx, constr := range t.Constraints { + if err := constr.Validate(); err != nil { + outer := fmt.Errorf("Constraint %d validation failed: %s", idx+1, err) + mErr.Errors = append(mErr.Errors, outer) + } + } return mErr.ErrorOrNil() } @@ -1015,6 +1035,26 @@ func (c *Constraint) String() string { return fmt.Sprintf("%s %s %s", c.LTarget, c.Operand, c.RTarget) } +func (c *Constraint) Validate() error { + var mErr multierror.Error + if c.Operand == "" { + mErr.Errors = append(mErr.Errors, errors.New("Missing constraint operand")) + } + + // Perform additional validation based on operand + switch c.Operand { + case "regexp": + if _, err := regexp.Compile(c.RTarget); err != nil { + mErr.Errors = append(mErr.Errors, fmt.Errorf("Regular expression failed to compile: %v", err)) + } + case "version": + if _, err := version.NewConstraint(c.RTarget); err != nil { + mErr.Errors = append(mErr.Errors, fmt.Errorf("Version constraint is invalid: %v", err)) + } + } + return mErr.ErrorOrNil() +} + const ( AllocDesiredStatusRun = "run" // Allocation should run AllocDesiredStatusStop = "stop" // Allocation should stop diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index ca72f540bc40..6df231a677d6 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -125,6 +125,43 @@ func TestTask_Validate(t *testing.T) { } } +func TestConstraint_Validate(t *testing.T) { + c := &Constraint{} + err := c.Validate() + mErr := err.(*multierror.Error) + if !strings.Contains(mErr.Errors[0].Error(), "Missing constraint operand") { + t.Fatalf("err: %s", err) + } + + c = &Constraint{ + LTarget: "$attr.kernel.name", + RTarget: "linux", + Operand: "=", + } + err = c.Validate() + if err != nil { + t.Fatalf("err: %v", err) + } + + // Perform additional regexp validation + c.Operand = "regexp" + c.RTarget = "(foo" + err = c.Validate() + mErr = err.(*multierror.Error) + if !strings.Contains(mErr.Errors[0].Error(), "missing closing") { + t.Fatalf("err: %s", err) + } + + // Perform version validation + c.Operand = "version" + c.RTarget = "~> foo" + err = c.Validate() + mErr = err.(*multierror.Error) + if !strings.Contains(mErr.Errors[0].Error(), "Malformed constraint") { + t.Fatalf("err: %s", err) + } +} + func TestResource_NetIndex(t *testing.T) { r := &Resources{ Networks: []*NetworkResource{ From 641b9f3ee4133352fef6cfab43e4f85e3de4e8eb Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sun, 11 Oct 2015 15:57:06 -0400 Subject: [PATCH 8/9] scheduler: support lexical ordering constraints --- scheduler/feasible.go | 29 ++++++++++++++++++++++-- scheduler/feasible_test.go | 45 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 5ecfbedabaf1..7482a953de26 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -250,8 +250,7 @@ func checkConstraint(operand string, lVal, rVal interface{}) bool { case "!=", "not": return !reflect.DeepEqual(lVal, rVal) case "<", "<=", ">", ">=": - // TODO: Implement - return false + return checkLexicalOrder(operand, lVal, rVal) case "version": return checkVersionConstraint(lVal, rVal) case "regexp": @@ -261,6 +260,32 @@ func checkConstraint(operand string, lVal, rVal interface{}) bool { } } +// checkLexicalOrder is used to check for lexical ordering +func checkLexicalOrder(op string, lVal, rVal interface{}) bool { + // Ensure the values are strings + lStr, ok := lVal.(string) + if !ok { + return false + } + rStr, ok := rVal.(string) + if !ok { + return false + } + + switch op { + case "<": + return lStr < rStr + case "<=": + return lStr <= rStr + case ">": + return lStr > rStr + case ">=": + return lStr >= rStr + default: + return false + } +} + // checkVersionConstraint is used to compare a version on the // left hand side with a set of constraints on the right hand side func checkVersionConstraint(lVal, rVal interface{}) bool { diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index 8ee3c6dc5845..a69b4c8cebc9 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -254,6 +254,11 @@ func TestCheckConstraint(t *testing.T) { lVal: "foobarbaz", rVal: "[\\w]+", result: true, }, + { + op: "<", + lVal: "foo", rVal: "bar", + result: false, + }, } for _, tc := range cases { @@ -263,6 +268,46 @@ func TestCheckConstraint(t *testing.T) { } } +func TestCheckLexicalOrder(t *testing.T) { + type tcase struct { + op string + lVal, rVal interface{} + result bool + } + cases := []tcase{ + { + op: "<", + lVal: "bar", rVal: "foo", + result: true, + }, + { + op: "<=", + lVal: "foo", rVal: "foo", + result: true, + }, + { + op: ">", + lVal: "bar", rVal: "foo", + result: false, + }, + { + op: ">=", + lVal: "bar", rVal: "bar", + result: true, + }, + { + op: ">", + lVal: 1, rVal: "foo", + result: false, + }, + } + for _, tc := range cases { + if res := checkLexicalOrder(tc.op, tc.lVal, tc.rVal); res != tc.result { + t.Fatalf("TC: %#v, Result: %v", tc, res) + } + } +} + func TestCheckVersionConstraint(t *testing.T) { type tcase struct { lVal, rVal interface{} From 6fdbdd83ca2d0db24ccfca83a4f4860c073889d0 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sun, 11 Oct 2015 15:58:25 -0400 Subject: [PATCH 9/9] website: document lexical ordering operators --- website/source/docs/jobspec/index.html.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/website/source/docs/jobspec/index.html.md b/website/source/docs/jobspec/index.html.md index b8dd7119a659..8f772417dbb7 100644 --- a/website/source/docs/jobspec/index.html.md +++ b/website/source/docs/jobspec/index.html.md @@ -218,7 +218,8 @@ The `constraint` object supports the following keys: to true. Soft constraints are not currently supported. * `operator` - Specifies the comparison operator. Defaults to equality, - and can be `=`, `==`, `is`, `!=`, `not`. + and can be `=`, `==`, `is`, `!=`, `not`, `>`, `>=`, `<`, `<=`. The + ordering is compared lexically. * `value` - Specifies the value to compare the attribute against. This can be a literal value or another attribute.