From 6a9d44c46337b55eb2ea708d90ff413140fbfdaa Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Tue, 24 Dec 2024 04:45:52 +0000 Subject: [PATCH 01/13] issue-2732: add change device state button --- .../disk_registry/disk_registry_actor.h | 5 + ...isk_registry_actor_change_device_state.cpp | 244 ++++++++++++++++++ .../disk_registry_actor_monitoring.cpp | 33 ++- .../libs/storage/disk_registry/ya.make | 1 + cloud/blockstore/tests/monitoring/test.py | 62 +++++ cloud/blockstore/tests/python/lib/client.py | 7 + 6 files changed, 349 insertions(+), 3 deletions(-) create mode 100644 cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h index 32a19889b3a..9c6aad4943e 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h @@ -346,6 +346,11 @@ class TDiskRegistryActor final const TCgiParameters& params, TRequestInfoPtr requestInfo); + void HandleHttpInfo_ChangeDeviseState( + const NActors::TActorContext& ctx, + const TCgiParameters& params, + TRequestInfoPtr requestInfo); + void HandleHttpInfo_RenderDisks( const NActors::TActorContext& ctx, const TCgiParameters& params, diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp new file mode 100644 index 00000000000..3a655669750 --- /dev/null +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -0,0 +1,244 @@ +#include "disk_registry_actor.h" +#include "util/string/join.h" + +#include +#include + +namespace NCloud::NBlockStore::NStorage { + +using namespace NActors; + +using namespace NKikimr; + +using namespace NMonitoringUtils; + +namespace { + +//////////////////////////////////////////////////////////////////////////////// + +class TChangeDeviceStateActor final + : public TActorBootstrapped +{ +private: + const TActorId Owner; + const ui64 TabletID; + const TRequestInfoPtr RequestInfo; + const TString DeviceUUID; + const NProto::EDeviceState NewState; + +public: + TChangeDeviceStateActor( + const TActorId& owner, + ui64 tabletID, + TRequestInfoPtr requestInfo, + TString deviceId, + NProto::EDeviceState newState); + + void Bootstrap(const TActorContext& ctx); + +private: + void Notify( + const TActorContext& ctx, + TString message, + const EAlertLevel alertLevel); + + void ReplyAndDie(const TActorContext& ctx, NProto::TError error); + +private: + STFUNC(StateWork); + + void HandleChangeDeviceStateResponse( + const TEvDiskRegistry::TEvChangeDeviceStateResponse::TPtr& ev, + const TActorContext& ctx); + + void HandlePoisonPill( + const TEvents::TEvPoisonPill::TPtr& ev, + const TActorContext& ctx); +}; + +//////////////////////////////////////////////////////////////////////////////// + +TChangeDeviceStateActor::TChangeDeviceStateActor( + const TActorId& owner, + ui64 tabletID, + TRequestInfoPtr requestInfo, + TString deviceId, + NProto::EDeviceState newState) + : Owner(owner) + , TabletID(tabletID) + , RequestInfo(std::move(requestInfo)) + , DeviceUUID(std::move(deviceId)) + , NewState(newState) +{} + +void TChangeDeviceStateActor::Bootstrap(const TActorContext& ctx) +{ + auto request = + std::make_unique(); + + request->Record.SetDeviceUUID(DeviceUUID); + request->Record.SetDeviceState(NewState); + + NCloud::Send(ctx, Owner, std::move(request)); + + Become(&TThis::StateWork); +} + +void TChangeDeviceStateActor::Notify( + const TActorContext& ctx, + TString message, + const EAlertLevel alertLevel) +{ + TStringStream out; + + BuildNotifyPageWithRedirect( + out, + std::move(message), + TStringBuilder() << "./app?action=dev&TabletId=" << TabletID + << "&DeviceUUID=" << DeviceUUID, + alertLevel); + + auto response = + std::make_unique(std::move(out.Str())); + NCloud::Reply(ctx, *RequestInfo, std::move(response)); +} + +void TChangeDeviceStateActor::ReplyAndDie( + const TActorContext& ctx, + NProto::TError error) +{ + if (SUCCEEDED(error.GetCode())) { + Notify(ctx, "Operation successfully completed", EAlertLevel::SUCCESS); + } else { + Notify( + ctx, + TStringBuilder() + << "failed to change device " << DeviceUUID.Quote() + << " state to " << static_cast(NewState) << ": " << FormatError(error), + EAlertLevel::DANGER); + } + + NCloud::Send( + ctx, + Owner, + std::make_unique()); + + Die(ctx); +} + +//////////////////////////////////////////////////////////////////////////////// + +void TChangeDeviceStateActor::HandlePoisonPill( + const TEvents::TEvPoisonPill::TPtr& ev, + const TActorContext& ctx) +{ + Y_UNUSED(ev); + ReplyAndDie(ctx, MakeError(E_REJECTED, "Tablet is dead")); +} + +void TChangeDeviceStateActor::HandleChangeDeviceStateResponse( + const TEvDiskRegistry::TEvChangeDeviceStateResponse::TPtr& ev, + const TActorContext& ctx) +{ + const auto* response = ev->Get(); + + ReplyAndDie(ctx, response->GetError()); +} + +//////////////////////////////////////////////////////////////////////////////// + +STFUNC(TChangeDeviceStateActor::StateWork) +{ + switch (ev->GetTypeRewrite()) { + HFunc(TEvents::TEvPoisonPill, HandlePoisonPill); + + HFunc( + TEvDiskRegistry::TEvChangeDeviceStateResponse, + HandleChangeDeviceStateResponse); + + default: + HandleUnexpectedEvent( + ev, + TBlockStoreComponents::DISK_REGISTRY_WORKER); + break; + } +} + +} // namespace + +//////////////////////////////////////////////////////////////////////////////// + +void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( + const TActorContext& ctx, + const TCgiParameters& params, + TRequestInfoPtr requestInfo) +{ + const auto& newStateRaw = params.Get("NewState"); + const auto& deviceUUID = params.Get("DeviceUUID"); + + + if (!newStateRaw) { + RejectHttpRequest(ctx, *requestInfo, "No new state is given"); + return; + } + + if (!deviceUUID) { + RejectHttpRequest(ctx, *requestInfo, "No device id is given"); + return; + } + + static const std::vector> + NewStateWhiteList = { + {NProto::EDeviceState::DEVICE_STATE_ONLINE, + ToString( + static_cast(NProto::EDeviceState::DEVICE_STATE_ONLINE))}, + {NProto::EDeviceState::DEVICE_STATE_WARNING, + ToString( + static_cast(NProto::EDeviceState::DEVICE_STATE_WARNING))}, + }; + auto it = FindIf( + NewStateWhiteList, + [&](const auto& state) { return state.second == newStateRaw; }); + auto newState = it->first; + Cerr << "Change state req." << Endl; + Cerr << newStateRaw << Endl; + Cerr << "Whitelist: " + << JoinSeq( + " ", + NewStateWhiteList | + std::views::transform([](const auto& val) + { return val.second; })) + .c_str() << Endl; + if (it == NewStateWhiteList.end()) { + RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); + return; + } + + const auto& device = State->GetDevice(deviceUUID); + if (device.GetState() == NProto::DEVICE_STATE_ERROR) { + RejectHttpRequest( + ctx, + *requestInfo, + "Can't change state of device in ERROR state"); + return; + } + + LOG_INFO( + ctx, + TBlockStoreComponents::DISK_REGISTRY, + "Change state of device[%s] from monitoring page to %s", + deviceUUID.Quote().c_str(), + newStateRaw.c_str()); + + auto actor = NCloud::Register( + ctx, + SelfId(), + TabletID(), + std::move(requestInfo), + deviceUUID, + newState); + + Actors.insert(actor); +} + +} // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp index 0ce007f271c..2a1d489d04d 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp @@ -462,6 +462,31 @@ void TDiskRegistryActor::RenderDeviceHtmlInfo( } DIV() { out << "State Message: " << device.GetStateMessage(); } + if (device.GetState() != NProto::EDeviceState::DEVICE_STATE_ERROR) { + DIV() + { + out << Sprintf( + R"(
+
+ + + + + + +
)", + device.GetDeviceUUID().c_str(), + static_cast(NProto::EDeviceState::DEVICE_STATE_ONLINE), + static_cast( + NProto::EDeviceState::DEVICE_STATE_WARNING), + device.GetDeviceUUID().c_str(), + TabletID()); + } + } + if (auto diskId = State->FindDisk(id)) { DIV() { out << "Disk: "; @@ -2253,9 +2278,11 @@ void TDiskRegistryActor::HandleHttpInfo( using THttpHandlers = THashMap; - static const THttpHandlers postActions {{ - {"volumeRealloc", &TDiskRegistryActor::HandleHttpInfo_VolumeRealloc }, - {"replaceDevice", &TDiskRegistryActor::HandleHttpInfo_ReplaceDevice }, + static const THttpHandlers postActions{{ + {"volumeRealloc", &TDiskRegistryActor::HandleHttpInfo_VolumeRealloc}, + {"replaceDevice", &TDiskRegistryActor::HandleHttpInfo_ReplaceDevice}, + {"changeDeviceState", + &TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState}, }}; static const THttpHandlers getActions {{ diff --git a/cloud/blockstore/libs/storage/disk_registry/ya.make b/cloud/blockstore/libs/storage/disk_registry/ya.make index fc8d924270c..77dbe268053 100644 --- a/cloud/blockstore/libs/storage/disk_registry/ya.make +++ b/cloud/blockstore/libs/storage/disk_registry/ya.make @@ -4,6 +4,7 @@ SRCS( disk_registry_actor_acquire.cpp disk_registry_actor_allocate.cpp disk_registry_actor_backup_state.cpp + disk_registry_actor_change_device_state.cpp disk_registry_actor_change_disk_device.cpp disk_registry_actor_checkpoint.cpp disk_registry_actor_remove_orphan_devices.cpp diff --git a/cloud/blockstore/tests/monitoring/test.py b/cloud/blockstore/tests/monitoring/test.py index 2b4ee7a1371..646a6b137b7 100644 --- a/cloud/blockstore/tests/monitoring/test.py +++ b/cloud/blockstore/tests/monitoring/test.py @@ -13,6 +13,7 @@ TServerAppConfig, TKikimrServiceConfig from cloud.blockstore.config.storage_pb2 import TStorageServiceConfig from cloud.blockstore.tests.python.lib.nbs_runner import LocalNbs +from cloud.blockstore.tests.python.lib.client import NbsClient from cloud.blockstore.tests.python.lib.nonreplicated_setup import \ setup_nonreplicated, create_file_devices, \ setup_disk_registry_config_simple, enable_writable_state @@ -409,6 +410,64 @@ def check_replacedevice(self): self.session, self.base_url, self.dr_id, params, "

