From 4b1e82eaaa602dfa3e21b5adc0e9a7ae24359974 Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@newdream.net>
Date: Tue, 16 Nov 2021 17:48:27 -0500
Subject: [PATCH] mon/ConfigMap: push kv parsing logic into add_option()

Push shared code into ConfigMap from mon + mgr.  Pass in a function to
fetch the Option*, since in the mon case it may come from somewhere other
than g_conf.

Signed-off-by: Sage Weil <sage@newdream.net>
(cherry picked from commit d8ccd221549cdd28702932deb148f075cb6d88b3)

Fixed conflict: `mopt.localized_name = name;` is added to add_option()
(due to 3821722e5660437298a7c0f41e1061d363090103)
---
 src/mgr/ActivePyModules.cc | 41 +++--------------------
 src/mon/ConfigMap.cc       | 57 ++++++++++++++++++++++++++++++++
 src/mon/ConfigMap.h        |  7 ++++
 src/mon/ConfigMonitor.cc   | 68 +++++++++-----------------------------
 4 files changed, 84 insertions(+), 89 deletions(-)

diff --git a/src/mgr/ActivePyModules.cc b/src/mgr/ActivePyModules.cc
index 701b41bef8e..4f75670591a 100644
--- a/src/mgr/ActivePyModules.cc
+++ b/src/mgr/ActivePyModules.cc
@@ -866,42 +866,11 @@ void ActivePyModules::_refresh_config_map()
     string who;
     config_map.parse_key(key, &name, &who);
 
-    const Option *opt = g_conf().find_option(name);
-    if (!opt) {
-      config_map.stray_options.push_back(
-	std::unique_ptr<Option>(
-	  new Option(name, Option::TYPE_STR, Option::LEVEL_UNKNOWN)));
-      opt = config_map.stray_options.back().get();
-    }
-
-    string err;
-    int r = opt->pre_validate(&value, &err);
-    if (r < 0) {
-      dout(10) << __func__ << " pre-validate failed on '" << name << "' = '"
-	       << value << "' for " << name << dendl;
-    }
-
-    MaskedOption mopt(opt);
-    mopt.raw_value = value;
-    string section_name;
-    if (who.size() &&
-	!ConfigMap::parse_mask(who, &section_name, &mopt.mask)) {
-      derr << __func__ << " invalid mask for key " << key << dendl;
-    } else if (opt->has_flag(Option::FLAG_NO_MON_UPDATE)) {
-      dout(10) << __func__ << " NO_MON_UPDATE option '"
-	       << name << "' = '" << value << "' for " << name
-	       << dendl;
-    } else {
-      Section *section = &config_map.global;;
-      if (section_name.size() && section_name != "global") {
-	if (section_name.find('.') != std::string::npos) {
-	  section = &config_map.by_id[section_name];
-	} else {
-	  section = &config_map.by_type[section_name];
-	}
-      }
-      section->options.insert(make_pair(name, std::move(mopt)));
-    }
+    config_map.add_option(
+      g_ceph_context, name, who, value,
+      [&](const std::string& name) {
+	return  g_conf().find_option(name);
+      });
   }
 }
 
diff --git a/src/mon/ConfigMap.cc b/src/mon/ConfigMap.cc
index dff1a94c1e6..86528c1dedf 100644
--- a/src/mon/ConfigMap.cc
+++ b/src/mon/ConfigMap.cc
@@ -7,6 +7,10 @@
 #include "crush/CrushWrapper.h"
 #include "common/entity_name.h"
 
+#define dout_subsys ceph_subsys_mon
+#undef dout_prefix
+#include "common/dout.h"
+
 using namespace std::literals;
 
 using std::cerr;
@@ -250,6 +254,59 @@ void ConfigMap::parse_key(
   }
 }
 
+int ConfigMap::add_option(
+  CephContext *cct,
+  const std::string& name,
+  const std::string& who,
+  const std::string& orig_value,
+  std::function<const Option *(const std::string&)> get_opt)
+{
+  const Option *opt = get_opt(name);
+  if (!opt) {
+    ldout(cct, 10) << __func__ << " unrecognized option '" << name << "'" << dendl;
+    stray_options.push_back(
+      std::unique_ptr<Option>(
+	new Option(name, Option::TYPE_STR, Option::LEVEL_UNKNOWN)));
+    opt = stray_options.back().get();
+  }
+
+  string err;
+  string value = orig_value;
+  int r = opt->pre_validate(&value, &err);
+  if (r < 0) {
+    ldout(cct, 10) << __func__ << " pre-validate failed on '" << name << "' = '"
+		   << value << "' for " << name << dendl;
+  }
+
+  int ret = 0;
+  MaskedOption mopt(opt);
+  mopt.raw_value = value;
+  mopt.localized_name = name;
+  string section_name;
+  if (who.size() &&
+      !ConfigMap::parse_mask(who, &section_name, &mopt.mask)) {
+    lderr(cct) << __func__ << " invalid mask for option " << name << " mask " << who
+	       << dendl;
+    ret = -EINVAL;
+  } else if (opt->has_flag(Option::FLAG_NO_MON_UPDATE)) {
+    ldout(cct, 10) << __func__ << " NO_MON_UPDATE option '"
+		   << name << "' = '" << value << "' for " << name
+		   << dendl;
+    ret = -EINVAL;
+  } else {
+    Section *section = &global;;
+    if (section_name.size() && section_name != "global") {
+      if (section_name.find('.') != std::string::npos) {
+	section = &by_id[section_name];
+      } else {
+	section = &by_type[section_name];
+      }
+    }
+    section->options.insert(make_pair(name, std::move(mopt)));
+  }
+  return ret;
+}
+
 
 // --------------
 
