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 <[email protected]>

Author: adk3798

src/pybind/mgr/cephadm/schedule.py

@@ -97,19 +97,24 @@ def assign_rank_generation(
             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:
             return False
         # 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 @@ def __init__(self,
                  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 @@ def __init__(self,
         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 @@ def place_per_host_daemons(
             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 @@ def expand_candidates(ls: List[DaemonPlacement], num: int) -> List[DaemonPlaceme
         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)

src/pybind/mgr/cephadm/serve.py

@@ -826,6 +826,7 @@ def has_interface_for_vip(host: str, sspec: ServiceSpec) -> bool:
             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: