Repository: juju/python-libjuju
Pull request #21 (2016-11-28 18:30:03)

Fixed addMachines.
We now accept a single dict called params, which is what we get back
from the planner, rather than expecting expanded arguments.

We do some parsing of constraints, to put them in a format that should
make juju core happy.

@tvansteenburgh @johnsca: This is a partial fix for deploying hadoop processing. It gets us past all the errors when adding machines. We still get an error unmarshalling the instance.Placement object further down the line, but that error does not appear to stem from this part of the code (I'll work on fixing it next).


Patch:
diff --git a/.travis.yml b/.travis.yml
deleted file mode 100644
index b7e82d2..0000000
--- a/.travis.yml
+++ /dev/null
@@ -1,18 +0,0 @@
-dist: trusty
-sudo: required
-language: python
-python:
-  - "3.5"
-before_install:
-  - sudo add-apt-repository ppa:ubuntu-lxc/lxd-stable -y
-  - sudo add-apt-repository ppa:juju/daily -y
-  - sudo apt-get update -q
-  - sudo apt-get install lxd juju -y
-  - sudo usermod -a -G lxd $USER
-  - sudo lxd init --auto
-install: pip install tox-travis
-before_script:
-  - sudo -E sudo -u $USER -E bash -c "juju bootstrap localhost test"
-script: tox
-after_script:
-  - sudo -E sudo -u $USER -E bash -c "juju destroy-controller --destroy-all-models -y test"
diff --git a/examples/leadership.py b/examples/leadership.py
deleted file mode 100644
index c52de60..0000000
--- a/examples/leadership.py
+++ /dev/null
@@ -1,26 +0,0 @@
-"""
-This example:
-
-1. Connects to the current model.
-2. Prints out leadership status for all deployed units in the model.
-3. Cleanly disconnects.
-
-"""
-import asyncio
-
-from juju.model import Model
-
-async def report_leadership():
-    model = Model()
-    await model.connect_current()
-
-    print("Leadership: ")
-    for app in model.applications.values():
-        for unit in app.units:
-            print("{}: {}".format(
-                unit.name, await unit.is_leader_from_status()))
-
-    await model.disconnect()
-
-loop = asyncio.get_event_loop()
-loop.run_until_complete(report_leadership())
diff --git a/juju/action.py b/juju/action.py
index 7a136a7..941fa94 100644
--- a/juju/action.py
+++ b/juju/action.py
@@ -2,9 +2,4 @@ from . import model
 
 
 class Action(model.ModelEntity):
-    @property
-    def status(self):
-        return self.data['status']
-
-    async def wait(self):
-        return await self.model.wait_for_action(self.id)
+    pass
diff --git a/juju/application.py b/juju/application.py
index 5a30235..4a98622 100644
--- a/juju/application.py
+++ b/juju/application.py
@@ -49,10 +49,6 @@ class Application(model.ModelEntity):
         """
         return self.data['status']['message']
 
-    @property
-    def tag(self):
-        return 'application-%s' % self.name
-
     async def add_relation(self, local_relation, remote_relation):
         """Add a relation to another application.
 
@@ -341,6 +337,3 @@ class Application(model.ModelEntity):
 
         """
         pass
-
-    async def get_metrics(self):
-        return await self.model.get_metrics(self.tag)
diff --git a/juju/constraints.py b/juju/constraints.py
new file mode 100644
index 0000000..43c048a
--- /dev/null
+++ b/juju/constraints.py
@@ -0,0 +1,71 @@
+#
+# Module that parses constraints
+#
+# The current version of juju core expects the client to take
+# constraints given in the form "mem=10G foo=bar" and parse them into
+# json that looks like {"mem": 10240, "foo": "bar"}. This module helps us
+# accomplish that task.
+#
+# We do not attempt to duplicate the checking done in
+# client/_client.py:Value here. That class will verify that the
+# constraints keys are valid, and that we can successfully dump the
+# constraints dict to json.
+#
+# Once https://bugs.launchpad.net/juju/+bug/1645402 is addressed, this
+# module should be deprecated.
+#
+
+import re
+
+# Matches on a string specifying memory size
+MEM = re.compile('^[1-9][0-9]*[MGTP]$')
+
+# Multiplication factors to get Megabytes
+FACTORS = {
+    "M": 1,
+    "G": 1024,
+    "T": 1024 * 1024,
+    "P": 1024 * 1024 * 1024
+}
+
+def parse(constraints):
+    """
+    Constraints must be expressed as a string containing only spaces
+    and key value pairs joined by an '='.
+
+    """
+    if constraints is None:
+        return None
+
+    if type(constraints) is dict:
+        # Fowards compatibilty: already parsed
+        return constraints
+
+    constraints = {
+        normalize_key(k): normalize_value(v) for k, v in [
+            s.split("=") for s in constraints.split(" ")]}
+
+    return constraints
+
+
+def normalize_key(key):
+    key = key.strip()
+
+    key = key.replace("-", "_")  # Our _client lib wants "_" in place of "-"
+    return key
+
+
+def normalize_value(value):
+    value = value.strip()
+
+    if MEM.match(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
+
+    return value
diff --git a/juju/controller.py b/juju/controller.py
index f0ef3b9..ca871a1 100644
--- a/juju/controller.py
+++ b/juju/controller.py
@@ -63,32 +63,6 @@ class Controller(object):
 
         return model
 
-    async def destroy_models(self, *args):
-
-        """Destroy a model to this controller.
-
-        :param str : <UUID> of the Model
-        param accepts string of <UUID> only OR `model-<UUID>`
-
-
-        """
-        model_facade = client.ModelManagerFacade()
-        model_facade.connect(self.connection)
-
-        #Generate list of args, pre-pend 'model-'
-        prependarg = list(args)
-        for index, item in enumerate(prependarg):
-            if not item.startswith('model-'):
-                prependarg[index]="model-%s" % item
-
-        #Create list of objects to pass to DestroyModels()
-        arglist = []
-        for arg in prependarg:
-            arglist.append(client.Entity(arg))
-            log.debug('Destroying Model %s', arg)
-
-        await model_facade.DestroyModels(arglist)
-
     def add_user(self, username, display_name=None, acl=None, models=None):
         """Add a user to this controller.
 
diff --git a/juju/model.py b/juju/model.py
index 56924cf..8d6c666 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -12,6 +12,7 @@ from theblues import charmstore
 from .client import client
 from .client import watcher
 from .client import connection
+from .constraints import parse as parse_constraints
 from .delta import get_entity_delta
 from .delta import get_entity_class
 from .exceptions import DeadEntityException
@@ -576,24 +577,15 @@ class Model(object):
         entity_id = await q.get()
         return self.state._live_entity_map(entity_type)[entity_id]
 
-    async def _wait_for_new(self, entity_type, entity_id=None, predicate=None):
+    async def _wait_for_new(self, entity_type, entity_id, predicate=None):
         """Wait for a new object to appear in the Model and return it.
 
         Waits for an object of type ``entity_type`` with id ``entity_id``.
-        If ``entity_id`` is ``None``, it will wait for the first new entity
-        of the correct type.
 
         This coroutine blocks until the new object appears in the model.
 
         """
-        # if the entity is already in the model, just return it
-        if entity_id in self.state._live_entity_map(entity_type):
-            return self.state._live_entity_map(entity_type)[entity_id]
-        # if we know the entity_id, we can trigger on any action that puts
-        # the enitty into the model; otherwise, we have to watch for the
-        # next "add" action on that entity_type
-        action = 'add' if entity_id is None else None
-        return await self._wait(entity_type, entity_id, action, predicate)
+        return await self._wait(entity_type, entity_id, 'add', predicate)
 
     async def wait_for_action(self, action_id):
         """Given an action, wait for it to complete."""
@@ -1206,36 +1198,6 @@ class Model(object):
     def charmstore(self):
         return self._charmstore
 
-    async def get_metrics(self, *tags):
-        """Retrieve metrics.
-
-        :param str \*tags: Tags of entities from which to retrieve metrics.
-            No tags retrieves the metrics of all units in the model.
-        """
-        log.debug("Retrieving metrics for %s",
-                  ', '.join(tags) if tags else "all units")
-
-        metrics_facade = client.MetricsDebugFacade()
-        metrics_facade.connect(self.connection)
-
-        entities = [client.Entity(tag) for tag in tags]
-        metrics_result = await metrics_facade.GetMetrics(entities)
-
-        metrics = collections.defaultdict(list)
-
-        for entity_metrics in metrics_result.results:
-            error = entity_metrics.error
-            if error:
-                if "is not a valid tag" in error:
-                    raise ValueError(error.message)
-                else:
-                    raise Exception(error.message)
-
-            for metric in entity_metrics.metrics:
-                metrics[metric.unit].append(metric.to_json())
-
-        return metrics
-
 
 class BundleHandler(object):
     """
@@ -1294,35 +1256,41 @@ class BundleHandler(object):
         await self.client_facade.AddCharm(None, entity_id)
         return entity_id
 
-    async def addMachines(self, series, constraints, container_type,
-                          parent_id):
-        """
-        :param series string:
-            Series holds the optional machine OS series.
-
-        :param constraints string:
-            Constraints holds the optional machine constraints.
-
-        :param Container_type string:
-            ContainerType optionally holds the type of the container (for
-            instance ""lxc" or kvm"). It is not specified for top level
-            machines.
-
-        :param parent_id string:
-            ParentId optionally holds a placeholder pointing to another machine
-            change or to a unit change. This value is only specified in the
-            case this machine is a container, in which case also ContainerType
-            is set.
-        """
-        params = client.AddMachineParams(
-            series=series,
-            constraints=constraints,
-            container_type=container_type,
-            parent_id=self.resolve(parent_id),
-        )
-        results = await self.client_facade.AddMachines(params)
-        log.debug('Added new machine %s', results[0].machine)
-        return results[0].machine
+    async def addMachines(self, params=None):
+        """
+        :param params dict:
+            Dictionary specifying the machine to add. All keys are optional.
+            Keys include:
+
+            series: string specifying the machine OS series.
+            constraints: string holding machine constraints, if any. We'll
+                parse this into the json friendly dict that the juju api
+                expects.
+            container_type: string holding the type of the container (for
+                instance ""lxc" or kvm"). It is not specified for top level
+                machines.
+            parent_id: string holding a placeholder pointing to another
+                machine change or to a unit change. This value is only
+                specified in the case this machine is a container, in
+                which case also ContainerType is set.
+        """
+        params = params or {}
+
+        if 'parent_id' in params:
+            params['parent_id'] = self.resolve(params['parent_id'])
+
+        params['constraints'] = parse_constraints(
+            params.get('constraints'))
+        params['jobs'] = params.get('jobs', ['JobHostUnits'])
+
+        params = client.AddMachineParams(**params)
+        results = await self.client_facade.AddMachines([params])
+        error = results.machines[0].error
+        if error:
+            raise ValueError("Error adding machine: %s", error.message)
+        machine = results.machines[0].machine
+        log.debug('Added new machine %s', machine)
+        return machine
 
     async def addRelation(self, endpoint1, endpoint2):
         """
@@ -1391,8 +1359,6 @@ class BundleHandler(object):
         # do the do
         log.info('Deploying %s', charm)
         await self.app_facade.Deploy([app])
-        # ensure the app is in the model for future operations
-        await self.model._wait_for_new('application', application)
         return application
 
     async def addUnit(self, application, to):
diff --git a/juju/unit.py b/juju/unit.py
index 8511a47..3dbc1e9 100644
--- a/juju/unit.py
+++ b/juju/unit.py
@@ -1,3 +1,4 @@
+import asyncio
 import logging
 from datetime import datetime
 
@@ -56,10 +57,6 @@ class Unit(model.ModelEntity):
         """
         return self.data['workload-status']['message']
 
-    @property
-    def tag(self):
-        return 'unit-%s' % self.name.replace('/', '-')
-
     def add_storage(self, name, constraints=None):
         """Add unit storage dynamically.
 
@@ -131,39 +128,14 @@ class Unit(model.ModelEntity):
         )
         return await self.model.wait_for_action(res.results[0].action.tag)
 
