Skip to content

Commit e19fbfa

Browse files
committed
host volumes: add configuration to GC on node GC
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
1 parent 456d95a commit e19fbfa

File tree

15 files changed

+92
-7
lines changed

15 files changed

+92
-7
lines changed

.changelog/25903.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff 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+
```

api/nodes.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,7 @@ type Node struct {
566566
Events []*NodeEvent
567567
Drivers map[string]*DriverInfo
568568
HostVolumes map[string]*HostVolumeInfo
569+
GCVolumesOnNodeGC bool
569570
HostNetworks map[string]*HostNetworkInfo
570571
CSIControllerPlugins map[string]*CSIInfo
571572
CSINodePlugins map[string]*CSIInfo

client/client.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1595,6 +1595,8 @@ func (c *Client) setupNode() error {
15951595
node.HostVolumes[k] = v.Copy()
15961596
}
15971597
}
1598+
node.GCVolumesOnNodeGC = newConfig.GCVolumesOnNodeGC
1599+
15981600
if node.HostNetworks == nil {
15991601
if l := len(newConfig.HostNetworks); l != 0 {
16001602
node.HostNetworks = make(map[string]*structs.ClientHostNetworkConfig, l)

client/config/config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,11 @@ type Config struct {
242242
// before garbage collection is triggered.
243243
GCMaxAllocs int
244244

245+
// GCVolumesOnNodeGC indicates that the server should GC any dynamic host
246+
// volumes on this node when the node is GC'd. This should only be set if
247+
// you know that a GC'd node can never come back
248+
GCVolumesOnNodeGC bool
249+
245250
// NoHostUUID disables using the host's UUID and will force generation of a
246251
// random UUID.
247252
NoHostUUID bool

command/agent/agent.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,8 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) {
946946
conf.GCDiskUsageThreshold = agentConfig.Client.GCDiskUsageThreshold
947947
conf.GCInodeUsageThreshold = agentConfig.Client.GCInodeUsageThreshold
948948
conf.GCMaxAllocs = agentConfig.Client.GCMaxAllocs
949+
conf.GCVolumesOnNodeGC = agentConfig.Client.GCVolumesOnNodeGC
950+
949951
if agentConfig.Client.NoHostUUID != nil {
950952
conf.NoHostUUID = *agentConfig.Client.NoHostUUID
951953
} else {

command/agent/config.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,11 @@ type ClientConfig struct {
341341
// before garbage collection is triggered.
342342
GCMaxAllocs int `hcl:"gc_max_allocs"`
343343

344+
// GCVolumesOnNodeGC indicates that the server should GC any dynamic host
345+
// volumes on this node when the node is GC'd. This should only be set if
346+
// you know that a GC'd node can never come back
347+
GCVolumesOnNodeGC bool `hcl:"gc_volumes_on_node_gc"`
348+
344349
// NoHostUUID disables using the host's UUID and will force generation of a
345350
// random UUID.
346351
NoHostUUID *bool `hcl:"no_host_uuid"`
@@ -2543,6 +2548,9 @@ func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig {
25432548
if b.GCMaxAllocs != 0 {
25442549
result.GCMaxAllocs = b.GCMaxAllocs
25452550
}
2551+
if b.GCVolumesOnNodeGC {
2552+
result.GCVolumesOnNodeGC = b.GCVolumesOnNodeGC
2553+
}
25462554
// NoHostUUID defaults to true, merge if false
25472555
if b.NoHostUUID != nil {
25482556
result.NoHostUUID = b.NoHostUUID

command/agent/config_parse_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ var basicConfig = &Config{
8888
GCDiskUsageThreshold: 82,
8989
GCInodeUsageThreshold: 91,
9090
GCMaxAllocs: 50,
91+
GCVolumesOnNodeGC: true,
9192
NoHostUUID: pointer.Of(false),
9293
DisableRemoteExec: true,
9394
HostVolumes: []*structs.ClientHostVolumeConfig{

command/agent/testdata/basic.hcl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ client {
9595
gc_disk_usage_threshold = 82
9696
gc_inode_usage_threshold = 91
9797
gc_max_allocs = 50
98+
gc_volumes_on_node_gc = true
9899
no_host_uuid = false
99100
disable_remote_exec = true
100101

command/agent/testdata/basic.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
"gc_interval": "6s",
9595
"gc_max_allocs": 50,
9696
"gc_parallel_destroys": 6,
97+
"gc_volumes_on_node_gc": true,
9798
"host_volume": [
9899
{
99100
"tmp": [

nomad/state/events_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -821,7 +821,7 @@ func TestNodeEventsFromChanges(t *testing.T) {
821821
return upsertNodeTxn(tx, tx.Index, testNode())
822822
},
823823
Mutate: func(s *StateStore, tx *txn) error {
824-
return deleteNodeTxn(tx, tx.Index, []string{testNodeID()})
824+
return s.deleteNodeTxn(tx, tx.Index, []string{testNodeID()})
825825
},
826826
WantEvents: []structs.Event{{
827827
Topic: structs.TopicNode,
@@ -842,7 +842,7 @@ func TestNodeEventsFromChanges(t *testing.T) {
842842
return upsertNodeTxn(tx, tx.Index, testNode(nodeIDTwo))
843843
},
844844
Mutate: func(s *StateStore, tx *txn) error {
845-
return deleteNodeTxn(tx, tx.Index, []string{testNodeID(), testNodeIDTwo()})
845+
return s.deleteNodeTxn(tx, tx.Index, []string{testNodeID(), testNodeIDTwo()})
846846
},
847847
WantEvents: []structs.Event{
848848
{

nomad/state/state_store.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,14 +1052,14 @@ func (s *StateStore) DeleteNode(msgType structs.MessageType, index uint64, nodes
10521052
txn := s.db.WriteTxn(index)
10531053
defer txn.Abort()
10541054

1055-
err := deleteNodeTxn(txn, index, nodes)
1055+
err := s.deleteNodeTxn(txn, index, nodes)
10561056
if err != nil {
10571057
return nil
10581058
}
10591059
return txn.Commit()
10601060
}
10611061

1062-
func deleteNodeTxn(txn *txn, index uint64, nodes []string) error {
1062+
func (s *StateStore) deleteNodeTxn(txn *txn, index uint64, nodes []string) error {
10631063
if len(nodes) == 0 {
10641064
return fmt.Errorf("node ids missing")
10651065
}
@@ -1082,6 +1082,11 @@ func deleteNodeTxn(txn *txn, index uint64, nodes []string) error {
10821082
if err := deleteNodeCSIPlugins(txn, node, index); err != nil {
10831083
return fmt.Errorf("csi plugin delete failed: %v", err)
10841084
}
1085+
if node.GCVolumesOnNodeGC {
1086+
if err := s.deleteHostVolumesOnNode(txn, index, node.ID); err != nil {
1087+
return fmt.Errorf("dynamic host volume delete failed: %v", err)
1088+
}
1089+
}
10851090
}
10861091

10871092
if err := txn.Insert("index", &IndexEntry{"nodes", index}); err != nil {

nomad/state/state_store_host_volumes.go

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,20 @@ func (s *StateStore) DeleteHostVolume(index uint64, ns string, id string) error
122122
txn := s.db.WriteTxnMsgT(structs.HostVolumeDeleteRequestType, index)
123123
defer txn.Abort()
124124

125+
if err := s.deleteHostVolumeTxn(txn, index, ns, id); err != nil {
126+
return err
127+
}
128+
129+
if err := txn.Insert(tableIndex, &IndexEntry{TableHostVolumes, index}); err != nil {
130+
return fmt.Errorf("index update failed: %w", err)
131+
}
132+
133+
return txn.Commit()
134+
}
135+
136+
// deleteHostVolumeTxn implements a single dynamic host volume delete. The
137+
// caller is responsible for updating the index and committing the transaction.
138+
func (s *StateStore) deleteHostVolumeTxn(txn *txn, index uint64, ns string, id string) error {
125139
obj, err := txn.First(TableHostVolumes, indexID, ns, id)
126140
if err != nil {
127141
return err
@@ -153,12 +167,35 @@ func (s *StateStore) DeleteHostVolume(index uint64, ns string, id string) error
153167
}
154168
}
155169

156-
if err := txn.Insert(tableIndex, &IndexEntry{TableHostVolumes, index}); err != nil {
157-
return fmt.Errorf("index update failed: %w", err)
170+
return nil
171+
}
172+
173+
func (s *StateStore) deleteHostVolumesOnNode(txn *txn, index uint64, nodeID string) error {
174+
iter, err := s.HostVolumesByNodeID(nil, nodeID, SortDefault)
175+
if err != nil {
176+
return err
158177
}
159178

160-
return txn.Commit()
179+
deleted := false // only update index if there was a volume to delete
180+
for {
181+
raw := iter.Next()
182+
if raw == nil {
183+
break
184+
}
185+
vol := raw.(*structs.HostVolume)
186+
err := s.deleteHostVolumeTxn(txn, index, vol.Namespace, vol.ID)
187+
if err != nil {
188+
return err
189+
}
190+
deleted = true
191+
}
192+
if deleted {
193+
if err := txn.Insert(tableIndex, &IndexEntry{TableHostVolumes, index}); err != nil {
194+
return fmt.Errorf("index update failed: %w", err)
195+
}
196+
}
161197

198+
return nil
162199
}
163200

164201
// HostVolumes queries all the host volumes and is mostly used for

nomad/state/state_store_host_volumes_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ func TestStateStore_HostVolumes_CRUD(t *testing.T) {
103103

104104
// simulate a node registering one of the volumes
105105
nodes[2] = nodes[2].Copy()
106+
nodes[2].GCVolumesOnNodeGC = true
106107
nodes[2].HostVolumes = map[string]*structs.ClientHostVolumeConfig{"example": {
107108
Name: vols[2].Name,
108109
Path: vols[2].HostPath,
@@ -180,6 +181,13 @@ func TestStateStore_HostVolumes_CRUD(t *testing.T) {
180181
index++
181182
must.NoError(t, store.UpdateAllocsFromClient(structs.MsgTypeTestSetup,
182183
index, []*structs.Allocation{alloc}))
184+
185+
index++
186+
must.NoError(t, store.DeleteNode(structs.MsgTypeTestSetup, index, []string{vol2.NodeID}))
187+
iter, err = store.HostVolumesByNodeID(nil, vol2.NodeID, SortDefault)
188+
must.NoError(t, err)
189+
must.Nil(t, iter.Next(), must.Sprint("expected volume to be GC'd with node"))
190+
183191
for _, v := range vols {
184192
index++
185193
must.NoError(t, store.DeleteHostVolume(index, v.Namespace, v.ID))

nomad/structs/structs.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2157,6 +2157,11 @@ type Node struct {
21572157
// HostVolumes is a map of host volume names to their configuration
21582158
HostVolumes map[string]*ClientHostVolumeConfig
21592159

2160+
// GCVolumesOnNodeGC indicates that the server should GC any dynamic host
2161+
// volumes on this node when the node is GC'd. This should only be set if
2162+
// you know that a GC'd node can never come back
2163+
GCVolumesOnNodeGC bool
2164+
21602165
// HostNetworks is a map of host host_network names to their configuration
21612166
HostNetworks map[string]*ClientHostNetworkConfig
21622167

website/content/docs/configuration/client.mdx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,12 @@ client {
166166
parallel destroys allowed by the garbage collector. This value should be
167167
relatively low to avoid high resource usage during garbage collections.
168168

169+
- `gc_volumes_on_node_gc` `(bool: false)` - Specifies that the server should
170+
delete any dynamic host volumes on this node when the node is garbage
171+
collected. You should only set this to `true` if you know that garbage
172+
collected nodes will never rejoin the cluster, such as with ephemeral cloud
173+
hosts.
174+
169175
- `no_host_uuid` `(bool: true)` - By default a random node UUID will be
170176
generated, but setting this to `false` will use the system's UUID.
171177

0 commit comments

Comments
 (0)