cli: do not parse injectargs arguments twice

The arguments of injectargs being valid ceph arguments, they are.
consumed when the ceph cli calls rados.conf_parse_argv(). It can be
worked around by obsuring them as in:

   ceph tell osd.0 injectargs '--osd_debug_drop_ping_probability 444'

where '--osd_debug_drop_ping_probability 444' is a single argument that
does not match any known argument. The trick is that it will be
evaluated again once it reaches the OSD or the MON and translated into
the expected list of arguments. Although it is clear once explained, it
is obscure and leads to strange combinations such as:

   ceph tell osd.0 injectargs '--osd_debug_op_order '

(note the extra space at the end) to set boolean parameters. A better
workaround is to add a -- marking the end of the options as in:

   ceph tell osd.0 -- injectargs --osd_debug_op_order

this one is unfortunately much less documented and the user does not
usually know the exact semantic of --, let alone where it should be
placed.

The simpler solution is to split the argument list in two if
"injectargs" is found. The arguments that show after the "injectargs"
argument is removed from the list of arguments until parsing is
complete. It implements the more intuitive syntax:

   ceph tell osd.0 injectargs --osd_debug_op_order

and the other forms are still valid for backward compatibility.

http://tracker.ceph.com/issues/9372 Fixes: #9372

Signed-off-by: Loic Dachary <loic-201408@dachary.org>

diff --git a/qa/workunits/cephtool/test.sh b/qa/workunits/cephtool/test.sh
index bc0b23a4597b5a8f51c2bd39e75298a22bfb15eb..9da3603d764d50bdda47746993059ccb550234b3 100755 (executable)
--- a/qa/workunits/cephtool/test.sh
+++ b/qa/workunits/cephtool/test.sh
@@ -155,6 +155,18 @@ function expect_config_value()
   fi
 }
 
+function test_mon_injectargs()
+{
+  ceph tell osd.0 injectargs '--osd_debug_op_order --osd_debug_drop_ping_probability 444' >& $TMPFILE || return 1
+  check_response "osd_debug_drop_ping_probability = '444' osd_debug_op_order = 'true'"
+  ceph tell osd.0 injectargs --no-osd_debug_op_order >& $TMPFILE || return 1
+  check_response "osd_debug_op_order = 'false'"
+  ceph tell osd.0 injectargs -- --osd_debug_op_order >& $TMPFILE || return 1
+  check_response "osd_debug_op_order = 'true'"
+  ceph tell osd.0 injectargs -- '--osd_debug_op_order --osd_debug_drop_ping_probability 555' >& $TMPFILE || return 1
+  check_response "osd_debug_drop_ping_probability = '555' osd_debug_op_order = 'true'" 
+}
+
 function test_mon_injectargs_SI()
 {
   # Test SI units during injectargs and 'config set'
@@ -179,7 +191,10 @@ function test_mon_injectargs_SI()
   expect_config_value "mon.a" "mon_pg_warn_min_objects" 10240
   ceph tell mon.a injectargs '--mon_pg_warn_min_objects 1G'
   expect_config_value "mon.a" "mon_pg_warn_min_objects" 1073741824
-  expect_false ceph injectargs mon.a '--mon_pg_warn_min_objects 10F'
+  # < /dev/null accounts for the fact that ceph will go in interactive mode
+  # because injectargs is discarded (actually saved for the benefit of 
+  # a tell command that never comes)
+  expect_false ceph injectargs mon.a '--mon_pg_warn_min_objects 10F' < /dev/null 2> /dev/null
   $SUDO ceph daemon mon.a config set mon_pg_warn_min_objects $initial_value
 }
 
@@ -1264,6 +1279,7 @@ function test_osd_bench()
 
 set +x
 TESTS=(
+  mon_injectargs
   mon_injectargs_SI
   tiering
   auth

diff --git a/src/ceph.in b/src/ceph.in
index e157c80aec1b93d2b37c1b877e39675313f07efe..ebcfb2edce107626650e1922a3163a2f41cc1a11 100755 (executable)
--- a/src/ceph.in
+++ b/src/ceph.in
@@ -612,6 +612,16 @@ def main():
         'log_flush_on_exit':'true',
     }
 
+    if 'injectargs' in childargs:
+        position = childargs.index('injectargs')
+        injectargs = childargs[position:]
+        childargs = childargs[:position]
+        if verbose:
+            print >> sys.stderr, 'Separate childargs {0} from injectargs {1}'.\
+                format(childargs, injectargs)
+    else:
+        injectargs = None
+
     clustername = 'ceph'
     if parsed_args.cluster:
         clustername = parsed_args.cluster
@@ -626,7 +636,6 @@ def main():
             format(e.__class__.__name__)
         return 1
 
-    #tmp = childargs
     childargs = retargs
     if not childargs:
         childargs = []
@@ -745,10 +754,13 @@ def main():
         childargs = childargs[2:]
         is_tell = True
 
-    if is_tell and not len(childargs):
-        print >> sys.stderr, \
+    if is_tell:
+        if injectargs:
+            childargs = injectargs
+        if not len(childargs):
+            print >> sys.stderr, \
                 'Cannot use \'tell\' with interactive mode'
-        return errno.EINVAL
+            return errno.EINVAL
 
     # fetch JSON sigs from command
     # each line contains one command signature (a placeholder name