From 0bb84af4a444f01a70f952cca35b766fae1ccbbf Mon Sep 17 00:00:00 2001 From: Dmitrii Shcherbakov Date: Wed, 7 Jun 2023 23:41:29 +0300 Subject: [PATCH 1/8] Reuse the existing net setup code As a prerequisite to testing the data plane connectivity with routes advertised by NDR, this change makes the configuration step reuse the existing network config code that is able to do things like plugging an extra interface to be used in bridge-interface-mappings. --- .../charm_tests/dragent/configure.py | 52 ++----------------- 1 file changed, 5 insertions(+), 47 deletions(-) diff --git a/zaza/openstack/charm_tests/dragent/configure.py b/zaza/openstack/charm_tests/dragent/configure.py index 7a7ab7e..b9588d4 100644 --- a/zaza/openstack/charm_tests/dragent/configure.py +++ b/zaza/openstack/charm_tests/dragent/configure.py @@ -18,46 +18,15 @@ import logging import zaza.model -from zaza.openstack.configure import ( - network, - bgp_speaker, -) +from zaza.openstack.configure import bgp_speaker from zaza.openstack.utilities import ( - cli as cli_utils, generic as generic_utils, openstack as openstack_utils, ) +from zaza.openstack.charm_tests.neutron.setup import basic_overcloud_network DEFAULT_PEER_APPLICATION_NAME = "osci-frr" -# The overcloud network configuration settings are declared. -# These are the network configuration settings under test. -OVERCLOUD_NETWORK_CONFIG = { - "network_type": "gre", - "router_name": openstack_utils.PROVIDER_ROUTER, - "ip_version": "4", - "address_scope": "public", - "external_net_name": openstack_utils.EXT_NET, - "external_subnet_name": openstack_utils.EXT_NET_SUBNET, - "prefix_len": "24", - "subnetpool_name": "pooled_subnets", - "subnetpool_prefix": "192.168.0.0/16", -} - -# The undercloud network configuration settings are substrate specific to -# the environment where the tests are being executed. These settings may be -# overridden by environment variables. See the doc string documentation for -# zaza.openstack.utilities.generic_utils.get_undercloud_env_vars for the -# environment variables required to be exported and available to zaza. -# These are default settings provided as an example. -DEFAULT_UNDERCLOUD_NETWORK_CONFIG = { - "start_floating_ip": "10.5.150.0", - "end_floating_ip": "10.5.150.254", - "external_dns": "10.5.0.2", - "external_net_cidr": "10.5.0.0/16", - "default_gateway": "10.5.0.1", -} - def setup(): """Run setup for BGP networking. @@ -72,23 +41,12 @@ def setup(): :returns: None :rtype: None """ - cli_utils.setup_logging() + # Reuse the existing network configuration code. + basic_overcloud_network() - # Get network configuration settings - network_config = {} - # Declared overcloud settings - network_config.update(OVERCLOUD_NETWORK_CONFIG) - # Default undercloud settings - network_config.update(DEFAULT_UNDERCLOUD_NETWORK_CONFIG) - # Environment specific settings - network_config.update(generic_utils.get_undercloud_env_vars()) - - # Get keystone session + # Get a keystone session keystone_session = openstack_utils.get_overcloud_keystone_session() - # Confugre the overcloud network - network.setup_sdn(network_config, keystone_session=keystone_session) - # LP Bugs #1784083 and #1841459, require a late restart of the # neutron-bgp-dragent service logging.warning("Due to LP Bugs #1784083 and #1841459, we require a late " From 110242796ca854aa5c048bbd49ff922639db0c07 Mon Sep 17 00:00:00 2001 From: Dmitrii Shcherbakov Date: Wed, 7 Jun 2023 23:43:45 +0300 Subject: [PATCH 2/8] Allow an optional FIP service subnet to be set up A separate service subnet for FIPs is useful in making sure that connectivity based on the advertised routes really works as opposed to relying on directly connected routes to FIPs in the undercloud network subnet used as an external network. --- .../charm_tests/dragent/configure.py | 5 +++-- zaza/openstack/charm_tests/neutron/setup.py | 21 ++++++++++++++++++- zaza/openstack/configure/network.py | 17 +++++++++++++++ zaza/openstack/utilities/openstack.py | 9 +++++++- 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/zaza/openstack/charm_tests/dragent/configure.py b/zaza/openstack/charm_tests/dragent/configure.py index b9588d4..79c1720 100644 --- a/zaza/openstack/charm_tests/dragent/configure.py +++ b/zaza/openstack/charm_tests/dragent/configure.py @@ -41,8 +41,9 @@ def setup(): :returns: None :rtype: None """ - # Reuse the existing network configuration code. - basic_overcloud_network() + # Reuse the existing network configuration code but ask for a separate + # service subnet to be created for FIPs. + basic_overcloud_network(use_separate_fip_subnet=True) # Get a keystone session keystone_session = openstack_utils.get_overcloud_keystone_session() diff --git a/zaza/openstack/charm_tests/neutron/setup.py b/zaza/openstack/charm_tests/neutron/setup.py index d6a1ab2..f81590d 100644 --- a/zaza/openstack/charm_tests/neutron/setup.py +++ b/zaza/openstack/charm_tests/neutron/setup.py @@ -68,6 +68,17 @@ DEFAULT_UNDERCLOUD_NETWORK_CONFIG = { "default_gateway": "10.5.0.1", } +# For Neutron Dynamic Tests it is useful to avoid relying on the directly +# connected routes and instead using the advertised routes on the southbound +# path and default routes on the northbound path. To do that, a separate +# service subnet may be optionally created to force Neutron to use that instead +# of the external network subnet without concrete service IPs which is used as +# a fallback only. +DEFAULT_FIP_SERVICE_SUBNET_CONFIG = { + "fip_service_subnet_name": openstack_utils.FIP_SERVICE_SUBNET_NAME, + "fip_service_subnet_cidr": "100.64.0.0/24" +} + def undercloud_and_charm_setup(limit_gws=None): """Perform undercloud and charm setup for network plumbing. @@ -111,7 +122,7 @@ def undercloud_and_charm_setup(limit_gws=None): .format(provider_type)) -def basic_overcloud_network(limit_gws=None): +def basic_overcloud_network(limit_gws=None, use_separate_fip_subnet=False): """Run setup for neutron networking. Configure the following: @@ -119,6 +130,10 @@ def basic_overcloud_network(limit_gws=None): :param limit_gws: Limit the number of gateways that get a port attached :type limit_gws: int + :param use_separate_fip_subnet: Use a separate service subnet for floating + ips instead of relying on the external + network subnet for FIP allocations. + :type use_separate_fip_subnet: bool """ cli_utils.setup_logging() @@ -128,6 +143,10 @@ def basic_overcloud_network(limit_gws=None): network_config.update(OVERCLOUD_NETWORK_CONFIG) # Default undercloud settings network_config.update(DEFAULT_UNDERCLOUD_NETWORK_CONFIG) + + if use_separate_fip_subnet: + network_config.update(DEFAULT_FIP_SERVICE_SUBNET_CONFIG) + # Environment specific settings network_config.update(generic_utils.get_undercloud_env_vars()) diff --git a/zaza/openstack/configure/network.py b/zaza/openstack/configure/network.py index 20cde9f..33fab27 100755 --- a/zaza/openstack/configure/network.py +++ b/zaza/openstack/configure/network.py @@ -132,6 +132,23 @@ def setup_sdn(network_config, keystone_session=None): neutron_client, project_id, network_config["external_net_name"]) + + # If a separate service subnet for FIPs is requested, create one. This is + # useful for testing dynamic routing scenarios to avoid relying on directly + # connected routes to the external network subnet. + if network_config.get('fip_service_subnet_name'): + openstack_utils.create_provider_subnet( + neutron_client, + project_id, + ext_network, + subnet_name=network_config["fip_service_subnet_name"], + cidr=network_config["fip_service_subnet_cidr"], + # Disable DHCP as we don't need a metadata port serving this + # subnet while Neutron would fail to allocate a fixed IP for it + # with a service subnet constraint below. + dhcp=False, + service_types=['network:floatingip'] + ) openstack_utils.create_provider_subnet( neutron_client, project_id, diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index df540e5..19f8ce9 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -199,6 +199,9 @@ KEYSTONE_REMOTE_CACERT = ( # Network/router names EXT_NET = os.environ.get('TEST_EXT_NET', 'ext_net') EXT_NET_SUBNET = os.environ.get('TEST_EXT_NET_SUBNET', 'ext_net_subnet') +# An optional service subnet for FIPs is necessary. +FIP_SERVICE_SUBNET_NAME = os.environ.get('TEST_FIP_SERVICE_SUBNET_NAME', + 'fip_service_subnet') PRIVATE_NET = os.environ.get('TEST_PRIVATE_NET', 'private') PRIVATE_NET_SUBNET = os.environ.get('TEST_PRIVATE_NET_SUBNET', 'private_subnet') @@ -1301,7 +1304,7 @@ def create_provider_subnet(neutron_client, project_id, network, subnet_name=EXT_NET_SUBNET, default_gateway=None, cidr=None, start_floating_ip=None, end_floating_ip=None, - dhcp=False): + dhcp=False, service_types=None): """Create the provider subnet. :param neutron_client: Authenticated neutronclient @@ -1322,6 +1325,8 @@ def create_provider_subnet(neutron_client, project_id, network, :type end_floating_ip: string or None :param dhcp: Run DHCP on this subnet :type dhcp: boolean + :param service_types: Optional subnet service types + :type service_types: List[str] :returns: Subnet object :rtype: dict """ @@ -1345,6 +1350,8 @@ def create_provider_subnet(neutron_client, project_id, network, 'end': end_floating_ip, } subnet_msg['allocation_pools'] = [allocation_pool] + if service_types: + subnet_msg['service_types'] = service_types logging.info('Creating new subnet') subnet = neutron_client.create_subnet({'subnet': subnet_msg})['subnet'] From 77aeb7300f88e688625f7acb20bf6961087dae80 Mon Sep 17 00:00:00 2001 From: Dmitrii Shcherbakov Date: Thu, 15 Jun 2023 06:48:27 +0400 Subject: [PATCH 3/8] Allow skipping instance connectivity checks For the NDR data plane testing case those checks cannot be performed from the node that runs Zaza and instead need to be performed from a unit that receives dynamic routes from the BGP speaker. In order to allow for that case an additional argument is introduced. --- zaza/openstack/charm_tests/test_utils.py | 9 +++- zaza/openstack/configure/guest.py | 52 +++++++++++++----------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py index 9f4a8aa..c6873a6 100644 --- a/zaza/openstack/charm_tests/test_utils.py +++ b/zaza/openstack/charm_tests/test_utils.py @@ -796,7 +796,7 @@ class OpenStackBaseTest(BaseCharmTest): def launch_guest(self, guest_name, userdata=None, use_boot_volume=False, instance_key=None, flavor_name=None, attach_to_external_network=False, - keystone_session=None): + keystone_session=None, perform_connectivity_check=True): """Launch one guest to use in tests. Note that it is up to the caller to have set the RESOURCE_PREFIX class @@ -817,6 +817,9 @@ class OpenStackBaseTest(BaseCharmTest): network. :type attach_to_external_network: bool :param keystone_session: Keystone session to use. + :param perform_connectivity_check: Whether to perform a connectivity + check. + :type perform_connectivity_check: bool :type keystone_session: Optional[keystoneauth1.session.Session] :returns: Nova instance objects :rtype: Server @@ -848,7 +851,9 @@ class OpenStackBaseTest(BaseCharmTest): userdata=userdata, flavor_name=flavor_name, attach_to_external_network=attach_to_external_network, - keystone_session=keystone_session) + keystone_session=keystone_session, + perform_connectivity_check=perform_connectivity_check + ) def launch_guests(self, userdata=None, attach_to_external_network=False, flavor_name=None): diff --git a/zaza/openstack/configure/guest.py b/zaza/openstack/configure/guest.py index 059a17d..bcaccb1 100644 --- a/zaza/openstack/configure/guest.py +++ b/zaza/openstack/configure/guest.py @@ -95,7 +95,7 @@ def launch_instance(instance_key, use_boot_volume=False, vm_name=None, private_network_name=None, image_name=None, flavor_name=None, external_network_name=None, meta=None, userdata=None, attach_to_external_network=False, - keystone_session=None): + keystone_session=None, perform_connectivity_check=True): """Launch an instance. :param instance_key: Key to collect associated config data with. @@ -123,6 +123,8 @@ def launch_instance(instance_key, use_boot_volume=False, vm_name=None, :type attach_to_external_network: bool :param keystone_session: Keystone session to use. :type keystone_session: Optional[keystoneauth1.session.Session] + :param perform_connectivity_check: Whether to perform a connectivity check. + :type perform_connectivity_check: bool :returns: the created instance :rtype: novaclient.Server """ @@ -211,28 +213,30 @@ def launch_instance(instance_key, use_boot_volume=False, vm_name=None, external_network_name, port=port)['floating_ip_address'] logging.info('Assigned floating IP {} to {}'.format(ip, vm_name)) - try: - for attempt in Retrying( - stop=stop_after_attempt(8), - wait=wait_exponential(multiplier=1, min=2, max=60)): - with attempt: - try: - openstack_utils.ping_response(ip) - except subprocess.CalledProcessError as e: - logging.error('Pinging {} failed with {}' - .format(ip, e.returncode)) - logging.error('stdout: {}'.format(e.stdout)) - logging.error('stderr: {}'.format(e.stderr)) - raise - except RetryError: - raise openstack_exceptions.NovaGuestNoPingResponse() - # Check ssh'ing to instance. - logging.info('Testing ssh access.') - openstack_utils.ssh_test( - username=boot_tests[instance_key]['username'], - ip=ip, - vm_name=vm_name, - password=boot_tests[instance_key].get('password'), - privkey=openstack_utils.get_private_key(nova_utils.KEYPAIR_NAME)) + if perform_connectivity_check: + try: + for attempt in Retrying( + stop=stop_after_attempt(8), + wait=wait_exponential(multiplier=1, min=2, max=60)): + with attempt: + try: + openstack_utils.ping_response(ip) + except subprocess.CalledProcessError as e: + logging.error('Pinging {} failed with {}' + .format(ip, e.returncode)) + logging.error('stdout: {}'.format(e.stdout)) + logging.error('stderr: {}'.format(e.stderr)) + raise + except RetryError: + raise openstack_exceptions.NovaGuestNoPingResponse() + + # Check ssh'ing to instance. + logging.info('Testing ssh access.') + openstack_utils.ssh_test( + username=boot_tests[instance_key]['username'], + ip=ip, + vm_name=vm_name, + password=boot_tests[instance_key].get('password'), + privkey=openstack_utils.get_private_key(nova_utils.KEYPAIR_NAME)) return instance From 8fae64082625547f850f561b5fed5581f475cdd9 Mon Sep 17 00:00:00 2001 From: Dmitrii Shcherbakov Date: Wed, 14 Jun 2023 21:32:11 +0400 Subject: [PATCH 4/8] Add L3 data plane testing NDR tests In order to actually test L3 for advertised routes, a simple ping test is necessary to perform from a unit that receives dynamic routes. --- zaza/openstack/charm_tests/dragent/test.py | 104 ------------------- zaza/openstack/charm_tests/dragent/tests.py | 109 ++++++++++++++++++-- 2 files changed, 103 insertions(+), 110 deletions(-) delete mode 100644 zaza/openstack/charm_tests/dragent/test.py diff --git a/zaza/openstack/charm_tests/dragent/test.py b/zaza/openstack/charm_tests/dragent/test.py deleted file mode 100644 index 5b56fdc..0000000 --- a/zaza/openstack/charm_tests/dragent/test.py +++ /dev/null @@ -1,104 +0,0 @@ -#!/usr/bin/env python3 - -# Copyright 2018 Canonical Ltd. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# 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. - -"""Run BGP tests.""" - -import argparse -import logging -import sys -import tenacity - -from zaza import model -from zaza.openstack.utilities import ( - cli as cli_utils, - juju as juju_utils, - openstack as openstack_utils, -) - - -def test_bgp_routes(peer_application_name="osci-frr", keystone_session=None): - """Test BGP routes. - - :param peer_application_name: String name of BGP peer application - :type peer_application_name: string - :param keystone_session: Keystone session object for overcloud - :type keystone_session: keystoneauth1.session.Session object - :raises: AssertionError if expected BGP routes are not found - :returns: None - :rtype: None - """ - # If a session has not been provided, acquire one - if not keystone_session: - keystone_session = openstack_utils.get_overcloud_keystone_session() - - # Get authenticated clients - neutron_client = openstack_utils.get_neutron_session_client( - keystone_session) - - # Get the peer unit - peer_unit = model.get_units(peer_application_name)[0].entity_id - - # Get expected advertised routes - private_cidr = neutron_client.list_subnets( - name="private_subnet")["subnets"][0]["cidr"] - floating_ip_cidr = "{}/32".format( - neutron_client.list_floatingips() - ["floatingips"][0]["floating_ip_address"]) - - # This test may run immediately after configuration. It may take time for - # routes to propogate via BGP. Do a binary backoff. - @tenacity.retry(wait=tenacity.wait_exponential(multiplier=1, max=60), - reraise=True, stop=tenacity.stop_after_attempt(10)) - def _assert_cidr_in_peer_routing_table(peer_unit, cidr): - logging.debug("Checking for {} on BGP peer {}" - .format(cidr, peer_unit)) - # Run show ip route bgp on BGP peer - routes = juju_utils.remote_run( - peer_unit, remote_cmd='vtysh -c "show ip route bgp"') - logging.info(routes) - assert cidr in routes, ( - "CIDR, {}, not found in BGP peer's routing table: {}" - .format(cidr, routes)) - - _assert_cidr_in_peer_routing_table(peer_unit, private_cidr) - logging.info("Private subnet CIDR, {}, found in routing table" - .format(private_cidr)) - _assert_cidr_in_peer_routing_table(peer_unit, floating_ip_cidr) - logging.info("Floating IP CIDR, {}, found in routing table" - .format(floating_ip_cidr)) - - -def run_from_cli(): - """Run test for BGP routes from CLI. - - :returns: None - :rtype: None - """ - cli_utils.setup_logging() - parser = argparse.ArgumentParser() - parser.add_argument("--peer-application", "-a", - help="BGP Peer application name. Default: osci-frr", - default="osci-frr") - options = parser.parse_args() - - peer_application_name = cli_utils.parse_arg(options, - "peer_application") - - test_bgp_routes(peer_application_name) - - -if __name__ == "__main__": - sys.exit(run_from_cli()) diff --git a/zaza/openstack/charm_tests/dragent/tests.py b/zaza/openstack/charm_tests/dragent/tests.py index e95086d..67748c8 100644 --- a/zaza/openstack/charm_tests/dragent/tests.py +++ b/zaza/openstack/charm_tests/dragent/tests.py @@ -16,25 +16,122 @@ """Define class of BGP tests.""" +import logging +import tenacity import unittest +import zaza.openstack.charm_tests.neutron.tests as neutron_tests -from zaza.openstack.utilities import cli as cli_utils -from zaza.openstack.charm_tests.dragent import test +from zaza import model +from zaza.openstack.utilities import ( + cli as cli_utils, + juju as juju_utils, +) -class DRAgentTest(unittest.TestCase): +class DRAgentTest(neutron_tests.NeutronNetworkingBase): """Class to encapsulate BPG tests.""" BGP_PEER_APPLICATION = 'osci-frr' + def setUp(self): + """Run setup actions specific to the class.""" + super().setUp() + self._peer_unit = model.get_units( + self.BGP_PEER_APPLICATION)[0].entity_id + @classmethod def setUpClass(cls): - """Run setup for BGP tests.""" + """Run setup actions specific to the class.""" + super().setUpClass() cli_utils.setup_logging() + @staticmethod + @tenacity.retry(wait=tenacity.wait_exponential(multiplier=1, max=60), + reraise=True, stop=tenacity.stop_after_attempt(10)) + def _assert_cidr_in_peer_routing_table(peer_unit, cidr): + logging.debug("Checking for {} on BGP peer {}" + .format(cidr, peer_unit)) + # Run show ip route bgp on BGP peer + routes = juju_utils.remote_run( + peer_unit, remote_cmd='vtysh -c "show ip route bgp"') + logging.info(routes) + assert cidr in routes, ( + "CIDR, {}, not found in BGP peer's routing table: {}" + .format(cidr, routes)) + + @staticmethod + @tenacity.retry(wait=tenacity.wait_exponential(multiplier=1, max=60), + reraise=True, stop=tenacity.stop_after_attempt(10)) + def _assert_ip_reachable_via_peer(peer_unit, address): + logging.debug(f"Checking if {peer_unit} can reach {address} using " + f"routes adverised by NDR") + # Ping with -w specified will return an exit code if there is no + # response after the number of seconds specified. This is to ignore the + # first ping that may not arrive due to an ARP resolution. + juju_utils.remote_run(peer_unit, fatal=True, + remote_cmd=f'ping -w4 {address}') + def test_bgp_routes(self): - """Run bgp tests.""" - test.test_bgp_routes(peer_application_name=self.BGP_PEER_APPLICATION) + """Test BGP routes. + + A test that checks only the control plane of Neutron Dynamic Routing. + + :raises: AssertionError if expected BGP routes are not found + :returns: None + :rtype: None + """ + # Get expected advertised routes + private_cidr = self.project_subnet['cidr'] + floating_ip_cidr = "{}/32".format( + self.neutron_client.list_floatingips() + ["floatingips"][0]["floating_ip_address"]) + + # This test may run immediately after configuration. + # It may take time for routes to propagate via BGP. Do a + # binary backoff. + self._assert_cidr_in_peer_routing_table(self._peer_unit, private_cidr) + logging.info("Private subnet CIDR, {}, found in routing table" + .format(private_cidr)) + self._assert_cidr_in_peer_routing_table(self._peer_unit, + floating_ip_cidr) + logging.info("Floating IP CIDR, {}, found in routing table" + .format(floating_ip_cidr)) + + def test_instance_connectivity(self): + """Test connectivity to instances via dynamic routes. + + Make sure that with routes advertised via NDR it is actually possible + to reach instances from a unit that gets those routes programmed into + its routing table. + """ + # Get an instance but do not perform connectivity checks as the machine + # running those tests does not have the dynamic routes advertised to + # the peer unit by NDR. + fip_instance = self.launch_guest('fip-instance', instance_key='jammy', + perform_connectivity_check=False) + + @tenacity.retry(wait=tenacity.wait_exponential(multiplier=1, max=60), + reraise=True, stop=tenacity.stop_after_attempt(10)) + def get_fip(nova_client, instance_id): + """Try to get a FIP from an instance. + + Instance FIPs may not be immediately accessible from the Nova + object after the instance creation so a retry logic is necessary. + """ + # The reason for looking up an instance object again is that the + # Nova client does not refresh address information after + instance = nova_client.servers.find(id=instance_id) + fips = neutron_tests.floating_ips_from_instance(instance) + if not fips: + raise tenacity.TryAgain + return fips[0] + + fip = get_fip(self.nova_client, fip_instance.id) + + # First check that the FIP is present in the peer unit's routing table. + self._assert_cidr_in_peer_routing_table(self._peer_unit, f'{fip}/32') + # Once it is, check if it is actually reachable. + self._assert_ip_reachable_via_peer(self._peer_unit, fip) if __name__ == "__main__": From e6d215119e54f5efa5f45b964037edc068ee9def Mon Sep 17 00:00:00 2001 From: Dmitrii Shcherbakov Date: Sat, 17 Jun 2023 14:59:03 +0400 Subject: [PATCH 5/8] Be specific about a FIP to be checked in CP tests The configure step for the DRAgent tests configures a name for a floating IP to be checked for at the peer side. Since the data plane tests add one as well for an instance, make sure the control plane-only tests rely on the FIP that has a specific name. This can be useful if the tests are run in a different order. --- zaza/openstack/charm_tests/dragent/tests.py | 3 ++- zaza/openstack/configure/bgp_speaker.py | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/zaza/openstack/charm_tests/dragent/tests.py b/zaza/openstack/charm_tests/dragent/tests.py index 67748c8..a32b0c8 100644 --- a/zaza/openstack/charm_tests/dragent/tests.py +++ b/zaza/openstack/charm_tests/dragent/tests.py @@ -26,6 +26,7 @@ from zaza.openstack.utilities import ( cli as cli_utils, juju as juju_utils, ) +from zaza.openstack.configure.bgp_speaker import NDR_TEST_FIP class DRAgentTest(neutron_tests.NeutronNetworkingBase): @@ -83,7 +84,7 @@ class DRAgentTest(neutron_tests.NeutronNetworkingBase): # Get expected advertised routes private_cidr = self.project_subnet['cidr'] floating_ip_cidr = "{}/32".format( - self.neutron_client.list_floatingips() + self.neutron_client.list_floatingips(name=NDR_TEST_FIP) ["floatingips"][0]["floating_ip_address"]) # This test may run immediately after configuration. diff --git a/zaza/openstack/configure/bgp_speaker.py b/zaza/openstack/configure/bgp_speaker.py index 94aba98..6144474 100755 --- a/zaza/openstack/configure/bgp_speaker.py +++ b/zaza/openstack/configure/bgp_speaker.py @@ -26,7 +26,7 @@ from zaza.openstack.utilities import ( ) -FIP_TEST = "FIP TEST" +NDR_TEST_FIP = "NDR_TEST_FIP" def setup_bgp_speaker(peer_application_name, keystone_session=None): @@ -90,7 +90,8 @@ def setup_bgp_speaker(peer_application_name, keystone_session=None): # Create Floating IP to advertise logging.info("Creating floating IP to advertise") port = openstack_utils.create_port(neutron_client, - FIP_TEST, openstack_utils.PRIVATE_NET) + NDR_TEST_FIP, + openstack_utils.PRIVATE_NET) floating_ip = openstack_utils.create_floating_ip(neutron_client, openstack_utils.EXT_NET, port=port) From 35c5fca0c7ea77958ec11c835ce44f91fa0b2c5d Mon Sep 17 00:00:00 2001 From: Dmitrii Shcherbakov Date: Sat, 17 Jun 2023 15:07:33 +0400 Subject: [PATCH 6/8] Fix an incomplete comment about the Nova client --- zaza/openstack/charm_tests/dragent/tests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zaza/openstack/charm_tests/dragent/tests.py b/zaza/openstack/charm_tests/dragent/tests.py index a32b0c8..a2f80f7 100644 --- a/zaza/openstack/charm_tests/dragent/tests.py +++ b/zaza/openstack/charm_tests/dragent/tests.py @@ -120,7 +120,8 @@ class DRAgentTest(neutron_tests.NeutronNetworkingBase): object after the instance creation so a retry logic is necessary. """ # The reason for looking up an instance object again is that the - # Nova client does not refresh address information after + # Nova client does not refresh address information after the + # initial retrieval. instance = nova_client.servers.find(id=instance_id) fips = neutron_tests.floating_ips_from_instance(instance) if not fips: From 4b80d0db761b282a7af2495ade94e85ea9c988b8 Mon Sep 17 00:00:00 2001 From: Dmitrii Shcherbakov Date: Tue, 20 Jun 2023 22:16:04 +0400 Subject: [PATCH 7/8] Add a workaround for LP: #2024481 Due to the race condition described in LP: #2024481, before adding networks to the speaker we need to wait until it gets scheduled, otherwise it may not advertise some routes that are attempted to be advertised before the speaker is scheduled to a dragent service causing a non-fatal service error. --- zaza/openstack/configure/bgp_speaker.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/zaza/openstack/configure/bgp_speaker.py b/zaza/openstack/configure/bgp_speaker.py index 6144474..5ac3156 100755 --- a/zaza/openstack/configure/bgp_speaker.py +++ b/zaza/openstack/configure/bgp_speaker.py @@ -19,6 +19,9 @@ import argparse import logging import sys +import tenacity +import zaza.model + from zaza.openstack.utilities import ( cli as cli_utils, openstack as openstack_utils, @@ -29,6 +32,20 @@ from zaza.openstack.utilities import ( NDR_TEST_FIP = "NDR_TEST_FIP" +@tenacity.retry(wait=tenacity.wait_exponential(multiplier=1, max=60), + reraise=True, stop=tenacity.stop_after_attempt(10)) +def _assert_speaker_added(local_as): + logging.debug(f"Checking that a BGP speaker for {local_as} has been added") + # As soon as this message appears in the log on a pristine machine we can + # proceed with adding routes. The check is due to LP: #2024481. + grep_cmd = (f'grep "Added BGP Speaker for local_as={local_as}"' + f' /var/log/neutron/neutron-bgp-dragent.log') + # Usually we only have one unit in test bundles but let's be generic. + for unit in zaza.model.get_units("neutron-dynamic-routing"): + juju_utils.remote_run(unit, fatal=True, + remote_cmd=grep_cmd) + + def setup_bgp_speaker(peer_application_name, keystone_session=None): """Perform BGP Speaker setup. @@ -66,6 +83,10 @@ def setup_bgp_speaker(peer_application_name, keystone_session=None): bgp_speaker = openstack_utils.create_bgp_speaker( neutron_client, local_as=dr_asn) + # Due to LP: #2024481 make sure the BGP speaker is actually scheduled + # on this unit before adding any networks to it. + _assert_speaker_added(local_as=bgp_speaker["local_as"]) + # Add networks to bgp speaker logging.info("Advertising BGP routes") openstack_utils.add_network_to_bgp_speaker( From 3e289d2f001a3a0a09b97ab51d067a56e86672c6 Mon Sep 17 00:00:00 2001 From: Dmitrii Shcherbakov Date: Wed, 21 Jun 2023 03:09:00 +0400 Subject: [PATCH 8/8] Use the name attribute of a unit get_units returns a unit objects the name of which needs to be accessed by using the .name attribute. --- zaza/openstack/configure/bgp_speaker.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/zaza/openstack/configure/bgp_speaker.py b/zaza/openstack/configure/bgp_speaker.py index 5ac3156..35ffdb6 100755 --- a/zaza/openstack/configure/bgp_speaker.py +++ b/zaza/openstack/configure/bgp_speaker.py @@ -42,8 +42,7 @@ def _assert_speaker_added(local_as): f' /var/log/neutron/neutron-bgp-dragent.log') # Usually we only have one unit in test bundles but let's be generic. for unit in zaza.model.get_units("neutron-dynamic-routing"): - juju_utils.remote_run(unit, fatal=True, - remote_cmd=grep_cmd) + juju_utils.remote_run(unit.name, fatal=True, remote_cmd=grep_cmd) def setup_bgp_speaker(peer_application_name, keystone_session=None):