Merge lp:~mskalka/juju-ci-tools/better-expose-test into lp:juju-ci-tools

[Michael Skalka]

Fixes the expose test to actually test for exposed units. If there are exposed units in the deployment this runs through and deploys a network-health charm under every application, aliased as network-health-<application name> and exposes the network-health charms whos parent charm is also exposed. Then it hits each of the network-health charms with curl to its public address on port 8039 and collects the results. Finally if the expose test is run it cleans up the deployed charms to keep the goal of running under an existing deployment.

This also adds the WaitApplicationNotPresent class into jujupy, allowing jujupy to wait for an application to be removed before continuing.

[lp:~mskalka/juju-ci-tools/better-expose-test]

1944. By Michael Skalka on 2017-03-20

moved early expose test return to start of function

[Curtis Hovey (sinzui)]

Thank you. We need a test for WaitApplicationNotPresent added to test_client(). I have a question inline about ensure_exposed.

review: Needs Information (code)

[lp:~mskalka/juju-ci-tools/better-expose-test]

1945. By Michael Skalka on 2017-03-20

WaitApplicationNotPresent unittest

1946. By Michael Skalka on 2017-03-21

cleanup, logging

1947. By Michael Skalka on 2017-03-21

changed expose test logic to always run

[Curtis Hovey (sinzui)]

Thank you. We agreed you will add a comment to setup_expose_test() to explain why the network-health charms are removed and redeployed. You can merge once the comment is added.

review: Approve (code)

[lp:~mskalka/juju-ci-tools/better-expose-test]

1948. By Michael Skalka on 2017-03-21

added comment to setup_expose test

=== modified file 'assess_network_health.py'
--- assess_network_health.py	2017-03-17 14:19:36 +0000
+++ assess_network_health.py	2017-03-21 16:28:48 +0000
@@ -15,7 +15,10 @@
 from collections import defaultdict
 
 from jujupy import (
-    client_for_existing,
+    client_for_existing
+    )
+from jujupy.client import (
+    WaitApplicationNotPresent
     )
 from deploy_stack import (
     BootstrapManager
@@ -36,6 +39,8 @@
 
 NO_EXPOSED_UNITS = 'No exposed units'
 
+PORT = 8039
+
 
 class AssessNetworkHealth:
 
@@ -47,6 +52,7 @@
                             args.temp_env_name)
         self.expose_client = None
         self.existing_series = set([])
+        self.expose_test_charms = set([])
 
     def assess_network_health(self, client, bundle=None, target_model=None,
                               reboot=False, series=None, maas=None):
@@ -102,15 +108,14 @@
                                         json.dumps(vis_result,
                                                    indent=4,
                                                    sort_keys=True)))
-        exp_result = None
-        if not target_model:
-            exp_result = self.ensure_exposed(client, series)
-            log.info('{0}Exposure '
-                     'result:\n {1}'.format(reboot_msg,
-                                            json.dumps(exp_result,
-                                                       indent=4,
-                                                       sort_keys=True)) or
-                     NO_EXPOSED_UNITS)
+
+        exp_result = self.ensure_exposed(client, series)
+        log.info('{0}Exposure '
+                 'result:\n {1}'.format(reboot_msg,
+                                        json.dumps(exp_result,
+                                                   indent=4,
+                                                   sort_keys=True)) or
+                 NO_EXPOSED_UNITS)
         log.info('Tests complete.')
         return self.parse_final_results(vis_result, int_result,
                                         exp_result)
@@ -194,14 +199,15 @@
 
     def cleanup(self, client):
         log.info('Cleaning up deployed test charms and models.')
+        if self.expose_test_charms:
+            for charm in self.expose_test_charms:
+                client.remove_service(charm)
+            return
         for series in self.existing_series:
             client.remove_service('network-health-{}'.format(series))
-        if 'exposetest' in client.get_models().keys():
-            client.get_models()['exposetest'].destroy_model()
-        log.info('Cleanup complete.')
 
     def get_unit_info(self, client):
-        """Gets the machine or container interface and dns info.
+        """Gets the machine or container interface info.
 
         :param client: Client to get results from
         :return: Dict of machine results as
@@ -294,10 +300,13 @@
                 for ip in target_ips:
                     result[app][unit][ip] = False
                     pattern = r"(pass)"
