Skip to content

Commit

Permalink
Made template diffing a bit smarter.
Browse files Browse the repository at this point in the history
Previously templates diff always showed up as delete & add. The new code
always diffs if there is only one template and if there are multiple
templates it uses DestPath as unique identifier (this works because
nomad checks that this is actually unique).
  • Loading branch information
apollo13 committed Oct 23, 2021
1 parent 10d3056 commit 16dd186
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 9 deletions.
3 changes: 3 additions & 0 deletions .changelog/11378.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
plan: Smarter diffing of template stanzas
```
77 changes: 69 additions & 8 deletions nomad/structs/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,14 +516,8 @@ func (t *Task) Diff(other *Task, contextual bool) (*TaskDiff, error) {
diff.Objects = append(diff.Objects, vDiff)
}

// Template diff
tmplDiffs := primitiveObjectSetDiff(
interfaceSlice(t.Templates),
interfaceSlice(other.Templates),
nil,
"Template",
contextual)
if tmplDiffs != nil {
// Templates diff
if tmplDiffs := templateDiffs(t.Templates, other.Templates, contextual); tmplDiffs != nil {
diff.Objects = append(diff.Objects, tmplDiffs...)
}

Expand Down Expand Up @@ -769,6 +763,73 @@ func findServiceMatch(service *Service, serviceIndex int, services []*Service, m
return indexMatch
}

// templateDiff returns the diff of two template objects. If contextual diff is
// enabled, all fields will be returned, even if no diff occurred.
func templateDiff(old, new *Template, contextual bool) *ObjectDiff {
diff := &ObjectDiff{Type: DiffTypeNone, Name: "Template"}
var oldPrimitiveFlat, newPrimitiveFlat map[string]string

if reflect.DeepEqual(old, new) {
return nil
} else if old == nil {
diff.Type = DiffTypeAdded
newPrimitiveFlat = flatmap.Flatten(new, nil, true)
} else if new == nil {
diff.Type = DiffTypeDeleted
oldPrimitiveFlat = flatmap.Flatten(old, nil, true)
} else {
diff.Type = DiffTypeEdited
oldPrimitiveFlat = flatmap.Flatten(old, nil, true)
newPrimitiveFlat = flatmap.Flatten(new, nil, true)
}

// Diff the primitive fields.
diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, contextual)

return diff
}

// templateDiffs diffs a set of templates. If contextual diff is enabled, unchanged
// fields within objects nested in the tasks will be returned.
func templateDiffs(old, new []*Template, contextual bool) []*ObjectDiff {
// Handle trivial case.
if len(old) == 1 && len(new) == 1 {
if diff := templateDiff(old[0], new[0], contextual); diff != nil {
return []*ObjectDiff{diff}
}
return nil
}

oldMap := make(map[string]*Template, len(old))
newMap := make(map[string]*Template, len(new))
for _, o := range old {
oldMap[o.DestPath] = o
}
for _, n := range new {
newMap[n.DestPath] = n
}

var diffs []*ObjectDiff
for path, oldTemplate := range oldMap {
// Diff the same, deleted and edited
if diff := templateDiff(oldTemplate, newMap[path], contextual); diff != nil {
diffs = append(diffs, diff)
}
}

for path, newTemplate := range newMap {
// Diff the added
if old, ok := oldMap[path]; !ok {
if diff := templateDiff(old, newTemplate, contextual); diff != nil {
diffs = append(diffs, diff)
}
}
}

sort.Sort(ObjectDiffs(diffs))
return diffs
}

// serviceCheckDiff returns the diff of two service check objects. If contextual
// diff is enabled, all fields will be returned, even if no diff occurred.
func serviceCheckDiff(old, new *ServiceCheck, contextual bool) *ObjectDiff {
Expand Down
14 changes: 13 additions & 1 deletion nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6914,7 +6914,7 @@ func TestTaskDiff(t *testing.T) {
{
SourcePath: "foo",
DestPath: "bar",
EmbeddedTmpl: "baz",
EmbeddedTmpl: "baz new",
ChangeMode: "bam",
ChangeSignal: "SIGHUP",
Splay: 1,
Expand All @@ -6934,6 +6934,18 @@ func TestTaskDiff(t *testing.T) {
Expected: &TaskDiff{
Type: DiffTypeEdited,
Objects: []*ObjectDiff{
{
Type: DiffTypeEdited,
Name: "Template",
Fields: []*FieldDiff{
{
Type: DiffTypeEdited,
Name: "EmbeddedTmpl",
Old: "baz",
New: "baz new",
},
},
},
{
Type: DiffTypeAdded,
Name: "Template",
Expand Down

0 comments on commit 16dd186

Please sign in to comment.