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

jobspec: add diff for Task Group Scaling block #18332

Merged
merged 2 commits into from
Aug 28, 2023
Merged
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/18332.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
jobspec: Add diff for Task Group scaling block
```
65 changes: 65 additions & 0 deletions nomad/structs/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand Down Expand Up @@ -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 {
Expand Down
196 changes: 196 additions & 0 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down