Diff code fixes

command/agent/job_endpoint.go

@@ -424,18 +424,22 @@ func ApiJobToStructJob(job *api.Job) *structs.Job {
 		VaultToken:  *job.VaultToken,
 	}
 
-	j.Constraints = make([]*structs.Constraint, len(job.Constraints))
+	if l := len(job.Constraints); l != 0 {
+		j.Constraints = make([]*structs.Constraint, l)
 		for i, c := range job.Constraints {
 			con := &structs.Constraint{}
 			ApiConstraintToStructs(c, con)
 			j.Constraints[i] = con
 		}
+	}
+
 	if job.Update != nil {
 		j.Update = structs.UpdateStrategy{
 			Stagger:     job.Update.Stagger,
 			MaxParallel: job.Update.MaxParallel,
 		}
 	}
+
 	if job.Periodic != nil {
 		j.Periodic = &structs.PeriodicConfig{
 			Enabled:         *job.Periodic.Enabled,
@@ -443,10 +447,12 @@ func ApiJobToStructJob(job *api.Job) *structs.Job {
 			ProhibitOverlap: *job.Periodic.ProhibitOverlap,
 			TimeZone:        *job.Periodic.TimeZone,
 		}
+
 		if job.Periodic.Spec != nil {
 			j.Periodic.Spec = *job.Periodic.Spec
 		}
 	}
+
 	if job.ParameterizedJob != nil {
 		j.ParameterizedJob = &structs.ParameterizedJobConfig{
 			Payload:      job.ParameterizedJob.Payload,
@@ -455,12 +461,14 @@ func ApiJobToStructJob(job *api.Job) *structs.Job {
 		}
 	}
 
-	j.TaskGroups = make([]*structs.TaskGroup, len(job.TaskGroups))
+	if l := len(job.TaskGroups); l != 0 {
+		j.TaskGroups = make([]*structs.TaskGroup, l)
 		for i, taskGroup := range job.TaskGroups {
 			tg := &structs.TaskGroup{}
 			ApiTgToStructsTG(taskGroup, tg)
 			j.TaskGroups[i] = tg
 		}
+	}
 
 	return j
 }
@@ -469,31 +477,38 @@ func ApiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup) {
 	tg.Name = *taskGroup.Name
 	tg.Count = *taskGroup.Count
 	tg.Meta = taskGroup.Meta
-	tg.Constraints = make([]*structs.Constraint, len(taskGroup.Constraints))
+
+	if l := len(taskGroup.Constraints); l != 0 {
+		tg.Constraints = make([]*structs.Constraint, l)
 		for k, constraint := range taskGroup.Constraints {
 			c := &structs.Constraint{}
 			ApiConstraintToStructs(constraint, c)
 			tg.Constraints[k] = c
 		}
+	}
+
 	tg.RestartPolicy = &structs.RestartPolicy{
 		Attempts: *taskGroup.RestartPolicy.Attempts,
 		Interval: *taskGroup.RestartPolicy.Interval,
 		Delay:    *taskGroup.RestartPolicy.Delay,
 		Mode:     *taskGroup.RestartPolicy.Mode,
 	}
+
 	tg.EphemeralDisk = &structs.EphemeralDisk{
 		Sticky:  *taskGroup.EphemeralDisk.Sticky,
 		SizeMB:  *taskGroup.EphemeralDisk.SizeMB,
 		Migrate: *taskGroup.EphemeralDisk.Migrate,
 	}
-	tg.Meta = taskGroup.Meta
+
-	tg.Tasks = make([]*structs.Task, len(taskGroup.Tasks))
+	if l := len(taskGroup.Tasks); l != 0 {
+		tg.Tasks = make([]*structs.Task, l)
 		for l, task := range taskGroup.Tasks {
 			t := &structs.Task{}
 			ApiTaskToStructsTask(task, t)
 			tg.Tasks[l] = t
 		}
 	}
+}
 
 func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
 	structsTask.Name = apiTask.Name
@@ -501,21 +516,30 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
 	structsTask.User = apiTask.User
 	structsTask.Leader = apiTask.Leader
 	structsTask.Config = apiTask.Config
