Repository: typesafehub/conductr-cli

Issue:
Introduce wait behaviour for conduct unload command
The `conduct load`, `conduct run`, and `conduct stop` has its default behaviour (to wait) and `--no-wait` option.

Implement the same behaviour for `conduct unload` command to allow scripting of the stop and uninstallation of bundles within ConductR

Discussion:

Patch:
diff --git a/conductr_cli/bundle_installation.py b/conductr_cli/bundle_installation.py
index 885499a..a97e64a 100644
--- a/conductr_cli/bundle_installation.py
+++ b/conductr_cli/bundle_installation.py
@@ -21,29 +21,45 @@ def count_installations(bundle_id, args):
     return 0
 
 
+def wait_for_uninstallation(bundle_id, args):
+    return wait_for_condition(bundle_id, is_uninstalled, 'uninstalled', args)
+
+
 def wait_for_installation(bundle_id, args):
+    return wait_for_condition(bundle_id, is_installed, 'installed', args)
+
+
+def wait_for_condition(bundle_id, condition, condition_name, args):
     log = logging.getLogger(__name__)
     start_time = datetime.now()
 
     installed_bundles = count_installations(bundle_id, args)
-    if installed_bundles > 0:
-        log.info('Bundle {} is installed'.format(bundle_id))
+    if condition(installed_bundles):
+        log.info('Bundle {} is {}'.format(bundle_id, condition_name))
         return
     else:
-        log.info('Bundle {} waiting to be installed'.format(bundle_id))
+        log.info('Bundle {} waiting to be {}'.format(bundle_id, condition_name))
         bundle_events_url = conduct_url.url('bundles/events', args)
         sse_events = sse_client.get_events(bundle_events_url)
         for event in sse_events:
             elapsed = (datetime.now() - start_time).total_seconds()
             if elapsed > args.wait_timeout:
-                raise WaitTimeoutError('Bundle {} waiting to be installed'.format(bundle_id))
+                raise WaitTimeoutError('Bundle {} waiting to be {}'.format(bundle_id, condition_name))
 
             if event.event and event.event.startswith('bundleInstallation'):
                 installed_bundles = count_installations(bundle_id, args)
-                if installed_bundles > 0:
-                    log.info('Bundle {} installed'.format(bundle_id))
+                if condition(installed_bundles):
+                    log.info('Bundle {} {}'.format(bundle_id, condition_name))
                     return
                 else:
-                    log.info('Bundle {} still waiting to be installed'.format(bundle_id))
+                    log.info('Bundle {} still waiting to be {}'.format(bundle_id, condition_name))
+
+        raise WaitTimeoutError('Bundle {} still waiting to be {}'.format(bundle_id, condition_name))
+
+
+def is_installed(number_of_installations):
+    return number_of_installations > 0
+
 
-        raise WaitTimeoutError('Bundle {} still waiting to be installed'.format(bundle_id))
+def is_uninstalled(number_of_installations):
+    return number_of_installations <= 0
diff --git a/conductr_cli/conduct.py b/conductr_cli/conduct.py
index 7b8c042..1e6f945 100755
--- a/conductr_cli/conduct.py
+++ b/conductr_cli/conduct.py
@@ -205,6 +205,8 @@ def build_parser():
     unload_parser.add_argument('bundle',
                                help='The ID of the bundle')
     add_default_arguments(unload_parser)
+    add_wait_timeout(unload_parser)
+    add_no_wait(unload_parser)
     unload_parser.set_defaults(func=conduct_unload.unload)
 
     # Sub-parser for `events` sub-command
diff --git a/conductr_cli/conduct_unload.py b/conductr_cli/conduct_unload.py
index 5e1f47a..24f7466 100644
--- a/conductr_cli/conduct_unload.py
+++ b/conductr_cli/conduct_unload.py
@@ -1,4 +1,5 @@
-from conductr_cli import conduct_url, validation
+from conductr_cli import conduct_url, validation, bundle_installation
+import json
 import logging
 import requests
 from conductr_cli.http import DEFAULT_HTTP_TIMEOUT
