Merge pull request #7618 from hashicorp/b-shutdown-delay-updates

Fixes bug that prevented group shutdown_delay updates

CHANGELOG.md

@@ -46,6 +46,7 @@ BUG FIXES:
  * connect: Fixed a bug where Connect enabled allocation would not stop after promotion [[GH-7540](https://github.com/hashicorp/nomad/issues/7540)]
  * driver/docker: Fixed handling of seccomp `security_opts` option [[GH-7554](https://github.com/hashicorp/nomad/issues/7554)]
  * driver/docker: Fixed a bug causing docker containers to use swap memory unexpectedly [[GH-7550](https://github.com/hashicorp/nomad/issues/7550)]
+ * scheduler: Fixed a bug where changes to task group `shutdown_delay` were not persisted or displayed in plan output [[GH-7618](https://github.com/hashicorp/nomad/issues/7618)]
  * ui: Fixed handling of multi-byte unicode characters in allocation log view [[GH-7470](https://github.com/hashicorp/nomad/issues/7470)] [[GH-7551](https://github.com/hashicorp/nomad/pull/7551)]
 
 ## 0.10.5 (March 24, 2020)

client/allocrunner/groupservice_hook.go

@@ -111,10 +111,17 @@ func (h *groupServiceHook) Update(req *interfaces.RunnerUpdateRequest) error {
 		networks = req.Alloc.AllocatedResources.Shared.Networks
 	}
 
+	tg := req.Alloc.Job.LookupTaskGroup(h.group)
+	var shutdown time.Duration
+	if tg.ShutdownDelay != nil {
+		shutdown = *tg.ShutdownDelay
+	}
+
 	// Update group service hook fields
 	h.networks = networks
-	h.services = req.Alloc.Job.LookupTaskGroup(h.group).Services
+	h.services = tg.Services
 	h.canary = canary
+	h.delay = shutdown
 	h.taskEnvBuilder.UpdateTask(req.Alloc, nil)
 
 	// Create new task services struct with those new values

client/allocrunner/groupservice_hook_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/nomad/client/consul"
 	"github.com/hashicorp/nomad/client/taskenv"
 	agentconsul "github.com/hashicorp/nomad/command/agent/consul"
+	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/helper/testlog"
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
@@ -56,6 +57,43 @@ func TestGroupServiceHook_NoGroupServices(t *testing.T) {
 	require.Equal(t, "remove", ops[2].Op)
 }
 
+// TestGroupServiceHook_ShutdownDelayUpdate asserts calling group service hooks
+// update updates the hooks delay value.
+func TestGroupServiceHook_ShutdownDelayUpdate(t *testing.T) {
+	t.Parallel()
+
+	alloc := mock.Alloc()
+	alloc.Job.TaskGroups[0].ShutdownDelay = helper.TimeToPtr(10 * time.Second)
+
+	logger := testlog.HCLogger(t)
+	consulClient := consul.NewMockConsulServiceClient(t, logger)
+
+	h := newGroupServiceHook(groupServiceHookConfig{
+		alloc:          alloc,
+		consul:         consulClient,
+		restarter:      agentconsul.NoopRestarter(),
+		taskEnvBuilder: taskenv.NewBuilder(mock.Node(), alloc, nil, alloc.Job.Region),
+		logger:         logger,
+	})
+	require.NoError(t, h.Prerun())
+
+	// Incease shutdown Delay
+	alloc.Job.TaskGroups[0].ShutdownDelay = helper.TimeToPtr(15 * time.Second)
+	req := &interfaces.RunnerUpdateRequest{Alloc: alloc}
+	require.NoError(t, h.Update(req))
+
+	// Assert that update updated the delay value
+	require.Equal(t, h.delay, 15*time.Second)
+
+	// Remove shutdown delay
+	alloc.Job.TaskGroups[0].ShutdownDelay = nil
+	req = &interfaces.RunnerUpdateRequest{Alloc: alloc}
+	require.NoError(t, h.Update(req))
+
+	// Assert that update updated the delay value
+	require.Equal(t, h.delay, 0*time.Second)
+}
+
 // TestGroupServiceHook_GroupServices asserts group service hooks with group
 // services does not error.
 func TestGroupServiceHook_GroupServices(t *testing.T) {

nomad/structs/diff.go

@@ -224,6 +224,20 @@ func (tg *TaskGroup) Diff(other *TaskGroup, contextual bool) (*TaskGroupDiff, er
 		newPrimitiveFlat = flatmap.Flatten(other, filter, true)
 	}
 
+	// ShutdownDelay diff
+	if oldPrimitiveFlat != nil && newPrimitiveFlat != nil {
+		if tg.ShutdownDelay == nil {
+			oldPrimitiveFlat["ShutdownDelay"] = ""
+		} else {
+			oldPrimitiveFlat["ShutdownDelay"] = fmt.Sprintf("%d", *tg.ShutdownDelay)
+		}
+		if other.ShutdownDelay == nil {
+			newPrimitiveFlat["ShutdownDelay"] = ""
+		} else {
+			newPrimitiveFlat["ShutdownDelay"] = fmt.Sprintf("%d", *other.ShutdownDelay)
+		}
+	}
+
 	// Diff the primitive fields.
 	diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, false)
 

nomad/structs/diff_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/nomad/helper"
 	"github.com/stretchr/testify/require"
 )
 
@@ -3029,6 +3030,62 @@ func TestTaskGroupDiff(t *testing.T) {
 				},
 			},
 		},
+		{
+			TestCase: "TaskGroup shutdown_delay edited",
+			Old: &TaskGroup{
+				ShutdownDelay: helper.TimeToPtr(30 * time.Second),
+			},
+			New: &TaskGroup{
+				ShutdownDelay: helper.TimeToPtr(5 * time.Second),
+			},
+			Expected: &TaskGroupDiff{
+				Type: DiffTypeEdited,
+				Fields: []*FieldDiff{
+					{
+						Type: DiffTypeEdited,
+						Name: "ShutdownDelay",
+						Old:  "30000000000",
+						New:  "5000000000",
+					},
+				},
+			},
+		},
+		{
+			TestCase: "TaskGroup shutdown_delay removed",
+			Old: &TaskGroup{
+				ShutdownDelay: helper.TimeToPtr(30 * time.Second),
+			},
+			New: &TaskGroup{},
+			Expected: &TaskGroupDiff{
+				Type: DiffTypeEdited,
+				Fields: []*FieldDiff{
+					{
+						Type: DiffTypeDeleted,
+						Name: "ShutdownDelay",
+						Old:  "30000000000",
+						New:  "",
+					},
+				},
+			},
+		},
+		{
+			TestCase: "TaskGroup shutdown_delay added",
+			Old:      &TaskGroup{},
+			New: &TaskGroup{
+				ShutdownDelay: helper.TimeToPtr(30 * time.Second),
+			},
+			Expected: &TaskGroupDiff{
+				Type: DiffTypeEdited,
+				Fields: []*FieldDiff{
+					{
+						Type: DiffTypeAdded,
+						Name: "ShutdownDelay",
+						Old:  "",
+						New:  "30000000000",
+					},
+				},
+			},
+		},
 	}
 
 	for i, c := range cases {