CMS API per-drive locks (#16064)

Add support for CMS API to address single PDisk for REPLACE_DEVICES action. Since PDisks can be restarted, there is no need to lock and shutdown entire host.

(cherry picked from commit 6e6034a)

Author: SammyVimes

ydb/core/cms/api_adapters.cpp

@@ -43,14 +43,44 @@ namespace {
         }
     }
 
+    static bool ConvertToPDiskId(const TString &name, TPDiskID &id) {
+        int size;
+
+        if (sscanf(name.data(), "pdisk-%" SCNu32 "-%" SCNu32 "%n", &id.NodeId, &id.DiskId, &size) != 2) {
+            return false;
+        }
+
+        if (size != static_cast<int>(name.size())) {
+            return false;
+        }
+
+        return true;
+    }
+
     void ConvertAction(const NKikimrCms::TAction& cmsAction, Ydb::Maintenance::LockAction& action) {
         *action.mutable_duration() = TimeUtil::MicrosecondsToDuration(cmsAction.GetDuration());
 
-        ui32 nodeId;
-        if (TryFromString(cmsAction.GetHost(), nodeId)) {
-            action.mutable_scope()->set_node_id(nodeId);
+        if (cmsAction.DevicesSize() > 0) {
+            Y_ABORT_UNLESS(cmsAction.DevicesSize() == 1);
+            auto& device = cmsAction.GetDevices()[0];
+            auto* pdisk = action.mutable_scope()->mutable_pdisk();
+            TPDiskID id;
+            if (ConvertToPDiskId(device, id)) {
+                auto* pdiskId = pdisk->mutable_pdisk_id();
+                pdiskId->set_node_id(id.NodeId);
+                pdiskId->set_pdisk_id(id.DiskId);
+            } else {
+                auto* pdiskLocation = pdisk->mutable_pdisk_location();
+                pdiskLocation->set_host(cmsAction.GetHost());
+                pdiskLocation->set_path(device);
+            }
         } else {
-            action.mutable_scope()->set_host(cmsAction.GetHost());
+            ui32 nodeId;
+            if (TryFromString(cmsAction.GetHost(), nodeId)) {
+                action.mutable_scope()->set_node_id(nodeId);
+            } else {
+                action.mutable_scope()->set_host(cmsAction.GetHost());
+            }
         }
     }
 
@@ -343,6 +373,7 @@ class TCreateMaintenanceTask: public TPermissionResponseProcessor<
         switch (scope.scope_case()) {
         case Ydb::Maintenance::ActionScope::kNodeId:
         case Ydb::Maintenance::ActionScope::kHost:
+        case Ydb::Maintenance::ActionScope::kPdisk:
             return true;
         default:
             Reply(Ydb::StatusIds::BAD_REQUEST, "Unknown scope");
@@ -399,18 +430,35 @@ class TCreateMaintenanceTask: public TPermissionResponseProcessor<
         return true;
     }
 
+    static void ConvertPDiskId(const Ydb::Maintenance::ActionScope_PDiskId& pdiskId, TString& out) {
+        out = Sprintf("pdisk-%" PRIu32 "-%" PRIu32, pdiskId.node_id(), pdiskId.pdisk_id());
+    }
+
     static void ConvertAction(const Ydb::Maintenance::LockAction& action, NKikimrCms::TAction& cmsAction) {
-        cmsAction.SetType(NKikimrCms::TAction::SHUTDOWN_HOST);
         cmsAction.SetDuration(TimeUtil::DurationToMicroseconds(action.duration()));
 
         const auto& scope = action.scope();
         switch (scope.scope_case()) {
         case Ydb::Maintenance::ActionScope::kNodeId:
+            cmsAction.SetType(NKikimrCms::TAction::SHUTDOWN_HOST);
             cmsAction.SetHost(ToString(scope.node_id()));
             break;
         case Ydb::Maintenance::ActionScope::kHost:
+            cmsAction.SetType(NKikimrCms::TAction::SHUTDOWN_HOST);
             cmsAction.SetHost(scope.host());
             break;
+        case Ydb::Maintenance::ActionScope::kPdisk: {
+            cmsAction.SetType(NKikimrCms::TAction::REPLACE_DEVICES);
+            auto& pdisk = scope.pdisk();
+            if (pdisk.has_pdisk_id()) {
+                ConvertPDiskId(pdisk.pdisk_id(), *cmsAction.add_devices());
+            } else if (pdisk.has_pdisk_location()) {
+                auto& pdiskLocation = pdisk.pdisk_location();
+                cmsAction.SetHost(pdiskLocation.host());
+                *cmsAction.add_devices() = pdiskLocation.path();
+            }
+            break;
+        }
         default:
             Y_ABORT("unreachable");
         }

ydb/core/cms/cluster_info.cpp

@@ -725,6 +725,8 @@ TSet<TLockableItem *> TClusterInfo::FindLockedItems(const NKikimrCms::TAction &a
 
             if (HasPDisk(device))
                 item = &PDiskRef(device);
+            else if (HasPDisk(action.GetHost(), device))
+                item = &PDiskRef(action.GetHost(), device);
             else if (HasVDisk(device))
                 item = &VDiskRef(device);
 

ydb/core/cms/cluster_info.h

@@ -978,6 +978,10 @@ class TClusterInfo : public TThrRefBase {
         return PDiskRef(id);
     }
 
+    TPDiskInfo &PDiskRef(const TString &hostName, const TString &path) {
+        return PDiskRef(HostNamePathToPDiskId(hostName, path));
+    }
+
     TVDiskInfo &VDiskRef(const TVDiskID &vdId) {
         Y_ABORT_UNLESS(HasVDisk(vdId));
         return *VDisks.find(vdId)->second;

ydb/core/cms/cms_maintenance_api_ut.cpp

@@ -41,7 +41,7 @@ Y_UNIT_TEST_SUITE(TMaintenanceApiTest) {
                 MakeLockAction(env.GetNodeId(0), TDuration::Minutes(10))
             )
         );
-        
+
         auto response = env.CheckMaintenanceTaskCreate("task-2", Ydb::StatusIds::SUCCESS,
             MakeActionGroup(
                 MakeLockAction(env.GetNodeId(1), TDuration::Minutes(10)),
@@ -116,6 +116,94 @@ Y_UNIT_TEST_SUITE(TMaintenanceApiTest) {
         const auto &a = response.action_group_states(0).action_states(0);
         UNIT_ASSERT_VALUES_EQUAL(a.status(), ActionState::ACTION_STATUS_PERFORMED);
     }
+
+    Y_UNIT_TEST(RequestReplaceDevicePDisk) {
+        std::function<void(NCms::TPDiskID&, Ydb::Maintenance::ActionScope_PDisk*)> serializeIdFn = [](NCms::TPDiskID& pdiskId, Ydb::Maintenance::ActionScope_PDisk* pdisk) {
+            auto* id = pdisk->mutable_pdisk_id();
+            id->set_node_id(pdiskId.NodeId);
+            id->set_pdisk_id(pdiskId.DiskId);
+        };
+
+        std::function<void(const Ydb::Maintenance::ActionScope_PDisk&, NCms::TPDiskID&)> deserializeIdFn = [](const Ydb::Maintenance::ActionScope_PDisk& pdisk, NCms::TPDiskID& pdiskId) {
+            auto& id = pdisk.Getpdisk_id();
+            pdiskId.NodeId = id.node_id();
+            pdiskId.DiskId = id.pdisk_id();
+        };
+
+        std::function<void(NCms::TPDiskID&, Ydb::Maintenance::ActionScope_PDisk*)> serializeLocationFn = [](NCms::TPDiskID& pdiskId, Ydb::Maintenance::ActionScope_PDisk* pdisk) {
+            auto* location = pdisk->mutable_pdisk_location();
+            location->set_host("::1"); // All nodes live on this host.
+            location->set_path("/" + std::to_string(pdiskId.NodeId) + "/pdisk-" + std::to_string(pdiskId.DiskId) + ".data");
+        };
+
+        std::function<void(const Ydb::Maintenance::ActionScope_PDisk&, NCms::TPDiskID&)> deserializeLocationFn = [](const Ydb::Maintenance::ActionScope_PDisk& pdisk, NCms::TPDiskID& pdiskId) {
+            auto& location = pdisk.Getpdisk_location();
+            sscanf(location.path().c_str(), "/%d/pdisk-%d.data", &pdiskId.NodeId, &pdiskId.DiskId);
+        };
+
+        std::vector<std::function<void(NCms::TPDiskID&, Ydb::Maintenance::ActionScope_PDisk*)>> serializeFns = {serializeIdFn, serializeLocationFn};
+
+        std::vector<std::function<void(const Ydb::Maintenance::ActionScope_PDisk&, NCms::TPDiskID&)>> deserializeFns = {deserializeIdFn, deserializeLocationFn};
+
+        for (size_t i = 0; i < serializeFns.size(); i++) {
+            auto serializeFn = serializeFns[i];
+            auto deserializeFn = deserializeFns[i];
+
+            TTestEnvOpts envOpts(8);
+            envOpts.WithSentinel();
+
+            TCmsTestEnv env(envOpts);
+
+            auto req = std::make_unique<NKikimr::NCms::TEvCms::TEvCreateMaintenanceTaskRequest>();
+
+            Ydb::Maintenance::CreateMaintenanceTaskRequest* rec = req->Record.MutableRequest();
+
+            req->Record.SetUserSID("user");
+
+            auto* options = rec->mutable_task_options();
+            options->set_availability_mode(::Ydb::Maintenance::AVAILABILITY_MODE_STRONG);
+
+            auto* actionGroup = rec->add_action_groups();
+            auto* action = actionGroup->Addactions();
+            auto* lockAction = action->mutable_lock_action();
+
+            auto* scope = lockAction->mutable_scope();
+
+            auto* pdisk = scope->mutable_pdisk();
+            NCms::TPDiskID disk = env.PDiskId(0);
+            serializeFn(disk, pdisk);
+            lockAction->mutable_duration()->set_seconds(60);
+
+            env.SendToCms(req.release());
+
+            auto response = env.GrabEdgeEvent<NCms::TEvCms::TEvMaintenanceTaskResponse>();
+
+            UNIT_ASSERT_VALUES_EQUAL(response->Record.GetStatus(), Ydb::StatusIds_StatusCode_SUCCESS);
+
+            {
+                auto& result = response->Record.GetResult();
+                UNIT_ASSERT_VALUES_EQUAL(result.action_group_states().size(), 1);
+                UNIT_ASSERT_VALUES_EQUAL(result.action_group_states(0).action_states().size(), 1);
+                const auto &actionState = result.action_group_states(0).action_states(0);
+
+                UNIT_ASSERT(actionState.has_action());
+                const auto& action = actionState.action();
+                UNIT_ASSERT(action.has_lock_action());
+                const auto& lockAction = action.lock_action();
+
+                UNIT_ASSERT(lockAction.has_scope());
+                const auto& scope = lockAction.scope();
+                UNIT_ASSERT(scope.has_pdisk());
+                const auto& pdisk = scope.pdisk();
+
+                NCms::TPDiskID resultDisk;
+                deserializeFn(pdisk, resultDisk);
+                UNIT_ASSERT_VALUES_EQUAL(resultDisk, disk);
+            }
+
+            env.CheckRequest("user", "user-r-1", false, NKikimrCms::TStatus::TStatus::WRONG_REQUEST);
+        }
+    }
 }
 
-} // namespace NKikimr::NCmsTest 
+} // namespace NKikimr::NCmsTest

ydb/core/cms/cms_ut.cpp

@@ -459,6 +459,94 @@ Y_UNIT_TEST_SUITE(TCmsTest) {
                                               "vdisk-3-1-0-1-0", "vdisk-3-1-0-5-0"));
     }
 
