From 6d6063a9b2b66248b6183b097d24505bacc2cfd5 Mon Sep 17 00:00:00 2001
From: Adam King <adking@redhat.com>
Date: Wed, 28 May 2025 12:08:27 -0400
Subject: [PATCH] mgr/cephadm: don't remove and deploy new daemon if ports
 change during upgrade

If we're not mid-upgrade, a port/ip change is most likely a user
changing the configuration for the daemon and starting a new
daemon with the new ports can be justified. During upgrade, their
are a number of different upgrade paths (espeically considering
our N+2 upgrade support) where something internal to cephadm
has made the ip we're binding to or the ports being used
change. In these cases, the process of upgrading the daemon
will resolve the issue. Howeverm by having the scheduler
unilaterally remove and deploy fresh versions of daemons
when it sees port changes, we may effectively "upgrade" some
daemons out of the intended order just to make the ports
match up. This was seen with nvmeof which needs to be upgraded
after the mon daemons, but was being removed and redeployed
after the mgr upgrade once cephadm saw the set of expected
ports had changed.

This patch adds a new "upgrade_in_progress" attribute to the
HostAssignment class in the scheduler to make it aware of
an ongoing upgrade. It also changes the behavior specifically
around whether a daemon matches another if the ports match
up when "upgrade_in_progress" was set to True.

Signed-off-by: Adam King <adking@redhat.com>
---
 src/pybind/mgr/cephadm/schedule.py | 23 +++++++++++++++--------
 src/pybind/mgr/cephadm/serve.py    |  1 +
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/pybind/mgr/cephadm/schedule.py b/src/pybind/mgr/cephadm/schedule.py
index 5cd68299ed422..e71c0345f7560 100644
--- a/src/pybind/mgr/cephadm/schedule.py
+++ b/src/pybind/mgr/cephadm/schedule.py
@@ -97,7 +97,7 @@ class DaemonPlacement(NamedTuple):
             gen,
         )
 
-    def matches_daemon(self, dd: DaemonDescription) -> bool:
+    def matches_daemon(self, dd: DaemonDescription, upgrade_in_progress: bool = False) -> bool:
         if self.daemon_type != dd.daemon_type:
             return False
         if self.hostname != dd.hostname:
@@ -105,11 +105,16 @@ class DaemonPlacement(NamedTuple):
         # fixme: how to match against network?
         if self.name and self.name != dd.daemon_id:
             return False
-        if self.ports:
-            if self.ports != dd.ports and dd.ports:
-                return False
-            if self.ip != dd.ip and dd.ip:
-                return False
+        # only consider daemon "not matching" on port/ip
+        # differences if we're not mid upgrade. During upgrade
+        # it's very likely we'll deploy the daemon with the
+        # new port/ips as part of the upgrade process
+        if not upgrade_in_progress:
+            if self.ports:
+                if self.ports != dd.ports and dd.ports:
+                    return False
+                if self.ip != dd.ip and dd.ip:
+                    return False
         return True
 
     def matches_rank_map(
@@ -154,6 +159,7 @@ class HostAssignment(object):
                  per_host_daemon_type: Optional[str] = None,
                  rank_map: Optional[Dict[int, Dict[int, Optional[str]]]] = None,
                  blocking_daemon_hosts: Optional[List[orchestrator.HostSpec]] = None,
+                 upgrade_in_progress: bool = False
                  ):
         assert spec
         self.spec = spec  # type: ServiceSpec
@@ -171,6 +177,7 @@ class HostAssignment(object):
         self.per_host_daemon_type = per_host_daemon_type
         self.ports_start = spec.get_port_start()
         self.rank_map = rank_map
+        self.upgrade_in_progress = upgrade_in_progress
 
     def hosts_by_label(self, label: str) -> List[orchestrator.HostSpec]:
         return [h for h in self.hosts if label in h.labels]
@@ -234,7 +241,7 @@ class HostAssignment(object):
             for dd in existing:
                 found = False
                 for p in host_slots:
-                    if p.matches_daemon(dd):
+                    if p.matches_daemon(dd, self.upgrade_in_progress):
                         host_slots.remove(p)
                         found = True
                         break
@@ -311,7 +318,7 @@ class HostAssignment(object):
         for dd in daemons:
             found = False
             for p in others:
-                if p.matches_daemon(dd) and p.matches_rank_map(dd, self.rank_map, ranks):
+                if p.matches_daemon(dd, self.upgrade_in_progress) and p.matches_rank_map(dd, self.rank_map, ranks):
                     others.remove(p)
                     if dd.is_active:
                         existing_active.append(dd)
diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py
index 68dc5e38b34dc..fa455c3cb68e0 100644
--- a/src/pybind/mgr/cephadm/serve.py
+++ b/src/pybind/mgr/cephadm/serve.py
@@ -826,6 +826,7 @@ class CephadmServe:
             primary_daemon_type=svc.primary_daemon_type(spec),
             per_host_daemon_type=svc.per_host_daemon_type(spec),
             rank_map=rank_map,
+            upgrade_in_progress=(self.mgr.upgrade.upgrade_state is not None)
         )
 
         try:
-- 
2.39.5