From d2849b8a7678110f2c4643d5036ab55e6872770b Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Thu, 23 Nov 2023 14:51:10 -0500 Subject: [PATCH] cli: skip allocs with replacements on job restart (#19155) The `nomad job restart` command should skip allocations that already have replacements. Restarting an allocation with a replacement is a no-op because the allocation status is terminal and the command's replacement monitor returns immediatelly. But by not skipping them, the effective batch size is computed incorrectly. --- .changelog/19155.txt | 3 +++ command/job_restart.go | 13 +++++++++++++ command/job_restart_test.go | 15 +++++++++++++++ 3 files changed, 31 insertions(+) create mode 100644 .changelog/19155.txt diff --git a/.changelog/19155.txt b/.changelog/19155.txt new file mode 100644 index 000000000..2060c184e --- /dev/null +++ b/.changelog/19155.txt @@ -0,0 +1,3 @@ +```release-note:bug +cli: Fixed a bug that caused the `nomad job restart` command to miscount the allocations to restart +``` diff --git a/command/job_restart.go b/command/job_restart.go index 8a0888c23..395f4b84b 100644 --- a/command/job_restart.go +++ b/command/job_restart.go @@ -633,6 +633,19 @@ func (c *JobRestartCommand) filterAllocs(stubs []AllocationListStubWithJob) []Al continue } + // Skip allocations that have already been replaced. + if stub.NextAllocation != "" { + if c.verbose { + c.Ui.Output(c.Colorize().Color(fmt.Sprintf( + "[dark_gray] %s: Skipping allocation %q because it has already been replaced by %q[reset]", + formatTime(time.Now()), + shortAllocID, + limit(stub.NextAllocation, c.length), + ))) + } + continue + } + // Skip allocations for groups that were not requested. if c.groups.Size() > 0 { if !c.groups.Contains(stub.TaskGroup) { diff --git a/command/job_restart_test.go b/command/job_restart_test.go index 442cc14aa..ee50f935f 100644 --- a/command/job_restart_test.go +++ b/command/job_restart_test.go @@ -1245,6 +1245,21 @@ func TestJobRestartCommand_filterAllocs(t *testing.T) { } allocs[key] = alloc allAllocs = append(allAllocs, alloc) + + // Allocations with a replacement must always be skipped. + replacedAlloc := AllocationListStubWithJob{ + AllocationListStub: &api.AllocationListStub{ + ID: key, + JobVersion: *job.Version, + TaskGroup: *tg.Name, + DesiredStatus: desired, + ClientStatus: client, + NextAllocation: alloc.ID, + }, + Job: job, + } + allocs[key+"_replaced"] = replacedAlloc + allAllocs = append(allAllocs, replacedAlloc) } } }