From: Jason Dillaman <dillaman@redhat.com>
Date: Thu, 30 Jul 2020 17:00:22 +0000 (-0400)
Subject: mon/MonClient: apply MON config before finishing bootstrap
X-Git-Tag: v16.1.0~1354^2~5
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=313aeaa55d67089e0a2e173718899e603e51ce2e;p=ceph.git

mon/MonClient: apply MON config before finishing bootstrap

In general, non-primitive config options cannot be updated after
the service threads have been marked as running. In librados, this
happens after the MonClient bootstrap process. However, the configs
received from the MON were applied in a separate thread which can
race with librados marking the service threads as running.

Now the received MON configs will be applied within the context of
the bootstrap function call instead of deferring to a separate
thread.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc
index de51bc70416c..8213622636f0 100644
--- a/src/mon/MonClient.cc
+++ b/src/mon/MonClient.cc
@@ -141,6 +141,14 @@ int MonClient::get_monmap_and_config()
     }
   });
 
+  want_bootstrap_config = true;
+  auto shutdown_config = make_scope_guard([this] {
+    std::unique_lock l(monc_lock);
+    want_bootstrap_config = false;
+    bootstrap_config.reset();
+  });
+
+  ceph::ref_t<MConfig> config;
   while (tries-- > 0) {
     r = init();
     if (r < 0) {
@@ -164,13 +172,15 @@ int MonClient::get_monmap_and_config()
 	r = 0;
 	break;
       }
-      while ((!got_config || monmap.get_epoch() == 0) && r == 0) {
+      while ((!bootstrap_config || monmap.get_epoch() == 0) && r == 0) {
 	ldout(cct,20) << __func__ << " waiting for monmap|config" << dendl;
 	map_cond.wait_for(l, ceph::make_timespan(
           cct->_conf->mon_client_hunt_interval));
       }
-      if (got_config) {
+
+      if (bootstrap_config) {
 	ldout(cct,10) << __func__ << " success" << dendl;
+	config = std::move(bootstrap_config);
 	r = 0;
 	break;
       }
@@ -180,6 +190,12 @@ int MonClient::get_monmap_and_config()
     continue;
   }
 
+  if (config) {
+    // apply the bootstrap config to ensure its applied prior to completing
+    // the bootstrap
+    cct->_conf.set_mon_vals(cct, config->config, config_cb);
+  }
+
   shutdown();
   return r;
 }
@@ -436,6 +452,15 @@ void MonClient::handle_monmap(MMonMap *m)
 void MonClient::handle_config(MConfig *m)
 {
   ldout(cct,10) << __func__ << " " << *m << dendl;
+
+  if (want_bootstrap_config) {
+    // get_monmap_and_config is waiting for config which it will apply
+    // synchronously
+    bootstrap_config = ceph::ref_t<MConfig>(m, false);
+    map_cond.notify_all();
+    return;
+  }
+
   // Take the sledgehammer approach to ensuring we don't depend on
   // anything in MonClient.
   boost::asio::defer(finish_strand,
@@ -448,8 +473,6 @@ void MonClient::handle_config(MConfig *m)
 		      }
 		      m->put();
 		    });
-  got_config = true;
-  map_cond.notify_all();
 }
 
 // ----------------------
diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h
index 4683b0005d80..e079213c23b9 100644
--- a/src/mon/MonClient.h
+++ b/src/mon/MonClient.h
@@ -326,7 +326,9 @@ private:
   bool want_monmap;
   ceph::condition_variable map_cond;
   bool passthrough_monmap = false;
-  bool got_config = false;
+
+  bool want_bootstrap_config = false;
+  ceph::ref_t<MConfig> bootstrap_config;
 
   // authenticate
   std::unique_ptr<AuthClientHandler> auth;