Merge lp:~mskalka/juju-ci-tools/better-network-checking into lp:juju-ci-tools

[Michael Skalka]

rework neighbor visibility

Changes the behavior of neighbor_visibility to attempt to curl a simple http server on an associated network-health charm, which better mimics actual network behavior a charm would generate. This also makes the test compatible with public clouds.

Also removes the controller visibility test as this is not a supported feature of juju and does a few small re-orgs.

[Christopher Lee (veebers)]

A couple of queries inline, also some suggestions.

review: Needs Information

[lp:~mskalka/juju-ci-tools/better-network-checking]

1940. By Michael Skalka on 2017-03-17

cleanup as per MP comments

[Curtis Hovey (sinzui)]

Thank you. We spoke and agreed that port 80 risks conflicts with other services so we will switch to a less popular port in a future branch.

review: Approve (code)

=== modified file 'assess_network_health.py'
--- assess_network_health.py	2017-03-10 15:43:14 +0000
+++ assess_network_health.py	2017-03-17 14:19:58 +0000
@@ -7,12 +7,12 @@
 import sys
 import json
 import yaml
-import ast
 import subprocess
 import re
 import time
 import os
 import socket
+from collections import defaultdict
 
 from jujupy import (
     client_for_existing,
@@ -91,11 +91,6 @@
         interface_info = self.get_unit_info(client)
         log.info('{0}Interface information:\n{1}'.format(
             reboot_msg, json.dumps(interface_info, indent=4, sort_keys=True)))
-        con_result = self.juju_controller_visibility(client)
-        log.info('{0}Controller Visibility '
-                 'result:\n {1}'.format(reboot_msg,
-                                        json.dumps(con_result, indent=4,
-                                                   sort_keys=True)))
         int_result = self.internet_connection(client)
         log.info('{0}Internet Test '
                  'result:\n {1}'.format(reboot_msg,
@@ -117,7 +112,7 @@
                                                        sort_keys=True)) or
                      NO_EXPOSED_UNITS)
         log.info('Tests complete.')
