luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

core: remove internalization of affinity strings 

Basically the same as #10896 but with the Affinity struct.
Since we use reflect.DeepEquals for job comparison, there is
risk of false positives for changes due to a job struct with
memoized vs non-memoized strings.

Closes #10897
This commit is contained in:
Seth Hoenig 2021-07-15 15:11:36 -05:00
parent 996ea1fa46
commit ac5c83cafd
4 changed files with 23 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/10897.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
core: Fixed a bug where affinity memoization may cause planning problems
```

12
nomad/structs/diff_test.go
View File

@ -761,14 +761,12 @@ func TestJobDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "bar", LTarget: "bar",
RTarget: "bar", RTarget: "bar",
Operand: "bar", Operand: "bar",
Weight: 20, Weight: 20,
str: "bar",
}, },
}, },
}, },
@ -779,14 +777,12 @@ func TestJobDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "baz", LTarget: "baz",
RTarget: "baz", RTarget: "baz",
Operand: "baz", Operand: "baz",
Weight: 20, Weight: 20,
str: "baz",
}, },
}, },
}, },
@ -1558,14 +1554,12 @@ func TestTaskGroupDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "bar", LTarget: "bar",
RTarget: "bar", RTarget: "bar",
Operand: "bar", Operand: "bar",
Weight: 20, Weight: 20,
str: "bar",
}, },
}, },
}, },
@ -1576,14 +1570,12 @@ func TestTaskGroupDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "baz", LTarget: "baz",
RTarget: "baz", RTarget: "baz",
Operand: "baz", Operand: "baz",
Weight: 20, Weight: 20,
str: "baz",
}, },
}, },
}, },
@ -4192,14 +4184,12 @@ func TestTaskDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "bar", LTarget: "bar",
RTarget: "bar", RTarget: "bar",
Operand: "bar", Operand: "bar",
Weight: 20, Weight: 20,
str: "bar",
}, },
}, },
}, },
@ -4210,14 +4200,12 @@ func TestTaskDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "baz", LTarget: "baz",
RTarget: "baz", RTarget: "baz",
Operand: "baz", Operand: "baz",
Weight: 20, Weight: 20,
str: "baz",
}, },
}, },
}, },

18
nomad/structs/structs.go
View File

@ -8366,10 +8366,9 @@ type Affinity struct {
RTarget string // Right-hand target RTarget string // Right-hand target
Operand string // Affinity operand (<=, <, =, !=, >, >=), set_contains_all, set_contains_any Operand string // Affinity operand (<=, <, =, !=, >, >=), set_contains_all, set_contains_any
Weight int8 // Weight applied to nodes that match the affinity. Can be negative Weight int8 // Weight applied to nodes that match the affinity. Can be negative
str string // Memoized string
} }
// Equal checks if two affinities are equal // Equals checks if two affinities are equal.
func (a *Affinity) Equals(o *Affinity) bool { func (a *Affinity) Equals(o *Affinity) bool {
return a == o || return a == o ||
a.LTarget == o.LTarget && a.LTarget == o.LTarget &&
@ -8386,17 +8385,16 @@ func (a *Affinity) Copy() *Affinity {
if a == nil { if a == nil {
return nil return nil
} }
na := new(Affinity) return &Affinity{
*na = *a LTarget: a.LTarget,
return na RTarget: a.RTarget,
Operand: a.Operand,
Weight: a.Weight,
}
} }
func (a *Affinity) String() string { func (a *Affinity) String() string {
if a.str != "" { return fmt.Sprintf("%s %s %s %v", a.LTarget, a.Operand, a.RTarget, a.Weight)
return a.str
}
a.str = fmt.Sprintf("%s %s %s %v", a.LTarget, a.Operand, a.RTarget, a.Weight)
return a.str
} }
func (a *Affinity) Validate() error { func (a *Affinity) Validate() error {

18
nomad/structs/structs_test.go
View File

@ -284,6 +284,12 @@ func TestJob_SpecChanged(t *testing.T) {
Original: &Job{Constraints: []*Constraint{{"A", "B", "="}}}, Original: &Job{Constraints: []*Constraint{{"A", "B", "="}}},
New: &Job{Constraints: []*Constraint{{"A", "B", "="}}}, New: &Job{Constraints: []*Constraint{{"A", "B", "="}}},
}, },
{
Name: "With Affinities",
Changed: false,
Original: &Job{Affinities: []*Affinity{{"A", "B", "=", 1}}},
New: &Job{Affinities: []*Affinity{{"A", "B", "=", 1}}},
},
} }
for _, c := range cases { for _, c := range cases {
@ -814,14 +820,14 @@ func TestJob_PartEqual(t *testing.T) {
})) }))
as := &Affinities{ as := &Affinities{
&Affinity{"left0", "right0", "=", 0, ""}, &Affinity{"left0", "right0", "=", 0},
&Affinity{"left1", "right1", "=", 0, ""}, &Affinity{"left1", "right1", "=", 0},
&Affinity{"left2", "right2", "=", 0, ""}, &Affinity{"left2", "right2", "=", 0},
} }
require.True(t, as.Equals(&Affinities{ require.True(t, as.Equals(&Affinities{
&Affinity{"left0", "right0", "=", 0, ""}, &Affinity{"left0", "right0", "=", 0},
&Affinity{"left2", "right2", "=", 0, ""}, &Affinity{"left2", "right2", "=", 0},
&Affinity{"left1", "right1", "=", 0, ""}, &Affinity{"left1", "right1", "=", 0},
})) }))
} }