From: Sebastian Wagner Date: Fri, 13 Mar 2020 11:13:13 +0000 (+0100) Subject: python-common: PlacementSpec.from_string: drop `host_pattern:` prefix X-Git-Tag: v15.2.0~51^2~3^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=13ef41dad74fd71018dbf01e4f49eee48962d0c8;p=ceph.git python-common: PlacementSpec.from_string: drop `host_pattern:` prefix * `host_pattern` is works as any other host spec. * Improved type annotations * Added tests * `mgr/cephadm/test_scheduling`: Remove tests that don't have a valid spec. Signed-off-by: Sebastian Wagner --- diff --git a/src/pybind/mgr/cephadm/tests/test_scheduling.py b/src/pybind/mgr/cephadm/tests/test_scheduling.py index 72e1e0c6151c..2922c99e1831 100644 --- a/src/pybind/mgr/cephadm/tests/test_scheduling.py +++ b/src/pybind/mgr/cephadm/tests/test_scheduling.py @@ -24,19 +24,10 @@ class NodeAssignmentTest(NamedTuple): [], ['smithi060'] ), - # zero count - NodeAssignmentTest( - 'mon', - PlacementSpec(count=0), - ['smithi060'], - [], - [] - ), - # all_hosts NodeAssignmentTest( 'mon', - PlacementSpec(all_hosts=True), + PlacementSpec(host_pattern='*'), 'host1 host2 host3'.split(), [ DaemonDescription('mon', 'a', 'host1'), @@ -64,16 +55,16 @@ class NodeAssignmentTest(NamedTuple): ], ['host1', 'host2', 'host3'] ), - # zero count + partial host list + # count 1 + partial host list NodeAssignmentTest( 'mon', - PlacementSpec(count=0, hosts=['host3']), + PlacementSpec(count=1, hosts=['host3']), 'host1 host2 host3'.split(), [ DaemonDescription('mon', 'a', 'host1'), DaemonDescription('mon', 'b', 'host2'), ], - [] + ['host3'] ), # count + partial host list + existing NodeAssignmentTest( diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index d20a4b8b0287..e65ceaf83dd3 100644 --- a/src/python-common/ceph/deployment/service_spec.py +++ b/src/python-common/ceph/deployment/service_spec.py @@ -1,7 +1,7 @@ import fnmatch import re from collections import namedtuple -from typing import Optional, Dict, Any, List +from typing import Optional, Dict, Any, List, Union import six @@ -108,20 +108,27 @@ class PlacementSpec(object): For APIs that need to specify a host subset """ - def __init__(self, label=None, hosts=None, count=None, host_pattern=None): - # type: (Optional[str], Optional[List], Optional[int], Optional[str]) -> None + def __init__(self, + label=None, # type: Optional[str] + hosts=None, # type: Union[List[str],List[HostPlacementSpec]] + count=None, # type: Optional[int] + host_pattern=None # type: Optional[str] + ): + # type: (...) -> None self.label = label self.hosts = [] # type: List[HostPlacementSpec] + if hosts: if all([isinstance(host, HostPlacementSpec) for host in hosts]): - self.hosts = hosts + self.hosts = hosts # type: ignore else: - self.hosts = [HostPlacementSpec.parse(x, require_network=False) for x in hosts if x] + self.hosts = [HostPlacementSpec.parse(x, require_network=False) # type: ignore + for x in hosts if x] self.count = count # type: Optional[int] - #: An fnmatch pattern to select hosts. Can also be a single host. - self.host_pattern = host_pattern + #: fnmatch patterns to select hosts. Can also be a single host. + self.host_pattern = host_pattern # type: Optional[str] self.validate() @@ -151,7 +158,7 @@ class PlacementSpec(object): if self.hosts: kv.append('%s' % ','.join([str(h) for h in self.hosts])) if self.host_pattern: - kv.append('host_pattern:{}'.format(self.host_pattern)) + kv.append(self.host_pattern) return ' '.join(kv) def __repr__(self): @@ -190,7 +197,7 @@ class PlacementSpec(object): if self.count is not None and self.count <= 0: raise ServiceSpecValidationError("num/count must be > 1") if self.host_pattern and self.hosts: - raise ServiceSpecValidationError('cannot combine host_pattern and hosts') + raise ServiceSpecValidationError('cannot combine host patterns and hosts') @classmethod def from_string(cls, arg): @@ -219,13 +226,13 @@ tPlacementSpec(hostname='host2', network='', name='')]) PlacementSpec(count=3, label='mon') fnmatch is also supported: - >>> PlacementSpec.from_string('host_pattern:data[1-3]') + >>> PlacementSpec.from_string('data[1-3]') PlacementSpec(host_pattern='data[1-3]') >>> PlacementSpec.from_string(None) PlacementSpec() """ - if arg is None: + if arg is None or not arg: strings = [] elif isinstance(arg, str): if ' ' in arg: @@ -259,23 +266,27 @@ tPlacementSpec(hostname='host2', network='', name='')]) except ValueError: pass - if '*' in strings: - strings.remove('*') - strings.append('host_pattern:*') + advanced_hostspecs = [h for h in strings if + (':' in h or '=' in h or not any(c in '[]?*:=' for c in h)) and + 'label:' not in h] + for a_h in advanced_hostspecs: + strings.remove(a_h) - hosts = [x for x in strings - if x != '*' and 'label:' not in x and not x.startswith('host_pattern:')] - labels = [x[6:] for x in strings if 'label:' in x] + labels = [x for x in strings if 'label:' in x] if len(labels) > 1: raise ServiceSpecValidationError('more than one label provided: {}'.format(labels)) - host_patterns = [x[13:] for x in strings if x.startswith('host_pattern:')] + for l in labels: + strings.remove(l) + label = labels[0][6:] if labels else None + + host_patterns = strings if len(host_patterns) > 1: - raise ServiceSpecValidationError('more than one host_patterns provided: {}'.format( - host_patterns)) + raise ServiceSpecValidationError( + 'more than one host pattern provided: {}'.format(host_patterns)) ps = PlacementSpec(count=count, - hosts=hosts, - label=labels[0] if labels else None, + hosts=advanced_hostspecs, + label=label, host_pattern=host_patterns[0] if host_patterns else None) return ps diff --git a/src/python-common/ceph/tests/test_service_spec.py b/src/python-common/ceph/tests/test_service_spec.py index 83c1864f5656..4b2c85c4b1fd 100644 --- a/src/python-common/ceph/tests/test_service_spec.py +++ b/src/python-common/ceph/tests/test_service_spec.py @@ -4,7 +4,7 @@ import json import pytest from ceph.deployment.service_spec import HostPlacementSpec, PlacementSpec, RGWSpec, \ - servicespec_validate_add + servicespec_validate_add, ServiceSpecValidationError @pytest.mark.parametrize("test_input,expected, require_network", @@ -31,17 +31,36 @@ def test_parse_host_placement_specs(test_input, expected, require_network): ("count:2", "PlacementSpec(count=2)"), ("3", "PlacementSpec(count=3)"), ("host1 host2", "PlacementSpec(hosts=[HostPlacementSpec(hostname='host1', network='', name=''), HostPlacementSpec(hostname='host2', network='', name='')])"), + ("host1;host2", "PlacementSpec(hosts=[HostPlacementSpec(hostname='host1', network='', name=''), HostPlacementSpec(hostname='host2', network='', name='')])"), + ("host1,host2", "PlacementSpec(hosts=[HostPlacementSpec(hostname='host1', network='', name=''), HostPlacementSpec(hostname='host2', network='', name='')])"), + ("host1 host2=b", "PlacementSpec(hosts=[HostPlacementSpec(hostname='host1', network='', name=''), HostPlacementSpec(hostname='host2', network='', name='b')])"), ("host1=a host2=b", "PlacementSpec(hosts=[HostPlacementSpec(hostname='host1', network='', name='a'), HostPlacementSpec(hostname='host2', network='', name='b')])"), ("host1:1.2.3.4=a host2:1.2.3.5=b", "PlacementSpec(hosts=[HostPlacementSpec(hostname='host1', network='1.2.3.4', name='a'), HostPlacementSpec(hostname='host2', network='1.2.3.5', name='b')])"), + ("myhost:[v1:10.1.1.10:6789]", "PlacementSpec(hosts=[HostPlacementSpec(hostname='myhost', network='[v1:10.1.1.10:6789]', name='')])"), ('2 host1 host2', "PlacementSpec(count=2, hosts=[HostPlacementSpec(hostname='host1', network='', name=''), HostPlacementSpec(hostname='host2', network='', name='')])"), ('label:foo', "PlacementSpec(label='foo')"), ('3 label:foo', "PlacementSpec(count=3, label='foo')"), ('*', "PlacementSpec(host_pattern='*')"), + ('3 data[1-3]', "PlacementSpec(count=3, host_pattern='data[1-3]')"), + ('3 data?', "PlacementSpec(count=3, host_pattern='data?')"), + ('3 data*', "PlacementSpec(count=3, host_pattern='data*')"), ]) def test_parse_placement_specs(test_input, expected): ret = PlacementSpec.from_string(test_input) assert str(ret) == expected +@pytest.mark.parametrize( + "test_input", + [ + ("host=a host*"), + ("host=a label:wrong"), + ("host? host*"), + ] +) +def test_parse_placement_specs_raises(test_input): + with pytest.raises(ServiceSpecValidationError): + PlacementSpec.from_string(test_input) + @pytest.mark.parametrize("test_input", # wrong subnet [("myhost:1.1.1.1/24"),