-                    out = client.run(['curl {}:80'.format(ip)],
+                    log.info('Attempting to contact {}:{} '
+                             'from {}'.format(ip, PORT, unit))
+                    out = client.run(['curl {}:{}'.format(ip, PORT)],
                                      units=[unit])
                     match = re.search(pattern, json.dumps(out[0]))
                     if match:
+                        log.info('pass')
                         result[app][unit][ip] = True
         return result
 
@@ -308,38 +317,92 @@
         :return: Exposure test results in dict by pass/fail
         """
         log.info('Starting test of exposed units.')
+
         apps = client.get_status().get_applications()
-        targets = self.parse_targets(client.get_status())
-        exposed = [app for app, e in apps.items() if e.get('exposed') is True]
+        exposed = [app for app, e in apps.items() if e.get('exposed')
+                   is True and 'network-health' not in app]
         if len(exposed) is 0:
-            log.info('No exposed units, aboring test.')
-            return None
-        new_client = self.setup_expose_test(client, series)
+            nh_only = True
+            log.info('No exposed units, testing with network-health '
+                     'charms only.')
+        else:
+            nh_only = False
+            self.setup_expose_test(client, series, exposed)
+
         service_results = {}
-        for service, units in targets.items():
-            service_results[service] = self.ping_units(new_client,
-                                                       'network-health/0',
-                                                       units)
+        for unit, info in client.get_status().iter_units():
+            ip = info['public-address']
+            if nh_only and 'network-health' in unit:
+                service_results[unit] = self.curl(ip)
+            elif not nh_only and 'network-health' not in unit:
+                service_results[unit] = self.curl(ip)
         log.info(service_results)
         return self.parse_expose_results(service_results, exposed)
 
-    def setup_expose_test(self, client, series):
-        """Sets up new model to run exposure test.
-
-        :param client: The juju client in use
-        :return: New juju client object
-        """
-        if not self.expose_client:
-            new_client = client.add_model('exposetest')
-            new_client.deploy('ubuntu', series=series)
-            new_client.deploy('~juju-qa/network-health', series=series)
-            new_client.wait_for_started()
-            new_client.wait_for_workloads()
-            new_client.juju('add-relation', ('ubuntu', 'network-health'))
-            new_client.wait_for_subordinate_units('ubuntu', 'network-health')
-            self.expose_client = new_client
-
-        return self.expose_client
+    def curl(self, ip):
+        log.info('Attempting to curl unit at {}:{}'.format(ip, PORT))
+        try:
+            out = subprocess.check_output(
+                'curl {}:{} -m 5'.format(ip, PORT), shell=True)
+        except subprocess.CalledProcessError as e:
+            out = ''
+            log.warning('Curl failed for error:\n{}'.format(e))
+        log.info('Got: "{}" from unit at {}:{}'.format(out, ip, PORT))
+        if 'pass' in out:
+            return True
+        return False
+
+    def setup_expose_test(self, client, series, exposed):
+        """Sets up the expose test using aliased NH charms.
+
+        :param client: juju client object used in the test.
+        :param series: Charm series
+        :param exposed: List of exposed charms
+        """
+
+        log.info('Removing previous network-health charms')
+
+        """
+        This is done to work with the beahvior used in other network-health
+        tests to circumvent Juju's lack of support for multi-series charms.
+        If a multi-series subordinate is deployed under one of its available
+        series, then a second copy of that charm in a differnt series cannot
+        also be deployed. Subsequently, when we deploy the NH charms for the
+        above tests, the series is appended to the end of the charm. In order
+        for the expose test to work properly the NH charm has to be exposed,
+        which in Juju means all of the NH charms under that alias or none.
+        So if there are existing exposed units, the test redeploys an aliased
+        NH charm under each so that it can expose them individually, ensuring
+        valid test results.
+        On the subject of speed, since the deps in network-health's wheelhouse
+        have already been built on the target machine or container, this is a
+        relatively fast process at ~30 seconds for large(6+ charm) deployments.
+        """
+        for series in self.existing_series:
+            alias = 'network-health-{}'.format(series)
+            client.remove_service(alias)
+        for series in self.existing_series:
+            alias = 'network-health-{}'.format(series)
+            client.wait_for(WaitApplicationNotPresent(alias))
+        log.info('Deploying aliased network-health charms')
+        apps = client.get_status().get_applications()
+        for app, info in apps.items():
+            if 'network-health' not in app:
+                alias = 'network-health-{}'.format(app)
+                client.deploy('~juju-qa/network-health', alias=alias,
+                              series=info['series'])
+                try:
+                    client.juju('add-relation', (app, alias))
+                    self.expose_test_charms.add(alias)
+                except subprocess.CalledProcessError as e:
+                    log.warning('Could not relate {}, {} due to '
+                                'error:\n{}'.format(app, alias, e))
+        for app in apps.keys():
+            if 'network-health' not in app:
+                client.wait_for_subordinate_units(
+                    app, 'network-health-{}'.format(app))
+        for app in exposed:
+            client.juju('expose', ('network-health-{}'.format(app)))
 
     def parse_expose_results(self, service_results, exposed):
         """Parses expose test results into dict of pass/fail.
@@ -347,21 +410,19 @@
         :param service_results: Raw results from expose test
         :return: Parsed results dict
         """
-        result = {'fail': (),
-                  'pass': ()}
-        for service, results in service_results.items():
-            # If we could connect but shouldn't, fail
-            if 'True' in results and service not in exposed:
-                result['fail'] = result['fail'] + (service,)
-            # If we could connect but should, pass
-            elif 'True' in results and service in exposed:
-                result['pass'] = result['pass'] + (service,)
-            # If we couldn't connect and shouldn't, pass
-            elif 'False' in results and service not in exposed:
-                result['pass'] = result['pass'] + (service,)
+        results = {'fail': (),
+                   'pass': ()}
+        for unit, result in service_results.items():
+            app = unit.split('/')[0]
+            if app in exposed and result:
+                results['pass'] += (unit,)
+            elif app in exposed and not result:
+                results['fail'] += (unit,)
+            elif app not in exposed and result:
+                results['fail'] += (unit,)
             else:
-                result['fail'] = result['fail'] + (service,)
-        return result
+                results['pass'] += (unit,)
+        return results
 
     def parse_final_results(self, visibility, internet, exposed):
         """Parses test results and raises an error if any failed.
@@ -572,6 +633,7 @@
                                        reboot=args.reboot)
         finally:
             test.cleanup(client)
