diff --git a/CHANGELOG.md b/CHANGELOG.md index edb051b00d8f..b29857466eff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/nomad/structs/diff.go b/nomad/structs/diff.go index 6a08e7c4e7b8..4d4c7a5fc540 100644 --- a/nomad/structs/diff.go +++ b/nomad/structs/diff.go @@ -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 { @@ -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 { diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 46c94a8828f9..4056b417818f 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -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 {