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 <sebastian.wagner@suse.com>

diff --git a/src/pybind/mgr/cephadm/tests/test_scheduling.py b/src/pybind/mgr/cephadm/tests/test_scheduling.py
index 72e1e0c6151c5eecbb96c4cf75275178c7d675d1..2922c99e183128bfb0d4a42ae66c8bf5cf6d0349 100644 (file)
--- 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 d20a4b8b0287e19dee4f8ea4820be0036d43fe36..e65ceaf83dd3ec0e6c9ef16a95b88617e2caf489 100644 (file)
--- 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 83c1864f5656fb4e057d8916b7e4ddc0cb4131b7..4b2c85c4b1fd397a9969a5f5b358d0673cbd7707 100644 (file)
--- 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"),