Skip to content

Commit

Permalink
Merge pull request #351 from hashicorp/f-clean-constraints
Browse files Browse the repository at this point in the history
Remove weight and hard/soft fields from constraint
  • Loading branch information
dadgar committed Oct 28, 2015
2 parents dfbbbaf + c46447a commit 98185ca
Show file tree
Hide file tree
Showing 15 changed files with 16 additions and 104 deletions.
12 changes: 3 additions & 9 deletions api/compose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ func TestCompose(t *testing.T) {
task := NewTask("task1", "exec").
SetConfig("foo", "bar").
SetMeta("foo", "bar").
Constrain(HardConstraint("kernel.name", "=", "linux")).
Constrain(NewConstraint("kernel.name", "=", "linux")).
Require(&Resources{
CPU: 1250,
MemoryMB: 1024,
Expand All @@ -27,15 +27,15 @@ func TestCompose(t *testing.T) {

// Compose a task group
grp := NewTaskGroup("grp1", 2).
Constrain(HardConstraint("kernel.name", "=", "linux")).
Constrain(NewConstraint("kernel.name", "=", "linux")).
SetMeta("foo", "bar").
AddTask(task)

// Compose a job
job := NewServiceJob("job1", "myjob", "region1", 2).
SetMeta("foo", "bar").
AddDatacenter("dc1").
Constrain(HardConstraint("kernel.name", "=", "linux")).
Constrain(NewConstraint("kernel.name", "=", "linux")).
AddTaskGroup(grp)

// Check that the composed result looks correct
Expand All @@ -53,11 +53,9 @@ func TestCompose(t *testing.T) {
},
Constraints: []*Constraint{
&Constraint{
Hard: true,
LTarget: "kernel.name",
RTarget: "linux",
Operand: "=",
Weight: 0,
},
},
TaskGroups: []*TaskGroup{
Expand All @@ -66,11 +64,9 @@ func TestCompose(t *testing.T) {
Count: 2,
Constraints: []*Constraint{
&Constraint{
Hard: true,
LTarget: "kernel.name",
RTarget: "linux",
Operand: "=",
Weight: 0,
},
},
Tasks: []*Task{
Expand All @@ -95,11 +91,9 @@ func TestCompose(t *testing.T) {
},
Constraints: []*Constraint{
&Constraint{
Hard: true,
LTarget: "kernel.name",
RTarget: "linux",
Operand: "=",
Weight: 0,
},
},
Config: map[string]string{
Expand Down
20 changes: 2 additions & 18 deletions api/constraint.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,16 @@ package api

// Constraint is used to serialize a job placement constraint.
type Constraint struct {
Hard bool
LTarget string
RTarget string
Operand string
Weight int
}

// HardConstraint is used to create a new hard constraint.
func HardConstraint(left, operand, right string) *Constraint {
return constraint(left, operand, right, true, 0)
}

// SoftConstraint is used to create a new soft constraint. It
// takes an additional weight parameter to allow balancing
// multiple soft constraints amongst eachother.
func SoftConstraint(left, operand, right string, weight int) *Constraint {
return constraint(left, operand, right, false, weight)
}

// constraint generates a new job placement constraint.
func constraint(left, operand, right string, hard bool, weight int) *Constraint {
// NewConstraint generates a new job placement constraint.
func NewConstraint(left, operand, right string) *Constraint {
return &Constraint{
Hard: hard,
LTarget: left,
RTarget: right,
Operand: operand,
Weight: weight,
}
}
16 changes: 1 addition & 15 deletions api/constraint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,11 @@ import (
)

func TestCompose_Constraints(t *testing.T) {
c := HardConstraint("kernel.name", "=", "darwin")
c := NewConstraint("kernel.name", "=", "darwin")
expect := &Constraint{
Hard: true,
LTarget: "kernel.name",
RTarget: "darwin",
Operand: "=",
Weight: 0,
}
if !reflect.DeepEqual(c, expect) {
t.Fatalf("expect: %#v, got: %#v", expect, c)
}

c = SoftConstraint("memory.totalbytes", ">=", "250000000", 5)
expect = &Constraint{
Hard: false,
LTarget: "memory.totalbytes",
RTarget: "250000000",
Operand: ">=",
Weight: 5,
}
if !reflect.DeepEqual(c, expect) {
t.Fatalf("expect: %#v, got: %#v", expect, c)
Expand Down
8 changes: 2 additions & 6 deletions api/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ func TestJobs_Constrain(t *testing.T) {
job := &Job{Constraints: nil}

// Create and add a constraint
out := job.Constrain(HardConstraint("kernel.name", "=", "darwin"))
out := job.Constrain(NewConstraint("kernel.name", "=", "darwin"))
if n := len(job.Constraints); n != 1 {
t.Fatalf("expected 1 constraint, got: %d", n)
}
Expand All @@ -286,21 +286,17 @@ func TestJobs_Constrain(t *testing.T) {
}

// Adding another constraint preserves the original
job.Constrain(SoftConstraint("memory.totalbytes", ">=", "128000000", 2))
job.Constrain(NewConstraint("memory.totalbytes", ">=", "128000000"))
expect := []*Constraint{
&Constraint{
Hard: true,
LTarget: "kernel.name",
RTarget: "darwin",
Operand: "=",
Weight: 0,
},
&Constraint{
Hard: false,
LTarget: "memory.totalbytes",
RTarget: "128000000",
Operand: ">=",
Weight: 2,
},
}
if !reflect.DeepEqual(job.Constraints, expect) {
Expand Down
16 changes: 4 additions & 12 deletions api/tasks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestTaskGroup_Constrain(t *testing.T) {
grp := NewTaskGroup("grp1", 1)

// Add a constraint to the group
out := grp.Constrain(HardConstraint("kernel.name", "=", "darwin"))
out := grp.Constrain(NewConstraint("kernel.name", "=", "darwin"))
if n := len(grp.Constraints); n != 1 {
t.Fatalf("expected 1 constraint, got: %d", n)
}
Expand All @@ -31,21 +31,17 @@ func TestTaskGroup_Constrain(t *testing.T) {
}

// Add a second constraint
grp.Constrain(SoftConstraint("memory.totalbytes", ">=", "128000000", 2))
grp.Constrain(NewConstraint("memory.totalbytes", ">=", "128000000"))
expect := []*Constraint{
&Constraint{
Hard: true,
LTarget: "kernel.name",
RTarget: "darwin",
Operand: "=",
Weight: 0,
},
&Constraint{
Hard: false,
LTarget: "memory.totalbytes",
RTarget: "128000000",
Operand: ">=",
Weight: 2,
},
}
if !reflect.DeepEqual(grp.Constraints, expect) {
Expand Down Expand Up @@ -193,7 +189,7 @@ func TestTask_Constrain(t *testing.T) {
task := NewTask("task1", "exec")

// Add a constraint to the task
out := task.Constrain(HardConstraint("kernel.name", "=", "darwin"))
out := task.Constrain(NewConstraint("kernel.name", "=", "darwin"))
if n := len(task.Constraints); n != 1 {
t.Fatalf("expected 1 constraint, got: %d", n)
}
Expand All @@ -204,21 +200,17 @@ func TestTask_Constrain(t *testing.T) {
}

// Add a second constraint
task.Constrain(SoftConstraint("memory.totalbytes", ">=", "128000000", 2))
task.Constrain(NewConstraint("memory.totalbytes", ">=", "128000000"))
expect := []*Constraint{
&Constraint{
Hard: true,
LTarget: "kernel.name",
RTarget: "darwin",
Operand: "=",
Weight: 0,
},
&Constraint{
Hard: false,
LTarget: "memory.totalbytes",
RTarget: "128000000",
Operand: ">=",
Weight: 2,
},
}
if !reflect.DeepEqual(task.Constraints, expect) {
Expand Down
5 changes: 0 additions & 5 deletions jobspec/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,6 @@ func parseConstraints(result *[]*structs.Constraint, obj *hclobj.Object) error {
m["RTarget"] = m["value"]
m["Operand"] = m["operator"]

// Default constraint to being hard
if _, ok := m["hard"]; !ok {
m["hard"] = true
}

// If "version" is provided, set the operand
// to "version" and the value to the "RTarget"
if constraint, ok := m[structs.ConstraintVersion]; ok {
Expand Down
6 changes: 0 additions & 6 deletions jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ func TestParse(t *testing.T) {

Constraints: []*structs.Constraint{
&structs.Constraint{
Hard: true,
LTarget: "kernel.os",
RTarget: "windows",
Operand: "=",
Expand Down Expand Up @@ -68,7 +67,6 @@ func TestParse(t *testing.T) {
Count: 5,
Constraints: []*structs.Constraint{
&structs.Constraint{
Hard: true,
LTarget: "kernel.os",
RTarget: "linux",
Operand: "=",
Expand Down Expand Up @@ -114,7 +112,6 @@ func TestParse(t *testing.T) {
},
Constraints: []*structs.Constraint{
&structs.Constraint{
Hard: true,
LTarget: "kernel.arch",
RTarget: "amd64",
Operand: "=",
Expand Down Expand Up @@ -162,7 +159,6 @@ func TestParse(t *testing.T) {
Type: "service",
Constraints: []*structs.Constraint{
&structs.Constraint{
Hard: true,
LTarget: "$attr.kernel.version",
RTarget: "~> 3.2",
Operand: structs.ConstraintVersion,
Expand All @@ -182,7 +178,6 @@ func TestParse(t *testing.T) {
Type: "service",
Constraints: []*structs.Constraint{
&structs.Constraint{
Hard: true,
LTarget: "$attr.kernel.version",
RTarget: "[0-9.]+",
Operand: structs.ConstraintRegex,
Expand All @@ -202,7 +197,6 @@ func TestParse(t *testing.T) {
Type: "service",
Constraints: []*structs.Constraint{
&structs.Constraint{
Hard: true,
Operand: structs.ConstraintDistinctHosts,
},
},
Expand Down
2 changes: 0 additions & 2 deletions nomad/mock/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ func Job() *structs.Job {
Datacenters: []string{"dc1"},
Constraints: []*structs.Constraint{
&structs.Constraint{
Hard: true,
LTarget: "$attr.kernel.name",
RTarget: "linux",
Operand: "=",
Expand Down Expand Up @@ -123,7 +122,6 @@ func SystemJob() *structs.Job {
Datacenters: []string{"dc1"},
Constraints: []*structs.Constraint{
&structs.Constraint{
Hard: true,
LTarget: "$attr.kernel.name",
RTarget: "linux",
Operand: "=",
Expand Down
9 changes: 2 additions & 7 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1033,15 +1033,11 @@ const (
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.
// Constraints are used to restrict placement options.
type Constraint struct {
Hard bool // Hard or soft constraint
LTarget string // Left-hand target
RTarget string // Right-hand target
Operand string // Constraint operand (<=, <, =, !=, >, >=), contains, near
Weight int // Soft constraints can vary the weight
}

func (c *Constraint) String() string {
Expand Down Expand Up @@ -1185,8 +1181,7 @@ type AllocMetric struct {
// NodesEvaluated is the number of nodes that were evaluated
NodesEvaluated int

// NodesFiltered is the number of nodes filtered due to
// a hard constraint
// NodesFiltered is the number of nodes filtered due to a constraint
NodesFiltered int

// ClassFiltered is the number of nodes filtered by class
Expand Down
5 changes: 0 additions & 5 deletions scheduler/feasible.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,6 @@ func (iter *ConstraintIterator) meetsConstraints(option *structs.Node) bool {
}

func (iter *ConstraintIterator) meetsConstraint(constraint *structs.Constraint, option *structs.Node) bool {
// Only enforce hard constraints, soft constraints are used for ranking
if !constraint.Hard {
return true
}

// Resolve the targets
lVal, ok := resolveConstraintTarget(constraint.LTarget, option)
if !ok {
Expand Down
2 changes: 0 additions & 2 deletions scheduler/feasible_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,11 @@ func TestConstraintIterator(t *testing.T) {

constraints := []*structs.Constraint{
&structs.Constraint{
Hard: true,
Operand: "=",
LTarget: "$node.datacenter",
RTarget: "dc1",
},
&structs.Constraint{
Hard: true,
Operand: "is",
LTarget: "$attr.kernel.name",
RTarget: "linux",
Expand Down
4 changes: 2 additions & 2 deletions scheduler/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,9 +600,9 @@ func TestEvictAndPlace_LimitGreaterThanAllocs(t *testing.T) {
}

func TestTaskGroupConstraints(t *testing.T) {
constr := &structs.Constraint{Hard: true}
constr := &structs.Constraint{RTarget: "bar"}
constr2 := &structs.Constraint{LTarget: "foo"}
constr3 := &structs.Constraint{Weight: 10}
constr3 := &structs.Constraint{Operand: "<"}

tg := &structs.TaskGroup{
Name: "web",
Expand Down
6 changes: 0 additions & 6 deletions website/source/docs/http/alloc.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,9 @@ be specified using the `?region=` query parameter.
],
"Constraints": [
{
"Hard": false,
"LTarget": "kernel.os",
"RTarget": "windows",
"Operand": "=",
"Weight": 0
}
],
"TaskGroups": [
Expand All @@ -67,11 +65,9 @@ be specified using the `?region=` query parameter.
"Count": 5,
"Constraints": [
{
"Hard": false,
"LTarget": "kernel.os",
"RTarget": "linux",
"Operand": "=",
"Weight": 0
}
],
"Tasks": [
Expand Down Expand Up @@ -108,11 +104,9 @@ be specified using the `?region=` query parameter.
},
"Constraints": [
{
"Hard": false,
"LTarget": "kernel.arch",
"RTarget": "amd64",
"Operand": "=",
"Weight": 0
}
],
"Resources": {
Expand Down
Loading

0 comments on commit 98185ca

Please sign in to comment.