mgr/PyModule: correctly remove config options

Previously, incorrect parameters were being passed to "config rm",
causing it to do nothing.  This commit also ensures the correct
error message is shown for both the set and remove failure cases.
I've also moved the update of the in-memory config map to *after*
the value is persisted, to ensure the config map actually reflects
what's stored.

Fixes: https://tracker.ceph.com/issues/42958
Signed-off-by: Tim Serong <tserong@suse.com>
(cherry picked from commit 0520ff571cfb480872c8bd429c94bd4ce15eeb40)

diff --git a/src/mgr/PyModule.cc b/src/mgr/PyModule.cc
index 20a1d4cae5abebb44fa57d72eac1b6e74dbf2d1e..8c8859ae5a0c96217836d0ece6d5c76571fdd0f3 100644 (file)
--- a/src/mgr/PyModule.cc
+++ b/src/mgr/PyModule.cc
@@ -147,16 +147,6 @@ void PyModuleConfig::set_config(
 {
   const std::string global_key = PyModule::config_prefix
                                    + module_name + "/" + key;
-  {
-    std::lock_guard l(lock);
-
-    if (val) {
-      config[global_key] = *val;
-    } else {
-      config.erase(global_key);
-    }
-  }
-
   Command set_cmd;
   {
     std::ostringstream cmd_json;
@@ -164,23 +154,33 @@ void PyModuleConfig::set_config(
     jf.open_object_section("cmd");
     if (val) {
       jf.dump_string("prefix", "config set");
-      jf.dump_string("who", "mgr");
-      jf.dump_string("name", global_key);
       jf.dump_string("value", *val);
     } else {
       jf.dump_string("prefix", "config rm");
-      jf.dump_string("name", "mgr");
-      jf.dump_string("key", global_key);
     }
+    jf.dump_string("who", "mgr");
+    jf.dump_string("name", global_key);
     jf.close_section();
     jf.flush(cmd_json);
     set_cmd.run(monc, cmd_json.str());
   }
   set_cmd.wait();
 
-  if (set_cmd.r != 0) {
-    dout(0) << "`config set mgr" << global_key << " " << val << "` failed: "
-      << cpp_strerror(set_cmd.r) << dendl;
+  if (set_cmd.r == 0) {
+    std::lock_guard l(lock);
+    if (val) {
+      config[global_key] = *val;
+    } else {
+      config.erase(global_key);
+    }
+  } else {
+    if (val) {
+      dout(0) << "`config set mgr " << global_key << " " << val << "` failed: "
+        << cpp_strerror(set_cmd.r) << dendl;
+    } else {
+      dout(0) << "`config rm mgr " << global_key << "` failed: "
+        << cpp_strerror(set_cmd.r) << dendl;
+    }
     dout(0) << "mon returned " << set_cmd.r << ": " << set_cmd.outs << dendl;
   }
 }