Skip to content

Commit

Permalink
volumes: implement plan diff for volume requests
Browse files Browse the repository at this point in the history 
The details of host volume and CSI volume requests do not show up in `nomad
plan` outputs, although the updates are detected by the scheduler and result
in an update as expected.
  • Loading branch information
@tgross
tgross committed Feb 4, 2021
1 parent 84b8a19 commit a617b7e
Show file tree
Hide file tree
Showing 3 changed files with 244 additions and 3 deletions.
6 changes: 3 additions & 3 deletions CHANGELOG.md
View file
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
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
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

0 comments on commit a617b7e

Please sign in to comment.