-	structsTask.Constraints = make([]*structs.Constraint, len(apiTask.Constraints))
+	structsTask.Env = apiTask.Env
+	structsTask.Meta = apiTask.Meta
+	structsTask.KillTimeout = *apiTask.KillTimeout
+
+	if l := len(apiTask.Constraints); l != 0 {
+		structsTask.Constraints = make([]*structs.Constraint, l)
 		for i, constraint := range apiTask.Constraints {
 			c := &structs.Constraint{}
 			ApiConstraintToStructs(constraint, c)
 			structsTask.Constraints[i] = c
 		}
-	structsTask.Env = apiTask.Env
+	}
-	structsTask.Services = make([]*structs.Service, len(apiTask.Services))
+
+	if l := len(apiTask.Services); l != 0 {
+		structsTask.Services = make([]*structs.Service, l)
 		for i, service := range apiTask.Services {
 			structsTask.Services[i] = &structs.Service{
 				Name:      service.Name,
 				PortLabel: service.PortLabel,
 				Tags:      service.Tags,
 			}
-		structsTask.Services[i].Checks = make([]*structs.ServiceCheck, len(service.Checks))
+
+			if l := len(service.Checks); l != 0 {
+				structsTask.Services[i].Checks = make([]*structs.ServiceCheck, l)
 				for j, check := range service.Checks {
 					structsTask.Services[i].Checks[j] = &structs.ServiceCheck{
 						Name:          check.Name,
@@ -531,26 +555,36 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
 					}
 				}
 			}
+		}
+	}
+
 	structsTask.Resources = &structs.Resources{
 		CPU:      *apiTask.Resources.CPU,
 		MemoryMB: *apiTask.Resources.MemoryMB,
 		IOPS:     *apiTask.Resources.IOPS,
 	}
-	structsTask.Resources.Networks = make([]*structs.NetworkResource, len(apiTask.Resources.Networks))
+
+	if l := len(apiTask.Resources.Networks); l != 0 {
+		structsTask.Resources.Networks = make([]*structs.NetworkResource, l)
 		for i, nw := range apiTask.Resources.Networks {
 			structsTask.Resources.Networks[i] = &structs.NetworkResource{
 				CIDR:  nw.CIDR,
 				IP:    nw.IP,
 				MBits: *nw.MBits,
 			}
-		structsTask.Resources.Networks[i].DynamicPorts = make([]structs.Port, len(nw.DynamicPorts))
+
-		structsTask.Resources.Networks[i].ReservedPorts = make([]structs.Port, len(nw.ReservedPorts))
+			if l := len(nw.DynamicPorts); l != 0 {
+				structsTask.Resources.Networks[i].DynamicPorts = make([]structs.Port, l)
 				for j, dp := range nw.DynamicPorts {
 					structsTask.Resources.Networks[i].DynamicPorts[j] = structs.Port{
 						Label: dp.Label,
 						Value: dp.Value,
 					}
 				}
+			}
+
+			if l := len(nw.ReservedPorts); l != 0 {
+				structsTask.Resources.Networks[i].ReservedPorts = make([]structs.Port, l)
 				for j, rp := range nw.ReservedPorts {
 					structsTask.Resources.Networks[i].ReservedPorts[j] = structs.Port{
 						Label: rp.Label,
@@ -558,13 +592,16 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
 					}
 				}
 			}
-	structsTask.Meta = apiTask.Meta
+		}
-	structsTask.KillTimeout = *apiTask.KillTimeout
+	}
+
 	structsTask.LogConfig = &structs.LogConfig{
 		MaxFiles:      *apiTask.LogConfig.MaxFiles,
 		MaxFileSizeMB: *apiTask.LogConfig.MaxFileSizeMB,
 	}
-	structsTask.Artifacts = make([]*structs.TaskArtifact, len(apiTask.Artifacts))
+
+	if l := len(apiTask.Artifacts); l != 0 {
+		structsTask.Artifacts = make([]*structs.TaskArtifact, l)
 		for k, ta := range apiTask.Artifacts {
 			structsTask.Artifacts[k] = &structs.TaskArtifact{
 				GetterSource:  *ta.GetterSource,
@@ -572,6 +609,8 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
 				RelativeDest:  *ta.RelativeDest,
 			}
 		}
+	}
+
 	if apiTask.Vault != nil {
 		structsTask.Vault = &structs.Vault{
 			Policies:     apiTask.Vault.Policies,
@@ -580,7 +619,9 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
 			ChangeSignal: *apiTask.Vault.ChangeSignal,
 		}
 	}
