mon: MonmapMonitor: make 'ceph mon add' idempotent

MonMap changes lead to bootstraps.  Callbacks waiting for a proposal to
finish can have several fates, depending on what happens: finished, rerun
or aborted.

In the case of a bootstrap right after a monmap change, callbacks are
rerun.  Considering we queued the message that lead to the monmap change
on this queue, if we instead of finishing it end up reruning it, we will
end up trying to perform the same modification twice -- the last one will
try to modify an already existing state and we will return just that:
whatever you're attempting to do has already been done.

This patch makes 'ceph mon add' completely idempotent.  If one tries to
add an already existing monitor (i.e., same name, same ip:port), one
simply gets a 'monitor foo added', with return 0, no matter how many
times one runs the command.

Fixes: #5896
Signed-off-by: Joao Eduardo Luis <joao.luis@inktank.com>

diff --git a/src/mon/MonmapMonitor.cc b/src/mon/MonmapMonitor.cc
index 799f19df154522bca8a645c33c20ca37b0a97b3b..ca855592445f1aab66da50391b8f8cbcf652bc2d 100644 (file)
--- a/src/mon/MonmapMonitor.cc
+++ b/src/mon/MonmapMonitor.cc
@@ -298,20 +298,45 @@ bool MonmapMonitor::prepare_command(MMonCommand *m)
       addr.set_port(CEPH_MON_PORT);
     }
 
-    if (pending_map.contains(addr) ||
-       pending_map.contains(name)) {
+    /**
+     * If we have a monitor with the same name and different addr, then EEXIST
+     * If we have a monitor with the same addr and different name, then EEXIST
+     * If we have a monitor with the same addr and same name, then return as if
+     * we had just added the monitor.
+     * If we don't have the monitor, add it.
+     */
+
+    err = 0;
+    if (!ss.str().empty())
+      ss << "; ";
+
+    do {
+      if (pending_map.contains(addr)) {
+        string n = pending_map.get_name(addr);
+        if (n == name)
+          break;
+      } else if (pending_map.contains(name)) {
+        entity_addr_t tmp_addr = pending_map.get_addr(name);
+        if (tmp_addr == addr)
+          break;
+      } else {
+        break;
+      }
       err = -EEXIST;
-      if (!ss.str().empty())
-       ss << "; ";
-      ss << "mon " << name << " " << addr << " already exists";
+      ss << "mon." << name << " at " << addr << " already exists";
+      goto out;
+    } while (false);
+
+    ss << "added mon." << name << " at " << addr;
+    if (pending_map.contains(name)) {
       goto out;
     }
 
     pending_map.add(name, addr);
     pending_map.last_changed = ceph_clock_now(g_ceph_context);
-    ss << "added mon." << name << " at " << addr;
     getline(ss, rs);
-    wait_for_finished_proposal(new Monitor::C_Command(mon, m, 0, rs, get_last_committed()));
+    wait_for_finished_proposal(new Monitor::C_Command(mon, m, 0, rs,
+                                                      get_last_committed()));
     return true;
 
   } else if (prefix == "mon remove") {