issue-370: add config dispatcher support for nbs (#2069)

* issue-370: add config dispatcher support for nbs * add intg test * more tests * update * update * update * update
ydb-platform · Sep 23, 2024 · 67dbfd7 · 67dbfd7
1 parent e476bcb
commit 67dbfd7
Show file tree

Hide file tree

Showing 12 changed files with 217 additions and 6 deletions.
diff --git a/cloud/blockstore/config/storage.proto b/cloud/blockstore/config/storage.proto
@@ -4,6 +4,7 @@ package NCloud.NBlockStore.NProto;
 
 import "cloud/storage/core/protos/authorization_mode.proto";
 import "cloud/storage/core/protos/certificate.proto";
+import "cloud/storage/core/protos/config_dispatcher_settings.proto";
 
 option go_package = "github.com/ydb-platform/nbs/cloud/blockstore/config";
 
@@ -1035,4 +1036,7 @@ message TStorageServiceConfig
 
     // Node type
     optional string NodeType = 384;
+
+    // settings for ydb config dispatcher service.
+    optional NCloud.NProto.TConfigDispatcherSettings ConfigDispatcherSettings = 385;
 }
diff --git a/cloud/blockstore/libs/storage/core/config.cpp b/cloud/blockstore/libs/storage/core/config.cpp
@@ -123,6 +123,9 @@ TDuration MSeconds(ui32 value)
     xxx(NodeType,                             TString,               {}       )\
     xxx(NodeRegistrationRootCertsFile,        TString,               {}       )\
     xxx(NodeRegistrationCert,                 TCertificate,          {}       )\
+    xxx(ConfigDispatcherSettings,                                              \
+        NCloud::NProto::TConfigDispatcherSettings,                             \
+        {}                                                                    )\
 // BLOCKSTORE_STORAGE_CONFIG_RO
 
 #define BLOCKSTORE_STORAGE_CONFIG_RW(xxx)                                      \
@@ -596,6 +599,12 @@ bool IsEmpty(const T& t)
     return !t;
 }
 
