mon/OSDMonitor: remove unused crush rules after erasure code pools deleted

When erasure code pools are created, a corresponding Crush rule is concurrently added to the Crush map.
However, when these pools are subsequently deleted, the associated rule persists within the Crush map.
In the event that a pool is re-created with the same name, the rule already exists.
However, if any modifications were made to the rule prior to the pool's deletion,
the new pool will inherit these modifications.

The proposed solution involves the automatic deletion of the Crush rule when a pool is deleted,
but only if no other pools are utilizing that particular rule.

Fixes: https://tracker.ceph.com/issues/62826
Signed-off-by: Nitzan Mordechai <nmordech@redhat.com>

diff --git a/qa/standalone/mon/osd-crush.sh b/qa/standalone/mon/osd-crush.sh
index aa7cac6946944a6d90171143488fec5076b72dc6..cf21f921026e13f134d3e0ab57f3fa651dccf1e7 100755 (executable)
--- a/qa/standalone/mon/osd-crush.sh
+++ b/qa/standalone/mon/osd-crush.sh
@@ -120,6 +120,51 @@ function TEST_crush_rule_create_erasure() {
     ! ceph osd crush rule ls | grep $rule || return 1
 }
 
+function TEST_erasure_pool_crush_rule_rm() {
+    local dir=$1
+
+    run_mon $dir a || return 1
+    # should have at least one OSD
+    run_osd $dir 0 || return 1
+
+    # create a new rule with pool name as rule name
+    # delete the rule when the pool is deleted
+    local pool_name=cpool
+    ceph osd erasure-code-profile set profile_name plugin=jerasure technique=reed_sol_van k=2 m=2 crush-failure-domain=osd || return 1
+    ceph osd pool create $pool_name 12 12 erasure profile_name || return 1
+    ceph osd crush rule ls | grep $pool_name || return 1
+    ceph osd pool delete $pool_name $pool_name --yes-i-really-really-mean-it || return 1
+    ! ceph osd crush rule ls | grep $pool_name || return 1
+
+    # create few pools that using the same rule
+    # make sure the rule is not deleted when the pool is deleted
+    # unlease it was the last pool using the rule
+    local pool_name1=pool_name1
+    local pool_name2=pool_name2
+
+    ceph osd pool create $pool_name1 12 12 erasure profile_name || return 1
+    ceph osd crush rule ls | grep $pool_name1 || return 1
+    ceph osd pool create $pool_name2 12 12 erasure profile_name || return 1
+    ceph osd crush rule ls | grep $pool_name2 || return 1
+    ceph osd pool delete $pool_name1 $pool_name1 --yes-i-really-really-mean-it || return 1
+    ! ceph osd crush rule ls | grep $pool_name || return 1
+    ceph osd pool delete $pool_name2 $pool_name2 --yes-i-really-really-mean-it || return 1
+    ! ceph osd crush rule ls | grep $pool_name2 || return 1
+
+    # crush rule should be deleted when the last pool using it is deleted
+    local default_rule_name=erasure-code
+    ceph osd pool create $pool_name-1 12 12 erasure || return 1
+    ceph osd pool create $pool_name-2 12 12 erasure || return 1
+    ceph osd pool create $pool_name-3 12 12 erasure || return 1
+    ceph osd crush rule ls | grep $default_rule_name || return 1
+    ceph osd pool delete $pool_name-1 $pool_name-1 --yes-i-really-really-mean-it || return 1
+    ceph osd crush rule ls | grep $default_rule_name || return 1
+    ceph osd pool delete $pool_name-2 $pool_name-2 --yes-i-really-really-mean-it || return 1
+    ceph osd crush rule ls | grep $default_rule_name || return 1
+    ceph osd pool delete $pool_name-3 $pool_name-3 --yes-i-really-really-mean-it || return 1
+    ! ceph osd crush rule ls | grep $default_rule_name || return 1
+}
+
 function TEST_add_rule_failed() {
     local dir=$1
 

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 1c746113ce9dc0fea9aaeda3c6df02e4bb5896bc..d1dff9595634d8f51881c7512f499f2a2333515d 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -15382,6 +15382,26 @@ int OSDMonitor::_prepare_remove_pool(
     pending_inc.crush.clear();
     newcrush.encode(pending_inc.crush, mon.get_quorum_con_features());
   }
+
+  // remove any crush rules for this pool
+  const pg_pool_t *pi = osdmap.get_pg_pool(pool);
+  if (pi->is_erasure() && newcrush.rule_exists(pi->get_crush_rule())) {
+    int ruleno = pi->get_crush_rule();
+    ceph_assert(ruleno >= 0);
+
+    auto rule_in_use = false;
+    for (const auto &_pool : osdmap.pools) {
+      if (_pool.second.get_crush_rule() == ruleno && pool != _pool.first)
+        rule_in_use = true;
+    }
+    if (!rule_in_use) {
+      dout(10) << __func__ << " removing crush rule for pool " << pool << dendl;
+      newcrush.remove_rule(ruleno);
+      pending_inc.crush.clear();
+      newcrush.encode(pending_inc.crush, mon.get_quorum_con_features());
+    }
+  }
+
   return 0;
 }