-    async def run_action(self, action_name, **params):
-        """Run an action on this unit.
+    def run_action(self, action_name, **params):
+        """Run action on this unit.
 
         :param str action_name: Name of action to run
         :param \*\*params: Action parameters
-        :returns: An `juju.action.Action` instance.
-
-        Note that this only enqueues the action.  You will need to call
-        ``action.wait()`` on the resulting `Action` instance if you wish
-        to block until the action is complete.
-        """
-        action_facade = client.ActionFacade()
-        action_facade.connect(self.connection)
-
-        log.debug('Starting action `%s` on %s', action_name, self.name)
-
-        res = await action_facade.Enqueue([client.Action(
-            name=action_name,
-            parameters=params,
-            receiver=self.tag,
-        )])
-        action = res.results[0].action
-        error = res.results[0].error
-        if error and error.code == 'not found':
-            raise ValueError('Action `%s` not found on %s' % (action_name,
-                                                              self.name))
-        elif error:
-            raise Exception('Unknown action error: %s' % error.serialize())
-        action_id = action.tag[len('action-'):]
-        log.debug('Action started as %s', action_id)
-        # we mustn't use wait_for_action because that blocks until the
-        # action is complete, rather than just being in the model
-        return await self.model._wait_for_new('action', action_id)
+
+        """
+        pass
 
     def scp(
             self, source_path, user=None, destination_path=None, proxy=False,
@@ -206,39 +178,3 @@ class Unit(model.ModelEntity):
 
         """
         pass
-
-    async def is_leader_from_status(self):
-        """
-        Check to see if this unit is the leader. Returns True if so, and
-        False if it is not, or if leadership does not make sense
-        (e.g., there is no leader in this application.)
-
-        This method is a kluge that calls FullStatus in the
-        ClientFacade to get its information. Once
-        https://bugs.launchpad.net/juju/+bug/1643691 is resolved, we
-        should add a simple .is_leader property, and deprecate this
-        method.
-
-        """
-        app = self.name.split("/")[0]
-
-        c = client.ClientFacade()
-        c.connect(self.model.connection)
-
-        status = await c.FullStatus(None)
-
-        try:
-            return status.applications[app]['units'][self.name].get(
-                'leader', False)
-        except KeyError:
-            # FullStatus may be more up-to-date than the model
-            # referenced by this class. If this unit has been
-            # destroyed between the time the class was created and the
-            # time that we call this method, we'll get a KeyError. In
-            # that case, we simply return False, as a destroyed unit
-            # is not a leader.
-            return False
-
-    async def get_metrics(self):
-        metrics = await self.model.get_metrics(self.tag)
-        return metrics[self.name]


Test patch:
diff --git a/tests/unit/test_constraints.py b/tests/unit/test_constraints.py
new file mode 100644
index 0000000..ec50bdd
--- /dev/null
+++ b/tests/unit/test_constraints.py
@@ -0,0 +1,45 @@
+#
+# Test our constraints parser
+#
+
+import unittest
+
+from juju import constraints
+
+class TestConstraints(unittest.TestCase):
+
+    def test_mem_regex(self):
+        m = constraints.MEM
+        self.assertTrue(m.match("10G"))
+        self.assertTrue(m.match("1G"))
+        self.assertFalse(m.match("1Gb"))
+        self.assertFalse(m.match("a1G"))
+        self.assertFalse(m.match("1000"))
+
+    def test_normalize_key(self):
+        _ = constraints.normalize_key
+
+        self.assertEqual(_("test-key"), "test_key")
+        self.assertEqual(_("test-key  "), "test_key")
+        self.assertEqual(_("  test-key"), "test_key")
+
+    def test_normalize_val(self):
+        _ = constraints.normalize_value
+
+        self.assertEqual(_("10G"), 10 * 1024)
+        self.assertEqual(_("10M"), 10)
+        self.assertEqual(_("10"), "10")
+        self.assertEqual(_("foo,bar"), ["foo", "bar"])
+
+    def test_parse_constraints(self):
+        _ = constraints.parse
+
+        self.assertEqual(
+            _("mem=10G"),
+            {"mem": 10 * 1024}
+        )
+
+        self.assertEqual(
+            _("mem=10G foo=bar,baz"),
+            {"mem": 10 * 1024, "foo": ["bar", "baz"]}
+        )