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.

Sign up for GitHub

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

Jump to bottom

volumes: implement plan diff for volume requests #9973

Merged
merged 1 commit into from
Feb 4, 2021
Merged

volumes: implement plan diff for volume requests #9973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ IMPROVEMENTS:
* cli: Improved `scaling policy` commands with -verbose, auto-completion, and prefix-matching [[GH-9964](https://github.com/hashicorp/nomad/issues/9964)]
* consul/connect: Made handling of sidecar task container image URLs consistent with the `docker` task driver. [[GH-9580](https://github.com/hashicorp/nomad/issues/9580)]


BUG FIXES:
* consul: Fixed a bug where failing tasks with group services would only cause the allocation to restart once instead of respecting the `restart` field. [[GH-9869](https://github.com/hashicorp/nomad/issues/9869)]
* consul/connect: Fixed a bug where gateway proxy connection default timeout not set [[GH-9851](https://github.com/hashicorp/nomad/pull/9851)]
* consul/connect: Fixed a bug preventing more than one connect gateway per Nomad client [[GH-9849](https://github.com/hashicorp/nomad/pull/9849)]
* drivers/docker: Fixed a bug preventing multiple ports to be mapped to the same container port [[GH-9951](https://github.com/hashicorp/nomad/issues/9951)]
* driver/qemu: Fixed a bug where network namespaces were not supported for QEMU workloads [[GH-9861](https://github.com/hashicorp/nomad/pull/9861)]
* scheduler: Fixed a bug where shared ports were not persisted during inplace updates for service jobs. [[GH-9830](https://github.com/hashicorp/nomad/issues/9830)]
* scheduler: Fixed a bug where job statuses and summaries where duplicated and miscalculated when registering a job. [[GH-9768](https://github.com/hashicorp/nomad/issues/9768)]
* scheduler (Enterprise): Fixed a bug where the deprecated network `mbits` field was being considered as part of quota enforcement. [[GH-9920](https://github.com/hashicorp/nomad/issues/9920)]
* driver/qemu: Fixed a bug where network namespaces were not supported for QEMU workloads [[GH-9861](https://github.com/hashicorp/nomad/pull/9861)]
* scheduler (Enterprise): Fixed a bug where the deprecated network `mbits` field was being considered as part of quota enforcement. [[GH-9920](https://github.com/hashicorp/nomad/issues/9920)]
* volumes: Fixed a bug where volume diffs were not displayed in the output of `nomad plan`. [[GH-9973](https://github.com/hashicorp/nomad/issues/9973)]

## 1.0.3 (January 28, 2021)

Expand Down
102 changes: 102 additions & 0 deletions nomad/structs/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,11 @@ func (tg *TaskGroup) Diff(other *TaskGroup, contextual bool) (*TaskGroupDiff, er
diff.Objects = append(diff.Objects, sDiffs...)
}

// Volumes diff
if vDiffs := volumeDiffs(tg.Volumes, other.Volumes, contextual); vDiffs != nil {
diff.Objects = append(diff.Objects, vDiffs...)
}

// Tasks diff
tasks, err := taskDiffs(tg.Tasks, other.Tasks, contextual)
if err != nil {
Expand Down Expand Up @@ -1559,6 +1564,103 @@ Loop:
return diff
}

// volumeDiffs returns the diff of a group's volume requests. If contextual
// diff is enabled, all fields will be returned, even if no diff occurred.
func volumeDiffs(oldVR, newVR map[string]*VolumeRequest, contextual bool) []*ObjectDiff {
if reflect.DeepEqual(oldVR, newVR) {
return nil
}

diffs := []*ObjectDiff{} //Type: DiffTypeNone, Name: "Volumes"}
seen := map[string]bool{}
for name, oReq := range oldVR {
nReq := newVR[name] // might be nil, that's ok
seen[name] = true
diff := volumeDiff(oReq, nReq, contextual)
if diff != nil {
diffs = append(diffs, diff)
}
}
for name, nReq := range newVR {
if !seen[name] {
// we know old is nil at this point, or we'd have hit it before
diff := volumeDiff(nil, nReq, contextual)
if diff != nil {
diffs = append(diffs, diff)
}
}
}
return diffs
}

// volumeDiff returns the diff between two volume requests. If contextual diff
// is enabled, all fields will be returned, even if no diff occurred.
func volumeDiff(oldVR, newVR *VolumeRequest, contextual bool) *ObjectDiff {
if reflect.DeepEqual(oldVR, newVR) {
return nil
}

diff := &ObjectDiff{Type: DiffTypeNone, Name: "Volume"}
var oldPrimitiveFlat, newPrimitiveFlat map[string]string

if oldVR == nil {
oldVR = &VolumeRequest{}
diff.Type = DiffTypeAdded
newPrimitiveFlat = flatmap.Flatten(newVR, nil, true)
} else if newVR == nil {
newVR = &VolumeRequest{}
diff.Type = DiffTypeDeleted
oldPrimitiveFlat = flatmap.Flatten(oldVR, nil, true)
} else {
diff.Type = DiffTypeEdited
oldPrimitiveFlat = flatmap.Flatten(oldVR, nil, true)
newPrimitiveFlat = flatmap.Flatten(newVR, nil, true)
}

diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, contextual)

mOptsDiff := volumeCSIMountOptionsDiff(oldVR.MountOptions, newVR.MountOptions, contextual)
if mOptsDiff != nil {
diff.Objects = append(diff.Objects, mOptsDiff)
}

return diff
}

// volumeCSIMountOptionsDiff returns the diff between volume mount options. If
// contextual diff is enabled, all fields will be returned, even if no diff
// occurred.
func volumeCSIMountOptionsDiff(oldMO, newMO *CSIMountOptions, contextual bool) *ObjectDiff {
if reflect.DeepEqual(oldMO, newMO) {
return nil
}

diff := &ObjectDiff{Type: DiffTypeNone, Name: "MountOptions"}
var oldPrimitiveFlat, newPrimitiveFlat map[string]string

if oldMO == nil && newMO != nil {
oldMO = &CSIMountOptions{}
diff.Type = DiffTypeAdded
newPrimitiveFlat = flatmap.Flatten(newMO, nil, true)
} else if oldMO == nil && newMO != nil {
newMO = &CSIMountOptions{}
diff.Type = DiffTypeDeleted
oldPrimitiveFlat = flatmap.Flatten(oldMO, nil, true)
} else {
diff.Type = DiffTypeEdited
oldPrimitiveFlat = flatmap.Flatten(oldMO, nil, true)
newPrimitiveFlat = flatmap.Flatten(newMO, nil, true)
}

diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, contextual)

setDiff := stringSetDiff(oldMO.MountFlags, newMO.MountFlags, "MountFlags", contextual)
if setDiff != nil {
diff.Objects = append(diff.Objects, setDiff)
}
return diff
}

// Diff returns a diff of two resource objects. If contextual diff is enabled,
// non-changed fields will still be returned.
func (r *Resources) Diff(other *Resources, contextual bool) *ObjectDiff {
Expand Down
139 changes: 139 additions & 0 deletions nomad/structs/diff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3609,6 +3609,145 @@ func TestTaskGroupDiff(t *testing.T) {
},
},
},

{
TestCase: "TaskGroup volumes added",
Old: &TaskGroup{},
New: &TaskGroup{
Volumes: map[string]*VolumeRequest{
"foo": &VolumeRequest{
Name: "foo",
Type: "host",
Source: "foo-src",
ReadOnly: true,
},
},
},

Expected: &TaskGroupDiff{
Type: DiffTypeEdited,
Objects: []*ObjectDiff{
{
Type: DiffTypeAdded,
Name: "Volume",
Fields: []*FieldDiff{
{
Type: DiffTypeAdded,
Name: "Name",
Old: "",
New: "foo",
},
{
Type: DiffTypeAdded,
Name: "ReadOnly",
Old: "",
New: "true",
},
{
Type: DiffTypeAdded,
Name: "Source",
Old: "",
New: "foo-src",
},
{
Type: DiffTypeAdded,
Name: "Type",
Old: "",
New: "host",
},
},
},
},
},
},

{
TestCase: "TaskGroup volumes edited",
Old: &TaskGroup{
Volumes: map[string]*VolumeRequest{
"foo": &VolumeRequest{
Name: "foo",
Type: "csi",
Source: "foo-src1",
ReadOnly: false,
MountOptions: &CSIMountOptions{
FSType: "ext4",
MountFlags: []string{"relatime", "rw"},
},
},
"bar": &VolumeRequest{
Name: "bar",
Type: "host",
Source: "bar-src",
ReadOnly: true,
},
},
},
New: &TaskGroup{
Volumes: map[string]*VolumeRequest{
"foo": &VolumeRequest{
Name: "foo",
Type: "csi",
Source: "foo-src2",
ReadOnly: true,
MountOptions: &CSIMountOptions{
FSType: "ext4",
MountFlags: []string{"relatime", "rw", "nosuid"},
},
},
"bar": &VolumeRequest{ // untouched
Name: "bar",
Type: "host",
Source: "bar-src",
ReadOnly: true,
},
},
},

Expected: &TaskGroupDiff{
Type: DiffTypeEdited,
Objects: []*ObjectDiff{
{
Type: DiffTypeEdited,
Name: "Volume",
Fields: []*FieldDiff{
{
Type: DiffTypeEdited,
Name: "ReadOnly",
Old: "false",
New: "true",
},
{
Type: DiffTypeEdited,
Name: "Source",
Old: "foo-src1",
New: "foo-src2",
},
},
Objects: []*ObjectDiff{
{
Type: DiffTypeEdited,
Name: "MountOptions",
Objects: []*ObjectDiff{
{
Type: DiffTypeAdded,
Name: "MountFlags",
Fields: []*FieldDiff{
{
Type: DiffTypeAdded,
Name: "MountFlags",
Old: "",
New: "nosuid",
},
},
},
},
},
},
},
},
},
},
}

for i, c := range cases {
Expand Down