Skip to content

Commit

Permalink
Fixed plan diffing to handle non-unique service names.
Browse files Browse the repository at this point in the history
  • Loading branch information
apollo13 committed Jul 29, 2021
1 parent 445b115 commit b18e8c4
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 9 deletions.
3 changes: 3 additions & 0 deletions .changelog/10965.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
plan: Handle multiple services with the same name.
```
60 changes: 51 additions & 9 deletions nomad/structs/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -640,35 +640,77 @@ func serviceDiff(old, new *Service, contextual bool) *ObjectDiff {
return diff
}

// serviceDiffs diffs a set of services. If contextual diff is enabled, unchanged
// fields within objects nested in the tasks will be returned.
func serviceDiffs(old, new []*Service, contextual bool) []*ObjectDiff {
// serviceDiffsInternal diffs a set of services with the same name.
func serviceDiffsInternal(old, new []*Service, contextual bool) []*ObjectDiff {
oldMap := make(map[string]*Service, len(old))
newMap := make(map[string]*Service, len(new))
for _, o := range old {
oldMap[o.Name] = o
oldMap[o.PortLabel] = o
}
for _, n := range new {
newMap[n.Name] = n
newMap[n.PortLabel] = n
}

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

for name, newService := range newMap {
for portLabel, newService := range newMap {
// Diff the added
if old, ok := oldMap[name]; !ok {
if old, ok := oldMap[portLabel]; !ok {
if diff := serviceDiff(old, newService, contextual); diff != nil {
diffs = append(diffs, diff)
}
}
}

return diffs
}

// serviceDiffs diffs a set of services. If contextual diff is enabled, unchanged
// fields within objects nested in the tasks will be returned.
func serviceDiffs(old, new []*Service, contextual bool) []*ObjectDiff {
oldMap := make(map[string][]*Service, len(old))
newMap := make(map[string][]*Service, len(new))
for _, o := range old {
oldMap[o.Name] = append(oldMap[o.Name], o)
}
for _, n := range new {
newMap[n.Name] = append(newMap[n.Name], n)
}

var diffs []*ObjectDiff
for name, oldServices := range oldMap {
// services were deleted
if newServices, ok := newMap[name]; !ok {
for _, oldService := range oldServices {
diffs = append(diffs, serviceDiff(oldService, nil, contextual))
}
} else { // services where changed
// Handle the case of a "simple" change
if len(newServices) == 1 && len(oldServices) == 1 {
if diff := serviceDiff(oldServices[0], newServices[0], contextual); diff != nil {
diffs = append(diffs, diff)
}
} else { // More complex changes
diffs = append(diffs, serviceDiffsInternal(oldServices, newServices, contextual)...)
}
}
}

for name, newServices := range newMap {
// services where added
if _, ok := oldMap[name]; !ok {
for _, newService := range newServices {
diffs = append(diffs, serviceDiff(nil, newService, contextual))
}
}
}

sort.Sort(ObjectDiffs(diffs))
return diffs
}
Expand Down
68 changes: 68 additions & 0 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3885,6 +3885,74 @@ func TestTaskGroupDiff(t *testing.T) {
},
},
},

{
TestCase: "TaskGroup service added with same name",
Contextual: true,
Old: &TaskGroup{
Services: []*Service{
{
Name: "foo",
PortLabel: "label1",
},
},
},

New: &TaskGroup{
Services: []*Service{
{
Name: "foo",
PortLabel: "label1",
},
{
Name: "foo",
PortLabel: "label2",
},
},
},
Expected: &TaskGroupDiff{
Type: DiffTypeEdited,
Objects: []*ObjectDiff{
{
Type: DiffTypeAdded,
Name: "Service",
Fields: []*FieldDiff{
{
Type: DiffTypeNone,
Name: "AddressMode",
},
{
Type: DiffTypeAdded,
Name: "EnableTagOverride",
New: "false",
},
{
Type: DiffTypeAdded,
Name: "Name",
New: "foo",
},
{
Type: DiffTypeNone,
Name: "Namespace",
},
{
Type: DiffTypeNone,
Name: "OnUpdate",
},
{
Type: DiffTypeAdded,
Name: "PortLabel",
New: "label2",
},
{
Type: DiffTypeNone,
Name: "TaskName",
},
},
},
},
},
},
}

for i, c := range cases {
Expand Down

0 comments on commit b18e8c4

Please sign in to comment.