From: Sage Weil <sage@newdream.net>
Date: Wed, 10 Mar 2021 22:31:31 +0000 (-0500)
Subject: python-common: count-per-host must be combined with label or hosts or host_pattern
X-Git-Tag: v16.2.0~106^2~29
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=931a454673fc3249726fb7784affa57533fdada4;p=ceph.git

python-common: count-per-host must be combined with label or hosts or host_pattern

I think this is better for the same reason we made PlacementSpec() not
mean 'all hosts' by default.  If you really want N daemons for every host
in the cluster, be specific with 'count-per-host:2 *'.

Signed-off-by: Sage Weil <sage@newdream.net>
(cherry picked from commit c7e0fb1e8e7cb06097c23d9e1643b6ba852f0eb0)
---

diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py
index 2a954f6d313f..e3241fcd8ae2 100644
--- a/src/python-common/ceph/deployment/service_spec.py
+++ b/src/python-common/ceph/deployment/service_spec.py
@@ -210,15 +210,12 @@ class PlacementSpec(object):
         if self.hosts:
             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:
+        if self.label:
             return [hs.hostname for hs in hostspecs if self.label in hs.labels]
-        elif self.host_pattern:
-            all_hosts = [hs.hostname for hs in hostspecs]
+        all_hosts = [hs.hostname for hs in hostspecs]
+        if self.host_pattern:
             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 []
+        return all_hosts
 
     def get_host_selection_size(self, hostspecs: Iterable[HostSpec]) -> int:
         if self.count:
@@ -293,6 +290,14 @@ class PlacementSpec(object):
             raise ServiceSpecValidationError("num/count must be > 1")
         if self.count_per_host is not None and self.count_per_host < 1:
             raise ServiceSpecValidationError("count-per-host must be >= 1")
+        if self.count_per_host is not None and not (
+                self.label
+                or self.hosts
+                or self.host_pattern
+        ):
+            raise ServiceSpecValidationError(
+                "count-per-host must be combined with label or hosts or host_pattern"
+            )
         if self.count is not None and self.count_per_host is not None:
             raise ServiceSpecValidationError("cannot combine count and count-per-host")
         if (
diff --git a/src/python-common/ceph/tests/test_service_spec.py b/src/python-common/ceph/tests/test_service_spec.py
index 8cbadcf8c8a3..8422d1805468 100644
--- a/src/python-common/ceph/tests/test_service_spec.py
+++ b/src/python-common/ceph/tests/test_service_spec.py
@@ -62,7 +62,7 @@ def test_parse_host_placement_specs(test_input, expected, require_network):
         ('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*')"),
-        ("count-per-host:4", "PlacementSpec(count_per_host=4)"),
+        ("count-per-host:4 label:foo", "PlacementSpec(count_per_host=4, label='foo')"),
     ])
 def test_parse_placement_specs(test_input, expected):
     ret = PlacementSpec.from_string(test_input)
@@ -80,6 +80,7 @@ def test_parse_placement_specs(test_input, expected):
         ('count:2 count-per-host:1'),
         ('host1=a host2=b count-per-host:2'),
         ('host1:10/8 count-per-host:2'),
+        ('count-per-host:2'),
     ]
 )
 def test_parse_placement_specs_raises(test_input):