host volumes: add configuration to GC on node GC

tgross · tgross · commit 9a010e871cf4 · 2025-05-21T11:51:51.000-04:00
When a node is garbage collected, any dynamic host volumes on the node are orphaned in the state store. We generally don't want to automatically collect these volumes and risk data loss, and have provided a CLI flag to `-force` remove them in #25902. But for clusters running on ephemeral cloud instances (ex. AWS EC2 in an autoscaling group), deleting host volumes may add excessive friction. Add a configuration knob to the client configuration to remove host volumes from the state store on node GC. Ref: #25902 Ref: #25762 Ref: https://hashicorp.atlassian.net/browse/NMD-705
diff --git a/.changelog/25903.txt b/.changelog/25903.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+client: Add gc_volumes_on_node_gc configuration to delete host volumes when nodes are garbage collected
+```
diff --git a/api/nodes.go b/api/nodes.go
@@ -566,6 +566,7 @@ type Node struct {
 	Events                []*NodeEvent
 	Drivers               map[string]*DriverInfo
 	HostVolumes           map[string]*HostVolumeInfo
+	GCVolumesOnNodeGC     bool
 	HostNetworks          map[string]*HostNetworkInfo
 	CSIControllerPlugins  map[string]*CSIInfo
 	CSINodePlugins        map[string]*CSIInfo
diff --git a/client/client.go b/client/client.go
@@ -1595,6 +1595,8 @@ func (c *Client) setupNode() error {
 			node.HostVolumes[k] = v.Copy()
 		}
 	}
+	node.GCVolumesOnNodeGC = newConfig.GCVolumesOnNodeGC
+
 	if node.HostNetworks == nil {
 		if l := len(newConfig.HostNetworks); l != 0 {
 			node.HostNetworks = make(map[string]*structs.ClientHostNetworkConfig, l)
diff --git a/client/config/config.go b/client/config/config.go
@@ -242,6 +242,11 @@ type Config struct {
 	// before garbage collection is triggered.
 	GCMaxAllocs int
 
+	// GCVolumesOnNodeGC indicates that the server should GC any dynamic host
+	// volumes on this node when the node is GC'd. This should only be set if
+	// you know that a GC'd node can never come back
+	GCVolumesOnNodeGC bool
+
 	// NoHostUUID disables using the host's UUID and will force generation of a
 	// random UUID.
 	NoHostUUID bool
diff --git a/command/agent/agent.go b/command/agent/agent.go
@@ -946,6 +946,8 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) {
 	conf.GCDiskUsageThreshold = agentConfig.Client.GCDiskUsageThreshold
 	conf.GCInodeUsageThreshold = agentConfig.Client.GCInodeUsageThreshold
 	conf.GCMaxAllocs = agentConfig.Client.GCMaxAllocs
+	conf.GCVolumesOnNodeGC = agentConfig.Client.GCVolumesOnNodeGC
+
 	if agentConfig.Client.NoHostUUID != nil {
 		conf.NoHostUUID = *agentConfig.Client.NoHostUUID
 	} else {
diff --git a/command/agent/config.go b/command/agent/config.go
@@ -341,6 +341,11 @@ type ClientConfig struct {
 	// before garbage collection is triggered.
 	GCMaxAllocs int `hcl:"gc_max_allocs"`
 
+	// GCVolumesOnNodeGC indicates that the server should GC any dynamic host
+	// volumes on this node when the node is GC'd. This should only be set if
+	// you know that a GC'd node can never come back
+	GCVolumesOnNodeGC bool `hcl:"gc_volumes_on_node_gc"`
+
 	// NoHostUUID disables using the host's UUID and will force generation of a
 	// random UUID.
 	NoHostUUID *bool `hcl:"no_host_uuid"`
@@ -2543,6 +2548,9 @@ func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig {
 	if b.GCMaxAllocs != 0 {
 		result.GCMaxAllocs = b.GCMaxAllocs
 	}
+	if b.GCVolumesOnNodeGC {
+		result.GCVolumesOnNodeGC = b.GCVolumesOnNodeGC
+	}
 	// NoHostUUID defaults to true, merge if false
 	if b.NoHostUUID != nil {
 		result.NoHostUUID = b.NoHostUUID
diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go
@@ -88,6 +88,7 @@ var basicConfig = &Config{
 		GCDiskUsageThreshold:  82,
 		GCInodeUsageThreshold: 91,
 		GCMaxAllocs:           50,
+		GCVolumesOnNodeGC:     true,
 		NoHostUUID:            pointer.Of(false),
 		DisableRemoteExec:     true,
 		HostVolumes: []*structs.ClientHostVolumeConfig{
diff --git a/command/agent/testdata/basic.hcl b/command/agent/testdata/basic.hcl
@@ -95,6 +95,7 @@ client {
   gc_disk_usage_threshold  = 82
   gc_inode_usage_threshold = 91
   gc_max_allocs            = 50
+  gc_volumes_on_node_gc    = true
   no_host_uuid             = false
   disable_remote_exec      = true
 
diff --git a/command/agent/testdata/basic.json b/command/agent/testdata/basic.json
@@ -94,6 +94,7 @@
       "gc_interval": "6s",
       "gc_max_allocs": 50,
       "gc_parallel_destroys": 6,
+      "gc_volumes_on_node_gc": true,
       "host_volume": [
         {
           "tmp": [
diff --git a/nomad/state/events_test.go b/nomad/state/events_test.go
@@ -821,7 +821,7 @@ func TestNodeEventsFromChanges(t *testing.T) {
 				return upsertNodeTxn(tx, tx.Index, testNode())
 			},
 			Mutate: func(s *StateStore, tx *txn) error {
-				return deleteNodeTxn(tx, tx.Index, []string{testNodeID()})
+				return s.deleteNodeTxn(tx, tx.Index, []string{testNodeID()})
 			},
 			WantEvents: []structs.Event{{
 				Topic: structs.TopicNode,
@@ -842,7 +842,7 @@ func TestNodeEventsFromChanges(t *testing.T) {
 				return upsertNodeTxn(tx, tx.Index, testNode(nodeIDTwo))
 			},
 			Mutate: func(s *StateStore, tx *txn) error {
-				return deleteNodeTxn(tx, tx.Index, []string{testNodeID(), testNodeIDTwo()})
+				return s.deleteNodeTxn(tx, tx.Index, []string{testNodeID(), testNodeIDTwo()})
 			},
 			WantEvents: []structs.Event{
 				{
diff --git a/nomad/state/state_store.go b/nomad/state/state_store.go
@@ -1052,14 +1052,14 @@ func (s *StateStore) DeleteNode(msgType structs.MessageType, index uint64, nodes
 	txn := s.db.WriteTxn(index)
 	defer txn.Abort()
 
-	err := deleteNodeTxn(txn, index, nodes)
+	err := s.deleteNodeTxn(txn, index, nodes)
 	if err != nil {
 		return nil
 	}
 	return txn.Commit()
 }
 
-func deleteNodeTxn(txn *txn, index uint64, nodes []string) error {
+func (s *StateStore) deleteNodeTxn(txn *txn, index uint64, nodes []string) error {
 	if len(nodes) == 0 {
 		return fmt.Errorf("node ids missing")
 	}
@@ -1082,6 +1082,11 @@ func deleteNodeTxn(txn *txn, index uint64, nodes []string) error {
 		if err := deleteNodeCSIPlugins(txn, node, index); err != nil {
 			return fmt.Errorf("csi plugin delete failed: %v", err)
 		}
+		if node.GCVolumesOnNodeGC {
+			if err := s.deleteHostVolumesOnNode(txn, index, node.ID); err != nil {
+				return fmt.Errorf("dynamic host volume delete failed: %v", err)
+			}
+		}
 	}
 
 	if err := txn.Insert("index", &IndexEntry{"nodes", index}); err != nil {
diff --git a/nomad/state/state_store_host_volumes.go b/nomad/state/state_store_host_volumes.go
@@ -122,6 +122,20 @@ func (s *StateStore) DeleteHostVolume(index uint64, ns string, id string) error
 	txn := s.db.WriteTxnMsgT(structs.HostVolumeDeleteRequestType, index)
 	defer txn.Abort()
 
+	if err := s.deleteHostVolumeTxn(txn, index, ns, id); err != nil {
+		return err
+	}
+
+	if err := txn.Insert(tableIndex, &IndexEntry{TableHostVolumes, index}); err != nil {
+		return fmt.Errorf("index update failed: %w", err)
+	}
+
+	return txn.Commit()
+}
+
+// deleteHostVolumeTxn implements a single dynamic host volume delete. The
+// caller is responsible for updating the index and committing the transaction.
+func (s *StateStore) deleteHostVolumeTxn(txn *txn, index uint64, ns string, id string) error {
 	obj, err := txn.First(TableHostVolumes, indexID, ns, id)
 	if err != nil {
 		return err
@@ -153,12 +167,35 @@ func (s *StateStore) DeleteHostVolume(index uint64, ns string, id string) error
 		}
 	}
 
-	if err := txn.Insert(tableIndex, &IndexEntry{TableHostVolumes, index}); err != nil {
-		return fmt.Errorf("index update failed: %w", err)
+	return nil
+}
+
+func (s *StateStore) deleteHostVolumesOnNode(txn *txn, index uint64, nodeID string) error {
+	iter, err := s.HostVolumesByNodeID(nil, nodeID, SortDefault)
+	if err != nil {
+		return err
 	}
 
-	return txn.Commit()
+	deleted := false // only update index if there was a volume to delete
+	for {
+		raw := iter.Next()
+		if raw == nil {
+			break
+		}
+		vol := raw.(*structs.HostVolume)
+		err := s.deleteHostVolumeTxn(txn, index, vol.Namespace, vol.ID)
+		if err != nil {
+			return err
+		}
+		deleted = true
+	}
+	if deleted {
+		if err := txn.Insert(tableIndex, &IndexEntry{TableHostVolumes, index}); err != nil {
+			return fmt.Errorf("index update failed: %w", err)
+		}
+	}
 
+	return nil
 }
 
 // HostVolumes queries all the host volumes and is mostly used for
diff --git a/nomad/state/state_store_host_volumes_test.go b/nomad/state/state_store_host_volumes_test.go
@@ -103,6 +103,7 @@ func TestStateStore_HostVolumes_CRUD(t *testing.T) {
 
 	// simulate a node registering one of the volumes
 	nodes[2] = nodes[2].Copy()
+	nodes[2].GCVolumesOnNodeGC = true
 	nodes[2].HostVolumes = map[string]*structs.ClientHostVolumeConfig{"example": {
 		Name: vols[2].Name,
 		Path: vols[2].HostPath,
@@ -180,6 +181,13 @@ func TestStateStore_HostVolumes_CRUD(t *testing.T) {
 	index++
 	must.NoError(t, store.UpdateAllocsFromClient(structs.MsgTypeTestSetup,
 		index, []*structs.Allocation{alloc}))
+
+	index++
+	must.NoError(t, store.DeleteNode(structs.MsgTypeTestSetup, index, []string{vol2.NodeID}))
+	iter, err = store.HostVolumesByNodeID(nil, vol2.NodeID, SortDefault)
+	must.NoError(t, err)
+	must.Nil(t, iter.Next(), must.Sprint("expected volume to be GC'd with node"))
+
 	for _, v := range vols {
 		index++
 		must.NoError(t, store.DeleteHostVolume(index, v.Namespace, v.ID))
diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
@@ -2157,6 +2157,11 @@ type Node struct {
 	// HostVolumes is a map of host volume names to their configuration
 	HostVolumes map[string]*ClientHostVolumeConfig
 
+	// GCVolumesOnNodeGC indicates that the server should GC any dynamic host
+	// volumes on this node when the node is GC'd. This should only be set if
+	// you know that a GC'd node can never come back
+	GCVolumesOnNodeGC bool
+
 	// HostNetworks is a map of host host_network names to their configuration
 	HostNetworks map[string]*ClientHostNetworkConfig
 
diff --git a/website/content/docs/configuration/client.mdx b/website/content/docs/configuration/client.mdx
@@ -166,6 +166,12 @@ client {
   parallel destroys allowed by the garbage collector. This value should be
   relatively low to avoid high resource usage during garbage collections.
 
+- `gc_volumes_on_node_gc` `(bool: false)` - Specifies that the server should
+  delete any dynamic host volumes on this node when the node is garbage
+  collected. You should only set this to `true` if you know that garbage
+  collected nodes will never rejoin the cluster, such as with ephemeral cloud
+  hosts.
+
 - `no_host_uuid` `(bool: true)` - By default a random node UUID will be
   generated, but setting this to `false` will use the system's UUID.
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+```release-note:improvement
	`2`	`+client: Add gc_volumes_on_node_gc configuration to delete host volumes when nodes are garbage collected`
	`3`	+```
Original file line number	Diff line number	Diff line change
`@@ -1595,6 +1595,8 @@ func (c *Client) setupNode() error {`
`1595`	`1595`	`node.HostVolumes[k] = v.Copy()`
`1596`	`1596`	`}`
`1597`	`1597`	`}`
	`1598`	`+ node.GCVolumesOnNodeGC = newConfig.GCVolumesOnNodeGC`
	`1599`	`+`
`1598`	`1600`	`if node.HostNetworks == nil {`
`1599`	`1601`	`if l := len(newConfig.HostNetworks); l != 0 {`
`1600`	`1602`	`node.HostNetworks = make(map[string]*structs.ClientHostNetworkConfig, l)`
Original file line number	Diff line number	Diff line change
`@@ -94,6 +94,7 @@`
`94`	`94`	`"gc_interval": "6s",`
`95`	`95`	`"gc_max_allocs": 50,`
`96`	`96`	`"gc_parallel_destroys": 6,`
	`97`	`+ "gc_volumes_on_node_gc": true,`
`97`	`98`	`"host_volume": [`
`98`	`99`	`{`
`99`	`100`	`"tmp": [`