-        return self.parse_final_results(con_result, vis_result, int_result,
+        return self.parse_final_results(vis_result, int_result,
                                         exp_result)
 
     def setup_testing_environment(self, client, bundle, target_model,
@@ -142,11 +137,14 @@
             try:
                 client.deploy('~juju-qa/network-health', series=series,
                               alias='network-health-{}'.format(series))
+
             except subprocess.CalledProcessError:
                 log.info('Could not deploy network-health-{} as it is already'
                          ' present in the juju deployment.'.format(series))
         client.wait_for_started()
         client.wait_for_workloads()
+        for series in self.existing_series:
+            client.juju('expose', ('network-health-{}'.format(series)))
         apps = client.get_status().get_applications()
         log.info('Known applications: {}'.format(apps.keys()))
         for app, info in apps.items():
@@ -207,48 +205,23 @@
 
         :param client: Client to get results from
         :return: Dict of machine results as
-        <machine>:{'dns':<dns>, 'interfaces':<interfaces>}
+        <machine>:{'interfaces':<interfaces>}
         """
         results = {}
         apps = client.get_status().get_applications()
-        nh_units = self.get_nh_units(apps, by_unit=True)
-        for app, unit in nh_units.items():
+        nh_units = self.get_nh_unit_info(apps, by_unit=True)
+        for app, units in nh_units.items():
             machine = apps[app.split('/')[0]]['units'][app]['machine']
-            results[machine] = {}
-            out = client.action_do(unit[0], 'unit-info')
-            out = client.action_fetch(out)
-            out = yaml.safe_load(out)
-            results[machine]['dns'] = out['results']['dns']
-            results[machine]['interfaces'] = out['results']['interfaces']
+            results[machine] = defaultdict(defaultdict)
+            results[machine]['interfaces'] = {}
+            for nh_unit in units.keys():
+                out = client.action_do(nh_unit, 'unit-info')
+                out = client.action_fetch(out)
+                out = yaml.safe_load(out)
+                interfaces = out['results']['interfaces']
+                results[machine]['interfaces'][nh_unit] = interfaces
         return results
 
-    def juju_controller_visibility(self, client):
-        """Determine if known juju machines are visible from controller.
-
-        :param machine: List of machine IPs to test
-        :return: Connection attempt results
-        """
-        cont_client = client.get_controller_client()
-        log.info('Starting controller visibility test')
-        machines = client.get_status().iter_machines(containers=True)
-        result = {}
-        for machine, info in machines:
-            result[machine] = {}
-            for ip in info['ip-addresses']:
-                if self.is_ipv6(ip):
-                    cmd = 'ping6'
-                else:
-                    cmd = 'ping'
-                result[machine][ip] = False
-                try:
-                    self.ssh(cont_client, '0', "{} -c 1 ".format(cmd) + ip)
-                except subprocess.CalledProcessError as e:
-                    log.error('Error with ping attempt '
-                              'to {}: {}'.format(ip, e))
-                    continue
-                result[machine][ip] = True
-        return result
-
     def internet_connection(self, client):
         """Test that targets can ping their default route.
 
@@ -284,17 +257,25 @@
             results[unit[0]] = True
         return results
 
-    def get_nh_units(self, apps, by_unit=False):
-        nh_units = []
-        subs_by_unit = {}
-        for service, s_info in apps.items():
-            for unit, u_info in s_info.get('units', {}).items():
-                nh_subs = [u for u in u_info.get('subordinates').keys()
-                           if 'network-health' in u]
-                subs_by_unit[unit] = nh_subs
-                nh_units.extend(nh_subs)
+    def get_nh_unit_info(self, apps, by_unit=False):
+        """Parses juju status information to return deployed network-health units.
+
+        :param apps: Dict of apps given by get_status().get_applications()
+        :param by_unit: Bool, returns dict of NH units keyed by the unit they
+        are subordinate to
+        :return: Dict of network-health units
+        """
+        nh_units = {}
+        nh_by_unit = {}
+        for app, units in apps.items():
+            for unit, info in units.get('units', {}).items():
+                nh_by_unit[unit] = {}
+                for sub, sub_info in info.get('subordinates', {}).items():
+                    if 'network-health' in sub:
+                        nh_by_unit[unit][sub] = sub_info
+                        nh_units[sub] = sub_info
         if by_unit:
-            return subs_by_unit
+            return nh_by_unit
         return nh_units
 
     def neighbor_visibility(self, client):
@@ -304,15 +285,20 @@
         """
         log.info('Starting neighbor visibility test')
         apps = client.get_status().get_applications()
-        targets = self.parse_targets(client.get_status())
+        nh_units = self.get_nh_unit_info(apps)
+        target_ips = [ip['public-address'] for ip in nh_units.values()]
         result = {}
-        nh_units = self.get_nh_units(apps)
-        for nh_unit in nh_units:
-            service_results = {}
-            for service, units in targets.items():
-                res = self.ping_units(client, nh_unit, units)
-                service_results[service] = ast.literal_eval(res)
-            result[nh_unit] = service_results
+        for app, units in apps.items():
+            result[app] = defaultdict(defaultdict)
+            for unit, info in units.get('units', {}).items():
+                for ip in target_ips:
+                    result[app][unit][ip] = False
+                    pattern = r"(pass)"
+                    out = client.run(['curl {}:80'.format(ip)],
+                                     units=[unit])
+                    match = re.search(pattern, json.dumps(out[0]))
+                    if match:
+                        result[app][unit][ip] = True
         return result
 
     def ensure_exposed(self, client, series):
@@ -377,28 +363,20 @@
                 result['fail'] = result['fail'] + (service,)
         return result
 
-    def parse_final_results(self, controller, visibility, internet,
-                            exposed):
+    def parse_final_results(self, visibility, internet, exposed):
         """Parses test results and raises an error if any failed.
 
-        :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 controller.items():
-            for ip, res in machine_result.items():
-                if res is False:
-                    error = ('Failed to contact controller from machine {0} '
-                             'at address {1}'.format(machine, ip))
-                    error_string.append(error)
         for nh_source, service_result in visibility.items():
                 for service, unit_res in service_result.items():
                     if False in unit_res.values():
                         failed = [u for u, r in unit_res.items() if r is False]
                         error = ('NH-Unit {0} failed to contact '
-                                 'unit(s): {1}'.format(nh_source, failed))
+                                 'targets(s): {1}'.format(nh_source, failed))
                         error_string.append(error)
         for unit, res in internet.items():
             if not res:

=== modified file 'tests/test_assess_network_health.py'
--- tests/test_assess_network_health.py	2017-03-10 15:43:14 +0000
+++ tests/test_assess_network_health.py	2017-03-17 14:19:58 +0000
@@ -95,6 +95,7 @@
             juju-status:
               current: idle
             public-address: 1.1.1.1
+            machine: '0'
           ubuntu/1:
             subordinates:
               network-health/1:
@@ -104,6 +105,7 @@
             juju-status:
               current: idle
             public-address: 1.1.1.2
+            machine: '1'
         application-status:
           current: unknown
           since: 01 Jan 2017 00:00:00-00:00
@@ -145,8 +147,9 @@
 timing:
   completed: 2017-01-01 00:00:01 +0000 UTC
   enqueued: 2017-01-01 00:00:01 +0000 UTC
-  started: 2017-01-01 00:00:01 +0000 UTC
+  started: 2017-01-01 00:00:01 +0000 UTC}
 """)
+curl_result = [{'foo': 'pass'}]
 
 dummy_charm = 'dummy'
 series = 'trusty'
@@ -186,6 +189,7 @@
                          alias='network-health-trusty', series='trusty'),
              call.wait_for_started(),
              call.wait_for_workloads(),
+             call.juju('expose', 'network-health-trusty'),
              call.get_status(),
              call.juju('add-relation', ('ubuntu', 'network-health-trusty')),
              call.juju('add-relation', ('network-health',
@@ -214,6 +218,7 @@
                          alias='network-health-trusty', series='trusty'),
              call.wait_for_started(),
              call.wait_for_workloads(),
+             call.juju('expose', 'network-health-trusty'),
              call.get_status(),
              call.juju('add-relation', ('ubuntu', 'network-health-trusty')),
              call.juju('add-relation', ('network-health',
@@ -225,20 +230,6 @@
                                              'network-health-trusty')],
             client.mock_calls)
 
-    def test_juju_controller_visibility(self):
-        args = self.parse_args([])
-        net_health = AssessNetworkHealth(args)
-        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 = net_health.juju_controller_visibility(client)
-        expected = {'1': {'1.1.1.2': True}, '0': {'1.1.1.1': True}}
-        self.assertEqual(expected, out)
-
     def test_connect_to_existing_model_when_different(self):
         model = {'bar': 'baz'}
         args = self.parse_args([])
@@ -267,20 +258,17 @@
         net_health = AssessNetworkHealth(args)
         client = Mock(wraps=fake_juju_client())
         client.bootstrap()
-        client._backend.set_action_result('network-health/0', 'ping',
-                                          ping_result)
-        client._backend.set_action_result('network-health/1', 'ping',
-                                          ping_result)
         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, series='trusty')
-                client.deploy('network-health', series='trusty')
-                out = net_health.neighbor_visibility(client)
-        expected = {'network-health/0': {'ubuntu': {u'ubuntu/0': True,
-                                                    u'ubuntu/1': True}},
-                    'network-health/1': {'ubuntu': {u'ubuntu/0': True,
-                                                    u'ubuntu/1': True}}}
+                with patch.object(client, 'run', return_value=curl_result):
+                    client.deploy('ubuntu', num=2, series='trusty')
+                    client.deploy('network-health', series='trusty')
+                    out = net_health.neighbor_visibility(client)
+        expected = {'network-health': {},
+                    'ubuntu': {'ubuntu/0': {'1.1.1.1': True, '1.1.1.2': True},
+                               'ubuntu/1': {'1.1.1.1': True, '1.1.1.2': True}}}
+
         self.assertEqual(expected, out)
 
     def test_internet_connection(self):
@@ -399,17 +387,14 @@
     def test_parse_final_results_with_fail(self):
         args = self.parse_args([])
         net_health = AssessNetworkHealth(args)
-        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")}
-        out = net_health.parse_final_results(controller, visible, internet,
+        out = net_health.parse_final_results(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']",
+        error_strings = ["Machine 0 failed internet connection.",
+                         "NH-Unit bar/0 failed to contact targets(s): "
+                         "['foo/0']",
                          "Application(s) foo failed expose test"]
         for line in out:
             self.assertTrue(line in error_strings)
@@ -417,11 +402,10 @@
     def test_parse_final_results_without_fail(self):
         args = self.parse_args([])
         net_health = AssessNetworkHealth(args)
-        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")}
-        net_health.parse_final_results(controller, visible, internet, exposed)
+        net_health.parse_final_results(visible, internet, exposed)
 
     def test_ping_units(self):
         args = self.parse_args([])