diff --git a/src/mon/ConfigMap.h b/src/mon/ConfigMap.h
index bf2bd97c0ba..5a14d089a72 100644
--- a/src/mon/ConfigMap.h
+++ b/src/mon/ConfigMap.h
@@ -149,6 +149,13 @@ struct ConfigMap {
     const std::string& in,
     std::string *section,
     OptionMask *mask);
+
+  int add_option(
+    CephContext *cct,
+    const std::string& name,
+    const std::string& who,
+    const std::string& value,
+    std::function<const Option *(const std::string&)> get_opt);
 };
 
 
diff --git a/src/mon/ConfigMonitor.cc b/src/mon/ConfigMonitor.cc
index a5f61cbea52..6480674926d 100644
--- a/src/mon/ConfigMonitor.cc
+++ b/src/mon/ConfigMonitor.cc
@@ -785,9 +785,10 @@ void ConfigMonitor::load_config()
 
   unsigned num = 0;
   KeyValueDB::Iterator it = mon.store->get_iterator(KV_PREFIX);
-  it->lower_bound(KEY_PREFIX);
-  while (it->valid() &&
-	 it->key().compare(0, KEY_PREFIX.size(), KEY_PREFIX) == 0) {
+  for (it->lower_bound(KEY_PREFIX);
+       it->valid() &&
+	 it->key().compare(0, KEY_PREFIX.size(), KEY_PREFIX) == 0;
+       it->next(), ++num) {
     string key = it->key().substr(KEY_PREFIX.size());
     string value = it->value().to_str();
 
@@ -811,58 +812,19 @@ void ConfigMonitor::load_config()
       }
     }
 
-    const Option *opt = g_conf().find_option(name);
-    if (!opt) {
-      opt = mon.mgrmon()->find_module_option(name);
-    }
-    if (!opt) {
-      dout(10) << __func__ << " unrecognized option '" << name << "'" << dendl;
-      config_map.stray_options.push_back(
-	std::unique_ptr<Option>(
-	  new Option(name, Option::TYPE_STR, Option::LEVEL_UNKNOWN)));
-      opt = config_map.stray_options.back().get();
-    }
-
-    string err;
-    int r = opt->pre_validate(&value, &err);
-    if (r < 0) {
-      dout(10) << __func__ << " pre-validate failed on '" << name << "' = '"
-	       << value << "' for " << name << dendl;
-    }
-    
-    MaskedOption mopt(opt);
-    mopt.raw_value = value;
-    mopt.localized_name = name;
-    string section_name;
-    if (who.size() &&
-	!ConfigMap::parse_mask(who, &section_name, &mopt.mask)) {
-      derr << __func__ << " invalid mask for key " << key << dendl;
-      pending_cleanup[key].reset();
-    } else if (opt->has_flag(Option::FLAG_NO_MON_UPDATE)) {
-      dout(10) << __func__ << " NO_MON_UPDATE option '"
-	       << name << "' = '" << value << "' for " << name
-	       << dendl;
-      pending_cleanup[key].reset();
-    } else {
-      if (section_name.empty()) {
-	// we prefer global/$option instead of just $option
-	derr << __func__ << " adding global/ prefix to key '" << key << "'"
-	     << dendl;
-	pending_cleanup[key].reset();
-	pending_cleanup["global/"s + key] = it->value();
-      }
-      Section *section = &config_map.global;;
-      if (section_name.size() && section_name != "global") {
-	if (section_name.find('.') != std::string::npos) {
-	  section = &config_map.by_id[section_name];
-	} else {
-	  section = &config_map.by_type[section_name];
+    int r = config_map.add_option(
+      g_ceph_context, name, who, value,
+      [&](const std::string& name) {
+	const Option *opt = g_conf().find_option(name);
+	if (!opt) {
+	  opt = mon.mgrmon()->find_module_option(name);
 	}
-      }
-      section->options.insert(make_pair(name, std::move(mopt)));
-      ++num;
+	return opt;
+      });
+    if (r == -EINVAL) {
+      dout(10) << __func__ << " will clean up key " << key << dendl;
+      pending_cleanup[key].reset();
     }
-    it->next();
   }
   dout(10) << __func__ << " got " << num << " keys" << dendl;
 
-- 
2.39.5