Merge lp:~mskalka/juju-ci-tools/add-more-network-tests into lp:juju-ci-tools

[Michael Skalka]

Adds an additional network test and changes the way the agnostic visibility test is run. This changes it from having the script itself pinging the units to having the Juju controller do it. Ideally this would give us a better idea if an agent is 'lost' or just dropped off the network. The additional network test is borrowed from assess_container_networking and ensures the unit can ping its default gateway.

[lp:~mskalka/juju-ci-tools/add-more-network-tests]

1908. By Michael Skalka on 2017-02-24

updated bzr ignore

1909. By Michael Skalka on 2017-02-24

revert makefile change

[Curtis Hovey (sinzui)]

We agreed to make some series fixes and that the machine handling of reboot wont work. We We can fix the reboot feature in another branch.

review: Approve (code)

[lp:~mskalka/juju-ci-tools/add-more-network-tests]

1910. By Michael Skalka on 2017-02-24

cleanup, fixed series

=== modified file '.bzrignore'
--- .bzrignore	2016-12-14 12:40:18 +0000
+++ .bzrignore	2017-02-24 18:28:22 +0000
@@ -4,4 +4,3 @@
 /logs
 /repository
 ./.coverage
-

=== modified file 'assess_container_networking.py'
--- assess_container_networking.py	2017-01-20 20:58:41 +0000
+++ assess_container_networking.py	2017-02-24 18:28:22 +0000
@@ -127,7 +127,6 @@
         host, type, id = c[0].split('/')
         if type in containers and host in containers[type]:
             containers[type][host].append(c[0])
-
     return hosts, containers
 
 

=== modified file 'assess_network_health.py'
--- assess_network_health.py	2017-02-22 19:05:46 +0000
+++ assess_network_health.py	2017-02-24 18:28:22 +0000
@@ -9,14 +9,14 @@
 import yaml
 import ast
 import subprocess
+import re
+import time
 
 from jujupy import client_from_config
 from deploy_stack import (
     BootstrapManager,
     )
