From: Rishabh Dave <ridave@redhat.com>
Date: Fri, 23 Aug 2024 12:49:43 +0000 (+0530)
Subject: mon,cephfs: require confirmation when changing max_mds on unhealthy cluster
X-Git-Tag: testing/wip-jcollin-testing-20251001.014437-squid^2~2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=fe45c051dc7ff829d874ff0f0c1832122cff5e40;p=ceph-ci.git

mon,cephfs: require confirmation when changing max_mds on unhealthy cluster

User must pass the confirmation flag (--yes-i-really-mean-it) to change
the value of CephFS setting variable "max_mds" when the Ceph cluster is
unhealthy.

This measure was decided upon to prevent users from changing "max_mds"
as a measure of troubleshotoing unhealthy cluster.

Fixes: https://tracker.ceph.com/issues/66301
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit a55a75c57e7a42a1317e4d7fc86c1964b71137f0)

Conflicts:

src/mon/FSCommands.cc
- Method set_val() in present in this file in main branch but it's
  absent in Squid branch.
---

diff --git a/src/mon/FSCommands.cc b/src/mon/FSCommands.cc
index e9774230997..63abe2973e5 100644
--- a/src/mon/FSCommands.cc
+++ b/src/mon/FSCommands.cc
@@ -336,12 +336,25 @@ public:
       ss << "Invalid variable";
       return -EINVAL;
     }
+
     string val;
-    string interr;
-    int64_t n = 0;
     if (!cmd_getval(cmdmap, "val", val)) {
       return -EINVAL;
     }
+
+    bool confirm = false;
+    cmd_getval(cmdmap, "yes_i_really_mean_it", confirm);
+    if (var == "max_mds" && !confirm && mon->mdsmon()->has_any_health_warning()) {
+      ss << "One or more file system health warnings are present. Modifying "
+	 << "the file system setting variable \"max_mds\" may not help "
+	 << "troubleshoot or recover from these warnings and may further "
+	 << "destabilize the system. If you really wish to proceed, run "
+	 << "again with --yes-i-really-mean-it";
+      return -EPERM;
+    }
+
+    string interr;
+    int64_t n = 0;
     // we got a string.  see if it contains an int.
     n = strict_strtoll(val.c_str(), 10, &interr);
     if (var == "max_mds") {
diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc
index 76a57ac443d..d8cca4ceb61 100644
--- a/src/mon/MDSMonitor.cc
+++ b/src/mon/MDSMonitor.cc
@@ -1557,6 +1557,13 @@ bool MDSMonitor::has_health_warnings(vector<mds_metric_t> warnings)
   return false;
 }
 
+bool MDSMonitor::has_any_health_warning()
+{
+  return std::any_of(
+    pending_daemon_health.begin(), pending_daemon_health.end(),
+    [](auto& it) { return !it.second.metrics.empty() ? true : false; });
+}
+
 int MDSMonitor::filesystem_command(
     FSMap &fsmap,
     MonOpRequestRef op,
diff --git a/src/mon/MDSMonitor.h b/src/mon/MDSMonitor.h
index b0f88cd3130..dd2a269009d 100644
--- a/src/mon/MDSMonitor.h
+++ b/src/mon/MDSMonitor.h
@@ -53,6 +53,7 @@ class MDSMonitor : public PaxosService, public PaxosFSMap, protected CommandHand
   bool prepare_update(MonOpRequestRef op) override;
   bool should_propose(double& delay) override;
   bool has_health_warnings(std::vector<mds_metric_t> warnings);
+  bool has_any_health_warning();
 
   bool should_print_status() const {
     auto& fs = get_fsmap();