Merge pull request #5181 from hashicorp/b-spread-weight

Make spread weight a pointer with default value if unset

api/compose_test.go

@@ -87,7 +87,7 @@ func TestCompose(t *testing.T) {
 				Spreads: []*Spread{
 					{
 						Attribute: "${node.datacenter}",
-						Weight:    30,
+						Weight:    helper.IntToPtr(30),
 						SpreadTarget: []*SpreadTarget{
 							{
 								Value:   "dc1",

api/jobs.go

@@ -707,6 +707,10 @@ func (j *Job) Canonicalize() {
 	for _, tg := range j.TaskGroups {
 		tg.Canonicalize(j)
 	}
+
+	for _, spread := range j.Spreads {
+		spread.Canonicalize()
+	}
 }
 
 // LookupTaskGroup finds a task group by name

api/jobs_test.go

@@ -1413,7 +1413,7 @@ func TestJobs_AddSpread(t *testing.T) {
 	expect := []*Spread{
 		{
 			Attribute: "${meta.rack}",
-			Weight:    100,
+			Weight:    helper.IntToPtr(100),
 			SpreadTarget: []*SpreadTarget{
 				{
 					Value:   "r1",
@@ -1423,7 +1423,7 @@ func TestJobs_AddSpread(t *testing.T) {
 		},
 		{
 			Attribute: "${node.datacenter}",
-			Weight:    100,
+			Weight:    helper.IntToPtr(100),
 			SpreadTarget: []*SpreadTarget{
 				{
 					Value:   "dc1",

api/tasks.go

@@ -223,7 +223,7 @@ func (p *ReschedulePolicy) String() string {
 // Spread is used to serialize task group allocation spread preferences
 type Spread struct {
 	Attribute    string
-	Weight       int
+	Weight       *int
 	SpreadTarget []*SpreadTarget
 }
 
@@ -243,11 +243,17 @@ func NewSpreadTarget(value string, percent uint32) *SpreadTarget {
 func NewSpread(attribute string, weight int, spreadTargets []*SpreadTarget) *Spread {
 	return &Spread{
 		Attribute:    attribute,
-		Weight:       weight,
+		Weight:       helper.IntToPtr(weight),
 		SpreadTarget: spreadTargets,
 	}
 }
 
+func (s *Spread) Canonicalize() {
+	if s.Weight == nil {
+		s.Weight = helper.IntToPtr(50)
+	}
+}
+
 // CheckRestart describes if and when a task should be restarted based on
 // failing health checks.
 type CheckRestart struct {
@@ -568,6 +574,11 @@ func (g *TaskGroup) Canonicalize(job *Job) {
 		defaultRestartPolicy.Merge(g.RestartPolicy)
 	}
 	g.RestartPolicy = defaultRestartPolicy
+
+	for _, spread := range g.Spreads {
+		spread.Canonicalize()
+	}
+
 }
 
 // Constrain is used to add a constraint to a task group.

api/tasks_test.go

@@ -143,7 +143,7 @@ func TestTaskGroup_AddSpread(t *testing.T) {
 	expect := []*Spread{
 		{
 			Attribute: "${meta.rack}",
-			Weight:    100,
+			Weight:    helper.IntToPtr(100),
 			SpreadTarget: []*SpreadTarget{
 				{
 					Value:   "r1",
@@ -153,7 +153,7 @@ func TestTaskGroup_AddSpread(t *testing.T) {
 		},
 		{
 			Attribute: "${node.datacenter}",
-			Weight:    100,
+			Weight:    helper.IntToPtr(100),
 			SpreadTarget: []*SpreadTarget{
 				{
 					Value:   "dc1",
@@ -752,3 +752,57 @@ func TestService_CheckRestart(t *testing.T) {
 	assert.Equal(t, *service.Checks[2].CheckRestart.Grace, 11*time.Second)
 	assert.True(t, service.Checks[2].CheckRestart.IgnoreWarnings)
 }
+
+// TestSpread_Canonicalize asserts that the spread stanza is canonicalized correctly
+func TestSpread_Canonicalize(t *testing.T) {
+	job := &Job{
+		ID:   helper.StringToPtr("test"),
+		Type: helper.StringToPtr("batch"),
+	}
+	job.Canonicalize()
+	tg := &TaskGroup{
+		Name: helper.StringToPtr("foo"),
+	}
+	type testCase struct {
+		desc           string
+		spread         *Spread
+		expectedWeight int
+	}
+	cases := []testCase{
+		{
+			"Nil spread",
+			&Spread{
+				Attribute: "test",
+				Weight:    nil,
+			},
+			50,
+		},
+		{
+			"Zero spread",
+			&Spread{
+				Attribute: "test",
+				Weight:    helper.IntToPtr(0),
+			},
+			0,
+		},
+		{
+			"Non Zero spread",
+			&Spread{
+				Attribute: "test",
+				Weight:    helper.IntToPtr(100),
+			},
+			100,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.desc, func(t *testing.T) {
+			require := require.New(t)
+			tg.Spreads = []*Spread{tc.spread}
+			tg.Canonicalize(job)
+			for _, spr := range tg.Spreads {
+				require.Equal(tc.expectedWeight, *spr.Weight)
+			}
+		})
+	}
+}

command/agent/job_endpoint.go

@@ -956,7 +956,7 @@ func ApiAffinityToStructs(a1 *api.Affinity) *structs.Affinity {
 func ApiSpreadToStructs(a1 *api.Spread) *structs.Spread {
 	ret := &structs.Spread{}
 	ret.Attribute = a1.Attribute
-	ret.Weight = a1.Weight
+	ret.Weight = *a1.Weight
 	if a1.SpreadTarget != nil {
 		ret.SpreadTarget = make([]*structs.SpreadTarget, len(a1.SpreadTarget))
 		for i, st := range a1.SpreadTarget {

command/agent/job_endpoint_test.go

@@ -1232,7 +1232,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 		Spreads: []*api.Spread{
 			{
 				Attribute: "${meta.rack}",
-				Weight:    100,
+				Weight:    helper.IntToPtr(100),
 				SpreadTarget: []*api.SpreadTarget{
 					{
 						Value:   "r1",
@@ -1299,7 +1299,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 				Spreads: []*api.Spread{
 					{
 						Attribute: "${node.datacenter}",
-						Weight:    100,
+						Weight:    helper.IntToPtr(100),
 						SpreadTarget: []*api.SpreadTarget{
 							{
 								Value:   "dc1",

jobspec/parse_test.go

@@ -57,7 +57,7 @@ func TestParse(t *testing.T) {
 				Spreads: []*api.Spread{
 					{
 						Attribute: "${meta.rack}",
-						Weight:    100,
+						Weight:    helper.IntToPtr(100),
 						SpreadTarget: []*api.SpreadTarget{
 							{
 								Value:   "r1",
@@ -131,7 +131,7 @@ func TestParse(t *testing.T) {
 						Spreads: []*api.Spread{
 							{
 								Attribute: "${node.datacenter}",
-								Weight:    50,
+								Weight:    helper.IntToPtr(50),
 								SpreadTarget: []*api.SpreadTarget{
 									{
 										Value:   "dc1",