From 1b184cd2c7cf750fb42559aebf6725edd656b1e8 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 21 Oct 2026 23:48:47 +0000 Subject: [PATCH] hscontrol: cancel ephemeral GC timer when a node is deleted API and logout deletes left the disconnect timer armed; it fired later and logged a bogus ERR for the missing node. --- hscontrol/app.go | 12 ++++++++++-- hscontrol/poll_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/hscontrol/app.go b/hscontrol/app.go index f0a6c3a19..9bf38fbc5 100644 --- a/hscontrol/app.go +++ b/hscontrol/app.go @@ -161,8 +161,8 @@ func NewHeadscale(cfg *types.Config) (*Headscale, error) { ephemeralGC := db.NewEphemeralGarbageCollector(func(ni types.NodeID) { node, ok := app.state.GetNodeByID(ni) if !ok { - log.Error().Uint64("node.id", ni.Uint64()).Msg("ephemeral node deletion failed") - log.Debug().Caller().Uint64("node.id", ni.Uint64()).Msg("ephemeral node deletion failed because node not found in NodeStore") + // A delete that raced this timer already did its job. + log.Debug().Caller().Uint64("node.id", ni.Uint64()).Msg("ephemeral node already deleted, skipping garbage collection") return } @@ -1274,6 +1274,14 @@ func readOrCreatePrivateKey(path string) (*key.MachinePrivate, error) { // All change should be enqueued here and empty will be automatically // ignored. func (h *Headscale) Change(cs ...change.Change) { + // Every delete path publishes its removal here; a deleted node's + // inactivity timer has nothing left to collect. + for _, c := range cs { + for _, id := range c.DeletedNodes { + h.ephemeralGC.Cancel(id) + } + } + h.mapBatcher.AddWork(slices.Concat(cs, h.state.DrainSelfRefreshes())...) } diff --git a/hscontrol/poll_test.go b/hscontrol/poll_test.go index e868dcb21..8172ed1a7 100644 --- a/hscontrol/poll_test.go +++ b/hscontrol/poll_test.go @@ -424,6 +424,45 @@ func TestEphemeralGCDoesNotDeleteReconnectedNode(t *testing.T) { "an idle ephemeral node must still be garbage collected") } +// TestDeleteNodeCancelsEphemeralGC deletes a disconnected ephemeral node +// through the API while its inactivity timer is armed. The timer belongs to +// the deleted node, so the delete must disarm it rather than leave it to fire +// at a node that is no longer there. +func TestDeleteNodeCancelsEphemeralGC(t *testing.T) { + t.Parallel() + + app := createTestApp(t) + app.StartEphemeralGCForTest(t) + + user := app.state.CreateUserForTest("eph-gc-delete-user") + pak, err := app.state.CreatePreAuthKey(user.TypedID(), false, true, nil, nil) + require.NoError(t, err) + + nodeKey := key.NewNode() + + _, err = app.handleRegister(context.Background(), tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{Hostname: "eph-gc-delete-node"}, + Expiry: time.Now().Add(24 * time.Hour), + }, key.NewMachine().Public()) + require.NoError(t, err) + + node, ok := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, ok) + require.True(t, node.IsEphemeral(), "test sanity: node must be ephemeral") + + // The state a disconnect leaves behind. + app.ephemeralGC.Schedule(node.ID(), time.Hour) + require.True(t, app.ephemeralGC.IsScheduled(node.ID()), "test sanity: GC timer must be armed") + + api := registerAPIV2(t, app) + require.Equal(t, http.StatusOK, api.Delete("/api/v2/device/"+node.ID().String()).Code) + + assert.False(t, app.ephemeralGC.IsScheduled(node.ID()), + "deleting a node must cancel its ephemeral GC timer") +} + // TestGitHubIssue3129_TransientlyBlockedWriteDoesNotLeaveLiveStaleSession // tests the scenario reported in // https://github.com/juanfont/headscale/issues/3129.