From ef82a333296b4f7b9170fb39b2d07b38c2862941 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Sat, 13 Oct 2018 18:38:08 -0700 Subject: [PATCH] Check constraints on devices --- plugins/shared/structs/attribute.go | 252 ++++++++++++++--------- plugins/shared/structs/attribute_test.go | 6 + scheduler/feasible.go | 159 +++++++++++++- scheduler/feasible_test.go | 188 +++++++++++++++++ 4 files changed, 504 insertions(+), 101 deletions(-) diff --git a/plugins/shared/structs/attribute.go b/plugins/shared/structs/attribute.go index 1d88db4ec..4db080fa4 100644 --- a/plugins/shared/structs/attribute.go +++ b/plugins/shared/structs/attribute.go @@ -53,6 +53,53 @@ func (u *Unit) Comparable(o *Unit) bool { return u.Base == o.Base } +// ParseAttribute takes a string and parses it into an attribute, pulling out +// units if they are specified as a suffix on a number. +func ParseAttribute(input string) *Attribute { + ll := len(input) + if ll == 0 { + return &Attribute{String: helper.StringToPtr(input)} + } + + // Check if the string is a number ending with potential units + var unit string + numeric := input + if unicode.IsLetter(rune(input[ll-1])) { + // Try suffix matching + for _, u := range lengthSortedUnits { + if strings.HasSuffix(input, u) { + unit = u + break + } + } + + // Check if we know about the unit. + if len(unit) != 0 { + numeric = strings.TrimSpace(strings.TrimSuffix(input, unit)) + } + } + + // Try to parse as an int + i, err := strconv.ParseInt(numeric, 10, 64) + if err == nil { + return &Attribute{Int: helper.Int64ToPtr(i), Unit: unit} + } + + // Try to parse as a float + f, err := strconv.ParseFloat(numeric, 64) + if err == nil { + return &Attribute{Float: helper.Float64ToPtr(f), Unit: unit} + } + + // Try to parse as a bool + b, err := strconv.ParseBool(input) + if err == nil { + return &Attribute{Bool: helper.BoolToPtr(b)} + } + + return &Attribute{String: helper.StringToPtr(input)} +} + // Attribute is used to describe the value of an attribute, optionally // specifying units type Attribute struct { @@ -72,7 +119,84 @@ type Attribute struct { Unit string } +// NewStringAttribute returns a new string attribute. +func NewStringAttribute(s string) *Attribute { + return &Attribute{ + String: helper.StringToPtr(s), + } +} + +// NewBoolAttribute returns a new boolean attribute. +func NewBoolAttribute(b bool) *Attribute { + return &Attribute{ + Bool: helper.BoolToPtr(b), + } +} + +// NewIntergerAttribute returns a new integer attribute. The unit is not checked +// to be valid. +func NewIntAttribute(i int64, unit string) *Attribute { + return &Attribute{ + Int: helper.Int64ToPtr(i), + Unit: unit, + } +} + +// NewFloatAttribute returns a new float attribute. The unit is not checked to +// be valid. +func NewFloatAttribute(f float64, unit string) *Attribute { + return &Attribute{ + Float: helper.Float64ToPtr(f), + Unit: unit, + } +} + +// GetString returns the string value of the attribute or false if the attribute +// doesn't contain a string. +func (a *Attribute) GetString() (value string, ok bool) { + if a.String == nil { + return "", false + } + + return *a.String, true +} + +// GetBool returns the boolean value of the attribute or false if the attribute +// doesn't contain a boolean. +func (a *Attribute) GetBool() (value bool, ok bool) { + if a.Bool == nil { + return false, false + } + + return *a.Bool, true +} + +// GetInt returns the integer value of the attribute or false if the attribute +// doesn't contain a integer. +func (a *Attribute) GetInt() (value int64, ok bool) { + if a.Int == nil { + return 0, false + } + + return *a.Int, true +} + +// GetFloat returns the float value of the attribute or false if the attribute +// doesn't contain a float. +func (a *Attribute) GetFloat() (value float64, ok bool) { + if a.Float == nil { + return 0.0, false + } + + return *a.Float, true +} + +// Copy returns a copied version of the attribute func (a *Attribute) Copy() *Attribute { + if a == nil { + return nil + } + ca := &Attribute{ Unit: a.Unit, } @@ -154,6 +278,39 @@ func (a *Attribute) Validate() error { return nil } +// Comparable returns whether the two attributes are comparable +func (a *Attribute) Comparable(b *Attribute) bool { + if a == nil || b == nil { + return false + } + + // First use the units to decide if comparison is possible + aUnit := a.getTypedUnit() + bUnit := b.getTypedUnit() + if aUnit != nil && bUnit != nil { + return aUnit.Comparable(bUnit) + } else if aUnit != nil && bUnit == nil { + return false + } else if aUnit == nil && bUnit != nil { + return false + } + + if a.String != nil { + if b.String != nil { + return true + } + return false + } + if a.Bool != nil { + if b.Bool != nil { + return true + } + return false + } + + return true +} + // Compare compares two attributes. If the returned boolean value is false, it // means the values are not comparable, either because they are of different // types (bool versus int) or the units are incompatible for comparison. @@ -299,102 +456,7 @@ func (a *Attribute) getInt() int64 { return i } -// Comparable returns whether they are comparable -func (a *Attribute) Comparable(b *Attribute) bool { - if a == nil || b == nil { - return false - } - - // First use the units to decide if comparison is possible - aUnit := a.getTypedUnit() - bUnit := b.getTypedUnit() - if aUnit != nil && bUnit != nil { - return aUnit.Comparable(bUnit) - } else if aUnit != nil && bUnit == nil { - return false - } else if aUnit == nil && bUnit != nil { - return false - } - - if a.String != nil { - if b.String != nil { - return true - } - return false - } - if a.Bool != nil { - if b.Bool != nil { - return true - } - return false - } - - return true -} - // getTypedUnit returns the Unit for the attribute or nil if no unit exists. func (a *Attribute) getTypedUnit() *Unit { return UnitIndex[a.Unit] } - -// ParseAttribute takes a string and parses it into an attribute, pulling out -// units if they are specified as a suffix on a number -func ParseAttribute(input string) *Attribute { - ll := len(input) - if ll == 0 { - return &Attribute{String: helper.StringToPtr(input)} - } - - // Try to parse as a bool - b, err := strconv.ParseBool(input) - if err == nil { - return &Attribute{Bool: helper.BoolToPtr(b)} - } - - // Check if the string is a number ending with potential units - if unicode.IsLetter(rune(input[ll-1])) { - // Try suffix matching - var unit string - for _, u := range lengthSortedUnits { - if strings.HasSuffix(input, u) { - unit = u - break - } - } - - // Check if we know about the unit. If we don't we can only treat this - // as a string - if len(unit) == 0 { - return &Attribute{String: helper.StringToPtr(input)} - } - - // Grab the numeric - numeric := strings.TrimSpace(strings.TrimSuffix(input, unit)) - - // Try to parse as an int - i, err := strconv.ParseInt(numeric, 10, 64) - if err == nil { - return &Attribute{Int: helper.Int64ToPtr(i), Unit: unit} - } - - // Try to parse as a float - f, err := strconv.ParseFloat(numeric, 64) - if err == nil { - return &Attribute{Float: helper.Float64ToPtr(f), Unit: unit} - } - } - - // Try to parse as an int - i, err := strconv.ParseInt(input, 10, 64) - if err == nil { - return &Attribute{Int: helper.Int64ToPtr(i)} - } - - // Try to parse as a float - f, err := strconv.ParseFloat(input, 64) - if err == nil { - return &Attribute{Float: helper.Float64ToPtr(f)} - } - - return &Attribute{String: helper.StringToPtr(input)} -} diff --git a/plugins/shared/structs/attribute_test.go b/plugins/shared/structs/attribute_test.go index fa9197206..95bdd6c37 100644 --- a/plugins/shared/structs/attribute_test.go +++ b/plugins/shared/structs/attribute_test.go @@ -558,6 +558,12 @@ func TestAttribute_ParseAndValidate(t *testing.T) { Bool: helper.BoolToPtr(false), }, }, + { + Input: "1", + Expected: &Attribute{ + Int: helper.Int64ToPtr(1), + }, + }, { Input: "100", Expected: &Attribute{ diff --git a/scheduler/feasible.go b/scheduler/feasible.go index a7aaf9bb4..ea0c1bec4 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -7,6 +7,8 @@ import ( "strconv" "strings" + psstructs "github.com/hashicorp/nomad/plugins/shared/structs" + "github.com/hashicorp/go-version" "github.com/hashicorp/nomad/nomad/structs" ) @@ -556,6 +558,48 @@ func checkVersionMatch(ctx Context, lVal, rVal interface{}) bool { return constraints.Check(vers) } +// 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 { + // Parse the version + var versionStr string + if s, ok := lVal.GetString(); ok { + versionStr = s + } else if i, ok := lVal.GetInt(); ok { + versionStr = fmt.Sprintf("%d", i) + } else { + return false + } + + // Parse the version + vers, err := version.NewVersion(versionStr) + if err != nil { + return false + } + + // Constraint must be a string + constraintStr, ok := rVal.GetString() + if !ok { + return false + } + + // Check the cache for a match + cache := ctx.VersionConstraintCache() + constraints := cache[constraintStr] + + // Parse the constraints + if constraints == nil { + constraints, err = version.NewConstraint(constraintStr) + if err != nil { + return false + } + cache[constraintStr] = constraints + } + + // Check the constraints against the version + return constraints.Check(vers) +} + // checkRegexpMatch is used to compare a value on the // left hand side with a regexp on the right hand side func checkRegexpMatch(ctx Context, lVal, rVal interface{}) bool { @@ -774,9 +818,7 @@ OUTER: type DeviceChecker struct { ctx Context - // required is a mapping of devices that must exist on the node - //required map[*structs.DeviceIdTuple][]*structs.RequestedDevice - + // required is the set of requested devices that must exist on the node required []*structs.RequestedDevice // requiresDevices indicates if the task group requires devices @@ -844,7 +886,6 @@ OUTER: for _, req := range c.required { // Determine how many there are to place desiredCount := req.Count - //desiredCount := remainingRequested[req] // Go through the device resources and see if we have a match for d, unused := range available { @@ -853,7 +894,7 @@ OUTER: continue } - if NodeDeviceMatches(d, req) { + if nodeDeviceMatches(c.ctx, d, req) { // Consume the instances if unused >= desiredCount { // This device satisfies all our requests @@ -879,7 +920,9 @@ OUTER: return true } -func NodeDeviceMatches(d *structs.NodeDeviceResource, req *structs.RequestedDevice) bool { +// nodeDeviceMatches checks if the device matches the request and its +// constraints. It doesn't check the count. +func nodeDeviceMatches(ctx Context, d *structs.NodeDeviceResource, req *structs.RequestedDevice) bool { if !d.ID().Matches(req.ID()) { return false } @@ -889,5 +932,109 @@ func NodeDeviceMatches(d *structs.NodeDeviceResource, req *structs.RequestedDevi return true } + 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 + } + + // Check if satisfied + if !checkAttributeConstraint(ctx, c.Operand, lVal, rVal) { + return false + } + } + return true } + +// resolveDeviceTarget is used to resolve the LTarget and RTarget of a Constraint +// when used on a device +func resolveDeviceTarget(target string, d *structs.NodeDeviceResource) (*psstructs.Attribute, bool) { + // If no prefix, this must be a literal value + if !strings.HasPrefix(target, "${") { + return psstructs.ParseAttribute(target), true + } + + // Handle the interpolations + switch { + case "${driver.model}" == target: + return psstructs.NewStringAttribute(d.Name), true + + case "${driver.vendor}" == target: + return psstructs.NewStringAttribute(d.Vendor), true + + case "${driver.type}" == target: + return psstructs.NewStringAttribute(d.Type), true + + case strings.HasPrefix(target, "${driver.attr."): + attr := strings.TrimSuffix(strings.TrimPrefix(target, "${driver.attr."), "}") + val, ok := d.Attributes[attr] + return val, ok + + default: + return nil, false + } +} + +// checkAttributeConstraint checks if a constraint is satisfied +func checkAttributeConstraint(ctx Context, operand string, lVal, rVal *psstructs.Attribute) bool { + // Check for constraints not handled by this checker. + switch operand { + case structs.ConstraintDistinctHosts, structs.ConstraintDistinctProperty: + return true + default: + break + } + + switch operand { + case "<", "<=", ">", ">=", "=", "==", "is", "!=", "not": + v, ok := lVal.Compare(rVal) + if !ok { + return false + } + + switch operand { + case "not", "!=": + return v != 0 + case "is", "==", "=": + return v == 0 + case "<": + return v == -1 + case "<=": + return v != 1 + case ">": + return v == 1 + case ">=": + return v != -1 + default: + return false + } + + case structs.ConstraintVersion: + return checkAttributeVersionMatch(ctx, lVal, rVal) + case structs.ConstraintRegex: + ls, ok := lVal.GetString() + rs, ok2 := rVal.GetString() + if !ok || !ok2 { + return false + } + return checkRegexpMatch(ctx, ls, rs) + case structs.ConstraintSetContains: + ls, ok := lVal.GetString() + rs, ok2 := rVal.GetString() + if !ok || !ok2 { + return false + } + + return checkSetContainsAll(ctx, ls, rs) + default: + return false + } + + return false +} diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index e1368d19e..41f1465de 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" + psstructs "github.com/hashicorp/nomad/plugins/shared/structs" "github.com/stretchr/testify/require" ) @@ -1680,6 +1681,11 @@ func TestDeviceChecker(t *testing.T) { Vendor: "nvidia", Type: "gpu", Name: "1080ti", + Attributes: map[string]*psstructs.Attribute{ + "memory": psstructs.NewIntAttribute(4, psstructs.UnitGiB), + "pci_bandwidth": psstructs.NewIntAttribute(995, psstructs.UnitMiBPerS), + "cores_clock": psstructs.NewIntAttribute(800, psstructs.UnitMHz), + }, Instances: []*structs.NodeDevice{ &structs.NodeDevice{ ID: uuid.Generate(), @@ -1796,6 +1802,105 @@ func TestDeviceChecker(t *testing.T) { NodeDevices: []*structs.NodeDeviceResource{nvidia, intel}, RequestedDevices: []*structs.RequestedDevice{gpuTypeHighCountReq}, }, + { + Name: "meets constraints requirement", + Result: true, + NodeDevices: []*structs.NodeDeviceResource{nvidia}, + RequestedDevices: []*structs.RequestedDevice{ + { + Name: "nvidia/gpu", + Count: 1, + Constraints: []*structs.Constraint{ + { + Operand: "=", + LTarget: "${driver.model}", + RTarget: "1080ti", + }, + { + Operand: ">", + LTarget: "${driver.attr.memory}", + RTarget: "1320.5 MB", + }, + { + Operand: "<=", + LTarget: "${driver.attr.pci_bandwidth}", + RTarget: ".98 GiB/s", + }, + { + Operand: "=", + LTarget: "${driver.attr.cores_clock}", + RTarget: "800MHz", + }, + }, + }, + }, + }, + { + Name: "does not meet first constraint", + Result: false, + NodeDevices: []*structs.NodeDeviceResource{nvidia}, + RequestedDevices: []*structs.RequestedDevice{ + { + Name: "nvidia/gpu", + Count: 1, + Constraints: []*structs.Constraint{ + { + Operand: "=", + LTarget: "${driver.model}", + RTarget: "2080ti", + }, + { + Operand: ">", + LTarget: "${driver.attr.memory}", + RTarget: "1320.5 MB", + }, + { + Operand: "<=", + LTarget: "${driver.attr.pci_bandwidth}", + RTarget: ".98 GiB/s", + }, + { + Operand: "=", + LTarget: "${driver.attr.cores_clock}", + RTarget: "800MHz", + }, + }, + }, + }, + }, + { + Name: "does not meet second constraint", + Result: false, + NodeDevices: []*structs.NodeDeviceResource{nvidia}, + RequestedDevices: []*structs.RequestedDevice{ + { + Name: "nvidia/gpu", + Count: 1, + Constraints: []*structs.Constraint{ + { + Operand: "=", + LTarget: "${driver.model}", + RTarget: "1080ti", + }, + { + Operand: "<", + LTarget: "${driver.attr.memory}", + RTarget: "1320.5 MB", + }, + { + Operand: "<=", + LTarget: "${driver.attr.pci_bandwidth}", + RTarget: ".98 GiB/s", + }, + { + Operand: "=", + LTarget: "${driver.attr.cores_clock}", + RTarget: "800MHz", + }, + }, + }, + }, + }, } for _, c := range cases { @@ -1809,3 +1914,86 @@ func TestDeviceChecker(t *testing.T) { }) } } + +func TestCheckAttributeConstraint(t *testing.T) { + type tcase struct { + op string + lVal, rVal *psstructs.Attribute + result bool + } + cases := []tcase{ + { + op: "=", + lVal: psstructs.NewStringAttribute("foo"), + rVal: psstructs.NewStringAttribute("foo"), + result: true, + }, + { + op: "is", + lVal: psstructs.NewStringAttribute("foo"), + rVal: psstructs.NewStringAttribute("foo"), + result: true, + }, + { + op: "==", + lVal: psstructs.NewStringAttribute("foo"), + rVal: psstructs.NewStringAttribute("foo"), + result: true, + }, + { + op: "!=", + lVal: psstructs.NewStringAttribute("foo"), + rVal: psstructs.NewStringAttribute("foo"), + result: false, + }, + { + op: "!=", + lVal: psstructs.NewStringAttribute("foo"), + rVal: psstructs.NewStringAttribute("bar"), + result: true, + }, + { + op: "not", + lVal: psstructs.NewStringAttribute("foo"), + rVal: psstructs.NewStringAttribute("bar"), + result: true, + }, + { + op: structs.ConstraintVersion, + lVal: psstructs.NewStringAttribute("1.2.3"), + rVal: psstructs.NewStringAttribute("~> 1.0"), + result: true, + }, + { + op: structs.ConstraintRegex, + lVal: psstructs.NewStringAttribute("foobarbaz"), + rVal: psstructs.NewStringAttribute("[\\w]+"), + result: true, + }, + { + op: "<", + lVal: psstructs.NewStringAttribute("foo"), + rVal: psstructs.NewStringAttribute("bar"), + result: false, + }, + { + op: structs.ConstraintSetContains, + lVal: psstructs.NewStringAttribute("foo,bar,baz"), + rVal: psstructs.NewStringAttribute("foo, bar "), + result: true, + }, + { + op: structs.ConstraintSetContains, + lVal: psstructs.NewStringAttribute("foo,bar,baz"), + rVal: psstructs.NewStringAttribute("foo,bam"), + result: false, + }, + } + + for _, tc := range cases { + _, ctx := testContext(t) + if res := checkAttributeConstraint(ctx, tc.op, tc.lVal, tc.rVal); res != tc.result { + t.Fatalf("TC: %#v, Result: %v", tc, res) + } + } +}