From 440ee4b274f6ea71c312dad332cd1f08daf42267 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Sun, 18 Oct 2020 22:28:32 +0200 Subject: [PATCH 01/16] octavia: Disable check for member operational status Temporarily disable this check until we figure out why operational_status sometimes does not become 'ONLINE' while the member does indeed work and the subsequent retrieval of payload through loadbalancer is successful ref LP: #1896729. --- zaza/openstack/charm_tests/octavia/tests.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/zaza/openstack/charm_tests/octavia/tests.py b/zaza/openstack/charm_tests/octavia/tests.py index eb0b217..e95a33f 100644 --- a/zaza/openstack/charm_tests/octavia/tests.py +++ b/zaza/openstack/charm_tests/octavia/tests.py @@ -280,7 +280,13 @@ class LBAASv2Test(test_utils.OpenStackBaseTest): lambda x: octavia_client.member_show( pool_id=pool_id, member_id=x), member_id, - operating_status='ONLINE' if monitor else '') + operating_status='') + # Temporarily disable this check until we figure out why + # operational_status sometimes does not become 'ONLINE' + # while the member does indeed work and the subsequent + # retrieval of payload through loadbalancer is successful + # ref LP: #1896729. + # operating_status='ONLINE' if monitor else '') logging.info(resp) return lb From 67aaa10ad6f15d9be513a5205b7871a18f90ebf2 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Tue, 20 Oct 2020 14:43:47 +0200 Subject: [PATCH 02/16] Retry Nova flavor- and keypair-creation Fixes #452 --- zaza/openstack/charm_tests/nova/setup.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/zaza/openstack/charm_tests/nova/setup.py b/zaza/openstack/charm_tests/nova/setup.py index c556c97..7f8ee53 100644 --- a/zaza/openstack/charm_tests/nova/setup.py +++ b/zaza/openstack/charm_tests/nova/setup.py @@ -12,7 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Code for configureing nova.""" +"""Code for configuring nova.""" + +import tenacity import zaza.openstack.utilities.openstack as openstack_utils from zaza.openstack.utilities import ( @@ -21,6 +23,9 @@ from zaza.openstack.utilities import ( import zaza.openstack.charm_tests.nova.utils as nova_utils +@tenacity.retry(stop=tenacity.stop_after_attempt(3), + wait=tenacity.wait_exponential( + multiplier=1, min=2, max=10)) def create_flavors(nova_client=None): """Create basic flavors. @@ -43,6 +48,9 @@ def create_flavors(nova_client=None): flavorid=nova_utils.FLAVORS[flavor]['flavorid']) +@tenacity.retry(stop=tenacity.stop_after_attempt(3), + wait=tenacity.wait_exponential( + multiplier=1, min=2, max=10)) def manage_ssh_key(nova_client=None): """Create basic flavors. From 9961c9468bac26dfe6675ada46fdd137316a5b4f Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Wed, 13 Jan 2021 09:42:34 +0100 Subject: [PATCH 03/16] Sync pertinent tox.ini settings from release-tools The new PIP resolver wreaks havoc for this repository. Long term we should split unit test requirements into a separate file and whip unit tests into shape wrt. mocking out everything as opposed to relying on having random modules installed in the test environment. --- tox.ini | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tox.ini b/tox.ini index 7d67e9f..dc09948 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,22 @@ [tox] envlist = pep8, py3 skipsdist = True +# NOTE: Avoid build/test env pollution by not enabling sitepackages. +sitepackages = False +# NOTE: Avoid false positives by not skipping missing interpreters. +skip_missing_interpreters = False +# NOTES: +# * We avoid the new dependency resolver by pinning pip < 20.3, see +# https://github.com/pypa/pip/issues/9187 +# * Pinning dependencies requires tox >= 3.2.0, see +# https://tox.readthedocs.io/en/latest/config.html#conf-requires +# * It is also necessary to pin virtualenv as a newer virtualenv would still +# lead to fetching the latest pip in the func* tox targets, see +# https://stackoverflow.com/a/38133283 +requires = pip < 20.3 + virtualenv < 20.0 +# NOTE: https://wiki.canonical.com/engineering/OpenStack/InstallLatestToxOnOsci +minversion = 3.2.0 [testenv] setenv = VIRTUAL_ENV={envdir} From ee0dd80cec01f9bf8649851ba4c052f92cbee463 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Tue, 12 Jan 2021 17:11:35 +0100 Subject: [PATCH 04/16] Split configure_gateway_ext_port function The function does three separate things today, and two of its tasks are useful for other provider types such as MAAS. Also fix create_additional_port_for_machines idempotency. We previously added a run time assertion to fail early when attempting to configure networking for an invalid bundle. The check had the side effect of prohibiting subsequent runs on already configured models. --- .../test_zaza_utilities_openstack.py | 78 +++-- zaza/openstack/utilities/openstack.py | 327 ++++++++++++------ 2 files changed, 274 insertions(+), 131 deletions(-) diff --git a/unit_tests/utilities/test_zaza_utilities_openstack.py b/unit_tests/utilities/test_zaza_utilities_openstack.py index 48408e0..7e0e8f1 100644 --- a/unit_tests/utilities/test_zaza_utilities_openstack.py +++ b/unit_tests/utilities/test_zaza_utilities_openstack.py @@ -1262,34 +1262,68 @@ class TestOpenStackUtils(ut_utils.BaseTestCase): self.get_application.side_effect = KeyError self.assertFalse(openstack_utils.ngw_present()) - def test_configure_gateway_ext_port(self): - # FIXME: this is not a complete unit test for the function as one did - # not exist at all I'm adding this to test one bit and we'll add more - # as we go. + def test_get_charm_networking_data(self): self.patch_object(openstack_utils, 'deprecated_external_networking') self.patch_object(openstack_utils, 'dvr_enabled') self.patch_object(openstack_utils, 'ovn_present') self.patch_object(openstack_utils, 'ngw_present') + self.patch_object(openstack_utils, 'get_ovs_uuids') self.patch_object(openstack_utils, 'get_gateway_uuids') - self.patch_object(openstack_utils, 'get_admin_net') + self.patch_object(openstack_utils, 'get_ovn_uuids') + self.patch_object(openstack_utils.model, 'get_application') self.dvr_enabled.return_value = False self.ovn_present.return_value = False - self.ngw_present.return_value = True - self.get_admin_net.return_value = {'id': 'fakeid'} + self.ngw_present.return_value = False + self.get_ovs_uuids.return_value = [] + self.get_gateway_uuids.return_value = [] + self.get_ovn_uuids.return_value = [] + self.get_application.side_effect = KeyError - novaclient = mock.MagicMock() - neutronclient = mock.MagicMock() - - def _fake_empty_generator(empty=True): - if empty: - return - yield - - self.get_gateway_uuids.side_effect = _fake_empty_generator with self.assertRaises(RuntimeError): - openstack_utils.configure_gateway_ext_port( - novaclient, neutronclient) - # provide a uuid and check that we don't raise RuntimeError - self.get_gateway_uuids.side_effect = ['fake-uuid'] - openstack_utils.configure_gateway_ext_port( - novaclient, neutronclient) + openstack_utils.get_charm_networking_data() + self.ngw_present.return_value = True + self.assertEquals( + openstack_utils.get_charm_networking_data(), + openstack_utils.CharmedOpenStackNetworkingData( + openstack_utils.OpenStackNetworkingTopology.ML2_OVS, + ['neutron-gateway'], + mock.ANY, + 'data-port', + {})) + self.dvr_enabled.return_value = True + self.assertEquals( + openstack_utils.get_charm_networking_data(), + openstack_utils.CharmedOpenStackNetworkingData( + openstack_utils.OpenStackNetworkingTopology.ML2_OVS_DVR, + ['neutron-gateway', 'neutron-openvswitch'], + mock.ANY, + 'data-port', + {})) + self.ngw_present.return_value = False + self.assertEquals( + openstack_utils.get_charm_networking_data(), + openstack_utils.CharmedOpenStackNetworkingData( + openstack_utils.OpenStackNetworkingTopology.ML2_OVS_DVR_SNAT, + ['neutron-openvswitch'], + mock.ANY, + 'data-port', + {})) + self.dvr_enabled.return_value = False + self.ovn_present.return_value = True + self.assertEquals( + openstack_utils.get_charm_networking_data(), + openstack_utils.CharmedOpenStackNetworkingData( + openstack_utils.OpenStackNetworkingTopology.ML2_OVN, + ['ovn-chassis'], + mock.ANY, + 'bridge-interface-mappings', + {'ovn-bridge-mappings': 'physnet1:br-ex'})) + self.get_application.side_effect = None + self.assertEquals( + openstack_utils.get_charm_networking_data(), + openstack_utils.CharmedOpenStackNetworkingData( + openstack_utils.OpenStackNetworkingTopology.ML2_OVN, + ['ovn-chassis', 'ovn-dedicated-chassis'], + mock.ANY, + 'bridge-interface-mappings', + {'ovn-bridge-mappings': 'physnet1:br-ex'})) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index 33f0e02..33b8a52 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -16,7 +16,10 @@ This module contains a number of functions for interacting with OpenStack. """ +import collections +import copy import datetime +import enum import io import itertools import juju_wait @@ -721,6 +724,211 @@ def add_interface_to_netplan(server_name, mac_address): model.run_on_unit(unit_name, "sudo netplan apply") +class OpenStackNetworkingTopology(enum.Enum): + """OpenStack Charms Network Topologies.""" + + ML2_OVS = 'ML2+OVS' + ML2_OVS_DVR = 'ML2+OVS+DVR' + ML2_OVS_DVR_SNAT = 'ML2+OVS+DVR, no dedicated GWs' + ML2_OVN = 'ML2+OVN' + + +CharmedOpenStackNetworkingData = collections.namedtuple( + 'CharmedOpenStackNetworkingData', + [ + 'topology', + 'application_names', + 'unit_machine_ids', + 'port_config_key', + 'other_config', + ]) + + +def get_charm_networking_data(limit_gws=None): + """Inspect Juju model, determine networking topology and return data. + + :param limit_gws: Limit the number of gateways that get a port attached + :type limit_gws: Optional[int] + :rtype: CharmedOpenStackNetworkingData[ + OpenStackNetworkingTopology, + List[str], + Iterator[str], + str, + Dict[str,str]] + :returns: Named Tuple with networking data, example: + CharmedOpenStackNetworkingData( + OpenStackNetworkingTopology.ML2_OVN, + ['ovn-chassis', 'ovn-dedicated-chassis'], + ['machine-id-1', 'machine-id-2'], # generator object + 'bridge-interface-mappings', + {'ovn-bridge-mappings': 'physnet1:br-ex'}) + :raises: RuntimeError + """ + # Initialize defaults, these will be amended to fit the reality of the + # model in the checks below. + topology = OpenStackNetworkingTopology.ML2_OVS + other_config = {} + port_config_key = ( + 'data-port' if not deprecated_external_networking() else 'ext-port') + unit_machine_ids = [] + application_names = [] + + if dvr_enabled(): + if ngw_present(): + application_names = ['neutron-gateway', 'neutron-openvswitch'] + topology = OpenStackNetworkingTopology.ML2_OVS_DVR + else: + application_names = ['neutron-openvswitch'] + topology = OpenStackNetworkingTopology.ML2_OVS_DVR_SNAT + unit_machine_ids = itertools.islice( + itertools.chain( + get_ovs_uuids(), + get_gateway_uuids()), + limit_gws) + elif ngw_present(): + unit_machine_ids = itertools.islice( + get_gateway_uuids(), limit_gws) + application_names = ['neutron-gateway'] + elif ovn_present(): + topology = OpenStackNetworkingTopology.ML2_OVN + unit_machine_ids = itertools.islice(get_ovn_uuids(), limit_gws) + application_names = ['ovn-chassis'] + try: + ovn_dc_name = 'ovn-dedicated-chassis' + model.get_application(ovn_dc_name) + application_names.append(ovn_dc_name) + except KeyError: + # ovn-dedicated-chassis not in deployment + pass + port_config_key = 'bridge-interface-mappings' + other_config.update({'ovn-bridge-mappings': 'physnet1:br-ex'}) + else: + raise RuntimeError('Unable to determine charm network topology.') + + return CharmedOpenStackNetworkingData( + topology, + application_names, + unit_machine_ids, + port_config_key, + other_config) + + +def create_additional_port_for_machines(novaclient, neutronclient, net_id, + unit_machine_ids, + add_dataport_to_netplan=False): + """Create additional port for machines for use with external networking. + + :param novaclient: Undercloud Authenticated novaclient. + :type novaclient: novaclient.Client object + :param neutronclient: Undercloud Authenticated neutronclient. + :type neutronclient: neutronclient.Client object + :param net_id: Network ID to create ports on. + :type net_id: string + :param unit_machine_ids: Juju provider specific machine IDs for which we + should add ports on. + :type unit_machine_ids: Iterator[str] + :param add_dataport_to_netplan: Whether the newly created port should be + added to instance system configuration so + that it is brought up on instance reboot. + :type add_dataport_to_netplan: Optional[bool] + :returns: List of MAC addresses for created ports. + :rtype: List[str] + :raises: RuntimeError + """ + eligible_machines = 0 + for uuid in unit_machine_ids: + eligible_machines += 1 + server = novaclient.servers.get(uuid) + ext_port_name = "{}_ext-port".format(server.name) + for port in neutronclient.list_ports(device_id=server.id)['ports']: + if port['name'] == ext_port_name: + logging.warning( + 'Instance {} already has additional port, skipping.' + .format(server.id)) + break + else: + logging.info('Attaching additional port to instance ("{}"), ' + 'connected to net id: {}' + .format(uuid, net_id)) + body_value = { + "port": { + "admin_state_up": True, + "name": ext_port_name, + "network_id": net_id, + "port_security_enabled": False, + } + } + port = neutronclient.create_port(body=body_value) + server.interface_attach(port_id=port['port']['id'], + net_id=None, fixed_ip=None) + if add_dataport_to_netplan: + mac_address = get_mac_from_port(port, neutronclient) + add_interface_to_netplan(server.name, + mac_address=mac_address) + if not eligible_machines: + # NOTE: unit_machine_ids may be an iterator so testing it for contents + # or length prior to iterating over it is futile. + raise RuntimeError('Unable to determine UUIDs for machines to attach ' + 'external networking to.') + + # Retrieve the just created ports from Neutron so that we can provide our + # caller with their MAC addresses. + return [ + port['mac_address'] + for port in neutronclient.list_ports(network_id=net_id)['ports'] + if 'ext-port' in port['name'] + ] + + +def configure_networking_charms(networking_data, macs, use_juju_wait=True): + """Configure external networking for networking charms. + + :param networking_data: Data on networking charm topology. + :type networking_data: CharmedOpenStackNetworkingData + :param macs: MAC addresses of ports for use with external networking. + :type macs: Iterator[str] + :param use_juju_wait: Whether to use juju wait to wait for the model to + settle once the gateway has been configured. Default is True + :type use_juju_wait: Optional[bool] + """ + br_mac_fmt = 'br-ex:{}' if not deprecated_external_networking() else '{}' + br_mac = [ + br_mac_fmt.format(mac) + for mac in macs + ] + + config = copy.deepcopy(networking_data.other_config) + config.update({networking_data.port_config_key: ' '.join(sorted(br_mac))}) + + for application_name in networking_data.application_names: + logging.info('Setting {} on {}'.format( + config, application_name)) + current_data_port = get_application_config_option( + application_name, + networking_data.port_config_key) + if current_data_port == config[networking_data.port_config_key]: + logging.info('Config already set to value') + return + + model.set_application_config( + application_name, + configuration=config) + # NOTE(fnordahl): We are stuck with juju_wait until we figure out how + # to deal with all the non ['active', 'idle', 'Unit is ready.'] + # workload/agent states and msgs that our mojo specs are exposed to. + if use_juju_wait: + juju_wait.wait(wait_for_workload=True) + else: + zaza.model.wait_for_agent_status() + # TODO: shouldn't access get_charm_config() here as it relies on + # ./tests/tests.yaml existing by default (regardless of the + # fatal=False) ... it's not great design. + test_config = zaza.charm_lifecycle.utils.get_charm_config( + fatal=False) + zaza.model.wait_for_application_states( + states=test_config.get('target_deploy_status', {})) + + def configure_gateway_ext_port(novaclient, neutronclient, net_id=None, add_dataport_to_netplan=False, limit_gws=None, @@ -739,123 +947,24 @@ def configure_gateway_ext_port(novaclient, neutronclient, net_id=None, settle once the gateway has been configured. Default is True :type use_juju_wait: boolean """ - deprecated_extnet_mode = deprecated_external_networking() - - port_config_key = 'data-port' - if deprecated_extnet_mode: - port_config_key = 'ext-port' - - config = {} - if dvr_enabled(): - uuids = itertools.islice(itertools.chain(get_ovs_uuids(), - get_gateway_uuids()), - limit_gws) + networking_data = get_charm_networking_data(limit_gws=limit_gws) + if networking_data.topology in ( + OpenStackNetworkingTopology.ML2_OVS_DVR, + OpenStackNetworkingTopology.ML2_OVS_DVR_SNAT): # If dvr, do not attempt to persist nic in netplan # https://github.com/openstack-charmers/zaza-openstack-tests/issues/78 add_dataport_to_netplan = False - application_names = ['neutron-openvswitch'] - try: - ngw = 'neutron-gateway' - model.get_application(ngw) - application_names.append(ngw) - except KeyError: - # neutron-gateway not in deployment - pass - elif ngw_present(): - uuids = itertools.islice(get_gateway_uuids(), limit_gws) - application_names = ['neutron-gateway'] - elif ovn_present(): - uuids = itertools.islice(get_ovn_uuids(), limit_gws) - application_names = ['ovn-chassis'] - try: - ovn_dc_name = 'ovn-dedicated-chassis' - model.get_application(ovn_dc_name) - application_names.append(ovn_dc_name) - except KeyError: - # ovn-dedicated-chassis not in deployment - pass - port_config_key = 'bridge-interface-mappings' - config.update({'ovn-bridge-mappings': 'physnet1:br-ex'}) - add_dataport_to_netplan = True - else: - raise RuntimeError('Unable to determine charm network topology.') if not net_id: net_id = get_admin_net(neutronclient)['id'] - ports_created = 0 - for uuid in uuids: - server = novaclient.servers.get(uuid) - ext_port_name = "{}_ext-port".format(server.name) - for port in neutronclient.list_ports(device_id=server.id)['ports']: - if port['name'] == ext_port_name: - logging.warning( - 'Neutron Gateway already has additional port') - break - else: - logging.info('Attaching additional port to instance ("{}"), ' - 'connected to net id: {}' - .format(uuid, net_id)) - body_value = { - "port": { - "admin_state_up": True, - "name": ext_port_name, - "network_id": net_id, - "port_security_enabled": False, - } - } - port = neutronclient.create_port(body=body_value) - ports_created += 1 - server.interface_attach(port_id=port['port']['id'], - net_id=None, fixed_ip=None) - if add_dataport_to_netplan: - mac_address = get_mac_from_port(port, neutronclient) - add_interface_to_netplan(server.name, - mac_address=mac_address) - if not ports_created: - # NOTE: uuids is an iterator so testing it for contents or length prior - # to iterating over it is futile. - raise RuntimeError('Unable to determine UUIDs for machines to attach ' - 'external networking to.') + macs = create_additional_port_for_machines( + novaclient, neutronclient, net_id, networking_data.unit_machine_ids, + add_dataport_to_netplan) - ext_br_macs = [] - for port in neutronclient.list_ports(network_id=net_id)['ports']: - if 'ext-port' in port['name']: - if deprecated_extnet_mode: - ext_br_macs.append(port['mac_address']) - else: - ext_br_macs.append('br-ex:{}'.format(port['mac_address'])) - ext_br_macs.sort() - ext_br_macs_str = ' '.join(ext_br_macs) - - if ext_br_macs: - config.update({port_config_key: ext_br_macs_str}) - for application_name in application_names: - logging.info('Setting {} on {}'.format( - config, application_name)) - current_data_port = get_application_config_option(application_name, - port_config_key) - if current_data_port == ext_br_macs_str: - logging.info('Config already set to value') - return - - model.set_application_config( - application_name, - configuration=config) - # NOTE(fnordahl): We are stuck with juju_wait until we figure out how - # to deal with all the non ['active', 'idle', 'Unit is ready.'] - # workload/agent states and msgs that our mojo specs are exposed to. - if use_juju_wait: - juju_wait.wait(wait_for_workload=True) - else: - zaza.model.wait_for_agent_status() - # TODO: shouldn't access get_charm_config() here as it relies on - # ./tests/tests.yaml existing by default (regardless of the - # fatal=False) ... it's not great design. - test_config = zaza.charm_lifecycle.utils.get_charm_config( - fatal=False) - zaza.model.wait_for_application_states( - states=test_config.get('target_deploy_status', {})) + if macs: + configure_networking_charms( + networking_data, macs, use_juju_wait=use_juju_wait) @tenacity.retry(wait=tenacity.wait_exponential(multiplier=1, max=60), From ec637329740247d8384917c9dbe4c57cf6857de2 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Wed, 13 Jan 2021 12:28:39 +0100 Subject: [PATCH 05/16] Support configuring networknig charms on MAAS When on MAAS support doing charm based configuration of OVS by retrieving MAC address of ports attached to external network from MAAS. Note that we should extend the MAAS support to also work with deployments where MAAS does the OVS configuration for us. --- unit_tests/__init__.py | 5 +++++ zaza/openstack/charm_tests/neutron/setup.py | 18 ++++++++++++++-- zaza/openstack/utilities/openstack.py | 23 +++++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/unit_tests/__init__.py b/unit_tests/__init__.py index 8203d13..03c4879 100644 --- a/unit_tests/__init__.py +++ b/unit_tests/__init__.py @@ -11,3 +11,8 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + +import sys +import unittest.mock as mock + +sys.modules['zaza.utilities.maas'] = mock.MagicMock() diff --git a/zaza/openstack/charm_tests/neutron/setup.py b/zaza/openstack/charm_tests/neutron/setup.py index f65b87a..a1d1dd4 100644 --- a/zaza/openstack/charm_tests/neutron/setup.py +++ b/zaza/openstack/charm_tests/neutron/setup.py @@ -15,6 +15,7 @@ """Setup for Neutron deployments.""" import functools +import logging from zaza.openstack.configure import ( network, @@ -89,12 +90,25 @@ def basic_overcloud_network(limit_gws=None): 'configure_gateway_ext_port_use_juju_wait', True) # Handle network for OpenStack-on-OpenStack scenarios - if juju_utils.get_provider_type() == "openstack": + provider_type = juju_utils.get_provider_type() + if provider_type == "openstack": undercloud_ks_sess = openstack_utils.get_undercloud_keystone_session() network.setup_gateway_ext_port(network_config, keystone_session=undercloud_ks_sess, - limit_gws=None, + limit_gws=limit_gws, use_juju_wait=use_juju_wait) + elif provider_type == "maas": + # NOTE(fnordahl): After validation of the MAAS+Netplan Open vSwitch + # integration support, we would most likely want to add multiple modes + # of operation with MAAS. + # + # Perform charm based OVS configuration + openstack_utils.configure_charmed_openstack_on_maas( + network_config, limit_gws=limit_gws) + else: + logging.warning('Unknown Juju provider type, "{}", will not perform' + ' charm network configuration.' + .format(provider_type)) # Confugre the overcloud network network.setup_sdn(network_config, keystone_session=keystone_session) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index 33b8a52..6e0fa20 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -62,6 +62,7 @@ from keystoneauth1.identity import ( import zaza.openstack.utilities.cert as cert import zaza.utilities.deployment_env as deployment_env import zaza.utilities.juju as juju_utils +import zaza.utilities.maas from novaclient import client as novaclient_client from neutronclient.v2_0 import client as neutronclient from neutronclient.common import exceptions as neutronexceptions @@ -967,6 +968,28 @@ def configure_gateway_ext_port(novaclient, neutronclient, net_id=None, networking_data, macs, use_juju_wait=use_juju_wait) +def configure_charmed_openstack_on_maas(network_config, limit_gws=None): + """Configure networking charms for charm-based OVS config on MAAS provider. + + :param network_config: Network configuration as provided in environment. + :type network_config: Dict[str] + :param limit_gws: Limit the number of gateways that get a port attached + :type limit_gws: Optional[int] + """ + networking_data = get_charm_networking_data(limit_gws=limit_gws) + macs = [ + mim.mac + for mim in zaza.utilities.maas.get_macs_from_cidr( + zaza.utilities.maas.get_maas_client_from_juju_cloud_data( + zaza.model.get_cloud_data()), + network_config['external_net_cidr'], + link_mode=zaza.utilities.maas.LinkMode.LINK_UP) + ] + if macs: + configure_networking_charms( + networking_data, macs, use_juju_wait=False) + + @tenacity.retry(wait=tenacity.wait_exponential(multiplier=1, max=60), reraise=True, retry=tenacity.retry_if_exception_type(KeyError)) def get_mac_from_port(port, neutronclient): From 2b715da7e43b94ad36975063764f33738cc7aa61 Mon Sep 17 00:00:00 2001 From: Aurelien Lourot Date: Thu, 14 Jan 2021 10:15:34 +0100 Subject: [PATCH 06/16] Make HaclusterScalebackTest more robust (#482) Note that this test will be superseded by #369 in the future. --- zaza/openstack/charm_tests/hacluster/tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/zaza/openstack/charm_tests/hacluster/tests.py b/zaza/openstack/charm_tests/hacluster/tests.py index af47b76..1690c85 100644 --- a/zaza/openstack/charm_tests/hacluster/tests.py +++ b/zaza/openstack/charm_tests/hacluster/tests.py @@ -125,8 +125,8 @@ class HaclusterScalebackTest(HaclusterBaseTest): logging.info('Waiting for model to settle') zaza.model.block_until_unit_wl_status(other_hacluster_unit, 'active') - # NOTE(lourot): the principle application remains blocked after scaling - # back up until lp:1400481 is solved. - zaza.model.block_until_unit_wl_status(other_principle_unit, 'blocked') + # NOTE(lourot): the principle application sometimes remain blocked + # after scaling back up until lp:1400481 is solved. + # zaza.model.block_until_unit_wl_status(other_principle_unit, 'active') zaza.model.block_until_all_units_idle() logging.debug('OK') From 35840a66d6788dc4899847747ee7523895f46e1b Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Sat, 16 Jan 2021 15:28:13 +0100 Subject: [PATCH 07/16] nova: Conditional security checklist based on presence of vault The current test expects Nova to never have TLS connections, let's expect them to be there whenever vault is present. Remove the 'is-volume-encryption-enabled' assertion as it is not a property of the Nova security checks. This was previously masked by the fact that action would always fail due to TLS tests not being enabled for any bundles. --- zaza/openstack/charm_tests/nova/tests.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/zaza/openstack/charm_tests/nova/tests.py b/zaza/openstack/charm_tests/nova/tests.py index b8e7552..edad683 100644 --- a/zaza/openstack/charm_tests/nova/tests.py +++ b/zaza/openstack/charm_tests/nova/tests.py @@ -495,17 +495,24 @@ class SecurityTests(test_utils.OpenStackBaseTest): # Changes fixing the below expected failures will be made following # this initial work to get validation in. There will be bugs targeted # to each one and resolved independently where possible. - expected_failures = [ - 'is-volume-encryption-enabled', - 'validate-uses-tls-for-glance', - 'validate-uses-tls-for-keystone', ] expected_passes = [ 'validate-file-ownership', 'validate-file-permissions', 'validate-uses-keystone', ] + tls_checks = [ + 'validate-uses-tls-for-glance', + 'validate-uses-tls-for-keystone', + ] + if zaza.model.get_relation_id( + 'nova-cloud-controller', + 'vault', + remote_interface_name='certificates'): + expected_passes.extend(tls_checks) + else: + expected_failures.extend(tls_checks) for unit in zaza.model.get_units(self.application_name, model_name=self.model_name): @@ -519,4 +526,4 @@ class SecurityTests(test_utils.OpenStackBaseTest): action_params={}), expected_passes, expected_failures, - expected_to_pass=False) + expected_to_pass=not len(expected_failures)) From 44adc1fb944403d7d19a7fc2772c910fa8213399 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Tue, 19 Jan 2021 10:42:20 +0000 Subject: [PATCH 08/16] Fix policyd call to _login 8681b023 changed the signature of _login but did not update the policyd test. This was not immediately picked up because the policyd test is skipped before groovy due to Bug #1880959 Closes-Bug: #1911923 --- .../charm_tests/openstack_dashboard/tests.py | 102 ++++++++++-------- 1 file changed, 58 insertions(+), 44 deletions(-) diff --git a/zaza/openstack/charm_tests/openstack_dashboard/tests.py b/zaza/openstack/charm_tests/openstack_dashboard/tests.py index d3fc625..9588c6f 100644 --- a/zaza/openstack/charm_tests/openstack_dashboard/tests.py +++ b/zaza/openstack/charm_tests/openstack_dashboard/tests.py @@ -67,7 +67,6 @@ def _login(dashboard_url, domain, username, password, cafile=None): # start session, get csrftoken client = requests.session() client.get(auth_url, verify=cafile) - if 'csrftoken' in client.cookies: csrftoken = client.cookies['csrftoken'] else: @@ -163,7 +162,60 @@ def _do_request(request, cafile=None): return urllib.request.urlopen(request, cafile=cafile) -class OpenStackDashboardTests(test_utils.OpenStackBaseTest): +class OpenStackDashboardBase(): + """Mixin for interacting with Horizon.""" + + def get_base_url(self): + """Return the base url for http(s) requests. + + :returns: URL + :rtype: str + """ + vip = (zaza_model.get_application_config(self.application_name) + .get("vip").get("value")) + if vip: + ip = vip + else: + unit = zaza_model.get_unit_from_name( + zaza_model.get_lead_unit_name(self.application_name)) + ip = unit.public_address + + logging.debug("Dashboard ip is:{}".format(ip)) + scheme = 'http' + if self.use_https: + scheme = 'https' + url = '{}://{}'.format(scheme, ip) + return url + + def get_horizon_url(self): + """Return the url for acccessing horizon. + + :returns: Horizon URL + :rtype: str + """ + url = '{}/horizon'.format(self.get_base_url()) + logging.info("Horizon URL is: {}".format(url)) + return url + + @property + def use_https(self): + """Whether dashboard is using https. + + :returns: Whether dashboard is using https + :rtype: boolean + """ + use_https = False + vault_relation = zaza_model.get_relation_id( + self.application, + 'vault', + remote_interface_name='certificates') + if vault_relation: + use_https = True + return use_https + + +class OpenStackDashboardTests(test_utils.OpenStackBaseTest, + OpenStackDashboardBase): """Encapsulate openstack dashboard charm tests.""" @classmethod @@ -171,13 +223,6 @@ class OpenStackDashboardTests(test_utils.OpenStackBaseTest): """Run class setup for running openstack dashboard charm tests.""" super(OpenStackDashboardTests, cls).setUpClass() cls.application = 'openstack-dashboard' - cls.use_https = False - vault_relation = zaza_model.get_relation_id( - cls.application, - 'vault', - remote_interface_name='certificates') - if vault_relation: - cls.use_https = True def test_050_local_settings_permissions_regression_check_lp1755027(self): """Assert regression check lp1755027. @@ -302,39 +347,6 @@ class OpenStackDashboardTests(test_utils.OpenStackBaseTest): mismatches.append(msg) return mismatches - def get_base_url(self): - """Return the base url for http(s) requests. - - :returns: URL - :rtype: str - """ - vip = (zaza_model.get_application_config(self.application_name) - .get("vip").get("value")) - if vip: - ip = vip - else: - unit = zaza_model.get_unit_from_name( - zaza_model.get_lead_unit_name(self.application_name)) - ip = unit.public_address - - logging.debug("Dashboard ip is:{}".format(ip)) - scheme = 'http' - if self.use_https: - scheme = 'https' - url = '{}://{}'.format(scheme, ip) - logging.debug("Base URL is: {}".format(url)) - return url - - def get_horizon_url(self): - """Return the url for acccessing horizon. - - :returns: Horizon URL - :rtype: str - """ - url = '{}/horizon'.format(self.get_base_url()) - logging.info("Horizon URL is: {}".format(url)) - return url - def test_400_connection(self): """Test that dashboard responds to http request. @@ -450,7 +462,8 @@ class OpenStackDashboardTests(test_utils.OpenStackBaseTest): logging.info("Testing pause resume") -class OpenStackDashboardPolicydTests(policyd.BasePolicydSpecialization): +class OpenStackDashboardPolicydTests(policyd.BasePolicydSpecialization, + OpenStackDashboardBase): """Test the policyd override using the dashboard.""" good = { @@ -476,6 +489,7 @@ class OpenStackDashboardPolicydTests(policyd.BasePolicydSpecialization): super(OpenStackDashboardPolicydTests, cls).setUpClass( application_name="openstack-dashboard") cls.application_name = "openstack-dashboard" + cls.application = cls.application_name def get_client_and_attempt_operation(self, ip): """Attempt to list users on the openstack-dashboard service. @@ -500,7 +514,7 @@ class OpenStackDashboardPolicydTests(policyd.BasePolicydSpecialization): username = 'admin', password = overcloud_auth['OS_PASSWORD'], client, response = _login( - unit.public_address, domain, username, password) + self.get_horizon_url(), domain, username, password) # now attempt to get the domains page _url = self.url.format(unit.public_address) result = client.get(_url) From a828774c4868c6c74fc69e6b0f2a1e690a64d5d9 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Sat, 23 Jan 2021 16:17:28 +0000 Subject: [PATCH 09/16] Handle change of CA cert. Closes issue #487 --- zaza/openstack/charm_tests/keystone/setup.py | 4 +- zaza/openstack/charm_tests/keystone/tests.py | 4 +- zaza/openstack/charm_tests/policyd/tests.py | 3 +- zaza/openstack/charm_tests/vault/setup.py | 4 +- zaza/openstack/charm_tests/vault/tests.py | 4 +- zaza/openstack/utilities/openstack.py | 82 +++++++++++++++++--- 6 files changed, 85 insertions(+), 16 deletions(-) diff --git a/zaza/openstack/charm_tests/keystone/setup.py b/zaza/openstack/charm_tests/keystone/setup.py index 748439f..8d64993 100644 --- a/zaza/openstack/charm_tests/keystone/setup.py +++ b/zaza/openstack/charm_tests/keystone/setup.py @@ -41,9 +41,11 @@ def wait_for_cacert(model_name=None): :type model_name: str """ logging.info("Waiting for cacert") + cert_file = openstack_utils.get_cert_file_name( + 'keystone') zaza.model.block_until_file_has_contents( 'keystone', - openstack_utils.KEYSTONE_REMOTE_CACERT, + cert_file, 'CERTIFICATE', model_name=model_name) zaza.model.block_until_all_units_idle(model_name=model_name) diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py index b2830ef..477152d 100644 --- a/zaza/openstack/charm_tests/keystone/tests.py +++ b/zaza/openstack/charm_tests/keystone/tests.py @@ -229,7 +229,7 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest): 'OS_DOMAIN_NAME': DEMO_DOMAIN, } if self.tls_rid: - openrc['OS_CACERT'] = openstack_utils.KEYSTONE_LOCAL_CACERT + openrc['OS_CACERT'] = openstack_utils.get_cacert() openrc['OS_AUTH_URL'] = ( openrc['OS_AUTH_URL'].replace('http', 'https')) logging.info('keystone IP {}'.format(ip)) @@ -259,7 +259,7 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest): """ def _validate_token_data(openrc): if self.tls_rid: - openrc['OS_CACERT'] = openstack_utils.KEYSTONE_LOCAL_CACERT + openrc['OS_CACERT'] = openstack_utils.get_cacert() openrc['OS_AUTH_URL'] = ( openrc['OS_AUTH_URL'].replace('http', 'https')) logging.info('keystone IP {}'.format(ip)) diff --git a/zaza/openstack/charm_tests/policyd/tests.py b/zaza/openstack/charm_tests/policyd/tests.py index eca316d..4722755 100644 --- a/zaza/openstack/charm_tests/policyd/tests.py +++ b/zaza/openstack/charm_tests/policyd/tests.py @@ -337,8 +337,7 @@ class BasePolicydSpecialization(PolicydTest, logging.info('Authentication for {} on keystone IP {}' .format(openrc['OS_USERNAME'], ip)) if self.tls_rid: - openrc['OS_CACERT'] = \ - openstack_utils.KEYSTONE_LOCAL_CACERT + openrc['OS_CACERT'] = openstack_utils.get_cacert() openrc['OS_AUTH_URL'] = ( openrc['OS_AUTH_URL'].replace('http', 'https')) logging.info('keystone IP {}'.format(ip)) diff --git a/zaza/openstack/charm_tests/vault/setup.py b/zaza/openstack/charm_tests/vault/setup.py index 1e4e05e..d605063 100644 --- a/zaza/openstack/charm_tests/vault/setup.py +++ b/zaza/openstack/charm_tests/vault/setup.py @@ -222,9 +222,11 @@ def validate_ca(cacertificate, application="keystone", port=5000): :returns: None :rtype: None """ + cert_file = zaza.openstack.utilities.openstack.get_cert_file_name( + application) zaza.model.block_until_file_has_contents( application, - zaza.openstack.utilities.openstack.KEYSTONE_REMOTE_CACERT, + cert_file, cacertificate.decode().strip()) vip = (zaza.model.get_application_config(application) .get("vip").get("value")) diff --git a/zaza/openstack/charm_tests/vault/tests.py b/zaza/openstack/charm_tests/vault/tests.py index 40227fd..e96f36a 100644 --- a/zaza/openstack/charm_tests/vault/tests.py +++ b/zaza/openstack/charm_tests/vault/tests.py @@ -154,9 +154,11 @@ class VaultTest(BaseVaultTest): test_config = lifecycle_utils.get_charm_config() del test_config['target_deploy_status']['vault'] + cert_file = zaza.openstack.utilities.openstack.get_cert_file_name( + 'keystone') zaza.model.block_until_file_has_contents( 'keystone', - zaza.openstack.utilities.openstack.KEYSTONE_REMOTE_CACERT, + cert_file, cacert.decode().strip()) zaza.model.wait_for_application_states( states=test_config.get('target_deploy_status', {})) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index 6e0fa20..b914c0f 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -69,6 +69,7 @@ from neutronclient.common import exceptions as neutronexceptions from octaviaclient.api.v2 import octavia as octaviaclient from swiftclient import client as swiftclient +from juju.errors import JujuError import zaza @@ -181,10 +182,68 @@ WORKLOAD_STATUS_EXCEPTIONS = { 'ceilometer and gnocchi')}} # For vault TLS certificates +LOCAL_CERT_DIR = "tests" KEYSTONE_CACERT = "keystone_juju_ca_cert.crt" KEYSTONE_REMOTE_CACERT = ( "/usr/local/share/ca-certificates/{}".format(KEYSTONE_CACERT)) -KEYSTONE_LOCAL_CACERT = ("tests/{}".format(KEYSTONE_CACERT)) +KEYSTONE_LOCAL_CACERT = ("{}/{}".format(LOCAL_CERT_DIR, KEYSTONE_CACERT)) + +VAULT_CACERT = "vault_juju_ca_cert.crt" +VAULT_REMOTE_CACERT = ( + "/usr/local/share/ca-certificates/{}".format(VAULT_CACERT)) +VAULT_LOCAL_CACERT = ("{}/{}".format(LOCAL_CERT_DIR, VAULT_CACERT)) + +REMOTE_CERTIFICATES = [VAULT_REMOTE_CACERT, KEYSTONE_REMOTE_CACERT] +LOCAL_CERTIFICATES = [VAULT_LOCAL_CACERT, KEYSTONE_LOCAL_CACERT] + + +async def async_get_cert_file_name(app, cert_files=None, block=True, + model_name=None, timeout=2700): + """Get the name of the CA cert file thats on all units of an application. + + :param app: Name of application + :type capp: str + :param cert_files: List of cert files to search for. + :type cert_files: List[str] + :param block: Whether to block until a consistent cert file is found. + :type block: bool + :param model_name: Name of model to run check in + :type model_name: str + :param timeout: Time to wait for consistent file + :type timeout: int + :returns: Credentials dictionary + :rtype: dict + """ + async def _check_for_file(model, cert_files): + units = model.applications[app].units + results = {u.entity_id: [] for u in units} + for unit in units: + try: + for cf in cert_files: + output = await unit.run('test -e "{}"; echo $?'.format(cf)) + contents = output.data.get('results')['Stdout'] + if "0" in contents: + results[unit.entity_id].append(cf) + except JujuError: + pass + for cert_file in cert_files: + # Check that the certificate file exists on all the units. + if all(cert_file in files for files in results.values()): + return cert_file + else: + return None + + if not cert_files: + cert_files = REMOTE_CERTIFICATES + cert_file = None + async with zaza.model.run_in_model(model_name) as model: + if block: + await zaza.model.async_block_until( + lambda: _check_for_file(model, cert_files), timeout=timeout) + cert_file = await _check_for_file(model, cert_files) + return cert_file + +get_cert_file_name = zaza.model.sync_wrapper(async_get_cert_file_name) def get_cacert(): @@ -193,8 +252,9 @@ def get_cacert(): :returns: Path to CA Certificate bundle or None. :rtype: Optional[str] """ - if os.path.exists(KEYSTONE_LOCAL_CACERT): - return KEYSTONE_LOCAL_CACERT + for _cert in LOCAL_CERTIFICATES: + if os.path.exists(_cert): + return _cert # OpenStack Client helpers @@ -1951,24 +2011,28 @@ def get_overcloud_auth(address=None, model_name=None): 'API_VERSION': 3, } if tls_rid: + cert_file = get_cert_file_name('keystone', model_name=model_name) unit = model.get_first_unit_name('keystone', model_name=model_name) # ensure that the path to put the local cacert in actually exists. The # assumption that 'tests/' exists for, say, mojo is false. # Needed due to: # commit: 537473ad3addeaa3d1e4e2d0fd556aeaa4018eb2 - _dir = os.path.dirname(KEYSTONE_LOCAL_CACERT) + _dir = os.path.dirname(cert_file) if not os.path.exists(_dir): os.makedirs(_dir) + _local_cert_file = "{}/{}".format( + LOCAL_CERT_DIR, + os.path.basename(cert_file)) model.scp_from_unit( unit, - KEYSTONE_REMOTE_CACERT, - KEYSTONE_LOCAL_CACERT) + cert_file, + _local_cert_file) - if os.path.exists(KEYSTONE_LOCAL_CACERT): - os.chmod(KEYSTONE_LOCAL_CACERT, 0o644) - auth_settings['OS_CACERT'] = KEYSTONE_LOCAL_CACERT + if os.path.exists(_local_cert_file): + os.chmod(_local_cert_file, 0o644) + auth_settings['OS_CACERT'] = _local_cert_file return auth_settings From a20733cd1460ada0987ebb2999086eb6dd77cee8 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Sun, 24 Jan 2021 14:31:29 +0000 Subject: [PATCH 10/16] Refactor ca functions --- zaza/openstack/charm_tests/keystone/setup.py | 4 +- zaza/openstack/charm_tests/vault/setup.py | 7 +- zaza/openstack/charm_tests/vault/tests.py | 4 +- zaza/openstack/utilities/openstack.py | 214 +++++++++++++------ 4 files changed, 153 insertions(+), 76 deletions(-) diff --git a/zaza/openstack/charm_tests/keystone/setup.py b/zaza/openstack/charm_tests/keystone/setup.py index 8d64993..bec43fe 100644 --- a/zaza/openstack/charm_tests/keystone/setup.py +++ b/zaza/openstack/charm_tests/keystone/setup.py @@ -41,9 +41,7 @@ def wait_for_cacert(model_name=None): :type model_name: str """ logging.info("Waiting for cacert") - cert_file = openstack_utils.get_cert_file_name( - 'keystone') - zaza.model.block_until_file_has_contents( + zaza.openstack.utilities.openstack.block_until_ca_exists( 'keystone', cert_file, 'CERTIFICATE', diff --git a/zaza/openstack/charm_tests/vault/setup.py b/zaza/openstack/charm_tests/vault/setup.py index d605063..ad1d535 100644 --- a/zaza/openstack/charm_tests/vault/setup.py +++ b/zaza/openstack/charm_tests/vault/setup.py @@ -222,9 +222,7 @@ def validate_ca(cacertificate, application="keystone", port=5000): :returns: None :rtype: None """ - cert_file = zaza.openstack.utilities.openstack.get_cert_file_name( - application) - zaza.model.block_until_file_has_contents( + zaza.openstack.utilities.openstack.block_until_ca_exists( application, cert_file, cacertificate.decode().strip()) @@ -238,3 +236,6 @@ def validate_ca(cacertificate, application="keystone", port=5000): fp.write(cacertificate.decode()) fp.flush() requests.get('https://{}:{}'.format(ip, str(port)), verify=fp.name) + +def get_cert(): + print(zaza.openstack.utilities.openstack.get_remote_ca_cert_file('masakari')) diff --git a/zaza/openstack/charm_tests/vault/tests.py b/zaza/openstack/charm_tests/vault/tests.py index e96f36a..141a1e6 100644 --- a/zaza/openstack/charm_tests/vault/tests.py +++ b/zaza/openstack/charm_tests/vault/tests.py @@ -154,9 +154,7 @@ class VaultTest(BaseVaultTest): test_config = lifecycle_utils.get_charm_config() del test_config['target_deploy_status']['vault'] - cert_file = zaza.openstack.utilities.openstack.get_cert_file_name( - 'keystone') - zaza.model.block_until_file_has_contents( + zaza.openstack.utilities.openstack.block_until_ca_exists( 'keystone', cert_file, cacert.decode().strip()) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index b914c0f..7d2fc9d 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -27,6 +27,7 @@ import logging import os import paramiko import re +import shutil import six import subprocess import sys @@ -182,7 +183,10 @@ WORKLOAD_STATUS_EXCEPTIONS = { 'ceilometer and gnocchi')}} # For vault TLS certificates +CACERT_FILENAME_FORMAT = "{}_juju_ca_cert.crt" +CERT_PROVIDORS = ['vault'] LOCAL_CERT_DIR = "tests" +REMOTE_CERT_DIR = "/usr/local/share/ca-certificates" KEYSTONE_CACERT = "keystone_juju_ca_cert.crt" KEYSTONE_REMOTE_CACERT = ( "/usr/local/share/ca-certificates/{}".format(KEYSTONE_CACERT)) @@ -197,53 +201,91 @@ REMOTE_CERTIFICATES = [VAULT_REMOTE_CACERT, KEYSTONE_REMOTE_CACERT] LOCAL_CERTIFICATES = [VAULT_LOCAL_CACERT, KEYSTONE_LOCAL_CACERT] -async def async_get_cert_file_name(app, cert_files=None, block=True, - model_name=None, timeout=2700): - """Get the name of the CA cert file thats on all units of an application. - - :param app: Name of application - :type capp: str - :param cert_files: List of cert files to search for. - :type cert_files: List[str] - :param block: Whether to block until a consistent cert file is found. - :type block: bool - :param model_name: Name of model to run check in - :type model_name: str - :param timeout: Time to wait for consistent file - :type timeout: int - :returns: Credentials dictionary - :rtype: dict - """ - async def _check_for_file(model, cert_files): - units = model.applications[app].units - results = {u.entity_id: [] for u in units} - for unit in units: - try: - for cf in cert_files: - output = await unit.run('test -e "{}"; echo $?'.format(cf)) - contents = output.data.get('results')['Stdout'] - if "0" in contents: - results[unit.entity_id].append(cf) - except JujuError: - pass - for cert_file in cert_files: - # Check that the certificate file exists on all the units. - if all(cert_file in files for files in results.values()): - return cert_file +async def async_block_until_ca_exists(application_name, ca_cert, model_name=None, timeout=2700): + async def _check_ca_present(model, ca_files): + units = model.applications[application_name].units + print(ca_files) + for ca_file in ca_files: + for unit in units: + print(unit) + print(ca_file) + try: + output = await unit.run('cat {}'.format(ca_file)) + contents = output.data.get('results').get('Stdout', '') + if not ca_cert in contents: + print("It's not here!") + print(ca_cert) + print(contents) + break + if ca_cert in contents: + print("It's here!") + # libjuju throws a generic error for connection failure. So we + # cannot differentiate between a connectivity issue and a + # target file not existing error. For now just assume the + # latter. + except JujuError: + continue + else: + return True else: - return None - - if not cert_files: - cert_files = REMOTE_CERTIFICATES - cert_file = None + return False + ca_files = await _async_get_remote_ca_cert_file_candidates(application_name, model_name=model_name) + print(ca_files) async with zaza.model.run_in_model(model_name) as model: - if block: - await zaza.model.async_block_until( - lambda: _check_for_file(model, cert_files), timeout=timeout) - cert_file = await _check_for_file(model, cert_files) - return cert_file + await zaza.model.async_block_until( + lambda: _check_ca_present(model, ca_files), timeout=timeout) -get_cert_file_name = zaza.model.sync_wrapper(async_get_cert_file_name) +block_until_ca_exists = zaza.model.sync_wrapper(async_block_until_ca_exists) + + + +#async def async_get_cert_file_name(app, cert_files=None, block=True, +# model_name=None, timeout=2700): +# """Get the name of the CA cert file thats on all units of an application. +# +# :param app: Name of application +# :type capp: str +# :param cert_files: List of cert files to search for. +# :type cert_files: List[str] +# :param block: Whether to block until a consistent cert file is found. +# :type block: bool +# :param model_name: Name of model to run check in +# :type model_name: str +# :param timeout: Time to wait for consistent file +# :type timeout: int +# :returns: Credentials dictionary +# :rtype: dict +# """ +# async def _check_for_file(model, cert_files): +# units = model.applications[app].units +# results = {u.entity_id: [] for u in units} +# for unit in units: +# try: +# for cf in cert_files: +# output = await unit.run('test -e "{}"; echo $?'.format(cf)) +# contents = output.data.get('results')['Stdout'] +# if "0" in contents: +# results[unit.entity_id].append(cf) +# except JujuError: +# pass +# for cert_file in cert_files: +# # Check that the certificate file exists on all the units. +# if all(cert_file in files for files in results.values()): +# return cert_file +# else: +# return None +# +# if not cert_files: +# cert_files = REMOTE_CERTIFICATES +# cert_file = None +# async with zaza.model.run_in_model(model_name) as model: +# if block: +# await zaza.model.async_block_until( +# lambda: _check_for_file(model, cert_files), timeout=timeout) +# cert_file = await _check_for_file(model, cert_files) +# return cert_file +# +#get_cert_file_name = zaza.model.sync_wrapper(async_get_cert_file_name) def get_cacert(): @@ -2010,32 +2052,70 @@ def get_overcloud_auth(address=None, model_name=None): 'OS_PROJECT_DOMAIN_NAME': 'admin_domain', 'API_VERSION': 3, } - if tls_rid: - cert_file = get_cert_file_name('keystone', model_name=model_name) - unit = model.get_first_unit_name('keystone', model_name=model_name) - - # ensure that the path to put the local cacert in actually exists. The - # assumption that 'tests/' exists for, say, mojo is false. - # Needed due to: - # commit: 537473ad3addeaa3d1e4e2d0fd556aeaa4018eb2 - _dir = os.path.dirname(cert_file) - if not os.path.exists(_dir): - os.makedirs(_dir) - - _local_cert_file = "{}/{}".format( - LOCAL_CERT_DIR, - os.path.basename(cert_file)) - model.scp_from_unit( - unit, - cert_file, - _local_cert_file) - - if os.path.exists(_local_cert_file): - os.chmod(_local_cert_file, 0o644) - auth_settings['OS_CACERT'] = _local_cert_file + local_ca_cert = get_remote_ca_cert_file('keystone', model_name=model_name) + if ca_cert: + auth_settings['OS_CACERT'] = local_ca_cert return auth_settings +async def _async_get_remote_ca_cert_file_candidates(application, model_name=None): + cert_files = [] + # unit = model.get_first_unit_name(application, model_name=model_name) + units = await model.async_get_units(application, model_name=model_name) + unit = units[0].name + for _providor in CERT_PROVIDORS: + tls_rid = await model.async_get_relation_id( + application, + _providor, + model_name=model_name, + remote_interface_name='certificates') + if tls_rid: + cert_files.append(REMOTE_CERT_DIR + '/' + CACERT_FILENAME_FORMAT.format(_providor)) + cert_files.append(REMOTE_CERT_DIR + '/' + KEYSTONE_CACERT) + return cert_files + +_get_remote_ca_cert_file_candidates = zaza.model.sync_wrapper(_async_get_remote_ca_cert_file_candidates) + +def get_remote_ca_cert_file(application, model_name=None): +# CACERT_FILENAME = "{}_juju_ca_cert.crt" +# cert_files = [] +# unit = model.get_first_unit_name(application, model_name=model_name) +# for _providor in CERT_PROVIDORS: +# tls_rid = model.get_relation_id( +# application, +# _providor, +# model_name=model_name, +# remote_interface_name='certificates') +# if tls_rid: +# cert_files.append(CACERT_FILENAME.format(_providor)) +# cert_files.append(KEYSTONE_CACERT) + unit = model.get_first_unit_name(application, model_name=model_name) + local_cert_file = None + cert_files = _get_remote_ca_cert_file_candidates(application, model_name=model_name) + for cert_file in cert_files: + _local_cert_file = "{}/{}".format( + LOCAL_CERT_DIR, + os.path.basename(cert_file)) + with tempfile.NamedTemporaryFile(mode="w", delete=False) as _tmp_ca_file: + try: + model.scp_from_unit( + unit, + cert_file, + _tmp_ca_file.name) + except JujuError: + continue + # ensure that the path to put the local cacert in actually exists. The + # assumption that 'tests/' exists for, say, mojo is false. + # Needed due to: + # commit: 537473ad3addeaa3d1e4e2d0fd556aeaa4018eb2 + _dir = os.path.dirname(_local_cert_file) + if not os.path.exists(_dir): + os.makedirs(_dir) + shutil.move(_tmp_ca_file.name, _local_cert_file) + os.chmod(_local_cert_file, 0o644) + local_cert_file = _local_cert_file + break + return local_cert_file def get_urllib_opener(): """Create a urllib opener taking into account proxy settings. From 24fbc068c94f99a702f7b3f8a2a01b7de0bf855d Mon Sep 17 00:00:00 2001 From: Liam Young Date: Sun, 24 Jan 2021 15:27:06 +0000 Subject: [PATCH 11/16] Correct args to block_until_ca_exists --- zaza/openstack/charm_tests/keystone/setup.py | 1 - zaza/openstack/charm_tests/vault/setup.py | 1 - zaza/openstack/charm_tests/vault/tests.py | 1 - 3 files changed, 3 deletions(-) diff --git a/zaza/openstack/charm_tests/keystone/setup.py b/zaza/openstack/charm_tests/keystone/setup.py index bec43fe..73264cd 100644 --- a/zaza/openstack/charm_tests/keystone/setup.py +++ b/zaza/openstack/charm_tests/keystone/setup.py @@ -43,7 +43,6 @@ def wait_for_cacert(model_name=None): logging.info("Waiting for cacert") zaza.openstack.utilities.openstack.block_until_ca_exists( 'keystone', - cert_file, 'CERTIFICATE', model_name=model_name) zaza.model.block_until_all_units_idle(model_name=model_name) diff --git a/zaza/openstack/charm_tests/vault/setup.py b/zaza/openstack/charm_tests/vault/setup.py index ad1d535..25389d5 100644 --- a/zaza/openstack/charm_tests/vault/setup.py +++ b/zaza/openstack/charm_tests/vault/setup.py @@ -224,7 +224,6 @@ def validate_ca(cacertificate, application="keystone", port=5000): """ zaza.openstack.utilities.openstack.block_until_ca_exists( application, - cert_file, cacertificate.decode().strip()) vip = (zaza.model.get_application_config(application) .get("vip").get("value")) diff --git a/zaza/openstack/charm_tests/vault/tests.py b/zaza/openstack/charm_tests/vault/tests.py index 141a1e6..68130a0 100644 --- a/zaza/openstack/charm_tests/vault/tests.py +++ b/zaza/openstack/charm_tests/vault/tests.py @@ -156,7 +156,6 @@ class VaultTest(BaseVaultTest): del test_config['target_deploy_status']['vault'] zaza.openstack.utilities.openstack.block_until_ca_exists( 'keystone', - cert_file, cacert.decode().strip()) zaza.model.wait_for_application_states( states=test_config.get('target_deploy_status', {})) From d637646a9e139245029a4506afc2d0b71f2060bc Mon Sep 17 00:00:00 2001 From: Liam Young Date: Sun, 24 Jan 2021 16:19:45 +0000 Subject: [PATCH 12/16] Fix typo --- zaza/openstack/utilities/openstack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index 7d2fc9d..5fb4980 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -2053,7 +2053,7 @@ def get_overcloud_auth(address=None, model_name=None): 'API_VERSION': 3, } local_ca_cert = get_remote_ca_cert_file('keystone', model_name=model_name) - if ca_cert: + if local_ca_cert: auth_settings['OS_CACERT'] = local_ca_cert return auth_settings From 401829f0a7b657f1eb9b3d129844476ce869839d Mon Sep 17 00:00:00 2001 From: Liam Young Date: Sun, 24 Jan 2021 17:24:20 +0000 Subject: [PATCH 13/16] Code tidy and docstrings --- zaza/openstack/charm_tests/vault/setup.py | 3 - zaza/openstack/utilities/openstack.py | 152 ++++++++-------------- 2 files changed, 53 insertions(+), 102 deletions(-) diff --git a/zaza/openstack/charm_tests/vault/setup.py b/zaza/openstack/charm_tests/vault/setup.py index 25389d5..c792508 100644 --- a/zaza/openstack/charm_tests/vault/setup.py +++ b/zaza/openstack/charm_tests/vault/setup.py @@ -235,6 +235,3 @@ def validate_ca(cacertificate, application="keystone", port=5000): fp.write(cacertificate.decode()) fp.flush() requests.get('https://{}:{}'.format(ip, str(port)), verify=fp.name) - -def get_cert(): - print(zaza.openstack.utilities.openstack.get_remote_ca_cert_file('masakari')) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index 5fb4980..a975206 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -184,7 +184,7 @@ WORKLOAD_STATUS_EXCEPTIONS = { # For vault TLS certificates CACERT_FILENAME_FORMAT = "{}_juju_ca_cert.crt" -CERT_PROVIDORS = ['vault'] +CERT_PROVIDERS = ['vault'] LOCAL_CERT_DIR = "tests" REMOTE_CERT_DIR = "/usr/local/share/ca-certificates" KEYSTONE_CACERT = "keystone_juju_ca_cert.crt" @@ -192,33 +192,29 @@ KEYSTONE_REMOTE_CACERT = ( "/usr/local/share/ca-certificates/{}".format(KEYSTONE_CACERT)) KEYSTONE_LOCAL_CACERT = ("{}/{}".format(LOCAL_CERT_DIR, KEYSTONE_CACERT)) -VAULT_CACERT = "vault_juju_ca_cert.crt" -VAULT_REMOTE_CACERT = ( - "/usr/local/share/ca-certificates/{}".format(VAULT_CACERT)) -VAULT_LOCAL_CACERT = ("{}/{}".format(LOCAL_CERT_DIR, VAULT_CACERT)) -REMOTE_CERTIFICATES = [VAULT_REMOTE_CACERT, KEYSTONE_REMOTE_CACERT] -LOCAL_CERTIFICATES = [VAULT_LOCAL_CACERT, KEYSTONE_LOCAL_CACERT] +async def async_block_until_ca_exists(application_name, ca_cert, + model_name=None, timeout=2700): + """Block until a CA cert is on all units of application_name. - -async def async_block_until_ca_exists(application_name, ca_cert, model_name=None, timeout=2700): + :param application_name: Name of application to check + :type application_name: str + :param ca_cert: The certificate to look for. + :type ca_cert: str + :param model_name: Name of model to query. + :type model_name: str + :param timeout: How long in seconds to wait + :type timeout: int + """ async def _check_ca_present(model, ca_files): units = model.applications[application_name].units - print(ca_files) for ca_file in ca_files: for unit in units: - print(unit) - print(ca_file) try: output = await unit.run('cat {}'.format(ca_file)) contents = output.data.get('results').get('Stdout', '') - if not ca_cert in contents: - print("It's not here!") - print(ca_cert) - print(contents) + if ca_cert not in contents: break - if ca_cert in contents: - print("It's here!") # libjuju throws a generic error for connection failure. So we # cannot differentiate between a connectivity issue and a # target file not existing error. For now just assume the @@ -229,8 +225,9 @@ async def async_block_until_ca_exists(application_name, ca_cert, model_name=None return True else: return False - ca_files = await _async_get_remote_ca_cert_file_candidates(application_name, model_name=model_name) - print(ca_files) + ca_files = await _async_get_remote_ca_cert_file_candidates( + application_name, + model_name=model_name) async with zaza.model.run_in_model(model_name) as model: await zaza.model.async_block_until( lambda: _check_ca_present(model, ca_files), timeout=timeout) @@ -238,65 +235,19 @@ async def async_block_until_ca_exists(application_name, ca_cert, model_name=None block_until_ca_exists = zaza.model.sync_wrapper(async_block_until_ca_exists) - -#async def async_get_cert_file_name(app, cert_files=None, block=True, -# model_name=None, timeout=2700): -# """Get the name of the CA cert file thats on all units of an application. -# -# :param app: Name of application -# :type capp: str -# :param cert_files: List of cert files to search for. -# :type cert_files: List[str] -# :param block: Whether to block until a consistent cert file is found. -# :type block: bool -# :param model_name: Name of model to run check in -# :type model_name: str -# :param timeout: Time to wait for consistent file -# :type timeout: int -# :returns: Credentials dictionary -# :rtype: dict -# """ -# async def _check_for_file(model, cert_files): -# units = model.applications[app].units -# results = {u.entity_id: [] for u in units} -# for unit in units: -# try: -# for cf in cert_files: -# output = await unit.run('test -e "{}"; echo $?'.format(cf)) -# contents = output.data.get('results')['Stdout'] -# if "0" in contents: -# results[unit.entity_id].append(cf) -# except JujuError: -# pass -# for cert_file in cert_files: -# # Check that the certificate file exists on all the units. -# if all(cert_file in files for files in results.values()): -# return cert_file -# else: -# return None -# -# if not cert_files: -# cert_files = REMOTE_CERTIFICATES -# cert_file = None -# async with zaza.model.run_in_model(model_name) as model: -# if block: -# await zaza.model.async_block_until( -# lambda: _check_for_file(model, cert_files), timeout=timeout) -# cert_file = await _check_for_file(model, cert_files) -# return cert_file -# -#get_cert_file_name = zaza.model.sync_wrapper(async_get_cert_file_name) - - def get_cacert(): """Return path to CA Certificate bundle for verification during test. :returns: Path to CA Certificate bundle or None. - :rtype: Optional[str] + :rtype: Union[str, None] """ - for _cert in LOCAL_CERTIFICATES: + for _provider in CERT_PROVIDERS: + _cert = LOCAL_CERT_DIR + '/' + CACERT_FILENAME_FORMAT.format( + _provider) if os.path.exists(_cert): return _cert + if os.path.exists(KEYSTONE_LOCAL_CACERT): + return KEYSTONE_LOCAL_CACERT # OpenStack Client helpers @@ -2058,65 +2009,68 @@ def get_overcloud_auth(address=None, model_name=None): return auth_settings -async def _async_get_remote_ca_cert_file_candidates(application, model_name=None): + +async def _async_get_remote_ca_cert_file_candidates(application, + model_name=None): cert_files = [] - # unit = model.get_first_unit_name(application, model_name=model_name) - units = await model.async_get_units(application, model_name=model_name) - unit = units[0].name - for _providor in CERT_PROVIDORS: + for _provider in CERT_PROVIDERS: tls_rid = await model.async_get_relation_id( application, - _providor, + _provider, model_name=model_name, remote_interface_name='certificates') if tls_rid: - cert_files.append(REMOTE_CERT_DIR + '/' + CACERT_FILENAME_FORMAT.format(_providor)) - cert_files.append(REMOTE_CERT_DIR + '/' + KEYSTONE_CACERT) + cert_files.append( + REMOTE_CERT_DIR + '/' + CACERT_FILENAME_FORMAT.format( + _provider)) + cert_files.append(KEYSTONE_LOCAL_CACERT) return cert_files -_get_remote_ca_cert_file_candidates = zaza.model.sync_wrapper(_async_get_remote_ca_cert_file_candidates) +_get_remote_ca_cert_file_candidates = zaza.model.sync_wrapper( + _async_get_remote_ca_cert_file_candidates) + def get_remote_ca_cert_file(application, model_name=None): -# CACERT_FILENAME = "{}_juju_ca_cert.crt" -# cert_files = [] -# unit = model.get_first_unit_name(application, model_name=model_name) -# for _providor in CERT_PROVIDORS: -# tls_rid = model.get_relation_id( -# application, -# _providor, -# model_name=model_name, -# remote_interface_name='certificates') -# if tls_rid: -# cert_files.append(CACERT_FILENAME.format(_providor)) -# cert_files.append(KEYSTONE_CACERT) + """Collect CA certificate from application. + + :param application: Name of application to collect file from. + :type application: str + :param model_name: Name of model to query. + :type model_name: str + :returns: Path to cafile + :rtype: str + """ unit = model.get_first_unit_name(application, model_name=model_name) local_cert_file = None - cert_files = _get_remote_ca_cert_file_candidates(application, model_name=model_name) + cert_files = _get_remote_ca_cert_file_candidates( + application, + model_name=model_name) for cert_file in cert_files: _local_cert_file = "{}/{}".format( LOCAL_CERT_DIR, os.path.basename(cert_file)) - with tempfile.NamedTemporaryFile(mode="w", delete=False) as _tmp_ca_file: + with tempfile.NamedTemporaryFile(mode="w", delete=False) as _tmp_ca: try: model.scp_from_unit( unit, cert_file, - _tmp_ca_file.name) + _tmp_ca.name) except JujuError: continue - # ensure that the path to put the local cacert in actually exists. The - # assumption that 'tests/' exists for, say, mojo is false. + # ensure that the path to put the local cacert in actually exists. + # The assumption that 'tests/' exists for, say, mojo is false. # Needed due to: # commit: 537473ad3addeaa3d1e4e2d0fd556aeaa4018eb2 _dir = os.path.dirname(_local_cert_file) if not os.path.exists(_dir): os.makedirs(_dir) - shutil.move(_tmp_ca_file.name, _local_cert_file) + shutil.move(_tmp_ca.name, _local_cert_file) os.chmod(_local_cert_file, 0o644) local_cert_file = _local_cert_file break return local_cert_file + def get_urllib_opener(): """Create a urllib opener taking into account proxy settings. From e047150f5bae0b8a996cb57542491131348b92f1 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Mon, 25 Jan 2021 09:21:00 +0000 Subject: [PATCH 14/16] Add unit tests --- .../test_zaza_utilities_openstack.py | 157 +++++++++++++++++- ..._zaza_utilities_parallel_series_upgrade.py | 24 +-- unit_tests/utils.py | 22 +++ zaza/openstack/utilities/openstack.py | 12 +- 4 files changed, 190 insertions(+), 25 deletions(-) diff --git a/unit_tests/utilities/test_zaza_utilities_openstack.py b/unit_tests/utilities/test_zaza_utilities_openstack.py index 7e0e8f1..e4abfb1 100644 --- a/unit_tests/utilities/test_zaza_utilities_openstack.py +++ b/unit_tests/utilities/test_zaza_utilities_openstack.py @@ -17,6 +17,8 @@ import datetime import io import mock import subprocess +import sys +import unittest import tenacity import unit_tests.utils as ut_utils @@ -191,6 +193,7 @@ class TestOpenStackUtils(ut_utils.BaseTestCase): self.patch_object(openstack_utils, 'get_application_config_option') self.patch_object(openstack_utils, 'get_keystone_ip') self.patch_object(openstack_utils, "get_current_os_versions") + self.patch_object(openstack_utils, "get_remote_ca_cert_file") self.patch_object(openstack_utils.juju_utils, 'leader_get') if tls_relation: self.patch_object(openstack_utils.model, "scp_from_unit") @@ -204,6 +207,7 @@ class TestOpenStackUtils(ut_utils.BaseTestCase): self.get_relation_id.return_value = None self.get_application_config_option.return_value = None self.leader_get.return_value = 'openstack' + self.get_remote_ca_cert_file.return_value = None if tls_relation or ssl_cert: port = 35357 transport = 'https' @@ -245,7 +249,8 @@ class TestOpenStackUtils(ut_utils.BaseTestCase): 'API_VERSION': 3, } if tls_relation: - expect['OS_CACERT'] = openstack_utils.KEYSTONE_LOCAL_CACERT + self.get_remote_ca_cert_file.return_value = '/tmp/a.cert' + expect['OS_CACERT'] = '/tmp/a.cert' self.assertEqual(openstack_utils.get_overcloud_auth(), expect) @@ -1327,3 +1332,153 @@ class TestOpenStackUtils(ut_utils.BaseTestCase): mock.ANY, 'bridge-interface-mappings', {'ovn-bridge-mappings': 'physnet1:br-ex'})) + + def test_get_cacert(self): + self.patch_object(openstack_utils.os.path, 'exists') + results = { + 'tests/vault_juju_ca_cert.crt': True} + self.exists.side_effect = lambda x: results[x] + self.assertEqual( + openstack_utils.get_cacert(), + 'tests/vault_juju_ca_cert.crt') + + results = { + 'tests/vault_juju_ca_cert.crt': False, + 'tests/keystone_juju_ca_cert.crt': True} + self.assertEqual( + openstack_utils.get_cacert(), + 'tests/keystone_juju_ca_cert.crt') + + results = { + 'tests/vault_juju_ca_cert.crt': False, + 'tests/keystone_juju_ca_cert.crt': False} + self.assertIsNone(openstack_utils.get_cacert()) + + def test_get_remote_ca_cert_file(self): + self.patch_object(openstack_utils.model, 'get_first_unit_name') + self.patch_object( + openstack_utils, + '_get_remote_ca_cert_file_candidates') + self.patch_object(openstack_utils.model, 'scp_from_unit') + self.patch_object(openstack_utils.os.path, 'exists') + self.patch_object(openstack_utils.shutil, 'move') + self.patch_object(openstack_utils.os, 'chmod') + self.patch_object(openstack_utils.tempfile, 'NamedTemporaryFile') + enter_mock = mock.MagicMock() + enter_mock.__enter__.return_value.name = 'tempfilename' + self.NamedTemporaryFile.return_value = enter_mock + self.get_first_unit_name.return_value = 'neutron-api/0' + self._get_remote_ca_cert_file_candidates.return_value = [ + '/tmp/ca1.cert'] + self.exists.return_value = True + + openstack_utils.get_remote_ca_cert_file('neutron-api') + self.scp_from_unit.assert_called_once_with( + 'neutron-api/0', + '/tmp/ca1.cert', + 'tempfilename') + self.chmod.assert_called_once_with('tests/ca1.cert', 0o644) + self.move.assert_called_once_with('tempfilename', 'tests/ca1.cert') + + +class TestAsyncOpenstackUtils(ut_utils.AioTestCase): + + def setUp(self): + super(TestAsyncOpenstackUtils, self).setUp() + if sys.version_info < (3, 6, 0): + raise unittest.SkipTest("Can't AsyncMock in py35") + model_mock = mock.MagicMock() + test_mock = mock.MagicMock() + + class AsyncContextManagerMock(test_mock): + async def __aenter__(self): + yield model_mock + + async def __aexit__(self, *args): + pass + + self.model_mock = model_mock + self.patch_object(openstack_utils.zaza.model, "async_block_until") + + async def _block_until(f, timeout): + # Store the result of the call to _check_ca_present to validate + # tests + self.result = await f() + self.async_block_until.side_effect = _block_until + self.patch('zaza.model.run_in_model', name='_run_in_model') + self._run_in_model.return_value = AsyncContextManagerMock + self._run_in_model().__aenter__.return_value = self.model_mock + + async def test_async_block_until_ca_exists(self): + def _get_action_output(stdout, code, stderr=None): + stderr = stderr or '' + action = mock.MagicMock() + action.data = { + 'results': { + 'Code': code, + 'Stderr': stderr, + 'Stdout': stdout}} + return action + results = { + '/tmp/missing.cert': _get_action_output( + '', + '1', + 'cat: /tmp/missing.cert: No such file or directory'), + '/tmp/good.cert': _get_action_output('CERTIFICATE', '0')} + + async def _run(command, timeout=None): + return results[command.split()[-1]] + self.unit1 = mock.MagicMock() + self.unit2 = mock.MagicMock() + self.unit2.run.side_effect = _run + self.unit1.run.side_effect = _run + self.units = [self.unit1, self.unit2] + _units = mock.MagicMock() + _units.units = self.units + self.model_mock.applications = { + 'keystone': _units + } + self.patch_object( + openstack_utils, + "_async_get_remote_ca_cert_file_candidates") + + # Test a missing cert then a good cert. + self._async_get_remote_ca_cert_file_candidates.return_value = [ + '/tmp/missing.cert', + '/tmp/good.cert'] + await openstack_utils.async_block_until_ca_exists( + 'keystone', + 'CERTIFICATE') + self.assertTrue(self.result) + + # Test a single missing + self._async_get_remote_ca_cert_file_candidates.return_value = [ + '/tmp/missing.cert'] + await openstack_utils.async_block_until_ca_exists( + 'keystone', + 'CERTIFICATE') + self.assertFalse(self.result) + + async def test__async_get_remote_ca_cert_file_candidates(self): + self.patch_object(openstack_utils.zaza.model, "async_get_relation_id") + rel_id_out = { + } + + def _get_relation_id(app, cert_app, model_name, remote_interface_name): + return rel_id_out[cert_app] + self.async_get_relation_id.side_effect = _get_relation_id + + rel_id_out['vault'] = 'certs:1' + r = await openstack_utils._async_get_remote_ca_cert_file_candidates( + 'neutron-api', 'mymodel') + self.assertEqual( + r, + ['/usr/local/share/ca-certificates/vault_juju_ca_cert.crt', + '/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt']) + + rel_id_out['vault'] = None + r = await openstack_utils._async_get_remote_ca_cert_file_candidates( + 'neutron-api', 'mymodel') + self.assertEqual( + r, + ['/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt']) diff --git a/unit_tests/utilities/test_zaza_utilities_parallel_series_upgrade.py b/unit_tests/utilities/test_zaza_utilities_parallel_series_upgrade.py index 0231dbf..70ffd3b 100644 --- a/unit_tests/utilities/test_zaza_utilities_parallel_series_upgrade.py +++ b/unit_tests/utilities/test_zaza_utilities_parallel_series_upgrade.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import asyncio import mock import sys import unittest @@ -139,28 +138,7 @@ class Test_ParallelSeriesUpgradeSync(ut_utils.BaseTestCase): self.assertEqual(expected, config) -class AioTestCase(ut_utils.BaseTestCase): - def __init__(self, methodName='runTest', loop=None): - self.loop = loop or asyncio.get_event_loop() - self._function_cache = {} - super(AioTestCase, self).__init__(methodName=methodName) - - def coroutine_function_decorator(self, func): - def wrapper(*args, **kw): - return self.loop.run_until_complete(func(*args, **kw)) - return wrapper - - def __getattribute__(self, item): - attr = object.__getattribute__(self, item) - if asyncio.iscoroutinefunction(attr) and item.startswith('test_'): - if item not in self._function_cache: - self._function_cache[item] = ( - self.coroutine_function_decorator(attr)) - return self._function_cache[item] - return attr - - -class TestParallelSeriesUpgrade(AioTestCase): +class TestParallelSeriesUpgrade(ut_utils.AioTestCase): def setUp(self): super(TestParallelSeriesUpgrade, self).setUp() if sys.version_info < (3, 6, 0): diff --git a/unit_tests/utils.py b/unit_tests/utils.py index 4694d0d..8e31f45 100644 --- a/unit_tests/utils.py +++ b/unit_tests/utils.py @@ -19,6 +19,7 @@ """Module to provide helper for writing unit tests.""" +import asyncio import contextlib import io import mock @@ -96,3 +97,24 @@ class BaseTestCase(unittest.TestCase): started.return_value = return_value self._patches_start[name] = started setattr(self, name, started) + + +class AioTestCase(BaseTestCase): + def __init__(self, methodName='runTest', loop=None): + self.loop = loop or asyncio.get_event_loop() + self._function_cache = {} + super(AioTestCase, self).__init__(methodName=methodName) + + def coroutine_function_decorator(self, func): + def wrapper(*args, **kw): + return self.loop.run_until_complete(func(*args, **kw)) + return wrapper + + def __getattribute__(self, item): + attr = object.__getattribute__(self, item) + if asyncio.iscoroutinefunction(attr) and item.startswith('test_'): + if item not in self._function_cache: + self._function_cache[item] = ( + self.coroutine_function_decorator(attr)) + return self._function_cache[item] + return attr diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index a975206..d883e0a 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -222,6 +222,7 @@ async def async_block_until_ca_exists(application_name, ca_cert, except JujuError: continue else: + # The CA was found in `ca_file` on all units. return True else: return False @@ -2012,6 +2013,15 @@ def get_overcloud_auth(address=None, model_name=None): async def _async_get_remote_ca_cert_file_candidates(application, model_name=None): + """Return a list of possible remote CA file names. + + :param application: Name of application to examine. + :type application: str + :param model_name: Name of model to query. + :type model_name: str + :returns: List of paths to possible ca files. + :rtype: List[str] + """ cert_files = [] for _provider in CERT_PROVIDERS: tls_rid = await model.async_get_relation_id( @@ -2023,7 +2033,7 @@ async def _async_get_remote_ca_cert_file_candidates(application, cert_files.append( REMOTE_CERT_DIR + '/' + CACERT_FILENAME_FORMAT.format( _provider)) - cert_files.append(KEYSTONE_LOCAL_CACERT) + cert_files.append(KEYSTONE_REMOTE_CACERT) return cert_files _get_remote_ca_cert_file_candidates = zaza.model.sync_wrapper( From 5be8fc377179e8fce9de8e8e7d9744fc169d65a4 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Mon, 25 Jan 2021 11:41:39 +0000 Subject: [PATCH 15/16] Fix docstring and bug --- zaza/openstack/utilities/openstack.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index d883e0a..522df31 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -199,7 +199,7 @@ async def async_block_until_ca_exists(application_name, ca_cert, :param application_name: Name of application to check :type application_name: str - :param ca_cert: The certificate to look for. + :param ca_cert: The certificate content. :type ca_cert: str :param model_name: Name of model to query. :type model_name: str @@ -220,7 +220,7 @@ async def async_block_until_ca_exists(application_name, ca_cert, # target file not existing error. For now just assume the # latter. except JujuError: - continue + break else: # The CA was found in `ca_file` on all units. return True From 93a9aff92796ce626ae969fc0554f3411dc3adf2 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Mon, 25 Jan 2021 11:47:42 +0000 Subject: [PATCH 16/16] Fix context manager mock --- unit_tests/utilities/test_zaza_utilities_openstack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unit_tests/utilities/test_zaza_utilities_openstack.py b/unit_tests/utilities/test_zaza_utilities_openstack.py index e4abfb1..67cd417 100644 --- a/unit_tests/utilities/test_zaza_utilities_openstack.py +++ b/unit_tests/utilities/test_zaza_utilities_openstack.py @@ -1392,7 +1392,7 @@ class TestAsyncOpenstackUtils(ut_utils.AioTestCase): class AsyncContextManagerMock(test_mock): async def __aenter__(self): - yield model_mock + return self async def __aexit__(self, *args): pass