From e79f8e3e9878b35fb4001feffce03c5c7be07dee Mon Sep 17 00:00:00 2001 From: 3nprob <74199244+3nprob@users.noreply.github.com> Date: Mon, 2 Jun 2025 16:52:20 +0000 Subject: [PATCH] fix: consider volume_mounts in sidecarTaskDiff (#25878) * fix: consider volume_mounts in sidecarTaskDiff * chore: add changelog entry * test: add test for sidecar task diff * fix diff test * make cl match #25528 --------- Co-authored-by: 3np <3np@example.com> Co-authored-by: Michael Schurter --- .changelog/25878.txt | 3 + nomad/structs/diff.go | 5 + nomad/structs/diff_test.go | 254 +++++++++++++++++++++++++++++++++++++ 3 files changed, 262 insertions(+) create mode 100644 .changelog/25878.txt diff --git a/.changelog/25878.txt b/.changelog/25878.txt new file mode 100644 index 000000000..24b0c7444 --- /dev/null +++ b/.changelog/25878.txt @@ -0,0 +1,3 @@ +```release-note:bug +job: Ensure sidecar task volume_mounts are added to planning diff object +``` diff --git a/nomad/structs/diff.go b/nomad/structs/diff.go index 0ae43e74f..4f5116afe 100644 --- a/nomad/structs/diff.go +++ b/nomad/structs/diff.go @@ -1709,6 +1709,11 @@ func sidecarTaskDiff(old, new *SidecarTask, contextual bool) *ObjectDiff { diff.Objects = append(diff.Objects, lDiff) } + // volume_mount diff + if vDiffs := volumeMountsDiffs(old.VolumeMounts, new.VolumeMounts, contextual); vDiffs != nil { + diff.Objects = append(diff.Objects, vDiffs...) + } + return diff } diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 90a5f5c0e..8dc77084e 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -4085,6 +4085,17 @@ func TestTaskGroupDiff(t *testing.T) { }, }, }, + { + Type: DiffTypeDeleted, + Name: "VolumeMount", + Fields: []*FieldDiff{ + {Type: DiffTypeDeleted, Name: "Destination", Old: "/path"}, + {Type: DiffTypeDeleted, Name: "PropagationMode", Old: "private"}, + {Type: DiffTypeDeleted, Name: "ReadOnly", Old: "false"}, + {Type: DiffTypeDeleted, Name: "SELinuxLabel", Old: "Z"}, + {Type: DiffTypeDeleted, Name: "Volume", Old: "vol0"}, + }, + }, }, }, { @@ -10425,3 +10436,246 @@ func TestServicesDiff(t *testing.T) { }) } } + +// TestDiff_SidecarVolumes asserts changes to sidecar task volumes are +// detected. See #25878 +func TestDiff_SidecarVolumes(t *testing.T) { + oldTask := &SidecarTask{ + Name: "old", + Driver: "docker", + User: "sidecar", + Config: map[string]any{"foo": "bar"}, + Env: map[string]string{"FOO": "BAR"}, + Resources: &Resources{ + Cores: 2, + NUMA: &NUMA{ + Affinity: "none", + }, + }, + Meta: map[string]string{"meta": "val"}, + KillTimeout: pointer.Of(10 * time.Second), + LogConfig: &LogConfig{ + MaxFiles: 3, + MaxFileSizeMB: 100, + }, + ShutdownDelay: pointer.Of(20 * time.Second), + KillSignal: "SIGUSR1", + VolumeMounts: []*VolumeMount{ + { + Volume: "foo", + ReadOnly: true, + }, + }, + } + newTask := &SidecarTask{ + Name: "new", + Driver: "podman", + User: "proxy", + Config: map[string]any{"eggs": "spam"}, + Env: map[string]string{"EGGS": "SPAM"}, + Resources: &Resources{ + Cores: 4, + NUMA: &NUMA{ + Affinity: "prefer", + }, + }, + Meta: map[string]string{"meta": "val"}, + KillTimeout: pointer.Of(10 * time.Second), + LogConfig: &LogConfig{ + MaxFiles: 3, + MaxFileSizeMB: 100, + }, + ShutdownDelay: pointer.Of(20 * time.Second), + KillSignal: "SIGUSR1", + VolumeMounts: []*VolumeMount{ + { + Volume: "foo", + ReadOnly: false, + }, + { + Volume: "bar", + ReadOnly: true, + }, + }, + } + expected := &ObjectDiff{ + Type: DiffTypeEdited, + Name: "SidecarTask", + Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "Driver", + Old: "docker", + New: "podman", + }, + { + Type: DiffTypeAdded, + Name: "Env[EGGS]", + Old: "", + New: "SPAM", + }, + { + Type: DiffTypeDeleted, + Name: "Env[FOO]", + Old: "BAR", + New: "", + }, + { + Type: DiffTypeEdited, + Name: "Name", + Old: "old", + New: "new", + }, + { + Type: DiffTypeEdited, + Name: "User", + Old: "sidecar", + New: "proxy", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Config", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "eggs", + Old: "", + New: "spam", + }, + { + Type: DiffTypeDeleted, + Name: "foo", + Old: "bar", + New: "", + }, + }, + }, + { + Type: DiffTypeEdited, + Name: "Resources", + Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "CPU", + Old: "0", + New: "0", + }, + { + Type: DiffTypeEdited, + Name: "Cores", + Old: "2", + New: "4", + }, + { + Type: DiffTypeNone, + Name: "DiskMB", + Old: "0", + New: "0", + }, + { + Type: DiffTypeNone, + Name: "IOPS", + Old: "0", + New: "0", + }, + { + Type: DiffTypeNone, + Name: "MemoryMB", + Old: "0", + New: "0", + }, + { + Type: DiffTypeNone, + Name: "MemoryMaxMB", + Old: "0", + New: "0", + }, + { + Type: DiffTypeNone, + Name: "SecretsMB", + Old: "0", + New: "0", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "NUMA", + Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "Affinity", + Old: "none", + New: "prefer", + }, + }, + }, + }, + }, + { + Type: DiffTypeEdited, + Name: "VolumeMount", + Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Destination", + }, + { + Type: DiffTypeNone, + Name: "PropagationMode", + }, + { + Type: DiffTypeEdited, + Name: "ReadOnly", + Old: "true", + New: "false", + }, + { + Type: DiffTypeNone, + Name: "SELinuxLabel", + }, + { + Type: DiffTypeNone, + Name: "Volume", + Old: "foo", + New: "foo", + }, + }, + }, + { + Type: DiffTypeAdded, + Name: "VolumeMount", + Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Destination", + }, + { + Type: DiffTypeNone, + Name: "PropagationMode", + }, + { + Type: DiffTypeAdded, + Name: "ReadOnly", + New: "true", + }, + { + Type: DiffTypeNone, + Name: "SELinuxLabel", + }, + { + Type: DiffTypeAdded, + Name: "Volume", + Old: "", + New: "bar", + }, + }, + }, + }, + } + + actual := sidecarTaskDiff(oldTask, newTask, true) + must.Eq(t, expected, actual) +}