mgr: override OSD hostname with CRUSH physical host in all metadata paths

In containerized deployments (e.g. Rook), OSD daemons report the
pod/container hostname. The CRUSH map records the physical host.
This mismatch causes 'ceph device ls' to show pod names instead of
physical hosts, breaking host-based device correlation.

Fix by deriving the CRUSH host bucket name for each OSD and substituting
it as the canonical hostname in all three metadata paths:
- MetadataUpdate::finish (async mon metadata fetch, primary path)
- Mgr::load_all_metadata OSD bulk-load (startup/failover)
- DaemonServer::fetch_missing_metadata (via MetadataUpdate)

The override is a no-op when the OSD is absent from CRUSH or when the
OSD map is not yet available.

Fixes: https://tracker.ceph.com/issues/73080
Signed-off-by: senol.colak <senol.colak@sap.com>

diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc
index 210ff663208483c2c9765a7ee2aaca9bd3103a45..fe4ffd8bedbf779b26a284716faa7a52a09ee667 100644 (file)
--- a/src/mgr/DaemonServer.cc
+++ b/src/mgr/DaemonServer.cc
@@ -539,7 +539,7 @@ void DaemonServer::fetch_missing_metadata(const DaemonKey& key,
   if (!daemon_state.is_updating(key) &&
       (key.type == "osd" || key.type == "mds" || key.type == "mon")) {
     std::ostringstream oss;
-    auto c = new MetadataUpdate(daemon_state, key);
+    auto c = new MetadataUpdate(daemon_state, cluster_state, key);
     if (key.type == "osd") {
       oss << "{\"prefix\": \"osd metadata\", \"id\": "
          << key.name<< "}";
@@ -3677,7 +3677,7 @@ void DaemonServer::got_mgr_map()
   cluster_state.with_mgrmap([&](const MgrMap& mgrmap) {
       auto md_update = [&] (DaemonKey key) {
         std::ostringstream oss;
-        auto c = new MetadataUpdate(daemon_state, key);
+        auto c = new MetadataUpdate(daemon_state, cluster_state, key);
        // FIXME remove post-nautilus: include 'id' for luminous mons
         oss << "{\"prefix\": \"mgr metadata\", \"who\": \""
            << key.name << "\", \"id\": \"" << key.name << "\"}";

diff --git a/src/mgr/Mgr.cc b/src/mgr/Mgr.cc
index 50fb01fce48e6f0fd10e22d0c0d58fc4dc070821..51c20f6b7c8e589514ce1d1f59a4ce2c45fc4780 100644 (file)
--- a/src/mgr/Mgr.cc
+++ b/src/mgr/Mgr.cc
@@ -83,6 +83,21 @@ Mgr::~Mgr()
 {
 }
 
+static std::string crush_hostname_for_osd(ClusterState& cluster_state, int osd_id)
+{
+  std::string hostname;
+  cluster_state.with_osdmap([&](const OSDMap& osdmap) {
+    if (osdmap.crush) {
+      auto loc = osdmap.crush->get_full_location(osd_id);
+      auto it = loc.find("host");
+      if (it != loc.end()) {
+        hostname = it->second;
+      }
+    }
+  });
+  return hostname;
+}
+
 void MetadataUpdate::finish(int r)
 {
   daemon_state.clear_updating(key);
@@ -120,11 +135,20 @@ void MetadataUpdate::finish(int r)
 
       if (daemon_state.exists(key)) {
         DaemonStatePtr state = daemon_state.get(key);
+       // Resolve CRUSH host before taking state->lock to preserve lock ordering
+       // (Objecter::rwlock must not be acquired under state->lock)
+       std::string crush_host;
+       if (key.type == "osd") {
+         try {
+           crush_host = crush_hostname_for_osd(cluster_state, std::stoi(key.name));
+         } catch (const std::exception& e) {
+           dout(5) << "cannot derive CRUSH hostname for " << key
+                   << ": " << e.what() << dendl;
+         }
+       }
        std::map<string,string> m;
        {
          std::lock_guard l(state->lock);
-         state->hostname = daemon_meta.at("hostname").get_str();
-
          if (key.type == "mds" || key.type == "mgr" || key.type == "mon") {
            daemon_meta.erase("name");
          } else if (key.type == "osd") {
@@ -134,7 +158,14 @@ void MetadataUpdate::finish(int r)
          for (const auto &[key, val] : daemon_meta) {
            m.emplace(key, val.get_str());
          }
+         // prefer CRUSH physical host over container/pod hostname (tracker.ceph.com/issues/73080)
+         if (!crush_host.empty()) {
+           m["hostname"] = crush_host;
+         }
        }
+       // update_metadata calls _rm then _insert using state->hostname read from
+       // the map via set_metadata, so hostname must be set through m, not
+       // directly on state->hostname before the call (that would corrupt by_server)
        daemon_state.update_metadata(state, m);
       } else {
         auto state = std::make_shared<DaemonState>(daemon_state.types);
@@ -152,6 +183,20 @@ void MetadataUpdate::finish(int r)
         for (const auto &[key, val] : daemon_meta) {
           m.emplace(key, val.get_str());
         }
+       // prefer CRUSH physical host over container/pod hostname (tracker.ceph.com/issues/73080)
+       if (key.type == "osd") {
+         try {
+           std::string crush_host = crush_hostname_for_osd(cluster_state,
+                                                           std::stoi(key.name));
+           if (!crush_host.empty()) {
+             state->hostname = crush_host;
+             m["hostname"] = crush_host;
+           }
+         } catch (const std::exception& e) {
+           dout(5) << "cannot derive CRUSH hostname for " << key
+                   << ": " << e.what() << dendl;
+         }
+       }
        state->set_metadata(m);
 
         daemon_state.insert(state);
@@ -478,13 +523,19 @@ void Mgr::load_all_metadata()
       dout(1) << "Skipping incomplete metadata entry" << dendl;
       continue;
     }
-    dout(4) << osd_metadata.at("hostname").get_str() << dendl;
 
     DaemonStatePtr dm = std::make_shared<DaemonState>(daemon_state.types);
-    dm->key = DaemonKey{"osd",
-                        stringify(osd_metadata.at("id").get_int())};
+    int osd_id = osd_metadata.at("id").get_int();
+    dm->key = DaemonKey{"osd", stringify(osd_id)};
     dm->hostname = osd_metadata.at("hostname").get_str();
 
+    // prefer CRUSH physical host over container/pod hostname (tracker.ceph.com/issues/73080)
+    std::string crush_host = crush_hostname_for_osd(cluster_state, osd_id);
+    if (!crush_host.empty()) {
+      dm->hostname = crush_host;
+    }
+    dout(4) << dm->hostname << dendl;
+
     osd_metadata.erase("id");
     osd_metadata.erase("hostname");
 
@@ -550,7 +601,7 @@ void Mgr::handle_osd_map()
         update_meta = true;
       }
       if (update_meta) {
-        auto c = new MetadataUpdate(daemon_state, k);
+        auto c = new MetadataUpdate(daemon_state, cluster_state, k);
         std::ostringstream cmd;
         cmd << "{\"prefix\": \"osd metadata\", \"id\": "
             << osd_id << "}";
@@ -597,7 +648,7 @@ void Mgr::handle_mon_map()
     if (daemon_state.is_updating(k)) {
       continue;
     }
-    auto c = new MetadataUpdate(daemon_state, k);
+    auto c = new MetadataUpdate(daemon_state, cluster_state, k);
     constexpr std::string_view cmd = R"({{"prefix": "mon metadata", "id": "{}"}})";
     monc->start_mon_command({fmt::format(cmd, name)}, {},
                            &c->outbl, &c->outs, c);
@@ -736,7 +787,7 @@ void Mgr::handle_fs_map(ref_t<MFSMap> m)
     }
 
     if (update) {
-      auto c = new MetadataUpdate(daemon_state, k);
+      auto c = new MetadataUpdate(daemon_state, cluster_state, k);
 
       // Older MDS daemons don't have addr in the metadata, so
       // fake it if the returned metadata doesn't have the field.

diff --git a/src/mgr/Mgr.h b/src/mgr/Mgr.h
index ec3bad9f42b0c2c6f1dd87fbe40466aa5c3386fe..952e446751da4ee97ffdf942af3211a419f86671 100644 (file)
--- a/src/mgr/Mgr.h
+++ b/src/mgr/Mgr.h
@@ -115,6 +115,7 @@ class MetadataUpdate : public Context
 
 private:
   DaemonStateIndex &daemon_state;
+  ClusterState &cluster_state;
   DaemonKey key;
 
   std::map<std::string, std::string> defaults;
@@ -123,8 +124,9 @@ public:
   bufferlist outbl;
   std::string outs;
 
-  MetadataUpdate(DaemonStateIndex &daemon_state_, const DaemonKey &key_)
-    : daemon_state(daemon_state_), key(key_)
+  MetadataUpdate(DaemonStateIndex &daemon_state_, ClusterState &cluster_state_,
+                 const DaemonKey &key_)
+    : daemon_state(daemon_state_), cluster_state(cluster_state_), key(key_)
   {
       daemon_state.notify_updating(key);
   }