Skip to content

Commit

Permalink
jobspec: add diff for Task Group Scaling block (#18332)
Browse files Browse the repository at this point in the history
  • Loading branch information
nvanthao committed Aug 28, 2023
1 parent f25480c commit f187afa
Show file tree
Hide file tree
Showing 3 changed files with 264 additions and 0 deletions.
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

0 comments on commit f187afa

Please sign in to comment.