@@ -19,6 +20,14 @@ def unload(args):
         log.verbose(validation.pretty_json(response.text))
 
     log.info('Bundle unload request sent.')
+
+    response_json = json.loads(response.text)
+    if not args.no_wait:
+        bundle_installation.wait_for_uninstallation(response_json['bundleId'], args)
+
     log.info('Print ConductR info with: conduct info{}'.format(args.cli_parameters))
 
+    if not log.is_info_enabled() and log.is_quiet_enabled():
+        log.quiet(response_json['bundleId'])
+
     return True


Test patch:
diff --git a/conductr_cli/test/test_bundle_installation.py b/conductr_cli/test/test_bundle_installation.py
index 567b81a..eb32335 100644
--- a/conductr_cli/test/test_bundle_installation.py
+++ b/conductr_cli/test/test_bundle_installation.py
@@ -8,8 +8,14 @@ except ImportError:
     from mock import call, patch, MagicMock
 
 
+def create_test_event(event_name):
+    sse_mock = MagicMock()
+    sse_mock.event = event_name
+    return sse_mock
+
+
 class TestCountInstallation(CliTestCase):
-    def test_return_scale(self):
+    def test_return_installation_count(self):
         bundles_endpoint_reply = """
             [{
                 "bundleId": "a101449418187d92c789d1adc240b6d6",
@@ -36,7 +42,7 @@ class TestCountInstallation(CliTestCase):
 
         http_method.assert_called_with('http://127.0.0.1:9005/bundles')
 
-    def test_return_scale_v2(self):
+    def test_return_installation_count_v2(self):
         bundles_endpoint_reply = """
             [{
                 "bundleId": "a101449418187d92c789d1adc240b6d6",
@@ -63,7 +69,7 @@ class TestCountInstallation(CliTestCase):
 
         http_method.assert_called_with('http://127.0.0.1:9005/v2/bundles')
 
-    def test_return_zero_v1(self):
+    def test_return_zero_installation_count_v1(self):
         bundles_endpoint_reply = '[]'
         http_method = self.respond_with(text=bundles_endpoint_reply)
 
@@ -79,7 +85,7 @@ class TestCountInstallation(CliTestCase):
 
         http_method.assert_called_with('http://127.0.0.1:9005/bundles')
 
-    def test_return_zero_v2(self):
+    def test_return_zero_installation_count_v2(self):
         bundles_endpoint_reply = '[]'
         http_method = self.respond_with(text=bundles_endpoint_reply)
 
@@ -96,14 +102,14 @@ class TestCountInstallation(CliTestCase):
         http_method.assert_called_with('http://127.0.0.1:9005/v2/bundles')
 
 
-class TestWaitForScale(CliTestCase):
+class TestWaitForInstallation(CliTestCase):
     def test_wait_for_installation(self):
         count_installations_mock = MagicMock(side_effect=[0, 1])
         url_mock = MagicMock(return_value='/bundle-events/endpoint')
         get_events_mock = MagicMock(return_value=[
-            self.create_test_event(None),
-            self.create_test_event('bundleInstallationAdded'),
-            self.create_test_event('bundleInstallationAdded')
+            create_test_event(None),
+            create_test_event('bundleInstallationAdded'),
+            create_test_event('bundleInstallationAdded')
         ])
 
         stdout = MagicMock()
@@ -124,7 +130,6 @@ class TestWaitForScale(CliTestCase):
         ])
 
         url_mock.assert_called_with('bundles/events', args)