+            log.info('Cleanup complete.')
     return 0
 
 

=== modified file 'jujupy/client.py'
--- jujupy/client.py	2017-03-15 20:09:30 +0000
+++ jujupy/client.py	2017-03-21 16:28:48 +0000
@@ -1338,6 +1338,35 @@
                         self.machine)
 
 
+class WaitApplicationNotPresent(BaseCondition):
+    """Condition satisfied when a given machine is not present."""
+
+    def __init__(self, application, timeout=300):
+        super(WaitApplicationNotPresent, self).__init__(timeout)
+        self.application = application
+
+    def __eq__(self, other):
+        if not type(self) is type(other):
+            return False
+        if self.timeout != other.timeout:
+            return False
+        if self.application != other.application:
+            return False
+        return True
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+    def iter_blocking_state(self, status):
+        for application in status.get_applications().keys():
+            if application == self.application:
+                yield application, 'still-present'
+
+    def do_raise(self, model_name, status):
+        raise Exception("Timed out waiting for application "
+                        "removal {}".format(self.application))
+
+
 class MachineDown(BaseCondition):
     """Condition satisfied when a given machine is down."""
 

=== modified file 'jujupy/tests/test_client.py'
--- jujupy/tests/test_client.py	2017-03-15 20:09:30 +0000
+++ jujupy/tests/test_client.py	2017-03-21 16:28:48 +0000
@@ -86,6 +86,7 @@
     uniquify_local,
     UnitError,
     WaitMachineNotPresent,