Replace device is not allowed

") + def check_change_device(self): + params = {"action": "changeDeviceState"} + check_tablet_get_redirect( + self.session, self.base_url, + self.dr_id, params, "

Wrong HTTP method

") + check_tablet_post_redirect( + self.session, self.base_url, + self.dr_id, params, "

No new state is given

") + + params = { + "action": "changeDeviceState", + "NewState" : "0", + } + check_tablet_post_redirect( + self.session, self.base_url, + self.dr_id, params, "

No device id is given

") + + + params = { + "action": "changeDeviceState", + "NewState" : "1000", + "DeviceUUID": "FileDevice-1" + } + check_tablet_post_redirect( + self.session, self.base_url, + self.dr_id, params, "

Invalid new state

") + + self.client.change_device_state("FileDevice-1", "2") + + params = { + "action": "changeDeviceState", + "NewState" : "0", + "DeviceUUID": "FileDevice-1" + } + + check_tablet_post_redirect( + self.session, self.base_url, + self.dr_id, params, "

Can't change state of device in ERROR state

") + + self.client.change_device_state("FileDevice-1", "0") + + params = { + "action": "changeDeviceState", + "NewState" : "1", + "DeviceUUID": "FileDevice-1" + } + + check_tablet_post_redirect( + self.session, self.base_url, + self.dr_id, params, "

Operation successfully completed