-from jujucharm import (
-    local_charm_path,
-    )
+
 from utility import (
     add_basic_testing_arguments,
     configure_logging,
@@ -36,7 +36,8 @@
         self.message = message
 
 
-def assess_network_health(client, bundle=None, target_model=None, series=None):
+def assess_network_health(client, bundle=None, target_model=None, reboot=False,
+                          series=None):
     """Assesses network health for a given deployment or bundle.
 
     :param client: The juju client in use
@@ -44,22 +45,50 @@
     :param model: Optional existing model to test under
     """
     setup_testing_environment(client, bundle, target_model, series)
-    log.info("Starting network tests")
-    agnostic_result = ensure_juju_agnostic_visibility(client)
-    log.info('Agnostic result:\n {}'.format(json.dumps(agnostic_result,
-                                                       indent=4,
-                                                       sort_keys=True)))
-    visibility_result = neighbor_visibility(client)
-    log.info('Visibility result:\n {}'.format(json.dumps(visibility_result,
+    log.info('Starting network tests')
+    testing_iterations(client, series, '')
+    if not reboot:
+        return
+    log.info('Units passed pre-reboot tests, rebooting machines')
+    # TODO: Need to use itermachines and need to divide containers
+    # from reboot cycle
+    machines = get_juju_status(client)['machines']
+    client.run('sudo reboot', machines=machines)
+    log.info('Waiting for units to restart')
+    client.wait_for_started()
+    client.wait_for_workloads()
+    log.info('Starting post-reboot network tests')
+    testing_iterations(client, series, 'Post-reboot ')
+
+
+def testing_iterations(client, series, reboot_msg):
+    """Runs through each test given for a given client and series
+
+    :param client: Client
+    """
+    con_result = juju_controller_visibility(client)
+    log.info('{}Controller Visibility '
+             'result:\n {}'.format(reboot_msg, json.dumps(con_result,
+                                                          indent=4,
+                                                          sort_keys=True)))
+    int_result = internet_connection(client)
+    log.info('{}Internet Test result:\n {}'.format(reboot_msg,
+                                                   json.dumps(int_result,
+                                                              indent=4,
+                                                              sort_keys=True)))
+    vis_result = neighbor_visibility(client)
+    log.info('{}Visibility result:\n {}'.format(reboot_msg,
+                                                json.dumps(vis_result,
+                                                           indent=4,
+                                                           sort_keys=True)))
+    exp_result = ensure_exposed(client, series)
+    log.info('{}Exposure result:\n {}'.format(reboot_msg,
+                                              json.dumps(exp_result,
                                                          indent=4,
-                                                         sort_keys=True)))
-    exposed_result = ensure_exposed(client, series)
-    log.info('Exposure result:\n {}'.format(json.dumps(exposed_result,
-                                                       indent=4,
-                                                       sort_keys=True)) or
-                                                       NO_EXPOSED_UNITS)
-    log.info('Network tests complete, parsing results.')
-    parse_final_results(agnostic_result, visibility_result, exposed_result)
+                                                         sort_keys=True)) or
+             NO_EXPOSED_UNITS)
+    log.info('Tests complete.')
+    parse_final_results(con_result, vis_result, int_result, exp_result)
 
 
 def setup_testing_environment(client, bundle, target_model, series=None):
@@ -77,9 +106,7 @@
     elif bundle is None and target_model is None:
         setup_dummy_deployment(client, series)
 
-    charm_path = local_charm_path(charm='network-health', series=series,
-                                  juju_ver=client.version)
-    client.deploy(charm_path)
+    client.deploy('~juju-qa/network-health', series=series)
     client.wait_for_started()
     client.wait_for_workloads()
     applications = get_juju_status(client)['applications'].keys()
@@ -88,7 +115,7 @@
     for app in applications:
         try:
             client.juju('add-relation', (app, 'network-health'))
-        except subprocess.CalledProcessError:
+        except subprocess.cessError:
             log.error('Could not relate {} & network-health'.format(app))
 
     client.wait_for_workloads()
@@ -113,9 +140,7 @@
     :param client: Bootstrapped juju client
     """
     log.info("Deploying dummy charm for basic testing")
-    dummy_path = local_charm_path(charm='ubuntu', series=series,
-                                  juju_ver=client.version)
-    client.deploy(dummy_path, num=2)
+    client.deploy('ubuntu', num=2, series=series)
     client.juju('expose', ('ubuntu',))
 
 
@@ -136,12 +161,13 @@
     return client.get_status().status
 
 
-def ensure_juju_agnostic_visibility(client):
-    """Determine if known juju machines are visible.
+def juju_controller_visibility(client):
+    """Determine if known juju machines are visible from controller.
 
     :param machine: List of machine IPs to test
     :return: Connection attempt results
     """
+    controller_client = client.get_controller_client()
     log.info('Starting agnostic visibility test')
     machines = get_juju_status(client)['machines']
     result = {}
@@ -149,14 +175,40 @@
         result[machine] = {}
         for ip in info['ip-addresses']:
             try:
-                output = subprocess.check_output("ping -c 1 " + ip, shell=True)
-            except subprocess.CalledProcessError, e:
+                ssh(controller_client, '0', "ping -c 1 " + ip)
+            except subprocess.CalledProcessError as e:
                 log.error('Error with ping attempt to {}: {}'.format(ip, e))
                 result[machine][ip] = False
             result[machine][ip] = True
     return result
 
 
+def internet_connection(client):
+    """Test that targets can ping their default route.
+
+    :param client: Juju client
+    :return: Dict of results by machine
+    """
+    log.info('Assessing internet connection.')
+    results = {}
+    units = get_juju_status(client)['machines']
+    for unit in units:
+        log.info("Assessing internet connection for {}".format(unit))
+        routes = ssh(client, unit, 'ip route show')
+        d = re.search(r'^default\s+via\s+([\d\.]+)\s+', routes, re.MULTILINE)
+        if d:
+            rc = client.juju('ssh', ('--proxy', unit,
+                                     'ping -c1 -q ' + d.group(1)), check=False)
+            if rc != 0:
+                log.error('{0} unable to ping default route'.format(unit))
+                results[unit] = False
+        else:
+            log.error("Default route not found")
+            results[unit] = False
+        results[unit] = True
+    return results
+
+
 def neighbor_visibility(client):
     """Check if each application's units are visible, including our own.
 
@@ -208,12 +260,8 @@
     :return: New juju client object
     """
     new_client = client.add_model('exposetest')
-    dummy_path = local_charm_path(charm='ubuntu', series=series,
-                                  juju_ver=client.version)
-    new_client.deploy(dummy_path)
-    charm_path = local_charm_path(charm='network-health', series=series,
-                                  juju_ver=client.version)
-    new_client.deploy(charm_path)
+    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'))
@@ -244,18 +292,19 @@
     return result
 
 
-def parse_final_results(agnostic, visibility, exposed=None):
+def parse_final_results(controller, visibility, internet, exposed=None):
     """Parses test results and raises an error if any failed.
 
-    :param agnostic: Agnostic test result
+    :param controller: Controller test result
     :param visibility: Visibility test result
     :param exposed: Exposure test result
     """
+    log.info('Parsing final results.')
     error_string = []
-    for machine, machine_result in agnostic.items():
+    for machine, machine_result in controller.items():
         for ip, res in machine_result.items():
             if res is False:
-                error = ('Failed to ping machine {0} '
+                error = ('Failed to contact controller from machine {0} '
                          'at address {1}\n'.format(machine, ip))
                 error_string.append(error)
     for nh_source, service_result in visibility.items():
@@ -265,18 +314,44 @@
                     error = ('NH-Unit {0} failed to contact '
                              'unit(s): {1}\n'.format(nh_source, failed))
                     error_string.append(error)
-
+    for machine, res in internet.items():
+        if not res:
+            error = 'Machine {} failed internet connection.'.format(machine)
+            error_string.append(error)
     if exposed and exposed['fail'] is not ():
         error = ('Application(s) {0} failed expose '
                  'test\n'.format(exposed['fail']))
         error_string.append(error)
-
     if error_string:
         raise ConnectionError('\n'.join(error_string))
 
     return
 
 
+def ssh(client, machine, cmd):
+    """Convenience function: run a juju ssh command and get back the output
+    :param client: A Juju client
+    :param machine: ID of the machine on which to run a command
+    :param cmd: the command to run
+    :return: text output of the command
+    """
+    back_off = 2
+    attempts = 4
+    for attempt in range(attempts):
+        try:
+            return client.get_juju_output('ssh', '--proxy', machine, cmd)
+        except subprocess.CalledProcessError as e:
+            # If the connection to the host failed, try again in a couple of
+            # seconds. This is usually due to heavy load.
+            if(attempt < attempts - 1 and
+                re.search('ssh_exchange_identification: '
+                          'Connection closed by remote host', e.stderr)):
+                time.sleep(back_off)
+                back_off *= 2
+            else:
+                raise
+
+
 def ping_units(client, source, units):
     """Calls out to our subordinate network-health charm to ping targets.
 
@@ -286,8 +361,8 @@
     """
     units = to_json(units)
     args = "targets='{}'".format(units)
-    retval = client.action_do(source, 'ping', args)
-    result = client.action_fetch(retval)
+    action_id = client.action_do(source, 'ping', args)
+    result = client.action_fetch(action_id)
     result = yaml.safe_load(result)['results']['results']
     return result
 
@@ -326,6 +401,9 @@
     add_basic_testing_arguments(parser)
     parser.add_argument('--bundle', help='Bundle to test network against')
     parser.add_argument('--model', help='Existing Juju model to test under')
+    parser.add_argument('--reboot', help='Reboot machines and re-run tests'
+                        'default=False')
+    parser.set_defaults(reboot=False)
     parser.set_defaults(series='trusty')
     return parser.parse_args(argv)
 

=== modified file 'tests/test_assess_network_health.py'
--- tests/test_assess_network_health.py	2017-02-22 20:34:37 +0000
+++ tests/test_assess_network_health.py	2017-02-24 18:28:22 +0000
@@ -1,4 +1,3 @@
-import json
 import yaml
 import StringIO
 import logging
@@ -24,13 +23,12 @@
 )
 from assess_network_health import (
     main,
-    assess_network_health,
     setup_testing_environment,
-    connect_to_existing_model,
     setup_dummy_deployment,
     setup_bundle_deployment,
     get_juju_status,
-    ensure_juju_agnostic_visibility,
+    juju_controller_visibility,
+    internet_connection,
     neighbor_visibility,
     ensure_exposed,
     setup_expose_test,
@@ -118,14 +116,9 @@
           since: 01 Jan 2017 00:00:00-00:00
         subordinate-to:
         - ubuntu
-
-    relations:
-      juju-info:
-      - network-health
-      network-health:
-        exposed: false
-        application-status:
-          current: unknown
+        relations:
+          juju-info:
+          - network-health
 """)
 status = Status(yaml.safe_load(status_value), status_value)
 
@@ -162,12 +155,11 @@
                                       series)
             return mock_client
 
-        client = setup_iteration(bundle=None,
-                                 target_model=None, series=series)
+        client = setup_iteration(bundle=None, target_model=None, series=series)
         self.assertEqual(
-            [call.deploy('ubuntu', num=2),
+            [call.deploy('ubuntu', num=2, series='trusty'),
              call.juju('expose', ('ubuntu',)),
-             call.deploy('network-health'),
+             call.deploy('~juju-qa/network-health', series='trusty'),
              call.wait_for_started(),
              call.wait_for_workloads(),
              call.get_status(),
@@ -175,8 +167,8 @@
              call.wait_for_workloads(),
              call.wait_for_subordinate_units('ubuntu', 'network-health')],
             client.mock_calls)
-        client = setup_iteration(bundle=bundle_string,
-                                 target_model=None, series=series)
+        client = setup_iteration(bundle=bundle_string, target_model=None,
+                                 series=series)
         self.assertEqual(
             [call.deploy_bundle('services:\n  foo:\n    '
                                 'charm: local:trusty/foo\n    '
@@ -184,7 +176,7 @@
                                 'bar:\n    charm: local:trusty/bar\n    '
                                 'num_units: 1\nseries: trusty\nrelations:\n'
                                 '- - foo:baz\n  - bar:baz\n'),
-             call.deploy('network-health'),
+             call.deploy('~juju-qa/network-health', series='trusty'),
              call.wait_for_started(),
              call.wait_for_workloads(),
              call.get_status(),
@@ -193,16 +185,15 @@
              call.wait_for_subordinate_units('ubuntu', 'network-health')],
             client.mock_calls)
 
-    def test_ensure_juju_agnostic_visibility(self):
+    def test_juju_controller_visibility(self):
         client = fake_juju_client()
         client.bootstrap()
-        ag_return = True
         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('subprocess.check_output',
                            return_value=0):
-                    out = ensure_juju_agnostic_visibility(client)
+                    out = juju_controller_visibility(client)
         expected = {'1': {'1.1.1.2': True}, '0': {'1.1.1.1': True}}
         self.assertEqual(expected, out)
 
@@ -220,8 +211,8 @@
         now = datetime.now() + timedelta(days=1)
         with patch('utility.until_timeout.now', return_value=now):
             with patch.object(client, 'get_status', return_value=status):
-                client.deploy('ubuntu', num=2)
-                client.deploy('network-health')
+                client.deploy('ubuntu', num=2, series='trusty')
+                client.deploy('network-health', series='trusty')
                 out = neighbor_visibility(client)
         expected = {'network-health/0': {'ubuntu': {u'ubuntu/0': True,
                                                     u'ubuntu/1': True}},
@@ -229,6 +220,18 @@
                                                     u'ubuntu/1': True}}}
         self.assertEqual(expected, out)
 
+    def test_internet_connection(self):
+        client = fake_juju_client()
+        client.bootstrap()
+        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('subprocess.check_output',
+                           return_value=0):
+                    out = internet_connection(client)
+        expected = {'1': True, '0': True}
+        self.assertEqual(expected, out)
+
     def test_ensure_exposed(self):
         client = Mock(wraps=fake_juju_client())
         client.bootstrap()
@@ -236,8 +239,8 @@
         new_client.bootstrap()
         new_client._backend.set_action_result('network-health/0', 'ping',
                                               ping_result)
-        new_client.deploy('ubuntu', num=2)
-        new_client.deploy('network-health')
+        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):
@@ -255,7 +258,7 @@
         client = Mock(wraps=fake_juju_client())
         client.bootstrap()
         setup_dummy_deployment(client, series)
-        client.deploy.assert_called_once_with('ubuntu', num=2)
+        client.deploy.assert_called_once_with('ubuntu', num=2, series='trusty')
 
     def test_bundle_deployment(self):
         client = Mock(wraps=fake_juju_client())
@@ -275,8 +278,9 @@
         setup_expose_test(mock_client, series)
         self.assertEqual(
             [call.add_model('exposetest'),
-             call.add_model().deploy('ubuntu'),
-             call.add_model().deploy('network-health'),
+             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',
@@ -302,22 +306,27 @@
         self.assertEqual(expected, result)
 
     def test_parse_final_results_with_fail(self):
-        agnostic = {"0": {"1.1.1.1": False}}
+        controller = {"0": {"1.1.1.1": False},
+                      "1": {"1.1.1.2": True}}
         visible = {"bar/0": {"foo": {"foo/0": False, "foo/1": True}}}
+        internet = {"0": False, "1": True}
         exposed = {"fail": ("foo"), "pass": ("bar", "baz")}
         with self.assertRaises(ConnectionError) as context:
-            parse_final_results(agnostic, visible, exposed)
-        error_strings = ["Failed to ping machine 0 at address 1.1.1.1",
+            parse_final_results(controller, visible, internet, exposed)
+        error_strings = ["Failed to contact controller from machine 0 "
+                         "at address 1.1.1.1",
+                         "Machine 0 failed internet connection.",
                          "NH-Unit bar/0 failed to contact unit(s): ['foo/0']",
                          "Application(s) foo failed expose test"]
         for line in error_strings:
             self.assertTrue(line in context.exception.message)
 
     def test_parse_final_results_without_fail(self):
-        agnostic = {"0": {"1.1.1.1": True}}
+        controller = {"0": {"1.1.1.1": True}}
         visible = {"bar/0": {"foo": {"foo/0": True, "foo/1": True}}}
+        internet = {"0": True, "1": True}
         exposed = {"fail": (), "pass": ("foo", "bar", "baz")}
-        parse_final_results(agnostic, visible, exposed)
+        parse_final_results(controller, visible, internet, exposed)
 
     def test_ping_units(self):
         client = fake_juju_client()
@@ -344,7 +353,6 @@
 
     def test_main(self):
         argv = ["an-env", "/bin/juju", "/tmp/logs", "an-env-mod", "--verbose"]
-        env = object()
         client = Mock(spec=["is_jes_enabled"])
         with patch("assess_network_health.configure_logging",
                    autospec=True) as mock_cl: