From d5cae058e8721b6d9ebef10c882d7db99522893c Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Tue, 9 Mar 2021 18:17:49 -0500 Subject: [PATCH] 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 (cherry picked from commit 3201516ea0708873b2d93d4452b38e1802346932) --- src/pybind/mgr/cephadm/migrations.py | 5 ++-- src/pybind/mgr/cephadm/module.py | 4 +-- src/pybind/mgr/cephadm/schedule.py | 6 ++--- src/pybind/mgr/cephadm/serve.py | 2 +- .../mgr/cephadm/tests/test_scheduling.py | 27 ++++++++++++------- 5 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/pybind/mgr/cephadm/migrations.py b/src/pybind/mgr/cephadm/migrations.py index 6b02e81e75acc..b48165836c750 100644 --- 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 50fede4e15e47..ca110de5f861d 100644 --- 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 c71b0a9d98b31..e64991cee8a8b 100644 --- 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 be4da498432f4..3c9e38fe67541 100644 --- 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 78f38b07b82c6..0dbd414b6c9b7 100644 --- 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 -- 2.39.5