-        self.maxDiff = None
 
         self.assertEqual(strip_margin("""|Bundle a101449418187d92c789d1adc240b6d6 waiting to be installed
                                          |Bundle a101449418187d92c789d1adc240b6d6 installed
@@ -154,9 +159,9 @@ class TestWaitForScale(CliTestCase):
         count_installations_mock = MagicMock(side_effect=[0, 1, 1])
         url_mock = MagicMock(return_value='/bundle-events/endpoint')
         get_events_mock = MagicMock(return_value=[
-            self.create_test_event('bundleExecutionAdded'),
-            self.create_test_event('bundleExecutionAdded'),
-            self.create_test_event('bundleExecutionAdded')
+            create_test_event('bundleInstallationAdded'),
+            create_test_event('bundleInstallationAdded'),
+            create_test_event('bundleInstallationAdded')
         ])
 
         stdout = MagicMock()
@@ -185,9 +190,9 @@ class TestWaitForScale(CliTestCase):
         count_installations_mock = MagicMock(return_value=0)
         url_mock = MagicMock(return_value='/bundle-events/endpoint')
         get_events_mock = MagicMock(return_value=[
-            self.create_test_event('bundleInstallationAdded'),
-            self.create_test_event('bundleInstallationAdded'),
-            self.create_test_event('bundleInstallationAdded')
+            create_test_event('bundleInstallationAdded'),
+            create_test_event('bundleInstallationAdded'),
+            create_test_event('bundleInstallationAdded')
         ])
 
         stdout = MagicMock()
@@ -217,7 +222,123 @@ class TestWaitForScale(CliTestCase):
                                          |Bundle a101449418187d92c789d1adc240b6d6 still waiting to be installed
                                          |"""), self.output(stdout))
 
