diff --git a/CHANGELOG.md b/CHANGELOG.md index a4e8c605580a..7fa7539f21c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ __BACKWARDS INCOMPATIBILITIES:__ * core: Switch to structured logging using [go-hclog](https://github.com/hashicorp/go-hclog) + * core: Allow the != constraint to match against keys that do not exist [[GH-4875](https://github.com/hashicorp/nomad/pull/4875)] IMPROVEMENTS: * core: Added advertise address to client node meta data [[GH-4390](https://github.com/hashicorp/nomad/issues/4390)] diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 3ee54247569d..bf3747352374 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -6324,13 +6324,15 @@ func (ta *TaskArtifact) validateChecksum() error { } const ( - ConstraintDistinctProperty = "distinct_property" - ConstraintDistinctHosts = "distinct_hosts" - ConstraintRegex = "regexp" - ConstraintVersion = "version" - ConstraintSetContains = "set_contains" - ConstraintSetContainsAll = "set_contains_all" - ConstraintSetContainsAny = "set_contains_any" + ConstraintDistinctProperty = "distinct_property" + ConstraintDistinctHosts = "distinct_hosts" + ConstraintRegex = "regexp" + ConstraintVersion = "version" + ConstraintSetContains = "set_contains" + ConstraintSetContainsAll = "set_contains_all" + ConstraintSetContainsAny = "set_contains_any" + ConstraintAttributeIsSet = "is_set" + ConstraintAttributeIsNotSet = "is_not_set" ) // Constraints are used to restrict placement options. @@ -6401,6 +6403,10 @@ func (c *Constraint) Validate() error { mErr.Errors = append(mErr.Errors, fmt.Errorf("Distinct Property must have an allowed count of 1 or greater: %d < 1", count)) } } + case ConstraintAttributeIsSet, ConstraintAttributeIsNotSet: + if c.RTarget != "" { + mErr.Errors = append(mErr.Errors, fmt.Errorf("Operator %q does not support an RTarget", c.Operand)) + } case "=", "==", "is", "!=", "not", "<", "<=", ">", ">=": if c.RTarget == "" { mErr.Errors = append(mErr.Errors, fmt.Errorf("Operator %q requires an RTarget", c.Operand)) diff --git a/scheduler/device.go b/scheduler/device.go index 8fbc79c63a55..1970647163da 100644 --- a/scheduler/device.go +++ b/scheduler/device.go @@ -74,19 +74,13 @@ func (d *deviceAllocator) AssignDevice(ask *structs.RequestedDevice) (out *struc totalWeight := 0.0 for _, a := range ask.Affinities { // Resolve the targets - lVal, ok := resolveDeviceTarget(a.LTarget, devInst.Device) - if !ok { - continue - } - rVal, ok := resolveDeviceTarget(a.RTarget, devInst.Device) - if !ok { - continue - } + lVal, lOk := resolveDeviceTarget(a.LTarget, devInst.Device) + rVal, rOk := resolveDeviceTarget(a.RTarget, devInst.Device) totalWeight += math.Abs(a.Weight) // Check if satisfied - if !checkAttributeAffinity(d.ctx, a.Operand, lVal, rVal) { + if !checkAttributeAffinity(d.ctx, a.Operand, lVal, rVal, lOk, rOk) { continue } choiceScore += a.Weight diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 4db5ae1276e1..ce0f6d1ca970 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -398,21 +398,16 @@ func (c *ConstraintChecker) Feasible(option *structs.Node) bool { } func (c *ConstraintChecker) meetsConstraint(constraint *structs.Constraint, option *structs.Node) bool { - // Resolve the targets - lVal, ok := resolveTarget(constraint.LTarget, option) - if !ok { - return false - } - rVal, ok := resolveTarget(constraint.RTarget, option) - if !ok { - return false - } + // Resolve the targets. Targets that are not present are treated as `nil`. + // This is to allow for matching constraints where a target is not present. + lVal, lOk := resolveTarget(constraint.LTarget, option) + rVal, rOk := resolveTarget(constraint.RTarget, option) // Check if satisfied - return checkConstraint(c.ctx, constraint.Operand, lVal, rVal) + return checkConstraint(c.ctx, constraint.Operand, lVal, rVal, lOk, rOk) } -// resolveTarget is used to resolve the LTarget and RTarget of a Constraint +// resolveTarget is used to resolve the LTarget and RTarget of a Constraint. func resolveTarget(target string, node *structs.Node) (interface{}, bool) { // If no prefix, this must be a literal value if !strings.HasPrefix(target, "${") { @@ -448,8 +443,9 @@ func resolveTarget(target string, node *structs.Node) (interface{}, bool) { } } -// checkConstraint checks if a constraint is satisfied -func checkConstraint(ctx Context, operand string, lVal, rVal interface{}) bool { +// checkConstraint checks if a constraint is satisfied. The lVal and rVal +// interfaces may be nil. +func checkConstraint(ctx Context, operand string, lVal, rVal interface{}, lFound, rFound bool) bool { // Check for constraints not handled by this checker. switch operand { case structs.ConstraintDistinctHosts, structs.ConstraintDistinctProperty: @@ -460,32 +456,36 @@ func checkConstraint(ctx Context, operand string, lVal, rVal interface{}) bool { switch operand { case "=", "==", "is": - return reflect.DeepEqual(lVal, rVal) + return lFound && rFound && reflect.DeepEqual(lVal, rVal) case "!=", "not": return !reflect.DeepEqual(lVal, rVal) case "<", "<=", ">", ">=": - return checkLexicalOrder(operand, lVal, rVal) + return lFound && rFound && checkLexicalOrder(operand, lVal, rVal) + case structs.ConstraintAttributeIsSet: + return lFound + case structs.ConstraintAttributeIsNotSet: + return !lFound case structs.ConstraintVersion: - return checkVersionMatch(ctx, lVal, rVal) + return lFound && rFound && checkVersionMatch(ctx, lVal, rVal) case structs.ConstraintRegex: - return checkRegexpMatch(ctx, lVal, rVal) + return lFound && rFound && checkRegexpMatch(ctx, lVal, rVal) case structs.ConstraintSetContains, structs.ConstraintSetContainsAll: - return checkSetContainsAll(ctx, lVal, rVal) + return lFound && rFound && checkSetContainsAll(ctx, lVal, rVal) case structs.ConstraintSetContainsAny: - return checkSetContainsAny(lVal, rVal) + return lFound && rFound && checkSetContainsAny(lVal, rVal) default: return false } } // checkAffinity checks if a specific affinity is satisfied -func checkAffinity(ctx Context, operand string, lVal, rVal interface{}) bool { - return checkConstraint(ctx, operand, lVal, rVal) +func checkAffinity(ctx Context, operand string, lVal, rVal interface{}, lFound, rFound bool) bool { + return checkConstraint(ctx, operand, lVal, rVal, lFound, rFound) } // checkAttributeAffinity checks if an affinity is satisfied -func checkAttributeAffinity(ctx Context, operand string, lVal, rVal *psstructs.Attribute) bool { - return checkAttributeConstraint(ctx, operand, lVal, rVal) +func checkAttributeAffinity(ctx Context, operand string, lVal, rVal *psstructs.Attribute, lFound, rFound bool) bool { + return checkAttributeConstraint(ctx, operand, lVal, rVal, lFound, rFound) } // checkLexicalOrder is used to check for lexical ordering @@ -931,17 +931,11 @@ func nodeDeviceMatches(ctx Context, d *structs.NodeDeviceResource, req *structs. for _, c := range req.Constraints { // Resolve the targets - lVal, ok := resolveDeviceTarget(c.LTarget, d) - if !ok { - return false - } - rVal, ok := resolveDeviceTarget(c.RTarget, d) - if !ok { - return false - } + lVal, lOk := resolveDeviceTarget(c.LTarget, d) + rVal, rOk := resolveDeviceTarget(c.RTarget, d) // Check if satisfied - if !checkAttributeConstraint(ctx, c.Operand, lVal, rVal) { + if !checkAttributeConstraint(ctx, c.Operand, lVal, rVal, lOk, rOk) { return false } } @@ -979,8 +973,9 @@ func resolveDeviceTarget(target string, d *structs.NodeDeviceResource) (*psstruc } } -// checkAttributeConstraint checks if a constraint is satisfied -func checkAttributeConstraint(ctx Context, operand string, lVal, rVal *psstructs.Attribute) bool { +// checkAttributeConstraint checks if a constraint is satisfied. nil equality +// comparisons are considered to be false. +func checkAttributeConstraint(ctx Context, operand string, lVal, rVal *psstructs.Attribute, lFound, rFound bool) bool { // Check for constraints not handled by this checker. switch operand { case structs.ConstraintDistinctHosts, structs.ConstraintDistinctProperty: @@ -990,15 +985,36 @@ func checkAttributeConstraint(ctx Context, operand string, lVal, rVal *psstructs } switch operand { - case "<", "<=", ">", ">=", "=", "==", "is", "!=", "not": + case "!=", "not": + // Neither value was provided, nil != nil == false + if !(lFound || rFound) { + return false + } + + // Only 1 value was provided, therefore nil != some == true + if lFound != rFound { + return true + } + + // Both values were provided, so actually compare them + v, ok := lVal.Compare(rVal) + if !ok { + return false + } + + return v != 0 + + case "<", "<=", ">", ">=", "=", "==", "is": + if !(lFound && rFound) { + return false + } + v, ok := lVal.Compare(rVal) if !ok { return false } switch operand { - case "not", "!=": - return v != 0 case "is", "==", "=": return v == 0 case "<": @@ -1014,8 +1030,16 @@ func checkAttributeConstraint(ctx Context, operand string, lVal, rVal *psstructs } case structs.ConstraintVersion: + if !(lFound && rFound) { + return false + } + return checkAttributeVersionMatch(ctx, lVal, rVal) case structs.ConstraintRegex: + if !(lFound && rFound) { + return false + } + ls, ok := lVal.GetString() rs, ok2 := rVal.GetString() if !ok || !ok2 { @@ -1023,6 +1047,10 @@ func checkAttributeConstraint(ctx Context, operand string, lVal, rVal *psstructs } return checkRegexpMatch(ctx, ls, rs) case structs.ConstraintSetContains, structs.ConstraintSetContainsAll: + if !(lFound && rFound) { + return false + } + ls, ok := lVal.GetString() rs, ok2 := rVal.GetString() if !ok || !ok2 { @@ -1031,6 +1059,10 @@ func checkAttributeConstraint(ctx Context, operand string, lVal, rVal *psstructs return checkSetContainsAll(ctx, ls, rs) case structs.ConstraintSetContainsAny: + if !(lFound && rFound) { + return false + } + ls, ok := lVal.GetString() rs, ok2 := rVal.GetString() if !ok || !ok2 { @@ -1038,6 +1070,10 @@ func checkAttributeConstraint(ctx Context, operand string, lVal, rVal *psstructs } return checkSetContainsAny(ls, rs) + case structs.ConstraintAttributeIsSet: + return lFound + case structs.ConstraintAttributeIsNotSet: + return !lFound default: return false } diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index abbdcb484d08..98a2f6b5e6db 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -197,12 +197,12 @@ func TestConstraintChecker(t *testing.T) { mock.Node(), mock.Node(), mock.Node(), - mock.Node(), } nodes[0].Attributes["kernel.name"] = "freebsd" nodes[1].Datacenter = "dc2" nodes[2].NodeClass = "large" + nodes[2].Attributes["foo"] = "bar" constraints := []*structs.Constraint{ { @@ -216,9 +216,13 @@ func TestConstraintChecker(t *testing.T) { RTarget: "linux", }, { - Operand: "is", + Operand: "!=", LTarget: "${node.class}", - RTarget: "large", + RTarget: "linux-medium-pci", + }, + { + Operand: "is_set", + LTarget: "${attr.foo}", }, } checker := NewConstraintChecker(ctx, constraints) @@ -294,6 +298,7 @@ func TestResolveConstraintTarget(t *testing.T) { { target: "${attr.rand}", node: node, + val: "", result: false, }, { @@ -305,6 +310,7 @@ func TestResolveConstraintTarget(t *testing.T) { { target: "${meta.rand}", node: node, + val: "", result: false, }, } @@ -342,6 +348,21 @@ func TestCheckConstraint(t *testing.T) { lVal: "foo", rVal: "foo", result: true, }, + { + op: "==", + lVal: "foo", rVal: nil, + result: false, + }, + { + op: "==", + lVal: nil, rVal: "foo", + result: false, + }, + { + op: "==", + lVal: nil, rVal: nil, + result: false, + }, { op: "!=", lVal: "foo", rVal: "foo", @@ -352,6 +373,21 @@ func TestCheckConstraint(t *testing.T) { lVal: "foo", rVal: "bar", result: true, }, + { + op: "!=", + lVal: nil, rVal: "foo", + result: true, + }, + { + op: "!=", + lVal: "foo", rVal: nil, + result: true, + }, + { + op: "!=", + lVal: nil, rVal: nil, + result: false, + }, { op: "not", lVal: "foo", rVal: "bar", @@ -362,16 +398,31 @@ func TestCheckConstraint(t *testing.T) { lVal: "1.2.3", rVal: "~> 1.0", result: true, }, + { + op: structs.ConstraintVersion, + lVal: nil, rVal: "~> 1.0", + result: false, + }, { op: structs.ConstraintRegex, lVal: "foobarbaz", rVal: "[\\w]+", result: true, }, + { + op: structs.ConstraintRegex, + lVal: nil, rVal: "[\\w]+", + result: false, + }, { op: "<", lVal: "foo", rVal: "bar", result: false, }, + { + op: "<", + lVal: nil, rVal: "bar", + result: false, + }, { op: structs.ConstraintSetContains, lVal: "foo,bar,baz", rVal: "foo, bar ", @@ -382,11 +433,31 @@ func TestCheckConstraint(t *testing.T) { lVal: "foo,bar,baz", rVal: "foo,bam", result: false, }, + { + op: structs.ConstraintAttributeIsSet, + lVal: "foo", + result: true, + }, + { + op: structs.ConstraintAttributeIsSet, + lVal: nil, + result: false, + }, + { + op: structs.ConstraintAttributeIsNotSet, + lVal: nil, + result: true, + }, + { + op: structs.ConstraintAttributeIsNotSet, + lVal: "foo", + result: false, + }, } for _, tc := range cases { _, ctx := testContext(t) - if res := checkConstraint(ctx, tc.op, tc.lVal, tc.rVal); res != tc.result { + if res := checkConstraint(ctx, tc.op, tc.lVal, tc.rVal, tc.lVal != nil, tc.rVal != nil); res != tc.result { t.Fatalf("TC: %#v, Result: %v", tc, res) } } @@ -1972,6 +2043,12 @@ func TestCheckAttributeConstraint(t *testing.T) { rVal: psstructs.NewStringAttribute("foo"), result: true, }, + { + op: "=", + lVal: nil, + rVal: nil, + result: false, + }, { op: "is", lVal: psstructs.NewStringAttribute("foo"), @@ -1990,6 +2067,18 @@ func TestCheckAttributeConstraint(t *testing.T) { rVal: psstructs.NewStringAttribute("foo"), result: false, }, + { + op: "!=", + lVal: nil, + rVal: psstructs.NewStringAttribute("foo"), + result: true, + }, + { + op: "!=", + lVal: psstructs.NewStringAttribute("foo"), + rVal: nil, + result: true, + }, { op: "!=", lVal: psstructs.NewStringAttribute("foo"), @@ -2044,11 +2133,31 @@ func TestCheckAttributeConstraint(t *testing.T) { rVal: psstructs.NewStringAttribute("foo,bam"), result: true, }, + { + op: structs.ConstraintAttributeIsSet, + lVal: psstructs.NewStringAttribute("foo,bar,baz"), + result: true, + }, + { + op: structs.ConstraintAttributeIsSet, + lVal: nil, + result: false, + }, + { + op: structs.ConstraintAttributeIsNotSet, + lVal: psstructs.NewStringAttribute("foo,bar,baz"), + result: false, + }, + { + op: structs.ConstraintAttributeIsNotSet, + lVal: nil, + result: true, + }, } for _, tc := range cases { _, ctx := testContext(t) - if res := checkAttributeConstraint(ctx, tc.op, tc.lVal, tc.rVal); res != tc.result { + if res := checkAttributeConstraint(ctx, tc.op, tc.lVal, tc.rVal, tc.lVal != nil, tc.rVal != nil); res != tc.result { t.Fatalf("TC: %#v, Result: %v", tc, res) } } diff --git a/scheduler/rank.go b/scheduler/rank.go index 7d2b4ad4e931..1ffd028e8dcf 100644 --- a/scheduler/rank.go +++ b/scheduler/rank.go @@ -564,17 +564,11 @@ func (iter *NodeAffinityIterator) Next() *RankedNode { func matchesAffinity(ctx Context, affinity *structs.Affinity, option *structs.Node) bool { //TODO(preetha): Add a step here that filters based on computed node class for potential speedup // Resolve the targets - lVal, ok := resolveTarget(affinity.LTarget, option) - if !ok { - return false - } - rVal, ok := resolveTarget(affinity.RTarget, option) - if !ok { - return false - } + lVal, lOk := resolveTarget(affinity.LTarget, option) + rVal, rOk := resolveTarget(affinity.RTarget, option) // Check if satisfied - return checkAffinity(ctx, affinity.Operand, lVal, rVal) + return checkAffinity(ctx, affinity.Operand, lVal, rVal, lOk, rOk) } // ScoreNormalizationIterator is used to combine scores from various prior diff --git a/website/source/docs/job-specification/constraint.html.md b/website/source/docs/job-specification/constraint.html.md index 6f355799dbc8..0425cff3a3ca 100644 --- a/website/source/docs/job-specification/constraint.html.md +++ b/website/source/docs/job-specification/constraint.html.md @@ -85,6 +85,8 @@ all groups (and tasks) in the job. regexp set_contains version + is_set + is_not_set ``` For a detailed explanation of these values and their behavior, please see @@ -196,6 +198,13 @@ constraint { } ``` +- `"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 + nodes that don't have that attribute set. + +- `"is_not_set"` - Specifies that a given attribute must not be present. + ## `constraint` Examples The following examples only show the `constraint` stanzas. Remember that the