From f35da80f2783a39ad49e5bb9c7c59de0520ed458 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Tue, 7 Apr 2020 11:04:46 +0200 Subject: [PATCH 1/3] Unpin flake8, fix lint Add Python 3.8 env in Travis CI test matrix. At present the pinning of flake8 disallows running of lint on Python 3.8 systems. Update flake8 ignore-list to ignore W504 instead of W503, the PEP guidance is that either is ok, but there must be local consistency. There are more occurences of binary operator before line-break than after in this repository, and we have also chosen to ignore W504 in most of our other repositories. --- .travis.yml | 3 ++ requirements.txt | 2 +- tox.ini | 2 +- .../charm_tests/openstack_dashboard/tests.py | 12 ++--- zaza/openstack/charm_tests/policyd/tests.py | 8 ++-- zaza/openstack/charm_tests/quagga/tests.py | 44 +++++++++---------- zaza/openstack/charm_tests/test_utils.py | 2 +- zaza/openstack/utilities/generic.py | 40 ++++++++--------- zaza/openstack/utilities/openstack.py | 4 +- 9 files changed, 60 insertions(+), 57 deletions(-) diff --git a/.travis.yml b/.travis.yml index c1558f3..7635f2d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,5 +13,8 @@ matrix: - name: "Python 3.7" python: 3.7 env: ENV=pep8,py3 + - name: "Python 3.8" + python: 3.7 + env: ENV=pep8,py3 script: - tox -c tox.ini -e $ENV diff --git a/requirements.txt b/requirements.txt index cbf31df..ed9c353 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ boto3 juju juju_wait PyYAML<=4.2,>=3.0 -flake8>=2.2.4,<=3.5.0 +flake8>=2.2.4 flake8-docstrings flake8-per-file-ignores pydocstyle<4.0.0 diff --git a/tox.ini b/tox.ini index f5be2fa..6a37cfe 100644 --- a/tox.ini +++ b/tox.ini @@ -25,7 +25,7 @@ deps = -r{toxinidir}/requirements.txt commands = /bin/true [flake8] -ignore = E402,E226,W503 +ignore = E402,E226,W504 per-file-ignores = unit_tests/**: D diff --git a/zaza/openstack/charm_tests/openstack_dashboard/tests.py b/zaza/openstack/charm_tests/openstack_dashboard/tests.py index 187cdc3..7ac0dd0 100644 --- a/zaza/openstack/charm_tests/openstack_dashboard/tests.py +++ b/zaza/openstack/charm_tests/openstack_dashboard/tests.py @@ -105,12 +105,12 @@ def _login(dashboard_ip, domain, username, password): # services/information missing that horizon wants to display data # for. # Redirect to /horizon/identity/ instead. - if (openstack_utils.get_os_release() - >= openstack_utils.get_os_release('xenial_queens')): + if (openstack_utils.get_os_release() >= + openstack_utils.get_os_release('xenial_queens')): auth['next'] = '/horizon/identity/' - if (openstack_utils.get_os_release() - >= openstack_utils.get_os_release('bionic_stein')): + if (openstack_utils.get_os_release() >= + openstack_utils.get_os_release('bionic_stein')): auth['region'] = 'default' if api_version == 2: @@ -122,8 +122,8 @@ def _login(dashboard_ip, domain, username, password): # NOTE(ajkavanagh) there used to be a trusty/icehouse test in the # amulet test, but as the zaza tests only test from trusty/mitaka # onwards, the test has been dropped - if (openstack_utils.get_os_release() - >= openstack_utils.get_os_release('bionic_stein')): + if (openstack_utils.get_os_release() >= + openstack_utils.get_os_release('bionic_stein')): expect = "Sign Out" # update the in dashboard seems to require region to be default in # this test configuration diff --git a/zaza/openstack/charm_tests/policyd/tests.py b/zaza/openstack/charm_tests/policyd/tests.py index 703f0af..360736d 100644 --- a/zaza/openstack/charm_tests/policyd/tests.py +++ b/zaza/openstack/charm_tests/policyd/tests.py @@ -214,8 +214,8 @@ class GenericPolicydTest(PolicydTest, test_utils.OpenStackBaseTest): def setUpClass(cls, application_name=None): """Run class setup for running KeystonePolicydTest tests.""" super(GenericPolicydTest, cls).setUpClass(application_name) - if (openstack_utils.get_os_release() - < openstack_utils.get_os_release('xenial_queens')): + if (openstack_utils.get_os_release() < + openstack_utils.get_os_release('xenial_queens')): raise unittest.SkipTest( "zaza.openstack.charm_tests.policyd.tests.GenericPolicydTest " "not valid before xenial_queens") @@ -276,8 +276,8 @@ class BasePolicydSpecialization(PolicydTest, def setUpClass(cls, application_name=None): """Run class setup for running KeystonePolicydTest tests.""" super(BasePolicydSpecialization, cls).setUpClass(application_name) - if (openstack_utils.get_os_release() - < openstack_utils.get_os_release('xenial_queens')): + if (openstack_utils.get_os_release() < + openstack_utils.get_os_release('xenial_queens')): raise unittest.SkipTest( "zaza.openstack.charm_tests.policyd.tests.* " "not valid before xenial_queens") diff --git a/zaza/openstack/charm_tests/quagga/tests.py b/zaza/openstack/charm_tests/quagga/tests.py index 67a49d6..67408f2 100644 --- a/zaza/openstack/charm_tests/quagga/tests.py +++ b/zaza/openstack/charm_tests/quagga/tests.py @@ -38,26 +38,26 @@ class QuaggaTest(unittest.TestCase): 'tor2', 'peer0', 'peer1'] if app in status.applications.keys()) for application in applications: - for unit in zaza.model.get_units(application): - bgp_sum = zaza.model.run_on_unit( - unit.entity_id, - 'echo "sh bgp ipv4 unicast summary" | vtysh')['Stdout'] - r = re.compile('^(\d+\.\d+\.\d+\.\d+)') - ip_list = [] - for line in bgp_sum.splitlines(): - m = r.match(line) - if m: - ip_list.append(m.group(1)) - logging.info('unit {} neighbours {}' - .format(unit.entity_id, ip_list)) + for unit in zaza.model.get_units(application): + bgp_sum = zaza.model.run_on_unit( + unit.entity_id, + 'echo "sh bgp ipv4 unicast summary" | vtysh')['Stdout'] + r = re.compile(r'^(\d+\.\d+\.\d+\.\d+)') + ip_list = [] + for line in bgp_sum.splitlines(): + m = r.match(line) + if m: + ip_list.append(m.group(1)) + logging.info('unit {} neighbours {}' + .format(unit.entity_id, ip_list)) - if not ip_list: - raise Exception('FAILED: Unit {} has no BGP peers.' - .format(unit.entity_id)) - for ip in ip_list: - result = zaza.model.run_on_unit( - unit.entity_id, - 'ping -c 3 {}'.format(ip)) - logging.info(result['Stdout']) - if result['Code'] == '1': - raise Exception('FAILED') + if not ip_list: + raise Exception('FAILED: Unit {} has no BGP peers.' + .format(unit.entity_id)) + for ip in ip_list: + result = zaza.model.run_on_unit( + unit.entity_id, + 'ping -c 3 {}'.format(ip)) + logging.info(result['Stdout']) + if result['Code'] == '1': + raise Exception('FAILED') diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py index ac8eb27..7b4cf36 100644 --- a/zaza/openstack/charm_tests/test_utils.py +++ b/zaza/openstack/charm_tests/test_utils.py @@ -50,7 +50,7 @@ def skipUntilVersion(service, package, release): stderr=subprocess.STDOUT, universal_newlines=True) return f(*args, **kwargs) - except subprocess.CalledProcessError as cp: + except subprocess.CalledProcessError: logging.warn("Skipping test for older ({})" "service {}, requested {}".format( package_version, service, release)) diff --git a/zaza/openstack/utilities/generic.py b/zaza/openstack/utilities/generic.py index 3a22404..64596d6 100644 --- a/zaza/openstack/utilities/generic.py +++ b/zaza/openstack/utilities/generic.py @@ -534,28 +534,28 @@ def get_file_contents(unit, f): def is_port_open(port, address): - """Determine if TCP port is accessible. + """Determine if TCP port is accessible. - Connect to the MySQL port on the VIP. + Connect to the MySQL port on the VIP. - :param port: Port number - :type port: str - :param address: IP address - :type port: str - :returns: True if port is reachable - :rtype: boolean - """ - try: - telnetlib.Telnet(address, port) - return True - except socket.error as e: - if e.errno == 113: - logging.error("could not connect to {}:{}" - .format(address, port)) - if e.errno == 111: - logging.error("connection refused connecting" - " to {}:{}".format(address, port)) - return False + :param port: Port number + :type port: str + :param address: IP address + :type port: str + :returns: True if port is reachable + :rtype: boolean + """ + try: + telnetlib.Telnet(address, port) + return True + except socket.error as e: + if e.errno == 113: + logging.error("could not connect to {}:{}" + .format(address, port)) + if e.errno == 111: + logging.error("connection refused connecting" + " to {}:{}".format(address, port)) + return False def port_knock_units(units, port=22, expect_success=True): diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index df10e6e..f4f1374 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -1411,11 +1411,11 @@ def get_os_code_info(package, pkg_version): pkg_version = pkg_version.split(':')[1:][0] if 'swift' in package: # Fully x.y.z match for swift versions - match = re.match('^(\d+)\.(\d+)\.(\d+)', pkg_version) + match = re.match(r'^(\d+)\.(\d+)\.(\d+)', pkg_version) else: # x.y match only for 20XX.X # and ignore patch level for other packages - match = re.match('^(\d+)\.(\d+)', pkg_version) + match = re.match(r'^(\d+)\.(\d+)', pkg_version) if match: vers = match.group(0) From 116da9ca6cf5130ef21f94e7caf9e995fe9a4014 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Tue, 7 Apr 2020 10:55:09 +0200 Subject: [PATCH 2/3] octavia: tear down resources created during LBaasV2 test Fixes #221 --- zaza/openstack/charm_tests/octavia/tests.py | 69 ++++++++++++++++----- 1 file changed, 54 insertions(+), 15 deletions(-) diff --git a/zaza/openstack/charm_tests/octavia/tests.py b/zaza/openstack/charm_tests/octavia/tests.py index bd23cc9..a5265e4 100644 --- a/zaza/openstack/charm_tests/octavia/tests.py +++ b/zaza/openstack/charm_tests/octavia/tests.py @@ -18,6 +18,8 @@ import logging import subprocess import tenacity +import osc_lib.exceptions + import zaza.openstack.charm_tests.test_utils as test_utils import zaza.openstack.utilities.openstack as openstack_utils @@ -47,15 +49,52 @@ class LBAASv2Test(test_utils.OpenStackBaseTest): """Run class setup for running LBaaSv2 service tests.""" super(LBAASv2Test, cls).setUpClass() + cls.keystone_session = openstack_utils.get_overcloud_keystone_session() + cls.neutron_client = openstack_utils.get_neutron_session_client( + cls.keystone_session) + cls.octavia_client = openstack_utils.get_octavia_session_client( + cls.keystone_session) + + # NOTE(fnordahl): in the event of a test failure we do not want to run + # tear down code as it will make debugging a problem virtually + # impossible. To alleviate each test method will set the + # `run_tearDown` instance variable at the end which will let us run + # tear down only when there were no failure. + cls.run_tearDown = False + # List of load balancers created by this test + cls.loadbalancers = [] + # LIst of floating IPs created by this test + cls.fips = [] + + @classmethod + def tearDown(cls): + """Remove resources created during test execution. + + Note that resources created in the configure step prior to executing + the test should not be touched here. + """ + for lb in cls.loadbalancers: + cls.octavia_client.load_balancer_delete(lb['id'], cascade=True) + try: + cls.wait_for_lb_resource( + cls.octavia_client.load_balancer_show, lb['id'], + provisioning_status='DELETED') + except osc_lib.exceptions.NotFound: + pass + for fip in cls.fips: + cls.neutron_client.delete_floatingip(fip) + @staticmethod - @tenacity.retry(wait=tenacity.wait_fixed(1), - reraise=True, stop=tenacity.stop_after_delay(900)) + @tenacity.retry(retry=tenacity.retry_if_exception_type(AssertionError), + wait=tenacity.wait_fixed(1), reraise=True, + stop=tenacity.stop_after_delay(900)) def wait_for_lb_resource(octavia_show_func, resource_id, - operating_status=None): + provisioning_status=None, operating_status=None): """Wait for loadbalancer resource to reach expected status.""" + provisioning_status = provisioning_status or 'ACTIVE' resp = octavia_show_func(resource_id) logging.info(resp['provisioning_status']) - assert resp['provisioning_status'] == 'ACTIVE', ( + assert resp['provisioning_status'] == provisioning_status, ( 'load balancer resource has not reached ' 'expected provisioning status: {}' .format(resp)) @@ -197,11 +236,8 @@ class LBAASv2Test(test_utils.OpenStackBaseTest): def test_create_loadbalancer(self): """Create load balancer.""" - keystone_session = openstack_utils.get_overcloud_keystone_session() - neutron_client = openstack_utils.get_neutron_session_client( - keystone_session) nova_client = openstack_utils.get_nova_session_client( - keystone_session) + self.keystone_session) # Get IP of the prepared payload instances payload_ips = [] @@ -209,24 +245,24 @@ class LBAASv2Test(test_utils.OpenStackBaseTest): payload_ips.append(server.networks['private'][0]) self.assertTrue(len(payload_ips) > 0) - resp = neutron_client.list_networks(name='private') + resp = self.neutron_client.list_networks(name='private') subnet_id = resp['networks'][0]['subnets'][0] if openstack_utils.dvr_enabled(): - resp = neutron_client.list_networks(name='private_lb_fip_network') + resp = self.neutron_client.list_networks( + name='private_lb_fip_network') vip_subnet_id = resp['networks'][0]['subnets'][0] else: vip_subnet_id = subnet_id - octavia_client = openstack_utils.get_octavia_session_client( - keystone_session) - for provider in self.get_lb_providers(octavia_client).keys(): + for provider in self.get_lb_providers(self.octavia_client).keys(): logging.info('Creating loadbalancer with provider {}' .format(provider)) - lb = self._create_lb_resources(octavia_client, provider, + lb = self._create_lb_resources(self.octavia_client, provider, vip_subnet_id, subnet_id, payload_ips) + self.loadbalancers.append(lb) lb_fp = openstack_utils.create_floating_ip( - neutron_client, 'ext_net', port={'id': lb['vip_port_id']}) + self.neutron_client, 'ext_net', port={'id': lb['vip_port_id']}) snippet = 'This is the default welcome page' assert snippet in self._get_payload(lb_fp['floating_ip_address']) @@ -234,3 +270,6 @@ class LBAASv2Test(test_utils.OpenStackBaseTest): ' (provider="{}") at "http://{}/"' .format(snippet, provider, lb_fp['floating_ip_address'])) + + # If we get here, it means the tests passed + self.run_tearDown = True From 91c120a650283c91a5b92bdf00ccb930f60bcc08 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Wed, 8 Apr 2020 07:35:14 +0200 Subject: [PATCH 3/3] octavia: only run tearDown on success --- zaza/openstack/charm_tests/octavia/tests.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/zaza/openstack/charm_tests/octavia/tests.py b/zaza/openstack/charm_tests/octavia/tests.py index a5265e4..b189484 100644 --- a/zaza/openstack/charm_tests/octavia/tests.py +++ b/zaza/openstack/charm_tests/octavia/tests.py @@ -73,6 +73,8 @@ class LBAASv2Test(test_utils.OpenStackBaseTest): Note that resources created in the configure step prior to executing the test should not be touched here. """ + if not cls.run_tearDown: + return for lb in cls.loadbalancers: cls.octavia_client.load_balancer_delete(lb['id'], cascade=True) try: