From 908bd443f335979f428833059161673b5330e745 Mon Sep 17 00:00:00 2001 From: Sebastian Wagner Date: Mon, 14 Sep 2020 11:27:02 +0200 Subject: [PATCH] mgr/cephadm: Simplify HostAssignment arguments `get_hosts_func` -> `hosts`, as `get_hosts_func` is only called once in __init__ Signed-off-by: Sebastian Wagner --- src/pybind/mgr/cephadm/inventory.py | 6 +-- src/pybind/mgr/cephadm/migrations.py | 2 +- src/pybind/mgr/cephadm/module.py | 6 +-- src/pybind/mgr/cephadm/schedule.py | 6 +-- .../mgr/cephadm/tests/test_scheduling.py | 37 ++++--------------- 5 files changed, 17 insertions(+), 40 deletions(-) diff --git a/src/pybind/mgr/cephadm/inventory.py b/src/pybind/mgr/cephadm/inventory.py index 6d6247e835f6e..6633bc73874b4 100644 --- a/src/pybind/mgr/cephadm/inventory.py +++ b/src/pybind/mgr/cephadm/inventory.py @@ -85,7 +85,7 @@ class Inventory: else: yield h - def spec_from_dict(self, info): + def spec_from_dict(self, info) -> HostSpec: hostname = info['hostname'] return HostSpec( hostname, @@ -94,8 +94,8 @@ class Inventory: status='Offline' if hostname in self.mgr.offline_hosts else info.get('status', ''), ) - def all_specs(self) -> Iterator[HostSpec]: - return map(self.spec_from_dict, self._inventory.values()) + def all_specs(self) -> List[HostSpec]: + return list(map(self.spec_from_dict, self._inventory.values())) def save(self): self.mgr.set_store('inventory', json.dumps(self._inventory)) diff --git a/src/pybind/mgr/cephadm/migrations.py b/src/pybind/mgr/cephadm/migrations.py index 9e1612dc40e1e..4837cb19aaf1a 100644 --- a/src/pybind/mgr/cephadm/migrations.py +++ b/src/pybind/mgr/cephadm/migrations.py @@ -91,7 +91,7 @@ class Migrations: def convert_to_explicit(spec: ServiceSpec) -> None: placements = HostAssignment( spec=spec, - get_hosts_func=self.mgr._get_hosts, + hosts=self.mgr.inventory.all_specs(), get_daemons_func=self.mgr.cache.get_daemons_by_service ).place() diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index 24d3eb9e46d53..e17394df8a845 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -2114,7 +2114,7 @@ To check that the host is reachable: ha = HostAssignment( spec=spec, - get_hosts_func=self._get_hosts, + hosts=self.inventory.all_specs(), get_daemons_func=self.cache.get_daemons_by_service, filter_new_host=matches_network if daemon_type == 'mon' else None, ) @@ -2375,7 +2375,7 @@ To check that the host is reachable: ha = HostAssignment( spec=spec, - get_hosts_func=self._get_hosts, + hosts=self.inventory.all_specs(), get_daemons_func=self.cache.get_daemons_by_service, ) ha.validate() @@ -2429,7 +2429,7 @@ To check that the host is reachable: HostAssignment( spec=spec, - get_hosts_func=self._get_hosts, + hosts=self.inventory.all_specs(), get_daemons_func=self.cache.get_daemons_by_service, ).validate() diff --git a/src/pybind/mgr/cephadm/schedule.py b/src/pybind/mgr/cephadm/schedule.py index da1beb158dcce..2797ed1b86783 100644 --- a/src/pybind/mgr/cephadm/schedule.py +++ b/src/pybind/mgr/cephadm/schedule.py @@ -55,15 +55,15 @@ class HostAssignment(object): def __init__(self, spec, # type: ServiceSpec - get_hosts_func, # type: Callable + hosts: List[orchestrator.HostSpec], get_daemons_func, # type: Callable[[str],List[orchestrator.DaemonDescription]] filter_new_host=None, # type: Optional[Callable[[str],bool]] scheduler=None, # type: Optional[BaseScheduler] ): - assert spec and get_hosts_func and get_daemons_func + assert spec and get_daemons_func self.spec = spec # type: ServiceSpec self.scheduler = scheduler if scheduler else SimpleScheduler(self.spec) - self.hosts: List[orchestrator.HostSpec] = get_hosts_func(as_hostspec=True) + 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) diff --git a/src/pybind/mgr/cephadm/tests/test_scheduling.py b/src/pybind/mgr/cephadm/tests/test_scheduling.py index 9287f170b3a9f..32fe8205b2d2c 100644 --- a/src/pybind/mgr/cephadm/tests/test_scheduling.py +++ b/src/pybind/mgr/cephadm/tests/test_scheduling.py @@ -130,7 +130,7 @@ def run_scheduler_test(results, mk_spec, get_hosts_func, get_daemons_func, key_e spec = mk_spec() host_res = HostAssignment( spec=spec, - get_hosts_func=get_hosts_func, + hosts=get_hosts_func(), get_daemons_func=get_daemons_func).place() if isinstance(host_res, list): e = ', '.join(repr(h.hostname) for h in host_res) @@ -144,7 +144,7 @@ def run_scheduler_test(results, mk_spec, get_hosts_func, get_daemons_func, key_e spec = mk_spec() host_res = HostAssignment( spec=spec, - get_hosts_func=get_hosts_func, + hosts=get_hosts_func(), get_daemons_func=get_daemons_func).place() assert_res(sorted([h.hostname for h in host_res])) @@ -427,11 +427,6 @@ class NodeAssignmentTest(NamedTuple): ), ]) def test_node_assignment(service_type, placement, hosts, daemons, expected): - def get_hosts_func(label=None, as_hostspec=False): - if as_hostspec: - return [HostSpec(h, labels=['foo']) for h in hosts] - return hosts - service_id = None if service_type == 'rgw': service_id = 'realm.zone' @@ -442,7 +437,7 @@ def test_node_assignment(service_type, placement, hosts, daemons, expected): hosts = HostAssignment( spec=spec, - get_hosts_func=get_hosts_func, + hosts=[HostSpec(h, labels=['foo']) for h in hosts], get_daemons_func=lambda _: daemons).place() assert sorted([h.hostname for h in hosts]) == sorted(expected) @@ -518,14 +513,9 @@ class NodeAssignmentTest2(NamedTuple): ]) def test_node_assignment2(service_type, placement, hosts, daemons, expected_len, in_set): - def get_hosts_func(label=None, as_hostspec=False): - if as_hostspec: - return [HostSpec(h, labels=['foo']) for h in hosts] - return hosts - hosts = HostAssignment( spec=ServiceSpec(service_type, placement=placement), - get_hosts_func=get_hosts_func, + hosts=[HostSpec(h, labels=['foo']) for h in hosts], get_daemons_func=lambda _: daemons).place() assert len(hosts) == expected_len for h in [h.hostname for h in hosts]: @@ -554,14 +544,9 @@ def test_node_assignment2(service_type, placement, hosts, ]) def test_node_assignment3(service_type, placement, hosts, daemons, expected_len, must_have): - def get_hosts_func(label=None, as_hostspec=False): - if as_hostspec: - return [HostSpec(h) for h in hosts] - return hosts - hosts = HostAssignment( spec=ServiceSpec(service_type, placement=placement), - get_hosts_func=get_hosts_func, + hosts=[HostSpec(h) for h in hosts], get_daemons_func=lambda _: daemons).place() assert len(hosts) == expected_len for h in must_have: @@ -617,14 +602,10 @@ class NodeAssignmentTestBadSpec(NamedTuple): ), ]) def test_bad_specs(service_type, placement, hosts, daemons, expected): - def get_hosts_func(label=None, as_hostspec=False): - if as_hostspec: - return [HostSpec(h) for h in hosts] - return hosts with pytest.raises(OrchestratorValidationError) as e: hosts = HostAssignment( spec=ServiceSpec(service_type, placement=placement), - get_hosts_func=get_hosts_func, + hosts=[HostSpec(h) for h in hosts], get_daemons_func=lambda _: daemons).place() assert str(e.value) == expected @@ -766,10 +747,6 @@ class ActiveAssignmentTest(NamedTuple): ]) def test_active_assignment(service_type, placement, hosts, daemons, expected): - def get_hosts_func(label=None, as_hostspec=False): - if as_hostspec: - return [HostSpec(h) for h in hosts] - return hosts spec = ServiceSpec(service_type=service_type, service_id=None, @@ -777,6 +754,6 @@ def test_active_assignment(service_type, placement, hosts, daemons, expected): hosts = HostAssignment( spec=spec, - get_hosts_func=get_hosts_func, + hosts=[HostSpec(h) for h in hosts], get_daemons_func=lambda _: daemons).place() assert sorted([h.hostname for h in hosts]) in expected -- 2.39.5