diff --git a/nomad/structs/csi.go b/nomad/structs/csi.go index ade9181fb..71a61ef39 100644 --- a/nomad/structs/csi.go +++ b/nomad/structs/csi.go @@ -5,6 +5,7 @@ import ( "strings" "time" + multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/helper" ) @@ -853,32 +854,41 @@ func (p *CSIPlugin) DeleteNode(nodeID string) error { func (p *CSIPlugin) DeleteNodeForType(nodeID string, pluginType CSIPluginType) error { switch pluginType { case CSIPluginTypeController: - prev, ok := p.Controllers[nodeID] - if ok { + if prev, ok := p.Controllers[nodeID]; ok { if prev == nil { return fmt.Errorf("plugin missing controller: %s", nodeID) } if prev.Healthy { - p.ControllersHealthy -= 1 + p.ControllersHealthy-- } + delete(p.Controllers, nodeID) } - delete(p.Controllers, nodeID) case CSIPluginTypeNode: - prev, ok := p.Nodes[nodeID] - if ok { + if prev, ok := p.Nodes[nodeID]; ok { if prev == nil { return fmt.Errorf("plugin missing node: %s", nodeID) } if prev.Healthy { - p.NodesHealthy -= 1 + p.NodesHealthy-- } + delete(p.Nodes, nodeID) } - delete(p.Nodes, nodeID) case CSIPluginTypeMonolith: - p.DeleteNodeForType(nodeID, CSIPluginTypeController) - p.DeleteNodeForType(nodeID, CSIPluginTypeNode) + var result error + + err := p.DeleteNodeForType(nodeID, CSIPluginTypeController) + if err != nil { + result = multierror.Append(result, err) + } + + err = p.DeleteNodeForType(nodeID, CSIPluginTypeNode) + if err != nil { + result = multierror.Append(result, err) + } + + return result } return nil diff --git a/nomad/structs/csi_test.go b/nomad/structs/csi_test.go index b1c2ceda5..35baf3cb0 100644 --- a/nomad/structs/csi_test.go +++ b/nomad/structs/csi_test.go @@ -204,10 +204,192 @@ func TestCSIPluginCleanup(t *testing.T) { require.Equal(t, 1, plug.ControllersHealthy) require.Equal(t, 1, plug.NodesHealthy) - plug.DeleteNode("n0") + err := plug.DeleteNode("n0") + require.NoError(t, err) + require.Equal(t, 0, plug.ControllersHealthy) require.Equal(t, 0, plug.NodesHealthy) require.Equal(t, 0, len(plug.Controllers)) require.Equal(t, 0, len(plug.Nodes)) } + +func TestDeleteNodeForType_Controller(t *testing.T) { + info := &CSIInfo{ + PluginID: "foo", + AllocID: "a0", + Healthy: true, + Provider: "foo-provider", + RequiresControllerPlugin: true, + RequiresTopologies: false, + ControllerInfo: &CSIControllerInfo{}, + } + + plug := NewCSIPlugin("foo", 1000) + + plug.Controllers["n0"] = info + plug.ControllersHealthy = 1 + + err := plug.DeleteNodeForType("n0", CSIPluginTypeController) + require.NoError(t, err) + + require.Equal(t, 0, plug.ControllersHealthy) + require.Equal(t, 0, len(plug.Controllers)) +} + +func TestDeleteNodeForType_NilController(t *testing.T) { + plug := NewCSIPlugin("foo", 1000) + + plug.Controllers["n0"] = nil + plug.ControllersHealthy = 1 + + err := plug.DeleteNodeForType("n0", CSIPluginTypeController) + require.Error(t, err) + require.Equal(t, 1, len(plug.Controllers)) + + _, ok := plug.Controllers["foo"] + require.False(t, ok) +} + +func TestDeleteNodeForType_Node(t *testing.T) { + info := &CSIInfo{ + PluginID: "foo", + AllocID: "a0", + Healthy: true, + Provider: "foo-provider", + RequiresControllerPlugin: true, + RequiresTopologies: false, + NodeInfo: &CSINodeInfo{}, + } + + plug := NewCSIPlugin("foo", 1000) + + plug.Nodes["n0"] = info + plug.NodesHealthy = 1 + + err := plug.DeleteNodeForType("n0", CSIPluginTypeNode) + require.NoError(t, err) + + require.Equal(t, 0, plug.NodesHealthy) + require.Equal(t, 0, len(plug.Nodes)) +} + +func TestDeleteNodeForType_NilNode(t *testing.T) { + plug := NewCSIPlugin("foo", 1000) + + plug.Nodes["n0"] = nil + plug.NodesHealthy = 1 + + err := plug.DeleteNodeForType("n0", CSIPluginTypeNode) + require.Error(t, err) + require.Equal(t, 1, len(plug.Nodes)) + + _, ok := plug.Nodes["foo"] + require.False(t, ok) +} + +func TestDeleteNodeForType_Monolith(t *testing.T) { + controllerInfo := &CSIInfo{ + PluginID: "foo", + AllocID: "a0", + Healthy: true, + Provider: "foo-provider", + RequiresControllerPlugin: true, + RequiresTopologies: false, + ControllerInfo: &CSIControllerInfo{}, + } + + nodeInfo := &CSIInfo{ + PluginID: "foo", + AllocID: "a0", + Healthy: true, + Provider: "foo-provider", + RequiresControllerPlugin: true, + RequiresTopologies: false, + NodeInfo: &CSINodeInfo{}, + } + + plug := NewCSIPlugin("foo", 1000) + + plug.Controllers["n0"] = controllerInfo + plug.ControllersHealthy = 1 + + plug.Nodes["n0"] = nodeInfo + plug.NodesHealthy = 1 + + err := plug.DeleteNodeForType("n0", CSIPluginTypeMonolith) + require.NoError(t, err) + + require.Equal(t, 0, len(plug.Controllers)) + require.Equal(t, 0, len(plug.Nodes)) + + _, ok := plug.Nodes["foo"] + require.False(t, ok) + + _, ok = plug.Controllers["foo"] + require.False(t, ok) +} + +func TestDeleteNodeForType_Monolith_NilController(t *testing.T) { + plug := NewCSIPlugin("foo", 1000) + + plug.Controllers["n0"] = nil + plug.ControllersHealthy = 1 + + nodeInfo := &CSIInfo{ + PluginID: "foo", + AllocID: "a0", + Healthy: true, + Provider: "foo-provider", + RequiresControllerPlugin: true, + RequiresTopologies: false, + NodeInfo: &CSINodeInfo{}, + } + + plug.Nodes["n0"] = nodeInfo + plug.NodesHealthy = 1 + + err := plug.DeleteNodeForType("n0", CSIPluginTypeMonolith) + require.Error(t, err) + + require.Equal(t, 1, len(plug.Controllers)) + require.Equal(t, 0, len(plug.Nodes)) + + _, ok := plug.Nodes["foo"] + require.False(t, ok) + + _, ok = plug.Controllers["foo"] + require.False(t, ok) +} + +func TestDeleteNodeForType_Monolith_NilNode(t *testing.T) { + plug := NewCSIPlugin("foo", 1000) + + plug.Nodes["n0"] = nil + plug.NodesHealthy = 1 + + controllerInfo := &CSIInfo{ + PluginID: "foo", + AllocID: "a0", + Healthy: true, + Provider: "foo-provider", + RequiresControllerPlugin: true, + RequiresTopologies: false, + ControllerInfo: &CSIControllerInfo{}, + } + + plug.Controllers["n0"] = controllerInfo + plug.ControllersHealthy = 1 + + err := plug.DeleteNodeForType("n0", CSIPluginTypeMonolith) + require.Error(t, err) + + require.Equal(t, 0, len(plug.Controllers)) + require.Equal(t, 1, len(plug.Nodes)) + + _, ok := plug.Nodes["foo"] + require.False(t, ok) + + _, ok = plug.Controllers["foo"] + require.False(t, ok) +}