+template <>
+bool IsEmpty(const NCloud::NProto::TConfigDispatcherSettings& value)
+{
+    return !value.HasAllowList() && !value.HasDenyList();
+}
+
 template <typename T>
 bool IsEmpty(const google::protobuf::RepeatedPtrField<T>& value)
 {

diff --git a/cloud/blockstore/libs/storage/core/config.h b/cloud/blockstore/libs/storage/core/config.h
@@ -604,6 +604,8 @@ class TStorageConfig
     TString GetNodeRegistrationRootCertsFile() const;
     TCertificate GetNodeRegistrationCert() const;
     TString GetNodeType() const;
+
+    NCloud::NProto::TConfigDispatcherSettings GetConfigDispatcherSettings() const;
 };
 
 ui64 GetAllocationUnit(

diff --git a/cloud/blockstore/libs/storage/init/common/actorsystem.cpp b/cloud/blockstore/libs/storage/init/common/actorsystem.cpp
@@ -36,6 +36,8 @@ void TActorSystem::Init()
     InitializeAppData(runConfig);
     InitializeLogSettings(runConfig);
 
+    Args.PrepareKikimrRunConfig(runConfig);
+
     LogSettings->Append(
         TBlockStoreComponents::START,
         TBlockStoreComponents::END,

diff --git a/cloud/blockstore/libs/storage/init/common/actorsystem.h b/cloud/blockstore/libs/storage/init/common/actorsystem.h
@@ -31,6 +31,8 @@ struct TActorSystemArgs
     std::function<void(
         NKikimr::TKikimrRunConfig&,
         NKikimr::TServiceInitializersList&)> OnInitialize;
+    std::function<void(
+        NKikimr::TKikimrRunConfig&)> PrepareKikimrRunConfig;
     NKikimr::TBasicKikimrServicesMask ServicesMask;
     std::function<void(IActorSystem&)> OnStart;
 };

diff --git a/cloud/blockstore/libs/storage/init/disk_agent/actorsystem.cpp b/cloud/blockstore/libs/storage/init/disk_agent/actorsystem.cpp
@@ -13,6 +13,7 @@
 #include <cloud/blockstore/libs/storage/undelivered/undelivered.h>
 #include <cloud/storage/core/libs/api/hive_proxy.h>
 #include <cloud/storage/core/libs/hive_proxy/hive_proxy.h>
+#include <cloud/storage/core/libs/kikimr/config_dispatcher_helpers.h>
 
 namespace NCloud::NBlockStore::NStorage {
 
@@ -130,6 +131,13 @@ IActorSystemPtr CreateDiskAgentActorSystem(const TDiskAgentActorSystemArgs& daAr
     TVector<TIntrusivePtr<IServiceInitializer>> initializers = {
         new TStorageServicesInitializer(daArgs)
     };
+    auto prepareKikimrRunConfig = [&] (TKikimrRunConfig& runConfig) {
+        if (daArgs.StorageConfig->GetConfigsDispatcherServiceEnabled()) {
+            SetupConfigDispatcher(
+                daArgs.StorageConfig->GetConfigDispatcherSettings(),
+                &runConfig.ConfigsDispatcherInitInfo);
+        }
+    };
     auto onInitialize = [&] (
         TKikimrRunConfig& runConfig,
         TServiceInitializersList& initializers)
@@ -171,6 +179,7 @@ IActorSystemPtr CreateDiskAgentActorSystem(const TDiskAgentActorSystemArgs& daAr
         .AppConfig = daArgs.AppConfig,
         .AsyncLogger = daArgs.AsyncLogger,
         .OnInitialize = std::move(onInitialize),
+        .PrepareKikimrRunConfig = std::move(prepareKikimrRunConfig),
         .ServicesMask = servicesMask,
         .OnStart = [] (IActorSystem&) {},
     };

diff --git a/cloud/blockstore/libs/storage/init/server/actorsystem.cpp b/cloud/blockstore/libs/storage/init/server/actorsystem.cpp
@@ -37,6 +37,7 @@
 #include <cloud/storage/core/libs/diagnostics/monitoring.h>
 #include <cloud/storage/core/libs/hive_proxy/hive_proxy.h>
 #include <cloud/storage/core/libs/kikimr/actorsystem.h>
+#include <cloud/storage/core/libs/kikimr/config_dispatcher_helpers.h>
 #include <cloud/storage/core/libs/kikimr/tenant.h>
 #include <cloud/storage/core/libs/user_stats/user_stats.h>
 
@@ -481,6 +482,13 @@ class TCustomLocalServiceInitializer final
 
 IActorSystemPtr CreateActorSystem(const TServerActorSystemArgs& sArgs)
 {
+    auto prepareKikimrRunConfig = [&] (TKikimrRunConfig& runConfig) {
+        if (sArgs.StorageConfig->GetConfigsDispatcherServiceEnabled()) {
+            SetupConfigDispatcher(
+                sArgs.StorageConfig->GetConfigDispatcherSettings(),
+                &runConfig.ConfigsDispatcherInitInfo);
+        }
+    };
     auto onInitialize = [&] (
         TKikimrRunConfig& runConfig,
         TServiceInitializersList& initializers)
@@ -554,6 +562,7 @@ IActorSystemPtr CreateActorSystem(const TServerActorSystemArgs& sArgs)
         .AppConfig = sArgs.AppConfig,
         .AsyncLogger = sArgs.AsyncLogger,
         .OnInitialize = std::move(onInitialize),
+        .PrepareKikimrRunConfig = std::move(prepareKikimrRunConfig),
         .ServicesMask = servicesMask,
         .OnStart = std::move(onStart),
     };

diff --git a/cloud/blockstore/tests/config_dispatcher/test.py b/cloud/blockstore/tests/config_dispatcher/test.py
@@ -0,0 +1,129 @@
+import os
+import requests
+import signal
+import time
+
+
+from cloud.blockstore.config.server_pb2 import TServerConfig, 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.test_base import thread_count, wait_for_nbs_server
+
+from contrib.ydb.core.protos import config_pb2
+
+from contrib.ydb.tests.library.harness.kikimr_cluster import kikimr_cluster_factory
+from contrib.ydb.tests.library.harness.kikimr_config import KikimrConfigGenerator
+
+from contrib.ydb.core.protos.config_pb2 import TLogConfig
+
+import yatest.common as yatest_common
+
+
+def test_config_dispatcher():
+    kikimr_binary_path = yatest_common.binary_path('contrib/ydb/apps/ydbd/ydbd')
+    configurator = KikimrConfigGenerator(
+        erasure=None,
+        binary_path=kikimr_binary_path,
+        has_cluster_uuid=False,
+        use_in_memory_pdisks=True,
+        dynamic_storage_pools=[
+            dict(name='dynamic_storage_pool:1', kind='hdd', pdisk_user_kind=0),
+            dict(name='dynamic_storage_pool:2', kind='ssd', pdisk_user_kind=0)
+        ])
+    kikimr_cluster = kikimr_cluster_factory(configurator=configurator)
+    kikimr_cluster.start()
+
+    server_app_config = TServerAppConfig()
+    server_app_config.ServerConfig.CopyFrom(TServerConfig())
+    server_app_config.ServerConfig.ThreadsCount = thread_count()
+    server_app_config.ServerConfig.StrictContractValidation = False
+    server_app_config.KikimrServiceConfig.CopyFrom(TKikimrServiceConfig())
+    server_app_config.ServerConfig.NodeType = 'nbs'
+
+    certs_dir = yatest_common.source_path('cloud/blockstore/tests/certs')
+    server_app_config.ServerConfig.RootCertsFile = os.path.join(certs_dir, 'server.crt')
+    cert = server_app_config.ServerConfig.Certs.add()
+    cert.CertFile = os.path.join(certs_dir, 'server.crt')
+    cert.CertPrivateKeyFile = os.path.join(certs_dir, 'server.key')
+
+    pm = yatest_common.network.PortManager()
+    nbs_port = pm.get_port()
+    nbs_secure_port = pm.get_port()
+    kikimr_port = list(kikimr_cluster.nodes.values())[0].port
+
+    # file config
+    storage = TStorageServiceConfig()
+    storage.ConfigsDispatcherServiceEnabled = True
+    storage.ConfigDispatcherSettings.AllowList.Names.append('NameserviceConfigItem')
+
+    nbs_binary_path = yatest_common.binary_path('cloud/blockstore/apps/server/nbsd')
+    nbs = LocalNbs(
+        kikimr_port,
+        configurator.domains_txt,
+        server_app_config=server_app_config,
+        storage_config_patches=[storage],
+        enable_tls=True,
+        nbs_secure_port=nbs_secure_port,
+        nbs_port=nbs_port,
+        kikimr_binary_path=kikimr_binary_path,
+        nbs_binary_path=nbs_binary_path)
+    nbs.start()
+    wait_for_nbs_server(nbs.nbs_port)
+
+    def query_monitoring(url, text):
+        r = requests.get(url, timeout=10)
+        r.raise_for_status()
+        return r.text.find(text) != -1
+
+    assert query_monitoring(
+        f'http://localhost:{nbs.mon_port}/actors/logger?c=1025',
+        'Sampling rate: 0')
+
+    app_config = config_pb2.TAppConfig()
+
+    # add new log entry
+    component_to_test = 'BLOCKSTORE_SERVER'.encode()
+    app_config = config_pb2.TAppConfig()
+    log_config = TLogConfig()
+    entry = log_config.Entry.add()
+    entry.Component = component_to_test
+    entry.SamplingRate = 1000
+    app_config.LogConfig.MergeFrom(log_config)
+    kikimr_cluster.client.add_config_item(app_config)
+
+    # add new static node
+    app_config = config_pb2.TAppConfig()
+    naming_config = configurator.names_txt
+    node = naming_config.Node.add(
+        NodeId=2,
+        Address='::1',
+        Port=65535,
+        Host='somewhere',
+    )
+    node.WalleLocation.DataCenter = 'xyz'
+    node.WalleLocation.Rack = 'somewhere'
+    node.WalleLocation.Body = 1
+    app_config.NameserviceConfig.MergeFrom(naming_config)
+    kikimr_cluster.client.add_config_item(app_config)
+
+    def query_monitoring(url, text):
+        r = requests.get(url, timeout=10)
+        r.raise_for_status()
+        return r.text.find(text) != -1
+
+    # wait for nameservice config update
+    while True:
+        if query_monitoring(f'http://localhost:{nbs.mon_port}/actors/dnameserver', 'somewhere'):
+            break
+        else:
+            time.sleep(10)
+
+    # check that logging config was not changed
+    result = query_monitoring(
+        f'http://localhost:{nbs.mon_port}/actors/logger?c=1025',
+        'Sampling rate: 0')
+
+    os.kill(nbs.pid, signal.SIGTERM)
+
+    assert result
diff --git a/cloud/blockstore/tests/config_dispatcher/ya.make b/cloud/blockstore/tests/config_dispatcher/ya.make
@@ -0,0 +1,29 @@
+PY3TEST()
+
+INCLUDE(${ARCADIA_ROOT}/cloud/storage/core/tests/recipes/medium.inc)
+
+TEST_SRCS(test.py)
+
+PEERDIR(
+    cloud/blockstore/config
+    cloud/blockstore/tests/python/lib
+
+    library/python/testing/yatest_common
+
+    contrib/ydb/tests/library
+
+    contrib/python/requests/py3
+)
+
+DEPENDS(
+    cloud/blockstore/apps/client
+    cloud/blockstore/apps/server
+    contrib/ydb/apps/ydbd
+)
+
+DATA(
+    arcadia/cloud/blockstore/tests/certs/server.crt
+    arcadia/cloud/blockstore/tests/certs/server.key
+)
+
+END()
diff --git a/cloud/blockstore/tests/ya.make b/cloud/blockstore/tests/ya.make
@@ -7,6 +7,7 @@ ENDIF()
 
 RECURSE(
     client
+    config_dispatcher
     csi_driver
     disk_agent_config
     external_endpoint

diff --git a/cloud/storage/core/libs/kikimr/config_dispatcher_helpers.cpp b/cloud/storage/core/libs/kikimr/config_dispatcher_helpers.cpp
@@ -12,6 +12,10 @@ void SetupConfigDispatcher(
     const NProto::TConfigDispatcherSettings& settings,
     NKikimr::NConfig::TConfigsDispatcherInitInfo* config)
 {
+    if (!settings.HasAllowList() && !settings.HasDenyList()) {
+        return;
+    }
+
     const auto& names = settings.HasAllowList()
         ? settings.GetAllowList().GetNames()
         : settings.GetDenyList().GetNames();
@@ -35,10 +39,6 @@ void SetupConfigDispatcher(
             << ") as NKikimrConsole::TConfigItem::EKind value");
     }
 
-    if (items.empty()) {
-        return;
-    }
-
     auto& rules = config->ItemsServeRules;
 
     if (settings.HasAllowList()) {

diff --git a/cloud/storage/core/libs/kikimr/config_dispatcher_helpers_ut.cpp b/cloud/storage/core/libs/kikimr/config_dispatcher_helpers_ut.cpp
@@ -45,7 +45,8 @@ Y_UNIT_TEST_SUITE(TConfigDispatcherHelpersTest)
             SetupConfigDispatcher(settings, &config);
 
             UNIT_ASSERT(
-                std::holds_alternative<std::monostate>(config.ItemsServeRules));
+                std::holds_alternative<TAllowList>(config.ItemsServeRules));
+            UNIT_ASSERT(std::get<TAllowList>(config.ItemsServeRules).Items.empty());
             UNIT_ASSERT_VALUES_EQUAL(1, counter->Val());
         }
 
@@ -58,11 +59,25 @@ Y_UNIT_TEST_SUITE(TConfigDispatcherHelpersTest)
             SetupConfigDispatcher(settings, &config);
 
             UNIT_ASSERT(
-                std::holds_alternative<std::monostate>(config.ItemsServeRules));
+                std::holds_alternative<TDenyList>(config.ItemsServeRules));
+            UNIT_ASSERT(std::get<TDenyList>(config.ItemsServeRules).Items.empty());
             UNIT_ASSERT_VALUES_EQUAL(1, counter->Val());
         }
     }
 
+    Y_UNIT_TEST(ShouldAllowAllConfigsWhenAllSettingsAreEmpty)
+    {
+        auto counter = SetupCriticalEvent();
+
+        NKikimr::NConfig::TConfigsDispatcherInitInfo config;
+        NProto::TConfigDispatcherSettings settings;
+        SetupConfigDispatcher(settings, &config);
+
+        UNIT_ASSERT(
+            std::holds_alternative<std::monostate>(config.ItemsServeRules));
+        UNIT_ASSERT_VALUES_EQUAL(0, counter->Val());
+    }
+
     Y_UNIT_TEST(ShouldParseConfigDispatcherItems)
     {
         NKikimr::NConfig::TConfigsDispatcherInitInfo config;