python-common: add location property to HostSpec, + tests

Signed-off-by: Sage Weil <sage@newdream.net>

src/python-common/ceph/deployment/hostspec.py

@@ -1,7 +1,7 @@
 from collections import OrderedDict
 import errno
 try:
-    from typing import Optional, List, Any
+    from typing import Optional, List, Any, Dict
 except ImportError:
     pass  # just for type checking
 
@@ -23,10 +23,11 @@ class HostSpec(object):
     Information about hosts. Like e.g. ``kubectl get nodes``
     """
     def __init__(self,
-                 hostname,  # type: str
-                 addr=None,  # type: Optional[str]
-                 labels=None,  # type: Optional[List[str]]
-                 status=None,  # type: Optional[str]
+                 hostname: str,
+                 addr: Optional[str] = None,
+                 labels: Optional[List[str]] = None,
+                 status: Optional[str] = None,
+                 location: Optional[Dict[str, str]] = None,
                  ):
         self.service_type = 'host'
 
@@ -42,34 +43,57 @@ class HostSpec(object):
         #: human readable status
         self.status = status or ''  # type: str
 
-    def to_json(self) -> dict:
-        return {
+        self.location = location
+
+    def to_json(self) -> Dict[str, Any]:
+        r: Dict[str, Any] = {
             'hostname': self.hostname,
             'addr': self.addr,
             'labels': list(OrderedDict.fromkeys((self.labels))),
             'status': self.status,
         }
+        if self.location:
+            r['location'] = self.location
+        return r
 
     @classmethod
     def from_json(cls, host_spec: dict) -> 'HostSpec':
         host_spec = cls.normalize_json(host_spec)
-        _cls = cls(host_spec['hostname'],
-                   host_spec['addr'] if 'addr' in host_spec else None,
-                   list(OrderedDict.fromkeys(
-                       host_spec['labels'])) if 'labels' in host_spec else None,
-                   host_spec['status'] if 'status' in host_spec else None)
+        _cls = cls(
+            host_spec['hostname'],
+            host_spec['addr'] if 'addr' in host_spec else None,
+            list(OrderedDict.fromkeys(
+                host_spec['labels'])) if 'labels' in host_spec else None,
+            host_spec['status'] if 'status' in host_spec else None,
+            host_spec.get('location'),
+        )
         return _cls
 
     @staticmethod
     def normalize_json(host_spec: dict) -> dict:
         labels = host_spec.get('labels')
-        if labels is None:
-            return host_spec
-        if isinstance(labels, list):
-            return host_spec
-        if not isinstance(labels, str):
-            raise SpecValidationError(f'Labels ({labels}) must be a string or list of strings')
-        host_spec['labels'] = [labels]
+        if labels is not None:
+            if isinstance(labels, str):
+                host_spec['labels'] = [labels]
+            elif (
+                    not isinstance(labels, list)
+                    or any(not isinstance(v, str) for v in labels)
+            ):
+                raise SpecValidationError(
+                    f'Labels ({labels}) must be a string or list of strings'
+                )
+
+        loc = host_spec.get('location')
+        if loc is not None:
+            if (
+                    not isinstance(loc, dict)
+                    or any(not isinstance(k, str) for k in loc.keys())
+                    or any(not isinstance(v, str) for v in loc.values())
+            ):
+                raise SpecValidationError(
+                    f'Location ({loc}) must be a dictionary of strings to strings'
+                )
+
         return host_spec
 
     def __repr__(self) -> str:
@@ -80,6 +104,8 @@ class HostSpec(object):
             args.append(self.labels)
         if self.status:
             args.append(self.status)
+        if self.location:
+            args.append(self.location)
 
         return "HostSpec({})".format(', '.join(map(repr, args)))
 
@@ -92,4 +118,5 @@ class HostSpec(object):
         # Let's omit `status` for the moment, as it is still the very same host.
         return self.hostname == other.hostname and \
                self.addr == other.addr and \
-               self.labels == other.labels
+               sorted(self.labels) == sorted(other.labels) and \
+               self.location == other.location

src/python-common/ceph/tests/test_hostspec.py

@@ -0,0 +1,40 @@
+# flake8: noqa
+import json
+import yaml
+
+import pytest
+
+from ceph.deployment.hostspec import HostSpec, SpecValidationError
+
+
+@pytest.mark.parametrize(
+    "test_input,expected",
+    [
+        ({"hostname": "foo"}, HostSpec('foo')),
+        ({"hostname": "foo", "labels": "l1"}, HostSpec('foo', labels=['l1'])),
+        ({"hostname": "foo", "labels": ["l1", "l2"]}, HostSpec('foo', labels=['l1', 'l2'])),
+        ({"hostname": "foo", "location": {"rack": "foo"}}, HostSpec('foo', location={'rack': 'foo'})),
+    ]
+)
+def test_parse_host_specs(test_input, expected):
+    hs = HostSpec.from_json(test_input)
+    assert hs == expected
+
+
+@pytest.mark.parametrize(
+    "bad_input",
+    [
+        ({"hostname": "foo", "labels": 124}),
+        ({"hostname": "foo", "labels": {"a", "b"}}),
+        ({"hostname": "foo", "labels": {"a", "b"}}),
+        ({"hostname": "foo", "labels": ["a", 2]}),
+        ({"hostname": "foo", "location": "rack=bar"}),
+        ({"hostname": "foo", "location": ["a"]}),
+        ({"hostname": "foo", "location": {"rack", 1}}),
+        ({"hostname": "foo", "location": {1: "rack"}}),
+    ]
+)
+def test_parse_host_specs(bad_input):
+    with pytest.raises(SpecValidationError):
+        hs = HostSpec.from_json(bad_input)
+