From 4730710d23d7794ee8e366049f7957909695cc65 Mon Sep 17 00:00:00 2001 From: Sebastian Wagner Date: Thu, 4 Jun 2020 14:23:46 +0200 Subject: [PATCH] python-common: Add Placement.filter_matching_hostspecs() Signed-off-by: Sebastian Wagner --- src/pybind/mgr/cephadm/inventory.py | 5 ++-- src/pybind/mgr/cephadm/module.py | 4 +-- .../mgr/cephadm/tests/test_scheduling.py | 29 ++++++++++++++++--- .../ceph/deployment/service_spec.py | 18 ++++++++---- .../ceph/tests/test_drive_group.py | 5 ++-- 5 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/pybind/mgr/cephadm/inventory.py b/src/pybind/mgr/cephadm/inventory.py index e107ee4abb4..22244c98a38 100644 --- a/src/pybind/mgr/cephadm/inventory.py +++ b/src/pybind/mgr/cephadm/inventory.py @@ -83,8 +83,9 @@ class Inventory: for h, hostspec in self._inventory.items(): if not label or label in hostspec.get('labels', []): if as_hostspec: - yield hostspec - yield h + yield self.spec_from_dict(hostspec) + else: + yield h def spec_from_dict(self, info): hostname = info['hostname'] diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index de9673aa039..2985b1f2a6c 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -1276,7 +1276,7 @@ you may want to run: osd_count += 1 sm[n].size = osd_count else: - sm[n].size = spec.placement.get_host_selection_size(self._get_hosts) + sm[n].size = spec.placement.get_host_selection_size(self.inventory.all_specs()) sm[n].created = self.spec_store.spec_created[n] if service_type == 'nfs': @@ -1301,7 +1301,7 @@ you may want to run: continue sm[n] = orchestrator.ServiceDescription( spec=spec, - size=spec.placement.get_host_selection_size(self._get_hosts), + size=spec.placement.get_host_selection_size(self.inventory.all_specs()), running=0, ) if service_type == 'nfs': diff --git a/src/pybind/mgr/cephadm/tests/test_scheduling.py b/src/pybind/mgr/cephadm/tests/test_scheduling.py index bc6a8b6b8fd..e8de347db4e 100644 --- a/src/pybind/mgr/cephadm/tests/test_scheduling.py +++ b/src/pybind/mgr/cephadm/tests/test_scheduling.py @@ -1,6 +1,7 @@ from typing import NamedTuple, List import pytest +from ceph.deployment.hostspec import HostSpec from ceph.deployment.service_spec import ServiceSpec, PlacementSpec, ServiceSpecValidationError from cephadm.module import HostAssignment @@ -114,9 +115,15 @@ 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) for h in hosts] + return hosts + + hosts = HostAssignment( spec=ServiceSpec(service_type, placement=placement), - get_hosts_func=lambda label=None, as_hostspec=False: hosts, + get_hosts_func=get_hosts_func, get_daemons_func=lambda _: daemons).place() assert sorted([h.hostname for h in hosts]) == sorted(expected) @@ -202,9 +209,14 @@ 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) for h in hosts] + return hosts + hosts = HostAssignment( spec=ServiceSpec(service_type, placement=placement), - get_hosts_func=lambda label=None, as_hostspec=False: hosts, + get_hosts_func=get_hosts_func, get_daemons_func=lambda _: daemons).place() assert len(hosts) == expected_len for h in [h.hostname for h in hosts]: @@ -233,9 +245,14 @@ 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=lambda label=None, as_hostspec=False: hosts, + get_hosts_func=get_hosts_func, get_daemons_func=lambda _: daemons).place() assert len(hosts) == expected_len for h in must_have: @@ -291,9 +308,13 @@ 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=lambda label=None, as_hostspec=False: hosts, + get_hosts_func=get_hosts_func, get_daemons_func=lambda _: daemons).place() assert str(e.value) == expected diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index 627a7ca6a28..9cd09f52ef6 100644 --- a/src/python-common/ceph/deployment/service_spec.py +++ b/src/python-common/ceph/deployment/service_spec.py @@ -2,10 +2,12 @@ import fnmatch import re from collections import namedtuple from functools import wraps -from typing import Optional, Dict, Any, List, Union, Callable +from typing import Optional, Dict, Any, List, Union, Callable, Iterator import six +from ceph.deployment.hostspec import HostSpec + class ServiceSpecValidationError(Exception): """ @@ -174,22 +176,26 @@ class PlacementSpec(object): self.hosts = hosts def filter_matching_hosts(self, _get_hosts_func: Callable) -> List[str]: + return self.filter_matching_hostspecs(_get_hosts_func(as_hostspec=True)) + + def filter_matching_hostspecs(self, hostspecs: Iterator[HostSpec]) -> List[str]: if self.hosts: - all_hosts = _get_hosts_func(label=None, as_hostspec=False) + all_hosts = [hs.hostname for hs in hostspecs] return [h.hostname for h in self.hosts if h.hostname in all_hosts] elif self.label: - return _get_hosts_func(label=self.label, as_hostspec=False) + return [hs.hostname for hs in hostspecs if self.label in hs.labels] elif self.host_pattern: - return fnmatch.filter(_get_hosts_func(label=None, as_hostspec=False), self.host_pattern) + all_hosts = [hs.hostname for hs in hostspecs] + return fnmatch.filter(all_hosts, self.host_pattern) else: # This should be caught by the validation but needs to be here for # get_host_selection_size return [] - def get_host_selection_size(self, _get_hosts_func): + def get_host_selection_size(self, hostspecs: Iterator[HostSpec]): if self.count: return self.count - return len(self.filter_matching_hosts(_get_hosts_func)) + return len(self.filter_matching_hostspecs(hostspecs)) def pretty_str(self): kv = [] diff --git a/src/python-common/ceph/tests/test_drive_group.py b/src/python-common/ceph/tests/test_drive_group.py index a34e93aaa27..d98152bc374 100644 --- a/src/python-common/ceph/tests/test_drive_group.py +++ b/src/python-common/ceph/tests/test_drive_group.py @@ -3,6 +3,7 @@ import pytest import yaml from ceph.deployment import drive_selection, translate +from ceph.deployment.hostspec import HostSpec from ceph.deployment.inventory import Device from ceph.deployment.service_spec import PlacementSpec, ServiceSpecValidationError from ceph.tests.utils import _mk_inventory, _mk_device @@ -24,7 +25,7 @@ from ceph.deployment.drive_group import DriveGroupSpec, DeviceSelection, \ ]) def test_DriveGroup(test_input): dg = [DriveGroupSpec.from_json(inp) for inp in test_input][0] - assert dg.placement.filter_matching_hosts(lambda label=None, as_hostspec=None: ['hostname']) == ['hostname'] + assert dg.placement.filter_matching_hostspecs([HostSpec('hostname')]) == ['hostname'] assert dg.service_id == 'testing_drivegroup' assert all([isinstance(x, Device) for x in dg.data_devices.paths]) assert dg.data_devices.paths[0].path == '/dev/sda' @@ -53,7 +54,7 @@ def test_DriveGroup_fail(test_input): def test_drivegroup_pattern(): dg = DriveGroupSpec(PlacementSpec(host_pattern='node[1-3]'), data_devices=DeviceSelection(all=True)) - assert dg.placement.filter_matching_hosts(lambda label=None, as_hostspec=None: ['node{}'.format(i) for i in range(10)]) == ['node1', 'node2', 'node3'] + assert dg.placement.filter_matching_hostspecs([HostSpec('node{}'.format(i)) for i in range(10)]) == ['node1', 'node2', 'node3'] def test_drive_selection(): -- 2.39.5