-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Changes from 7 commits
3ae9340
9572878
7127f6e
9f259c2
a52bdd9
db90849
bd10a6b
ec78455
cccef7a
302c989
2ab5790
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
job "foo" { | ||
constraint { | ||
distinctHosts = "true" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -150,6 +150,117 @@ func (iter *DriverIterator) hasDrivers(option *structs.Node) bool { | |
return true | ||
} | ||
|
||
// DynamicConstraintIterator is a FeasibleIterator which returns nodes that | ||
// match constraints that are not static such as Node attributes but are | ||
// effected by alloc placements. Examples are distinctHosts and tenancy constraints. | ||
// This is used to filter on job and task group constraints. | ||
type DynamicConstraintIterator struct { | ||
ctx Context | ||
source FeasibleIterator | ||
tg *structs.TaskGroup | ||
job *structs.Job | ||
|
||
// Store whether the Job or TaskGroup has a distinctHosts constraints so | ||
// they don't have to be calculated every time Next() is called. | ||
tgDistinctHosts bool | ||
jobDistinctHosts bool | ||
} | ||
|
||
// NewDynamicConstraintIterator creates a DynamicConstraintIterator from a | ||
// source. | ||
func NewDynamicConstraintIterator(ctx Context, source FeasibleIterator) *DynamicConstraintIterator { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename to |
||
iter := &DynamicConstraintIterator{ | ||
ctx: ctx, | ||
source: source, | ||
} | ||
return iter | ||
} | ||
|
||
func (iter *DynamicConstraintIterator) SetTaskGroup(tg *structs.TaskGroup) { | ||
iter.tg = tg | ||
iter.tgDistinctHosts = iter.hasDistinctHostsConstraint(tg.Constraints) | ||
} | ||
|
||
func (iter *DynamicConstraintIterator) SetJob(job *structs.Job) { | ||
iter.job = job | ||
iter.jobDistinctHosts = iter.hasDistinctHostsConstraint(job.Constraints) | ||
} | ||
|
||
func (iter *DynamicConstraintIterator) hasDistinctHostsConstraint(constraints []*structs.Constraint) bool { | ||
if constraints == nil { | ||
return false | ||
} | ||
|
||
for _, con := range constraints { | ||
if con.Operand == "distinctHosts" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this a const in |
||
return true | ||
} | ||
} | ||
return false | ||
} | ||
|
||
func (iter *DynamicConstraintIterator) Next() *structs.Node { | ||
if iter.job == nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to redo these checks as the entire stack is broken if no job or TG |
||
iter.ctx.Logger().Printf("[ERR] sched.dynamic-constraint: job not set") | ||
return nil | ||
} | ||
|
||
if iter.tg == nil { | ||
iter.ctx.Logger().Printf("[ERR] sched.dynamic-constraint: task group not set") | ||
return nil | ||
} | ||
|
||
for { | ||
// Get the next option from the source | ||
option := iter.source.Next() | ||
|
||
// Hot-path if the option is nil or there are no distinctHosts constraints. | ||
if option == nil || (!iter.jobDistinctHosts && !iter.tgDistinctHosts) { | ||
return option | ||
} | ||
|
||
if !iter.satisfiesDistinctHosts(option, iter.jobDistinctHosts) { | ||
iter.ctx.Metrics().FilterNode(option, "distinctHosts") | ||
continue | ||
} | ||
|
||
return option | ||
} | ||
} | ||
|
||
// satisfiesDistinctHosts checks if the node satisfies a distinctHosts | ||
// constraint either specified at the job level or the TaskGroup level. | ||
func (iter *DynamicConstraintIterator) satisfiesDistinctHosts(option *structs.Node, job bool) bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// Get the proposed allocations | ||
proposed, err := iter.ctx.ProposedAllocs(option.ID) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Investigate caching this |
||
if err != nil { | ||
iter.ctx.Logger().Printf( | ||
"[ERR] sched.dynamic-constraint: failed to get proposed allocations: %v", err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/sched/scheduler/g The other log statement in this file says scheduler There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch |
||
return false | ||
} | ||
|
||
// Skip the node if the task group has already been allocated on it. | ||
for _, alloc := range proposed { | ||
jobCollision := alloc.JobID == iter.job.ID | ||
taskCollision := alloc.TaskGroup == iter.tg.Name | ||
|
||
// If the job has a distinctHosts 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. | ||
jobInvalid := job && jobCollision | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function should not depend on the callers behavior to implicit rule out the |
||
tgInvalid := !job && jobCollision && taskCollision | ||
if jobInvalid || tgInvalid { | ||
return false | ||
} | ||
} | ||
|
||
return true | ||
} | ||
|
||
func (iter *DynamicConstraintIterator) 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. | ||
|
@@ -257,6 +368,14 @@ 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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not merge this with the next one? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
case "distinctHosts": | ||
return true | ||
default: | ||
break | ||
} | ||
|
||
switch operand { | ||
case "=", "==", "is": | ||
return reflect.DeepEqual(lVal, rVal) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing in HCL uses camal case