") + + state = self.client.backup_disk_registry_state() + + for agent in state["Backup"]["Agents"]: + for device in agent["Devices"]: + if device["DeviceUUID"] == "FileDevice-1": + assert device["State"] == "DEVICE_STATE_WARNING" + def check_showdisk(self): params = {"action": "disk"} check_tablet_get_redirect( @@ -447,8 +506,11 @@ def run(self, nbs, nbs_http_port): self.dr_id = nbs.get_dr_tablet_id() logging.info("disk registry tablet %s" % self.dr_id) + self.client = NbsClient(nbs.nbs_port) + self.check_mainpage() self.check_replacedevice() + self.check_change_device() self.check_reallocate() self.check_showdisk() self.check_showdevice() diff --git a/cloud/blockstore/tests/python/lib/client.py b/cloud/blockstore/tests/python/lib/client.py index e3844bd4a10..3936bbc7db5 100644 --- a/cloud/blockstore/tests/python/lib/client.py +++ b/cloud/blockstore/tests/python/lib/client.py @@ -173,3 +173,10 @@ def get_storage_service_config(self, disk_id=None, timeout=300): resp = self.__execute_action('getstorageconfig', req, timeout) return json.loads(resp) + + def change_device_state(self, device_uuid, state, timeout=300): + req = {"ChangeDeviceState": {"DeviceUUID": device_uuid, "State": state}, "Message":"XXX"} + + resp = self.__execute_action("diskregistrychangestate", req, timeout) + + return json.loads(resp) \ No newline at end of file From 92a23bdb6b46604c0b833a233132bd7f360b1a49 Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Tue, 24 Dec 2024 07:49:37 +0000 Subject: [PATCH 02/13] issue-2732: add change agent state button and tests --- .../disk_registry/disk_registry_actor.h | 5 + ...disk_registry_actor_change_agent_state.cpp | 225 ++++++++++++++++++ ...isk_registry_actor_change_device_state.cpp | 10 - .../disk_registry_actor_monitoring.cpp | 78 ++++-- .../libs/storage/disk_registry/ya.make | 1 + cloud/blockstore/tests/monitoring/test.py | 50 +++- cloud/blockstore/tests/python/lib/client.py | 9 +- 7 files changed, 346 insertions(+), 32 deletions(-) create mode 100644 cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h index 9c6aad4943e..805a501d9fd 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h @@ -351,6 +351,11 @@ class TDiskRegistryActor final const TCgiParameters& params, TRequestInfoPtr requestInfo); + void HandleHttpInfo_ChangeAgentState( + const NActors::TActorContext& ctx, + const TCgiParameters& params, + TRequestInfoPtr requestInfo); + void HandleHttpInfo_RenderDisks( const NActors::TActorContext& ctx, const TCgiParameters& params, diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp new file mode 100644 index 00000000000..52a26e402dd --- /dev/null +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp @@ -0,0 +1,225 @@ +#include "disk_registry_actor.h" + +#include + +namespace NCloud::NBlockStore::NStorage { + +using namespace NActors; + +using namespace NKikimr; + +using namespace NMonitoringUtils; + +namespace { + +//////////////////////////////////////////////////////////////////////////////// + +class TChangeAgentStateActor final + : public TActorBootstrapped +{ +private: + const TActorId Owner; + const ui64 TabletID; + const TRequestInfoPtr RequestInfo; + const TString AgentID; + const NProto::EAgentState NewState; + +public: + TChangeAgentStateActor( + const TActorId& owner, + ui64 tabletID, + TRequestInfoPtr requestInfo, + TString agentId, + NProto::EAgentState newState); + + void Bootstrap(const TActorContext& ctx); + +private: + void Notify( + const TActorContext& ctx, + TString message, + const EAlertLevel alertLevel); + + void ReplyAndDie(const TActorContext& ctx, NProto::TError error); + +private: + STFUNC(StateWork); + + void HandleChangeAgentStateResponse( + const TEvDiskRegistry::TEvChangeAgentStateResponse::TPtr& ev, + const TActorContext& ctx); + + void HandlePoisonPill( + const TEvents::TEvPoisonPill::TPtr& ev, + const TActorContext& ctx); +}; + +//////////////////////////////////////////////////////////////////////////////// + +TChangeAgentStateActor::TChangeAgentStateActor( + const TActorId& owner, + ui64 tabletID, + TRequestInfoPtr requestInfo, + TString agentId, + NProto::EAgentState newState) + : Owner(owner) + , TabletID(tabletID) + , RequestInfo(std::move(requestInfo)) + , AgentID(std::move(agentId)) + , NewState(newState) +{} + +void TChangeAgentStateActor::Bootstrap(const TActorContext& ctx) +{ + auto request = + std::make_unique(); + + request->Record.SetAgentId(AgentID); + request->Record.SetAgentState(NewState); + + NCloud::Send(ctx, Owner, std::move(request)); + + Become(&TThis::StateWork); +} + +void TChangeAgentStateActor::Notify( + const TActorContext& ctx, + TString message, + const EAlertLevel alertLevel) +{ + TStringStream out; + + BuildNotifyPageWithRedirect( + out, + std::move(message), + TStringBuilder() << "./app?action=agent&TabletId=" << TabletID + << "&AgentID=" << AgentID, + alertLevel); + + auto response = + std::make_unique(std::move(out.Str())); + NCloud::Reply(ctx, *RequestInfo, std::move(response)); +} + +void TChangeAgentStateActor::ReplyAndDie( + const TActorContext& ctx, + NProto::TError error) +{ + if (SUCCEEDED(error.GetCode())) { + Notify(ctx, "Operation successfully completed", EAlertLevel::SUCCESS); + } else { + Notify( + ctx, + TStringBuilder() + << "failed to change agent[" << AgentID.Quote() << "] state to " + << static_cast(NewState) << ": " << FormatError(error), + EAlertLevel::DANGER); + } + + NCloud::Send( + ctx, + Owner, + std::make_unique()); + + Die(ctx); +} + +//////////////////////////////////////////////////////////////////////////////// + +void TChangeAgentStateActor::HandlePoisonPill( + const TEvents::TEvPoisonPill::TPtr& ev, + const TActorContext& ctx) +{ + Y_UNUSED(ev); + ReplyAndDie(ctx, MakeError(E_REJECTED, "Tablet is dead")); +} + +void TChangeAgentStateActor::HandleChangeAgentStateResponse( + const TEvDiskRegistry::TEvChangeAgentStateResponse::TPtr& ev, + const TActorContext& ctx) +{ + const auto* response = ev->Get(); + + ReplyAndDie(ctx, response->GetError()); +} + +//////////////////////////////////////////////////////////////////////////////// + +STFUNC(TChangeAgentStateActor::StateWork) +{ + switch (ev->GetTypeRewrite()) { + HFunc(TEvents::TEvPoisonPill, HandlePoisonPill); + + HFunc( + TEvDiskRegistry::TEvChangeAgentStateResponse, + HandleChangeAgentStateResponse); + + default: + HandleUnexpectedEvent( + ev, + TBlockStoreComponents::DISK_REGISTRY_WORKER); + break; + } +} + +} // namespace + +//////////////////////////////////////////////////////////////////////////////// + +void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( + const TActorContext& ctx, + const TCgiParameters& params, + TRequestInfoPtr requestInfo) +{ + const auto& newStateRaw = params.Get("NewState"); + const auto& agentId = params.Get("AgentID"); + + if (!newStateRaw) { + RejectHttpRequest(ctx, *requestInfo, "No new state is given"); + return; + } + + if (!agentId) { + RejectHttpRequest(ctx, *requestInfo, "No agent id is given"); + return; + } + + static const std::vector> + NewStateWhiteList = { + {NProto::EAgentState::AGENT_STATE_ONLINE, + ToString( + static_cast(NProto::EAgentState::AGENT_STATE_ONLINE))}, + {NProto::EAgentState::AGENT_STATE_WARNING, + ToString( + static_cast(NProto::EAgentState::AGENT_STATE_WARNING))}, + }; + + auto it = FindIf( + NewStateWhiteList, + [&](const auto& state) { return state.second == newStateRaw; }); + auto newState = it->first; + + if (it == NewStateWhiteList.end()) { + RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); + return; + } + + LOG_INFO( + ctx, + TBlockStoreComponents::DISK_REGISTRY, + "Change state of agent[%s] from monitoring page to %s", + agentId.Quote().c_str(), + newStateRaw.c_str()); + + auto actor = NCloud::Register( + ctx, + SelfId(), + TabletID(), + std::move(requestInfo), + agentId, + newState); + + Actors.insert(actor); +} + +} // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp index 3a655669750..50ef025dee8 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -1,7 +1,6 @@ #include "disk_registry_actor.h" #include "util/string/join.h" -#include #include namespace NCloud::NBlockStore::NStorage { @@ -200,15 +199,6 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( NewStateWhiteList, [&](const auto& state) { return state.second == newStateRaw; }); auto newState = it->first; - Cerr << "Change state req." << Endl; - Cerr << newStateRaw << Endl; - Cerr << "Whitelist: " - << JoinSeq( - " ", - NewStateWhiteList | - std::views::transform([](const auto& val) - { return val.second; })) - .c_str() << Endl; if (it == NewStateWhiteList.end()) { RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); return; diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp index 2a1d489d04d..151ddd0a0d4 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp @@ -103,6 +103,56 @@ void BuildDeviceReplaceButton( ); } +void BuildChangeDeviceStateButton( + IOutputStream& out, + ui64 tabletId, + const TString& deviceUUID) +{ + out << Sprintf( + R"(
+
+ + + + + + +
)", + deviceUUID.c_str(), + static_cast(NProto::EDeviceState::DEVICE_STATE_ONLINE), + static_cast(NProto::EDeviceState::DEVICE_STATE_WARNING), + deviceUUID.c_str(), + tabletId); +} + +void BuildChangeAgentStateButton( + IOutputStream& out, + ui64 tabletId, + const TString& agentId) +{ + out << Sprintf( + R"(
+
+ + + + + + +
)", + agentId.c_str(), + static_cast(NProto::EAgentState::AGENT_STATE_ONLINE), + static_cast(NProto::EAgentState::AGENT_STATE_WARNING), + agentId.c_str(), + tabletId); +} + void GenerateDiskRegistryActionsJS(IOutputStream& out) { out << R"html( @@ -465,25 +515,10 @@ void TDiskRegistryActor::RenderDeviceHtmlInfo( if (device.GetState() != NProto::EDeviceState::DEVICE_STATE_ERROR) { DIV() { - out << Sprintf( - R"(
-
- - - - - - -
)", - device.GetDeviceUUID().c_str(), - static_cast(NProto::EDeviceState::DEVICE_STATE_ONLINE), - static_cast( - NProto::EDeviceState::DEVICE_STATE_WARNING), - device.GetDeviceUUID().c_str(), - TabletID()); + BuildChangeDeviceStateButton( + out, + TabletID(), + device.GetDeviceUUID()); } } @@ -557,6 +592,9 @@ void TDiskRegistryActor::RenderAgentHtmlInfo( out << "State Timestamp: " << TInstant::MicroSeconds(agent->GetStateTs()); } + DIV() { + BuildChangeAgentStateButton(out, TabletID(), agent->GetAgentId()); + } DIV() { out << "State Message: " << agent->GetStateMessage(); } DIV() { out << "CMS Timestamp: " @@ -2283,6 +2321,8 @@ void TDiskRegistryActor::HandleHttpInfo( {"replaceDevice", &TDiskRegistryActor::HandleHttpInfo_ReplaceDevice}, {"changeDeviceState", &TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState}, + {"changeAgentState", + &TDiskRegistryActor::HandleHttpInfo_ChangeAgentState}, }}; static const THttpHandlers getActions {{ diff --git a/cloud/blockstore/libs/storage/disk_registry/ya.make b/cloud/blockstore/libs/storage/disk_registry/ya.make index 77dbe268053..3a31877c313 100644 --- a/cloud/blockstore/libs/storage/disk_registry/ya.make +++ b/cloud/blockstore/libs/storage/disk_registry/ya.make @@ -4,6 +4,7 @@ SRCS( disk_registry_actor_acquire.cpp disk_registry_actor_allocate.cpp disk_registry_actor_backup_state.cpp + disk_registry_actor_change_agent_state.cpp disk_registry_actor_change_device_state.cpp disk_registry_actor_change_disk_device.cpp disk_registry_actor_checkpoint.cpp diff --git a/cloud/blockstore/tests/monitoring/test.py b/cloud/blockstore/tests/monitoring/test.py index 646a6b137b7..69c1d7db1ae 100644 --- a/cloud/blockstore/tests/monitoring/test.py +++ b/cloud/blockstore/tests/monitoring/test.py @@ -410,7 +410,7 @@ def check_replacedevice(self): self.session, self.base_url, self.dr_id, params, "

Replace device is not allowed

") - def check_change_device(self): + def check_change_device_state(self): params = {"action": "changeDeviceState"} check_tablet_get_redirect( self.session, self.base_url, @@ -468,6 +468,51 @@ def check_change_device(self): if device["DeviceUUID"] == "FileDevice-1": assert device["State"] == "DEVICE_STATE_WARNING" + def check_change_agent_state(self): + params = {"action": "changeAgentState"} + check_tablet_get_redirect( + self.session, self.base_url, + self.dr_id, params, "

Wrong HTTP method

") + check_tablet_post_redirect( + self.session, self.base_url, + self.dr_id, params, "

No new state is given

") + + params = { + "action": "changeAgentState", + "NewState" : "0", + } + check_tablet_post_redirect( + self.session, self.base_url, + self.dr_id, params, "

No agent id is given

") + + + params = { + "action": "changeAgentState", + "NewState" : "1000", + "AgentID": "localhost" + } + check_tablet_post_redirect( + self.session, self.base_url, + self.dr_id, params, "

Invalid new state

") + + params = { + "action": "changeAgentState", + "NewState" : "1", + "AgentID": "localhost" + } + + check_tablet_post_redirect( + self.session, self.base_url, + self.dr_id, params, "

Operation successfully completed

") + + state = self.client.backup_disk_registry_state() + + for agent in state["Backup"]["Agents"]: + if agent["AgentId"] == "localhost": + assert agent["State"] == "AGENT_STATE_WARNING" + + self.client.change_agent_state("localhost", "0") + def check_showdisk(self): params = {"action": "disk"} check_tablet_get_redirect( @@ -510,7 +555,8 @@ def run(self, nbs, nbs_http_port): self.check_mainpage() self.check_replacedevice() - self.check_change_device() + self.check_change_device_state() + self.check_change_agent_state() self.check_reallocate() self.check_showdisk() self.check_showdevice() diff --git a/cloud/blockstore/tests/python/lib/client.py b/cloud/blockstore/tests/python/lib/client.py index 3936bbc7db5..6563a6f78b0 100644 --- a/cloud/blockstore/tests/python/lib/client.py +++ b/cloud/blockstore/tests/python/lib/client.py @@ -179,4 +179,11 @@ def change_device_state(self, device_uuid, state, timeout=300): resp = self.__execute_action("diskregistrychangestate", req, timeout) - return json.loads(resp) \ No newline at end of file + return json.loads(resp) + + def change_agent_state(self, agent_id, state, timeout=300): + req = {"ChangeAgentState": {"AgentId": agent_id, "State": state}, "Message":"XXX"} + + resp = self.__execute_action("diskregistrychangestate", req, timeout) + + return json.loads(resp) From fb72b68ce164a30d6b8cf02c9fb2a8c35b6e4204 Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Wed, 25 Dec 2024 04:51:38 +0000 Subject: [PATCH 03/13] issue-2732: correct issues --- ...disk_registry_actor_change_agent_state.cpp | 27 ++++++----------- ...isk_registry_actor_change_device_state.cpp | 29 +++++++------------ .../disk_registry_actor_monitoring.cpp | 16 +++++----- cloud/blockstore/tests/monitoring/test.py | 14 ++++----- 4 files changed, 35 insertions(+), 51 deletions(-) diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp index 52a26e402dd..64be9f4914b 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp @@ -112,7 +112,7 @@ void TChangeAgentStateActor::ReplyAndDie( ctx, TStringBuilder() << "failed to change agent[" << AgentID.Quote() << "] state to " - << static_cast(NewState) << ": " << FormatError(error), + << EAgentState_Name(NewState) << ": " << FormatError(error), EAlertLevel::DANGER); } @@ -184,22 +184,13 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( return; } - static const std::vector> - NewStateWhiteList = { - {NProto::EAgentState::AGENT_STATE_ONLINE, - ToString( - static_cast(NProto::EAgentState::AGENT_STATE_ONLINE))}, - {NProto::EAgentState::AGENT_STATE_WARNING, - ToString( - static_cast(NProto::EAgentState::AGENT_STATE_WARNING))}, - }; - - auto it = FindIf( - NewStateWhiteList, - [&](const auto& state) { return state.second == newStateRaw; }); - auto newState = it->first; - - if (it == NewStateWhiteList.end()) { + NProto::EAgentState newState; + if (!EAgentState_Parse(newStateRaw, &newState)) { + RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); + return; + } + + if (newState == NProto::EAgentState::AGENT_STATE_UNAVAILABLE) { RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); return; } @@ -209,7 +200,7 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( TBlockStoreComponents::DISK_REGISTRY, "Change state of agent[%s] from monitoring page to %s", agentId.Quote().c_str(), - newStateRaw.c_str()); + EAgentState_Name(newState).c_str()); auto actor = NCloud::Register( ctx, diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp index 50ef025dee8..1d97f14eeb3 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -111,9 +111,9 @@ void TChangeDeviceStateActor::ReplyAndDie( } else { Notify( ctx, - TStringBuilder() - << "failed to change device " << DeviceUUID.Quote() - << " state to " << static_cast(NewState) << ": " << FormatError(error), + TStringBuilder() << "failed to change device " << DeviceUUID.Quote() + << " state to " << EDeviceState_Name(NewState) + << ": " << FormatError(error), EAlertLevel::DANGER); } @@ -186,20 +186,13 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( return; } - static const std::vector> - NewStateWhiteList = { - {NProto::EDeviceState::DEVICE_STATE_ONLINE, - ToString( - static_cast(NProto::EDeviceState::DEVICE_STATE_ONLINE))}, - {NProto::EDeviceState::DEVICE_STATE_WARNING, - ToString( - static_cast(NProto::EDeviceState::DEVICE_STATE_WARNING))}, - }; - auto it = FindIf( - NewStateWhiteList, - [&](const auto& state) { return state.second == newStateRaw; }); - auto newState = it->first; - if (it == NewStateWhiteList.end()) { + NProto::EDeviceState newState; + if (!EDeviceState_Parse(newStateRaw, &newState)) { + RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); + return; + } + + if (newState == NProto::EDeviceState::DEVICE_STATE_ERROR) { RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); return; } @@ -218,7 +211,7 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( TBlockStoreComponents::DISK_REGISTRY, "Change state of device[%s] from monitoring page to %s", deviceUUID.Quote().c_str(), - newStateRaw.c_str()); + EDeviceState_Name(newState).c_str()); auto actor = NCloud::Register( ctx, diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp index 151ddd0a0d4..49bf9330f7c 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp @@ -113,8 +113,8 @@ void BuildChangeDeviceStateButton(
@@ -122,8 +122,8 @@ void BuildChangeDeviceStateButton( )", deviceUUID.c_str(), - static_cast(NProto::EDeviceState::DEVICE_STATE_ONLINE), - static_cast(NProto::EDeviceState::DEVICE_STATE_WARNING), + EDeviceState_Name(NProto::EDeviceState::DEVICE_STATE_ONLINE).c_str(), + EDeviceState_Name(NProto::EDeviceState::DEVICE_STATE_WARNING).c_str(), deviceUUID.c_str(), tabletId); } @@ -138,8 +138,8 @@ void BuildChangeAgentStateButton(
@@ -147,8 +147,8 @@ void BuildChangeAgentStateButton( )", agentId.c_str(), - static_cast(NProto::EAgentState::AGENT_STATE_ONLINE), - static_cast(NProto::EAgentState::AGENT_STATE_WARNING), + EAgentState_Name(NProto::EAgentState::AGENT_STATE_ONLINE).c_str(), + EAgentState_Name(NProto::EAgentState::AGENT_STATE_WARNING).c_str(), agentId.c_str(), tabletId); } diff --git a/cloud/blockstore/tests/monitoring/test.py b/cloud/blockstore/tests/monitoring/test.py index 69c1d7db1ae..be25f3f5c97 100644 --- a/cloud/blockstore/tests/monitoring/test.py +++ b/cloud/blockstore/tests/monitoring/test.py @@ -421,7 +421,7 @@ def check_change_device_state(self): params = { "action": "changeDeviceState", - "NewState" : "0", + "NewState" : "DEVICE_STATE_ONLINE", } check_tablet_post_redirect( self.session, self.base_url, @@ -430,7 +430,7 @@ def check_change_device_state(self): params = { "action": "changeDeviceState", - "NewState" : "1000", + "NewState" : "not a state", "DeviceUUID": "FileDevice-1" } check_tablet_post_redirect( @@ -441,7 +441,7 @@ def check_change_device_state(self): params = { "action": "changeDeviceState", - "NewState" : "0", + "NewState" : "DEVICE_STATE_ONLINE", "DeviceUUID": "FileDevice-1" } @@ -453,7 +453,7 @@ def check_change_device_state(self): params = { "action": "changeDeviceState", - "NewState" : "1", + "NewState" : "DEVICE_STATE_WARNING", "DeviceUUID": "FileDevice-1" } @@ -479,7 +479,7 @@ def check_change_agent_state(self): params = { "action": "changeAgentState", - "NewState" : "0", + "NewState" : "AGENT_STATE_ONLINE", } check_tablet_post_redirect( self.session, self.base_url, @@ -488,7 +488,7 @@ def check_change_agent_state(self): params = { "action": "changeAgentState", - "NewState" : "1000", + "NewState" : "not a state", "AgentID": "localhost" } check_tablet_post_redirect( @@ -497,7 +497,7 @@ def check_change_agent_state(self): params = { "action": "changeAgentState", - "NewState" : "1", + "NewState" : "AGENT_STATE_WARNING", "AgentID": "localhost" } From 1500e77020256e18cf70b7d4ad84c3b3e49fe236 Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Wed, 25 Dec 2024 06:17:57 +0000 Subject: [PATCH 04/13] issue-2732: correct issues --- .../disk_registry_actor_change_agent_state.cpp | 10 +++++++--- .../disk_registry_actor_change_device_state.cpp | 10 +++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp index 64be9f4914b..156e9bbc49e 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp @@ -96,8 +96,7 @@ void TChangeAgentStateActor::Notify( << "&AgentID=" << AgentID, alertLevel); - auto response = - std::make_unique(std::move(out.Str())); + auto response = std::make_unique(out.Str()); NCloud::Reply(ctx, *RequestInfo, std::move(response)); } @@ -190,7 +189,12 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( return; } - if (newState == NProto::EAgentState::AGENT_STATE_UNAVAILABLE) { + static const THashSet NewStateWhiteList = { + NProto::EAgentState::AGENT_STATE_ONLINE, + NProto::EAgentState::AGENT_STATE_WARNING, + }; + + if (!NewStateWhiteList.contains(newState)) { RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); return; } diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp index 1d97f14eeb3..8422b0f5d75 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -97,8 +97,7 @@ void TChangeDeviceStateActor::Notify( << "&DeviceUUID=" << DeviceUUID, alertLevel); - auto response = - std::make_unique(std::move(out.Str())); + auto response = std::make_unique(out.Str()); NCloud::Reply(ctx, *RequestInfo, std::move(response)); } @@ -192,7 +191,12 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( return; } - if (newState == NProto::EDeviceState::DEVICE_STATE_ERROR) { + static const THashSet NewStateWhiteList = { + NProto::EDeviceState::DEVICE_STATE_ONLINE, + NProto::EDeviceState::DEVICE_STATE_WARNING, + }; + + if (!NewStateWhiteList.contains(newState)) { RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); return; } From 8732aab9e2fb24f3fcf53d5aa97d9d1fb303fb61 Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Wed, 25 Dec 2024 07:15:25 +0000 Subject: [PATCH 05/13] issue-2732: fmt constructor --- .../disk_registry_actor_change_agent_state.cpp | 10 +++++----- .../disk_registry_actor_change_device_state.cpp | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp index 156e9bbc49e..f5ccf0e7519 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp @@ -57,11 +57,11 @@ class TChangeAgentStateActor final //////////////////////////////////////////////////////////////////////////////// TChangeAgentStateActor::TChangeAgentStateActor( - const TActorId& owner, - ui64 tabletID, - TRequestInfoPtr requestInfo, - TString agentId, - NProto::EAgentState newState) + const TActorId& owner, + ui64 tabletID, + TRequestInfoPtr requestInfo, + TString agentId, + NProto::EAgentState newState) : Owner(owner) , TabletID(tabletID) , RequestInfo(std::move(requestInfo)) diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp index 8422b0f5d75..865658d71fe 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -58,11 +58,11 @@ class TChangeDeviceStateActor final //////////////////////////////////////////////////////////////////////////////// TChangeDeviceStateActor::TChangeDeviceStateActor( - const TActorId& owner, - ui64 tabletID, - TRequestInfoPtr requestInfo, - TString deviceId, - NProto::EDeviceState newState) + const TActorId& owner, + ui64 tabletID, + TRequestInfoPtr requestInfo, + TString deviceId, + NProto::EDeviceState newState) : Owner(owner) , TabletID(tabletID) , RequestInfo(std::move(requestInfo)) From cdcd0f14339129a41592c5295b9d07d1ebee3c5c Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Thu, 9 Jan 2025 09:15:42 +0000 Subject: [PATCH 06/13] issue-2732: correct linter issues --- cloud/blockstore/tests/monitoring/test.py | 2 -- cloud/blockstore/tests/python/lib/client.py | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/cloud/blockstore/tests/monitoring/test.py b/cloud/blockstore/tests/monitoring/test.py index be25f3f5c97..f5827408faf 100644 --- a/cloud/blockstore/tests/monitoring/test.py +++ b/cloud/blockstore/tests/monitoring/test.py @@ -427,7 +427,6 @@ def check_change_device_state(self): self.session, self.base_url, self.dr_id, params, "

No device id is given

") - params = { "action": "changeDeviceState", "NewState" : "not a state", @@ -485,7 +484,6 @@ def check_change_agent_state(self): self.session, self.base_url, self.dr_id, params, "

No agent id is given

") - params = { "action": "changeAgentState", "NewState" : "not a state", diff --git a/cloud/blockstore/tests/python/lib/client.py b/cloud/blockstore/tests/python/lib/client.py index 6563a6f78b0..783a1a8b983 100644 --- a/cloud/blockstore/tests/python/lib/client.py +++ b/cloud/blockstore/tests/python/lib/client.py @@ -175,14 +175,14 @@ def get_storage_service_config(self, disk_id=None, timeout=300): return json.loads(resp) def change_device_state(self, device_uuid, state, timeout=300): - req = {"ChangeDeviceState": {"DeviceUUID": device_uuid, "State": state}, "Message":"XXX"} + req = {"ChangeDeviceState": {"DeviceUUID": device_uuid, "State": state}, "Message": "XXX"} resp = self.__execute_action("diskregistrychangestate", req, timeout) return json.loads(resp) def change_agent_state(self, agent_id, state, timeout=300): - req = {"ChangeAgentState": {"AgentId": agent_id, "State": state}, "Message":"XXX"} + req = {"ChangeAgentState": {"AgentId": agent_id, "State": state}, "Message": "XXX"} resp = self.__execute_action("diskregistrychangestate", req, timeout) From 124edf03816799409f62823193efa65d740e02d7 Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Fri, 10 Jan 2025 06:48:23 +0000 Subject: [PATCH 07/13] issue-2732: add config flag for buttons --- cloud/blockstore/config/storage.proto | 7 ++++ cloud/blockstore/libs/storage/core/config.cpp | 2 + cloud/blockstore/libs/storage/core/config.h | 2 + ...disk_registry_actor_change_agent_state.cpp | 38 ++++++++++++++++++- ...isk_registry_actor_change_device_state.cpp | 28 +++++++++++--- .../disk_registry_actor_monitoring.cpp | 29 ++++++++++---- cloud/blockstore/tests/monitoring/test.py | 17 ++------- 7 files changed, 94 insertions(+), 29 deletions(-) diff --git a/cloud/blockstore/config/storage.proto b/cloud/blockstore/config/storage.proto index c7fd81f92d1..93e2ccc5444 100644 --- a/cloud/blockstore/config/storage.proto +++ b/cloud/blockstore/config/storage.proto @@ -1083,4 +1083,11 @@ message TStorageServiceConfig // percentage, then the rejection of such agents does not occur - we assume // a connectivity failure in the cluster. optional double DiskRegistryInitialAgentRejectionThreshold = 396; + + // Enable buttons for agent/device state changing. + optional bool EnableToChangeStatesFromMonpage = 397; + + // Enable buttons for agent/device state changing, + // when they in unavailable/error state. + optional bool EnableToChangeErrorStatesFromMonpage = 398; } diff --git a/cloud/blockstore/libs/storage/core/config.cpp b/cloud/blockstore/libs/storage/core/config.cpp index 827ad3a8a01..f82d200745f 100644 --- a/cloud/blockstore/libs/storage/core/config.cpp +++ b/cloud/blockstore/libs/storage/core/config.cpp @@ -521,6 +521,8 @@ TDuration MSeconds(ui32 value) xxx(EncryptionAtRestForDiskRegistryBasedDisksEnabled, bool, false )\ xxx(DisableFullPlacementGroupCountCalculation, bool, false )\ xxx(DiskRegistryInitialAgentRejectionThreshold, double, 50 )\ + xxx(EnableToChangeStatesFromMonpage, bool, false )\ + xxx(EnableToChangeErrorStatesFromMonpage, bool, false )\ // BLOCKSTORE_STORAGE_CONFIG_RW #define BLOCKSTORE_STORAGE_CONFIG(xxx) \ diff --git a/cloud/blockstore/libs/storage/core/config.h b/cloud/blockstore/libs/storage/core/config.h index 96ed8b1968b..00bc62662c9 100644 --- a/cloud/blockstore/libs/storage/core/config.h +++ b/cloud/blockstore/libs/storage/core/config.h @@ -622,6 +622,8 @@ class TStorageConfig [[nodiscard]] bool GetDisableFullPlacementGroupCountCalculation() const; [[nodiscard]] double GetDiskRegistryInitialAgentRejectionThreshold() const; + [[nodiscard]] bool GetEnableToChangeStatesFromMonpage() const; + [[nodiscard]] bool GetEnableToChangeErrorStatesFromMonpage() const; }; ui64 GetAllocationUnit( diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp index f5ccf0e7519..1bcd856af30 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp @@ -130,7 +130,7 @@ void TChangeAgentStateActor::HandlePoisonPill( const TActorContext& ctx) { Y_UNUSED(ev); - ReplyAndDie(ctx, MakeError(E_REJECTED, "Tablet is dead")); + ReplyAndDie(ctx, MakeTabletIsDeadError(E_REJECTED, __LOCATION__)); } void TChangeAgentStateActor::HandleChangeAgentStateResponse( @@ -170,6 +170,11 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( const TCgiParameters& params, TRequestInfoPtr requestInfo) { + if (!Config->GetEnableToChangeStatesFromMonpage()) { + RejectHttpRequest(ctx, *requestInfo, "Can't change state from monpage"); + return; + } + const auto& newStateRaw = params.Get("NewState"); const auto& agentId = params.Get("AgentID"); @@ -189,7 +194,7 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( return; } - static const THashSet NewStateWhiteList = { + static const THashSet NewStateWhiteList{ NProto::EAgentState::AGENT_STATE_ONLINE, NProto::EAgentState::AGENT_STATE_WARNING, }; @@ -199,6 +204,35 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( return; } + const auto agentState = State->GetAgentState(agentId); + if (agentState.Empty()) { + RejectHttpRequest(ctx, *requestInfo, "Unknown agent"); + return; + } + + static const auto OldStateWhiteList = [&]() + { + THashSet whitelist = { + NProto::EAgentState::AGENT_STATE_ONLINE, + NProto::EAgentState::AGENT_STATE_WARNING, + }; + + if (Config->GetEnableToChangeErrorStatesFromMonpage()) { + whitelist.emplace(NProto::EAgentState::AGENT_STATE_UNAVAILABLE); + } + + return whitelist; + }(); + + + if (!OldStateWhiteList.contains(*agentState.Get())) { + RejectHttpRequest( + ctx, + *requestInfo, + "Can't change agent state from " + + EAgentState_Name(*agentState.Get())); + } + LOG_INFO( ctx, TBlockStoreComponents::DISK_REGISTRY, diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp index 865658d71fe..4a408f4033c 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -131,7 +131,7 @@ void TChangeDeviceStateActor::HandlePoisonPill( const TActorContext& ctx) { Y_UNUSED(ev); - ReplyAndDie(ctx, MakeError(E_REJECTED, "Tablet is dead")); + ReplyAndDie(ctx, MakeTabletIsDeadError(E_REJECTED, __LOCATION__)); } void TChangeDeviceStateActor::HandleChangeDeviceStateResponse( @@ -171,15 +171,17 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( const TCgiParameters& params, TRequestInfoPtr requestInfo) { + if (!Config->GetEnableToChangeStatesFromMonpage()) { + RejectHttpRequest(ctx, *requestInfo, "Can't change state from monpage"); + return; + } const auto& newStateRaw = params.Get("NewState"); const auto& deviceUUID = params.Get("DeviceUUID"); - if (!newStateRaw) { RejectHttpRequest(ctx, *requestInfo, "No new state is given"); return; } - if (!deviceUUID) { RejectHttpRequest(ctx, *requestInfo, "No device id is given"); return; @@ -195,18 +197,32 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( NProto::EDeviceState::DEVICE_STATE_ONLINE, NProto::EDeviceState::DEVICE_STATE_WARNING, }; - if (!NewStateWhiteList.contains(newState)) { RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); return; } + static const auto OldStateWhiteList = [&]() + { + THashSet whitelist = { + NProto::EDeviceState::DEVICE_STATE_ONLINE, + NProto::EDeviceState::DEVICE_STATE_WARNING, + }; + + if (Config->GetEnableToChangeErrorStatesFromMonpage()) { + whitelist.emplace(NProto::EDeviceState::DEVICE_STATE_ERROR); + } + + return whitelist; + }(); + const auto& device = State->GetDevice(deviceUUID); - if (device.GetState() == NProto::DEVICE_STATE_ERROR) { + if (!OldStateWhiteList.contains(device.GetState())) { RejectHttpRequest( ctx, *requestInfo, - "Can't change state of device in ERROR state"); + "Can't change device state from " + + EDeviceState_Name(device.GetState())); return; } diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp index 49bf9330f7c..3e596389388 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp @@ -512,13 +512,17 @@ void TDiskRegistryActor::RenderDeviceHtmlInfo( } DIV() { out << "State Message: " << device.GetStateMessage(); } - if (device.GetState() != NProto::EDeviceState::DEVICE_STATE_ERROR) { - DIV() + if (Config->GetEnableToChangeStatesFromMonpage()) { + if (device.GetState() != NProto::EDeviceState::DEVICE_STATE_ERROR || + Config->GetEnableToChangeErrorStatesFromMonpage()) { - BuildChangeDeviceStateButton( - out, - TabletID(), - device.GetDeviceUUID()); + DIV() + { + BuildChangeDeviceStateButton( + out, + TabletID(), + device.GetDeviceUUID()); + } } } @@ -593,7 +597,18 @@ void TDiskRegistryActor::RenderAgentHtmlInfo( << TInstant::MicroSeconds(agent->GetStateTs()); } DIV() { - BuildChangeAgentStateButton(out, TabletID(), agent->GetAgentId()); + if (Config->GetEnableToChangeStatesFromMonpage()) { + if (agent->GetState() != + NProto::EAgentState::AGENT_STATE_UNAVAILABLE || + Config->GetEnableToChangeErrorStatesFromMonpage()) + { + BuildChangeAgentStateButton( + out, + TabletID(), + agent->GetAgentId()); + } + } + } DIV() { out << "State Message: " << agent->GetStateMessage(); } DIV() { diff --git a/cloud/blockstore/tests/monitoring/test.py b/cloud/blockstore/tests/monitoring/test.py index f5827408faf..c9dba1b1dec 100644 --- a/cloud/blockstore/tests/monitoring/test.py +++ b/cloud/blockstore/tests/monitoring/test.py @@ -436,20 +436,6 @@ def check_change_device_state(self): self.session, self.base_url, self.dr_id, params, "

Invalid new state

") - self.client.change_device_state("FileDevice-1", "2") - - params = { - "action": "changeDeviceState", - "NewState" : "DEVICE_STATE_ONLINE", - "DeviceUUID": "FileDevice-1" - } - - check_tablet_post_redirect( - self.session, self.base_url, - self.dr_id, params, "

Can't change state of device in ERROR state

") - - self.client.change_device_state("FileDevice-1", "0") - params = { "action": "changeDeviceState", "NewState" : "DEVICE_STATE_WARNING", @@ -879,6 +865,9 @@ def __run_test(test_case): storage.NonReplicatedAgentMaxTimeout = 3000 storage.NonReplicatedDiskRecyclingPeriod = 5000 + storage.EnableToChangeStatesFromMonpage = True + storage.EnableToChangeErrorStatesFromMonpage = True + nbs = Nbs( kikimr_port, configurator.domains_txt, From d855ea0d3ed42d15b26c5b35458c06c0000d594f Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Fri, 10 Jan 2025 16:58:09 +0700 Subject: [PATCH 08/13] issue-2732: rename conf param and refactor --- cloud/blockstore/config/storage.proto | 7 ++- cloud/blockstore/libs/storage/core/config.cpp | 4 +- cloud/blockstore/libs/storage/core/config.h | 5 ++- ...disk_registry_actor_change_agent_state.cpp | 44 +++++++++---------- ...isk_registry_actor_change_device_state.cpp | 34 ++++++++------ .../disk_registry_actor_monitoring.cpp | 9 ++-- cloud/blockstore/tests/monitoring/test.py | 4 +- 7 files changed, 56 insertions(+), 51 deletions(-) diff --git a/cloud/blockstore/config/storage.proto b/cloud/blockstore/config/storage.proto index 93e2ccc5444..e032170f956 100644 --- a/cloud/blockstore/config/storage.proto +++ b/cloud/blockstore/config/storage.proto @@ -1085,9 +1085,8 @@ message TStorageServiceConfig optional double DiskRegistryInitialAgentRejectionThreshold = 396; // Enable buttons for agent/device state changing. - optional bool EnableToChangeStatesFromMonpage = 397; + optional bool EnableToChangeStatesFromDiskRegistryMonpage = 397; - // Enable buttons for agent/device state changing, - // when they in unavailable/error state. - optional bool EnableToChangeErrorStatesFromMonpage = 398; + // Enable buttons for device state changing, when they in error state. + optional bool EnableToChangeErrorStatesFromDiskRegistryMonpage = 398; } diff --git a/cloud/blockstore/libs/storage/core/config.cpp b/cloud/blockstore/libs/storage/core/config.cpp index f82d200745f..fddceb9b504 100644 --- a/cloud/blockstore/libs/storage/core/config.cpp +++ b/cloud/blockstore/libs/storage/core/config.cpp @@ -521,8 +521,8 @@ TDuration MSeconds(ui32 value) xxx(EncryptionAtRestForDiskRegistryBasedDisksEnabled, bool, false )\ xxx(DisableFullPlacementGroupCountCalculation, bool, false )\ xxx(DiskRegistryInitialAgentRejectionThreshold, double, 50 )\ - xxx(EnableToChangeStatesFromMonpage, bool, false )\ - xxx(EnableToChangeErrorStatesFromMonpage, bool, false )\ + xxx(EnableToChangeStatesFromDiskRegistryMonpage, bool, false )\ + xxx(EnableToChangeErrorStatesFromDiskRegistryMonpage, bool, false )\ // BLOCKSTORE_STORAGE_CONFIG_RW #define BLOCKSTORE_STORAGE_CONFIG(xxx) \ diff --git a/cloud/blockstore/libs/storage/core/config.h b/cloud/blockstore/libs/storage/core/config.h index 00bc62662c9..56e10d2a106 100644 --- a/cloud/blockstore/libs/storage/core/config.h +++ b/cloud/blockstore/libs/storage/core/config.h @@ -622,8 +622,9 @@ class TStorageConfig [[nodiscard]] bool GetDisableFullPlacementGroupCountCalculation() const; [[nodiscard]] double GetDiskRegistryInitialAgentRejectionThreshold() const; - [[nodiscard]] bool GetEnableToChangeStatesFromMonpage() const; - [[nodiscard]] bool GetEnableToChangeErrorStatesFromMonpage() const; + [[nodiscard]] bool GetEnableToChangeStatesFromDiskRegistryMonpage() const; + [[nodiscard]] bool + GetEnableToChangeErrorStatesFromDiskRegistryMonpage() const; }; ui64 GetAllocationUnit( diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp index 1bcd856af30..4314c271990 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp @@ -23,6 +23,7 @@ class TChangeAgentStateActor final const TRequestInfoPtr RequestInfo; const TString AgentID; const NProto::EAgentState NewState; + const NProto::EAgentState OldState; public: TChangeAgentStateActor( @@ -30,7 +31,8 @@ class TChangeAgentStateActor final ui64 tabletID, TRequestInfoPtr requestInfo, TString agentId, - NProto::EAgentState newState); + NProto::EAgentState newState, + NProto::EAgentState oldState); void Bootstrap(const TActorContext& ctx); @@ -61,12 +63,14 @@ TChangeAgentStateActor::TChangeAgentStateActor( ui64 tabletID, TRequestInfoPtr requestInfo, TString agentId, - NProto::EAgentState newState) + NProto::EAgentState newState, + NProto::EAgentState oldState) : Owner(owner) , TabletID(tabletID) , RequestInfo(std::move(requestInfo)) , AgentID(std::move(agentId)) , NewState(newState) + , OldState(oldState) {} void TChangeAgentStateActor::Bootstrap(const TActorContext& ctx) @@ -76,6 +80,7 @@ void TChangeAgentStateActor::Bootstrap(const TActorContext& ctx) request->Record.SetAgentId(AgentID); request->Record.SetAgentState(NewState); + request->Record.SetReason("Change state from disk registry mon page"); NCloud::Send(ctx, Owner, std::move(request)); @@ -110,7 +115,8 @@ void TChangeAgentStateActor::ReplyAndDie( Notify( ctx, TStringBuilder() - << "failed to change agent[" << AgentID.Quote() << "] state to " + << "failed to change agent[" << AgentID.Quote() + << "] state from " << EAgentState_Name(OldState) << " to " << EAgentState_Name(NewState) << ": " << FormatError(error), EAlertLevel::DANGER); } @@ -170,7 +176,7 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( const TCgiParameters& params, TRequestInfoPtr requestInfo) { - if (!Config->GetEnableToChangeStatesFromMonpage()) { + if (!Config->GetEnableToChangeStatesFromDiskRegistryMonpage()) { RejectHttpRequest(ctx, *requestInfo, "Can't change state from monpage"); return; } @@ -194,12 +200,12 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( return; } - static const THashSet NewStateWhiteList{ + static const THashSet NewStateWhitelist{ NProto::EAgentState::AGENT_STATE_ONLINE, NProto::EAgentState::AGENT_STATE_WARNING, }; - if (!NewStateWhiteList.contains(newState)) { + if (!NewStateWhitelist.contains(newState)) { RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); return; } @@ -210,22 +216,12 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( return; } - static const auto OldStateWhiteList = [&]() - { - THashSet whitelist = { - NProto::EAgentState::AGENT_STATE_ONLINE, - NProto::EAgentState::AGENT_STATE_WARNING, - }; - - if (Config->GetEnableToChangeErrorStatesFromMonpage()) { - whitelist.emplace(NProto::EAgentState::AGENT_STATE_UNAVAILABLE); - } - - return whitelist; - }(); - + static const THashSet OldStateAllowlist = { + NProto::EAgentState::AGENT_STATE_ONLINE, + NProto::EAgentState::AGENT_STATE_WARNING, + }; - if (!OldStateWhiteList.contains(*agentState.Get())) { + if (!OldStateAllowlist.contains(*agentState.Get())) { RejectHttpRequest( ctx, *requestInfo, @@ -236,8 +232,9 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( LOG_INFO( ctx, TBlockStoreComponents::DISK_REGISTRY, - "Change state of agent[%s] from monitoring page to %s", + "Changing state of agent[%s] from monitoring page from %s to %s", agentId.Quote().c_str(), + EAgentState_Name(*agentState.Get()).c_str(), EAgentState_Name(newState).c_str()); auto actor = NCloud::Register( @@ -246,7 +243,8 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( TabletID(), std::move(requestInfo), agentId, - newState); + newState, + *agentState.Get()); Actors.insert(actor); } diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp index 4a408f4033c..74adfc60fad 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -24,6 +24,7 @@ class TChangeDeviceStateActor final const TRequestInfoPtr RequestInfo; const TString DeviceUUID; const NProto::EDeviceState NewState; + const NProto::EDeviceState OldState; public: TChangeDeviceStateActor( @@ -31,7 +32,8 @@ class TChangeDeviceStateActor final ui64 tabletID, TRequestInfoPtr requestInfo, TString deviceId, - NProto::EDeviceState newState); + NProto::EDeviceState newState, + NProto::EDeviceState oldState); void Bootstrap(const TActorContext& ctx); @@ -62,12 +64,14 @@ TChangeDeviceStateActor::TChangeDeviceStateActor( ui64 tabletID, TRequestInfoPtr requestInfo, TString deviceId, - NProto::EDeviceState newState) + NProto::EDeviceState newState, + NProto::EDeviceState oldState) : Owner(owner) , TabletID(tabletID) , RequestInfo(std::move(requestInfo)) , DeviceUUID(std::move(deviceId)) , NewState(newState) + , OldState(oldState) {} void TChangeDeviceStateActor::Bootstrap(const TActorContext& ctx) @@ -77,6 +81,7 @@ void TChangeDeviceStateActor::Bootstrap(const TActorContext& ctx) request->Record.SetDeviceUUID(DeviceUUID); request->Record.SetDeviceState(NewState); + request->Record.SetReason("Change state from disk registry mon page"); NCloud::Send(ctx, Owner, std::move(request)); @@ -110,9 +115,10 @@ void TChangeDeviceStateActor::ReplyAndDie( } else { Notify( ctx, - TStringBuilder() << "failed to change device " << DeviceUUID.Quote() - << " state to " << EDeviceState_Name(NewState) - << ": " << FormatError(error), + TStringBuilder() + << "failed to change device " << DeviceUUID.Quote() + << " state from: " << EDeviceState_Name(OldState) << "to " + << EDeviceState_Name(NewState) << ": " << FormatError(error), EAlertLevel::DANGER); } @@ -171,7 +177,7 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( const TCgiParameters& params, TRequestInfoPtr requestInfo) { - if (!Config->GetEnableToChangeStatesFromMonpage()) { + if (!Config->GetEnableToChangeStatesFromDiskRegistryMonpage()) { RejectHttpRequest(ctx, *requestInfo, "Can't change state from monpage"); return; } @@ -193,23 +199,23 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( return; } - static const THashSet NewStateWhiteList = { + static const THashSet NewStateWhitelist = { NProto::EDeviceState::DEVICE_STATE_ONLINE, NProto::EDeviceState::DEVICE_STATE_WARNING, }; - if (!NewStateWhiteList.contains(newState)) { + if (!NewStateWhitelist.contains(newState)) { RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); return; } - static const auto OldStateWhiteList = [&]() + static const auto OldStateAllowlist = [&]() { THashSet whitelist = { NProto::EDeviceState::DEVICE_STATE_ONLINE, NProto::EDeviceState::DEVICE_STATE_WARNING, }; - if (Config->GetEnableToChangeErrorStatesFromMonpage()) { + if (Config->GetEnableToChangeErrorStatesFromDiskRegistryMonpage()) { whitelist.emplace(NProto::EDeviceState::DEVICE_STATE_ERROR); } @@ -217,7 +223,7 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( }(); const auto& device = State->GetDevice(deviceUUID); - if (!OldStateWhiteList.contains(device.GetState())) { + if (!OldStateAllowlist.contains(device.GetState())) { RejectHttpRequest( ctx, *requestInfo, @@ -229,8 +235,9 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( LOG_INFO( ctx, TBlockStoreComponents::DISK_REGISTRY, - "Change state of device[%s] from monitoring page to %s", + "Changing state of device[%s] from monitoring page from %s to %s", deviceUUID.Quote().c_str(), + EDeviceState_Name(device.GetState()).c_str(), EDeviceState_Name(newState).c_str()); auto actor = NCloud::Register( @@ -239,7 +246,8 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( TabletID(), std::move(requestInfo), deviceUUID, - newState); + newState, + device.GetState()); Actors.insert(actor); } diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp index 3e596389388..de21790e461 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp @@ -512,9 +512,9 @@ void TDiskRegistryActor::RenderDeviceHtmlInfo( } DIV() { out << "State Message: " << device.GetStateMessage(); } - if (Config->GetEnableToChangeStatesFromMonpage()) { + if (Config->GetEnableToChangeStatesFromDiskRegistryMonpage()) { if (device.GetState() != NProto::EDeviceState::DEVICE_STATE_ERROR || - Config->GetEnableToChangeErrorStatesFromMonpage()) + Config->GetEnableToChangeErrorStatesFromDiskRegistryMonpage()) { DIV() { @@ -597,10 +597,9 @@ void TDiskRegistryActor::RenderAgentHtmlInfo( << TInstant::MicroSeconds(agent->GetStateTs()); } DIV() { - if (Config->GetEnableToChangeStatesFromMonpage()) { + if (Config->GetEnableToChangeStatesFromDiskRegistryMonpage()) { if (agent->GetState() != - NProto::EAgentState::AGENT_STATE_UNAVAILABLE || - Config->GetEnableToChangeErrorStatesFromMonpage()) + NProto::EAgentState::AGENT_STATE_UNAVAILABLE) { BuildChangeAgentStateButton( out, diff --git a/cloud/blockstore/tests/monitoring/test.py b/cloud/blockstore/tests/monitoring/test.py index c9dba1b1dec..ed1d2fb4758 100644 --- a/cloud/blockstore/tests/monitoring/test.py +++ b/cloud/blockstore/tests/monitoring/test.py @@ -865,8 +865,8 @@ def __run_test(test_case): storage.NonReplicatedAgentMaxTimeout = 3000 storage.NonReplicatedDiskRecyclingPeriod = 5000 - storage.EnableToChangeStatesFromMonpage = True - storage.EnableToChangeErrorStatesFromMonpage = True + storage.EnableToChangeStatesFromDiskRegistryMonpage = True + storage.EnableToChangeErrorStatesFromDiskRegistryMonpage = True nbs = Nbs( kikimr_port, From 10379684e494f2e031389345c599cd88f83b36dd Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Mon, 13 Jan 2025 14:39:50 +0700 Subject: [PATCH 09/13] issue-2732: rename allowlist change log msgs and reson --- .../disk_registry_actor_change_agent_state.cpp | 8 ++++---- .../disk_registry_actor_change_device_state.cpp | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp index 4314c271990..2a779f9242a 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp @@ -80,7 +80,7 @@ void TChangeAgentStateActor::Bootstrap(const TActorContext& ctx) request->Record.SetAgentId(AgentID); request->Record.SetAgentState(NewState); - request->Record.SetReason("Change state from disk registry mon page"); + request->Record.SetReason("monpage action"); NCloud::Send(ctx, Owner, std::move(request)); @@ -200,12 +200,12 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( return; } - static const THashSet NewStateWhitelist{ + static const THashSet NewStateAllowlist{ NProto::EAgentState::AGENT_STATE_ONLINE, NProto::EAgentState::AGENT_STATE_WARNING, }; - if (!NewStateWhitelist.contains(newState)) { + if (!NewStateAllowlist.contains(newState)) { RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); return; } @@ -232,7 +232,7 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( LOG_INFO( ctx, TBlockStoreComponents::DISK_REGISTRY, - "Changing state of agent[%s] from monitoring page from %s to %s", + "Change state of agent[%s] on monitoring page from %s to %s", agentId.Quote().c_str(), EAgentState_Name(*agentState.Get()).c_str(), EAgentState_Name(newState).c_str()); diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp index 74adfc60fad..ffb183cebde 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -81,7 +81,7 @@ void TChangeDeviceStateActor::Bootstrap(const TActorContext& ctx) request->Record.SetDeviceUUID(DeviceUUID); request->Record.SetDeviceState(NewState); - request->Record.SetReason("Change state from disk registry mon page"); + request->Record.SetReason("monpage action"); NCloud::Send(ctx, Owner, std::move(request)); @@ -199,11 +199,11 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( return; } - static const THashSet NewStateWhitelist = { + static const THashSet NewStateAllowlist = { NProto::EDeviceState::DEVICE_STATE_ONLINE, NProto::EDeviceState::DEVICE_STATE_WARNING, }; - if (!NewStateWhitelist.contains(newState)) { + if (!NewStateAllowlist.contains(newState)) { RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); return; } @@ -235,7 +235,7 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( LOG_INFO( ctx, TBlockStoreComponents::DISK_REGISTRY, - "Changing state of device[%s] from monitoring page from %s to %s", + "Change state of device[%s] on monitoring page from %s to %s", deviceUUID.Quote().c_str(), EDeviceState_Name(device.GetState()).c_str(), EDeviceState_Name(newState).c_str()); From 6dcd139f5278b5a4fdb06fddb76e2165489b308a Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Mon, 13 Jan 2025 14:43:13 +0700 Subject: [PATCH 10/13] issue-2732: rename list --- .../disk_registry_actor_change_device_state.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp index ffb183cebde..8598c3b6baa 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -210,16 +210,16 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( static const auto OldStateAllowlist = [&]() { - THashSet whitelist = { + THashSet allowlist = { NProto::EDeviceState::DEVICE_STATE_ONLINE, NProto::EDeviceState::DEVICE_STATE_WARNING, }; if (Config->GetEnableToChangeErrorStatesFromDiskRegistryMonpage()) { - whitelist.emplace(NProto::EDeviceState::DEVICE_STATE_ERROR); + allowlist.emplace(NProto::EDeviceState::DEVICE_STATE_ERROR); } - return whitelist; + return allowlist; }(); const auto& device = State->GetDevice(deviceUUID); From 72b9a6edbc2494aa458f57e62a732954b015dbfa Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Mon, 13 Jan 2025 19:09:14 +0700 Subject: [PATCH 11/13] issue-2732: correct allowlist and enums --- ...disk_registry_actor_change_agent_state.cpp | 37 +++++++------- ...isk_registry_actor_change_device_state.cpp | 51 +++++++++---------- .../disk_registry_actor_monitoring.cpp | 8 +-- 3 files changed, 45 insertions(+), 51 deletions(-) diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp index 2a779f9242a..12d49736987 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp @@ -200,14 +200,13 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( return; } - static const THashSet NewStateAllowlist{ - NProto::EAgentState::AGENT_STATE_ONLINE, - NProto::EAgentState::AGENT_STATE_WARNING, - }; - - if (!NewStateAllowlist.contains(newState)) { - RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); - return; + switch (newState) { + case NProto::AGENT_STATE_ONLINE: + case NProto::AGENT_STATE_WARNING: + break; + default: + RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); + return; } const auto agentState = State->GetAgentState(agentId); @@ -216,17 +215,17 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeAgentState( return; } - static const THashSet OldStateAllowlist = { - NProto::EAgentState::AGENT_STATE_ONLINE, - NProto::EAgentState::AGENT_STATE_WARNING, - }; - - if (!OldStateAllowlist.contains(*agentState.Get())) { - RejectHttpRequest( - ctx, - *requestInfo, - "Can't change agent state from " + - EAgentState_Name(*agentState.Get())); + switch (newState) { + case NProto::AGENT_STATE_ONLINE: + case NProto::AGENT_STATE_WARNING: + break; + default: + RejectHttpRequest( + ctx, + *requestInfo, + "Can't change agent state from " + + EAgentState_Name(*agentState.Get())); + return; } LOG_INFO( diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp index 8598c3b6baa..f6652f5e1a6 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -199,37 +199,32 @@ void TDiskRegistryActor::HandleHttpInfo_ChangeDeviseState( return; } - static const THashSet NewStateAllowlist = { - NProto::EDeviceState::DEVICE_STATE_ONLINE, - NProto::EDeviceState::DEVICE_STATE_WARNING, - }; - if (!NewStateAllowlist.contains(newState)) { - RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); - return; + switch (newState) { + case NProto::DEVICE_STATE_ONLINE: + case NProto::DEVICE_STATE_WARNING: + break; + default: + RejectHttpRequest(ctx, *requestInfo, "Invalid new state"); + return; } - static const auto OldStateAllowlist = [&]() - { - THashSet allowlist = { - NProto::EDeviceState::DEVICE_STATE_ONLINE, - NProto::EDeviceState::DEVICE_STATE_WARNING, - }; - - if (Config->GetEnableToChangeErrorStatesFromDiskRegistryMonpage()) { - allowlist.emplace(NProto::EDeviceState::DEVICE_STATE_ERROR); - } - - return allowlist; - }(); - const auto& device = State->GetDevice(deviceUUID); - if (!OldStateAllowlist.contains(device.GetState())) { - RejectHttpRequest( - ctx, - *requestInfo, - "Can't change device state from " + - EDeviceState_Name(device.GetState())); - return; + switch (device.GetState()) { + case NProto::DEVICE_STATE_ONLINE: + case NProto::DEVICE_STATE_WARNING: + break; + case NProto::DEVICE_STATE_ERROR: + if (Config->GetEnableToChangeErrorStatesFromDiskRegistryMonpage()) { + break; + } + [[fallthrough]]; + default: + RejectHttpRequest( + ctx, + *requestInfo, + "Can't change device state from " + + EDeviceState_Name(device.GetState())); + return; } LOG_INFO( diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp index de21790e461..41cdddc4e34 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_monitoring.cpp @@ -122,8 +122,8 @@ void BuildChangeDeviceStateButton( )", deviceUUID.c_str(), - EDeviceState_Name(NProto::EDeviceState::DEVICE_STATE_ONLINE).c_str(), - EDeviceState_Name(NProto::EDeviceState::DEVICE_STATE_WARNING).c_str(), + EDeviceState_Name(NProto::DEVICE_STATE_ONLINE).c_str(), + EDeviceState_Name(NProto::DEVICE_STATE_WARNING).c_str(), deviceUUID.c_str(), tabletId); } @@ -147,8 +147,8 @@ void BuildChangeAgentStateButton( )", agentId.c_str(), - EAgentState_Name(NProto::EAgentState::AGENT_STATE_ONLINE).c_str(), - EAgentState_Name(NProto::EAgentState::AGENT_STATE_WARNING).c_str(), + EAgentState_Name(NProto::AGENT_STATE_ONLINE).c_str(), + EAgentState_Name(NProto::AGENT_STATE_WARNING).c_str(), agentId.c_str(), tabletId); } From cd361879dee98b1d60d295e2bcd48175b9c1e72e Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Wed, 15 Jan 2025 18:13:33 +0700 Subject: [PATCH 12/13] issue-2732: SUCCEDED -> !HasError --- .../disk_registry/disk_registry_actor_change_agent_state.cpp | 2 +- .../disk_registry/disk_registry_actor_change_device_state.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp index 12d49736987..4e276ce0f0d 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp @@ -109,7 +109,7 @@ void TChangeAgentStateActor::ReplyAndDie( const TActorContext& ctx, NProto::TError error) { - if (SUCCEEDED(error.GetCode())) { + if (!HasError(error.GetCode())) { Notify(ctx, "Operation successfully completed", EAlertLevel::SUCCESS); } else { Notify( diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp index f6652f5e1a6..0fdbd97f70d 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -110,7 +110,7 @@ void TChangeDeviceStateActor::ReplyAndDie( const TActorContext& ctx, NProto::TError error) { - if (SUCCEEDED(error.GetCode())) { + if (!HasError(error.GetCode())) { Notify(ctx, "Operation successfully completed", EAlertLevel::SUCCESS); } else { Notify( From 01f1c8f6a75e5202aa51831f9174dbf4e2fa7879 Mon Sep 17 00:00:00 2001 From: Stepanyuk Vladislav Date: Wed, 15 Jan 2025 18:22:03 +0700 Subject: [PATCH 13/13] issue-2732: correct build issue --- .../disk_registry/disk_registry_actor_change_agent_state.cpp | 2 +- .../disk_registry/disk_registry_actor_change_device_state.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp index 4e276ce0f0d..72c9472fcb4 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_agent_state.cpp @@ -109,7 +109,7 @@ void TChangeAgentStateActor::ReplyAndDie( const TActorContext& ctx, NProto::TError error) { - if (!HasError(error.GetCode())) { + if (!HasError(error)) { Notify(ctx, "Operation successfully completed", EAlertLevel::SUCCESS); } else { Notify( diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp index 0fdbd97f70d..192bd2c20b8 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_change_device_state.cpp @@ -110,7 +110,7 @@ void TChangeDeviceStateActor::ReplyAndDie( const TActorContext& ctx, NProto::TError error) { - if (!HasError(error.GetCode())) { + if (!HasError(error)) { Notify(ctx, "Operation successfully completed", EAlertLevel::SUCCESS); } else { Notify(