mgr/cephadm/schedule: pass daemons, not get_daemons_func

- simpler
- many/most callers already have the daemon list, so we save ourselves
duplicated effort

Signed-off-by: Sage Weil <sage@newdream.net>
(cherry picked from commit 3201516ea0708873b2d93d4452b38e1802346932)

diff --git a/src/pybind/mgr/cephadm/migrations.py b/src/pybind/mgr/cephadm/migrations.py
index 6b02e81e75acc71565b7913e4ffda5b190dce95f..b48165836c750ee7c51923f9325c02d9f6c011cc 100644 (file)
--- a/src/pybind/mgr/cephadm/migrations.py
+++ b/src/pybind/mgr/cephadm/migrations.py
@@ -89,14 +89,13 @@ class Migrations:
                 yield s
 
         def convert_to_explicit(spec: ServiceSpec) -> None:
+            existing_daemons = self.mgr.cache.get_daemons_by_service(spec.service_name())
             placements = HostAssignment(
                 spec=spec,
                 hosts=self.mgr.inventory.all_specs(),
-                get_daemons_func=self.mgr.cache.get_daemons_by_service
+                daemons=existing_daemons,
             ).place()
 
-            existing_daemons = self.mgr.cache.get_daemons_by_service(spec.service_name())
-
             # We have to migrate, only if the new scheduler would remove daemons
             if len(placements) >= len(existing_daemons):
                 return

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index 50fede4e15e47007856997b88e2851f742c09a6d..ca110de5f861d534a3e226caea406c0e5f4b1de9 100644 (file)
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -2086,7 +2086,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule,
         ha = HostAssignment(
             spec=spec,
             hosts=self._hosts_with_daemon_inventory(),
-            get_daemons_func=self.cache.get_daemons_by_service,
+            daemons=self.cache.get_daemons_by_service(spec.service_name()),
         )
         ha.validate()
         hosts = ha.place()
@@ -2144,7 +2144,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule,
         HostAssignment(
             spec=spec,
             hosts=self.inventory.all_specs(),  # All hosts, even those without daemon refresh
-            get_daemons_func=self.cache.get_daemons_by_service,
+            daemons=self.cache.get_daemons_by_service(spec.service_name()),
         ).validate()
 
         self.log.info('Saving service %s spec with placement %s' % (

diff --git a/src/pybind/mgr/cephadm/schedule.py b/src/pybind/mgr/cephadm/schedule.py
index c71b0a9d98b31d80457b88e1febf7c6a416fa5dd..e64991cee8a8bd64040e12ca8ed9133ee9eb80ea 100644 (file)
--- a/src/pybind/mgr/cephadm/schedule.py
+++ b/src/pybind/mgr/cephadm/schedule.py
@@ -56,17 +56,17 @@ class HostAssignment(object):
     def __init__(self,
                  spec,  # type: ServiceSpec
                  hosts: List[orchestrator.HostSpec],
-                 get_daemons_func,  # type: Callable[[str],List[orchestrator.DaemonDescription]]
+                 daemons: List[orchestrator.DaemonDescription],
                  filter_new_host=None,  # type: Optional[Callable[[str],bool]]
                  scheduler=None,  # type: Optional[BaseScheduler]
                  ):
-        assert spec and get_daemons_func
+        assert spec
         self.spec = spec  # type: ServiceSpec
         self.scheduler = scheduler if scheduler else SimpleScheduler(self.spec)
         self.hosts: List[orchestrator.HostSpec] = hosts
         self.filter_new_host = filter_new_host
         self.service_name = spec.service_name()
-        self.daemons = get_daemons_func(self.service_name)
+        self.daemons = daemons
 
     def hosts_by_label(self, label: str) -> List[orchestrator.HostSpec]:
         return [h for h in self.hosts if label in h.labels]

diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py
index be4da498432f49664aeb437e224421cb5d45c9c7..3c9e38fe675411caff5ba78c70744f3e17e13eb2 100644 (file)
--- a/src/pybind/mgr/cephadm/serve.py
+++ b/src/pybind/mgr/cephadm/serve.py
@@ -551,7 +551,7 @@ class CephadmServe:
         ha = HostAssignment(
             spec=spec,
             hosts=self.mgr._hosts_with_daemon_inventory(),
-            get_daemons_func=self.mgr.cache.get_daemons_by_service,
+            daemons=daemons,
             filter_new_host=matches_network if service_type == 'mon'
             else virtual_ip_allowed if service_type == 'ha-rgw' else None,
         )

diff --git a/src/pybind/mgr/cephadm/tests/test_scheduling.py b/src/pybind/mgr/cephadm/tests/test_scheduling.py
index 78f38b07b82c64e0d485a21ce41456c928c8e948..0dbd414b6c9b7f3ed7f6734aa41a9b5021c3a45d 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/test_scheduling.py
+++ b/src/pybind/mgr/cephadm/tests/test_scheduling.py
@@ -120,7 +120,7 @@ def mk_spec_and_host(spec_section, hosts, explicit_key, explicit, count):
     return mk_spec, hosts
 
 
-def run_scheduler_test(results, mk_spec, hosts, get_daemons_func, key_elems):
+def run_scheduler_test(results, mk_spec, hosts, daemons, key_elems):
     key = ' '.join('N' if e is None else str(e) for e in key_elems)
     try:
         assert_res = get_result(k(key), results)
@@ -130,7 +130,8 @@ def run_scheduler_test(results, mk_spec, hosts, get_daemons_func, key_elems):
             host_res = HostAssignment(
                 spec=spec,
                 hosts=hosts,
-                get_daemons_func=get_daemons_func).place()
+                daemons=daemons,
+            ).place()
             if isinstance(host_res, list):
                 e = ', '.join(repr(h.hostname) for h in host_res)
                 assert False, f'`(k("{key}"), exactly({e})),` not found'
@@ -144,7 +145,8 @@ def run_scheduler_test(results, mk_spec, hosts, get_daemons_func, key_elems):
             host_res = HostAssignment(
                 spec=spec,
                 hosts=hosts,
-                get_daemons_func=get_daemons_func).place()
+                daemons=daemons
+            ).place()
 
             assert_res(sorted([h.hostname for h in host_res]))
         except Exception as e:
