Skip to content

Commit

Permalink
Revert "PWX-37595: honour force flag over node status (#2479)"
Browse files Browse the repository at this point in the history
This reverts commit 24e88ce.

Signed-off-by: sanjain <[email protected]>
  • Loading branch information
sanjain-px committed Sep 25, 2024
1 parent 2872d00 commit a81e7a4
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 43 deletions.
3 changes: 1 addition & 2 deletions cluster/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
41 changes: 0 additions & 41 deletions cluster/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
}

0 comments on commit a81e7a4

Please sign in to comment.