Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "distinctHost" constraint #321

Merged
merged 11 commits into from
Oct 26, 2015
23 changes: 19 additions & 4 deletions jobspec/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"regexp"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -244,18 +245,32 @@ func parseConstraints(result *[]*structs.Constraint, obj *hclobj.Object) error {

// If "version" is provided, set the operand
// to "version" and the value to the "RTarget"
if constraint, ok := m["version"]; ok {
m["Operand"] = "version"
if constraint, ok := m[structs.ConstraintVersion]; ok {
m["Operand"] = structs.ConstraintVersion
m["RTarget"] = constraint
}

// If "regexp" is provided, set the operand
// to "regexp" and the value to the "RTarget"
if constraint, ok := m["regexp"]; ok {
m["Operand"] = "regexp"
if constraint, ok := m[structs.ConstraintRegex]; ok {
m["Operand"] = structs.ConstraintRegex
m["RTarget"] = constraint
}

if value, ok := m[structs.ConstraintDistinctHosts]; ok {
enabled, err := strconv.ParseBool(value.(string))
if err != nil {
return err
}

// If it is not enabled, skip the constraint.
if !enabled {
continue
}

m["Operand"] = structs.ConstraintDistinctHosts
}

// Build the constraint
var c structs.Constraint
if err := mapstructure.WeakDecode(m, &c); err != nil {
Expand Down
22 changes: 20 additions & 2 deletions jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func TestParse(t *testing.T) {
Hard: true,
LTarget: "$attr.kernel.version",
RTarget: "~> 3.2",
Operand: "version",
Operand: structs.ConstraintVersion,
},
},
},
Expand All @@ -185,7 +185,25 @@ func TestParse(t *testing.T) {
Hard: true,
LTarget: "$attr.kernel.version",
RTarget: "[0-9.]+",
Operand: "regexp",
Operand: structs.ConstraintRegex,
},
},
},
false,
},