@@ -221,7 +223,7 @@ def test_explicit_scheduler(host_key, hosts,
         results=test_explicit_scheduler_results,
         mk_spec=mk_spec,
         hosts=hosts,
-        get_daemons_func=lambda _: [],
+        daemons=[],
         key_elems=(host_key, explicit_key, count, spec_section_key)
     )
 
@@ -312,7 +314,7 @@ def test_scheduler_daemons(host_key, hosts,
         results=test_scheduler_daemons_results,
         mk_spec=mk_spec,
         hosts=hosts,
-        get_daemons_func=lambda _: dds,
+        daemons=dds,
         key_elems=(host_key, explicit_key, count, daemons_key, spec_section_key)
     )
 
@@ -467,7 +469,8 @@ def test_node_assignment(service_type, placement, hosts, daemons, expected):
     hosts = HostAssignment(
         spec=spec,
         hosts=[HostSpec(h, labels=['foo']) for h in hosts],
-        get_daemons_func=lambda _: daemons).place()
+        daemons=daemons
+    ).place()
     assert sorted([h.hostname for h in hosts]) == sorted(expected)
 
 
@@ -546,7 +549,8 @@ def test_node_assignment2(service_type, placement, hosts,
     hosts = HostAssignment(
         spec=ServiceSpec(service_type, placement=placement),
         hosts=[HostSpec(h, labels=['foo']) for h in hosts],
-        get_daemons_func=lambda _: daemons).place()
+        daemons=daemons,
+    ).place()
     assert len(hosts) == expected_len
     for h in [h.hostname for h in hosts]:
         assert h in in_set
@@ -578,7 +582,8 @@ def test_node_assignment3(service_type, placement, hosts,
     hosts = HostAssignment(
         spec=ServiceSpec(service_type, placement=placement),
         hosts=[HostSpec(h) for h in hosts],
-        get_daemons_func=lambda _: daemons).place()
+        daemons=daemons,
+    ).place()
     assert len(hosts) == expected_len
     for h in must_have:
         assert h in [h.hostname for h in hosts]
@@ -639,7 +644,8 @@ def test_bad_specs(service_type, placement, hosts, daemons, expected):
         hosts = HostAssignment(
             spec=ServiceSpec(service_type, placement=placement),
             hosts=[HostSpec(h) for h in hosts],
-            get_daemons_func=lambda _: daemons).place()
+            daemons=daemons,
+        ).place()
     assert str(e.value) == expected
 
 
@@ -789,5 +795,6 @@ def test_active_assignment(service_type, placement, hosts, daemons, expected):
     hosts = HostAssignment(
         spec=spec,
         hosts=[HostSpec(h) for h in hosts],
-        get_daemons_func=lambda _: daemons).place()
+        daemons=daemons,
+    ).place()
     assert sorted([h.hostname for h in hosts]) in expected