-    def create_test_event(self, event_name):
-        sse_mock = MagicMock()
-        sse_mock.event = event_name
-        return sse_mock
+
+class TestWaitForUninstallation(CliTestCase):
+    def test_wait_for_uninstallation(self):
+        count_installations_mock = MagicMock(side_effect=[1, 0])
+        url_mock = MagicMock(return_value='/bundle-events/endpoint')
+        get_events_mock = MagicMock(return_value=[
+            create_test_event(None),
+            create_test_event('bundleInstallationRemoved'),
+            create_test_event('bundleInstallationRemoved')
+        ])
+
+        stdout = MagicMock()
+
+        bundle_id = 'a101449418187d92c789d1adc240b6d6'
+        args = MagicMock(**{
+            'wait_timeout': 10
+        })
+        with patch('conductr_cli.conduct_url.url', url_mock), \
+                patch('conductr_cli.bundle_installation.count_installations', count_installations_mock), \
+                patch('conductr_cli.sse_client.get_events', get_events_mock):
+            logging_setup.configure_logging(args, stdout)
+            bundle_installation.wait_for_uninstallation(bundle_id, args)
+
+        self.assertEqual(count_installations_mock.call_args_list, [
+            call(bundle_id, args),
+            call(bundle_id, args)
+        ])
+
+        url_mock.assert_called_with('bundles/events', args)
+
+        self.assertEqual(strip_margin("""|Bundle a101449418187d92c789d1adc240b6d6 waiting to be uninstalled
+                                         |Bundle a101449418187d92c789d1adc240b6d6 uninstalled
+                                         |"""), self.output(stdout))
+
+    def test_return_immediately_if_uninstalled(self):
+        count_installations_mock = MagicMock(side_effect=[0])
+
+        stdout = MagicMock()
+
+        bundle_id = 'a101449418187d92c789d1adc240b6d6'
+        args = MagicMock(**{
+            'wait_timeout': 10
+        })
+        with patch('conductr_cli.bundle_installation.count_installations', count_installations_mock):
+            logging_setup.configure_logging(args, stdout)
+            bundle_installation.wait_for_uninstallation(bundle_id, args)
+
+        self.assertEqual(count_installations_mock.call_args_list, [
+            call(bundle_id, args)
+        ])
+
+        self.assertEqual(strip_margin("""|Bundle a101449418187d92c789d1adc240b6d6 is uninstalled
+                                         |"""), self.output(stdout))
+
+    def test_wait_timeout(self):
+        count_installations_mock = MagicMock(side_effect=[1, 1, 1])
+        url_mock = MagicMock(return_value='/bundle-events/endpoint')
+        get_events_mock = MagicMock(return_value=[
+            create_test_event('bundleInstallationAdded'),
+            create_test_event('bundleInstallationAdded'),
+            create_test_event('bundleInstallationAdded')
+        ])
+
+        stdout = MagicMock()
+
+        bundle_id = 'a101449418187d92c789d1adc240b6d6'
+        args = MagicMock(**{
+            # Purposely set no timeout to invoke the error
+            'wait_timeout': 0
+        })
+        with patch('conductr_cli.conduct_url.url', url_mock), \
+                patch('conductr_cli.bundle_installation.count_installations', count_installations_mock), \
+                patch('conductr_cli.sse_client.get_events', get_events_mock):
+            logging_setup.configure_logging(args, stdout)
+            self.assertRaises(WaitTimeoutError, bundle_installation.wait_for_uninstallation, bundle_id, args)
+
+        self.assertEqual(count_installations_mock.call_args_list, [
+            call(bundle_id, args)
+        ])
+
+        url_mock.assert_called_with('bundles/events', args)
+
+        self.assertEqual(strip_margin("""|Bundle a101449418187d92c789d1adc240b6d6 waiting to be uninstalled
+                                         |"""), self.output(stdout))
+
+    def test_wait_timeout_all_events(self):
+        count_installations_mock = MagicMock(return_value=1)
+        url_mock = MagicMock(return_value='/bundle-events/endpoint')
+        get_events_mock = MagicMock(return_value=[
+            create_test_event('bundleInstallationAdded'),
+            create_test_event('bundleInstallationAdded'),
+            create_test_event('bundleInstallationAdded')
+        ])
+
+        stdout = MagicMock()
+
+        bundle_id = 'a101449418187d92c789d1adc240b6d6'
+        args = MagicMock(**{
+            'wait_timeout': 10
+        })
+        with patch('conductr_cli.conduct_url.url', url_mock), \
+                patch('conductr_cli.bundle_installation.count_installations', count_installations_mock), \
+                patch('conductr_cli.sse_client.get_events', get_events_mock):
+            logging_setup.configure_logging(args, stdout)
+            self.assertRaises(WaitTimeoutError, bundle_installation.wait_for_uninstallation, bundle_id, args)
+
+        self.assertEqual(count_installations_mock.call_args_list, [
+            call(bundle_id, args),
+            call(bundle_id, args),
+            call(bundle_id, args),
+            call(bundle_id, args)
+        ])
+
+        url_mock.assert_called_with('bundles/events', args)
+
+        self.assertEqual(strip_margin("""|Bundle a101449418187d92c789d1adc240b6d6 waiting to be uninstalled
+                                         |Bundle a101449418187d92c789d1adc240b6d6 still waiting to be uninstalled
+                                         |Bundle a101449418187d92c789d1adc240b6d6 still waiting to be uninstalled
+                                         |Bundle a101449418187d92c789d1adc240b6d6 still waiting to be uninstalled
+                                         |"""), self.output(stdout))
diff --git a/conductr_cli/test/test_conduct.py b/conductr_cli/test/test_conduct.py
index f0c2141..15df94d 100644
--- a/conductr_cli/test/test_conduct.py
+++ b/conductr_cli/test/test_conduct.py
@@ -60,6 +60,8 @@ class TestConduct(TestCase):
         self.assertEqual(args.resolve_cache_dir, '{}/.conductr/cache'.format(os.path.expanduser('~')))
         self.assertEqual(args.verbose, False)
         self.assertEqual(args.long_ids, False)
+        self.assertEqual(args.no_wait, False)
+        self.assertEqual(args.wait_timeout, 60)
         self.assertEqual(args.bundle, 'path-to-bundle')
         self.assertEqual(args.configuration, 'path-to-conf')
 
@@ -74,6 +76,8 @@ class TestConduct(TestCase):
         self.assertEqual(args.resolve_cache_dir, '/somewhere')
         self.assertEqual(args.verbose, False)
         self.assertEqual(args.long_ids, False)
