Addressed PR comments.feature/conjure-up-support

conjure-up deploy now works via automagic. 

5 files changed, 60 insertions, 53 deletions

diff --git a/matrix/main.py b/matrix/main.py
index 8e65939..a2ae546 100644
--- a/matrix/main.py
+++ b/matrix/main.py
@@ -132,13 +132,6 @@ def setup(matrix, args=None):
 parser.add_argument("-k", "--keep-models", action="store_true",
 default=False, help="Keep the created per-test models "
 "for further inspection")
- parser.add_argument("-r", "--deployer", default="python-libjuju",
- help="Method to use to deploy charms. Defaults to "
- "'python-libjuju'. Other options include 'congure-up'."
- " Note that if you choose conjure-up, --path "
- "should point to a conjure-up spell "
- "(with metadata.yaml) "
- "instead of to a vanilla juju bundle.")
 parser.add_argument("-l", "--log-level", default=None,
 help="Set log level.")
 parser.add_argument("-L", "--log-name", nargs="*",
diff --git a/matrix/model.py b/matrix/model.py
index 34e32c4..98c7583 100644
--- a/matrix/model.py
+++ b/matrix/model.py
@@ -27,6 +27,11 @@ class TestFailure(Exception):
 self.message = message or "Test Failure"
 
 
+class InfraFailure(Exception):
+ "Indicate that we have run into an unexpected error while running a test."
+ pass
+
+
 @attr.s
 class Event:
 """A local or remote event tied to the context timeline."""
diff --git a/matrix/tasks/deploy.py b/matrix/tasks/deploy.py
index 68d0451..6659de2 100644
--- a/matrix/tasks/deploy.py
+++ b/matrix/tasks/deploy.py
@@ -1,7 +1,11 @@
-from matrix.utils import execute_process
-from matrix.model import TestFailure
-
 import yaml
+from distutils.spawn import find_executable
+from pathlib import Path
+
+from juju import client
+
+from matrix.model import InfraFailure
+from matrix.utils import execute_process
 
 
 async def libjuju(context, rule):
@@ -12,31 +16,39 @@ async def libjuju(context, rule):
 await context.juju_model.deploy(str(context.config.path))
 
 
-async def get_controller_name(controller_id, log):
+async def get_controller_name(controller_id, context, log):
 if controller_id.startswith('controller-'):
 controller_id = controller_id[11:]
 
- success, controllers, _ = await execute_process(
- ["juju", "list-controllers", "--format", "yaml"],
- log
- )
- if not success:
- raise TestFailure("Unable to get controllers.")
-
- controllers = yaml.safe_load(controllers.decode('utf8'))
- controllers = controllers.get('controllers')
+ controllers = client.connection.JujuData().controllers()
 
 if not controllers:
- raise TestFailure("Unable to get controllers.")
+ raise InfraFailure("Unable to get controllers.")
 
 for controller in controllers:
 if controllers[controller]['uuid'] == controller_id:
 return controller
 
- raise TestFailure(
+ raise InfraFailure(
 "Unable to find controller {}".format(controller_id))
 
 
+def is_spell(bundle):
+ """
+ Determine whether a bundle is a conjure-up friendly spell or not.
+
+ Currently, something is a spell if it has a metadata.yaml file
+ with a 'friendly-name' key in it.
+
+ """
+ metadata_yaml = Path(bundle, 'metadata.yaml')
+ if not metadata_yaml.exists():
+ return False
+
+ with metadata_yaml.open('r') as metadata:
+ return yaml.safe_load(metadata).get('friendly-name')
+
+
 async def conjureup(context, rule):
 """
 Assuming that we've been passed a spell in place of a bundle,
@@ -46,6 +58,7 @@ async def conjureup(context, rule):
 cloud = await context.juju_controller.get_cloud()
 controller_name = await get_controller_name(
 context.juju_controller.connection.info['controller-tag'],
+ context,
 rule.log
 )
 cmd = [
@@ -58,26 +71,25 @@ async def conjureup(context, rule):
 
 success, _, err = await execute_process(cmd, rule.log)
 if not success:
- raise TestFailure("Unable to execute conjure-up: {}".format(err))
+ raise InfraFailure("Unable to execute conjure-up: {}".format(err))
 
 
-DEPLOYERS = {
- 'python-libjuju': libjuju,
- 'libjuju': libjuju,
- 'pythonlibjuju': libjuju,
- 'conjure-up': conjureup,
- 'conjureup': conjureup
-}
+async def deploy(context, rule, task, event=None):
+ """
+ Determine what method to use to deploy the bundle specified in
+ context.config.path, and deploy it via the appropriate method.
 
+ This routine assumes that the bundle path points to a local
+ directory; matrix currently does not have support for deploying
+ remotely stored bundles.
+
+ """
+ if find_executable('conjure-up') and is_spell(context.config.path):
+ rule.log.info("Deploying %s via conjure-up", context.config.path)
+ await conjureup(context, rule)
+ else:
+ rule.log.info("Deploying %s via python-libjuju", context.config.path)
+ await libjuju(context, rule)
 
-async def deploy(context, rule, task, event=None):
- rule.log.info(
- "Deploying %s via %s", context.config.path, context.config.deployer)
- try:
- deployer = DEPLOYERS[context.config.deployer]
- except KeyError:
- raise TestFailure(
- "Could not find deployer '{}'".format(context.config.deployer))
- await deployer(context, rule)
 rule.log.info("Deploy COMPLETE")
 return True
diff --git a/tests/deploy/test_deploy.py b/tests/deploy/test_deploy.py
index aa0eecb..8d8fed0 100644
--- a/tests/deploy/test_deploy.py
+++ b/tests/deploy/test_deploy.py
@@ -3,7 +3,7 @@ import pytest
 
 from pkg_resources import resource_filename
 
-from matrix.tasks.deploy import libjuju, conjureup, DEPLOYERS
+from matrix.tasks.deploy import libjuju
 from matrix.model import Context
 from matrix import rules
 
@@ -34,11 +34,3 @@ async def test_libjuju(event_loop):
 await libjuju(context, None)
 
 mock_deploy.assert_called_once_with("foo")
-
-
-def test_deployer_constant():
- for key in DEPLOYERS:
- if "conjure" in key:
- assert DEPLOYERS[key] == conjureup
- if "libjuju" in key:
- assert DEPLOYERS[key] == libjuju
diff --git a/tests/functional/spell_test.py b/tests/functional/spell_test.py
index 16260b6..abcdb48 100644
--- a/tests/functional/spell_test.py
+++ b/tests/functional/spell_test.py
@@ -1,10 +1,12 @@
-from distutils.spawn import find_executable
 import subprocess
 import unittest
+from distutils.spawn import find_executable
+from pathlib import Path
 
 from .harness import Harness
 
 
+@unittest.skipIf(not find_executable("conjure-up"), 'Test requires conjure-up')
 class TestBasicSpell(Harness):
 '''
 Verify that we can run the default matrix suite on the basic
@@ -20,14 +22,17 @@ class TestBasicSpell(Harness):
 '-s', 'raw',
 '-p', 'tests/basic-spell',
 '-d', self.tmpdir,
- '-r', 'conjure-up',
 '-D',
 ]
 
 def test_basic_spell(self):
 subprocess.run(self.cmd + ['tests/test_deploy.matrix'], timeout=1000)
 self.check_artifacts(1) # matrix.log
+ with Path(self.tmpdir, self.artifacts[0]).open() as matrix_log:
+ deployed_with_conjure = False
+ for line in matrix_log.readlines():
+ if "Deploying tests/basic-spell via conjure-up" in line:
+ deployed_with_conjure = True
+ break
 
- if not find_executable("conjure-up"):
- test_basic_spell = unittest.skip("Conjure up not installed")(
- test_basic_spell)
+ self.assertTrue(deployed_with_conjure)