ceph_argparse: introduce CephBool arguments

This replaces use of CephChoices for cases like
--yes-i-really-really-mean-it.

It's a relatively invasive change, because both
client and server need to handle backward compatibility:
 - Clients are easy, they just cope with the server's
   use of CephChoices/CephString with a few special case
   strings to recognise --yes-i-really-mean-it and similar
 - Servers are harder, they have to convert CephBool arguments
   into a similar CephChoices argument that older clients will
   understand.  This involves propagating feature flags into
   some code paths that couldn't see them before.

Signed-off-by: John Spray <john.spray@redhat.com>
This commit is contained in:
John Spray 2018-10-31 12:59:20 -04:00
parent 6c1e4b825c
commit 525623b6cd
13 changed files with 193 additions and 34 deletions

View File

@ -566,6 +566,7 @@ public:
ostringstream secname;
secname << "cmd" << setfill('0') << std::setw(3) << cmdnum;
dump_cmd_and_help_to_json(&jf,
CEPH_FEATURES_ALL,
secname.str().c_str(),
info.desc,
info.help);

View File

@ -72,7 +72,7 @@ arg_desc_t cmddesc_get_args(const String& cmddesc)
*/
void
dump_cmd_to_json(Formatter *f, const string& cmd)
dump_cmd_to_json(Formatter *f, uint64_t features, const string& cmd)
{
// put whole command signature in an already-opened container
// elements are: "name", meaning "the typeless name that means a literal"
@ -91,6 +91,23 @@ dump_cmd_to_json(Formatter *f, const string& cmd)
auto desckv = cmddesc_get_args(word);
// name the individual desc object based on the name key
f->open_object_section(string(desckv["name"]).c_str());
// Compatibility for pre-nautilus clients that don't know about CephBool
if (!HAVE_FEATURE(features, SERVER_NAUTILUS)) {
auto i = desckv.find("type");
if (i != desckv.end() && i->second == "CephBool") {
// Instruct legacy clients to send --foo-bar string in place
// of a 'true'/'false' value
std::ostringstream oss;
oss << std::string("--") << desckv["name"];
std::string val = oss.str();
std::replace(val.begin(), val.end(), '_', '-');
desckv["type"] = "CephChoices";
desckv["strings"] = val;
}
}
// dump all the keys including name into the array
for (auto [key, value] : desckv) {
f->dump_string(string(key).c_str(), string(value));
@ -101,13 +118,14 @@ dump_cmd_to_json(Formatter *f, const string& cmd)
void
dump_cmd_and_help_to_json(Formatter *jf,
uint64_t features,
const string& secname,
const string& cmdsig,
const string& helptext)
{
jf->open_object_section(secname.c_str());
jf->open_array_section("sig");
dump_cmd_to_json(jf, cmdsig);
dump_cmd_to_json(jf, features, cmdsig);
jf->close_section(); // sig array
jf->dump_string("help", helptext.c_str());
jf->close_section(); // cmd
@ -115,6 +133,7 @@ dump_cmd_and_help_to_json(Formatter *jf,
void
dump_cmddesc_to_json(Formatter *jf,
uint64_t features,
const string& secname,
const string& cmdsig,
const string& helptext,
@ -124,7 +143,7 @@ dump_cmddesc_to_json(Formatter *jf,
{
jf->open_object_section(secname.c_str());
jf->open_array_section("sig");
dump_cmd_to_json(jf, cmdsig);
dump_cmd_to_json(jf, features, cmdsig);
jf->close_section(); // sig array
jf->dump_string("help", helptext.c_str());
jf->dump_string("module", module.c_str());
@ -560,3 +579,37 @@ bool validate_cmd(CephContext* cct,
}
});
}
bool cmd_getval(CephContext *cct, const cmdmap_t& cmdmap,
const std::string& k, bool& val)
{
/*
* Specialized getval for booleans. CephBool didn't exist before Nautilus,
* so earlier clients are sent a CephChoices argdesc instead, and will
* send us a "--foo-bar" value string for boolean arguments.
*/
if (cmdmap.count(k)) {
try {
val = boost::get<bool>(cmdmap.find(k)->second);
return true;
} catch (boost::bad_get&) {
try {
std::string expected = "--" + k;
std::replace(expected.begin(), expected.end(), '_', '-');
std::string v_str = boost::get<std::string>(cmdmap.find(k)->second);
if (v_str == expected) {
val = true;
return true;
} else {
throw bad_cmd_get(k, cmdmap);
}
} catch (boost::bad_get&) {
throw bad_cmd_get(k, cmdmap);
}
}
}
return false;
}

View File

@ -23,12 +23,15 @@ typedef boost::variant<std::string,
typedef std::map<std::string, cmd_vartype, std::less<>> cmdmap_t;
std::string cmddesc_get_prefix(const std::string &cmddesc);
void dump_cmd_to_json(ceph::Formatter *f, const std::string& cmd);
void dump_cmd_to_json(ceph::Formatter *f, uint64_t features,
const std::string& cmd);
void dump_cmd_and_help_to_json(ceph::Formatter *f,
uint64_t features,
const std::string& secname,
const std::string& cmd,
const std::string& helptext);
void dump_cmddesc_to_json(ceph::Formatter *jf,
uint64_t features,
const std::string& secname,
const std::string& cmdsig,
const std::string& helptext,
@ -52,6 +55,9 @@ struct bad_cmd_get : public std::exception {
}
};
bool cmd_getval(CephContext *cct, const cmdmap_t& cmdmap,
const std::string& k, bool& val);
template <typename T>
bool cmd_getval(CephContext *cct, const cmdmap_t& cmdmap,
const std::string& k, T& val)

View File

@ -727,7 +727,8 @@ int MDSDaemon::_handle_command(
for (auto& c : get_commands()) {
ostringstream secname;
secname << "cmd" << setfill('0') << std::setw(3) << cmdnum;
dump_cmddesc_to_json(f.get(), secname.str(), c.cmdstring, c.helpstring,
dump_cmddesc_to_json(f.get(), m->get_connection()->get_features(),
secname.str(), c.cmdstring, c.helpstring,
c.module, "*", 0);
cmdnum++;
}

View File

@ -830,10 +830,11 @@ bool DaemonServer::_handle_command(
JSONFormatter f;
f.open_object_section("command_descriptions");
auto dump_cmd = [&cmdnum, &f](const MonCommand &mc){
auto dump_cmd = [&cmdnum, &f, m](const MonCommand &mc){
ostringstream secname;
secname << "cmd" << setfill('0') << std::setw(3) << cmdnum;
dump_cmddesc_to_json(&f, secname.str(), mc.cmdstring, mc.helpstring,
dump_cmddesc_to_json(&f, m->get_connection()->get_features(),
secname.str(), mc.cmdstring, mc.helpstring,
mc.module, mc.req_perms, 0);
cmdnum++;
};

View File

@ -738,7 +738,8 @@ COMMAND("osd pause", "pause osd", "osd", "rw")
COMMAND("osd unpause", "unpause osd", "osd", "rw")
COMMAND("osd erasure-code-profile set " \
"name=name,type=CephString,goodchars=[A-Za-z0-9-_.] " \
"name=profile,type=CephString,n=N,req=false", \
"name=profile,type=CephString,n=N,req=false " \
"name=force,type=CephBool,req=false", \
"create erasure code profile <name> with [<key[=value]> ...] pairs. Add a --force at the end to override an existing profile (VERY DANGEROUS)", \
"osd", "rw")
COMMAND("osd erasure-code-profile get " \
@ -942,16 +943,16 @@ COMMAND("osd pool create " \
COMMAND_WITH_FLAG("osd pool delete " \
"name=pool,type=CephPoolname " \
"name=pool2,type=CephPoolname,req=false " \
"name=sure,type=CephChoices,strings=--yes-i-really-really-mean-it|" \
"--yes-i-really-really-mean-it-not-faking,req=false", \
"name=yes_i_really_really_mean_it,type=CephBool,req=false "
"name=yes_i_really_really_mean_it_not_faking,type=CephBool,req=false ", \
"delete pool", \
"osd", "rw", \
FLAG(DEPRECATED))
COMMAND("osd pool rm " \
"name=pool,type=CephPoolname " \
"name=pool2,type=CephPoolname,req=false " \
"name=sure,type=CephChoices,strings=--yes-i-really-really-mean-it|" \
"--yes-i-really-really-mean-it-not-faking,req=false", \
"name=yes_i_really_really_mean_it,type=CephBool,req=false "
"name=yes_i_really_really_mean_it_not_faking,type=CephBool,req=false ", \
"remove pool", \
"osd", "rw")
COMMAND("osd pool rename " \

View File

@ -2869,6 +2869,7 @@ bool Monitor::_allowed_command(MonSession *s, const string &module,
void Monitor::format_command_descriptions(const std::vector<MonCommand> &commands,
Formatter *f,
uint64_t features,
bufferlist *rdata)
{
int cmdnum = 0;
@ -2877,7 +2878,7 @@ void Monitor::format_command_descriptions(const std::vector<MonCommand> &command
unsigned flags = cmd.flags;
ostringstream secname;
secname << "cmd" << setfill('0') << std::setw(3) << cmdnum;
dump_cmddesc_to_json(f, secname.str(),
dump_cmddesc_to_json(f, features, secname.str(),
cmd.cmdstring, cmd.helpstring, cmd.module,
cmd.req_perms, flags);
cmdnum++;
@ -2979,7 +2980,8 @@ void Monitor::handle_command(MonOpRequestRef op)
}
}
format_command_descriptions(commands, f, &rdata);
auto features = m->get_connection()->get_features();
format_command_descriptions(commands, f, features, &rdata);
delete f;
reply_command(op, 0, "", rdata, 0);
return;

View File

@ -963,6 +963,7 @@ private:
public:
static void format_command_descriptions(const std::vector<MonCommand> &commands,
Formatter *f,
uint64_t features,
bufferlist *rdata);
const std::vector<MonCommand> &get_local_commands(mon_feature_t f) {

View File

@ -9589,13 +9589,10 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
cmd_getval(cct, cmdmap, "name", name);
vector<string> profile;
cmd_getval(cct, cmdmap, "profile", profile);
bool force;
if (profile.size() > 0 && profile.back() == "--force") {
profile.pop_back();
force = true;
} else {
force = false;
}
bool force = false;
cmd_getval(cct, cmdmap, "force", force);
map<string,string> profile_map;
err = parse_erasure_code_profile(profile, &profile_map, &ss);
if (err)
@ -11591,7 +11588,6 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
string poolstr, poolstr2, sure;
cmd_getval(cct, cmdmap, "pool", poolstr);
cmd_getval(cct, cmdmap, "pool2", poolstr2);
cmd_getval(cct, cmdmap, "sure", sure);
int64_t pool = osdmap.lookup_pg_pool_name(poolstr.c_str());
if (pool < 0) {
ss << "pool '" << poolstr << "' does not exist";
@ -11599,9 +11595,12 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
goto reply;
}
bool force_no_fake = sure == "--yes-i-really-really-mean-it-not-faking";
bool force_no_fake = false;
cmd_getval(cct, cmdmap, "yes_i_really_really_mean_it", force_no_fake);
bool force = false;
cmd_getval(cct, cmdmap, "yes_i_really_really_mean_it_not_faking", force);
if (poolstr2 != poolstr ||
(sure != "--yes-i-really-really-mean-it" && !force_no_fake)) {
(!force && !force_no_fake)) {
ss << "WARNING: this will *PERMANENTLY DESTROY* all data stored in pool " << poolstr
<< ". If you are *ABSOLUTELY CERTAIN* that is what you want, pass the pool name *twice*, "
<< "followed by --yes-i-really-really-mean-it.";

View File

@ -6092,7 +6092,8 @@ int OSD::_do_command(
ostringstream secname;
secname << "cmd" << setfill('0') << std::setw(3) << cmdnum;
dump_cmddesc_to_json(f, secname.str(), cp->cmdstring, cp->helpstring,
dump_cmddesc_to_json(f, con->get_features(),
secname.str(), cp->cmdstring, cp->helpstring,
cp->module, cp->perm, 0);
cmdnum++;
}

View File

@ -499,6 +499,27 @@ class CephChoices(CephArgtype):
return all_elems
class CephBool(CephArgtype):
"""
A boolean argument, values may be case insensitive 'true', 'false', '0',
'1'. In keyword form, value may be left off (implies true).
"""
def __init__(self, strings='', **kwargs):
self.strings = strings.split('|')
def valid(self, s, partial=False):
lower_case = s.lower()
if lower_case in ['true', '1']:
self.val = True
elif lower_case in ['false', '0']:
self.val = False
else:
raise ArgumentValid("{0} not one of 'true', 'false'".format(s))
def __str__(self):
return '<bool>'
class CephFilepath(CephArgtype):
"""
Openable file
@ -681,6 +702,8 @@ class argdesc(object):
"""
if self.t == CephString:
chunk = '<{0}>'.format(self.name)
elif self.t == CephBool:
chunk = "--{0}".format(self.name.replace("_", "-"))
else:
chunk = str(self.instance)
s = chunk
@ -943,7 +966,6 @@ def validate(args, signature, flags=0, partial=False):
# A keyword argument?
if myarg:
# argdesc for the keyword argument, if we find one
kwarg_desc = None
@ -956,15 +978,31 @@ def validate(args, signature, flags=0, partial=False):
if kwarg_match:
# We have a "--foo=bar" style argument
kwarg_k, kwarg_v = kwarg_match.groups()
# Either "--foo-bar" or "--foo_bar" style is accepted
kwarg_k = kwarg_k.replace('-', '_')
kwarg_desc = arg_descs_by_name.get(kwarg_k, None)
elif len(myargs): # Some trailing arguments exist
# Maybe this is a "--foo bar" style argument
else:
# Maybe this is a "--foo bar" or "--bool" style argument
key_match = re.match(KWARG_SPACE, myarg)
if key_match:
kwarg_k = key_match.group(1)
# Permit --foo-bar=123 form or --foo_bar=123 form,
# assuming all command definitions use foo_bar argument
# naming style
kwarg_k = kwarg_k.replace('-', '_')
kwarg_desc = arg_descs_by_name.get(kwarg_k, None)
if kwarg_desc:
kwarg_v = myargs.pop(0)
if kwarg_desc.t == CephBool:
kwarg_v = 'true'
elif len(myargs): # Some trailing arguments exist
kwarg_v = myargs.pop(0)
else:
# Forget it, this is not a valid kwarg
kwarg_desc = None
if kwarg_desc:
validate_one(kwarg_v, kwarg_desc)
@ -976,18 +1014,16 @@ def validate(args, signature, flags=0, partial=False):
# "--yes-i-really-mean-it")
if myarg and myarg.startswith("--"):
# Special cases for instances of confirmation flags
# that were defined as CephString instead of CephChoices
# that were defined as CephString/CephChoices instead of CephBool
# in pre-nautilus versions of Ceph daemons.
is_value = desc.t == CephChoices \
or myarg == "--yes-i-really-mean-it" \
or myarg == "--yes-i-really-really-mean-it" \
or myarg == "--yes-i-really-really-mean-it-not-faking" \
or myarg == "--force" \
or injectargs
if not is_value:
sys.stderr.write("desc={0} t={1}".format(
desc, desc.t))
# Didn't get caught by kwarg handling, but has a "--", so
# we must assume it's something invalid, to avoid naively
# passing through mis-typed options as the values of

View File

@ -45,7 +45,8 @@ static void json_print(const std::vector<MonCommand> &mon_commands)
{
bufferlist rdata;
Formatter *f = Formatter::create("json");
Monitor::format_command_descriptions(mon_commands, f, &rdata);
Monitor::format_command_descriptions(mon_commands, f,
CEPH_FEATURES_ALL, &rdata);
delete f;
string data(rdata.c_str(), rdata.length());
cout << data << std::endl;

View File

@ -999,6 +999,62 @@ class TestOSD(TestArgparse):
assert_equal({}, validate_command(sigdict,
['osd', 'pool', 'create', "foo", "8", "--foo=bar"]))
def test_foo(self):
# Long form of a boolean argument (--foo=true)
assert_equal(
{
"prefix": "osd pool delete",
"pool": "foo",
"pool2": "foo",
"yes_i_really_really_mean_it": True
}, validate_command(sigdict, [
'osd', 'pool', 'delete', "foo", "foo",
"--yes-i-really-really-mean-it=true"]))
def test_pool_bool_args(self):
"""
Use pool deletion to exercise boolean arguments since it has
the --yes-i-really-really-mean-it flags
"""
# Short form of a boolean argument (--foo)
assert_equal(
{
"prefix": "osd pool delete",
"pool": "foo",
"pool2": "foo",
"yes_i_really_really_mean_it": True
}, validate_command(sigdict, [
'osd', 'pool', 'delete', "foo", "foo",
"--yes-i-really-really-mean-it"]))
# Long form of a boolean argument (--foo=true)
assert_equal(
{
"prefix": "osd pool delete",
"pool": "foo",
"pool2": "foo",
"yes_i_really_really_mean_it": True
}, validate_command(sigdict, [
'osd', 'pool', 'delete', "foo", "foo",
"--yes-i-really-really-mean-it=true"]))
# Negative form of a boolean argument (--foo=false)
assert_equal(
{
"prefix": "osd pool delete",
"pool": "foo",
"pool2": "foo",
"yes_i_really_really_mean_it": False
}, validate_command(sigdict, [
'osd', 'pool', 'delete', "foo", "foo",
"--yes-i-really-really-mean-it=false"]))
# Invalid value boolean argument (--foo=somethingelse)
assert_equal({}, validate_command(sigdict, [
'osd', 'pool', 'delete', "foo", "foo",
"--yes-i-really-really-mean-it=rhubarb"]))
def test_pool_create(self):
self.assert_valid_command(['osd', 'pool', 'create',
'poolname', '128'])