+        self.assertEqual(args.no_wait, False)
+        self.assertEqual(args.wait_timeout, 60)
         self.assertEqual(args.bundle, 'path-to-bundle')
         self.assertEqual(args.configuration, 'path-to-conf')
         self.assertFalse(args.quiet)
@@ -88,6 +92,8 @@ class TestConduct(TestCase):
         self.assertEqual(args.cli_settings_dir, '{}/.conductr'.format(os.path.expanduser('~')))
         self.assertEqual(args.verbose, False)
         self.assertEqual(args.long_ids, False)
+        self.assertEqual(args.no_wait, False)
+        self.assertEqual(args.wait_timeout, 60)
         self.assertEqual(args.scale, 5)
         self.assertEqual(args.bundle, 'path-to-bundle')
 
@@ -101,6 +107,8 @@ class TestConduct(TestCase):
         self.assertEqual(args.cli_settings_dir, '{}/.conductr'.format(os.path.expanduser('~')))
         self.assertEqual(args.verbose, False)
         self.assertEqual(args.long_ids, False)
+        self.assertEqual(args.no_wait, False)
+        self.assertEqual(args.wait_timeout, 60)
         self.assertEqual(args.bundle, 'path-to-bundle')
 
     def test_parser_unload(self):
@@ -113,6 +121,8 @@ class TestConduct(TestCase):
         self.assertEqual(args.cli_settings_dir, '{}/.conductr'.format(os.path.expanduser('~')))
         self.assertEqual(args.verbose, False)
         self.assertEqual(args.long_ids, False)
+        self.assertEqual(args.no_wait, False)
+        self.assertEqual(args.wait_timeout, 60)
         self.assertEqual(args.bundle, 'path-to-bundle')
 
     def test_get_cli_parameters(self):
diff --git a/conductr_cli/test/test_conduct_unload.py b/conductr_cli/test/test_conduct_unload.py
index b2adc98..92f69d4 100644
--- a/conductr_cli/test/test_conduct_unload.py
+++ b/conductr_cli/test/test_conduct_unload.py
@@ -22,6 +22,7 @@ class TestConductUnloadCommand(CliTestCase):
         'port': 9005,
         'api_version': '1',
         'verbose': False,
+        'no_wait': False,
         'quiet': False,
         'cli_parameters': '',
         'bundle': '45e0c477d3e5ea92aa8d85c0d8f3e25c'
@@ -37,51 +38,101 @@ class TestConductUnloadCommand(CliTestCase):
         return strip_margin(self.output_template.format(**{'params': params}))
 
     def test_success(self):
+        wait_for_uninstallation_mock = MagicMock()
         http_method = self.respond_with(200, self.default_response)
         stdout = MagicMock()
 
-        with patch('requests.delete', http_method):
-            logging_setup.configure_logging(MagicMock(**self.default_args), stdout)
-            result = conduct_unload.unload(MagicMock(**self.default_args))
+        input_args = MagicMock(**self.default_args)
+        with patch('requests.delete', http_method), \
+                patch('conductr_cli.bundle_installation.wait_for_uninstallation', wait_for_uninstallation_mock):
+            logging_setup.configure_logging(input_args, stdout)
+            result = conduct_unload.unload(input_args)
             self.assertTrue(result)
 
         http_method.assert_called_with(self.default_url, timeout=DEFAULT_HTTP_TIMEOUT)
+        wait_for_uninstallation_mock.assert_called_with('45e0c477d3e5ea92aa8d85c0d8f3e25c', input_args)
 
         self.assertEqual(self.default_output(), self.output(stdout))
 
     def test_success_verbose(self):
+        wait_for_uninstallation_mock = MagicMock()
         http_method = self.respond_with(200, self.default_response)
         stdout = MagicMock()
 
