From a81e7a46d48ab73045c54c3bbb42ff63c48d5d95 Mon Sep 17 00:00:00 2001
From: sanjain <sanjain@purestorage.com>
Date: Wed, 25 Sep 2024 11:56:13 +0000
Subject: [PATCH] Revert "PWX-37595: honour force flag over node status
 (#2479)"

This reverts commit 24e88ce1ee8f298a62c41cead5aee13d7ae27cfb.

Signed-off-by: sanjain <sanjain@purestorage.com>
---
 cluster/manager/manager.go      |  3 +--
 cluster/manager/manager_test.go | 41 ---------------------------------
 2 files changed, 1 insertion(+), 43 deletions(-)

diff --git a/cluster/manager/manager.go b/cluster/manager/manager.go
index fb3c8a545..3e7352d58 100644
--- a/cluster/manager/manager.go
+++ b/cluster/manager/manager.go
@@ -1830,8 +1830,7 @@ func (c *ClusterManager) Remove(nodes []api.Node, forceRemove bool) error {
 			// If node is not down, do not remove it
 			if nodeCacheStatus != api.Status_STATUS_OFFLINE &&
 				nodeCacheStatus != api.Status_STATUS_MAINTENANCE &&
-				nodeCacheStatus != api.Status_STATUS_DECOMMISSION &&
-				!forceRemove {
+				nodeCacheStatus != api.Status_STATUS_DECOMMISSION {
 
 				msg := fmt.Sprintf(decommissionErrMsg, nodes[i].Id)
 				logrus.Errorf(msg+", node status: %s", nodeCacheStatus)
diff --git a/cluster/manager/manager_test.go b/cluster/manager/manager_test.go
index 67b77a525..14f898512 100644
--- a/cluster/manager/manager_test.go
+++ b/cluster/manager/manager_test.go
@@ -16,16 +16,11 @@ limitations under the License.
 package manager
 
 import (
-	"container/list"
-	"errors"
 	"fmt"
 	"testing"
 	"time"
 
-	"github.com/golang/mock/gomock"
-	"github.com/libopenstorage/openstorage/api"
 	"github.com/libopenstorage/openstorage/cluster"
-	"github.com/libopenstorage/openstorage/cluster/mock"
 	"github.com/libopenstorage/openstorage/config"
 	"github.com/libopenstorage/openstorage/pkg/auth"
 	"github.com/libopenstorage/openstorage/pkg/auth/systemtoken"
@@ -119,39 +114,3 @@ func TestUpdateSchedulerNodeName(t *testing.T) {
 
 	cleanup()
 }
-
-func TestRemoveOnlineNode(t *testing.T) {
-	const testNodeID = "test id"
-	mockErr := errors.New("mock err")
-	ctrl := gomock.NewController(t)
-	mockListener := mock.NewMockClusterListener(ctrl)
-	nodeToRemove := api.Node{
-		Id:     testNodeID,
-		Status: api.Status_STATUS_OK,
-	}
-	clusterListener := list.New()
-	clusterListener.PushBack(mockListener)
-	testManager := ClusterManager{
-		nodeCache: map[string]api.Node{
-			testNodeID: nodeToRemove,
-		},
-		listeners: clusterListener,
-	}
-
-	kv, err := kvdb.New(mem.Name, "test", []string{}, nil, kvdb.LogFatalErrorCB)
-	assert.NoError(t, err)
-	err = kvdb.SetInstance(kv)
-	assert.NoError(t, err)
-
-	// when force flag is false, node status check should take precedence
-	err = testManager.Remove([]api.Node{nodeToRemove}, false)
-	assert.Error(t, err)
-	assert.EqualError(t, err, fmt.Sprintf(decommissionErrMsg, testNodeID))
-
-	// when force flag is true, we shouldn't abort due to node status
-	mockListener.EXPECT().String().Return(testNodeID)
-	mockListener.EXPECT().MarkNodeDown(gomock.Any()).Return(mockErr)
-
-	err = testManager.Remove([]api.Node{nodeToRemove}, true)
-	assert.EqualError(t, err, mockErr.Error())
-}