From 545bc83568d653bcede208e0d869bc2d187fd37b Mon Sep 17 00:00:00 2001 From: Joao Eduardo Luis Date: Mon, 13 Feb 2017 00:31:33 +0000 Subject: [PATCH 1/5] mon: drop weird/failed mon features debug cli Signed-off-by: Joao Eduardo Luis --- src/mon/MonCommands.h | 16 ------ src/mon/Monitor.cc | 117 ------------------------------------------ 2 files changed, 133 deletions(-) diff --git a/src/mon/MonCommands.h b/src/mon/MonCommands.h index fb0276c96da..0d7dde8907a 100644 --- a/src/mon/MonCommands.h +++ b/src/mon/MonCommands.h @@ -441,22 +441,6 @@ COMMAND("mon remove " \ COMMAND("mon rm " \ "name=name,type=CephString", \ "remove monitor named ", "mon", "rw", "cli,rest") -COMMAND("mon debug set_feature" \ - "name=feature_type,type=CephChoices,strings=persistent|optional " \ - "name=feature_name,type=CephString " \ - "name=sure,type=CephChoices,strings=--yes-i-really-mean-it,req=false", \ - "set provided feature on mon map", \ - "mon", "rw", "cli") -COMMAND("mon debug list_features " \ - "name=feature_type,type=CephString,req=false", \ - "list available mon map features to be set/unset", \ - "mon", "rw", "cli") -COMMAND("mon debug unset_feature " \ - "name=feature_type,type=CephChoices,strings=persistent|optional " \ - "name=feature_name,type=CephString " \ - "name=sure,type=CephChoices,strings=--yes-i-really-mean-it,req=false", \ - "unset provided feature from monmap", \ - "mon", "rw", "cli") /* * OSD commands diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc index b09638dd097..7205fdc638c 100644 --- a/src/mon/Monitor.cc +++ b/src/mon/Monitor.cc @@ -331,123 +331,6 @@ void Monitor::do_admin_command(string command, cmdmap_t& cmdmap, string format, if (f) { f->flush(ss); } - } else if (boost::starts_with(command, "debug mon features")) { - - // check if unsupported feature is set - if (!cct->check_experimental_feature_enabled("mon_debug_features_commands")) { - ss << "error: this is an experimental feature and is not enabled."; - goto abort; - } - - if (command == "debug mon features list") { - - mon_feature_t supported = ceph::features::mon::get_supported(); - mon_feature_t persistent = ceph::features::mon::get_persistent(); - - if (f) { - - f->open_object_section("features"); - f->open_object_section("ceph-mon"); - supported.dump_with_value(f.get(), "supported"); - persistent.dump_with_value(f.get(), "persistent"); - f->close_section(); // ceph-mon - f->open_object_section("monmap"); - monmap->persistent_features.dump_with_value(f.get(), "persistent"); - monmap->optional_features.dump_with_value(f.get(), "optional"); - mon_feature_t required = monmap->get_required_features(); - required.dump_with_value(f.get(), "required"); - f->close_section(); // monmap - f->close_section(); // features - - f->flush(ss); - } else { - ss << "only structured formats allowed when listing"; - } - } else if (command == "debug mon features set" || - command == "debug mon features set_val" || - command == "debug mon features unset" || - command == "debug mon features unset_val") { - - string n; - if (!cmd_getval(cct, cmdmap, "feature", n)) { - ss << "missing feature to set"; - goto abort; - } - - string f_type; - bool do_persistent = false, do_optional = false; - - if (cmd_getval(cct, cmdmap, "feature_type", f_type)) { - if (f_type == "--persistent") { - do_persistent = true; - } else { - do_optional = true; - } - } - - mon_feature_t feature; - - if (command == "debug mon features set" || - command == "debug mon features unset") { - feature = ceph::features::mon::get_feature_by_name(n); - if (feature == ceph::features::mon::FEATURE_NONE) { - ss << "no such feature '" << n << "'"; - goto abort; - } - } else { - uint64_t feature_val; - string interr; - feature_val = strict_strtoll(n.c_str(), 10, &interr); - if (!interr.empty()) { - ss << "unable to parse feature value: " << interr; - goto abort; - } - - feature = mon_feature_t(feature_val); - } - - bool do_unset = false; - if (boost::ends_with(command, "unset") || - boost::ends_with(command, "unset_val")) { - do_unset = true; - } - - ss << (do_unset? "un" : "") << "setting feature '"; - feature.print_with_value(ss); - ss << "' on current monmap\n"; - ss << "please note this change is not persistent; " - << "changes to monmap will overwrite the changes\n"; - - if (!do_persistent && !do_optional) { - if (ceph::features::mon::get_persistent().contains_all(feature)) { - do_persistent = true; - } else { - do_optional = true; - } - } - - ss << "\n" << (do_unset ? "un" : "") << "setting "; - - mon_feature_t &target_feature = (do_persistent ? - monmap->persistent_features : monmap->optional_features); - - if (do_persistent) { - ss << "persistent feature"; - } else { - ss << "optional feature"; - } - - if (do_unset) { - target_feature.unset_feature(feature); - } else { - target_feature.set_feature(feature); - } - - } else { - - ss << "unrecognized command"; - } - } else { assert(0 == "bad AdminSocket command binding"); } From 9c6a6f19d84831036152a207dc8d9a28743f7265 Mon Sep 17 00:00:00 2001 From: Joao Eduardo Luis Date: Mon, 13 Feb 2017 00:35:18 +0000 Subject: [PATCH 2/5] mon: better 'mon features' cli Allows listing supported and currently set monmap features, as well as setting and unsetting them. Signed-off-by: Joao Eduardo Luis --- src/mon/MonCommands.h | 9 +++ src/mon/MonmapMonitor.cc | 145 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 151 insertions(+), 3 deletions(-) diff --git a/src/mon/MonCommands.h b/src/mon/MonCommands.h index 0d7dde8907a..3f2ba92d0c8 100644 --- a/src/mon/MonCommands.h +++ b/src/mon/MonCommands.h @@ -441,6 +441,15 @@ COMMAND("mon remove " \ COMMAND("mon rm " \ "name=name,type=CephString", \ "remove monitor named ", "mon", "rw", "cli,rest") +COMMAND("mon feature list " \ + "name=with_value,type=CephChoices,strings=--with-value,req=false", \ + "list available mon map features to be set/unset", \ + "mon", "r", "cli,rest") +COMMAND("mon feature set " \ + "name=feature_name,type=CephString " \ + "name=sure,type=CephChoices,strings=--yes-i-really-mean-it,req=false", \ + "set provided feature on mon map", \ + "mon", "rw", "cli,rest") /* * OSD commands diff --git a/src/mon/MonmapMonitor.cc b/src/mon/MonmapMonitor.cc index 8d9ef32f37a..5e18dba7572 100644 --- a/src/mon/MonmapMonitor.cc +++ b/src/mon/MonmapMonitor.cc @@ -224,6 +224,10 @@ bool MonmapMonitor::preprocess_command(MonOpRequestRef op) return true; } + string format; + cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain")); + boost::scoped_ptr f(Formatter::create(format)); + if (prefix == "mon stat") { mon->monmap->print_summary(ss); ss << ", election epoch " << mon->get_epoch() << ", quorum " << mon->get_quorum() @@ -261,10 +265,7 @@ bool MonmapMonitor::preprocess_command(MonOpRequestRef op) r = 0; ss << "got monmap epoch " << p->get_epoch(); } else if (prefix == "mon dump") { - string format; - cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain")); stringstream ds; - boost::scoped_ptr f(Formatter::create(format)); if (f) { f->open_object_section("monmap"); p->dump(f.get()); @@ -286,6 +287,78 @@ bool MonmapMonitor::preprocess_command(MonOpRequestRef op) } if (p != mon->monmap) delete p; + + } else if (prefix == "mon feature list") { + + bool list_with_value = false; + string with_value; + if (cmd_getval(g_ceph_context, cmdmap, "with_value", with_value) && + with_value == "--with-value") { + list_with_value = true; + } + + MonMap *p = mon->monmap; + + // list features + mon_feature_t supported = ceph::features::mon::get_supported(); + mon_feature_t persistent = ceph::features::mon::get_persistent(); + mon_feature_t required = p->get_required_features(); + + stringstream ds; + auto print_feature = [&](mon_feature_t& m_features, const char* m_str) { + if (f) { + if (list_with_value) + m_features.dump_with_value(f.get(), m_str); + else + m_features.dump(f.get(), m_str); + } else { + if (list_with_value) + m_features.print_with_value(ds); + else + m_features.print(ds); + } + }; + + if (f) { + f->open_object_section("features"); + + f->open_object_section("all"); + print_feature(supported, "supported"); + print_feature(persistent, "persistent"); + f->close_section(); // all + + f->open_object_section("monmap"); + print_feature(p->persistent_features, "persistent"); + print_feature(p->optional_features, "optional"); + print_feature(required, "required"); + f->close_section(); // monmap + + f->close_section(); // features + f->flush(ds); + + } else { + ds << "all features" << std::endl + << "\tsupported: "; + print_feature(supported, nullptr); + ds << std::endl + << "\tpersistent: "; + print_feature(persistent, nullptr); + ds << std::endl + << std::endl; + + ds << "on current monmap (epoch " + << p->get_epoch() << ")" << std::endl + << "\tpersistent: "; + print_feature(p->persistent_features, nullptr); + ds << std::endl + // omit optional features in plain-text + // makes it easier to read, and they're, currently, empty. + << "\trequired: "; + print_feature(required, nullptr); + ds << std::endl; + } + rdata.append(ds); + r = 0; } reply: @@ -513,6 +586,72 @@ bool MonmapMonitor::prepare_command(MonOpRequestRef op) propose = true; err = 0; + } else if (prefix == "mon feature set") { + + /* PLEASE NOTE: + * + * We currently only support setting/unsetting persistent features. + * This is by design, given at the moment we still don't have optional + * features, and, as such, there is no point introducing an interface + * to manipulate them. This allows us to provide a cleaner, more + * intuitive interface to the user, modifying solely persistent + * features. + * + * In the future we should consider adding another interface to handle + * optional features/flags; e.g., 'mon feature flag set/unset', or + * 'mon flag set/unset'. + */ + string feature_name; + if (!cmd_getval(g_ceph_context, cmdmap, "feature_name", feature_name)) { + ss << "missing required feature name"; + err = -EINVAL; + goto reply; + } + + mon_feature_t feature; + feature = ceph::features::mon::get_feature_by_name(feature_name); + if (feature == ceph::features::mon::FEATURE_NONE) { + ss << "unknown feature '" << feature_name << "'"; + err = -ENOENT; + goto reply; + } + + string sure; + if (!cmd_getval(g_ceph_context, cmdmap, "sure", sure) || + sure != "--yes-i-really-mean-it") { + ss << "please specify '--yes-i-really-mean-it' if you " + << "really, **really** want to set feature '" + << feature << "' in the monmap."; + err = -EPERM; + goto reply; + } + + if (!mon->get_quorum_mon_features().contains_all(feature)) { + ss << "current quorum does not support feature '" << feature + << "'; supported features: " + << mon->get_quorum_mon_features(); + err = -EINVAL; + goto reply; + } + + ss << "setting feature '" << feature << "'"; + + err = 0; + if (monmap.persistent_features.contains_all(feature)) { + dout(10) << __func__ << " feature '" << feature + << "' already set on monmap; no-op." << dendl; + goto reply; + } + + pending_map.persistent_features.set_feature(feature); + pending_map.last_changed = ceph_clock_now(); + propose = true; + + dout(1) << __func__ << ss.str() << "; new features will be: " + << "persistent = " << pending_map.persistent_features + // output optional nevertheless, for auditing purposes. + << ", optional = " << pending_map.optional_features << dendl; + } else { ss << "unknown command " << prefix; err = -EINVAL; From 11c4946b636740bf4b44c77bd5bcc81c43d462e3 Mon Sep 17 00:00:00 2001 From: Joao Eduardo Luis Date: Mon, 13 Feb 2017 00:36:31 +0000 Subject: [PATCH 3/5] mon: enable persistent monmap features on full quorum We will now only enable persistent features automatically when ALL the monitors in the monmap are in the quorum. #noMonitorLeftBehind Signed-off-by: Joao Eduardo Luis --- src/mon/MonmapMonitor.cc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/mon/MonmapMonitor.cc b/src/mon/MonmapMonitor.cc index 5e18dba7572..d71810b71bf 100644 --- a/src/mon/MonmapMonitor.cc +++ b/src/mon/MonmapMonitor.cc @@ -129,6 +129,9 @@ void MonmapMonitor::apply_mon_features(const mon_feature_t& features) assert(is_writeable()); assert(features.contains_all(pending_map.persistent_features)); + // we should never hit this because `features` should be the result + // of the quorum's supported features. But if it happens, die. + assert(ceph::features::mon::get_supported().contains_all(features)); mon_feature_t new_features = (pending_map.persistent_features ^ @@ -140,6 +143,16 @@ void MonmapMonitor::apply_mon_features(const mon_feature_t& features) return; } + if (mon->get_quorum().size() < mon->monmap->size()) { + dout(1) << __func__ << " new features " << new_features + << " contains features that require a full quorum" + << " (quorum size is " << mon->get_quorum().size() + << ", requires " << mon->monmap->size() << "): " + << new_features + << " -- do not enable them!" << dendl; + return; + } + new_features |= pending_map.persistent_features; dout(5) << __func__ << " applying new features to monmap;" From 3cabcb7d5176fa9f85344e611f992be092550a03 Mon Sep 17 00:00:00 2001 From: Joao Eduardo Luis Date: Tue, 21 Feb 2017 01:27:54 +0000 Subject: [PATCH 4/5] qa/workunits/ceph-helpers: add wait_for_quorum() Takes optional timeout and desired quorum size Signed-off-by: Joao Eduardo Luis --- qa/workunits/ceph-helpers.sh | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/qa/workunits/ceph-helpers.sh b/qa/workunits/ceph-helpers.sh index 049baf74a34..efaff9cbfbd 100755 --- a/qa/workunits/ceph-helpers.sh +++ b/qa/workunits/ceph-helpers.sh @@ -710,6 +710,42 @@ function test_get_osds() { ####################################################################### +## +# Wait for the monitor to form quorum (optionally, of size N) +# +# @param timeout duration (lower-bound) to wait for quorum to be formed +# @param quorumsize size of quorum to wait for +# @return 0 on success, 1 on error +# +function wait_for_quorum() { + local timeout=$1 + local quorumsize=$2 + + if [[ -z "$timeout" ]]; then + timeout=300 + fi + + if [[ -z "$quorumsize" ]]; then + timeout $timeout ceph mon_status --format=json >&/dev/null || return 1 + return 0 + fi + + no_quorum=1 + wait_until=$((`date +%s` + $timeout)) + while [[ $(date +%s) -lt $wait_until ]]; do + jqfilter='.quorum | length == '$quorumsize + jqinput="$(timeout $timeout ceph mon_status --format=json 2>/dev/null)" + res=$(echo $jqinput | jq "$jqfilter") + if [[ "$res" == "true" ]]; then + no_quorum=0 + break + fi + done + return $no_quorum +} + +####################################################################### + ## # Return the PG of supporting the **objectname** stored in # **poolname**, as reported by ceph osd map. From 23740119c642279868d6757bba2a518cd18bace3 Mon Sep 17 00:00:00 2001 From: Joao Eduardo Luis Date: Tue, 14 Feb 2017 16:35:01 +0000 Subject: [PATCH 5/5] mon: test 'mon feature' cli Signed-off-by: Joao Eduardo Luis --- qa/workunits/cephtool/test.sh | 6 ++ src/test/mon/misc.sh | 119 ++++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+) diff --git a/qa/workunits/cephtool/test.sh b/qa/workunits/cephtool/test.sh index 804eaafdcc8..b7fc9c85b6a 100755 --- a/qa/workunits/cephtool/test.sh +++ b/qa/workunits/cephtool/test.sh @@ -1045,6 +1045,12 @@ function test_mon_mon() [ -s $TEMP_DIR/monmap.$$ ] # ceph mon tell ceph mon_status + + # test mon features + ceph mon feature list + ceph mon feature set kraken --yes-i-really-mean-it + expect_false ceph mon feature set abcd + expect_false ceph mon feature set abcd --yes-i-really-mean-it } function test_mon_osd() diff --git a/src/test/mon/misc.sh b/src/test/mon/misc.sh index cc7d8f97611..10984e31344 100755 --- a/src/test/mon/misc.sh +++ b/src/test/mon/misc.sh @@ -150,6 +150,125 @@ function TEST_no_segfault_for_bad_keyring() { teardown $dir || return 1 } +function jq_success() { + input="$1" + filter="$2" + expects="\"$3\"" + + in_escaped=$(printf %s "$input" | sed "s/'/'\\\\''/g") + filter_escaped=$(printf %s "$filter" | sed "s/'/'\\\\''/g") + + ret=$(echo "$in_escaped" | jq "$filter_escaped") + if [[ "$ret" == "true" ]]; then + return 0 + elif [[ -n "$expects" ]]; then + if [[ "$ret" == "$expects" ]]; then + return 0 + fi + fi + return 1 + input=$1 + filter=$2 + expects="$3" + + ret="$(echo $input | jq \"$filter\")" + if [[ "$ret" == "true" ]]; then + return 0 + elif [[ -n "$expects" && "$ret" == "$expects" ]]; then + return 0 + fi + return 1 +} + +function TEST_mon_features() { + local dir=$1 + setup $dir || return 1 + + fsid=$(uuidgen) + MONA=127.0.0.1:7127 # git grep '\<7127\>' ; there must be only one + MONB=127.0.0.1:7128 # git grep '\<7128\>' ; there must be only one + MONC=127.0.0.1:7129 # git grep '\<7129\>' ; there must be only one + CEPH_ARGS_orig=$CEPH_ARGS + CEPH_ARGS="--fsid=$fsid --auth-supported=none " + CEPH_ARGS+="--mon-initial-members=a,b,c " + CEPH_ARGS+="--mon-host=$MONA,$MONB,$MONC " + + run_mon $dir a --public-addr $MONA || return 1 + run_mon $dir b --public-addr $MONB || return 1 + timeout 120 ceph -s > /dev/null || return 1 + + # NOTE: + # jq only support --exit-status|-e from version 1.4 forwards, which makes + # returning on error waaaay prettier and straightforward. + # However, the current automated upstream build is running with v1.3, + # which has no idea what -e is. Hence the convoluted error checking we + # need. Sad. + # The next time someone changes this code, please check if v1.4 is now + # a thing, and, if so, please change these to use -e. Thanks. + + # jq '.all.supported | select([.[] == "foo"] | any)' + + # expect monmap to contain 3 monitors (a, b, and c) + jqinput="$(ceph mon_status --format=json 2>/dev/null)" + jq_success "$jqinput" '.monmap.mons | length == 3' || return 1 + # quorum contains two monitors + jq_success "$jqinput" '.quorum | length == 2' || return 1 + # quorum's monitor features contain kraken and luminous + jqfilter='.features.quorum_mon[]|select(. == "kraken")' + jq_success "$jqinput" "$jqfilter" "kraken" || return 1 + jqfilter='.features.quorum_mon[]|select(. == "luminous")' + jq_success "$jqinput" "$jqfilter" "luminous" || return 1 + + # monmap must have no persistent features set, because we + # don't currently have a quorum made out of all the monitors + # in the monmap. + jqfilter='.monmap.features.persistent | length == 0' + jq_success "$jqinput" "$jqfilter" || return 1 + + # nor do we have any optional features, for that matter. + jqfilter='.monmap.features.optional | length == 0' + jq_success "$jqinput" "$jqfilter" || return 1 + + # validate 'mon feature list' + + jqinput="$(ceph mon feature list --format=json 2>/dev/null)" + # 'kraken' and 'luminous' are supported + jqfilter='.all.supported[] | select(. == "kraken")' + jq_success "$jqinput" "$jqfilter" "kraken" || return 1 + jqfilter='.all.supported[] | select(. == "luminous")' + jq_success "$jqinput" "$jqfilter" "luminous" || return 1 + + # start third monitor + run_mon $dir c --public-addr $MONC || return 1 + + wait_for_quorum 300 3 || return 1 + + timeout 300 ceph -s > /dev/null || return 1 + + jqinput="$(ceph mon_status --format=json 2>/dev/null)" + # expect quorum to have all three monitors + jqfilter='.quorum | length == 3' + jq_success "$jqinput" "$jqfilter" || return 1 + # quorum's monitor features contain kraken and luminous + jqfilter='.features.quorum_mon[]|select(. == "kraken")' + jq_success "$jqinput" "$jqfilter" "kraken" || return 1 + jqfilter='.features.quorum_mon[]|select(. == "luminous")' + jq_success "$jqinput" "$jqfilter" "luminous" || return 1 + + # monmap must have no both 'kraken' and 'luminous' persistent + # features set. + jqfilter='.monmap.features.persistent | length == 2' + jq_success "$jqinput" "$jqfilter" || return 1 + jqfilter='.monmap.features.persistent[]|select(. == "kraken")' + jq_success "$jqinput" "$jqfilter" "kraken" || return 1 + jqfilter='.monmap.features.persistent[]|select(. == "luminous")' + jq_success "$jqinput" "$jqfilter" "luminous" || return 1 + + CEPH_ARGS=$CEPH_ARGS_orig + # that's all folks. thank you for tuning in. + teardown $dir || return 1 +} + main misc "$@" # Local Variables: