diff --git a/.changelog/28185.txt b/.changelog/28185.txt new file mode 100644 index 00000000000..03ad21b0b71 --- /dev/null +++ b/.changelog/28185.txt @@ -0,0 +1,3 @@ +```release-note:bug +client: Fixed a bug where a client could panic after an alloc is GC'd +``` diff --git a/nomad/alloc_endpoint.go b/nomad/alloc_endpoint.go index 77f1d6e22e2..58b8f1c7a0e 100644 --- a/nomad/alloc_endpoint.go +++ b/nomad/alloc_endpoint.go @@ -6,6 +6,7 @@ package nomad import ( "fmt" "net/http" + "slices" "time" "github.com/hashicorp/go-hclog" @@ -199,6 +200,7 @@ func (a *Alloc) GetAllocs(args *structs.AllocsGetRequest, } defer metrics.MeasureSince([]string{"nomad", "alloc", "get_allocs"}, time.Now()) + // The *maximum* we'll return is the number requested. allocs := make([]*structs.Allocation, len(args.AllocIDs)) // Setup the blocking query. We wait for at least one of the requested @@ -249,6 +251,10 @@ func (a *Alloc) GetAllocs(args *structs.AllocsGetRequest, // Setup the output if thresholdMet { + // Filter out nils, only if there are any to avoid superfluous mallocs. + if slices.Contains(allocs, nil) { + allocs = slices.DeleteFunc(allocs, func(a *structs.Allocation) bool { return a == nil }) + } reply.Allocs = allocs reply.Index = maxIndex } else { diff --git a/nomad/alloc_endpoint_test.go b/nomad/alloc_endpoint_test.go index 3c710c2b31a..4d7ed1a5658 100644 --- a/nomad/alloc_endpoint_test.go +++ b/nomad/alloc_endpoint_test.go @@ -989,6 +989,20 @@ func TestAllocEndpoint_GetAllocs(t *testing.T) { if err := msgpackrpc.CallWithCodec(codec, "Alloc.GetAllocs", get, &resp); err == nil { t.Fatalf("expect error") } + + // Lookup mixed existing and not-existing allocs to assert partial results. + get = &structs.AllocsGetRequest{ + AllocIDs: []string{alloc.ID, "00000000-0000-0000-0000-000000000000", alloc2.ID}, + QueryOptions: structs.QueryOptions{ + Region: "global", + AuthToken: node.SecretID, + }, + } + must.NoError(t, msgpackrpc.CallWithCodec(codec, "Alloc.GetAllocs", get, &resp)) + // The reply should include only the 2 real allocs, and importantly *not* any nils. + must.Len(t, 2, resp.Allocs, must.Sprint("should only include the two existing allocs")) + must.NotNil(t, resp.Allocs[0], must.Sprint("alloc should not be nil")) + must.NotNil(t, resp.Allocs[1], must.Sprint("alloc should not be nil")) } func TestAllocEndpoint_GetAllocs_Blocking(t *testing.T) {