+    WaitApplicationNotPresent
     )
 from jujupy.fake import (
     get_user_register_command_info,
@@ -370,6 +371,29 @@
             not_present.do_raise('', None)
 
 
+class TestWaitApplicationNotPresent(ClientTest):
+
+    def test_iter_blocking_state(self):
+        not_present = WaitApplicationNotPresent('foo')
+        client = fake_juju_client()
+        client.bootstrap()
+        self.assertItemsEqual(
+            [], not_present.iter_blocking_state(client.get_status()))
+        client.deploy('foo')
+        self.assertItemsEqual(
+            [('foo', 'still-present')],
+            not_present.iter_blocking_state(client.get_status()))
+        client.remove_service('foo')
+        self.assertItemsEqual(
+            [], not_present.iter_blocking_state(client.get_status()))
+
+    def test_do_raise(self):
+        not_present = WaitApplicationNotPresent('foo')
+        with self.assertRaisesRegexp(
+                Exception, 'Timed out waiting for application removal foo'):
+            not_present.do_raise('', None)
+
+
 class TestMachineDown(ClientTest):
 
     def test_iter_blocking_state(self):

=== modified file 'tests/test_assess_network_health.py'
--- tests/test_assess_network_health.py	2017-03-16 20:51:05 +0000
+++ tests/test_assess_network_health.py	2017-03-21 16:28:48 +0000
@@ -2,7 +2,6 @@
 import json
 import StringIO
 import logging
-import argparse
 import copy
 from textwrap import dedent
 from datetime import (
@@ -28,9 +27,7 @@
     parse_args,
     _setup_spaces
     )
-from utility import (
-    add_basic_testing_arguments
-    )
+
 
 apps = {'foo':
         {'charm-name': 'foo',
@@ -157,11 +154,6 @@
 
 class TestAssessNetworkHealth(TestCase):
 
-    def parse_args(self, args):
-        parser = argparse.ArgumentParser()
-        add_basic_testing_arguments(parser)
-        return parser.parse_args(args)
-
     def test_setup_testing_environment(self):
 
         def setup_iteration(bundle, target_model, series):
@@ -176,7 +168,7 @@
             net_health.setup_testing_environment(mock_client, bundle,
                                                  target_model, series)
             return mock_client
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         client = setup_iteration(bundle=None, target_model=None, series=series)
         self.assertEqual(
@@ -232,7 +224,7 @@
 
     def test_connect_to_existing_model_when_different(self):
         model = {'bar': 'baz'}
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         client = Mock(spec=["juju", "show_model", "switch"])
         with patch.object(client, 'show_model', return_value=model):
@@ -242,7 +234,7 @@
 
     def test_connect_to_existing_model_when_same(self):
         model = {'foo': 'baz'}
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         client = Mock(spec=["juju", "show_model", "switch"])
         with patch.object(client, 'show_model', return_value=model):
@@ -254,7 +246,7 @@
         pass
 
     def test_neighbor_visibility(self):
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         client = Mock(wraps=fake_juju_client())
         client.bootstrap()
@@ -272,7 +264,7 @@
         self.assertEqual(expected, out)
 
     def test_internet_connection(self):
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         client = fake_juju_client()
         client.bootstrap()
@@ -293,27 +285,30 @@
         self.assertEqual(expected, out)
 
     def test_ensure_exposed(self):
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
-        client = Mock(wraps=fake_juju_client())
-        client.bootstrap()
-        new_client = fake_juju_client()
-        new_client.bootstrap()
-        new_client._backend.set_action_result('network-health/0', 'ping',
-                                              ping_result)
-        new_client.deploy('ubuntu', num=2, series='trusty')
-        new_client.deploy('network-health', series='trusty')
-        now = datetime.now() + timedelta(days=1)
-        with patch('utility.until_timeout.now', return_value=now):
-            with patch.object(client, 'get_status', return_value=status):
-                with patch('assess_network_health.AssessNetworkHealth.'
-                           'setup_expose_test', return_value=new_client):
-                    out = net_health.ensure_exposed(client, series)
-        expected = {'fail': (), 'pass': ('ubuntu',)}
-        self.assertEqual(out, expected)
+        mock_client = Mock(spec=["juju", "deploy",
+                                 "wait_for_subordinate_units",
+                                 "get_status"])
+        mock_client.get_status.return_value = status
+        mock_client.series = 'xenial'
+        mock_client.version = '2.2'
+        with patch('subprocess.check_output', return_value='pass'):
+            net_health.ensure_exposed(mock_client, series)
+        self.assertEqual(
+            [call.get_status(),
+             call.get_status(),
+             call.deploy('~juju-qa/network-health',
+                         alias='network-health-ubuntu', series='trusty'),
+             call.juju('add-relation', ('ubuntu', 'network-health-ubuntu')),
+             call.wait_for_subordinate_units('ubuntu',
+                                             'network-health-ubuntu'),
+             call.juju('expose', 'network-health-ubuntu'),
+             call.get_status()],
+            mock_client.mock_calls)
 
     def test_dummy_deployment(self):
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         client = Mock(wraps=fake_juju_client())
         client.bootstrap()
@@ -321,38 +316,13 @@
         client.deploy.assert_called_once_with('ubuntu', num=2, series='trusty')
 
     def test_bundle_deployment(self):
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         client = Mock(wraps=fake_juju_client())
         client.bootstrap()
         net_health.setup_bundle_deployment(client, bundle_string)
         client.deploy_bundle.assert_called_once_with(bundle_string)
 
-    def test_setup_expose_test(self):
-        args = self.parse_args([])
-        net_health = AssessNetworkHealth(args)
-        mock_client = Mock(spec=["juju", "wait_for_started",
-                                 "wait_for_workloads", "deploy",
-                                 "get_juju_output",
-                                 "wait_for_subordinate_units",
-                                 "get_status", "deploy_bundle", "add_model"
-                                 ])
-        mock_client.series = 'trusty'
-        mock_client.version = '2.2'
-        net_health.setup_expose_test(mock_client, series)
-        self.assertEqual(
-            [call.add_model('exposetest'),
-             call.add_model().deploy('ubuntu', series='trusty'),
-             call.add_model().deploy('~juju-qa/network-health',
-                                     series='trusty'),
-             call.add_model().wait_for_started(),
-             call.add_model().wait_for_workloads(),
-             call.add_model().juju('add-relation', ('ubuntu',
-                                                    'network-health')),
-             call.add_model().wait_for_subordinate_units('ubuntu',
-                                                         'network-health')],
-            mock_client.mock_calls)
-
     def test_setup_spaces_existing_spaces(self):
         existing_spaces = maas_spaces
         new_spaces = _setup_spaces(bundle_yaml, existing_spaces)
@@ -374,18 +344,18 @@
         self.assertEqual(expected_spaces, new_spaces)
 
     def test_parse_expose_results(self):
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         exposed = ['bar', 'baz']
-        service_results = {'foo': "{'foo/0': 'True'}",
-                           'bar': "{'bar/0': 'False'}",
-                           'baz': "{'baz/0': 'True'}"}
-        expected = {"fail": ('foo', 'bar'), "pass": ('baz',)}
+        service_results = {'foo': False,
+                           'bar': True,
+                           'baz': False}
+        expected = {'fail': ('baz',), 'pass': ('foo', 'bar')}
         result = net_health.parse_expose_results(service_results, exposed)
         self.assertEqual(expected, result)
 
     def test_parse_final_results_with_fail(self):
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         visible = {"bar/0": {"foo": {"foo/0": False, "foo/1": True}}}
         internet = {"0": False, "1": True}
@@ -400,7 +370,7 @@
             self.assertTrue(line in error_strings)
 
     def test_parse_final_results_without_fail(self):
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         visible = {"bar/0": {"foo": {"foo/0": True, "foo/1": True}}}
         internet = {"0": True, "1": True}
@@ -408,7 +378,7 @@
         net_health.parse_final_results(visible, internet, exposed)
 
     def test_ping_units(self):
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         client = fake_juju_client()
         client.bootstrap()
@@ -419,7 +389,7 @@
         self.assertEqual(out, result['results']['results'])
 
     def test_to_json(self):
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         expected = '("foo"=("foo/0"="1.1.1.1","foo/1"="1.1.1.2"))'
         targets = {'foo': {'foo/0': '1.1.1.1', 'foo/1': '1.1.1.2'}}
@@ -427,7 +397,7 @@
         self.assertEqual(expected, json_like)
 
     def test_parse_targets(self):
-        args = self.parse_args([])
+        args = parse_args([])
         net_health = AssessNetworkHealth(args)
         client = fake_juju_client()
         client.bootstrap()