-	structsTask.Templates = make([]*structs.Template, len(apiTask.Templates))
+
+	if l := len(apiTask.Templates); l != 0 {
+		structsTask.Templates = make([]*structs.Template, l)
 		for i, template := range apiTask.Templates {
 			structsTask.Templates[i] = &structs.Template{
 				SourcePath:   *template.SourcePath,
@@ -594,6 +635,8 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
 				RightDelim:   *template.RightDelim,
 			}
 		}
+	}
+
 	if apiTask.DispatchPayload != nil {
 		structsTask.DispatchPayload = &structs.DispatchPayloadConfig{
 			File: apiTask.DispatchPayload.File,

nomad/eval_endpoint_test.go

@@ -1,6 +1,7 @@
 package nomad
 
 import (
+	"fmt"
 	"reflect"
 	"strings"
 	"testing"
@@ -272,10 +273,16 @@ func TestEvalEndpoint_Nack(t *testing.T) {
 	}
 
 	// Should get it back
+	testutil.WaitForResult(func() (bool, error) {
 		out2, _, _ := s1.evalBroker.Dequeue(defaultSched, time.Second)
 		if out2 != out {
-		t.Fatalf("nack failed")
+			return false, fmt.Errorf("nack failed")
 		}
+
+		return true, nil
+	}, func(err error) {
+		t.Fatal(err)
+	})
 }
 
 func TestEvalEndpoint_Update(t *testing.T) {

nomad/structs/diff.go

@@ -59,7 +59,8 @@ type JobDiff struct {
 func (j *Job) Diff(other *Job, contextual bool) (*JobDiff, error) {
 	diff := &JobDiff{Type: DiffTypeNone}
 	var oldPrimitiveFlat, newPrimitiveFlat map[string]string
-	filter := []string{"ID", "Status", "StatusDescription", "CreateIndex", "ModifyIndex", "JobModifyIndex"}
+	filter := []string{"ID", "Status", "StatusDescription", "Version", "Stable", "CreateIndex",
+		"ModifyIndex", "JobModifyIndex"}
 
 	// Have to treat this special since it is a struct literal, not a pointer
 	var jUpdate, otherUpdate *UpdateStrategy
@@ -83,10 +84,6 @@ func (j *Job) Diff(other *Job, contextual bool) (*JobDiff, error) {
 			return nil, fmt.Errorf("can not diff jobs with different IDs: %q and %q", j.ID, other.ID)
 		}
 
-		if !reflect.DeepEqual(j, other) {
-			diff.Type = DiffTypeEdited
-		}
-
 		jUpdate = &j.Update
 		otherUpdate = &other.Update
 		oldPrimitiveFlat = flatmap.Flatten(j, filter, true)
@@ -135,6 +132,35 @@ func (j *Job) Diff(other *Job, contextual bool) (*JobDiff, error) {
 		diff.Objects = append(diff.Objects, cDiff)
 	}
 
+	// Check to see if there is a diff. We don't use reflect because we are
+	// filtering quite a few fields that will change on each diff.
+	if diff.Type == DiffTypeNone {
+		for _, fd := range diff.Fields {
+			if fd.Type != DiffTypeNone {
+				diff.Type = DiffTypeEdited
+				break
+			}
+		}
+	}
+
+	if diff.Type == DiffTypeNone {
+		for _, od := range diff.Objects {
+			if od.Type != DiffTypeNone {
+				diff.Type = DiffTypeEdited
+				break
+			}
+		}
+	}
+
+	if diff.Type == DiffTypeNone {
+		for _, tg := range diff.TaskGroups {
+			if tg.Type != DiffTypeNone {
+				diff.Type = DiffTypeEdited
+				break
+			}
+		}
+	}
+
 	return diff, nil
 }
 

nomad/structs/diff_test.go

@@ -174,6 +174,12 @@ func TestJobDiff(t *testing.T) {
 						Old:  "foo",
 						New:  "",
 					},
+					{
+						Type: DiffTypeDeleted,
+						Name: "Stop",
+						Old:  "false",
+						New:  "",
+					},
 					{
 						Type: DiffTypeDeleted,
 						Name: "Type",
@@ -251,6 +257,12 @@ func TestJobDiff(t *testing.T) {
 						Old:  "",
 						New:  "foo",
 					},
+					{
+						Type: DiffTypeAdded,
+						Name: "Stop",
+						Old:  "",
+						New:  "false",
+					},
 					{
 						Type: DiffTypeAdded,
 						Name: "Type",