Merge lp:~frankban/juju-deployer/improve-guiserver-feedback into lp:juju-deployer

[Francesco Banconi]

Improve GUI server feedback handling

Improve how feedback errors are propagated from the deployer to the GUI server.

[Kapil Thangavelu (hazmat)]

looks good, merged.

review: Approve

=== modified file 'deployer/guiserver.py'
--- deployer/guiserver.py	2013-08-29 16:35:45 +0000
+++ deployer/guiserver.py	2014-01-30 10:51:34 +0000
@@ -23,6 +23,36 @@
 JUJU_HOME = '/var/lib/juju-gui/juju-home'
 
 
+class DeploymentError(Exception):
+    """One or more errors occurred during the deployment preparation."""
+
+    def __init__(self, errors):
+        self.errors = errors
+        super(DeploymentError, self).__init__(errors)
+
+    def __str__(self):
+        return '\n'.join(self.errors)
+
+
+class GUIDeployment(Deployment):
+    """Handle bundle deployments requested by the GUI server."""
+
+    def __init__(self, name, data):
+        super(GUIDeployment, self).__init__(name, data, [])
+
+    def _handle_feedback(self, feedback):
+        """Raise a DeploymentError if the given feedback includes errors.
+
+        The GUI server will catch and report failures propagating them through
+        the WebSocket connection to the client.
+        """
+        for message in feedback.get_warnings():
+            self.log.warning(message)
+        if feedback.has_errors:
+            # Errors are logged by the GUI server.
+            raise DeploymentError(feedback.get_errors())
+
+
 def _validate(env, bundle):
     """Bundle validation logic, used by both validate and import_bundle.
 
@@ -55,7 +85,7 @@
 def import_bundle(apiurl, password, name, bundle, options):
     """Import a bundle."""
     env = GUIEnvironment(apiurl, password)
-    deployment = Deployment(name, bundle, [])
+    deployment = GUIDeployment(name, bundle)
     importer = Importer(env, deployment, options)
     env.connect()
     # The Importer tries to retrieve the Juju home from the JUJU_HOME

=== modified file 'deployer/tests/test_guiserver.py'
--- deployer/tests/test_guiserver.py	2014-01-23 14:14:28 +0000
+++ deployer/tests/test_guiserver.py	2014-01-30 10:51:34 +0000
@@ -14,11 +14,53 @@
     cli,
     guiserver,
 )
-from deployer.deployment import Deployment
-
+from deployer.feedback import Feedback
 from deployer.tests.base import TEST_OFFLINE
 
 
+class TestDeploymentError(unittest.TestCase):
+
+    def test_error(self):
+        # A single error is properly stored and represented.
+        exception = guiserver.DeploymentError(['bad wolf'])
+        self.assertEqual(['bad wolf'], exception.errors)
+        self.assertEqual('bad wolf', str(exception))
+
+    def test_multiple_errors(self):
+        # Multiple deployment errors are properly stored and represented.
+        errors = ['error 1', 'error 2']
+        exception = guiserver.DeploymentError(errors)
+        self.assertEqual(errors, exception.errors)
+        self.assertEqual('error 1\nerror 2', str(exception))
+
+
+class TestGUIDeployment(unittest.TestCase):
+
+    def setUp(self):
+        # Set up a GUIDeployment instance and a Feedback object.
+        self.deployment = guiserver.GUIDeployment('mybundle', 'mydata')
+        self.feedback = Feedback()
+
+    def test_valid_deployment(self):
+        # If the bundle is well formed, the deployment proceeds normally.
+        self.assertIsNone(self.deployment._handle_feedback(self.feedback))
+
+    def test_warnings(self):
+        # Warning messages are properly logged.
+        self.feedback.warn('we are the Borg')
+        with mock.patch.object(self.deployment, 'log') as mock_log:
+            self.deployment._handle_feedback(self.feedback)
+        mock_log.warning.assert_called_once_with('we are the Borg')
+
+    def test_errors(self):
+        # A DeploymentError is raised if errors are found in the bundle.
+        self.feedback.error('error 1')
+        self.feedback.error('error 2')
+        with self.assertRaises(guiserver.DeploymentError) as cm:
+            self.deployment._handle_feedback(self.feedback)
+        self.assertEqual(['error 1', 'error 2'], cm.exception.errors)
+
+
 class DeployerFunctionsTestMixin(object):
     """Base set up for the functions that make use of the juju-deployer."""
 
@@ -149,7 +191,7 @@
         # The first argument passed to the importer is the environment.
         self.assertIs(mock_environment(), env)
         # The second argument is the deployment object.
-        self.assertIsInstance(deployment, Deployment)
+        self.assertIsInstance(deployment, guiserver.GUIDeployment)
         self.assertEqual(self.name, deployment.name)
         self.assertEqual(self.bundle, deployment.data)
         # The third and last argument is the options object.
@@ -207,6 +249,32 @@
             mock.call.close(),
         ], any_order=True)
 
+    def test_deployment_errors(self, mock_environment):
+        # A DeploymentError is raised if the deployment fails.
+        bundle = {
+            'services': {
+                'wordpress': {
+                    'charm': 'cs:precise/wordpress-20',
+                    'options': {'no-such-option': 42},  # Invalid options.
+                },
+                'mysql': {
+                    'charm': 'cs:precise/mysql-28',
+                    'options': {'bad': 'wolf'},  # Invalid options.
+                },
+            },
+        }
+        self.addCleanup(self.cleanup_series_path)
+        with self.patch_juju_home():
+            with self.assertRaises(guiserver.DeploymentError) as cm:
+                guiserver.import_bundle(
+                    self.apiurl, self.password, self.name, bundle,
+                    self.options)
+        expected_errors = set([
+            'Invalid config charm cs:precise/wordpress-20 no-such-option=42',
+            'Invalid config charm cs:precise/mysql-28 bad=wolf',
+        ])
+        self.assertEqual(expected_errors, set(cm.exception.errors))
+
     def test_options(self, mock_environment):
         # Ensure the default options are sane for us.
         expected = argparse.Namespace(