From 9efa56c345dbf240c1815f730be564c13f3bf3d3 Mon Sep 17 00:00:00 2001 From: Gerard Nguyen Date: Fri, 25 Aug 2023 23:53:48 +1000 Subject: [PATCH 1/2] add diff on TaskGroup Scaling block --- nomad/structs/diff.go | 65 ++++++++++++ nomad/structs/diff_test.go | 196 +++++++++++++++++++++++++++++++++++++ 2 files changed, 261 insertions(+) diff --git a/nomad/structs/diff.go b/nomad/structs/diff.go index e44e56ed53ab..73b6f8895340 100644 --- a/nomad/structs/diff.go +++ b/nomad/structs/diff.go @@ -343,6 +343,11 @@ func (tg *TaskGroup) Diff(other *TaskGroup, contextual bool) (*TaskGroupDiff, er diff.Objects = append(diff.Objects, nDiffs...) } + // Scaling diff + if scDiff := scalingDiff(tg.Scaling, other.Scaling, contextual); scDiff != nil { + diff.Objects = append(diff.Objects, scDiff) + } + // Services diff if sDiffs := serviceDiffs(tg.Services, other.Services, contextual); sDiffs != nil { diff.Objects = append(diff.Objects, sDiffs...) @@ -626,6 +631,66 @@ func (t TaskDiffs) Len() int { return len(t) } func (t TaskDiffs) Swap(i, j int) { t[i], t[j] = t[j], t[i] } func (t TaskDiffs) Less(i, j int) bool { return t[i].Name < t[j].Name } +// scalingDiff returns the diff of two Scaling objects. If contextual diff is enabled, unchanged +// fields within objects nested in the tasks will be returned. +func scalingDiff(old, new *ScalingPolicy, contextual bool) *ObjectDiff { + diff := &ObjectDiff{Type: DiffTypeNone, Name: "Scaling"} + var oldPrimitiveFlat, newPrimitiveFlat map[string]string + + filter := []string{"CreateIndex", "ModifyIndex", "ID", "Type", "Target[Job]", "Target[Group]", "Target[Namespace]"} + + if reflect.DeepEqual(old, new) { + return nil + } else if old == nil { + old = &ScalingPolicy{} + diff.Type = DiffTypeAdded + newPrimitiveFlat = flatmap.Flatten(new, filter, true) + } else if new == nil { + new = &ScalingPolicy{} + diff.Type = DiffTypeDeleted + oldPrimitiveFlat = flatmap.Flatten(old, filter, true) + } else { + diff.Type = DiffTypeEdited + oldPrimitiveFlat = flatmap.Flatten(old, filter, true) + newPrimitiveFlat = flatmap.Flatten(new, filter, true) + } + + // Diff the primitive fields. + diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, false) + + // Diff Policy + if pDiff := policyDiff(old.Policy, new.Policy, contextual); pDiff != nil { + diff.Objects = append(diff.Objects, pDiff) + } + + sort.Sort(FieldDiffs(diff.Fields)) + sort.Sort(ObjectDiffs(diff.Objects)) + + return diff +} + +// policyDiff returns the diff of two Scaling Policy objects. If contextual diff is enabled, unchanged +// fields within objects nested in the tasks will be returned. +func policyDiff(old, new map[string]interface{}, contextual bool) *ObjectDiff { + diff := &ObjectDiff{Type: DiffTypeNone, Name: "Policy"} + if reflect.DeepEqual(old, new) { + return nil + } else if len(old) == 0 { + diff.Type = DiffTypeAdded + } else if len(new) == 0 { + diff.Type = DiffTypeDeleted + } else { + diff.Type = DiffTypeEdited + } + + // Diff the primitive fields. + oldPrimitiveFlat := flatmap.Flatten(old, nil, false) + newPrimitiveFlat := flatmap.Flatten(new, nil, false) + diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, false) + + return diff +} + // serviceDiff returns the diff of two service objects. If contextual diff is // enabled, all fields will be returned, even if no diff occurred. func serviceDiff(old, new *Service, contextual bool) *ObjectDiff { diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 3548474327d9..b1e65a505213 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -4444,12 +4444,208 @@ func TestTaskGroupDiff(t *testing.T) { }, }, }, + { + TestCase: "Scaling added", + Old: &TaskGroup{}, + New: &TaskGroup{ + Scaling: &ScalingPolicy{ + Enabled: true, + Max: 10, + Min: 1, + Policy: map[string]interface{}{ + "cooldown": "1m", + "evaluation_interval": "5s", + }, + }, + }, + Expected: &TaskGroupDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Scaling", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Enabled", + Old: "", + New: "true", + }, + { + Type: DiffTypeAdded, + Name: "Max", + Old: "", + New: "10", + }, + { + Type: DiffTypeAdded, + Name: "Min", + Old: "", + New: "1", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Policy", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "cooldown", + Old: "", + New: "1m", + }, + { + Type: DiffTypeAdded, + Name: "evaluation_interval", + Old: "", + New: "5s", + }, + }, + }, + }, + }, + }, + }, + }, + { + TestCase: "Scaling deleted", + Old: &TaskGroup{ + Scaling: &ScalingPolicy{ + Enabled: true, + Max: 10, + Min: 1, + Policy: map[string]interface{}{ + "cooldown": "1m", + "evaluation_interval": "5s", + }, + }, + }, + New: &TaskGroup{}, + Expected: &TaskGroupDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeDeleted, + Name: "Scaling", + Fields: []*FieldDiff{ + { + Type: DiffTypeDeleted, + Name: "Enabled", + Old: "true", + New: "", + }, + { + Type: DiffTypeDeleted, + Name: "Max", + Old: "10", + New: "", + }, + { + Type: DiffTypeDeleted, + Name: "Min", + Old: "1", + New: "", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeDeleted, + Name: "Policy", + Fields: []*FieldDiff{ + { + Type: DiffTypeDeleted, + Name: "cooldown", + Old: "1m", + New: "", + }, + { + Type: DiffTypeDeleted, + Name: "evaluation_interval", + Old: "5s", + New: "", + }, + }, + }, + }, + }, + }, + }, + }, + { + TestCase: "Scaling edited", + Old: &TaskGroup{ + Scaling: &ScalingPolicy{ + Enabled: true, + Max: 10, + Min: 1, + Policy: map[string]interface{}{ + "cooldown": "1m", + "evaluation_interval": "5s", + }, + }, + }, + New: &TaskGroup{ + Scaling: &ScalingPolicy{ + Enabled: true, + Max: 15, + Min: 5, + Policy: map[string]interface{}{ + "cooldown": "2m", + "evaluation_interval": "10s", + }, + }, + }, + Expected: &TaskGroupDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Scaling", + Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "Max", + Old: "10", + New: "15", + }, + { + Type: DiffTypeEdited, + Name: "Min", + Old: "1", + New: "5", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Policy", + Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "cooldown", + Old: "1m", + New: "2m", + }, { + Type: DiffTypeEdited, + Name: "evaluation_interval", + Old: "5s", + New: "10s", + }, + }, + }, + }, + }, + }, + }, + }, } for i, c := range cases { require.NotEmpty(t, c.TestCase, "case #%d needs a name", i+1) t.Run(c.TestCase, func(t *testing.T) { + result, err := c.Old.Diff(c.New, c.Contextual) switch c.ExpErr { case true: From 97e1be4e2a78aedc7603fb648639ccdadbd5b216 Mon Sep 17 00:00:00 2001 From: Gerard Nguyen Date: Sat, 26 Aug 2023 00:09:16 +1000 Subject: [PATCH 2/2] update change log --- .changelog/18332.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/18332.txt diff --git a/.changelog/18332.txt b/.changelog/18332.txt new file mode 100644 index 000000000000..a75bf3ed9430 --- /dev/null +++ b/.changelog/18332.txt @@ -0,0 +1,3 @@ +```release-note:bug +jobspec: Add diff for Task Group scaling block +``` \ No newline at end of file