From 1d2fa5ac31f6fc6481d1ce5ebe3f98d683910f2c Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Wed, 13 Nov 2019 15:42:10 -0800 Subject: [PATCH 1/4] vendor: update go-version to include NewSemver --- .../github.com/hashicorp/go-version/README.md | 2 +- vendor/github.com/hashicorp/go-version/go.mod | 1 + .../hashicorp/go-version/version.go | 47 ++++++++++++++++--- vendor/vendor.json | 2 +- 4 files changed, 43 insertions(+), 9 deletions(-) create mode 100644 vendor/github.com/hashicorp/go-version/go.mod diff --git a/vendor/github.com/hashicorp/go-version/README.md b/vendor/github.com/hashicorp/go-version/README.md index 6f3a15ce772..2ee50c503c1 100644 --- a/vendor/github.com/hashicorp/go-version/README.md +++ b/vendor/github.com/hashicorp/go-version/README.md @@ -1,5 +1,5 @@ # Versioning Library for Go -[![Build Status](https://travis-ci.org/hashicorp/go-version.svg?branch=master)](https://travis-ci.org/hashicorp/go-version) +[![Build Status](https://circleci.com/gh/hashicorp/go-version/tree/master.svg?style=svg)](https://circleci.com/gh/hashicorp/go-version/tree/master) go-version is a library for parsing versions and version constraints, and verifying versions against a set of constraints. go-version diff --git a/vendor/github.com/hashicorp/go-version/go.mod b/vendor/github.com/hashicorp/go-version/go.mod new file mode 100644 index 00000000000..f5285555fa8 --- /dev/null +++ b/vendor/github.com/hashicorp/go-version/go.mod @@ -0,0 +1 @@ +module github.com/hashicorp/go-version diff --git a/vendor/github.com/hashicorp/go-version/version.go b/vendor/github.com/hashicorp/go-version/version.go index 4d1e6e2210c..30454e1b435 100644 --- a/vendor/github.com/hashicorp/go-version/version.go +++ b/vendor/github.com/hashicorp/go-version/version.go @@ -10,14 +10,25 @@ import ( ) // The compiled regular expression used to test the validity of a version. -var versionRegexp *regexp.Regexp +var ( + versionRegexp *regexp.Regexp + semverRegexp *regexp.Regexp +) // The raw regular expression string used for testing the validity // of a version. -const VersionRegexpRaw string = `v?([0-9]+(\.[0-9]+)*?)` + - `(-([0-9]+[0-9A-Za-z\-~]*(\.[0-9A-Za-z\-~]+)*)|(-?([A-Za-z\-~]+[0-9A-Za-z\-~]*(\.[0-9A-Za-z\-~]+)*)))?` + - `(\+([0-9A-Za-z\-~]+(\.[0-9A-Za-z\-~]+)*))?` + - `?` +const ( + VersionRegexpRaw string = `v?([0-9]+(\.[0-9]+)*?)` + + `(-([0-9]+[0-9A-Za-z\-~]*(\.[0-9A-Za-z\-~]+)*)|(-?([A-Za-z\-~]+[0-9A-Za-z\-~]*(\.[0-9A-Za-z\-~]+)*)))?` + + `(\+([0-9A-Za-z\-~]+(\.[0-9A-Za-z\-~]+)*))?` + + `?` + + // SemverRegexpRaw requires a separator between version and prerelease + SemverRegexpRaw string = `v?([0-9]+(\.[0-9]+)*?)` + + `(-([0-9]+[0-9A-Za-z\-~]*(\.[0-9A-Za-z\-~]+)*)|(-([A-Za-z\-~]+[0-9A-Za-z\-~]*(\.[0-9A-Za-z\-~]+)*)))?` + + `(\+([0-9A-Za-z\-~]+(\.[0-9A-Za-z\-~]+)*))?` + + `?` +) // Version represents a single version. type Version struct { @@ -30,12 +41,24 @@ type Version struct { func init() { versionRegexp = regexp.MustCompile("^" + VersionRegexpRaw + "$") + semverRegexp = regexp.MustCompile("^" + SemverRegexpRaw + "$") } // NewVersion parses the given version and returns a new // Version. func NewVersion(v string) (*Version, error) { - matches := versionRegexp.FindStringSubmatch(v) + return newVersion(v, versionRegexp) +} + +// NewSemver parses the given version and returns a new +// Version that adheres strictly to SemVer specs +// https://semver.org/ +func NewSemver(v string) (*Version, error) { + return newVersion(v, semverRegexp) +} + +func newVersion(v string, pattern *regexp.Regexp) (*Version, error) { + matches := pattern.FindStringSubmatch(v) if matches == nil { return nil, fmt.Errorf("Malformed version: %s", v) } @@ -89,7 +112,7 @@ func Must(v *Version, err error) *Version { // or larger than the other version, respectively. // // If you want boolean results, use the LessThan, Equal, -// or GreaterThan methods. +// GreaterThan, GreaterThanOrEqual or LessThanOrEqual methods. func (v *Version) Compare(other *Version) int { // A quick, efficient equality check if v.String() == other.String() { @@ -265,11 +288,21 @@ func (v *Version) GreaterThan(o *Version) bool { return v.Compare(o) > 0 } +// GreaterThanOrEqual tests if this version is greater than or equal to another version. +func (v *Version) GreaterThanOrEqual(o *Version) bool { + return v.Compare(o) >= 0 +} + // LessThan tests if this version is less than another version. func (v *Version) LessThan(o *Version) bool { return v.Compare(o) < 0 } +// LessThanOrEqual tests if this version is less than or equal to another version. +func (v *Version) LessThanOrEqual(o *Version) bool { + return v.Compare(o) <= 0 +} + // Metadata returns any metadata that was part of the version // string. // diff --git a/vendor/vendor.json b/vendor/vendor.json index 75b74730147..12cb96d328c 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -225,7 +225,7 @@ {"path":"github.com/hashicorp/go-sockaddr/template","checksumSHA1":"PDp9DVLvf3KWxhs4G4DpIwauMSU=","revision":"6d291a969b86c4b633730bfc6b8b9d64c3aafed9","revisionTime":"2018-03-20T11:50:54Z"}, {"path":"github.com/hashicorp/go-syslog","checksumSHA1":"eEqQGKsFOdSAwEprYgYnL7+J2e0=","revision":"8d1874e3e8d1862b74e0536851e218c4571066a5","revisionTime":"2019-01-18T15:19:36Z","version":"v1.0.0","versionExact":"v1.0.0"}, {"path":"github.com/hashicorp/go-uuid","checksumSHA1":"5AxXPtBqAKyFGcttFzxT5hp/3Tk=","revision":"4f571afc59f3043a65f8fe6bf46d887b10a01d43","revisionTime":"2018-11-28T13:14:45Z"}, - {"path":"github.com/hashicorp/go-version","checksumSHA1":"r0pj5dMHCghpaQZ3f1BRGoKiSWw=","revision":"b5a281d3160aa11950a6182bd9a9dc2cb1e02d50","revisionTime":"2018-08-24T00:43:55Z"}, + {"path":"github.com/hashicorp/go-version","checksumSHA1":"+wX9Wh8280cHZMckQlOit3UrwbA=","revision":"2046c9d0f0b03c779670f5186a2a4b2c85493a71","revisionTime":"2019-10-09T19:36:37Z"}, {"path":"github.com/hashicorp/golang-lru","checksumSHA1":"d9PxF1XQGLMJZRct2R8qVM/eYlE=","revision":"a0d98a5f288019575c6d1f4bb1573fef2d1fcdc4","revisionTime":"2016-02-07T21:47:19Z"}, {"path":"github.com/hashicorp/golang-lru/simplelru","checksumSHA1":"2nOpYjx8Sn57bqlZq17yM4YJuM4=","revision":"a0d98a5f288019575c6d1f4bb1573fef2d1fcdc4"}, {"path":"github.com/hashicorp/hcl","checksumSHA1":"vgGv8zuy7q8c5LBAFO1fnnQRRgE=","revision":"1804807358d86424faacbb42f50f0c04303cef11","revisionTime":"2019-06-10T16:16:27Z"}, From 75d6d4ec5e13e8847751286195228b031bfb8c33 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Wed, 13 Nov 2019 15:36:15 -0800 Subject: [PATCH 2/4] 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 7b18ade9ffa..3233a3bfdcd 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 00000000000..0a100623d32 --- /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 00000000000..0759414ae10 --- /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 3cff938e5e2..7d386abd8a6 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 95a2b6cdc2d..407666bfd4a 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 cbbd5d34939..19b8a596487 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 7f425930c9d..4e368386a08 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 1c858398f92..9a670c5d8a8 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 3040a8449cc..443dee42956 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 5367311d324..b8370a56c5b 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 ba806bc2cea..f1d38b3a67f 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 6a1c4fc4d3b..808e768cc1b 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 e62509a82dd..e38ae5b0f31 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.versionCache +} + +func (e *EvalCache) SemverConstraintCache() map[string]VerConstraints { + if e.semverCache == nil { + e.semverCache = make(map[string]VerConstraints) } - return e.constraintCache + return e.semverCache } // EvalContext is a Context used during an Evaluation diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 143e16d62ef..fbe184bb3b5 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 cc6ca022b6e..62b39acf89a 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{} From 88a9877a071574240d81c25060aaf83aeb604531 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Tue, 19 Nov 2019 10:26:25 -0800 Subject: [PATCH 3/4] docs: document semver constraint operator --- CHANGELOG.md | 1 + .../docs/job-specification/constraint.html.md | 21 ++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b15f713c1ec..497a99f0a1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ IMPROVEMENTS: BUG FIXES: * core: Ignore `server` config values if `server` is disabled [[GH-6047](https://github.com/hashicorp/nomad/issues/6047)] + * core: Added `semver` constraint for strict Semver 2.0 version comparisons [[GH-6699](https://github.com/hashicorp/nomad/issues/6699)] * api: Return a 404 if endpoint not found instead of redirecting to /ui/ [[GH-6658](https://github.com/hashicorp/nomad/issues/6658)] * api: Decompress web socket response body if gzipped on error responses [[GH-6650](https://github.com/hashicorp/nomad/issues/6650)] * api: Fixed a bug where some FS/Allocation API endpoints didn't return error messages [[GH-6427](https://github.com/hashicorp/nomad/issues/6427)] diff --git a/website/source/docs/job-specification/constraint.html.md b/website/source/docs/job-specification/constraint.html.md index 52c364400e3..db3ea4eff14 100644 --- a/website/source/docs/job-specification/constraint.html.md +++ b/website/source/docs/job-specification/constraint.html.md @@ -85,6 +85,7 @@ all groups (and tasks) in the job. regexp set_contains version + semver is_set is_not_set ``` @@ -186,7 +187,10 @@ constraint { - `"version"` - Specifies a version constraint against the attribute. This supports a comma-separated list of constraints, including the pessimistic - operator. For more examples please see the [go-version + operator. `version` will not consider a prerelease (eg `1.6.0-beta`) + sufficient to match a non-prerelease constraint (eg `>= 1.0`). Use the + `semver` constraint for strict [Semantic Versioning 2.0][semver2] ordering. + For more examples please see the [go-version repository](https://github.com/hashicorp/go-version) for more specific examples. @@ -198,6 +202,20 @@ constraint { } ``` +- `"semver"` - Specifies a version constraint against the attribute. Only + [Semantic Versioning 2.0][semver2] compliant versions and comparison + operators are supported, so there is no pessimistic operator. Unlike `version`, + this operator considers prereleases (eg `1.6.0-beta`) sufficient to satisfy + non-prerelease constraints (eg `>= 1.0`). _Added in Nomad v0.10.2._ + + ```hcl + constraint { + attribute = "..." + operator = "semver" + value = ">= 0.1.0, < 0.2" + } + ``` + - `"is_set"` - Specifies that a given attribute must be present. This can be combined with the `"!="` operator to require that an attribute has been set before checking for equality. The default behavior for `"!="` is to include @@ -289,3 +307,4 @@ constraint { [interpolation]: /docs/runtime/interpolation.html "Nomad interpolation" [node-variables]: /docs/runtime/interpolation.html#node-variables- "Nomad interpolation-Node variables" [client-meta]: /docs/configuration/client.html#custom-metadata-network-speed-and-node-class "Nomad Custom Metadata, Network Speed, and Node Class" +[semver2]: https://semver.org/spec/v2.0.0.html "Semantic Versioning 2.0" From ecd4ed1bdddf64b1a62d4fac9b989698025f14d9 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Tue, 19 Nov 2019 10:59:40 -0800 Subject: [PATCH 4/4] test: assert semvers are *not* compared lexically --- helper/constraints/semver/constraints_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/helper/constraints/semver/constraints_test.go b/helper/constraints/semver/constraints_test.go index 0759414ae10..5c50d49d603 100644 --- a/helper/constraints/semver/constraints_test.go +++ b/helper/constraints/semver/constraints_test.go @@ -58,6 +58,9 @@ func TestConstraintCheck(t *testing.T) { {"= 1.0", "1.0.0", true}, {"1.0", "1.0.0", true}, + // Assert numbers are *not* compared lexically as in #4729 + {"> 10", "8", false}, + // Pre-releases are ordered according to Semver v2 {"> 2.0", "2.1.0-beta", true}, {"> 2.1.0-a", "2.1.0-beta", true},