Repository: juju/python-libjuju

Issue:
Single tag constraint causes JujuAPIError
When a constraint spec is given to deploy or in a bundle with a single tag constraint (e.g., `tags=node1234` rather than `tags=node1234,othertag`) it causes the following exception:

```
Traceback (most recent call last):
  File "/snap/conjure-up/987/lib/python3.6/site-packages/conjureup/controllers/deploy/common.py", line 24, in do_deploy
    await app.juju.client.deploy(fn)
  File "/snap/conjure-up/987/lib/python3.6/site-packages/juju/model.py", line 1186, in deploy
    await handler.execute_plan()
  File "/snap/conjure-up/987/lib/python3.6/site-packages/juju/model.py", line 1780, in execute_plan
    result = await method(*step.args)
  File "/snap/conjure-up/987/lib/python3.6/site-packages/juju/model.py", line 1927, in deploy
    storage=storage,
  File "/snap/conjure-up/987/lib/python3.6/site-packages/juju/model.py", line 1299, in _deploy
    result = await app_facade.Deploy([app])
  File "/snap/conjure-up/987/lib/python3.6/site-packages/juju/client/facade.py", line 412, in wrapper
    reply = await f(*args, **kwargs)
  File "/snap/conjure-up/987/lib/python3.6/site-packages/juju/client/_client5.py", line 503, in Deploy
    reply = await self.rpc(msg)
  File "/snap/conjure-up/987/lib/python3.6/site-packages/juju/client/facade.py", line 537, in rpc
    result = await self.connection.rpc(msg, encoder=TypeEncoder)
  File "/snap/conjure-up/987/lib/python3.6/site-packages/juju/client/connection.py", line 314, in rpc
    raise errors.JujuAPIError(result)
juju.errors.JujuAPIError: json: cannot unmarshal string into Go struct field Value.tags of type []string
```

This is due to the constraints parsing [only creating a list if there is a comma](https://github.com/juju/python-libjuju/blob/master/juju/constraints.py#L75) whereas it needs to happen always for `tags` and `spaces`.

Discussion:

Patch:
diff --git a/.travis.yml b/.travis.yml
index 0e907f0..9a1fcce 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -3,28 +3,29 @@ sudo: required
 language: python
 python:
   - "3.6"
-before_script:
-  - sudo addgroup lxd || true
-  - sudo usermod -a -G lxd $USER || true
-  - sudo ln -s /snap/bin/juju /usr/bin/juju
-  - sudo ln -s /snap/bin/lxc /usr/bin/lxc
 before_install:
   - sudo add-apt-repository -y ppa:jonathonf/python-3.6
   - sudo add-apt-repository ppa:chris-lea/libsodium -y
   - sudo apt-get update -q
   - sudo apt-get remove -qy lxd lxd-client
   - sudo apt-get install snapd libsodium-dev -y
-  - sudo snap install lxd || true
+install:
+  - sudo snap install lxd || true  # ignore failures so that unit tests will still run, at least
+  - sudo snap install juju --classic --$JUJU_CHANNEL || true
   - sudo snap install juju-wait --classic || true
-install: pip install tox-travis
+  - pip install tox-travis
 env:
   global: >
     TEST_AGENTS='{"agents":[{"url":"https://api.staging.jujucharms.com/identity","username":"libjuju-ci@yellow"}],"key":{"private":"88OOCxIHQNguRG7zFg2y2Hx5Ob0SeVKKBRnjyehverc=","public":"fDn20+5FGyN2hYO7z0rFUyoHGUnfrleslUNtoYsjNSs="}}'
+    PATH="/snap/bin:$PATH"
   matrix:
     - JUJU_CHANNEL=stable
     - JUJU_CHANNEL=edge
+before_script:
+  - sudo bash -c 'for i in 5 10 15 30; do [[ -e /var/snap/lxd/common/lxd/unix.socket ]] && break; sleep $i; done'
+  - sudo lxd init --auto || true
+  - sudo addgroup lxd || true
+  - sudo usermod -a -G lxd $USER || true
 script:
-  - sudo snap install juju --classic --$JUJU_CHANNEL
-  - sudo ln -s /snap/bin/juju /usr/bin/juju || true
-  - sudo -E sudo -u $USER -E /snap/bin/juju bootstrap localhost test --config 'identity-url=https://api.staging.jujucharms.com/identity' --config 'allow-model-access=true'
+  - sudo -E sudo -u $USER -E juju bootstrap localhost test --config 'identity-url=https://api.staging.jujucharms.com/identity' --config 'allow-model-access=true'
   - tox -e py35,integration
diff --git a/juju/constraints.py b/juju/constraints.py
index 998862d..0050673 100644
--- a/juju/constraints.py
+++ b/juju/constraints.py
@@ -29,6 +29,8 @@ FACTORS = {
     "P": 1024 * 1024 * 1024
 }
 
+LIST_KEYS = {'tags', 'spaces'}
+
 SNAKE1 = re.compile(r'(.)([A-Z][a-z]+)')
 SNAKE2 = re.compile('([a-z0-9])([A-Z])')
 
@@ -47,8 +49,10 @@ def parse(constraints):
         return constraints
 
     constraints = {
-        normalize_key(k): normalize_value(v) for k, v in [
-            s.split("=") for s in constraints.split(" ")]}
+        normalize_key(k): (
+            normalize_list_value(v) if k in LIST_KEYS else
+            normalize_value(v)
+        ) for k, v in [s.split("=") for s in constraints.split(" ")]}
 
     return constraints
 
@@ -72,13 +76,12 @@ def normalize_value(value):
         # Translate aliases to Megabytes. e.g. 1G = 10240
         return int(value[:-1]) * FACTORS[value[-1:]]
 
-    if "," in value:
-        # Handle csv strings.
-        values = value.split(",")
-        values = [normalize_value(v) for v in values]
-        return values
-
     if value.isdigit():
         return int(value)
 
     return value
+
+
+def normalize_list_value(value):
+    values = value.strip().split(',')
+    return [normalize_value(value) for value in values]


Test patch:
diff --git a/tests/unit/test_constraints.py b/tests/unit/test_constraints.py
index 00b9156..3c52090 100644
--- a/tests/unit/test_constraints.py
+++ b/tests/unit/test_constraints.py
@@ -32,6 +32,12 @@ class TestConstraints(unittest.TestCase):
         self.assertEqual(_("10G"), 10 * 1024)
         self.assertEqual(_("10M"), 10)
         self.assertEqual(_("10"), 10)
+        self.assertEqual(_("foo,bar"), "foo,bar")
+
+    def test_normalize_list_val(self):
+        _ = constraints.normalize_list_value
+
+        self.assertEqual(_("foo"), ["foo"])
         self.assertEqual(_("foo,bar"), ["foo", "bar"])
 
     def test_parse_constraints(self):
@@ -43,6 +49,9 @@ class TestConstraints(unittest.TestCase):
         )
 
         self.assertEqual(
-            _("mem=10G foo=bar,baz"),
-            {"mem": 10 * 1024, "foo": ["bar", "baz"]}
+            _("mem=10G foo=bar,baz tags=tag1 spaces=space1,space2"),
+            {"mem": 10 * 1024,
+             "foo": "bar,baz",
+             "tags": ["tag1"],
+             "spaces": ["space1", "space2"]}
         )