Unit testing for node removal

Signed-off-by: Peter Sabaini <[email protected]>

microceph/client/wrap.go

@@ -14,6 +14,7 @@ type ClientInterface interface {
 	GetDisks(*microCli.Client) (types.Disks, error)
 	GetServices(*microCli.Client) (types.Services, error)
 	DeleteService(*microCli.Client, string, string) error
+	DeleteClusterMember(*microCli.Client, string, bool) error
 }
 
 type ClientImpl struct{}
@@ -49,5 +50,10 @@ func (c ClientImpl) DeleteService(cli *microCli.Client, target string, service s
 	return DeleteService(context.Background(), cli, target, service)
 }
 
+// DeleteClusterMember wraps the DeleteClusterMember function
+func (c ClientImpl) DeleteClusterMember(cli *microCli.Client, name string, force bool) error {
+	return cli.DeleteClusterMember(context.Background(), name, force)
+}
+
 // mocking point for unit tests
 var MClient ClientInterface = ClientImpl{}

microceph/cmd/microceph/cluster_remove.go

@@ -46,11 +46,16 @@ func (c *cmdClusterRemove) Run(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	logger.Debugf("Removing cluster member %v, force: %v", args[0], c.flagForce)
+	return removeNode(cli, args[0], c.flagForce)
+}
+
+func removeNode(cli *microCli.Client, node string, force bool) error {
+
+	logger.Debugf("Removing cluster member %v, force: %v", node, force)
 
 	// check prerquisites unless we're forcing
-	if !c.flagForce {
-		ok, err := checkPrerequisites(cli, args[0])
+	if !force {
+		ok, err := checkPrerequisites(cli, node)
 		if err != nil {
 			return fmt.Errorf("Error checking prereqs: %v", err)
 		}
@@ -60,18 +65,18 @@ func (c *cmdClusterRemove) Run(cmd *cobra.Command, args []string) error {
 	}
 
 	// delete from ceph
-	err = deleteNodeServices(cli, args[0])
+	err := deleteNodeServices(cli, node)
 	if err != nil {
 		// forcing makes errs non-fatal
-		if !c.flagForce {
+		if !force {
 			return err
 		}
-		logger.Warnf("Error deleting services from node %v: %v", args[0], err)
+		logger.Warnf("Error deleting services from node %v: %v", node, err)
 	}
 
 	// delete from cluster db
-	err = cli.DeleteClusterMember(context.Background(), args[0], c.flagForce)
-	logger.Debugf("DeleteClusterMember %v: %v", args[0], err)
+	err = client.MClient.DeleteClusterMember(cli, node, force)
+	logger.Debugf("DeleteClusterMember %v: %v", node, err)
 	if err != nil {
 		return err
 	}

microceph/cmd/microceph/cluster_remove_test.go

@@ -0,0 +1,128 @@
+package main
+
+import (
+	"github.com/canonical/microceph/microceph/api/types"
+	"github.com/canonical/microceph/microceph/client"
+	"github.com/canonical/microceph/microceph/mocks"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+)
+
+type clusterRemoveSuite struct {
+	suite.Suite
+}
+
+func TestClusterRemove(t *testing.T) {
+	suite.Run(t, new(clusterRemoveSuite))
+}
+
+func (s *clusterRemoveSuite) SetupTest() {
+
+}
+
+// TestRemoveNode tests the happy path of node removal
+func (s *clusterRemoveSuite) TestRemoveNode() {
+	m := mocks.NewClientInterface(s.T())
+
+	client.MClient = m
+	m.On("GetClusterMembers", mock.Anything).Return([]string{"foonode", "barnode", "quuxnode"}, nil).Once()
+	m.On("GetDisks", mock.Anything).Return(types.Disks{}, nil).Once()
+	m.On("GetServices", mock.Anything).Return(
+		types.Services{
+			{
+				Service:  "mon",
+				Location: "foonode",
+			},
+			{
+				Service:  "mon",
+				Location: "barnode",
+			},
+			{
+				Service:  "mgr",
+				Location: "barnode",
+			},
+			{
+				Service:  "mds",
+				Location: "barnode",
+			},
+		},
+		nil,
+	)
+	m.On("DeleteService", mock.Anything, "foonode", "mon").Return(nil).Once()
+	m.On("DeleteClusterMember", mock.Anything, "foonode", false).Return(nil).Once()
+
+	err := removeNode(nil, "foonode", false)
+
+	assert.NoError(s.T(), err)
+}
+
+// TestRemoveNodeWithDisks tests that we don't try to delete a node that has OSDs
+func (s *clusterRemoveSuite) TestRemoveNodeWithDisks() {
+	m := mocks.NewClientInterface(s.T())
+
+	client.MClient = m
+	m.On("GetClusterMembers", mock.Anything).Return([]string{"foonode", "barnode", "quuxnode"}, nil).Once()
+	m.On("GetDisks", mock.Anything).Return(types.Disks{
+		{
+			Location: "foonode",
+		},
+	}, nil).Once()
+
+	err := removeNode(nil, "foonode", false)
+
+	assert.Error(s.T(), err)
+
+	// assert that we didn't try to delete the node
+	m.AssertNotCalled(s.T(), "DeleteClusterMember", mock.Anything, "foonode", false)
+}
+
+// TestRemoveNodeLastMon tests that we don't try to delete a node that has the last mon
+func (s *clusterRemoveSuite) TestRemoveNodeLastMon() {
+	m := mocks.NewClientInterface(s.T())
+
+	client.MClient = m
+	m.On("GetClusterMembers", mock.Anything).Return([]string{"foonode", "barnode", "quuxnode"}, nil).Once()
+	m.On("GetDisks", mock.Anything).Return(types.Disks{}, nil).Once()
+	m.On("GetServices", mock.Anything).Return(
+		types.Services{
+			{
+				Service:  "mon",
+				Location: "foonode",
+			},
+		},
+		nil,
+	)
+
+	err := removeNode(nil, "foonode", false)
+
+	assert.Error(s.T(), err)
+
+	// assert that we didn't try to delete the node
+	m.AssertNotCalled(s.T(), "DeleteClusterMember", mock.Anything, "foonode", false)
+}
+
+// TestRemoveNodeForce tests that we don't check prerequisites and delete a node if forced
+func (s *clusterRemoveSuite) TestRemoveNodeForce() {
+	m := mocks.NewClientInterface(s.T())
+
+	client.MClient = m
+
+	m.On("GetServices", mock.Anything).Return(
+		types.Services{
+			{
+				Service:  "mon",
+				Location: "foonode",
+			},
+		},
+		nil,
+	)
+	m.On("DeleteService", mock.Anything, "foonode", "mon").Return(nil).Once()
+	m.On("DeleteClusterMember", mock.Anything, "foonode", true).Return(nil).Once()
+
+	err := removeNode(nil, "foonode", true)
+
+	assert.NoError(s.T(), err)
+}