more testing for ScalingPolicy, mainly around parsing and canonicalization for Min/Max

acl/policy.go

@@ -11,11 +11,11 @@ const (
 	// The following levels are the only valid values for the `policy = "read"` stanza.
 	// When policies are merged together, the most privilege is granted, except for deny
 	// which always takes precedence and supercedes.
-	PolicyDeny       = "deny"
-	PolicyRead       = "read"
-	PolicyList       = "list"
-	PolicyWrite      = "write"
-	PolicyAutoscaler = "autoscaler"
+	PolicyDeny  = "deny"
+	PolicyRead  = "read"
+	PolicyList  = "list"
+	PolicyWrite = "write"
+	PolicyScale = "scale"
 )
 
 const (
@@ -126,7 +126,7 @@ type PluginPolicy struct {
 // isPolicyValid makes sure the given string matches one of the valid policies.
 func isPolicyValid(policy string) bool {
 	switch policy {
-	case PolicyDeny, PolicyRead, PolicyWrite, PolicyAutoscaler:
+	case PolicyDeny, PolicyRead, PolicyWrite, PolicyScale:
 		return true
 	default:
 		return false
@@ -192,7 +192,7 @@ func expandNamespacePolicy(policy string) []string {
 		return read
 	case PolicyWrite:
 		return write
-	case PolicyAutoscaler:
+	case PolicyScale:
 		return []string{
 			NamespaceCapabilityListScalingPolicies,
 			NamespaceCapabilityReadScalingPolicy,

acl/policy_test.go

@@ -52,7 +52,7 @@ func TestParse(t *testing.T) {
 				capabilities = ["deny", "read-logs"]
 			}
 			namespace "autoscaler" {
-				policy = "autoscaler"
+				policy = "scale"
 			}
 			agent {
 				policy = "read"
@@ -117,7 +117,7 @@ func TestParse(t *testing.T) {
 					},
 					{
 						Name:   "autoscaler",
-						Policy: PolicyAutoscaler,
+						Policy: PolicyScale,
 						Capabilities: []string{
 							NamespaceCapabilityListScalingPolicies,
 							NamespaceCapabilityReadScalingPolicy,

api/scaling.go

@@ -28,18 +28,12 @@ func (s *Scaling) GetPolicy(ID string, q *QueryOptions) (*ScalingPolicy, *QueryM
 	return &policy, qm, nil
 }
 
-func (p *ScalingPolicy) Canonicalize(tg *TaskGroup) {
+func (p *ScalingPolicy) Canonicalize(taskGroupCount int) {
 	if p.Enabled == nil {
 		p.Enabled = boolToPtr(true)
 	}
 	if p.Min == nil {
-		var m int64
-		if tg.Count != nil {
-			m = int64(*tg.Count)
-		} else {
-			// this should not be at this point, but safeguard here just in case
-			m = 0
-		}
+		var m int64 = int64(taskGroupCount)
 		p.Min = &m
 	}
 }

api/tasks.go

@@ -450,6 +450,9 @@ func (g *TaskGroup) Canonicalize(job *Job) {
 			g.Count = intToPtr(1)
 		}
 	}
+	if g.Scaling != nil {
+		g.Scaling.Canonicalize(*g.Count)
+	}
 	for _, t := range g.Tasks {
 		t.Canonicalize(g, job)
 	}
@@ -548,10 +551,6 @@ func (g *TaskGroup) Canonicalize(job *Job) {
 	for _, s := range g.Services {
 		s.Canonicalize(nil, g, job)
 	}
-
-	if g.Scaling != nil {
-		g.Scaling.Canonicalize(g)
-	}
 }
 
 // Constrain is used to add a constraint to a task group.

api/tasks_test.go

@@ -457,27 +457,35 @@ func TestTaskGroup_Canonicalize_Scaling(t *testing.T) {
 	tg.Canonicalize(job)
 	require.NotNil(tg.Count)
 	require.NotNil(tg.Scaling.Min)
-	require.Equal(1, *tg.Count)
-	require.Equal(int64(*tg.Count), *tg.Scaling.Min)
+	require.EqualValues(1, *tg.Count)
+	require.EqualValues(*tg.Count, *tg.Scaling.Min)
 
+	// count == nil => count = Scaling.Min
 	tg.Count = nil
 	tg.Scaling.Min = int64ToPtr(5)
-	// count == nil => count = Scaling.Min
 	tg.Canonicalize(job)
 	require.NotNil(tg.Count)
 	require.NotNil(tg.Scaling.Min)
-	require.Equal(5, *tg.Count)
-	require.Equal(int64(*tg.Count), *tg.Scaling.Min)
+	require.EqualValues(5, *tg.Count)
+	require.EqualValues(*tg.Count, *tg.Scaling.Min)
 
 	// Scaling.Min == nil => Scaling.Min == count
 	tg.Count = intToPtr(5)
 	tg.Scaling.Min = nil
-	// count == nil => count = Scaling.Min
 	tg.Canonicalize(job)
 	require.NotNil(tg.Count)
 	require.NotNil(tg.Scaling.Min)
-	require.Equal(int64(5), *tg.Scaling.Min)
-	require.Equal(*tg.Scaling.Min, int64(*tg.Count))
+	require.EqualValues(5, *tg.Scaling.Min)
+	require.EqualValues(*tg.Scaling.Min, *tg.Count)
+
+	// both present, both persisted
+	tg.Count = intToPtr(5)
+	tg.Scaling.Min = int64ToPtr(1)
+	tg.Canonicalize(job)
+	require.NotNil(tg.Count)
+	require.NotNil(tg.Scaling.Min)
+	require.EqualValues(1, *tg.Scaling.Min)
+	require.EqualValues(5, *tg.Count)
 }
 
 func TestTaskGroup_Merge_Update(t *testing.T) {

api/utils.go

@@ -27,8 +27,8 @@ func uint64ToPtr(u uint64) *uint64 {
 }
 
 // int64ToPtr returns the pointer to a int64
-func int64ToPtr(u int64) *int64 {
-	return &u
+func int64ToPtr(i int64) *int64 {
+	return &i
 }
 
 // stringToPtr returns the pointer to a string

nomad/job_endpoint.go

@@ -925,8 +925,8 @@ func (j *Job) Scale(args *structs.JobScaleRequest, reply *structs.JobRegisterRes
 	}
 
 	// Populate the reply with job information
-	reply.JobModifyIndex = job.ModifyIndex
-	reply.Index = job.ModifyIndex
+	reply.JobModifyIndex = jobModifyIndex
+	reply.Index = reply.JobModifyIndex
 
 	// FINISH:
 	// register the scaling event to the scaling_event table, once that exists

nomad/mock/mock.go

@@ -1256,7 +1256,9 @@ func ACLManagementToken() *structs.ACLToken {
 
 func ScalingPolicy() *structs.ScalingPolicy {
 	return &structs.ScalingPolicy{
-		ID: uuid.Generate(),
+		ID:  uuid.Generate(),
+		Min: 1,
+		Max: 100,
 		Target: map[string]string{
 			structs.ScalingTargetNamespace: structs.DefaultNamespace,
 			structs.ScalingTargetJob:       uuid.Generate(),