From 895ee7921c3584fa654f230f87e13176b9a788a3 Mon Sep 17 00:00:00 2001 From: 0xavi0 Date: Mon, 23 May 2022 14:01:25 +0200 Subject: [PATCH] rgw/dbstore: change default value of dbstore_db_dir to /var/lib/ceph/radosgw Changes a few NULL to nullptr. Adds std::filesystem for path building so they're platform independant. Fixes a bug for DBStoreManager's second constructor not creating the DB. Adds unit tests to test DB path and prefix. Fixes: https://tracker.ceph.com/issues/55731 Signed-off-by: 0xavi0 --- qa/suites/rgw/dbstore/tasks/rgw_s3tests.yaml | 5 +- src/common/options/rgw.yaml.in | 2 +- src/rgw/store/dbstore/README.md | 9 +- src/rgw/store/dbstore/dbstore_mgr.cc | 16 +- src/rgw/store/dbstore/dbstore_mgr.h | 3 +- src/rgw/store/dbstore/tests/CMakeLists.txt | 4 + .../store/dbstore/tests/dbstore_mgr_tests.cc | 157 ++++++++++++++++++ 7 files changed, 182 insertions(+), 14 deletions(-) create mode 100644 src/rgw/store/dbstore/tests/dbstore_mgr_tests.cc diff --git a/qa/suites/rgw/dbstore/tasks/rgw_s3tests.yaml b/qa/suites/rgw/dbstore/tasks/rgw_s3tests.yaml index 2042f832b6ad..4c8764e4bab4 100644 --- a/qa/suites/rgw/dbstore/tasks/rgw_s3tests.yaml +++ b/qa/suites/rgw/dbstore/tasks/rgw_s3tests.yaml @@ -4,8 +4,9 @@ tasks: - rgw: [client.0] - exec: client.0: - - sudo chmod 0777 /var/run/ceph - - sudo chmod 0777 /var/run/ceph/dbstore-default_ns.db + - sudo chmod 0777 /var/lib/ceph + - sudo chmod 0777 /var/lib/ceph/radosgw + - sudo chmod 0777 /var/lib/ceph/radosgw/dbstore-default_ns.db - s3tests: client.0: force-branch: ceph-master diff --git a/src/common/options/rgw.yaml.in b/src/common/options/rgw.yaml.in index 65bc6b41f890..b174c9029802 100644 --- a/src/common/options/rgw.yaml.in +++ b/src/common/options/rgw.yaml.in @@ -3460,7 +3460,7 @@ options: type: str level: advanced desc: path for the directory for storing the db backend store data - default: /var/run/ceph + default: /var/lib/ceph/radosgw services: - rgw - name: dbstore_db_name_prefix diff --git a/src/rgw/store/dbstore/README.md b/src/rgw/store/dbstore/README.md index dedf8416accd..659bc205276a 100644 --- a/src/rgw/store/dbstore/README.md +++ b/src/rgw/store/dbstore/README.md @@ -29,7 +29,7 @@ The above vstart command brings up RGW server on dbstore and creates few default `radosgw-admin` can be used to create and remove other users. -By default, dbstore creates .db file *'/var/run/ceph/dbstore-default_ns.db'* to store the data. This can be configured using below options in ceph.conf +By default, dbstore creates .db file *'/var/lib/ceph/radosgw/dbstore-default_ns.db'* to store the data. This can be configured using below options in ceph.conf [client] dbstore db dir = @@ -39,12 +39,15 @@ By default, dbstore creates .db file *'/var/run/ceph/dbstore-default_ns.db'* to ## DBStore Unit Tests To execute DBStore unit test cases (using Gtest framework), from build directory - ninja src/rgw/store/dbstore/install - ./bin/dbstore-tests [logfile] [loglevel] + ninja unittest_dbstore_tests + ./bin/unittest_dbstore_tests [logfile] [loglevel] (default logfile: rgw_dbstore_tests.log, loglevel: 20) + ninja unittest_dbstore_mgr_tests + ./bin/unittest_dbstore_mgr_tests To execute Sample test file + ninja src/rgw/store/dbstore/install ./bin/dbstore-bin [logfile] [loglevel] (default logfile: rgw_dbstore_bin.log, loglevel: 20) diff --git a/src/rgw/store/dbstore/dbstore_mgr.cc b/src/rgw/store/dbstore/dbstore_mgr.cc index 5725793b71ba..e54039ccf448 100644 --- a/src/rgw/store/dbstore/dbstore_mgr.cc +++ b/src/rgw/store/dbstore/dbstore_mgr.cc @@ -4,6 +4,8 @@ #include "dbstore_mgr.h" #include "common/dbstore_log.h" +#include + using namespace std; /* Given a tenant, find and return the DBStore handle. @@ -29,7 +31,7 @@ DB *DBStoreManager::getDB (string tenant, bool create) not_found: if (!create) - return NULL; + return nullptr; dbs = createDB(tenant); @@ -41,16 +43,16 @@ DB *DBStoreManager::createDB(std::string tenant) { DB *dbs = nullptr; pair::iterator,bool> ret; const auto& db_path = g_conf().get_val("dbstore_db_dir"); - const auto& db_name = g_conf().get_val("dbstore_db_name_prefix"); + const auto& db_name = g_conf().get_val("dbstore_db_name_prefix") + "-" + tenant; - std::string db_full_path = db_path + "/" + db_name + "-" + tenant; - ldout(cct, 0) << "DB initialization full db_path("< DBStoreHandles; - DB *default_db = NULL; + DB *default_db = nullptr; CephContext *cct; public: @@ -38,6 +38,7 @@ public: cct->_log->set_log_file(logfile); cct->_log->reopen_log_file(); cct->_conf->subsys.set_log_level(ceph_subsys_rgw, loglevel); + default_db = createDB(default_tenant); }; ~DBStoreManager() { destroyAllHandles(); }; diff --git a/src/rgw/store/dbstore/tests/CMakeLists.txt b/src/rgw/store/dbstore/tests/CMakeLists.txt index 5ae2fba67b79..4e60dcf5ee2b 100644 --- a/src/rgw/store/dbstore/tests/CMakeLists.txt +++ b/src/rgw/store/dbstore/tests/CMakeLists.txt @@ -11,3 +11,7 @@ include_directories(${CMAKE_INCLUDE_DIR}) add_executable(unittest_dbstore_tests ${dbstore_tests_srcs}) target_link_libraries(unittest_dbstore_tests ${CMAKE_LINK_LIBRARIES}) add_ceph_unittest(unittest_dbstore_tests) + +add_executable(unittest_dbstore_mgr_tests dbstore_mgr_tests.cc) +target_link_libraries(unittest_dbstore_mgr_tests dbstore gtest_main) +add_ceph_unittest(unittest_dbstore_mgr_tests) diff --git a/src/rgw/store/dbstore/tests/dbstore_mgr_tests.cc b/src/rgw/store/dbstore/tests/dbstore_mgr_tests.cc new file mode 100644 index 000000000000..4f58f476465c --- /dev/null +++ b/src/rgw/store/dbstore/tests/dbstore_mgr_tests.cc @@ -0,0 +1,157 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "common/ceph_context.h" +#include "rgw/store/dbstore/dbstore_mgr.h" + +#include +#include +#include + +using namespace rgw; +namespace fs = std::filesystem; +const static std::string TEST_DIR = "rgw_dbstore_tests"; + +bool endsWith(const std::string &mainStr, const std::string &toMatch) +{ + if(mainStr.size() >= toMatch.size() && + mainStr.compare(mainStr.size() - toMatch.size(), toMatch.size(), toMatch) == 0) + return true; + else + return false; +} + +class TestDBStoreManager : public ::testing::Test { +protected: + void SetUp() override { + ctx_ = std::make_shared(CEPH_ENTITY_TYPE_CLIENT); + g_ceph_context = ctx_.get(); + fs::current_path(fs::temp_directory_path()); + fs::create_directory(TEST_DIR); + } + + void TearDown() override { + fs::current_path(fs::temp_directory_path()); + fs::remove_all(TEST_DIR); + } + + std::string getTestDir() const { + auto test_dir = fs::temp_directory_path() / TEST_DIR; + return test_dir.string(); + } + + fs::path getDBFullPath(const std::string & base_dir, + const std::string & tenant) const { + auto db_path = ctx_->_conf.get_val("dbstore_db_dir"); + const auto& db_name = ctx_->_conf.get_val("dbstore_db_name_prefix") + "-" + tenant + ".db"; + + auto db_full_path = std::filesystem::path(db_path) / db_name; + auto db_full_path_test = fs::path(base_dir) / db_full_path; + return db_full_path_test; + } + + std::string getDBTenant(const std::string & base_dir, + const std::string & tenant) const { + auto db_name = ctx_->_conf.get_val("dbstore_db_name_prefix"); + db_name += "-" + tenant; + auto db_full_path = fs::path(base_dir) / db_name; + return db_full_path.string(); + } + + std::string getDBTenant(const std::string & tenant = default_tenant) const { + return getDBTenant(getTestDir(), tenant); + } + + fs::path getDBFullPath(const std::string & tenant) const { + return getDBFullPath(getTestDir(), tenant); + } + + fs::path getLogFilePath(const std::string & log_file) { + return fs::temp_directory_path() / log_file; + } + + std::shared_ptr getContext() const { + return ctx_; + } + + private: + std::shared_ptr ctx_; +}; + +TEST_F(TestDBStoreManager, BasicInstantiateUsingDBDir) { + getContext()->_conf.set_val("dbstore_db_dir", getTestDir()); + + EXPECT_FALSE(fs::exists(getDBFullPath(default_tenant))); + auto dbstore_mgr = std::make_shared(getContext().get()); + EXPECT_TRUE(fs::exists(getDBFullPath(default_tenant))); +} + +TEST_F(TestDBStoreManager, DBNamePrefix) { + getContext()->_conf.set_val("dbstore_db_dir", getTestDir()); + std::string prefix = "testprefix"; + getContext()->_conf.set_val("dbstore_db_name_prefix", prefix); + + EXPECT_FALSE(fs::exists(getDBFullPath(default_tenant))); + auto dbstore_mgr = std::make_shared(getContext().get()); + EXPECT_TRUE(fs::exists(getDBFullPath(default_tenant))); + + // check that the database name contains the given prefix + std::string expected_db_name = prefix + "-" + default_tenant + ".db"; + EXPECT_TRUE(endsWith(getDBFullPath(default_tenant), expected_db_name)); +} + +TEST_F(TestDBStoreManager, BasicInstantiateSecondConstructor) { + getContext()->_conf.set_val("dbstore_db_dir", getTestDir()); + + EXPECT_FALSE(fs::exists(getDBFullPath(default_tenant))); + auto dbstore_mgr = std::make_shared(getContext().get(), getLogFilePath("test.log").string(), 10); + EXPECT_TRUE(fs::exists(getDBFullPath(default_tenant))); +} + +TEST_F(TestDBStoreManager, TestDBName) { + getContext()->_conf.set_val("dbstore_db_dir", getTestDir()); + + auto dbstore_mgr = std::make_shared(getContext().get()); + auto db = dbstore_mgr->getDB(default_tenant, false); + ASSERT_NE(nullptr, db); + EXPECT_EQ(getDBTenant(), db->getDBname()); +} + + +TEST_F(TestDBStoreManager, TestDBNameDefaultDB) { + getContext()->_conf.set_val("dbstore_db_dir", getTestDir()); + + auto dbstore_mgr = std::make_shared(getContext().get()); + // passing an empty tenant should return the default_db + auto db = dbstore_mgr->getDB("", false); + ASSERT_NE(nullptr, db); + EXPECT_EQ(getDBTenant(), db->getDBname()); +} + +TEST_F(TestDBStoreManager, TestDBBadTenant) { + getContext()->_conf.set_val("dbstore_db_dir", getTestDir()); + + auto dbstore_mgr = std::make_shared(getContext().get()); + auto db = dbstore_mgr->getDB("does-not-exist", false); + ASSERT_EQ(nullptr, db); +} + +TEST_F(TestDBStoreManager, TestGetNewDB) { + getContext()->_conf.set_val("dbstore_db_dir", getTestDir()); + + auto dbstore_mgr = std::make_shared(getContext().get()); + + auto new_tenant_path = "new_tenant"; + auto db = dbstore_mgr->getDB(new_tenant_path, true); + ASSERT_NE(nullptr, db); + EXPECT_EQ(getDBTenant(new_tenant_path), db->getDBname()); +} + +TEST_F(TestDBStoreManager, TestDelete) { + getContext()->_conf.set_val("dbstore_db_dir", getTestDir()); + + auto dbstore_mgr = std::make_shared(getContext().get()); + dbstore_mgr->deleteDB(default_tenant); + auto db = dbstore_mgr->getDB(default_tenant, false); + ASSERT_EQ(nullptr, db); +} -- 2.47.3