Skip to content

Commit

Permalink
Group shutdown delay fixes
Browse files Browse the repository at this point in the history
Group shutdown delay updates were not properly handled in Update hook.
This commit also ensures that plan output is displayed.
  • Loading branch information
drewbailey committed Apr 6, 2020
1 parent 5af7b0f commit b81a001
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 1 deletion.
10 changes: 9 additions & 1 deletion client/allocrunner/groupservice_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,19 @@ func (h *groupServiceHook) Update(req *interfaces.RunnerUpdateRequest) error {
}

// Update group service hook fields
tg := req.Alloc.Job.LookupTaskGroup(h.group)
h.networks = networks
h.services = req.Alloc.Job.LookupTaskGroup(h.group).Services
h.services = tg.Services
h.canary = canary
h.taskEnvBuilder.UpdateTask(req.Alloc, nil)

// Update shutdown delay
var shutdown time.Duration
if tg.ShutdownDelay != nil {
shutdown = *tg.ShutdownDelay
}
h.delay = shutdown

// Create new task services struct with those new values
newWorkloadServices := h.getWorkloadServices()

Expand Down
29 changes: 29 additions & 0 deletions client/allocrunner/groupservice_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -56,6 +57,34 @@ 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())

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)
}

// TestGroupServiceHook_GroupServices asserts group service hooks with group
// services does not error.
func TestGroupServiceHook_GroupServices(t *testing.T) {
Expand Down
12 changes: 12 additions & 0 deletions nomad/structs/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,18 @@ func (tg *TaskGroup) Diff(other *TaskGroup, contextual bool) (*TaskGroupDiff, er
newPrimitiveFlat = flatmap.Flatten(other, filter, true)
}

// ShutdownDelay diff
if tg.ShutdownDelay == nil {
oldPrimitiveFlat["ShutdownDelay"] = "nil"
} else {
oldPrimitiveFlat["ShutdownDelay"] = fmt.Sprintf("%d", *tg.ShutdownDelay)
}
if other.ShutdownDelay == nil {
newPrimitiveFlat["ShutdownDelay"] = "nil"
} else {
newPrimitiveFlat["ShutdownDelay"] = fmt.Sprintf("%d", *other.ShutdownDelay)
}

// Diff the primitive fields.
diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, false)

Expand Down
21 changes: 21 additions & 0 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"
"time"

"github.com/hashicorp/nomad/helper"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -3029,6 +3030,26 @@ 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",
},
},
},
},
}

for i, c := range cases {
Expand Down

0 comments on commit b81a001

Please sign in to comment.