public/headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale.git synced 2026-10-06 14:50:07 +09:00
Code Issues Packages Projects Releases Wiki Activity

state: keep node removal apart from policy refresh in DeleteNode

Browse Source 
Peers get the deletion as an explicit PeersRemoved change, independent of
their sent-peers tracking.

Updates tailscale/tailscale#15660
This commit is contained in:
Kristoffer Dalby
2026-09-28 09:19:58 +00:00
committed by Kristoffer Dalby
parent 1bd62737b9
commit 60d42808b8
11 changed files with 34 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hscontrol/api/v1/nodes.go
+2 -4
View File
@@ -263,10 +263,8 @@ func registerNodeWriteOps(api huma.API, b Backend) {
return nil, huma.Error404NotFound("node not found")
}
nodeChange, err := b.State.DeleteNode(node)
if !nodeChange.IsEmpty() {
b.Change(nodeChange)
}
changes, err := b.State.DeleteNode(node)
b.Change(changes...)
if err != nil {
return nil, huma.Error500InternalServerError("deleting node", err)
hscontrol/api/v2/devices.go
+2 -4
View File
@@ -177,10 +177,8 @@ func registerDevices(api huma.API, b Backend) {
return nil, err
}
nodeChange, err := b.State.DeleteNode(node)
if !nodeChange.IsEmpty() {
b.Change(nodeChange)
}
changes, err := b.State.DeleteNode(node)
b.Change(changes...)
if err != nil {
return nil, mapError("deleting device", err)
hscontrol/app.go
+2 -4
View File
@@ -155,10 +155,8 @@ func NewHeadscale(cfg *types.Config) (*Headscale, error) {
return
}
policyChanged, err := app.state.DeleteNode(node)
if !policyChanged.IsEmpty() {
app.Change(policyChanged)
}
changes, err := app.state.DeleteNode(node)
app.Change(changes...)
if err != nil {
log.Error().Err(err).EmbedObject(node).Msg("ephemeral node deletion failed")
hscontrol/auth.go
+2 -4
View File
@@ -215,10 +215,8 @@ func (h *Headscale) handleLogout(
EmbedObject(node).
Msg("Deleting ephemeral node during logout")
c, err := h.state.DeleteNode(node)
if !c.IsEmpty() {
h.Change(c)
}
changes, err := h.state.DeleteNode(node)
h.Change(changes...)
if err != nil {
return nil, fmt.Errorf("deleting ephemeral node: %w", err)
hscontrol/mapper/batcher_test.go
+2 -3
View File
@@ -2160,9 +2160,8 @@ func TestNodeDeletedWhileChangesPending(t *testing.T) {
// Delete the node from state - this returns a NodeRemoved change
// In production, this change is sent to batcher via app.Change()
nodeChange, err := st.DeleteNode(nodeToDelete)
nodeChanges, err := st.DeleteNode(nodeToDelete)
require.NoError(t, err, "should be able to delete node from state")
t.Logf("Deleted node %d from state, change: %s", node3.n.ID, nodeChange.Reason)
// Verify node is deleted from state
_, exists := st.GetNodeByID(node3.n.ID)
@@ -2170,7 +2169,7 @@ func TestNodeDeletedWhileChangesPending(t *testing.T) {
// Send the NodeRemoved change to batcher (this is what app.Change() does)
// With the fix, this should clean up node3 from batcher's internal state
batcher.AddWork(nodeChange)
batcher.AddWork(nodeChanges...)
// Wait for the batcher to process the removal and clean up the node
assert.EventuallyWithT(t, func(c *assert.CollectT) {
hscontrol/poll_test.go
+4 -4
View File
@@ -514,9 +514,9 @@ func TestDeletedNodeEndsLongPoll(t *testing.T) {
t.Fatal("expected the initial map write to start")
}
c, err := app.state.DeleteNode(nodeView)
cs, err := app.state.DeleteNode(nodeView)
require.NoError(t, err)
app.Change(c)
app.Change(cs...)
// The reconnect grace period is 10s, so a generous bound here still fails
// if teardown waits for a node that can never come back.
@@ -595,9 +595,9 @@ func TestDeletedNodeInterruptsBlockedWrite(t *testing.T) {
t.Fatal("expected the initial map write to block")
}
c, err := app.state.DeleteNode(nodeView)
cs, err := app.state.DeleteNode(nodeView)
require.NoError(t, err)
app.Change(c)
app.Change(cs...)
select {
case <-serveDone:
hscontrol/servertest/issues_test.go
+3 -3
View File
@@ -556,7 +556,7 @@ func TestIssuesServerMutations(t *testing.T) {
deleteChange, err := srv.State().DeleteNode(node2View)
require.NoError(t, err)
srv.App.Change(deleteChange)
srv.App.Change(deleteChange...)
c1.WaitForCondition(t, "deleted peer gone", 10*time.Second,
func(nm *netmap.NetworkMap) bool {
@@ -1041,7 +1041,7 @@ func TestPeerRemovedAsDelta(t *testing.T) {
changes, err := srv.State().DeleteNode(node2)
require.NoError(t, err)
srv.App.Change(changes)
srv.App.Change(changes...)
assertRemovedAsDelta(t, c1, nodeID2)
})
@@ -1060,7 +1060,7 @@ func TestPeerRemovedAsDelta(t *testing.T) {
changes, err := srv.State().DeleteNode(node2)
require.NoError(t, err)
srv.App.Change(changes)
srv.App.Change(changes...)
c1.Reconnect(t)
hscontrol/servertest/race_test.go
+1 -1
View File
@@ -197,7 +197,7 @@ func TestRaceConcurrentServerMutations(t *testing.T) {
wg.Go(func() {
delChange, err := srv.State().DeleteNode(nv2)
if err == nil {
srv.App.Change(delChange)
srv.App.Change(delChange...)
}
})
hscontrol/servertest/stress_test.go
+1 -1
View File
@@ -121,7 +121,7 @@ func TestStressConnectDisconnect(t *testing.T) {
deleteChange, err := srv.State().DeleteNode(nv)
require.NoError(t, err)
srv.App.Change(deleteChange)
srv.App.Change(deleteChange...)
// c2 should see c1 disappear.
c2.WaitForCondition(t, "deleted node gone", 10*time.Second,
hscontrol/state/persist_test.go
+5 -4
View File
@@ -512,7 +512,7 @@ func TestDeleteNodeKeepsStoreOnDBFailure(t *testing.T) {
c, err := s.DeleteNode(node)
require.NoError(t, s.db.DB.Callback().Delete().Remove("fail_node_delete"))
require.ErrorIs(t, err, errInjectedNodeDelete)
assert.True(t, c.IsEmpty(), "an uncommitted deletion must not stop the node's session")
assert.Empty(t, c, "an uncommitted deletion must not stop the node's session")
_, ok = s.GetNodeByID(nodeID)
assert.True(t, ok, "a database failure must leave the in-memory node available")
@@ -530,11 +530,12 @@ func TestDeleteNodeReturnsRemovalOnPolicyFailure(t *testing.T) {
s.polMan = failingSetNodesPolicyManager{PolicyManager: s.polMan}
c, err := s.DeleteNode(node)
cs, err := s.DeleteNode(node)
require.ErrorIs(t, err, errInjectedPolicyNodeUpdate)
assert.Equal(t, []types.NodeID{nodeID}, c.PeersRemoved,
require.NotEmpty(t, cs)
assert.Equal(t, []types.NodeID{nodeID}, cs[0].PeersRemoved,
"a committed deletion must still notify peers and stop the node's session")
assert.Equal(t, []types.NodeID{nodeID}, c.DeletedNodes,
assert.Equal(t, []types.NodeID{nodeID}, cs[0].DeletedNodes,
"a committed deletion must identify the session to stop")
_, ok = s.GetNodeByID(nodeID)
hscontrol/state/state.go
+10 -15
View File
@@ -612,11 +612,11 @@ func (s *State) SaveNode(node types.NodeView) (types.NodeView, change.Change, er
}
// DeleteNode permanently removes a node and cleans up associated resources.
// Once the database deletion commits, the returned change always contains the
// node-removal notification, even if a later policy refresh fails. Callers must
// publish a non-empty change before handling the error so live sessions are
// Once the database deletion commits, the returned changes always start with
// the node-removal notification, even if a later policy refresh fails. Callers
// must publish the changes before handling the error so live sessions are
// still torn down after a committed deletion.
func (s *State) DeleteNode(node types.NodeView) (change.Change, error) {
func (s *State) DeleteNode(node types.NodeView) ([]change.Change, error) {
genBefore := s.polMan.NodesGeneration()
s.persistMu.Lock()
@@ -625,7 +625,7 @@ func (s *State) DeleteNode(node types.NodeView) (change.Change, error) {
if err != nil {
s.persistMu.Unlock()
return change.Change{}, err
return nil, err
}
// The database is the durable source of truth. Only remove the in-memory
@@ -636,21 +636,16 @@ func (s *State) DeleteNode(node types.NodeView) (change.Change, error) {
s.ipAlloc.FreeIPs(node.IPs())
c := change.NodeRemoved(node.ID())
// An explicit removal of its own, ahead of the policy refresh, so peers
// learn of the deletion without depending on their sent-peers tracking.
removed := change.NodeRemoved(node.ID())
// Check if policy manager needs updating after node deletion
policyChange, err := s.updatePolicyManagerNodes(genBefore)
if err != nil {
return c.Merge(policyChange), fmt.Errorf("updating policy manager after node deletion: %w", err)
return []change.Change{removed, policyChange}, fmt.Errorf("updating policy manager after node deletion: %w", err)
}
if !policyChange.IsEmpty() {
// Merge policy change with NodeRemoved to preserve PeersRemoved info
// This ensures the batcher cleans up the deleted node from its state
c = c.Merge(policyChange)
}
return c, nil
return []change.Change{removed, policyChange}, nil
}
// Connect acquires a control session and returns the resulting changes