From 2f3c2c51403e3ceb9711466138bb9418ac66b4de Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Sat, 24 Dec 2022 16:07:02 +0300 Subject: [PATCH] Implement safety check for OSD removal, translate all '-' to '_' in cli options '-' to '_' translation fixes a bug with create --image_meta --- docs/usage/cli.en.md | 12 ++++ docs/usage/cli.ru.md | 12 ++++ src/cli.cpp | 33 ++++++---- src/cli.h | 2 +- src/cli_common.cpp | 17 ++++- src/cli_create.cpp | 2 +- src/cli_flatten.cpp | 2 +- src/cli_merge.cpp | 4 +- src/cli_modify.cpp | 2 +- src/cli_rm.cpp | 2 +- src/cli_rm_data.cpp | 2 +- src/cli_rm_osd.cpp | 147 ++++++++++++++++++++++++++++++++++++++++--- src/str_util.cpp | 10 +++ src/str_util.h | 1 + 14 files changed, 214 insertions(+), 34 deletions(-) diff --git a/docs/usage/cli.en.md b/docs/usage/cli.en.md index a21e19ab..6035be34 100644 --- a/docs/usage/cli.en.md +++ b/docs/usage/cli.en.md @@ -20,6 +20,7 @@ It supports the following commands: - [rm-data](#rm-data) - [merge-data](#merge-data) - [alloc-osd](#alloc-osd) +- [rm-osd](#rm-osd) Global options: @@ -175,3 +176,14 @@ Merge layer data without changing metadata. Merge ``..`` to `` `vitastor-cli alloc-osd` Allocate a new OSD number and reserve it by creating empty `/osd/stats/` key. + +## rm-osd + +`vitastor-cli rm-osd [--force] [--allow-data-loss] [--dry-run] [osd_id...]` + +Remove metadata and configuration for specified OSD(s) from etcd. + +Refuses to remove OSDs with data without `--force` and `--allow-data-loss`. + +With `--dry-run` only checks if deletion is possible without data loss and +redundancy degradation. diff --git a/docs/usage/cli.ru.md b/docs/usage/cli.ru.md index 3704f6da..c3bb3e2b 100644 --- a/docs/usage/cli.ru.md +++ b/docs/usage/cli.ru.md @@ -21,6 +21,7 @@ vitastor-cli - интерфейс командной строки для адм - [rm-data](#rm-data) - [merge-data](#merge-data) - [alloc-osd](#alloc-osd) +- [rm-osd](#rm-osd) Глобальные опции: @@ -186,3 +187,14 @@ vitastor-cli snap-create [-p|--pool ] @ Атомарно выделить новый номер OSD и зарезервировать его, создав в etcd пустой ключ `/osd/stats/`. + +## rm-osd + +`vitastor-cli rm-osd [--force] [--allow-data-loss] [--dry-run] [osd_id...]` + +Удалить метаданные и конфигурацию для заданных OSD из etcd. + +Отказывается удалять OSD с данными без опций `--force` и `--allow-data-loss`. + +С опцией `--dry-run` только проверяет, возможно ли удаление без потери данных и деградации +избыточности. diff --git a/src/cli.cpp b/src/cli.cpp index 30f36e92..2d401588 100644 --- a/src/cli.cpp +++ b/src/cli.cpp @@ -73,12 +73,15 @@ static const char* help_text = " must be a child of and may be one of the layers between\n" " and , including and .\n" "\n" - "vitastor-cli rm-osd [osd_id...]\n" - " Remove metadata and configuration for specified OSD(s) from etcd.\n" - "\n" "vitastor-cli alloc-osd\n" " Allocate a new OSD number and reserve it by creating empty /osd/stats/ key.\n" "\n" + "vitastor-cli rm-osd [--force] [--allow-data-loss] [--dry-run] [osd_id...]\n" + " Remove metadata and configuration for specified OSD(s) from etcd.\n" + " Refuses to remove OSDs with data without --force and --allow-data-loss.\n" + " With --dry-run only checks if deletion is possible without data loss and\n" + " redundancy degradation.\n" + "\n" "Use vitastor-cli --help for command details or vitastor-cli --help --all for all details.\n" "\n" "GLOBAL OPTIONS:\n" @@ -98,43 +101,47 @@ static json11::Json::object parse_args(int narg, const char *args[]) cfg["progress"] = "1"; for (int i = 1; i < narg; i++) { - if (args[i][0] == '-' && args[i][1] == 'h') + if (args[i][0] == '-' && args[i][1] == 'h' && args[i][2] == 0) { cfg["help"] = "1"; } - else if (args[i][0] == '-' && args[i][1] == 'l') + else if (args[i][0] == '-' && args[i][1] == 'l' && args[i][2] == 0) { cfg["long"] = "1"; } - else if (args[i][0] == '-' && args[i][1] == 'n') + else if (args[i][0] == '-' && args[i][1] == 'n' && args[i][2] == 0) { cfg["count"] = args[++i]; } - else if (args[i][0] == '-' && args[i][1] == 'p') + else if (args[i][0] == '-' && args[i][1] == 'p' && args[i][2] == 0) { cfg["pool"] = args[++i]; } - else if (args[i][0] == '-' && args[i][1] == 's') + else if (args[i][0] == '-' && args[i][1] == 's' && args[i][2] == 0) { cfg["size"] = args[++i]; } - else if (args[i][0] == '-' && args[i][1] == 'r') + else if (args[i][0] == '-' && args[i][1] == 'r' && args[i][2] == 0) { cfg["reverse"] = "1"; } - else if (args[i][0] == '-' && args[i][1] == 'f') + else if (args[i][0] == '-' && args[i][1] == 'f' && args[i][2] == 0) { cfg["force"] = "1"; } else if (args[i][0] == '-' && args[i][1] == '-') { const char *opt = args[i]+2; - cfg[opt] = i == narg-1 || !strcmp(opt, "json") || !strcmp(opt, "wait-list") || - !strcmp(opt, "long") || !strcmp(opt, "del") || !strcmp(opt, "no-color") || + cfg[opt] = i == narg-1 || !strcmp(opt, "json") || + !strcmp(opt, "wait-list") || !strcmp(opt, "wait_list") || + !strcmp(opt, "long") || !strcmp(opt, "del") || + !strcmp(opt, "no-color") || !strcmp(opt, "no_color") || !strcmp(opt, "readonly") || !strcmp(opt, "readwrite") || !strcmp(opt, "force") || !strcmp(opt, "reverse") || + !strcmp(opt, "allow-data-loss") || !strcmp(opt, "allow_data_loss") || + !strcmp(opt, "dry-run") || !strcmp(opt, "dry_run") || !strcmp(opt, "help") || !strcmp(opt, "all") || - !strcmp(opt, "writers-stopped") && strcmp("1", args[i+1]) != 0 + (!strcmp(opt, "writers-stopped") || !strcmp(opt, "writers_stopped")) && strcmp("1", args[i+1]) != 0 ? "1" : args[++i]; } else diff --git a/src/cli.h b/src/cli.h index 01c921ed..4d4c1012 100644 --- a/src/cli.h +++ b/src/cli.h @@ -45,7 +45,7 @@ public: cli_result_t etcd_err; json11::Json etcd_result; - void parse_config(json11::Json cfg); + void parse_config(json11::Json::object & cfg); void change_parent(inode_t cur, inode_t new_parent, cli_result_t *result); inode_config_t* get_inode_cfg(const std::string & name); diff --git a/src/cli_common.cpp b/src/cli_common.cpp index 94c0960f..9f7a953e 100644 --- a/src/cli_common.cpp +++ b/src/cli_common.cpp @@ -100,9 +100,20 @@ inode_config_t* cli_tool_t::get_inode_cfg(const std::string & name) return NULL; } -void cli_tool_t::parse_config(json11::Json cfg) +void cli_tool_t::parse_config(json11::Json::object & cfg) { - color = !cfg["no-color"].bool_value(); + for (auto kv_it = cfg.begin(); kv_it != cfg.end();) + { + // Translate all options with - to _ + if (kv_it->first.find("-") != std::string::npos) + { + cfg[str_replace(kv_it->first, "-", "_")] = kv_it->second; + cfg.erase(kv_it++); + } + else + kv_it++; + } + color = !cfg["no_color"].bool_value(); json_output = cfg["json"].bool_value(); iodepth = cfg["iodepth"].uint64_value(); if (!iodepth) @@ -112,7 +123,7 @@ void cli_tool_t::parse_config(json11::Json cfg) parallel_osds = 4; log_level = cfg["log_level"].int64_value(); progress = cfg["progress"].uint64_value() ? true : false; - list_first = cfg["wait-list"].uint64_value() ? true : false; + list_first = cfg["wait_list"].uint64_value() ? true : false; } struct cli_result_looper_t diff --git a/src/cli_create.cpp b/src/cli_create.cpp index 1c802cb5..b239ad20 100644 --- a/src/cli_create.cpp +++ b/src/cli_create.cpp @@ -517,7 +517,7 @@ std::function cli_tool_t::start_create(json11::Json cfg) image_creator->force_size = cfg["force_size"].bool_value(); if (cfg["image_meta"].is_object()) { - image_creator->new_meta = cfg["image-meta"]; + image_creator->new_meta = cfg["image_meta"]; } if (cfg["snapshot"].string_value() != "") { diff --git a/src/cli_flatten.cpp b/src/cli_flatten.cpp index 262c26b8..5adc7798 100644 --- a/src/cli_flatten.cpp +++ b/src/cli_flatten.cpp @@ -133,7 +133,7 @@ std::function cli_tool_t::start_flatten(json11::Json cfg) auto flattener = new snap_flattener_t(); flattener->parent = this; flattener->target_name = cfg["image"].string_value(); - flattener->fsync_interval = cfg["fsync-interval"].uint64_value(); + flattener->fsync_interval = cfg["fsync_interval"].uint64_value(); if (!flattener->fsync_interval) flattener->fsync_interval = 128; if (!cfg["cas"].is_null()) diff --git a/src/cli_merge.cpp b/src/cli_merge.cpp index fae43e35..3447af45 100644 --- a/src/cli_merge.cpp +++ b/src/cli_merge.cpp @@ -631,8 +631,8 @@ std::function cli_tool_t::start_merge(json11::Json cfg) merger->from_name = cfg["from"].string_value(); merger->to_name = cfg["to"].string_value(); merger->target_name = cfg["target"].string_value(); - merger->delete_source = cfg["delete-source"].string_value() != ""; - merger->fsync_interval = cfg["fsync-interval"].uint64_value(); + merger->delete_source = cfg["delete_source"].string_value() != ""; + merger->fsync_interval = cfg["fsync_interval"].uint64_value(); if (!merger->fsync_interval) merger->fsync_interval = 128; if (!cfg["cas"].is_null()) diff --git a/src/cli_modify.cpp b/src/cli_modify.cpp index c2e8ab55..43bb69c9 100644 --- a/src/cli_modify.cpp +++ b/src/cli_modify.cpp @@ -236,7 +236,7 @@ std::function cli_tool_t::start_modify(json11::Json cfg) changer->force = cfg["force"].bool_value(); changer->set_readonly = cfg["readonly"].bool_value(); changer->set_readwrite = cfg["readwrite"].bool_value(); - changer->fsync_interval = cfg["fsync-interval"].uint64_value(); + changer->fsync_interval = cfg["fsync_interval"].uint64_value(); if (!changer->fsync_interval) changer->fsync_interval = 128; // FIXME Check that the image doesn't have children when shrinking diff --git a/src/cli_rm.cpp b/src/cli_rm.cpp index 127e319f..82d491dd 100644 --- a/src/cli_rm.cpp +++ b/src/cli_rm.cpp @@ -639,7 +639,7 @@ std::function cli_tool_t::start_rm(json11::Json cfg) snap_remover->parent = this; snap_remover->from_name = cfg["from"].string_value(); snap_remover->to_name = cfg["to"].string_value(); - snap_remover->fsync_interval = cfg["fsync-interval"].uint64_value(); + snap_remover->fsync_interval = cfg["fsync_interval"].uint64_value(); if (!snap_remover->fsync_interval) snap_remover->fsync_interval = 128; if (!cfg["cas"].is_null()) diff --git a/src/cli_rm_data.cpp b/src/cli_rm_data.cpp index 6d9087e3..abd8bf6b 100644 --- a/src/cli_rm_data.cpp +++ b/src/cli_rm_data.cpp @@ -218,7 +218,7 @@ std::function cli_tool_t::start_rm_data(json11::Json cfg) remover->inode = (remover->inode & (((uint64_t)1 << (64-POOL_ID_BITS)) - 1)) | (((uint64_t)remover->pool_id) << (64-POOL_ID_BITS)); } remover->pool_id = INODE_POOL(remover->inode); - remover->min_offset = cfg["min-offset"].uint64_value(); + remover->min_offset = cfg["min_offset"].uint64_value(); return [remover](cli_result_t & result) { remover->loop(); diff --git a/src/cli_rm_osd.cpp b/src/cli_rm_osd.cpp index 8352ff48..9bfed147 100644 --- a/src/cli_rm_osd.cpp +++ b/src/cli_rm_osd.cpp @@ -13,11 +13,16 @@ struct rm_osd_t { cli_tool_t *parent; + bool dry_run, force_warning, force_dataloss; std::vector osd_ids; int state = 0; cli_result_t result; + std::set to_remove; + json11::Json::array pool_effects; + bool is_warning, is_dataloss; + bool is_done() { return state == 100; @@ -33,16 +38,136 @@ struct rm_osd_t state = 100; return; } + for (auto osd_id: osd_ids) + { + if (!osd_id) + { + result = (cli_result_t){ .err = EINVAL, .text = "OSD number can't be zero" }; + state = 100; + return; + } + to_remove.insert(osd_id); + } + // Check if OSDs are still used in data distribution + is_warning = is_dataloss = false; + for (auto & pp: parent->cli->st_cli.pool_config) + { + // Will OSD deletion make pool incomplete / down / degraded? + bool pool_incomplete = false, pool_down = false, pool_degraded = false; + bool hist_incomplete = false, hist_degraded = false; + auto & pool_cfg = pp.second; + uint64_t pg_data_size = (pool_cfg.scheme == POOL_SCHEME_REPLICATED ? 1 : pool_cfg.pg_size-pool_cfg.parity_chunks); + for (auto & pgp: pool_cfg.pg_config) + { + auto & pg_cfg = pgp.second; + int pg_cursize = 0, pg_rm = 0; + for (auto pg_osd: pg_cfg.target_set) + { + if (pg_osd != 0) + { + pg_cursize++; + if (to_remove.find(pg_osd) != to_remove.end()) + pg_rm++; + } + } + for (auto & hist_item: pg_cfg.target_history) + { + int hist_size = 0, hist_rm = 0; + for (auto & old_osd: hist_item) + { + if (old_osd != 0) + { + hist_size++; + if (to_remove.find(old_osd) != to_remove.end()) + hist_rm++; + } + } + if (hist_rm > 0) + { + hist_degraded = true; + if (hist_size-hist_rm == 0) + pool_incomplete = true; + else if (hist_size-hist_rm < pg_data_size) + hist_incomplete = true; + } + } + if (pg_rm > 0) + { + pool_degraded = true; + if (pg_cursize-pg_rm < pg_data_size) + pool_incomplete = true; + else if (pg_cursize-pg_rm < pool_cfg.pg_minsize) + pool_down = true; + } + } + if (pool_incomplete || pool_down || pool_degraded || hist_incomplete || hist_degraded) + { + pool_effects.push_back(json11::Json::object { + { "pool_id", (uint64_t)pool_cfg.id }, + { "pool_name", pool_cfg.name }, + { "effect", (pool_incomplete + ? "incomplete" + : (hist_incomplete + ? "has_incomplete" + : (pool_down + ? "offline" + : (pool_degraded + ? "degraded" + : (hist_degraded ? "has_degraded" : "?") + ) + ) + ) + ) }, + }); + is_warning = true; + if (pool_incomplete || hist_incomplete) + is_dataloss = true; + } + } + result.data = json11::Json::object { + { "osd_ids", osd_ids }, + { "pool_errors", pool_effects }, + }; + if (is_dataloss || is_warning || dry_run) + { + std::string error; + for (auto & e: pool_effects) + { + error += "Pool "+e["pool_name"].string_value()+" (ID "+e["pool_id"].as_string()+") will have "+( + e["effect"] == "has_incomplete" + ? std::string("INCOMPLETE objects (DATA LOSS)") + : (e["effect"] == "incomplete" + ? std::string("INCOMPLETE PGs (DATA LOSS)") + : (e["effect"] == "has_degraded" + ? std::string("DEGRADED objects") + : strtoupper(e["effect"].string_value())+" PGs")) + )+" after deleting OSD(s).\n"; + } + if (is_dataloss && !force_dataloss && !dry_run) + error += "OSDs not deleted. Please move data to other OSDs or bypass this check with --allow-data-loss if you know what you are doing.\n"; + else if (is_warning && !force_warning && !dry_run) + error += "OSDs not deleted. Please move data to other OSDs or bypass this check with --force if you know what you are doing.\n"; + else if (!is_dataloss && !is_warning && dry_run) + error += "OSDs can be deleted without data loss.\n"; + result.text = error; + if (is_dataloss && !force_dataloss || is_warning && !force_warning) + { + result.err = EBUSY; + state = 100; + return; + } + if (dry_run) + { + result.err = 0; + state = 100; + return; + } + } + // Remove keys from etcd { json11::Json::array rm_items; for (auto osd_id: osd_ids) { - if (!osd_id) - { - result = (cli_result_t){ .err = EINVAL, .text = "OSD number can't be zero" }; - state = 100; - return; - } rm_items.push_back("/config/osd/"+std::to_string(osd_id)); rm_items.push_back("/osd/stats/"+std::to_string(osd_id)); rm_items.push_back("/osd/state/"+std::to_string(osd_id)); @@ -76,11 +201,10 @@ struct rm_osd_t { ids += (ids.size() ? ", " : "")+std::to_string(osd_id); } + ids = (osd_ids.size() > 1 ? "OSDs " : "OSD ")+ids+(osd_ids.size() > 1 ? " are" : " is")+" removed from etcd"; state = 100; - result = (cli_result_t){ - .text = (osd_ids.size() > 1 ? "OSDs " : "OSD ")+ids+(osd_ids.size() > 1 ? " are" : " is")+" removed from etcd", - .data = json11::Json::object{ { "osd_ids", osd_ids } }, - }; + result.text = (result.text != "" ? ids+"\n"+result.text : ids); + result.err = 0; } }; @@ -88,6 +212,9 @@ std::function cli_tool_t::start_rm_osd(json11::Json cfg) { auto rm_osd = new rm_osd_t(); rm_osd->parent = this; + rm_osd->dry_run = cfg["dry_run"].bool_value(); + rm_osd->force_dataloss = cfg["allow_data_loss"].bool_value(); + rm_osd->force_warning = rm_osd->force_dataloss || cfg["force"].bool_value(); if (cfg["osd_id"].is_number() || cfg["osd_id"].is_string()) rm_osd->osd_ids.push_back(cfg["osd_id"].uint64_value()); else diff --git a/src/str_util.cpp b/src/str_util.cpp index 0484ceea..5f9e9fed 100644 --- a/src/str_util.cpp +++ b/src/str_util.cpp @@ -56,6 +56,16 @@ std::string base64_decode(const std::string &in) return out; } +std::string strtoupper(const std::string & in) +{ + std::string s = in; + for (int i = 0; i < s.length(); i++) + { + s[i] = toupper(s[i]); + } + return s; +} + std::string strtolower(const std::string & in) { std::string s = in; diff --git a/src/str_util.h b/src/str_util.h index 4e0d55e8..dc353d50 100644 --- a/src/str_util.h +++ b/src/str_util.h @@ -8,6 +8,7 @@ std::string base64_encode(const std::string &in); std::string base64_decode(const std::string &in); uint64_t parse_size(std::string size_str, bool *ok = NULL); +std::string strtoupper(const std::string & in); std::string strtolower(const std::string & in); std::string trim(const std::string & in, const char *rm_chars = " \n\r\t"); std::string str_replace(const std::string & in, const std::string & needle, const std::string & replacement);