Do not lock already locked node KIKIMR-20904 (#1401)

Author: pixcc

ydb/core/cms/cms_ut.cpp

@@ -459,6 +459,26 @@ Y_UNIT_TEST_SUITE(TCmsTest) {
                                               "vdisk-3-1-0-1-0", "vdisk-3-1-0-5-0"));
     }
 
+    Y_UNIT_TEST(RequestReplaceManyDevicesOnOneNode)
+    {
+        TCmsTestEnv env(16, 3);
+        NKikimrCms::TCmsConfig config;
+        config.MutableClusterLimits()->SetDisabledNodesLimit(3);
+        config.MutableClusterLimits()->SetDisabledNodesRatioLimit(0);
+        env.SetCmsConfig(config);
+
+        auto rec1 = env.CheckPermissionRequest("user", false, false, false, true, TStatus::ALLOW,
+                                               MakeAction(TAction::REPLACE_DEVICES, env.GetNodeId(0),
+                                                          60000000, env.PDiskName(0, 0),
+                                                          env.PDiskName(0, 1), env.PDiskName(0, 2)));
+        UNIT_ASSERT_VALUES_EQUAL(rec1.PermissionsSize(), 1);
+
+        auto rec2 = env.CheckPermissionRequest("user", false, false, false, true, TStatus::ALLOW,
+                                               MakeAction(TAction::SHUTDOWN_HOST, env.GetNodeId(9),
+                                                          60000000));
+        UNIT_ASSERT_VALUES_EQUAL(rec2.PermissionsSize(), 1);
+    }
+
     Y_UNIT_TEST(RequestReplaceBrokenDevices)
     {
         TCmsTestEnv env(8);

ydb/core/cms/node_checkers.cpp

@@ -38,8 +38,7 @@ void TNodesCounterBase::UpdateNode(ui32 nodeId, NKikimrCms::EState state) {
         --DownNodesCount;
     }
 
-    if (NodeToState[nodeId] == NODE_STATE_LOCKED ||
-        NodeToState[nodeId] == NODE_STATE_RESTART) {
+    if (IsNodeLocked(nodeId)) {
         --LockedNodesCount;
     }
 
@@ -55,8 +54,15 @@ void TNodesCounterBase::UpdateNode(ui32 nodeId, NKikimrCms::EState state) {
     }
 }
 
-void TNodesCounterBase::LockNode(ui32 nodeId) {
+bool TNodesCounterBase::IsNodeLocked(ui32 nodeId) const {
     Y_ABORT_UNLESS(NodeToState.contains(nodeId));
+    return NodeToState.at(nodeId) == NODE_STATE_RESTART || NodeToState.at(nodeId) == NODE_STATE_LOCKED;
+}   
+
+void TNodesCounterBase::LockNode(ui32 nodeId) {
+    if (IsNodeLocked(nodeId)) {
+        return;
+    }
 
     ++LockedNodesCount;
     if (NodeToState[nodeId] == NODE_STATE_DOWN) {
@@ -68,7 +74,9 @@ void TNodesCounterBase::LockNode(ui32 nodeId) {
 }
 
 void TNodesCounterBase::UnlockNode(ui32 nodeId) {
-    Y_ABORT_UNLESS(NodeToState.contains(nodeId));
+    if (!IsNodeLocked(nodeId)) {
+        return;
+    }
 
     --LockedNodesCount;
     if (NodeToState[nodeId] == NODE_STATE_RESTART) {

ydb/core/cms/node_checkers.h

@@ -34,6 +34,7 @@ class INodesChecker {
     virtual void AddNode(ui32 nodeId) = 0;
     virtual void UpdateNode(ui32 nodeId, NKikimrCms::EState) = 0;
 
+    virtual bool IsNodeLocked(ui32 nodeId) const = 0;
     virtual void LockNode(ui32 nodeId) = 0;
     virtual void UnlockNode(ui32 nodeId) = 0;
 
@@ -59,6 +60,7 @@ class TNodesCounterBase : public INodesChecker {
     void AddNode(ui32 nodeId) override;
     void UpdateNode(ui32 nodeId, NKikimrCms::EState) override;
 
+    bool IsNodeLocked(ui32 nodeId) const override;
     void LockNode(ui32 nodeId) override;
     void UnlockNode(ui32 nodeId) override;