From d2442d4d6ed989b765aebd8fd6031699e95427a2 Mon Sep 17 00:00:00 2001 From: Jorn Wijnands Date: Thu, 5 Jan 2017 13:32:40 +0100 Subject: [PATCH 1/4] Add balance_datacenter constraint * Add Balance constraint * Fix constraint checker * fix node iteration * Add proposed allocs in the balancing * Add balancing on tg and job level * Add some more tests --- nomad/structs/structs.go | 1 + scheduler/feasible.go | 118 +++++++++++++++++++++- scheduler/feasible_test.go | 195 +++++++++++++++++++++++++++++++++++++ 3 files changed, 312 insertions(+), 2 deletions(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 9c3d40e2e7c..0095af5a53c 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -3186,6 +3186,7 @@ func (ta *TaskArtifact) Validate() error { } const ( + ConstraintBalance = "balance_datacenter" ConstraintDistinctHosts = "distinct_hosts" ConstraintRegex = "regexp" ConstraintVersion = "version" diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 2ca75ee17d4..89b146a2be3 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -7,7 +7,9 @@ import ( "strconv" "strings" - "github.com/hashicorp/go-version" + "math" + + version "github.com/hashicorp/go-version" "github.com/hashicorp/nomad/nomad/structs" ) @@ -157,6 +159,9 @@ type ProposedAllocConstraintIterator struct { // they don't have to be calculated every time Next() is called. tgDistinctHosts bool jobDistinctHosts bool + + tgBalance bool + jobBalance bool } // NewProposedAllocConstraintIterator creates a ProposedAllocConstraintIterator @@ -171,11 +176,13 @@ func NewProposedAllocConstraintIterator(ctx Context, source FeasibleIterator) *P func (iter *ProposedAllocConstraintIterator) SetTaskGroup(tg *structs.TaskGroup) { iter.tg = tg iter.tgDistinctHosts = iter.hasDistinctHostsConstraint(tg.Constraints) + iter.tgBalance = iter.hasBalanceConstraint(tg.Constraints) } func (iter *ProposedAllocConstraintIterator) SetJob(job *structs.Job) { iter.job = job iter.jobDistinctHosts = iter.hasDistinctHostsConstraint(job.Constraints) + iter.jobBalance = iter.hasBalanceConstraint(job.Constraints) } func (iter *ProposedAllocConstraintIterator) hasDistinctHostsConstraint(constraints []*structs.Constraint) bool { @@ -187,13 +194,23 @@ func (iter *ProposedAllocConstraintIterator) hasDistinctHostsConstraint(constrai return false } +func (iter *ProposedAllocConstraintIterator) hasBalanceConstraint(constraints []*structs.Constraint) bool { + for _, con := range constraints { + if con.Operand == structs.ConstraintBalance { + 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) { + if option == nil || !(iter.jobDistinctHosts || iter.tgDistinctHosts || iter.jobBalance || iter.tgBalance) { return option } @@ -202,6 +219,11 @@ func (iter *ProposedAllocConstraintIterator) Next() *structs.Node { continue } + if !iter.satisfiesBalance(option) { + iter.ctx.Metrics().FilterNode(option, structs.ConstraintBalance) + continue + } + return option } } @@ -237,6 +259,96 @@ func (iter *ProposedAllocConstraintIterator) satisfiesDistinctHosts(option *stru return true } +// satisfiesBalance checks if the allocation on this node would make the zones +// unbalanced, this implies a greater then 1 difference between the lowest, and the +// highest zone +func (iter *ProposedAllocConstraintIterator) satisfiesBalance(option *structs.Node) bool { + // Check if there is no constraint set. + if !(iter.jobBalance || iter.tgBalance) { + return true + } + + // fill the map with all the dc's in the selected datacenter + balanceMap := make(map[string]int) + + if len(iter.job.Datacenters) == 0 { + iter.ctx.Logger().Print("[ERR] Job needs at least 1 datacenter to use balance") + return false + } + + for _, dc := range iter.job.Datacenters { + balanceMap[dc] = 0 + } + + // get all the nodes + nodeIter, err := iter.ctx.State().Nodes() + if err != nil { + iter.ctx.Logger().Print("[ERR] Failed to get nodes") + return false + } + + for { + next := nodeIter.Next() + if next == nil { + break + } + + node := next.(*structs.Node) + + // current allocations + allocs, err := iter.ctx.State().AllocsByNode(node.ID) + if err != nil { + iter.ctx.Logger().Printf("[ERR] scheduler.dynamic-constraint: failed to get node allocations: %v", err) + return false + } + + // proposed allocations + proposed, err := iter.ctx.ProposedAllocs(node.ID) + if err != nil { + iter.ctx.Logger().Printf("[ERR] scheduler.dynamic-constraint: failed to get proposed allocations: %v", err) + return false + } + + for _, alloc := range proposed { + allocs = append(allocs, alloc) + } + + for _, alloc := range allocs { + jobCollision := alloc.JobID == iter.job.ID + taskCollision := alloc.TaskGroup == iter.tg.Name + + // skip jobs not in this job or taskgroup (for jobBalance/tgBalance) + if !(jobCollision && (iter.jobBalance || taskCollision)) { + continue + } + + // skip allocation with DesiredStatus other then running + if alloc.DesiredStatus != structs.AllocDesiredStatusRun { + continue + } + + balanceMap[node.Datacenter]++ + } + } + + min := math.MaxInt32 + + for _, n := range balanceMap { + if n < min { + min = n + } + } + + iter.ctx.Logger().Printf("[DEBUG] Allocs per DC: Current: %d (%s), Lowest: %d", balanceMap[option.Datacenter], option.Datacenter, min) + + // if the current DC is higher then the minium, the node is not eligible + if balanceMap[option.Datacenter] > min { + return false + } + + return true +} + func (iter *ProposedAllocConstraintIterator) Reset() { iter.source.Reset() } @@ -329,6 +441,8 @@ func checkConstraint(ctx Context, operand string, lVal, rVal interface{}) bool { switch operand { case structs.ConstraintDistinctHosts: return true + case structs.ConstraintBalance: + return true default: break } diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index e6fe430e47d..c5ff9416464 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -612,6 +612,201 @@ func TestProposedAllocConstraint_TaskGroupDistinctHosts(t *testing.T) { } } +func TestPropsedAllocConstraint_JobBalance(t *testing.T) { + store, ctx := testContext(t) + + n3 := mock.Node() + n3.Datacenter = "dc2" + + nodes := []*structs.Node{ + mock.Node(), + mock.Node(), + n3, + } + + for i, node := range nodes { + store.UpsertNode(uint64(i), node) + } + + static := NewStaticIterator(ctx, nodes) + + // Create a job with balance constraint + tg1 := &structs.TaskGroup{Name: "bar"} + tg2 := &structs.TaskGroup{Name: "baz"} + + job := &structs.Job{ + ID: "foo", + Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalance}}, + Datacenters: []string{"dc1", "dc2"}, + TaskGroups: []*structs.TaskGroup{tg1, tg2}, + } + + propsed := NewProposedAllocConstraintIterator(ctx, static) + propsed.SetTaskGroup(tg1) + propsed.SetJob(job) + + out := collectFeasible(propsed) + + if len(out) != 3 { + t.Fatalf("Bad: %#v", out) + } +} + +func TestPropsedAllocConstraint_JobBalance_WithRunningJobs(t *testing.T) { + store, ctx := testContext(t) + + n3 := mock.Node() + n3.Datacenter = "dc2" + + nodes := []*structs.Node{ + mock.Node(), + mock.Node(), + n3, + } + + for i, node := range nodes { + store.UpsertNode(uint64(i), node) + } + + static := NewStaticIterator(ctx, nodes) + + // Create a job with balance constraint + tg1 := &structs.TaskGroup{Name: "bar"} + tg2 := &structs.TaskGroup{Name: "baz"} + + job := &structs.Job{ + ID: "foo", + Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalance}}, + Datacenters: []string{"dc1", "dc2"}, + TaskGroups: []*structs.TaskGroup{tg1, tg2}, + } + + plan := ctx.Plan() + plan.NodeAllocation[nodes[0].ID] = []*structs.Allocation{ + &structs.Allocation{ + TaskGroup: tg1.Name, + JobID: job.ID, + ID: structs.GenerateUUID(), + DesiredStatus: structs.AllocDesiredStatusRun, + }, + + // This should be ignored since it's another Job + &structs.Allocation{ + TaskGroup: tg2.Name, + JobID: "ignore", + ID: structs.GenerateUUID(), + DesiredStatus: structs.AllocDesiredStatusRun, + }, + } + + propsed := NewProposedAllocConstraintIterator(ctx, static) + propsed.SetTaskGroup(tg1) + propsed.SetJob(job) + + out := collectFeasible(propsed) + + if len(out) != 1 { + t.Fatalf("Bad: %#v", out) + } +} + +func TestPropsedAllocConstraint_JobBalance_WithStoppedJobs(t *testing.T) { + store, ctx := testContext(t) + + n3 := mock.Node() + n3.Datacenter = "dc2" + + nodes := []*structs.Node{ + mock.Node(), + mock.Node(), + n3, + } + + for i, node := range nodes { + store.UpsertNode(uint64(i), node) + } + + static := NewStaticIterator(ctx, nodes) + + // Create a job with balance constraint + tg1 := &structs.TaskGroup{Name: "bar"} + tg2 := &structs.TaskGroup{Name: "baz"} + + job := &structs.Job{ + ID: "foo", + Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalance}}, + Datacenters: []string{"dc1", "dc2"}, + TaskGroups: []*structs.TaskGroup{tg1, tg2}, + } + + plan := ctx.Plan() + plan.NodeAllocation[nodes[0].ID] = []*structs.Allocation{ + &structs.Allocation{ + TaskGroup: tg1.Name, + JobID: job.ID, + ID: structs.GenerateUUID(), + DesiredStatus: structs.AllocDesiredStatusRun, + }, + } + + // this allocation should be ignored since it's no longer active + plan.NodeAllocation[n3.ID] = []*structs.Allocation{ + &structs.Allocation{ + TaskGroup: tg2.Name, + JobID: job.ID, + ID: structs.GenerateUUID(), + DesiredStatus: structs.AllocDesiredStatusStop, + }, + } + + propsed := NewProposedAllocConstraintIterator(ctx, static) + propsed.SetTaskGroup(tg1) + propsed.SetJob(job) + + out := collectFeasible(propsed) + + if len(out) != 1 { + t.Fatalf("Bad: %#v", out) + } +} + +func TestPropsedAllocConstraint_JobBalance_InfeasibleDC(t *testing.T) { + store, ctx := testContext(t) + + nodes := []*structs.Node{ + mock.Node(), + mock.Node(), + } + + for i, node := range nodes { + store.UpsertNode(uint64(i), node) + } + + static := NewStaticIterator(ctx, nodes) + + // Create a job with balance constraint + tg1 := &structs.TaskGroup{Name: "bar"} + tg2 := &structs.TaskGroup{Name: "baz"} + + job := &structs.Job{ + ID: "foo", + Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalance}}, + Datacenters: []string{"dc1", "dc2"}, + TaskGroups: []*structs.TaskGroup{tg1, tg2}, + } + + propsed := NewProposedAllocConstraintIterator(ctx, static) + propsed.SetTaskGroup(tg1) + propsed.SetJob(job) + + out := collectFeasible(propsed) + + // Should only be able to schedule 1 tg on 2 nodes + if len(out) != 2 { + t.Fatalf("Bad: %#v", out) + } +} + func collectFeasible(iter FeasibleIterator) (out []*structs.Node) { for { next := iter.Next() From 8e50242b39d12b2a970aaaeafbd7724c491a4f3b Mon Sep 17 00:00:00 2001 From: Jorn Wijnands Date: Mon, 9 Jan 2017 12:45:43 +0100 Subject: [PATCH 2/4] Add documentation --- .../docs/job-specification/constraint.html.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/website/source/docs/job-specification/constraint.html.md b/website/source/docs/job-specification/constraint.html.md index 2e4d1e2624b..7025056653c 100644 --- a/website/source/docs/job-specification/constraint.html.md +++ b/website/source/docs/job-specification/constraint.html.md @@ -114,6 +114,19 @@ constraint { } ``` +- `"balance_datacenter"` - Instructs the scheduler to force an equal spread across + all datacenters specified in the Job. When specified as a job constraint, it + applies to all groups in the job. When specified as a group constraint, the + effect is constrained to that group. Note that the `attribute` parameter should + be omitted when using this constraint. + + ```hcl + constraint { + operator = "balance_datacenter" + value = "true" + } + ``` + - `"regexp"` - Specifies a regular expression constraint against the attribute. The syntax of the regular expressions accepted is the same general syntax used by Perl, Python, and many other languages. More precisely, it is the syntax From 8f10b435b46f9375b5276701475fef8f0e5cbbdb Mon Sep 17 00:00:00 2001 From: Jorn Wijnands Date: Wed, 11 Jan 2017 11:20:30 +0100 Subject: [PATCH 3/4] Fix duplicate allocations --- scheduler/feasible.go | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 89b146a2be3..85fe86602e7 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -260,7 +260,7 @@ func (iter *ProposedAllocConstraintIterator) satisfiesDistinctHosts(option *stru } // satisfiesBalance checks if the allocation on this node would make the zones -// unbalanced, this implies a greater then 1 difference between the lowest, and the +// unbalanced, this implies a greater than 1 difference between the lowest, and the // highest zone func (iter *ProposedAllocConstraintIterator) satisfiesBalance(option *structs.Node) bool { // Check if there is no constraint set. @@ -268,14 +268,14 @@ func (iter *ProposedAllocConstraintIterator) satisfiesBalance(option *structs.No return true } - // fill the map with all the dc's in the selected datacenter - balanceMap := make(map[string]int) - if len(iter.job.Datacenters) == 0 { iter.ctx.Logger().Print("[ERR] Job needs at least 1 datacenter to use balance") return false } + // fill the map with all the dc's in the selected datacenter + balanceMap := make(map[string]int) + for _, dc := range iter.job.Datacenters { balanceMap[dc] = 0 } @@ -295,13 +295,6 @@ func (iter *ProposedAllocConstraintIterator) satisfiesBalance(option *structs.No node := next.(*structs.Node) - // current allocations - allocs, err := iter.ctx.State().AllocsByNode(node.ID) - if err != nil { - iter.ctx.Logger().Printf("[ERR] scheduler.dynamic-constraint: failed to get node allocations: %v", err) - return false - } - // proposed allocations proposed, err := iter.ctx.ProposedAllocs(node.ID) if err != nil { @@ -310,10 +303,6 @@ func (iter *ProposedAllocConstraintIterator) satisfiesBalance(option *structs.No } for _, alloc := range proposed { - allocs = append(allocs, alloc) - } - - for _, alloc := range allocs { jobCollision := alloc.JobID == iter.job.ID taskCollision := alloc.TaskGroup == iter.tg.Name @@ -322,7 +311,7 @@ func (iter *ProposedAllocConstraintIterator) satisfiesBalance(option *structs.No continue } - // skip allocation with DesiredStatus other then running + // skip allocation with DesiredStatus other than running if alloc.DesiredStatus != structs.AllocDesiredStatusRun { continue } @@ -339,9 +328,7 @@ func (iter *ProposedAllocConstraintIterator) satisfiesBalance(option *structs.No } } - iter.ctx.Logger().Printf("[DEBUG] Allocs per DC: Current: %d (%s), Lowest: %d", balanceMap[option.Datacenter], option.Datacenter, min) - - // if the current DC is higher then the minium, the node is not eligible + // if the current DC is higher than the minium, the node is not eligible if balanceMap[option.Datacenter] > min { return false } From ede929f10334ebea6c55dbf5aa07d1b82b84e0c5 Mon Sep 17 00:00:00 2001 From: Jorn Wijnands Date: Thu, 23 Feb 2017 12:56:18 +0100 Subject: [PATCH 4/4] Change the balance_datacenters constraint * to rely on currently allocated allocations rather then on the RR nature of the iterator. The allocations per datacenter is now calculated deterministically based on the number of allocations and on the number of datacenters. --- nomad/structs/structs.go | 10 +- scheduler/feasible.go | 107 ++++++++++-------- scheduler/feasible_test.go | 48 ++++---- scheduler/util.go | 23 ++++ .../docs/job-specification/constraint.html.md | 13 ++- 5 files changed, 117 insertions(+), 84 deletions(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 0095af5a53c..eae02d5edbb 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -3186,11 +3186,11 @@ func (ta *TaskArtifact) Validate() error { } const ( - ConstraintBalance = "balance_datacenter" - ConstraintDistinctHosts = "distinct_hosts" - ConstraintRegex = "regexp" - ConstraintVersion = "version" - ConstraintSetContains = "set_contains" + ConstraintBalanceDatacenters = "balance_datacenters" + ConstraintDistinctHosts = "distinct_hosts" + ConstraintRegex = "regexp" + ConstraintVersion = "version" + ConstraintSetContains = "set_contains" ) // Constraints are used to restrict placement options. diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 85fe86602e7..461aba6068c 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -7,8 +7,7 @@ import ( "strconv" "strings" - "math" - + memdb "github.com/hashicorp/go-memdb" version "github.com/hashicorp/go-version" "github.com/hashicorp/nomad/nomad/structs" ) @@ -160,8 +159,10 @@ type ProposedAllocConstraintIterator struct { tgDistinctHosts bool jobDistinctHosts bool - tgBalance bool - jobBalance bool + // Store wherther the Job or TaskGroup has a balance_datacenters constraint so + // they don't have to be calculated every time Next() is called. + tgBalanceDatacenters bool + jobBalanceDatacenters bool } // NewProposedAllocConstraintIterator creates a ProposedAllocConstraintIterator @@ -176,13 +177,13 @@ func NewProposedAllocConstraintIterator(ctx Context, source FeasibleIterator) *P func (iter *ProposedAllocConstraintIterator) SetTaskGroup(tg *structs.TaskGroup) { iter.tg = tg iter.tgDistinctHosts = iter.hasDistinctHostsConstraint(tg.Constraints) - iter.tgBalance = iter.hasBalanceConstraint(tg.Constraints) + iter.tgBalanceDatacenters = iter.hasBalanceDatacentersConstraint(tg.Constraints) } func (iter *ProposedAllocConstraintIterator) SetJob(job *structs.Job) { iter.job = job iter.jobDistinctHosts = iter.hasDistinctHostsConstraint(job.Constraints) - iter.jobBalance = iter.hasBalanceConstraint(job.Constraints) + iter.jobBalanceDatacenters = iter.hasBalanceDatacentersConstraint(job.Constraints) } func (iter *ProposedAllocConstraintIterator) hasDistinctHostsConstraint(constraints []*structs.Constraint) bool { @@ -194,9 +195,9 @@ func (iter *ProposedAllocConstraintIterator) hasDistinctHostsConstraint(constrai return false } -func (iter *ProposedAllocConstraintIterator) hasBalanceConstraint(constraints []*structs.Constraint) bool { +func (iter *ProposedAllocConstraintIterator) hasBalanceDatacentersConstraint(constraints []*structs.Constraint) bool { for _, con := range constraints { - if con.Operand == structs.ConstraintBalance { + if con.Operand == structs.ConstraintBalanceDatacenters { return true } } @@ -209,8 +210,8 @@ func (iter *ProposedAllocConstraintIterator) 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 distinct_hosts constraints. - if option == nil || !(iter.jobDistinctHosts || iter.tgDistinctHosts || iter.jobBalance || iter.tgBalance) { + // Hot-path if the option is nil or there are no distinct_hosts or balance_datacenters constraints. + if option == nil || !(iter.jobDistinctHosts || iter.tgDistinctHosts || iter.jobBalanceDatacenters || iter.tgBalanceDatacenters) { return option } @@ -219,8 +220,8 @@ func (iter *ProposedAllocConstraintIterator) Next() *structs.Node { continue } - if !iter.satisfiesBalance(option) { - iter.ctx.Metrics().FilterNode(option, structs.ConstraintBalance) + if !iter.satisfiesBalanceDatacenters(option) { + iter.ctx.Metrics().FilterNode(option, structs.ConstraintBalanceDatacenters) continue } @@ -262,31 +263,29 @@ func (iter *ProposedAllocConstraintIterator) satisfiesDistinctHosts(option *stru // satisfiesBalance checks if the allocation on this node would make the zones // unbalanced, this implies a greater than 1 difference between the lowest, and the // highest zone -func (iter *ProposedAllocConstraintIterator) satisfiesBalance(option *structs.Node) bool { +func (iter *ProposedAllocConstraintIterator) satisfiesBalanceDatacenters(option *structs.Node) bool { // Check if there is no constraint set. - if !(iter.jobBalance || iter.tgBalance) { + if !(iter.jobBalanceDatacenters || iter.tgBalanceDatacenters) { return true } - if len(iter.job.Datacenters) == 0 { - iter.ctx.Logger().Print("[ERR] Job needs at least 1 datacenter to use balance") + if len(iter.job.Datacenters) < 1 { + iter.ctx.Logger().Print("[ERR] Job needs at least 2 datacenter to use balance") return false } - - // fill the map with all the dc's in the selected datacenter - balanceMap := make(map[string]int) - - for _, dc := range iter.job.Datacenters { - balanceMap[dc] = 0 - } + var allocationsInCurrentDatacenter int // get all the nodes - nodeIter, err := iter.ctx.State().Nodes() + ws := memdb.NewWatchSet() + nodeIter, err := iter.ctx.State().Nodes(ws) if err != nil { iter.ctx.Logger().Print("[ERR] Failed to get nodes") return false } + // Fetch all the proposed allocations for all the nodes in the current datacenter + var proposed []*structs.Allocation + for { next := nodeIter.Next() if next == nil { @@ -295,45 +294,59 @@ func (iter *ProposedAllocConstraintIterator) satisfiesBalance(option *structs.No node := next.(*structs.Node) + // we only care about the nodes which have the same datacenter as the current node (the option) + if node.Datacenter != option.Datacenter { + continue + } + // proposed allocations - proposed, err := iter.ctx.ProposedAllocs(node.ID) + nodeProposed, err := iter.ctx.ProposedAllocs(node.ID) if err != nil { iter.ctx.Logger().Printf("[ERR] scheduler.dynamic-constraint: failed to get proposed allocations: %v", err) return false } - for _, alloc := range proposed { - jobCollision := alloc.JobID == iter.job.ID - taskCollision := alloc.TaskGroup == iter.tg.Name + proposed = append(proposed, nodeProposed...) + } - // skip jobs not in this job or taskgroup (for jobBalance/tgBalance) - if !(jobCollision && (iter.jobBalance || taskCollision)) { - continue - } + for _, alloc := range proposed { + // If the job has a balance_datacenters 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 + tgCollision := alloc.TaskGroup == iter.tg.Name && jobCollision - // skip allocation with DesiredStatus other than running - if alloc.DesiredStatus != structs.AllocDesiredStatusRun { - continue - } + if !((iter.jobBalanceDatacenters && jobCollision) || tgCollision) { + continue + } - balanceMap[node.Datacenter]++ + // skip allocation with DesiredStatus other than running + if alloc.DesiredStatus != structs.AllocDesiredStatusRun { + continue } - } - min := math.MaxInt32 + allocationsInCurrentDatacenter++ + } - for _, n := range balanceMap { - if n < min { - min = n + // number of allocations per datacenter depends on the constraint location + // if the constraint is set on the job level, it is a sum of all the allocations + // in all the TaskGroups. If it is set on TaskGroup level, it's only for the Count + // of the (current) taskgroup + var allocations int + if iter.tgBalanceDatacenters { + allocations = iter.tg.Count + } else { + for _, tg := range iter.job.TaskGroups { + allocations += tg.Count } } - // if the current DC is higher than the minium, the node is not eligible - if balanceMap[option.Datacenter] > min { - return false + allocationsPerDatacenter := allocationsPerDatacenter(allocations, iter.job.Datacenters) + if c, ok := allocationsPerDatacenter[option.Datacenter]; ok && allocationsInCurrentDatacenter < c { + return true } - return true + return false } func (iter *ProposedAllocConstraintIterator) Reset() { @@ -428,7 +441,7 @@ func checkConstraint(ctx Context, operand string, lVal, rVal interface{}) bool { switch operand { case structs.ConstraintDistinctHosts: return true - case structs.ConstraintBalance: + case structs.ConstraintBalanceDatacenters: return true default: break diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index c5ff9416464..91016265f89 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -612,7 +612,7 @@ func TestProposedAllocConstraint_TaskGroupDistinctHosts(t *testing.T) { } } -func TestPropsedAllocConstraint_JobBalance(t *testing.T) { +func TestPropsedAllocConstraint_JobBalanceDatacenters(t *testing.T) { store, ctx := testContext(t) n3 := mock.Node() @@ -631,12 +631,12 @@ func TestPropsedAllocConstraint_JobBalance(t *testing.T) { static := NewStaticIterator(ctx, nodes) // Create a job with balance constraint - tg1 := &structs.TaskGroup{Name: "bar"} - tg2 := &structs.TaskGroup{Name: "baz"} + tg1 := &structs.TaskGroup{Name: "bar", Count: 1} + tg2 := &structs.TaskGroup{Name: "baz", Count: 1} job := &structs.Job{ ID: "foo", - Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalance}}, + Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalanceDatacenters}}, Datacenters: []string{"dc1", "dc2"}, TaskGroups: []*structs.TaskGroup{tg1, tg2}, } @@ -652,7 +652,7 @@ func TestPropsedAllocConstraint_JobBalance(t *testing.T) { } } -func TestPropsedAllocConstraint_JobBalance_WithRunningJobs(t *testing.T) { +func TestPropsedAllocConstraint_JobBalanceDatacenters_WithRunningJobs(t *testing.T) { store, ctx := testContext(t) n3 := mock.Node() @@ -671,12 +671,12 @@ func TestPropsedAllocConstraint_JobBalance_WithRunningJobs(t *testing.T) { static := NewStaticIterator(ctx, nodes) // Create a job with balance constraint - tg1 := &structs.TaskGroup{Name: "bar"} - tg2 := &structs.TaskGroup{Name: "baz"} + tg1 := &structs.TaskGroup{Name: "bar", Count: 1} + tg2 := &structs.TaskGroup{Name: "baz", Count: 1} job := &structs.Job{ ID: "foo", - Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalance}}, + Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalanceDatacenters}}, Datacenters: []string{"dc1", "dc2"}, TaskGroups: []*structs.TaskGroup{tg1, tg2}, } @@ -708,9 +708,14 @@ func TestPropsedAllocConstraint_JobBalance_WithRunningJobs(t *testing.T) { if len(out) != 1 { t.Fatalf("Bad: %#v", out) } + + // since there is an allocation on dc1, the yielded node should be on dc2 + if out[0].Datacenter != "dc2" { + t.Fatalf("Bad: proposed node is on the wrong datacenter, expected: dc1; got: %s", out[0].Datacenter) + } } -func TestPropsedAllocConstraint_JobBalance_WithStoppedJobs(t *testing.T) { +func TestPropsedAllocConstraint_JobBalanceDatacenters_WithStoppedJobs(t *testing.T) { store, ctx := testContext(t) n3 := mock.Node() @@ -729,26 +734,17 @@ func TestPropsedAllocConstraint_JobBalance_WithStoppedJobs(t *testing.T) { static := NewStaticIterator(ctx, nodes) // Create a job with balance constraint - tg1 := &structs.TaskGroup{Name: "bar"} - tg2 := &structs.TaskGroup{Name: "baz"} + tg1 := &structs.TaskGroup{Name: "bar", Count: 1} + tg2 := &structs.TaskGroup{Name: "baz", Count: 1} job := &structs.Job{ ID: "foo", - Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalance}}, + Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalanceDatacenters}}, Datacenters: []string{"dc1", "dc2"}, TaskGroups: []*structs.TaskGroup{tg1, tg2}, } plan := ctx.Plan() - plan.NodeAllocation[nodes[0].ID] = []*structs.Allocation{ - &structs.Allocation{ - TaskGroup: tg1.Name, - JobID: job.ID, - ID: structs.GenerateUUID(), - DesiredStatus: structs.AllocDesiredStatusRun, - }, - } - // this allocation should be ignored since it's no longer active plan.NodeAllocation[n3.ID] = []*structs.Allocation{ &structs.Allocation{ @@ -765,12 +761,12 @@ func TestPropsedAllocConstraint_JobBalance_WithStoppedJobs(t *testing.T) { out := collectFeasible(propsed) - if len(out) != 1 { + if len(out) != 3 { t.Fatalf("Bad: %#v", out) } } -func TestPropsedAllocConstraint_JobBalance_InfeasibleDC(t *testing.T) { +func TestPropsedAllocConstraint_JobBalanceDatacenters_InfeasibleDC(t *testing.T) { store, ctx := testContext(t) nodes := []*structs.Node{ @@ -785,12 +781,12 @@ func TestPropsedAllocConstraint_JobBalance_InfeasibleDC(t *testing.T) { static := NewStaticIterator(ctx, nodes) // Create a job with balance constraint - tg1 := &structs.TaskGroup{Name: "bar"} - tg2 := &structs.TaskGroup{Name: "baz"} + tg1 := &structs.TaskGroup{Name: "bar", Count: 1} + tg2 := &structs.TaskGroup{Name: "baz", Count: 1} job := &structs.Job{ ID: "foo", - Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalance}}, + Constraints: []*structs.Constraint{{Operand: structs.ConstraintBalanceDatacenters}}, Datacenters: []string{"dc1", "dc2"}, TaskGroups: []*structs.TaskGroup{tg1, tg2}, } diff --git a/scheduler/util.go b/scheduler/util.go index f305134cdec..463fa4f2a28 100644 --- a/scheduler/util.go +++ b/scheduler/util.go @@ -721,3 +721,26 @@ func updateNonTerminalAllocsToLost(plan *structs.Plan, tainted map[string]*struc } } } + +// allocationsPerDatacenter returns a partition with an equal spread of the number of allocations per datacenter +// the allocations will be equally spread (allocations/len(datacenters)) and the remaining x allocations will be +// placed on the first x datacenters. +func allocationsPerDatacenter(allocations int, datacenters []string) map[string]int { + dcCount := len(datacenters) + result := make(map[string]int, dcCount) + remainder := allocations % dcCount + + for _, dc := range datacenters { + // integer division of the number of allocations by the number of datacenters + // this will leave a "remainder", which we will add as long as there is some left + result[dc] = allocations / dcCount + + // divide the remainder across the first x datacenters until the remainder equals zero + if remainder > 0 { + result[dc]++ + remainder-- + } + } + + return result +} diff --git a/website/source/docs/job-specification/constraint.html.md b/website/source/docs/job-specification/constraint.html.md index 7025056653c..712445a1612 100644 --- a/website/source/docs/job-specification/constraint.html.md +++ b/website/source/docs/job-specification/constraint.html.md @@ -114,15 +114,16 @@ constraint { } ``` -- `"balance_datacenter"` - Instructs the scheduler to force an equal spread across - all datacenters specified in the Job. When specified as a job constraint, it - applies to all groups in the job. When specified as a group constraint, the - effect is constrained to that group. Note that the `attribute` parameter should - be omitted when using this constraint. +- `"balance_datacenters"` - Instructs the scheduler to force an equal spread across + all datacenters specified in the job's [datacenter + list](https://www.nomadproject.io/docs/job-specification/job.html#datacenters). + When specified as a job constraint, it applies to all groups in the job. When + specified as a group constraint, the effect is constrained to that group. Note + that the `attribute` parameter should be omitted when using this constraint. ```hcl constraint { - operator = "balance_datacenter" + operator = "balance_datacenters" value = "true" } ```