Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made template diffing a bit smarter. #11378

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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