db, state: let only the delete that removed a node free its addresses

Fixes ANT-2026-NWTIBB02

CHANGELOG.md

@@ -166,6 +166,7 @@ the ACME DNS-01 challenge through a built-in DNS provider: `acmedns`,
 - A registration follow-up from a machine other than the one that started the registration is refused before it waits
 - Only the newest follow-up waits on a pending registration or SSH check; older ones are answered at once
 - Registration locks are freed once no registration uses them
+- Concurrent deletes of one node free its addresses and announce its removal once
 
 ## 0.29.7 (202x-xx-xx)
 

hscontrol/db/node.go

@@ -232,34 +232,24 @@ func (hsdb *HSDatabase) DeleteNode(node *types.Node) error {
 
 // DeleteNode deletes a [types.Node] from the database.
 // Caller is responsible for notifying all of change.
+// Returns [ErrNodeNotFound] when the row is already gone, so only the call
+// that removed it acts on the deletion.
 func DeleteNode(tx *gorm.DB,
 	node *types.Node,
 ) error {
 	// Unscoped causes the node to be fully removed from the database.
-	err := tx.Unscoped().Delete(&types.Node{}, node.ID).Error
+	result := tx.Unscoped().Delete(&types.Node{}, node.ID)
-	if err != nil {
+	if result.Error != nil {
-		return err
+		return result.Error
+	}
+
+	if result.RowsAffected == 0 {
+		return ErrNodeNotFound
 	}
 
 	return nil
 }
 
-// DeleteEphemeralNode deletes a [types.Node] from the database, note that this method
-// will remove it straight, and not notify any changes or consider any routes.
-// It is intended for Ephemeral nodes.
-func (hsdb *HSDatabase) DeleteEphemeralNode(
-	nodeID types.NodeID,
-) error {
-	return hsdb.Write(func(tx *gorm.DB) error {
-		err := tx.Unscoped().Delete(&types.Node{}, nodeID).Error
-		if err != nil {
-			return err
-		}
-
-		return nil
-	})
-}
-
 // RegisterNodeForTest is used only for testing purposes to register a node directly in the database.
 // Production code should use [state.State.HandleNodeFromAuthPath] or [state.State.HandleNodeFromPreAuthKey].
 func RegisterNodeForTest(tx *gorm.DB, node types.Node, ipv4 *netip.Addr, ipv6 *netip.Addr) (*types.Node, error) {

hscontrol/db/node_test.go

@@ -67,6 +67,8 @@ func TestHardDeleteNode(t *testing.T) {
 
 	_, err = db.getNode(types.UserID(user.ID), "testnode3")
 	require.Error(t, err)
+
+	require.ErrorIs(t, db.DeleteNode(node), ErrNodeNotFound)
 }
 
 func TestListPeersManyNodes(t *testing.T) {

hscontrol/state/persist_test.go

@@ -573,6 +573,45 @@ func (failingSetNodesPolicyManager) SetNodes(views.Slice[types.NodeView]) (bool,
 	return false, errInjectedPolicyNodeUpdate
 }
 
+func TestDeleteNodeTwiceReleasesAddressOnce(t *testing.T) {
+	prefix := netip.MustParsePrefix("100.64.0.0/30")
+	cfg := persistTestConfig(t.TempDir() + "/headscale.db")
+	cfg.PrefixV4 = &prefix
+	cfg.PrefixV6 = nil
+	cfg.IPAllocation = types.IPAllocationStrategyRandom
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	user := s.CreateUserForTest("delete-user")
+	pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
+	require.NoError(t, err)
+	node, _, err := s.HandleNodeFromPreAuthKey(tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+		NodeKey:  key.NewNode().Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "delete-node"},
+	}, key.NewMachine().Public())
+	require.NoError(t, err)
+
+	stale := node
+
+	_, _, err = s.ipAlloc.Next()
+	require.NoError(t, err)
+	_, err = s.DeleteNode(node)
+	require.NoError(t, err)
+	reissued, _, err := s.ipAlloc.Next()
+	require.NoError(t, err)
+	require.Equal(t, node.IPv4().Get(), *reissued)
+
+	changes, err := s.DeleteNode(stale)
+	require.NoError(t, err)
+	assert.Empty(t, changes)
+
+	_, _, err = s.ipAlloc.Next()
+	require.ErrorIs(t, err, db.ErrCouldNotAllocateIP)
+}
+
 func TestDeleteNodeKeepsStoreOnDBFailure(t *testing.T) {
 	_, s, nodeID := persistTestSetup(t)
 	t.Cleanup(func() { _ = s.Close() })

hscontrol/state/state.go

@@ -710,7 +710,9 @@ func (s *State) SaveNode(node types.NodeView) (types.NodeView, change.Change, er
 // 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.
+// still torn down after a committed deletion. Concurrent deletes of one node
+// act once: a call that finds the row already gone returns no changes, and
+// only the call that removed the row frees the addresses.
 func (s *State) DeleteNode(node types.NodeView) ([]change.Change, error) {
 	genBefore := s.polMan.NodesGeneration()
 
@@ -720,6 +722,10 @@ func (s *State) DeleteNode(node types.NodeView) ([]change.Change, error) {
 	s.persistMu.Unlock()
 
 	if err != nil {
+		if errors.Is(err, hsdb.ErrNodeNotFound) {
+			return nil, nil
+		}
+
 		return nil, err
 	}