diff --git a/.changelog/25903.txt b/.changelog/25903.txt new file mode 100644 index 000000000..c776d5e57 --- /dev/null +++ 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 index 75df0957c..9f3574d34 100644 --- 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 index e3f02e03c..d993903c2 100644 --- 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 index b32956929..ebefd532a 100644 --- 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 index 23b83214a..5760b3747 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -947,6 +947,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 index 8b67fda80..dfe49aabe 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -342,6 +342,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"` @@ -2563,6 +2568,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 index d7d8b13fc..9212f0c4a 100644 --- 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/host_volume_endpoint.go b/command/agent/host_volume_endpoint.go index a3791dee6..9d740fad0 100644 --- a/command/agent/host_volume_endpoint.go +++ b/command/agent/host_volume_endpoint.go @@ -5,6 +5,7 @@ package agent import ( "net/http" + "strconv" "strings" "github.com/hashicorp/nomad/nomad/structs" @@ -132,6 +133,17 @@ func (s *HTTPServer) hostVolumeDelete(id string, resp http.ResponseWriter, req * args := structs.HostVolumeDeleteRequest{VolumeID: id} s.parseWriteRequest(req, &args.WriteRequest) + raw := req.URL.Query().Get("force") + var force bool + if raw != "" { + var err error + force, err = strconv.ParseBool(raw) + if err != nil { + return nil, CodedError(400, "invalid force value") + } + } + args.Force = force + var out structs.HostVolumeDeleteResponse if err := s.agent.RPC("HostVolume.Delete", &args, &out); err != nil { return nil, err diff --git a/command/agent/testdata/basic.hcl b/command/agent/testdata/basic.hcl index d0e20d0bd..db0b1b6dd 100644 --- 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 index 7394e7b05..15c09e925 100644 --- 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/host_volume_endpoint.go b/nomad/host_volume_endpoint.go index bcfebe54c..4cc49d5a0 100644 --- a/nomad/host_volume_endpoint.go +++ b/nomad/host_volume_endpoint.go @@ -671,7 +671,7 @@ func (v *HostVolume) Delete(args *structs.HostVolumeDeleteRequest, reply *struct // serialize client RPC and raft write per volume ID index, err := v.serializeCall(vol.ID, "delete", func() (uint64, error) { if err := v.deleteVolume(vol); err != nil { - if structs.IsErrUnknownNode(err) { + if structs.IsErrUnknownNode(err) || structs.IsErrNoNodeConn(err) { if !args.Force { return 0, fmt.Errorf( "volume cannot be removed from unknown node without force=true") diff --git a/nomad/state/events_test.go b/nomad/state/events_test.go index 6aca0d844..a9dc877bb 100644 --- a/nomad/state/events_test.go +++ b/nomad/state/events_test.go @@ -820,7 +820,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, @@ -841,7 +841,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 index a00656f65..a63e56795 100644 --- 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 index 33117540b..0cb167c32 100644 --- 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 index 0d0b4746c..8da48a6ad 100644 --- 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 index a872927b0..68398f62a 100644 --- 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 index 23e586cfc..73784f7a0 100644 --- a/website/content/docs/configuration/client.mdx +++ b/website/content/docs/configuration/client.mdx @@ -170,6 +170,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.