From 3ae9340614de5ab160f569284a6ea6c760becaf7 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 22 Oct 2015 14:31:12 -0700 Subject: [PATCH 01/11] DynamicConstraintIterator that implements the unique constraint --- scheduler/feasible.go | 116 +++++++++++++++++++++++++ scheduler/feasible_test.go | 174 +++++++++++++++++++++++++++++++++++++ 2 files changed, 290 insertions(+) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 559252045f19..663b5beef882 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -150,6 +150,114 @@ 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 unique 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 unique constraints so they don't + // have to be calculated every time Next() is called. + tgUnique bool + jobUnique bool +} + +// NewDynamicConstraintIterator creates a DynamicConstraintIterator from a +// source. +func NewDynamicConstraintIterator(ctx Context, source FeasibleIterator) *DynamicConstraintIterator { + iter := &DynamicConstraintIterator{ + ctx: ctx, + source: source, + } + return iter +} + +func (iter *DynamicConstraintIterator) SetTaskGroup(tg *structs.TaskGroup) { + iter.tg = tg + iter.tgUnique = iter.hasUniqueConstraint(tg.Constraints) +} + +func (iter *DynamicConstraintIterator) SetJob(job *structs.Job) { + iter.job = job + iter.jobUnique = iter.hasUniqueConstraint(job.Constraints) +} + +func (iter *DynamicConstraintIterator) hasUniqueConstraint(constraints []*structs.Constraint) bool { + if constraints == nil { + return false + } + + for _, con := range constraints { + if con.Operand == "unique" { + return true + } + } + return false +} + +func (iter *DynamicConstraintIterator) 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 unique constraints. + if option == nil || (!iter.jobUnique && !iter.tgUnique) { + return option + } + + // In the case the job has a unique constraint it applies to all + // TaskGroups. + if iter.jobUnique && !iter.satisfiesUnique(option, true) { + continue + } else if iter.tgUnique && !iter.satisfiesUnique(option, false) { + continue + } + + return option + } +} + +// satisfiesUnique checks if the node satisfies a unique constraint either +// specified at the job level or the TaskGroup level. +func (iter *DynamicConstraintIterator) satisfiesUnique(option *structs.Node, job bool) bool { + // Get the proposed allocations + proposed, err := iter.ctx.ProposedAllocs(option.ID) + if err != nil { + iter.ctx.Logger().Printf( + "[ERR] sched.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 { + jobCollision := alloc.JobID == iter.job.ID + taskCollision := alloc.TaskGroup == iter.tg.Name + + // If the job has a unique 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 + tgInvalid := !job && jobCollision && taskCollision + + if jobInvalid || tgInvalid { + iter.ctx.Metrics().FilterNode(option, "unique") + return false + } + } + + return true +} + +func (iter *DynamicConstraintIterator) Reset() { + iter.tg = nil + iter.job = nil + 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 +365,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 { + case "unique": + return true + default: + break + } + switch operand { case "=", "==", "is": return reflect.DeepEqual(lVal, rVal) diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index 1cf58d1f6b3a..b7871b8d69d2 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -382,6 +382,180 @@ func TestCheckRegexpConstraint(t *testing.T) { } } +func TestDynamicConstraint_JobUnique_Feasible(t *testing.T) { + _, ctx := testContext(t) + nodes := []*structs.Node{ + mock.Node(), + mock.Node(), + mock.Node(), + mock.Node(), + } + static := NewStaticIterator(ctx, nodes) + + // Create a job with a unique constraint and two task groups. + tg1 := &structs.TaskGroup{Name: "bar"} + tg2 := &structs.TaskGroup{Name: "baz"} + + job := &structs.Job{ + ID: "foo", + Constraints: []*structs.Constraint{{Operand: "unique"}}, + TaskGroups: []*structs.TaskGroup{tg1, tg2}, + } + + dynamic := NewDynamicConstraintIterator(ctx, static) + dynamic.SetTaskGroup(tg1) + dynamic.SetJob(job) + + out := collectFeasible(dynamic) + if len(out) != 4 { + t.Fatalf("Bad: %#v", out) + } + + selected := make(map[string]struct{}, 4) + for _, option := range out { + if _, ok := selected[option.ID]; ok { + t.Fatalf("selected node %v for more than one alloc", option) + } + selected[option.ID] = struct{}{} + } +} + +func TestDynamicConstraint_JobUnique_Infeasible(t *testing.T) { + _, ctx := testContext(t) + nodes := []*structs.Node{ + mock.Node(), + mock.Node(), + } + static := NewStaticIterator(ctx, nodes) + + // Create a job with a unique constraint and two task groups. + tg1 := &structs.TaskGroup{Name: "bar"} + tg2 := &structs.TaskGroup{Name: "baz"} + + job := &structs.Job{ + ID: "foo", + Constraints: []*structs.Constraint{{Operand: "unique"}}, + TaskGroups: []*structs.TaskGroup{tg1, tg2}, + } + + // Add allocs placing tg1 on node1 and tg2 on node2. This should make the + // job unsatisfiable. + plan := ctx.Plan() + plan.NodeAllocation[nodes[0].ID] = []*structs.Allocation{ + &structs.Allocation{ + TaskGroup: tg1.Name, + JobID: job.ID, + }, + + // Should be ignored as it is a different job. + &structs.Allocation{ + TaskGroup: tg2.Name, + JobID: "ignore 2", + }, + } + plan.NodeAllocation[nodes[1].ID] = []*structs.Allocation{ + &structs.Allocation{ + TaskGroup: tg2.Name, + JobID: job.ID, + }, + + // Should be ignored as it is a different job. + &structs.Allocation{ + TaskGroup: tg1.Name, + JobID: "ignore 2", + }, + } + + dynamic := NewDynamicConstraintIterator(ctx, static) + dynamic.SetTaskGroup(tg1) + dynamic.SetJob(job) + + out := collectFeasible(dynamic) + if len(out) != 0 { + t.Fatalf("Bad: %#v", out) + } +} + +func TestDynamicConstraint_JobUnique_InfeasibleCount(t *testing.T) { + _, ctx := testContext(t) + nodes := []*structs.Node{ + mock.Node(), + mock.Node(), + } + static := NewStaticIterator(ctx, nodes) + + // Create a job with a unique constraint and three task groups. + tg1 := &structs.TaskGroup{Name: "bar"} + tg2 := &structs.TaskGroup{Name: "baz"} + tg3 := &structs.TaskGroup{Name: "bam"} + + job := &structs.Job{ + ID: "foo", + Constraints: []*structs.Constraint{{Operand: "unique"}}, + TaskGroups: []*structs.TaskGroup{tg1, tg2, tg3}, + } + + dynamic := NewDynamicConstraintIterator(ctx, static) + dynamic.SetTaskGroup(tg1) + dynamic.SetJob(job) + + // It should not be able to place 3 tasks with only two nodes. + out := collectFeasible(dynamic) + if len(out) != 2 { + t.Fatalf("Bad: %#v", out) + } +} + +func TestDynamicConstraint_TaskGroupUnique(t *testing.T) { + _, ctx := testContext(t) + nodes := []*structs.Node{ + mock.Node(), + mock.Node(), + } + static := NewStaticIterator(ctx, nodes) + + // Create a task group with a unique constraint. + taskGroup := &structs.TaskGroup{ + Name: "example", + Constraints: []*structs.Constraint{ + {Operand: "unique"}, + }, + } + + // Add a planned alloc to node1. + plan := ctx.Plan() + plan.NodeAllocation[nodes[0].ID] = []*structs.Allocation{ + &structs.Allocation{ + TaskGroup: taskGroup.Name, + JobID: "foo", + }, + } + + // Add a planned alloc to node2 with the same task group name but a + // different job. + plan.NodeAllocation[nodes[1].ID] = []*structs.Allocation{ + &structs.Allocation{ + TaskGroup: taskGroup.Name, + JobID: "bar", + }, + } + + dynamic := NewDynamicConstraintIterator(ctx, static) + dynamic.SetTaskGroup(taskGroup) + dynamic.SetJob(&structs.Job{ID: "foo"}) + + out := collectFeasible(dynamic) + if len(out) != 1 { + t.Fatalf("Bad: %#v", out) + } + + // Expect it to skip the first node as there is a previous alloc on it for + // the same task group. + if out[0] != nodes[1] { + t.Fatalf("Bad: %v", out) + } +} + func collectFeasible(iter FeasibleIterator) (out []*structs.Node) { for { next := iter.Next() From 9572878a925c8256394256858d82678f03299153 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 22 Oct 2015 15:09:03 -0700 Subject: [PATCH 02/11] Add dynamic constraint to generic_scheduler --- scheduler/feasible.go | 12 ++++++++++-- scheduler/stack.go | 7 ++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 663b5beef882..6b137ab6cd28 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -200,6 +200,16 @@ func (iter *DynamicConstraintIterator) hasUniqueConstraint(constraints []*struct } func (iter *DynamicConstraintIterator) Next() *structs.Node { + if iter.job == nil { + 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() @@ -253,8 +263,6 @@ func (iter *DynamicConstraintIterator) satisfiesUnique(option *structs.Node, job } func (iter *DynamicConstraintIterator) Reset() { - iter.tg = nil - iter.job = nil iter.source.Reset() } diff --git a/scheduler/stack.go b/scheduler/stack.go index 2cda626c488f..379e4ef22ccc 100644 --- a/scheduler/stack.go +++ b/scheduler/stack.go @@ -41,6 +41,7 @@ type GenericStack struct { jobConstraint *ConstraintIterator taskGroupDrivers *DriverIterator taskGroupConstraint *ConstraintIterator + dynamicConstraint *DynamicConstraintIterator binPack *BinPackIterator jobAntiAff *JobAntiAffinityIterator limit *LimitIterator @@ -69,8 +70,10 @@ func NewGenericStack(batch bool, ctx Context) *GenericStack { // Filter on task group constraints second s.taskGroupConstraint = NewConstraintIterator(ctx, s.taskGroupDrivers, nil) + s.dynamicConstraint = NewDynamicConstraintIterator(ctx, s.taskGroupConstraint) + // Upgrade from feasible to rank iterator - rankSource := NewFeasibleRankIterator(ctx, s.taskGroupConstraint) + rankSource := NewFeasibleRankIterator(ctx, s.dynamicConstraint) // Apply the bin packing, this depends on the resources needed // by a particular task group. Only enable eviction for the service @@ -119,6 +122,7 @@ func (s *GenericStack) SetNodes(baseNodes []*structs.Node) { func (s *GenericStack) SetJob(job *structs.Job) { s.jobConstraint.SetConstraints(job.Constraints) + s.dynamicConstraint.SetJob(job) s.binPack.SetPriority(job.Priority) s.jobAntiAff.SetJob(job.ID) } @@ -135,6 +139,7 @@ func (s *GenericStack) Select(tg *structs.TaskGroup) (*RankedNode, *structs.Reso // Update the parameters of iterators s.taskGroupDrivers.SetDrivers(tgConstr.drivers) s.taskGroupConstraint.SetConstraints(tgConstr.constraints) + s.dynamicConstraint.SetTaskGroup(tg) s.binPack.SetTasks(tg.Tasks) // Find the node with the max score From 7127f6ec606e2df31ae7659a7b246f7fce9121ed Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 22 Oct 2015 16:37:20 -0700 Subject: [PATCH 03/11] Documentation and making unique first class in the constraint block --- jobspec/parse.go | 15 +++++++++++++++ jobspec/parse_test.go | 18 ++++++++++++++++++ website/source/docs/jobspec/index.html.md | 8 ++++++++ 3 files changed, 41 insertions(+) diff --git a/jobspec/parse.go b/jobspec/parse.go index b7ca31c54595..a7ef017990f7 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "regexp" + "strconv" "strings" "time" @@ -256,6 +257,20 @@ func parseConstraints(result *[]*structs.Constraint, obj *hclobj.Object) error { m["RTarget"] = constraint } + if value, ok := m["unique"]; 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"] = "unique" + } + // Build the constraint var c structs.Constraint if err := mapstructure.WeakDecode(m, &c); err != nil { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 60c5f1f0946f..979ef602056f 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -192,6 +192,24 @@ func TestParse(t *testing.T) { false, }, + { + "unique-constraint.hcl", + &structs.Job{ + ID: "foo", + Name: "foo", + Priority: 50, + Region: "global", + Type: "service", + Constraints: []*structs.Constraint{ + &structs.Constraint{ + Hard: true, + Operand: "unique", + }, + }, + }, + false, + }, + { "specify-job.hcl", &structs.Job{ diff --git a/website/source/docs/jobspec/index.html.md b/website/source/docs/jobspec/index.html.md index c1f463d002af..540ca1be271f 100644 --- a/website/source/docs/jobspec/index.html.md +++ b/website/source/docs/jobspec/index.html.md @@ -237,6 +237,14 @@ The `constraint` object supports the following keys: the attribute. This sets the operator to "regexp" and the `value` to the regular expression. +* `unique` - Unique accepts a boolean value and can be used to mark a Job or + a Task Group as requiring placement on unique nodes. If the `unique` + constraint is placed on a Job, all of it's Task Groups must be placed on + unique nodes. If the `unique` constraint is placed on a Task Group, then + multiple instances of that Task Group must be placed on unique nodes. This + sets the operator to "unique" if `unique` is set to "true". If set to "false", + the constraint is ignored as this is the default behavior. + Below is a table documenting the variables that can be interpreted: From 9f259c27f61cf6ced9c0668c5b32e2217a3906b4 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 22 Oct 2015 16:45:03 -0700 Subject: [PATCH 04/11] Fix test and simplify some boolean logic/fix metrics counting --- scheduler/feasible.go | 9 ++------- scheduler/util_test.go | 1 + 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 6b137ab6cd28..37c066faaee1 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -219,11 +219,8 @@ func (iter *DynamicConstraintIterator) Next() *structs.Node { return option } - // In the case the job has a unique constraint it applies to all - // TaskGroups. - if iter.jobUnique && !iter.satisfiesUnique(option, true) { - continue - } else if iter.tgUnique && !iter.satisfiesUnique(option, false) { + if !iter.satisfiesUnique(option, iter.jobUnique) { + iter.ctx.Metrics().FilterNode(option, "unique") continue } @@ -252,9 +249,7 @@ func (iter *DynamicConstraintIterator) satisfiesUnique(option *structs.Node, job // a job and TaskGroup collision. jobInvalid := job && jobCollision tgInvalid := !job && jobCollision && taskCollision - if jobInvalid || tgInvalid { - iter.ctx.Metrics().FilterNode(option, "unique") return false } } diff --git a/scheduler/util_test.go b/scheduler/util_test.go index 417737dcaad2..fc726391336e 100644 --- a/scheduler/util_test.go +++ b/scheduler/util_test.go @@ -555,6 +555,7 @@ func TestInplaceUpdate_Success(t *testing.T) { updates := []allocTuple{{Alloc: alloc, TaskGroup: tg}} stack := NewGenericStack(false, ctx) + stack.SetJob(job) // Do the inplace update. unplaced := inplaceUpdate(ctx, eval, job, stack, updates) From a52bdd914d9e2137bd25231bb05cdfa817c2244c Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 22 Oct 2015 16:52:00 -0700 Subject: [PATCH 05/11] Add test fixture --- jobspec/test-fixtures/unique-constraint.hcl | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 jobspec/test-fixtures/unique-constraint.hcl diff --git a/jobspec/test-fixtures/unique-constraint.hcl b/jobspec/test-fixtures/unique-constraint.hcl new file mode 100644 index 000000000000..47c31d2d924f --- /dev/null +++ b/jobspec/test-fixtures/unique-constraint.hcl @@ -0,0 +1,5 @@ +job "foo" { + constraint { + unique = "true" + } +} From db90849fb1b91acb83b31d766ae2ab6a9bd759b3 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 22 Oct 2015 17:40:41 -0700 Subject: [PATCH 06/11] Change "unique" to "distinctHosts" --- jobspec/parse.go | 4 +-- jobspec/parse_test.go | 4 +-- jobspec/test-fixtures/unique-constraint.hcl | 5 --- scheduler/feasible.go | 40 ++++++++++----------- scheduler/feasible_test.go | 24 ++++++------- website/source/docs/jobspec/index.html.md | 18 ++++++---- 6 files changed, 47 insertions(+), 48 deletions(-) delete mode 100644 jobspec/test-fixtures/unique-constraint.hcl diff --git a/jobspec/parse.go b/jobspec/parse.go index a7ef017990f7..18d1a4ff338c 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -257,7 +257,7 @@ func parseConstraints(result *[]*structs.Constraint, obj *hclobj.Object) error { m["RTarget"] = constraint } - if value, ok := m["unique"]; ok { + if value, ok := m["distinctHosts"]; ok { enabled, err := strconv.ParseBool(value.(string)) if err != nil { return err @@ -268,7 +268,7 @@ func parseConstraints(result *[]*structs.Constraint, obj *hclobj.Object) error { continue } - m["Operand"] = "unique" + m["Operand"] = "distinctHosts" } // Build the constraint diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 979ef602056f..07cb7c914b54 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -193,7 +193,7 @@ func TestParse(t *testing.T) { }, { - "unique-constraint.hcl", + "distinctHosts-constraint.hcl", &structs.Job{ ID: "foo", Name: "foo", @@ -203,7 +203,7 @@ func TestParse(t *testing.T) { Constraints: []*structs.Constraint{ &structs.Constraint{ Hard: true, - Operand: "unique", + Operand: "distinctHosts", }, }, }, diff --git a/jobspec/test-fixtures/unique-constraint.hcl b/jobspec/test-fixtures/unique-constraint.hcl deleted file mode 100644 index 47c31d2d924f..000000000000 --- a/jobspec/test-fixtures/unique-constraint.hcl +++ /dev/null @@ -1,5 +0,0 @@ -job "foo" { - constraint { - unique = "true" - } -} diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 37c066faaee1..977fa034c0e8 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -152,7 +152,7 @@ func (iter *DriverIterator) hasDrivers(option *structs.Node) bool { // 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 unique and tenancy constraints. +// 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 @@ -160,10 +160,10 @@ type DynamicConstraintIterator struct { tg *structs.TaskGroup job *structs.Job - // Store whether the Job or TaskGroup has unique constraints so they don't - // have to be calculated every time Next() is called. - tgUnique bool - jobUnique bool + // 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 @@ -178,21 +178,21 @@ func NewDynamicConstraintIterator(ctx Context, source FeasibleIterator) *Dynamic func (iter *DynamicConstraintIterator) SetTaskGroup(tg *structs.TaskGroup) { iter.tg = tg - iter.tgUnique = iter.hasUniqueConstraint(tg.Constraints) + iter.tgDistinctHosts = iter.hasDistinctHostsConstraint(tg.Constraints) } func (iter *DynamicConstraintIterator) SetJob(job *structs.Job) { iter.job = job - iter.jobUnique = iter.hasUniqueConstraint(job.Constraints) + iter.jobDistinctHosts = iter.hasDistinctHostsConstraint(job.Constraints) } -func (iter *DynamicConstraintIterator) hasUniqueConstraint(constraints []*structs.Constraint) bool { +func (iter *DynamicConstraintIterator) hasDistinctHostsConstraint(constraints []*structs.Constraint) bool { if constraints == nil { return false } for _, con := range constraints { - if con.Operand == "unique" { + if con.Operand == "distinctHosts" { return true } } @@ -214,13 +214,13 @@ func (iter *DynamicConstraintIterator) Next() *structs.Node { // Get the next option from the source option := iter.source.Next() - // Hot-path if the option is nil or there are no unique constraints. - if option == nil || (!iter.jobUnique && !iter.tgUnique) { + // Hot-path if the option is nil or there are no distinctHosts constraints. + if option == nil || (!iter.jobDistinctHosts && !iter.tgDistinctHosts) { return option } - if !iter.satisfiesUnique(option, iter.jobUnique) { - iter.ctx.Metrics().FilterNode(option, "unique") + if !iter.satisfiesDistinctHosts(option, iter.jobDistinctHosts) { + iter.ctx.Metrics().FilterNode(option, "distinctHosts") continue } @@ -228,9 +228,9 @@ func (iter *DynamicConstraintIterator) Next() *structs.Node { } } -// satisfiesUnique checks if the node satisfies a unique constraint either -// specified at the job level or the TaskGroup level. -func (iter *DynamicConstraintIterator) satisfiesUnique(option *structs.Node, job bool) bool { +// 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 { // Get the proposed allocations proposed, err := iter.ctx.ProposedAllocs(option.ID) if err != nil { @@ -244,9 +244,9 @@ func (iter *DynamicConstraintIterator) satisfiesUnique(option *structs.Node, job jobCollision := alloc.JobID == iter.job.ID taskCollision := alloc.TaskGroup == iter.tg.Name - // If the job has a unique 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. + // 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 tgInvalid := !job && jobCollision && taskCollision if jobInvalid || tgInvalid { @@ -370,7 +370,7 @@ func resolveConstraintTarget(target string, node *structs.Node) (interface{}, bo func checkConstraint(ctx Context, operand string, lVal, rVal interface{}) bool { // Check for constraints not handled by this iterator. switch operand { - case "unique": + case "distinctHosts": return true default: break diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index b7871b8d69d2..74eb99f03978 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -382,7 +382,7 @@ func TestCheckRegexpConstraint(t *testing.T) { } } -func TestDynamicConstraint_JobUnique_Feasible(t *testing.T) { +func TestDynamicConstraint_JobDistinctHosts(t *testing.T) { _, ctx := testContext(t) nodes := []*structs.Node{ mock.Node(), @@ -392,13 +392,13 @@ func TestDynamicConstraint_JobUnique_Feasible(t *testing.T) { } static := NewStaticIterator(ctx, nodes) - // Create a job with a unique constraint and two task groups. + // Create a job with a distinctHosts constraint and two task groups. tg1 := &structs.TaskGroup{Name: "bar"} tg2 := &structs.TaskGroup{Name: "baz"} job := &structs.Job{ ID: "foo", - Constraints: []*structs.Constraint{{Operand: "unique"}}, + Constraints: []*structs.Constraint{{Operand: "distinctHosts"}}, TaskGroups: []*structs.TaskGroup{tg1, tg2}, } @@ -420,7 +420,7 @@ func TestDynamicConstraint_JobUnique_Feasible(t *testing.T) { } } -func TestDynamicConstraint_JobUnique_Infeasible(t *testing.T) { +func TestDynamicConstraint_JobDistinctHosts_Infeasible(t *testing.T) { _, ctx := testContext(t) nodes := []*structs.Node{ mock.Node(), @@ -428,13 +428,13 @@ func TestDynamicConstraint_JobUnique_Infeasible(t *testing.T) { } static := NewStaticIterator(ctx, nodes) - // Create a job with a unique constraint and two task groups. + // Create a job with a distinctHosts constraint and two task groups. tg1 := &structs.TaskGroup{Name: "bar"} tg2 := &structs.TaskGroup{Name: "baz"} job := &structs.Job{ ID: "foo", - Constraints: []*structs.Constraint{{Operand: "unique"}}, + Constraints: []*structs.Constraint{{Operand: "distinctHosts"}}, TaskGroups: []*structs.TaskGroup{tg1, tg2}, } @@ -476,7 +476,7 @@ func TestDynamicConstraint_JobUnique_Infeasible(t *testing.T) { } } -func TestDynamicConstraint_JobUnique_InfeasibleCount(t *testing.T) { +func TestDynamicConstraint_JobDistinctHosts_InfeasibleCount(t *testing.T) { _, ctx := testContext(t) nodes := []*structs.Node{ mock.Node(), @@ -484,14 +484,14 @@ func TestDynamicConstraint_JobUnique_InfeasibleCount(t *testing.T) { } static := NewStaticIterator(ctx, nodes) - // Create a job with a unique constraint and three task groups. + // Create a job with a distinctHosts constraint and three task groups. tg1 := &structs.TaskGroup{Name: "bar"} tg2 := &structs.TaskGroup{Name: "baz"} tg3 := &structs.TaskGroup{Name: "bam"} job := &structs.Job{ ID: "foo", - Constraints: []*structs.Constraint{{Operand: "unique"}}, + Constraints: []*structs.Constraint{{Operand: "distinctHosts"}}, TaskGroups: []*structs.TaskGroup{tg1, tg2, tg3}, } @@ -506,7 +506,7 @@ func TestDynamicConstraint_JobUnique_InfeasibleCount(t *testing.T) { } } -func TestDynamicConstraint_TaskGroupUnique(t *testing.T) { +func TestDynamicConstraint_TaskGroupDistinctHosts(t *testing.T) { _, ctx := testContext(t) nodes := []*structs.Node{ mock.Node(), @@ -514,11 +514,11 @@ func TestDynamicConstraint_TaskGroupUnique(t *testing.T) { } static := NewStaticIterator(ctx, nodes) - // Create a task group with a unique constraint. + // Create a task group with a distinctHosts constraint. taskGroup := &structs.TaskGroup{ Name: "example", Constraints: []*structs.Constraint{ - {Operand: "unique"}, + {Operand: "distinctHosts"}, }, } diff --git a/website/source/docs/jobspec/index.html.md b/website/source/docs/jobspec/index.html.md index 540ca1be271f..3084b3b558ce 100644 --- a/website/source/docs/jobspec/index.html.md +++ b/website/source/docs/jobspec/index.html.md @@ -237,13 +237,17 @@ The `constraint` object supports the following keys: the attribute. This sets the operator to "regexp" and the `value` to the regular expression. -* `unique` - Unique accepts a boolean value and can be used to mark a Job or - a Task Group as requiring placement on unique nodes. If the `unique` - constraint is placed on a Job, all of it's Task Groups must be placed on - unique nodes. If the `unique` constraint is placed on a Task Group, then - multiple instances of that Task Group must be placed on unique nodes. This - sets the operator to "unique" if `unique` is set to "true". If set to "false", - the constraint is ignored as this is the default behavior. +* `distinctHosts` - `distinctHosts` accepts a boolean `true`. The default is + `false`. + + When `distinctHosts is `true` at the Job level, each instance of all Task + Groups specified in the job is placed on a separate host. + + When `distinctHosts` is `true` at the Task Group level with count > 1, each + instance of a Task Group is placed on a separate host. Different task groups in + the same job _may_ be co-scheduled. + + Tasks within a task group are always co-scheduled. Below is a table documenting the variables that can be interpreted: From bd10a6bb7695eee6bf38191ed5bfcd2cd12610b7 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 22 Oct 2015 17:41:35 -0700 Subject: [PATCH 07/11] add new test fixture --- jobspec/test-fixtures/distinctHosts-constraint.hcl | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 jobspec/test-fixtures/distinctHosts-constraint.hcl diff --git a/jobspec/test-fixtures/distinctHosts-constraint.hcl b/jobspec/test-fixtures/distinctHosts-constraint.hcl new file mode 100644 index 000000000000..7780c4b8cbbd --- /dev/null +++ b/jobspec/test-fixtures/distinctHosts-constraint.hcl @@ -0,0 +1,5 @@ +job "foo" { + constraint { + distinctHosts = "true" + } +} From ec78455e7f9afa0720af4c75a5dcf8dba35b7a32 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 23 Oct 2015 09:56:48 -0700 Subject: [PATCH 08/11] Fix markdown and log messages --- scheduler/feasible.go | 6 +++--- website/source/docs/jobspec/index.html.md | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 977fa034c0e8..7a96b66440e5 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -201,12 +201,12 @@ func (iter *DynamicConstraintIterator) hasDistinctHostsConstraint(constraints [] func (iter *DynamicConstraintIterator) Next() *structs.Node { if iter.job == nil { - iter.ctx.Logger().Printf("[ERR] sched.dynamic-constraint: job not set") + iter.ctx.Logger().Printf("[ERR] scheduler.dynamic-constraint: job not set") return nil } if iter.tg == nil { - iter.ctx.Logger().Printf("[ERR] sched.dynamic-constraint: task group not set") + iter.ctx.Logger().Printf("[ERR] scheduler.dynamic-constraint: task group not set") return nil } @@ -235,7 +235,7 @@ func (iter *DynamicConstraintIterator) satisfiesDistinctHosts(option *structs.No proposed, err := iter.ctx.ProposedAllocs(option.ID) if err != nil { iter.ctx.Logger().Printf( - "[ERR] sched.dynamic-constraint: failed to get proposed allocations: %v", err) + "[ERR] scheduler.dynamic-constraint: failed to get proposed allocations: %v", err) return false } diff --git a/website/source/docs/jobspec/index.html.md b/website/source/docs/jobspec/index.html.md index 3084b3b558ce..1cde52456063 100644 --- a/website/source/docs/jobspec/index.html.md +++ b/website/source/docs/jobspec/index.html.md @@ -240,7 +240,7 @@ The `constraint` object supports the following keys: * `distinctHosts` - `distinctHosts` accepts a boolean `true`. The default is `false`. - When `distinctHosts is `true` at the Job level, each instance of all Task + When `distinctHosts` is `true` at the Job level, each instance of all Task Groups specified in the job is placed on a separate host. When `distinctHosts` is `true` at the Task Group level with count > 1, each From cccef7ac84a975c0baa57a2f4e738ab499095e7b Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 26 Oct 2015 13:47:56 -0700 Subject: [PATCH 09/11] Constants for constraints and renaming to use undescore instead of camel --- jobspec/parse.go | 12 +++++------ jobspec/parse_test.go | 6 +++--- .../distinctHosts-constraint.hcl | 2 +- nomad/structs/structs.go | 10 ++++++++-- nomad/structs/structs_test.go | 4 ++-- scheduler/feasible.go | 20 +++++++++---------- scheduler/feasible_test.go | 20 +++++++++---------- website/source/docs/jobspec/index.html.md | 6 +++--- 8 files changed, 43 insertions(+), 37 deletions(-) diff --git a/jobspec/parse.go b/jobspec/parse.go index 18d1a4ff338c..9a98da5223ea 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -245,19 +245,19 @@ 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["distinctHosts"]; ok { + if value, ok := m[structs.ConstraintDistinctHosts]; ok { enabled, err := strconv.ParseBool(value.(string)) if err != nil { return err @@ -268,7 +268,7 @@ func parseConstraints(result *[]*structs.Constraint, obj *hclobj.Object) error { continue } - m["Operand"] = "distinctHosts" + m["Operand"] = structs.ConstraintDistinctHosts } // Build the constraint diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 07cb7c914b54..ccdc9989967a 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -165,7 +165,7 @@ func TestParse(t *testing.T) { Hard: true, LTarget: "$attr.kernel.version", RTarget: "~> 3.2", - Operand: "version", + Operand: structs.ConstraintVersion, }, }, }, @@ -185,7 +185,7 @@ func TestParse(t *testing.T) { Hard: true, LTarget: "$attr.kernel.version", RTarget: "[0-9.]+", - Operand: "regexp", + Operand: structs.ConstraintRegex, }, }, }, @@ -203,7 +203,7 @@ func TestParse(t *testing.T) { Constraints: []*structs.Constraint{ &structs.Constraint{ Hard: true, - Operand: "distinctHosts", + Operand: structs.ConstraintDistinctHosts, }, }, }, diff --git a/jobspec/test-fixtures/distinctHosts-constraint.hcl b/jobspec/test-fixtures/distinctHosts-constraint.hcl index 7780c4b8cbbd..cf6bc7bfc1f0 100644 --- a/jobspec/test-fixtures/distinctHosts-constraint.hcl +++ b/jobspec/test-fixtures/distinctHosts-constraint.hcl @@ -1,5 +1,5 @@ job "foo" { constraint { - distinctHosts = "true" + distinct_hosts = "true" } } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 8d58a985784d..c82dc88da086 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -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. @@ -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)) } diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 6df231a677d6..cabf83dfa6ef 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -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) @@ -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) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 7a96b66440e5..cbd87f03659c 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -152,7 +152,7 @@ func (iter *DriverIterator) hasDrivers(option *structs.Node) bool { // 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. +// effected by alloc placements. Examples are distinct_hosts and tenancy constraints. // This is used to filter on job and task group constraints. type DynamicConstraintIterator struct { ctx Context @@ -160,7 +160,7 @@ type DynamicConstraintIterator struct { tg *structs.TaskGroup job *structs.Job - // Store whether the Job or TaskGroup has a distinctHosts constraints so + // 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 @@ -192,7 +192,7 @@ func (iter *DynamicConstraintIterator) hasDistinctHostsConstraint(constraints [] } for _, con := range constraints { - if con.Operand == "distinctHosts" { + if con.Operand == structs.ConstraintDistinctHosts { return true } } @@ -214,13 +214,13 @@ func (iter *DynamicConstraintIterator) Next() *structs.Node { // Get the next option from the source option := iter.source.Next() - // Hot-path if the option is nil or there are no distinctHosts constraints. + // 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.jobDistinctHosts) { - iter.ctx.Metrics().FilterNode(option, "distinctHosts") + iter.ctx.Metrics().FilterNode(option, structs.ConstraintDistinctHosts) continue } @@ -228,7 +228,7 @@ func (iter *DynamicConstraintIterator) Next() *structs.Node { } } -// satisfiesDistinctHosts checks if the node satisfies a distinctHosts +// satisfiesDistinctHosts checks if the node satisfies a distinct_hosts // constraint either specified at the job level or the TaskGroup level. func (iter *DynamicConstraintIterator) satisfiesDistinctHosts(option *structs.Node, job bool) bool { // Get the proposed allocations @@ -244,7 +244,7 @@ func (iter *DynamicConstraintIterator) satisfiesDistinctHosts(option *structs.No jobCollision := alloc.JobID == iter.job.ID taskCollision := alloc.TaskGroup == iter.tg.Name - // If the job has a distinctHosts constraint we only need an alloc + // 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. jobInvalid := job && jobCollision @@ -370,7 +370,7 @@ func resolveConstraintTarget(target string, node *structs.Node) (interface{}, bo func checkConstraint(ctx Context, operand string, lVal, rVal interface{}) bool { // Check for constraints not handled by this iterator. switch operand { - case "distinctHosts": + case structs.ConstraintDistinctHosts: return true default: break @@ -383,9 +383,9 @@ func checkConstraint(ctx Context, operand string, lVal, rVal interface{}) bool { 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 diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index 74eb99f03978..a898baa9638a 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -248,12 +248,12 @@ func TestCheckConstraint(t *testing.T) { result: true, }, { - op: "version", + op: structs.ConstraintVersion, lVal: "1.2.3", rVal: "~> 1.0", result: true, }, { - op: "regexp", + op: structs.ConstraintRegex, lVal: "foobarbaz", rVal: "[\\w]+", result: true, }, @@ -392,13 +392,13 @@ func TestDynamicConstraint_JobDistinctHosts(t *testing.T) { } static := NewStaticIterator(ctx, nodes) - // Create a job with a distinctHosts constraint and two task groups. + // Create a job with a distinct_hosts constraint and two task groups. tg1 := &structs.TaskGroup{Name: "bar"} tg2 := &structs.TaskGroup{Name: "baz"} job := &structs.Job{ ID: "foo", - Constraints: []*structs.Constraint{{Operand: "distinctHosts"}}, + Constraints: []*structs.Constraint{{Operand: structs.ConstraintDistinctHosts}}, TaskGroups: []*structs.TaskGroup{tg1, tg2}, } @@ -428,13 +428,13 @@ func TestDynamicConstraint_JobDistinctHosts_Infeasible(t *testing.T) { } static := NewStaticIterator(ctx, nodes) - // Create a job with a distinctHosts constraint and two task groups. + // Create a job with a distinct_hosts constraint and two task groups. tg1 := &structs.TaskGroup{Name: "bar"} tg2 := &structs.TaskGroup{Name: "baz"} job := &structs.Job{ ID: "foo", - Constraints: []*structs.Constraint{{Operand: "distinctHosts"}}, + Constraints: []*structs.Constraint{{Operand: structs.ConstraintDistinctHosts}}, TaskGroups: []*structs.TaskGroup{tg1, tg2}, } @@ -484,14 +484,14 @@ func TestDynamicConstraint_JobDistinctHosts_InfeasibleCount(t *testing.T) { } static := NewStaticIterator(ctx, nodes) - // Create a job with a distinctHosts constraint and three task groups. + // Create a job with a distinct_hosts constraint and three task groups. tg1 := &structs.TaskGroup{Name: "bar"} tg2 := &structs.TaskGroup{Name: "baz"} tg3 := &structs.TaskGroup{Name: "bam"} job := &structs.Job{ ID: "foo", - Constraints: []*structs.Constraint{{Operand: "distinctHosts"}}, + Constraints: []*structs.Constraint{{Operand: structs.ConstraintDistinctHosts}}, TaskGroups: []*structs.TaskGroup{tg1, tg2, tg3}, } @@ -514,11 +514,11 @@ func TestDynamicConstraint_TaskGroupDistinctHosts(t *testing.T) { } static := NewStaticIterator(ctx, nodes) - // Create a task group with a distinctHosts constraint. + // Create a task group with a distinct_hosts constraint. taskGroup := &structs.TaskGroup{ Name: "example", Constraints: []*structs.Constraint{ - {Operand: "distinctHosts"}, + {Operand: structs.ConstraintDistinctHosts}, }, } diff --git a/website/source/docs/jobspec/index.html.md b/website/source/docs/jobspec/index.html.md index 1cde52456063..c16723e7cd47 100644 --- a/website/source/docs/jobspec/index.html.md +++ b/website/source/docs/jobspec/index.html.md @@ -237,13 +237,13 @@ The `constraint` object supports the following keys: the attribute. This sets the operator to "regexp" and the `value` to the regular expression. -* `distinctHosts` - `distinctHosts` accepts a boolean `true`. The default is +* `distinct_hosts` - `distinct_hosts` accepts a boolean `true`. The default is `false`. - When `distinctHosts` is `true` at the Job level, each instance of all Task + When `distinct_hosts` is `true` at the Job level, each instance of all Task Groups specified in the job is placed on a separate host. - When `distinctHosts` is `true` at the Task Group level with count > 1, each + When `distinct_hosts` is `true` at the Task Group level with count > 1, each instance of a Task Group is placed on a separate host. Different task groups in the same job _may_ be co-scheduled. From 302c9895092bd744a30a8e5bc1a27126bb2564b9 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 26 Oct 2015 14:01:32 -0700 Subject: [PATCH 10/11] Cleanup DynamicConstraintIterator --- scheduler/feasible.go | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index cbd87f03659c..a4d68fc0369e 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -187,10 +187,6 @@ func (iter *DynamicConstraintIterator) SetJob(job *structs.Job) { } func (iter *DynamicConstraintIterator) hasDistinctHostsConstraint(constraints []*structs.Constraint) bool { - if constraints == nil { - return false - } - for _, con := range constraints { if con.Operand == structs.ConstraintDistinctHosts { return true @@ -200,16 +196,6 @@ func (iter *DynamicConstraintIterator) hasDistinctHostsConstraint(constraints [] } func (iter *DynamicConstraintIterator) Next() *structs.Node { - if iter.job == nil { - iter.ctx.Logger().Printf("[ERR] scheduler.dynamic-constraint: job not set") - return nil - } - - if iter.tg == nil { - iter.ctx.Logger().Printf("[ERR] scheduler.dynamic-constraint: task group not set") - return nil - } - for { // Get the next option from the source option := iter.source.Next() @@ -219,7 +205,7 @@ func (iter *DynamicConstraintIterator) Next() *structs.Node { return option } - if !iter.satisfiesDistinctHosts(option, iter.jobDistinctHosts) { + if !iter.satisfiesDistinctHosts(option) { iter.ctx.Metrics().FilterNode(option, structs.ConstraintDistinctHosts) continue } @@ -230,7 +216,12 @@ func (iter *DynamicConstraintIterator) Next() *structs.Node { // satisfiesDistinctHosts checks if the node satisfies a distinct_hosts // constraint either specified at the job level or the TaskGroup level. -func (iter *DynamicConstraintIterator) satisfiesDistinctHosts(option *structs.Node, job bool) bool { +func (iter *DynamicConstraintIterator) 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) if err != nil { @@ -241,15 +232,12 @@ func (iter *DynamicConstraintIterator) satisfiesDistinctHosts(option *structs.No // 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 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. - jobInvalid := job && jobCollision - tgInvalid := !job && jobCollision && taskCollision - if jobInvalid || tgInvalid { + jobCollision := alloc.JobID == iter.job.ID + taskCollision := alloc.TaskGroup == iter.tg.Name + if iter.jobDistinctHosts && jobCollision || jobCollision && taskCollision { return false } } From 2ab5790b6fb76162eeada7d8c01e80c48c3d7512 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 26 Oct 2015 14:12:54 -0700 Subject: [PATCH 11/11] Rename Dynamic -> ProposedAllocConstraintIterator --- scheduler/feasible.go | 31 +++++++++++++++-------------- scheduler/feasible_test.go | 40 +++++++++++++++++++------------------- scheduler/stack.go | 31 +++++++++++++++-------------- 3 files changed, 52 insertions(+), 50 deletions(-) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index a4d68fc0369e..782d7c8751d0 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -150,11 +150,12 @@ func (iter *DriverIterator) hasDrivers(option *structs.Node) bool { return true } -// DynamicConstraintIterator is a FeasibleIterator which returns nodes that +// ProposedAllocConstraintIterator 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 distinct_hosts and tenancy constraints. -// This is used to filter on job and task group constraints. -type DynamicConstraintIterator struct { +// 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 @@ -166,27 +167,27 @@ type DynamicConstraintIterator struct { jobDistinctHosts bool } -// NewDynamicConstraintIterator creates a DynamicConstraintIterator from a -// source. -func NewDynamicConstraintIterator(ctx Context, source FeasibleIterator) *DynamicConstraintIterator { - iter := &DynamicConstraintIterator{ +// NewProposedAllocConstraintIterator creates a ProposedAllocConstraintIterator +// from a source. +func NewProposedAllocConstraintIterator(ctx Context, source FeasibleIterator) *ProposedAllocConstraintIterator { + iter := &ProposedAllocConstraintIterator{ ctx: ctx, source: source, } return iter } -func (iter *DynamicConstraintIterator) SetTaskGroup(tg *structs.TaskGroup) { +func (iter *ProposedAllocConstraintIterator) SetTaskGroup(tg *structs.TaskGroup) { iter.tg = tg iter.tgDistinctHosts = iter.hasDistinctHostsConstraint(tg.Constraints) } -func (iter *DynamicConstraintIterator) SetJob(job *structs.Job) { +func (iter *ProposedAllocConstraintIterator) SetJob(job *structs.Job) { iter.job = job iter.jobDistinctHosts = iter.hasDistinctHostsConstraint(job.Constraints) } -func (iter *DynamicConstraintIterator) hasDistinctHostsConstraint(constraints []*structs.Constraint) bool { +func (iter *ProposedAllocConstraintIterator) hasDistinctHostsConstraint(constraints []*structs.Constraint) bool { for _, con := range constraints { if con.Operand == structs.ConstraintDistinctHosts { return true @@ -195,13 +196,13 @@ func (iter *DynamicConstraintIterator) hasDistinctHostsConstraint(constraints [] return false } -func (iter *DynamicConstraintIterator) Next() *structs.Node { +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) { + if option == nil || !(iter.jobDistinctHosts || iter.tgDistinctHosts) { return option } @@ -216,7 +217,7 @@ func (iter *DynamicConstraintIterator) Next() *structs.Node { // satisfiesDistinctHosts checks if the node satisfies a distinct_hosts // constraint either specified at the job level or the TaskGroup level. -func (iter *DynamicConstraintIterator) satisfiesDistinctHosts(option *structs.Node) bool { +func (iter *ProposedAllocConstraintIterator) satisfiesDistinctHosts(option *structs.Node) bool { // Check if there is no constraint set. if !(iter.jobDistinctHosts || iter.tgDistinctHosts) { return true @@ -245,7 +246,7 @@ func (iter *DynamicConstraintIterator) satisfiesDistinctHosts(option *structs.No return true } -func (iter *DynamicConstraintIterator) Reset() { +func (iter *ProposedAllocConstraintIterator) Reset() { iter.source.Reset() } diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index a898baa9638a..b5aead21f830 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -382,7 +382,7 @@ func TestCheckRegexpConstraint(t *testing.T) { } } -func TestDynamicConstraint_JobDistinctHosts(t *testing.T) { +func TestProposedAllocConstraint_JobDistinctHosts(t *testing.T) { _, ctx := testContext(t) nodes := []*structs.Node{ mock.Node(), @@ -402,11 +402,11 @@ func TestDynamicConstraint_JobDistinctHosts(t *testing.T) { TaskGroups: []*structs.TaskGroup{tg1, tg2}, } - dynamic := NewDynamicConstraintIterator(ctx, static) - dynamic.SetTaskGroup(tg1) - dynamic.SetJob(job) + propsed := NewProposedAllocConstraintIterator(ctx, static) + propsed.SetTaskGroup(tg1) + propsed.SetJob(job) - out := collectFeasible(dynamic) + out := collectFeasible(propsed) if len(out) != 4 { t.Fatalf("Bad: %#v", out) } @@ -420,7 +420,7 @@ func TestDynamicConstraint_JobDistinctHosts(t *testing.T) { } } -func TestDynamicConstraint_JobDistinctHosts_Infeasible(t *testing.T) { +func TestProposedAllocConstraint_JobDistinctHosts_Infeasible(t *testing.T) { _, ctx := testContext(t) nodes := []*structs.Node{ mock.Node(), @@ -466,17 +466,17 @@ func TestDynamicConstraint_JobDistinctHosts_Infeasible(t *testing.T) { }, } - dynamic := NewDynamicConstraintIterator(ctx, static) - dynamic.SetTaskGroup(tg1) - dynamic.SetJob(job) + propsed := NewProposedAllocConstraintIterator(ctx, static) + propsed.SetTaskGroup(tg1) + propsed.SetJob(job) - out := collectFeasible(dynamic) + out := collectFeasible(propsed) if len(out) != 0 { t.Fatalf("Bad: %#v", out) } } -func TestDynamicConstraint_JobDistinctHosts_InfeasibleCount(t *testing.T) { +func TestProposedAllocConstraint_JobDistinctHosts_InfeasibleCount(t *testing.T) { _, ctx := testContext(t) nodes := []*structs.Node{ mock.Node(), @@ -495,18 +495,18 @@ func TestDynamicConstraint_JobDistinctHosts_InfeasibleCount(t *testing.T) { TaskGroups: []*structs.TaskGroup{tg1, tg2, tg3}, } - dynamic := NewDynamicConstraintIterator(ctx, static) - dynamic.SetTaskGroup(tg1) - dynamic.SetJob(job) + propsed := NewProposedAllocConstraintIterator(ctx, static) + propsed.SetTaskGroup(tg1) + propsed.SetJob(job) // It should not be able to place 3 tasks with only two nodes. - out := collectFeasible(dynamic) + out := collectFeasible(propsed) if len(out) != 2 { t.Fatalf("Bad: %#v", out) } } -func TestDynamicConstraint_TaskGroupDistinctHosts(t *testing.T) { +func TestProposedAllocConstraint_TaskGroupDistinctHosts(t *testing.T) { _, ctx := testContext(t) nodes := []*structs.Node{ mock.Node(), @@ -540,11 +540,11 @@ func TestDynamicConstraint_TaskGroupDistinctHosts(t *testing.T) { }, } - dynamic := NewDynamicConstraintIterator(ctx, static) - dynamic.SetTaskGroup(taskGroup) - dynamic.SetJob(&structs.Job{ID: "foo"}) + propsed := NewProposedAllocConstraintIterator(ctx, static) + propsed.SetTaskGroup(taskGroup) + propsed.SetJob(&structs.Job{ID: "foo"}) - out := collectFeasible(dynamic) + out := collectFeasible(propsed) if len(out) != 1 { t.Fatalf("Bad: %#v", out) } diff --git a/scheduler/stack.go b/scheduler/stack.go index 379e4ef22ccc..0a00fe686e11 100644 --- a/scheduler/stack.go +++ b/scheduler/stack.go @@ -35,17 +35,17 @@ type Stack interface { // GenericStack is the Stack used for the Generic scheduler. It is // designed to make better placement decisions at the cost of performance. type GenericStack struct { - batch bool - ctx Context - source *StaticIterator - jobConstraint *ConstraintIterator - taskGroupDrivers *DriverIterator - taskGroupConstraint *ConstraintIterator - dynamicConstraint *DynamicConstraintIterator - binPack *BinPackIterator - jobAntiAff *JobAntiAffinityIterator - limit *LimitIterator - maxScore *MaxScoreIterator + batch bool + ctx Context + source *StaticIterator + jobConstraint *ConstraintIterator + taskGroupDrivers *DriverIterator + taskGroupConstraint *ConstraintIterator + proposedAllocConstraint *ProposedAllocConstraintIterator + binPack *BinPackIterator + jobAntiAff *JobAntiAffinityIterator + limit *LimitIterator + maxScore *MaxScoreIterator } // NewGenericStack constructs a stack used for selecting service placements @@ -70,10 +70,11 @@ func NewGenericStack(batch bool, ctx Context) *GenericStack { // Filter on task group constraints second s.taskGroupConstraint = NewConstraintIterator(ctx, s.taskGroupDrivers, nil) - s.dynamicConstraint = NewDynamicConstraintIterator(ctx, s.taskGroupConstraint) + // Filter on constraints that are affected by propsed allocations. + s.proposedAllocConstraint = NewProposedAllocConstraintIterator(ctx, s.taskGroupConstraint) // Upgrade from feasible to rank iterator - rankSource := NewFeasibleRankIterator(ctx, s.dynamicConstraint) + rankSource := NewFeasibleRankIterator(ctx, s.proposedAllocConstraint) // Apply the bin packing, this depends on the resources needed // by a particular task group. Only enable eviction for the service @@ -122,7 +123,7 @@ func (s *GenericStack) SetNodes(baseNodes []*structs.Node) { func (s *GenericStack) SetJob(job *structs.Job) { s.jobConstraint.SetConstraints(job.Constraints) - s.dynamicConstraint.SetJob(job) + s.proposedAllocConstraint.SetJob(job) s.binPack.SetPriority(job.Priority) s.jobAntiAff.SetJob(job.ID) } @@ -139,7 +140,7 @@ func (s *GenericStack) Select(tg *structs.TaskGroup) (*RankedNode, *structs.Reso // Update the parameters of iterators s.taskGroupDrivers.SetDrivers(tgConstr.drivers) s.taskGroupConstraint.SetConstraints(tgConstr.constraints) - s.dynamicConstraint.SetTaskGroup(tg) + s.proposedAllocConstraint.SetTaskGroup(tg) s.binPack.SetTasks(tg.Tasks) // Find the node with the max score