{
"distinctHosts-constraint.hcl",
&structs.Job{
ID: "foo",
Name: "foo",
Priority: 50,
Region: "global",
Type: "service",
Constraints: []*structs.Constraint{
&structs.Constraint{
Hard: true,
Operand: structs.ConstraintDistinctHosts,
},
},
},
Expand Down
5 changes: 5 additions & 0 deletions jobspec/test-fixtures/distinctHosts-constraint.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
job "foo" {
constraint {
distinct_hosts = "true"
}
}
10 changes: 8 additions & 2 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,12 @@ func (t *Task) Validate() error {
return mErr.ErrorOrNil()
}

const (
ConstraintDistinctHosts = "distinct_hosts"
ConstraintRegex = "regexp"
ConstraintVersion = "version"
)

// Constraints are used to restrict placement options in the case of
// a hard constraint, and used to prefer a placement in the case of
// a soft constraint.
Expand All @@ -1050,11 +1056,11 @@ func (c *Constraint) Validate() error {

// Perform additional validation based on operand
switch c.Operand {
case "regexp":
case ConstraintRegex:
if _, err := regexp.Compile(c.RTarget); err != nil {
mErr.Errors = append(mErr.Errors, fmt.Errorf("Regular expression failed to compile: %v", err))
}
case "version":
case ConstraintVersion:
if _, err := version.NewConstraint(c.RTarget); err != nil {
mErr.Errors = append(mErr.Errors, fmt.Errorf("Version constraint is invalid: %v", err))
}
Expand Down
4 changes: 2 additions & 2 deletions nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func TestConstraint_Validate(t *testing.T) {
}

// Perform additional regexp validation
c.Operand = "regexp"
c.Operand = ConstraintRegex
c.RTarget = "(foo"
err = c.Validate()
mErr = err.(*multierror.Error)
Expand All @@ -153,7 +153,7 @@ func TestConstraint_Validate(t *testing.T) {
}

// Perform version validation
c.Operand = "version"
c.Operand = ConstraintVersion
c.RTarget = "~> foo"
err = c.Validate()
mErr = err.(*multierror.Error)
Expand Down
112 changes: 110 additions & 2 deletions scheduler/feasible.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,106 @@ func (iter *DriverIterator) hasDrivers(option *structs.Node) bool {
return true
}

// ProposedAllocConstraintIterator is a FeasibleIterator which returns nodes that
// match constraints that are not static such as Node attributes but are
// effected by proposed alloc placements. Examples are distinct_hosts and
// tenancy constraints. This is used to filter on job and task group
// constraints.
type ProposedAllocConstraintIterator struct {
ctx Context
source FeasibleIterator
tg *structs.TaskGroup
job *structs.Job

// Store whether the Job or TaskGroup has a distinct_hosts constraints so
// they don't have to be calculated every time Next() is called.
tgDistinctHosts bool
jobDistinctHosts bool
}

// NewProposedAllocConstraintIterator creates a ProposedAllocConstraintIterator
// from a source.
func NewProposedAllocConstraintIterator(ctx Context, source FeasibleIterator) *ProposedAllocConstraintIterator {
iter := &ProposedAllocConstraintIterator{
ctx: ctx,
source: source,
}
return iter
}

func (iter *ProposedAllocConstraintIterator) SetTaskGroup(tg *structs.TaskGroup) {
iter.tg = tg
iter.tgDistinctHosts = iter.hasDistinctHostsConstraint(tg.Constraints)
}

func (iter *ProposedAllocConstraintIterator) SetJob(job *structs.Job) {
iter.job = job
iter.jobDistinctHosts = iter.hasDistinctHostsConstraint(job.Constraints)
}

func (iter *ProposedAllocConstraintIterator) hasDistinctHostsConstraint(constraints []*structs.Constraint) bool {
for _, con := range constraints {
if con.Operand == structs.ConstraintDistinctHosts {
return true
}
}
return false
}

func (iter *ProposedAllocConstraintIterator) Next() *structs.Node {
for {
// Get the next option from the source
option := iter.source.Next()

// Hot-path if the option is nil or there are no distinct_hosts constraints.
if option == nil || !(iter.jobDistinctHosts || iter.tgDistinctHosts) {
return option
}

if !iter.satisfiesDistinctHosts(option) {
iter.ctx.Metrics().FilterNode(option, structs.ConstraintDistinctHosts)
continue
}

return option
}
}

// satisfiesDistinctHosts checks if the node satisfies a distinct_hosts
// constraint either specified at the job level or the TaskGroup level.
func (iter *ProposedAllocConstraintIterator) satisfiesDistinctHosts(option *structs.Node) bool {
// Check if there is no constraint set.
if !(iter.jobDistinctHosts || iter.tgDistinctHosts) {
return true
}

// Get the proposed allocations
proposed, err := iter.ctx.ProposedAllocs(option.ID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigate caching this

if err != nil {
iter.ctx.Logger().Printf(
"[ERR] scheduler.dynamic-constraint: failed to get proposed allocations: %v", err)
return false
}

// Skip the node if the task group has already been allocated on it.
for _, alloc := range proposed {
// If the job has a distinct_hosts constraint we only need an alloc
// collision on the JobID but if the constraint is on the TaskGroup then
// we need both a job and TaskGroup collision.
jobCollision := alloc.JobID == iter.job.ID
taskCollision := alloc.TaskGroup == iter.tg.Name
if iter.jobDistinctHosts && jobCollision || jobCollision && taskCollision {
return false
}
}

return true
}

func (iter *ProposedAllocConstraintIterator) Reset() {
iter.source.Reset()
}

// ConstraintIterator is a FeasibleIterator which returns nodes
// that match a given set of constraints. This is used to filter
// on job, task group, and task constraints.
Expand Down Expand Up @@ -257,16 +357,24 @@ func resolveConstraintTarget(target string, node *structs.Node) (interface{}, bo

// checkConstraint checks if a constraint is satisfied
func checkConstraint(ctx Context, operand string, lVal, rVal interface{}) bool {
// Check for constraints not handled by this iterator.
switch operand {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not merge this with the next one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we add more constraints that are not handled here I want it to be easily distinguishable which iterator is handling which constraints.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

case structs.ConstraintDistinctHosts:
return true
default:
break
}

switch operand {
case "=", "==", "is":
return reflect.DeepEqual(lVal, rVal)
case "!=", "not":
return !reflect.DeepEqual(lVal, rVal)
case "<", "<=", ">", ">=":
return checkLexicalOrder(operand, lVal, rVal)
case "version":
case structs.ConstraintVersion:
return checkVersionConstraint(ctx, lVal, rVal)
case "regexp":
case structs.ConstraintRegex:
return checkRegexpConstraint(ctx, lVal, rVal)
default:
return false
Expand Down
Loading