From c7e7fdfa845e40597b0f9d65c94a8b8b34f174ec Mon Sep 17 00:00:00 2001 From: Juana De La Cuesta Date: Wed, 27 Mar 2024 11:43:31 +0100 Subject: [PATCH] [f-gh-208] Force recreation and redeployment of task if volume label changes (#20074) Scheduler: Force recreation and redeployment of task if volume mount labels in the task definitions changes --- nomad/structs/volumes.go | 5 +++++ scheduler/util.go | 32 ++++++++++++++++++++++++++++++++ scheduler/util_test.go | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+) diff --git a/nomad/structs/volumes.go b/nomad/structs/volumes.go index 1b7c5a804..daacd5d86 100644 --- a/nomad/structs/volumes.go +++ b/nomad/structs/volumes.go @@ -251,6 +251,11 @@ type VolumeMount struct { SELinuxLabel string } +// Hash is a very basic string based implementation of a hasher. +func (v *VolumeMount) Hash() string { + return fmt.Sprintf("%#+v", v) +} + func (v *VolumeMount) Equal(o *VolumeMount) bool { if v == nil || o == nil { return v == o diff --git a/scheduler/util.go b/scheduler/util.go index 13151a2aa..57eaa55ed 100644 --- a/scheduler/util.go +++ b/scheduler/util.go @@ -12,6 +12,7 @@ import ( log "github.com/hashicorp/go-hclog" memdb "github.com/hashicorp/go-memdb" + "github.com/hashicorp/go-set/v2" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/nomad/structs" ) @@ -334,6 +335,11 @@ func tasksUpdated(jobA, jobB *structs.Job, taskGroup string) comparison { return difference("task log disabled", at.LogConfig.Disabled, bt.LogConfig.Disabled) } + // Check volume mount updates + if c := volumeMountsUpdated(at.VolumeMounts, bt.VolumeMounts); c.modified { + return c + } + // Check if restart.render_templates is updated if c := renderTemplatesUpdated(at.RestartPolicy, bt.RestartPolicy, "task restart render_templates"); c.modified { @@ -424,6 +430,32 @@ func connectServiceUpdated(servicesA, servicesB []*structs.Service) comparison { return same } +func volumeMountsUpdated(a, b []*structs.VolumeMount) comparison { + setA := set.HashSetFrom(a) + setB := set.HashSetFrom(b) + + if setA.Equal(setB) { + return same + } + + return difference("volume mounts", a, b) +} + +// volumeMountUpdated returns true if the definition of the volume mount +// has been updated in a manner that will requires the task to be recreated. +func volumeMountUpdated(mountA, mountB *structs.VolumeMount) comparison { + if mountA != nil && mountB == nil { + difference("volume mount removed", mountA, mountB) + } + + if mountA != nil && mountB != nil && + mountA.SELinuxLabel != mountB.SELinuxLabel { + return difference("volume mount selinux label", mountA.SELinuxLabel, mountB.SELinuxLabel) + } + + return same +} + // connectUpdated returns true if the connect block has been updated in a manner // that will require a destructive update. // diff --git a/scheduler/util_test.go b/scheduler/util_test.go index a70493338..b0d17b37a 100644 --- a/scheduler/util_test.go +++ b/scheduler/util_test.go @@ -547,6 +547,42 @@ func TestTasksUpdated(t *testing.T) { // Compare changed Template ErrMissingKey j30.TaskGroups[0].Tasks[0].Templates[0].ErrMissingKey = true must.True(t, tasksUpdated(j29, j30, name).modified) + + // Compare identical volume mounts + j31 := mock.Job() + j32 := j31.Copy() + + must.False(t, tasksUpdated(j31, j32, name).modified) + + // Modify volume mounts + j31.TaskGroups[0].Tasks[0].VolumeMounts = []*structs.VolumeMount{ + { + Volume: "myvolume", + SELinuxLabel: "z", + }, + } + + j32.TaskGroups[0].Tasks[0].VolumeMounts = []*structs.VolumeMount{ + { + Volume: "myvolume", + SELinuxLabel: "", + }, + } + + must.True(t, tasksUpdated(j31, j32, name).modified) + + // Add volume mount + j32.TaskGroups[0].Tasks[0].VolumeMounts = append(j32.TaskGroups[0].Tasks[0].VolumeMounts, + &structs.VolumeMount{ + Volume: "myvolume2", + SELinuxLabel: "Z", + }) + + // Remove volume mount + j32 = j31.Copy() + j32.TaskGroups[0].Tasks[0].VolumeMounts = nil + + must.True(t, tasksUpdated(j31, j32, name).modified) } func TestTasksUpdated_connectServiceUpdated(t *testing.T) {