-        with patch('requests.delete', http_method), patch('sys.stdout', stdout):
-            args = self.default_args.copy()
-            args.update({'verbose': True})
-            logging_setup.configure_logging(MagicMock(**args), stdout)
-            result = conduct_unload.unload(MagicMock(**args))
+        args = self.default_args.copy()
+        args.update({'verbose': True})
+        input_args = MagicMock(**args)
+
+        with patch('requests.delete', http_method), \
+                patch('conductr_cli.bundle_installation.wait_for_uninstallation', wait_for_uninstallation_mock):
+            logging_setup.configure_logging(input_args, stdout)
+            result = conduct_unload.unload(input_args)
             self.assertTrue(result)
 
         http_method.assert_called_with(self.default_url, timeout=DEFAULT_HTTP_TIMEOUT)
+        wait_for_uninstallation_mock.assert_called_with('45e0c477d3e5ea92aa8d85c0d8f3e25c', input_args)
 
         self.assertEqual(self.default_response + self.default_output(), self.output(stdout))
 
+    def test_success_quiet(self):
+        wait_for_uninstallation_mock = MagicMock()
+        http_method = self.respond_with(200, self.default_response)
+        stdout = MagicMock()
+
+        args = self.default_args.copy()
+        args.update({'quiet': True})
+        input_args = MagicMock(**args)
+
+        with patch('requests.delete', http_method), \
+                patch('conductr_cli.bundle_installation.wait_for_uninstallation', wait_for_uninstallation_mock):
+            logging_setup.configure_logging(input_args, stdout)
+            result = conduct_unload.unload(input_args)
+            self.assertTrue(result)
+
+        http_method.assert_called_with(self.default_url, timeout=DEFAULT_HTTP_TIMEOUT)
+        wait_for_uninstallation_mock.assert_called_with('45e0c477d3e5ea92aa8d85c0d8f3e25c', input_args)
+
+        self.assertEqual('45e0c477d3e5ea92aa8d85c0d8f3e25c\n', self.output(stdout))
+
     def test_success_with_configuration(self):
+        wait_for_uninstallation_mock = MagicMock()
         http_method = self.respond_with(200, self.default_response)
         stdout = MagicMock()
 
+        args = self.default_args.copy()
         cli_parameters = ' --ip 127.0.1.1 --port 9006'
-        with patch('requests.delete', http_method):
-            args = self.default_args.copy()
-            args.update({'cli_parameters': cli_parameters})
-            logging_setup.configure_logging(MagicMock(**args), stdout)
-            result = conduct_unload.unload(MagicMock(**args))
+        args.update({'cli_parameters': cli_parameters})
+        input_args = MagicMock(**args)
+
+        with patch('requests.delete', http_method), \
+                patch('conductr_cli.bundle_installation.wait_for_uninstallation', wait_for_uninstallation_mock):
+            logging_setup.configure_logging(input_args, stdout)
+            result = conduct_unload.unload(input_args)
             self.assertTrue(result)
 
         http_method.assert_called_with(self.default_url, timeout=DEFAULT_HTTP_TIMEOUT)
+        wait_for_uninstallation_mock.assert_called_with('45e0c477d3e5ea92aa8d85c0d8f3e25c', input_args)
 
         self.assertEqual(
             self.default_output(params=cli_parameters),
             self.output(stdout))
 
+    def test_success_no_wait(self):
+        http_method = self.respond_with(200, self.default_response)
+        stdout = MagicMock()
+
+        args = self.default_args.copy()
+        args.update({'no_wait': True})
+        input_args = MagicMock(**args)
+        with patch('requests.delete', http_method):
+            logging_setup.configure_logging(input_args, stdout)
+            result = conduct_unload.unload(input_args)
+            self.assertTrue(result)
+
+        http_method.assert_called_with(self.default_url, timeout=DEFAULT_HTTP_TIMEOUT)
+
+        self.assertEqual(self.default_output(), self.output(stdout))
+
     def test_failure(self):
         http_method = self.respond_with(404)
         stderr = MagicMock()