+    Y_UNIT_TEST(RequestReplaceDevicePDisk)
+    {
+        auto opts = TTestEnvOpts(8, 8).WithSentinel().WithDynamicGroups();
+        TCmsTestEnv env(opts);
+
+        env.CheckPermissionRequest("user", false, false, false, true, TStatus::NO_SUCH_DEVICE,
+                                   MakeAction(TAction::REPLACE_DEVICES, "::1", 60000000, "/dev/bad/device/path"));
+
+        env.CheckPermissionRequest(
+            MakePermissionRequest(TRequestOptions("user", false, false, false),
+                    MakeAction(TAction::REPLACE_DEVICES, 1, 60000000, env.PDiskName(0, 1))
+                ),
+            TStatus::ALLOW
+        );
+    }
+
+    Y_UNIT_TEST(RequestReplaceDevicePDiskByPath)
+    {
+        auto opts = TTestEnvOpts(8, 8).WithSentinel().WithDynamicGroups();
+        TCmsTestEnv env(opts);
+
+        auto pdiskId = env.PDiskId(0, 0);
+
+        TString pdiskPath = "/" + std::to_string(pdiskId.NodeId) + "/pdisk-" + std::to_string(pdiskId.DiskId) + ".data";
+
+        env.CheckPermissionRequest(
+            MakePermissionRequest(TRequestOptions("user", false, false, false),
+                    MakeAction(TAction::REPLACE_DEVICES, "::1", 60000000, pdiskPath)
+                ),
+            TStatus::ALLOW
+        );
+    }
+
+    Y_UNIT_TEST(RequestReplacePDiskDoesntBreakGroup)
+    {
+        auto opts = TTestEnvOpts(8, 2).WithSentinel().WithDynamicGroups();
+        TCmsTestEnv env(opts);
+
+        {
+            auto pdiskId = env.PDiskId(0, 0);
+    
+            TString pdiskPath = "/" + std::to_string(pdiskId.NodeId) + "/pdisk-" + std::to_string(pdiskId.DiskId) + ".data";
+    
+            env.CheckPermissionRequest(
+                MakePermissionRequest(TRequestOptions("user", false, false, false),
+                        MakeAction(TAction::REPLACE_DEVICES, "::1", 60000000, pdiskPath)
+                    ),
+                TStatus::ALLOW
+            );
+        }
+
+        {
+            auto pdiskId = env.PDiskId(1, 0);
+    
+            TString pdiskPath = "/" + std::to_string(pdiskId.NodeId) + "/pdisk-" + std::to_string(pdiskId.DiskId) + ".data";
+    
+            env.CheckPermissionRequest(
+                MakePermissionRequest(TRequestOptions("user", false, false, false),
+                        MakeAction(TAction::REPLACE_DEVICES, "::1", 60000000, pdiskPath)
+                    ),
+                TStatus::DISALLOW_TEMP
+            );
+        }
+    }
+
+    Y_UNIT_TEST(RequestReplacePDiskConsecutiveWithDone)
+    {
+        auto opts = TTestEnvOpts(8, 2).WithSentinel().WithDynamicGroups();
+        TCmsTestEnv env(opts);
+
+        for (ui32 i = 0; i < 8; ++i) {
+            auto pdiskId = env.PDiskId(i, 0);
+    
+            TString pdiskPath = "/" + std::to_string(pdiskId.NodeId) + "/pdisk-" + std::to_string(pdiskId.DiskId) + ".data";
+    
+            auto rec = env.CheckPermissionRequest(
+                MakePermissionRequest(TRequestOptions("user", false, false, false),
+                        MakeAction(TAction::REPLACE_DEVICES, "::1", 60000000, pdiskPath)
+                    ),
+                TStatus::ALLOW
+            );
+
+            auto pid = rec.GetPermissions(0).GetId();
+
+            env.CheckDonePermission("user", pid);
+        }
+    }
+
     Y_UNIT_TEST(RequestReplaceManyDevicesOnOneNode)
     {
         TCmsTestEnv env(16, 3);