From: senol.colak Date: Wed, 27 May 2026 16:47:20 +0000 (+0200) Subject: mgr: override OSD hostname with CRUSH physical host in all metadata paths X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=dcd9c5bac5e2670efd726e1ae89bc264bd404671;p=ceph.git 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 --- diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc index 210ff6632084..fe4ffd8bedbf 100644 --- 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 50fb01fce48e..51c20f6b7c8e 100644 --- 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 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(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(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 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 ec3bad9f42b0..952e446751da 100644 --- 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 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); }