From 796758b8a5025e173099ced822d1d4fe12bc5419 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Wed, 13 Nov 2019 15:36:15 -0800 Subject: [PATCH] core: add semver constraint The existing version constraint uses logic optimized for package managers, not schedulers, when checking prereleases: - 1.3.0-beta1 will *not* satisfy ">= 0.6.1" - 1.7.0-rc1 will *not* satisfy ">= 1.6.0-beta1" This is due to package managers wishing to favor final releases over prereleases. In a scheduler versions more often represent the earliest release all required features/APIs are available in a system. Whether the constraint or the version being evaluated are prereleases has no impact on ordering. This commit adds a new constraint - `semver` - which will use Semver v2.0 ordering when evaluating constraints. Given the above examples: - 1.3.0-beta1 satisfies ">= 0.6.1" using `semver` - 1.7.0-rc1 satisfies ">= 1.6.0-beta1" using `semver` Since existing jobspecs may rely on the old behavior, a new constraint was added and the implicit Consul Connect and Vault constraints were updated to use it. --- api/constraint.go | 1 + helper/constraints/semver/constraints.go | 147 ++++++++++++++++++ helper/constraints/semver/constraints_test.go | 125 +++++++++++++++ jobspec/parse.go | 16 ++ jobspec/parse_test.go | 5 + jobspec/test-fixtures/basic.hcl | 6 + nomad/job_endpoint.go | 6 + nomad/job_endpoint_hook_connect.go | 4 +- nomad/job_endpoint_hooks.go | 2 +- nomad/job_endpoint_test.go | 105 +++++++++++++ nomad/structs/structs.go | 10 ++ nomad/structs/structs_test.go | 13 +- scheduler/context.go | 26 +++- scheduler/feasible.go | 92 ++++++++--- scheduler/feasible_test.go | 82 +++++++++- 15 files changed, 603 insertions(+), 37 deletions(-) create mode 100644 helper/constraints/semver/constraints.go create mode 100644 helper/constraints/semver/constraints_test.go diff --git a/api/constraint.go b/api/constraint.go index 7b18ade9f..3233a3bfd 100644 --- a/api/constraint.go +++ b/api/constraint.go @@ -5,6 +5,7 @@ const ( ConstraintDistinctHosts = "distinct_hosts" ConstraintRegex = "regexp" ConstraintVersion = "version" + ConstraintSemver = "semver" ConstraintSetContains = "set_contains" ConstraintSetContainsAll = "set_contains_all" ConstraintSetContainsAny = "set_contains_any" diff --git a/helper/constraints/semver/constraints.go b/helper/constraints/semver/constraints.go new file mode 100644 index 000000000..0a100623d --- /dev/null +++ b/helper/constraints/semver/constraints.go @@ -0,0 +1,147 @@ +// semver is a Semver Constraints package copied from +// github.com/hashicorp/go-version @ 2046c9d0f0b03c779670f5186a2a4b2c85493a71 +// +// Unlike Constraints in go-version, Semver constraints use Semver 2.0 ordering +// rules and only accept properly formatted Semver versions. +package semver + +import ( + "fmt" + "regexp" + "strings" + + "github.com/hashicorp/go-version" +) + +// Constraint represents a single constraint for a version, such as ">= +// 1.0". +type Constraint struct { + f constraintFunc + check *version.Version + original string +} + +// Constraints is a slice of constraints. We make a custom type so that +// we can add methods to it. +type Constraints []*Constraint + +type constraintFunc func(v, c *version.Version) bool + +var constraintOperators map[string]constraintFunc + +var constraintRegexp *regexp.Regexp + +func init() { + constraintOperators = map[string]constraintFunc{ + "": constraintEqual, + "=": constraintEqual, + "!=": constraintNotEqual, + ">": constraintGreaterThan, + "<": constraintLessThan, + ">=": constraintGreaterThanEqual, + "<=": constraintLessThanEqual, + } + + ops := make([]string, 0, len(constraintOperators)) + for k := range constraintOperators { + ops = append(ops, regexp.QuoteMeta(k)) + } + + constraintRegexp = regexp.MustCompile(fmt.Sprintf( + `^\s*(%s)\s*(%s)\s*$`, + strings.Join(ops, "|"), + version.SemverRegexpRaw)) +} + +// NewConstraint will parse one or more constraints from the given +// constraint string. The string must be a comma-separated list of constraints. +func NewConstraint(v string) (Constraints, error) { + vs := strings.Split(v, ",") + result := make([]*Constraint, len(vs)) + for i, single := range vs { + c, err := parseSingle(single) + if err != nil { + return nil, err + } + + result[i] = c + } + + return Constraints(result), nil +} + +// Check tests if a version satisfies all the constraints. +func (cs Constraints) Check(v *version.Version) bool { + for _, c := range cs { + if !c.Check(v) { + return false + } + } + + return true +} + +// Returns the string format of the constraints +func (cs Constraints) String() string { + csStr := make([]string, len(cs)) + for i, c := range cs { + csStr[i] = c.String() + } + + return strings.Join(csStr, ",") +} + +// Check tests if a constraint is validated by the given version. +func (c *Constraint) Check(v *version.Version) bool { + return c.f(v, c.check) +} + +func (c *Constraint) String() string { + return c.original +} + +func parseSingle(v string) (*Constraint, error) { + matches := constraintRegexp.FindStringSubmatch(v) + if matches == nil { + return nil, fmt.Errorf("Malformed constraint: %s", v) + } + + check, err := version.NewSemver(matches[2]) + if err != nil { + return nil, err + } + + return &Constraint{ + f: constraintOperators[matches[1]], + check: check, + original: v, + }, nil +} + +//------------------------------------------------------------------- +// Constraint functions +//------------------------------------------------------------------- + +func constraintEqual(v, c *version.Version) bool { + return v.Equal(c) +} + +func constraintNotEqual(v, c *version.Version) bool { + return !v.Equal(c) +} + +func constraintGreaterThan(v, c *version.Version) bool { + return v.Compare(c) == 1 +} + +func constraintLessThan(v, c *version.Version) bool { + return v.Compare(c) == -1 +} + +func constraintGreaterThanEqual(v, c *version.Version) bool { + return v.Compare(c) >= 0 +} + +func constraintLessThanEqual(v, c *version.Version) bool { + return v.Compare(c) <= 0 +} diff --git a/helper/constraints/semver/constraints_test.go b/helper/constraints/semver/constraints_test.go new file mode 100644 index 000000000..0759414ae --- /dev/null +++ b/helper/constraints/semver/constraints_test.go @@ -0,0 +1,125 @@ +package semver + +import ( + "testing" + + "github.com/hashicorp/go-version" +) + +// This file is a copy of github.com/hashicorp/go-version/constraint_test.go +// with minimal changes to demonstrate differences. Diffing the files should +// illustrate behavior differences in Constraint and version.Constraint. + +func TestNewConstraint(t *testing.T) { + cases := []struct { + input string + count int + err bool + }{ + {">= 1.2", 1, false}, + {"1.0", 1, false}, + {">= 1.x", 0, true}, + {">= 1.2, < 1.0", 2, false}, + + // Out of bounds + {"11387778780781445675529500000000000000000", 0, true}, + + // Semver only + {">= 1.0beta1", 0, true}, + + // No pessimistic operator + {"~> 1.0", 0, true}, + } + + for _, tc := range cases { + v, err := NewConstraint(tc.input) + if tc.err && err == nil { + t.Fatalf("expected error for input: %s", tc.input) + } else if !tc.err && err != nil { + t.Fatalf("error for input %s: %s", tc.input, err) + } + + if len(v) != tc.count { + t.Fatalf("input: %s\nexpected len: %d\nactual: %d", + tc.input, tc.count, len(v)) + } + } +} + +func TestConstraintCheck(t *testing.T) { + cases := []struct { + constraint string + version string + check bool + }{ + {">= 1.0, < 1.2", "1.1.5", true}, + {"< 1.0, < 1.2", "1.1.5", false}, + {"= 1.0", "1.1.5", false}, + {"= 1.0", "1.0.0", true}, + {"1.0", "1.0.0", true}, + + // Pre-releases are ordered according to Semver v2 + {"> 2.0", "2.1.0-beta", true}, + {"> 2.1.0-a", "2.1.0-beta", true}, + {"> 2.1.0-a", "2.1.1-beta", true}, + {"> 2.0.0", "2.1.0-beta", true}, + {"> 2.1.0-a", "2.1.1", true}, + {"> 2.1.0-a", "2.1.1-beta", true}, + {"> 2.1.0-a", "2.1.0", true}, + {"<= 2.1.0-a", "2.0.0", true}, + {">= 0.6.1", "1.3.0-beta1", true}, + {"> 1.0-beta1", "1.0-rc1", true}, + + // Meta components are ignored according to Semver v2 + {">= 0.6.1", "1.3.0-beta1+ent", true}, + {">= 1.3.0-beta1", "1.3.0-beta1+ent", true}, + {"> 1.3.0-beta1+cgo", "1.3.0-beta1+ent", false}, + {"= 1.3.0-beta1+cgo", "1.3.0-beta1+ent", true}, + } + + for _, tc := range cases { + c, err := NewConstraint(tc.constraint) + if err != nil { + t.Fatalf("err: %s", err) + } + + v, err := version.NewSemver(tc.version) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := c.Check(v) + expected := tc.check + if actual != expected { + t.Fatalf("Version: %s\nConstraint: %s\nExpected: %#v", + tc.version, tc.constraint, expected) + } + } +} + +func TestConstraintsString(t *testing.T) { + cases := []struct { + constraint string + result string + }{ + {">= 1.0, < 1.2", ""}, + } + + for _, tc := range cases { + c, err := NewConstraint(tc.constraint) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := c.String() + expected := tc.result + if expected == "" { + expected = tc.constraint + } + + if actual != expected { + t.Fatalf("Constraint: %s\nExpected: %#v\nActual: %s", + tc.constraint, expected, actual) + } + } +} diff --git a/jobspec/parse.go b/jobspec/parse.go index 3cff938e5..7d386abd8 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -138,6 +138,7 @@ func parseConstraints(result *[]*api.Constraint, list *ast.ObjectList) error { "set_contains", "value", "version", + "semver", } if err := helper.CheckHCLKeys(o.Val, valid); err != nil { return err @@ -159,6 +160,13 @@ func parseConstraints(result *[]*api.Constraint, list *ast.ObjectList) error { m["RTarget"] = constraint } + // If "semver" is provided, set the operand + // to "semver" and the value to the "RTarget" + if constraint, ok := m[api.ConstraintSemver]; ok { + m["Operand"] = api.ConstraintSemver + m["RTarget"] = constraint + } + // If "regexp" is provided, set the operand // to "regexp" and the value to the "RTarget" if constraint, ok := m[api.ConstraintRegex]; ok { @@ -219,6 +227,7 @@ func parseAffinities(result *[]*api.Affinity, list *ast.ObjectList) error { "set_contains_all", "value", "version", + "semver", "weight", } if err := helper.CheckHCLKeys(o.Val, valid); err != nil { @@ -241,6 +250,13 @@ func parseAffinities(result *[]*api.Affinity, list *ast.ObjectList) error { m["RTarget"] = affinity } + // If "semver" is provided, set the operand + // to "semver" and the value to the "RTarget" + if affinity, ok := m[api.ConstraintSemver]; ok { + m["Operand"] = api.ConstraintSemver + m["RTarget"] = affinity + } + // If "regexp" is provided, set the operand // to "regexp" and the value to the "RTarget" if affinity, ok := m[api.ConstraintRegex]; ok { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 95a2b6cdc..407666bfd 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -43,6 +43,11 @@ func TestParse(t *testing.T) { RTarget: "windows", Operand: "=", }, + { + LTarget: "${attr.vault.version}", + RTarget: ">= 0.6.1", + Operand: "semver", + }, }, Affinities: []*api.Affinity{ diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index cbbd5d349..19b8a5964 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -16,6 +16,12 @@ job "binstore-storagelocker" { value = "windows" } + constraint { + attribute = "${attr.vault.version}" + value = ">= 0.6.1" + operator = "semver" + } + affinity { attribute = "${meta.team}" value = "mobile" diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 7f425930c..4e368386a 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -951,6 +951,12 @@ func (j *Job) Allocations(args *structs.JobSpecificRequest, return structs.ErrPermissionDenied } + // Ensure JobID is set otherwise everything works and never returns + // allocations which can hide bugs in request code. + if args.JobID == "" { + return fmt.Errorf("missing job ID") + } + // Setup the blocking query opts := blockingOptions{ queryOpts: &args.QueryOptions, diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go index 1c858398f..9a670c5d8 100644 --- a/nomad/job_endpoint_hook_connect.go +++ b/nomad/job_endpoint_hook_connect.go @@ -35,8 +35,8 @@ var ( connectVersionConstraint = func() *structs.Constraint { return &structs.Constraint{ LTarget: "${attr.consul.version}", - RTarget: ">= 1.6.0beta1", - Operand: "version", + RTarget: ">= 1.6.0-beta1", + Operand: structs.ConstraintSemver, } } ) diff --git a/nomad/job_endpoint_hooks.go b/nomad/job_endpoint_hooks.go index 3040a8449..443dee429 100644 --- a/nomad/job_endpoint_hooks.go +++ b/nomad/job_endpoint_hooks.go @@ -21,7 +21,7 @@ var ( vaultConstraint = &structs.Constraint{ LTarget: vaultConstraintLTarget, RTarget: ">= 0.6.1", - Operand: structs.ConstraintVersion, + Operand: structs.ConstraintSemver, } ) diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 5367311d3..b8370a56c 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -718,6 +718,7 @@ func TestJobEndpoint_Register_Dispatched(t *testing.T) { require.Error(err) require.Contains(err.Error(), "job can't be submitted with 'Dispatched'") } + func TestJobEndpoint_Register_EnforceIndex(t *testing.T) { t.Parallel() s1 := TestServer(t, func(c *Config) { @@ -1178,6 +1179,87 @@ func TestJobEndpoint_Register_Vault_Policies(t *testing.T) { } } +// TestJobEndpoint_Register_SemverConstraint asserts that semver ordering is +// used when evaluating semver constraints. +func TestJobEndpoint_Register_SemverConstraint(t *testing.T) { + t.Parallel() + s1 := TestServer(t, nil) + defer s1.Shutdown() + codec := rpcClient(t, s1) + testutil.WaitForLeader(t, s1.RPC) + state := s1.State() + + // Create a job with a semver constraint + job := mock.Job() + job.Constraints = []*structs.Constraint{ + { + LTarget: "${attr.vault.version}", + RTarget: ">= 0.6.1", + Operand: structs.ConstraintSemver, + }, + } + job.TaskGroups[0].Count = 1 + + // Insert 2 Nodes, 1 matching the constraint, 1 not + node1 := mock.Node() + node1.Attributes["vault.version"] = "1.3.0-beta1+ent" + node1.ComputeClass() + require.NoError(t, state.UpsertNode(1, node1)) + + node2 := mock.Node() + delete(node2.Attributes, "vault.version") + node2.ComputeClass() + require.NoError(t, state.UpsertNode(2, node2)) + + // Create the register request + req := &structs.JobRegisterRequest{ + Job: job, + WriteRequest: structs.WriteRequest{ + Region: "global", + Namespace: job.Namespace, + }, + } + + // Fetch the response + var resp structs.JobRegisterResponse + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp)) + require.NotZero(t, resp.Index) + + // Wait for placements + allocReq := &structs.JobSpecificRequest{ + JobID: job.ID, + QueryOptions: structs.QueryOptions{ + Region: "global", + Namespace: structs.DefaultNamespace, + }, + } + + testutil.WaitForResult(func() (bool, error) { + resp := structs.JobAllocationsResponse{} + err := msgpackrpc.CallWithCodec(codec, "Job.Allocations", allocReq, &resp) + if err != nil { + return false, err + } + if n := len(resp.Allocations); n != 1 { + return false, fmt.Errorf("expected 1 alloc, found %d", n) + } + alloc := resp.Allocations[0] + if alloc.NodeID != node1.ID { + return false, fmt.Errorf("expected alloc to be one node=%q but found node=%q", + node1.ID, alloc.NodeID) + } + return true, nil + }, func(waitErr error) { + evals, err := state.EvalsByJob(nil, structs.DefaultNamespace, job.ID) + require.NoError(t, err) + for i, e := range evals { + t.Logf("%d Eval: %s", i, pretty.Sprint(e)) + } + + require.NoError(t, waitErr) + }) +} + func TestJobEndpoint_Revert(t *testing.T) { t.Parallel() s1 := TestServer(t, func(c *Config) { @@ -3712,6 +3794,29 @@ func TestJobEndpoint_Allocations_Blocking(t *testing.T) { } } +// TestJobEndpoint_Allocations_NoJobID asserts not setting a JobID in the +// request returns an error. +func TestJobEndpoint_Allocations_NoJobID(t *testing.T) { + t.Parallel() + s1 := TestServer(t, nil) + defer s1.Shutdown() + codec := rpcClient(t, s1) + testutil.WaitForLeader(t, s1.RPC) + + get := &structs.JobSpecificRequest{ + JobID: "", + QueryOptions: structs.QueryOptions{ + Region: "global", + Namespace: structs.DefaultNamespace, + }, + } + + var resp structs.JobAllocationsResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Allocations", get, &resp) + require.Error(t, err) + require.Contains(t, err.Error(), "missing job ID") +} + func TestJobEndpoint_Evaluations(t *testing.T) { t.Parallel() s1 := TestServer(t, nil) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index ba806bc2c..f1d38b3a6 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -30,6 +30,7 @@ import ( "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/args" + "github.com/hashicorp/nomad/helper/constraints/semver" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/lib/kheap" psstructs "github.com/hashicorp/nomad/plugins/shared/structs" @@ -6619,6 +6620,7 @@ const ( ConstraintDistinctHosts = "distinct_hosts" ConstraintRegex = "regexp" ConstraintVersion = "version" + ConstraintSemver = "semver" ConstraintSetContains = "set_contains" ConstraintSetContainsAll = "set_contains_all" ConstraintSetContainsAny = "set_contains_any" @@ -6689,6 +6691,10 @@ func (c *Constraint) Validate() error { if _, err := version.NewConstraint(c.RTarget); err != nil { mErr.Errors = append(mErr.Errors, fmt.Errorf("Version constraint is invalid: %v", err)) } + case ConstraintSemver: + if _, err := semver.NewConstraint(c.RTarget); err != nil { + mErr.Errors = append(mErr.Errors, fmt.Errorf("Semver constraint is invalid: %v", err)) + } case ConstraintDistinctProperty: // If a count is set, make sure it is convertible to a uint64 if c.RTarget != "" { @@ -6803,6 +6809,10 @@ func (a *Affinity) Validate() error { if _, err := version.NewConstraint(a.RTarget); err != nil { mErr.Errors = append(mErr.Errors, fmt.Errorf("Version affinity is invalid: %v", err)) } + case ConstraintSemver: + if _, err := semver.NewConstraint(a.RTarget); err != nil { + mErr.Errors = append(mErr.Errors, fmt.Errorf("Semver affinity is invalid: %v", err)) + } case "=", "==", "is", "!=", "not", "<", "<=", ">", ">=": if a.RTarget == "" { mErr.Errors = append(mErr.Errors, fmt.Errorf("Operator %q requires an RTarget", a.Operand)) diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 6a1c4fc4d..808e768cc 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -1827,9 +1827,7 @@ func TestConstraint_Validate(t *testing.T) { Operand: "=", } err = c.Validate() - if err != nil { - t.Fatalf("err: %v", err) - } + require.NoError(t, err) // Perform additional regexp validation c.Operand = ConstraintRegex @@ -1849,6 +1847,15 @@ func TestConstraint_Validate(t *testing.T) { t.Fatalf("err: %s", err) } + // Perform semver validation + c.Operand = ConstraintSemver + err = c.Validate() + require.Error(t, err) + require.Contains(t, err.Error(), "Malformed constraint") + + c.RTarget = ">= 0.6.1" + require.NoError(t, c.Validate()) + // Perform distinct_property validation c.Operand = ConstraintDistinctProperty c.RTarget = "0" diff --git a/scheduler/context.go b/scheduler/context.go index e62509a82..e38ae5b0f 100644 --- a/scheduler/context.go +++ b/scheduler/context.go @@ -5,7 +5,6 @@ import ( log "github.com/hashicorp/go-hclog" memdb "github.com/hashicorp/go-memdb" - version "github.com/hashicorp/go-version" "github.com/hashicorp/nomad/nomad/structs" ) @@ -35,7 +34,10 @@ type Context interface { RegexpCache() map[string]*regexp.Regexp // VersionConstraintCache is a cache of version constraints - VersionConstraintCache() map[string]version.Constraints + VersionConstraintCache() map[string]VerConstraints + + // SemverConstraintCache is a cache of semver constraints + SemverConstraintCache() map[string]VerConstraints // Eligibility returns a tracker for node eligibility in the context of the // eval. @@ -44,8 +46,9 @@ type Context interface { // EvalCache is used to cache certain things during an evaluation type EvalCache struct { - reCache map[string]*regexp.Regexp - constraintCache map[string]version.Constraints + reCache map[string]*regexp.Regexp + versionCache map[string]VerConstraints + semverCache map[string]VerConstraints } func (e *EvalCache) RegexpCache() map[string]*regexp.Regexp { @@ -55,11 +58,18 @@ func (e *EvalCache) RegexpCache() map[string]*regexp.Regexp { return e.reCache } -func (e *EvalCache) VersionConstraintCache() map[string]version.Constraints { - if e.constraintCache == nil { - e.constraintCache = make(map[string]version.Constraints) +func (e *EvalCache) VersionConstraintCache() map[string]VerConstraints { + if e.versionCache == nil { + e.versionCache = make(map[string]VerConstraints) } - return e.constraintCache + return e.versionCache +} + +func (e *EvalCache) SemverConstraintCache() map[string]VerConstraints { + if e.semverCache == nil { + e.semverCache = make(map[string]VerConstraints) + } + return e.semverCache } // EvalContext is a Context used during an Evaluation diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 143e16d62..fbe184bb3 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -8,6 +8,7 @@ import ( "strings" version "github.com/hashicorp/go-version" + "github.com/hashicorp/nomad/helper/constraints/semver" "github.com/hashicorp/nomad/nomad/structs" psstructs "github.com/hashicorp/nomad/plugins/shared/structs" ) @@ -551,7 +552,11 @@ func checkConstraint(ctx Context, operand string, lVal, rVal interface{}, lFound case structs.ConstraintAttributeIsNotSet: return !lFound case structs.ConstraintVersion: - return lFound && rFound && checkVersionMatch(ctx, lVal, rVal) + parser := newVersionConstraintParser(ctx) + return lFound && rFound && checkVersionMatch(ctx, parser, lVal, rVal) + case structs.ConstraintSemver: + parser := newSemverConstraintParser(ctx) + return lFound && rFound && checkVersionMatch(ctx, parser, lVal, rVal) case structs.ConstraintRegex: return lFound && rFound && checkRegexpMatch(ctx, lVal, rVal) case structs.ConstraintSetContains, structs.ConstraintSetContainsAll: @@ -601,7 +606,7 @@ func checkLexicalOrder(op string, lVal, rVal interface{}) bool { // checkVersionMatch is used to compare a version on the // left hand side with a set of constraints on the right hand side -func checkVersionMatch(ctx Context, lVal, rVal interface{}) bool { +func checkVersionMatch(ctx Context, parse verConstraintParser, lVal, rVal interface{}) bool { // Parse the version var versionStr string switch v := lVal.(type) { @@ -625,17 +630,10 @@ func checkVersionMatch(ctx Context, lVal, rVal interface{}) bool { return false } - // Check the cache for a match - cache := ctx.VersionConstraintCache() - constraints := cache[constraintStr] - // Parse the constraints + constraints := parse(constraintStr) if constraints == nil { - constraints, err = version.NewConstraint(constraintStr) - if err != nil { - return false - } - cache[constraintStr] = constraints + return false } // Check the constraints against the version @@ -644,7 +642,7 @@ func checkVersionMatch(ctx Context, lVal, rVal interface{}) bool { // checkAttributeVersionMatch is used to compare a version on the // left hand side with a set of constraints on the right hand side -func checkAttributeVersionMatch(ctx Context, lVal, rVal *psstructs.Attribute) bool { +func checkAttributeVersionMatch(ctx Context, parse verConstraintParser, lVal, rVal *psstructs.Attribute) bool { // Parse the version var versionStr string if s, ok := lVal.GetString(); ok { @@ -667,17 +665,10 @@ func checkAttributeVersionMatch(ctx Context, lVal, rVal *psstructs.Attribute) bo return false } - // Check the cache for a match - cache := ctx.VersionConstraintCache() - constraints := cache[constraintStr] - // Parse the constraints + constraints := parse(constraintStr) if constraints == nil { - constraints, err = version.NewConstraint(constraintStr) - if err != nil { - return false - } - cache[constraintStr] = constraints + return false } // Check the constraints against the version @@ -1119,7 +1110,17 @@ func checkAttributeConstraint(ctx Context, operand string, lVal, rVal *psstructs return false } - return checkAttributeVersionMatch(ctx, lVal, rVal) + parser := newVersionConstraintParser(ctx) + return checkAttributeVersionMatch(ctx, parser, lVal, rVal) + + case structs.ConstraintSemver: + if !(lFound && rFound) { + return false + } + + parser := newSemverConstraintParser(ctx) + return checkAttributeVersionMatch(ctx, parser, lVal, rVal) + case structs.ConstraintRegex: if !(lFound && rFound) { return false @@ -1164,3 +1165,50 @@ func checkAttributeConstraint(ctx Context, operand string, lVal, rVal *psstructs } } + +// VerConstraints is the interface implemented by both go-verson constraints +// and semver constraints. +type VerConstraints interface { + Check(v *version.Version) bool + String() string +} + +// verConstraintParser returns a version constraints implementation (go-version +// or semver). +type verConstraintParser func(verConstraint string) VerConstraints + +func newVersionConstraintParser(ctx Context) verConstraintParser { + cache := ctx.VersionConstraintCache() + + return func(cstr string) VerConstraints { + if c := cache[cstr]; c != nil { + return c + } + + constraints, err := version.NewConstraint(cstr) + if err != nil { + return nil + } + cache[cstr] = constraints + + return constraints + } +} + +func newSemverConstraintParser(ctx Context) verConstraintParser { + cache := ctx.SemverConstraintCache() + + return func(cstr string) VerConstraints { + if c := cache[cstr]; c != nil { + return c + } + + constraints, err := semver.NewConstraint(cstr) + if err != nil { + return nil + } + cache[cstr] = constraints + + return constraints + } +} diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index cc6ca022b..62b39acf8 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -708,6 +708,8 @@ func TestCheckLexicalOrder(t *testing.T) { } func TestCheckVersionConstraint(t *testing.T) { + t.Parallel() + type tcase struct { lVal, rVal interface{} result bool @@ -733,15 +735,93 @@ func TestCheckVersionConstraint(t *testing.T) { lVal: 1, rVal: "~> 1.0", result: true, }, + { + // Prereleases are never > final releases + lVal: "1.3.0-beta1", rVal: ">= 0.6.1", + result: false, + }, + { + // Prerelease X.Y.Z must match + lVal: "1.7.0-alpha1", rVal: ">= 1.6.0-beta1", + result: false, + }, + { + // Meta is ignored + lVal: "1.3.0-beta1+ent", rVal: "= 1.3.0-beta1", + result: true, + }, } for _, tc := range cases { _, ctx := testContext(t) - if res := checkVersionMatch(ctx, tc.lVal, tc.rVal); res != tc.result { + p := newVersionConstraintParser(ctx) + if res := checkVersionMatch(ctx, p, tc.lVal, tc.rVal); res != tc.result { t.Fatalf("TC: %#v, Result: %v", tc, res) } } } +func TestCheckSemverConstraint(t *testing.T) { + t.Parallel() + + type tcase struct { + name string + lVal, rVal interface{} + result bool + } + cases := []tcase{ + { + name: "Pessimistic operator always fails 1", + lVal: "1.2.3", rVal: "~> 1.0", + result: false, + }, + { + name: "1.2.3 does satisfy >= 1.0, < 1.4", + lVal: "1.2.3", rVal: ">= 1.0, < 1.4", + result: true, + }, + { + name: "Pessimistic operator always fails 2", + lVal: "2.0.1", rVal: "~> 1.0", + result: false, + }, + { + name: "1.4 does not satisfy >= 1.0, < 1.4", + lVal: "1.4", rVal: ">= 1.0, < 1.4", + result: false, + }, + { + name: "Pessimistic operator always fails 3", + lVal: 1, rVal: "~> 1.0", + result: false, + }, + { + name: "Prereleases are handled according to semver 1", + lVal: "1.3.0-beta1", rVal: ">= 0.6.1", + result: true, + }, + { + name: "Prereleases are handled according to semver 2", + lVal: "1.7.0-alpha1", rVal: ">= 1.6.0-beta1", + result: true, + }, + { + name: "Meta is ignored according to semver", + lVal: "1.3.0-beta1+ent", rVal: "= 1.3.0-beta1", + result: true, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + _, ctx := testContext(t) + p := newSemverConstraintParser(ctx) + actual := checkVersionMatch(ctx, p, tc.lVal, tc.rVal) + require.Equal(t, tc.result, actual) + }) + } +} + func TestCheckRegexpConstraint(t *testing.T) { type tcase struct { lVal, rVal interface{}