From ac5c83cafdcbf5592a842d0886bbf64cd7c44ce8 Mon Sep 17 00:00:00 2001
From: Seth Hoenig <shoenig@hashicorp.com>
Date: Thu, 15 Jul 2021 15:11:36 -0500
Subject: [PATCH] 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
---
 .changelog/10897.txt          |  3 +++
 nomad/structs/diff_test.go    | 12 ------------
 nomad/structs/structs.go      | 18 ++++++++----------
 nomad/structs/structs_test.go | 18 ++++++++++++------
 4 files changed, 23 insertions(+), 28 deletions(-)
 create mode 100644 .changelog/10897.txt

diff --git a/.changelog/10897.txt b/.changelog/10897.txt
new file mode 100644
index 000000000..685199603
--- /dev/null
+++ b/.changelog/10897.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+core: Fixed a bug where affinity memoization may cause planning problems
+```
diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go
index 956282b5a..6eabcdc8b 100644
--- a/nomad/structs/diff_test.go
+++ b/nomad/structs/diff_test.go
@@ -761,14 +761,12 @@ func TestJobDiff(t *testing.T) {
 						RTarget: "foo",
 						Operand: "foo",
 						Weight:  20,
-						str:     "foo",
 					},
 					{
 						LTarget: "bar",
 						RTarget: "bar",
 						Operand: "bar",
 						Weight:  20,
-						str:     "bar",
 					},
 				},
 			},
@@ -779,14 +777,12 @@ func TestJobDiff(t *testing.T) {
 						RTarget: "foo",
 						Operand: "foo",
 						Weight:  20,
-						str:     "foo",
 					},
 					{
 						LTarget: "baz",
 						RTarget: "baz",
 						Operand: "baz",
 						Weight:  20,
-						str:     "baz",
 					},
 				},
 			},
@@ -1558,14 +1554,12 @@ func TestTaskGroupDiff(t *testing.T) {
 						RTarget: "foo",
 						Operand: "foo",
 						Weight:  20,
-						str:     "foo",
 					},
 					{
 						LTarget: "bar",
 						RTarget: "bar",
 						Operand: "bar",
 						Weight:  20,
-						str:     "bar",
 					},
 				},
 			},
@@ -1576,14 +1570,12 @@ func TestTaskGroupDiff(t *testing.T) {
 						RTarget: "foo",
 						Operand: "foo",
 						Weight:  20,
-						str:     "foo",
 					},
 					{
 						LTarget: "baz",
 						RTarget: "baz",
 						Operand: "baz",
 						Weight:  20,
-						str:     "baz",
 					},
 				},
 			},
@@ -4192,14 +4184,12 @@ func TestTaskDiff(t *testing.T) {
 						RTarget: "foo",
 						Operand: "foo",
 						Weight:  20,
-						str:     "foo",
 					},
 					{
 						LTarget: "bar",
 						RTarget: "bar",
 						Operand: "bar",
 						Weight:  20,
-						str:     "bar",
 					},
 				},
 			},
@@ -4210,14 +4200,12 @@ func TestTaskDiff(t *testing.T) {
 						RTarget: "foo",
 						Operand: "foo",
 						Weight:  20,
-						str:     "foo",
 					},
 					{
 						LTarget: "baz",
 						RTarget: "baz",
 						Operand: "baz",
 						Weight:  20,
-						str:     "baz",
 					},
 				},
 			},
diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
index 7aa67fcb3..71793496c 100644
--- a/nomad/structs/structs.go
+++ b/nomad/structs/structs.go
@@ -8366,10 +8366,9 @@ type Affinity struct {
 	RTarget string // Right-hand target
 	Operand string // Affinity operand (<=, <, =, !=, >, >=), set_contains_all, set_contains_any
 	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 {
 	return a == o ||
 		a.LTarget == o.LTarget &&
@@ -8386,17 +8385,16 @@ func (a *Affinity) Copy() *Affinity {
 	if a == nil {
 		return nil
 	}
-	na := new(Affinity)
-	*na = *a
-	return na
+	return &Affinity{
+		LTarget: a.LTarget,
+		RTarget: a.RTarget,
+		Operand: a.Operand,
+		Weight:  a.Weight,
+	}
 }
 
 func (a *Affinity) String() string {
-	if a.str != "" {
-		return a.str
-	}
-	a.str = fmt.Sprintf("%s %s %s %v", a.LTarget, a.Operand, a.RTarget, a.Weight)
-	return a.str
+	return fmt.Sprintf("%s %s %s %v", a.LTarget, a.Operand, a.RTarget, a.Weight)
 }
 
 func (a *Affinity) Validate() error {
diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go
index 7fda6ba0d..5d19f8b01 100644
--- a/nomad/structs/structs_test.go
+++ b/nomad/structs/structs_test.go
@@ -284,6 +284,12 @@ func TestJob_SpecChanged(t *testing.T) {
 			Original: &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 {
@@ -814,14 +820,14 @@ func TestJob_PartEqual(t *testing.T) {
 	}))
 
 	as := &Affinities{
-		&Affinity{"left0", "right0", "=", 0, ""},
-		&Affinity{"left1", "right1", "=", 0, ""},
-		&Affinity{"left2", "right2", "=", 0, ""},
+		&Affinity{"left0", "right0", "=", 0},
+		&Affinity{"left1", "right1", "=", 0},
+		&Affinity{"left2", "right2", "=", 0},
 	}
 	require.True(t, as.Equals(&Affinities{
-		&Affinity{"left0", "right0", "=", 0, ""},
-		&Affinity{"left2", "right2", "=", 0, ""},
-		&Affinity{"left1", "right1", "=", 0, ""},
+		&Affinity{"left0", "right0", "=", 0},
+		&Affinity{"left2", "right2", "=", 0},
+		&Affinity{"left1", "right1", "=", 0},
 	}))
 }