Skip to content

Commit

Permalink
persist shared ports during inplace updates
Browse files Browse the repository at this point in the history
AllocatedSharedResources were not being copied over to the new
allocation struct the scheduler makes during inplace updates. This
caused downstream issues after the plan was applied, namely the shared
ports were dropped causing issues with service
registration/deregistration.

test that shared ports are preserved

change log, also carry over shared network

copy networks
  • Loading branch information
drewbailey committed Jan 8, 2021
1 parent 463fed9 commit 1cc88d7
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ IMPROVEMENTS:

BUG FIXES:
* client: Fixed a bug where clients configured with `cpu_total_compute` did not update the `cpu.totalcompute` node attribute. [[GH-9532](https://github.com/hashicorp/nomad/issues/9532)]
* core: Fixed a bug where an in place update dropped an allocations shared allocated resources [[GH-9736](https://github.com/hashicorp/nomad/issues/9736)]
* consul: Fixed a bug where updating a task to include services would not work [[GH-9707](https://github.com/hashicorp/nomad/issues/9707)]
* consul: Fixed alloc address mode port advertisement to use the mapped `to` port value [[GH-9730](https://github.com/hashicorp/nomad/issues/9730)]
* consul/connect: Fixed a bug where absent ingress envoy proxy configuration could panic client [[GH-9669](https://github.com/hashicorp/nomad/issues/9669)]
Expand Down
4 changes: 3 additions & 1 deletion scheduler/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,9 @@ func inplaceUpdate(ctx Context, eval *structs.Evaluation, job *structs.Job,
Tasks: option.TaskResources,
TaskLifecycles: option.TaskLifecycles,
Shared: structs.AllocatedSharedResources{
DiskMB: int64(update.TaskGroup.EphemeralDisk.SizeMB),
DiskMB: int64(update.TaskGroup.EphemeralDisk.SizeMB),
Ports: update.Alloc.AllocatedResources.Shared.Ports,
Networks: update.Alloc.AllocatedResources.Shared.Networks.Copy(),
},
}
newAlloc.Metrics = ctx.Metrics()
Expand Down
57 changes: 57 additions & 0 deletions scheduler/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,63 @@ func TestInplaceUpdate_ChangedTaskGroup(t *testing.T) {
require.Empty(t, ctx.plan.NodeAllocation, "inplaceUpdate incorrectly did an inplace update")
}

func TestInplaceUpdate_AllocatedResources(t *testing.T) {
state, ctx := testContext(t)
eval := mock.Eval()
job := mock.Job()

node := mock.Node()
require.NoError(t, state.UpsertNode(structs.MsgTypeTestSetup, 900, node))

// Register an alloc
alloc := &structs.Allocation{
Namespace: structs.DefaultNamespace,
ID: uuid.Generate(),
EvalID: eval.ID,
NodeID: node.ID,
JobID: job.ID,
Job: job,
AllocatedResources: &structs.AllocatedResources{
Shared: structs.AllocatedSharedResources{
Ports: structs.AllocatedPorts{
{
Label: "api-port",
Value: 19910,
To: 8080,
},
},
},
},
DesiredStatus: structs.AllocDesiredStatusRun,
TaskGroup: "web",
}
alloc.TaskResources = map[string]*structs.Resources{"web": alloc.Resources}
require.NoError(t, state.UpsertJobSummary(1000, mock.JobSummary(alloc.JobID)))
require.NoError(t, state.UpsertAllocs(structs.MsgTypeTestSetup, 1001, []*structs.Allocation{alloc}))

// Update TG to add a new service (inplace)
tg := job.TaskGroups[0]
tg.Services = []*structs.Service{
{
Name: "tg-service",
},
}

updates := []allocTuple{{Alloc: alloc, TaskGroup: tg}}
stack := NewGenericStack(false, ctx)

// Do the inplace update.
unplaced, inplace := inplaceUpdate(ctx, eval, job, stack, updates)

require.True(t, len(unplaced) == 0 && len(inplace) == 1, "inplaceUpdate incorrectly did not perform an inplace update")
require.NotEmpty(t, ctx.plan.NodeAllocation, "inplaceUpdate incorrectly did an inplace update")
require.NotEmpty(t, ctx.plan.NodeAllocation[node.ID][0].AllocatedResources.Shared.Ports)

port, ok := ctx.plan.NodeAllocation[node.ID][0].AllocatedResources.Shared.Ports.Get("api-port")
require.True(t, ok)
require.Equal(t, 19910, port.Value)
}

func TestInplaceUpdate_NoMatch(t *testing.T) {
state, ctx := testContext(t)
eval := mock.Eval()
Expand Down

0 comments on commit 1cc88d7

Please sign in to comment.