From 45acc0c53bce770ce19a2601072cd6f5b0db9f5f Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Mon, 8 Jan 2024 15:50:40 -0500 Subject: [PATCH 1/4] Disallow subnet cidr of :: without PD Do not allow the subnet cidr of :: to be used when creating a subnet, except in the case IPv6 prefix delegation has been specified in the request. Conflicts: neutron/tests/unit/db/test_db_base_plugin_v2.py Closes-bug: #2028159 Change-Id: I480e9a117513996f3c070acd4ba39c2b9fe9c0f1 (cherry picked from commit 2f0011194012a2482f79603c310028736e9ff3c8) --- doc/source/admin/config-ipv6.rst | 10 ++-- neutron/db/db_base_plugin_v2.py | 24 ++++++--- .../tests/unit/db/test_db_base_plugin_v2.py | 52 +++++++++++++------ .../unit/db/test_ipam_pluggable_backend.py | 1 + 4 files changed, 59 insertions(+), 28 deletions(-) diff --git a/doc/source/admin/config-ipv6.rst b/doc/source/admin/config-ipv6.rst index f0f1c0cb967..73a8990748b 100644 --- a/doc/source/admin/config-ipv6.rst +++ b/doc/source/admin/config-ipv6.rst @@ -686,18 +686,19 @@ First, create a network and IPv6 subnet: +---------------------------+--------------------------------------+ $ openstack subnet create --ip-version 6 --ipv6-ra-mode slaac \ - --ipv6-address-mode slaac --use-default-subnet-pool \ + --ipv6-address-mode slaac --use-prefix-delegation \ --network ipv6-pd ipv6-pd-1 +------------------------+--------------------------------------+ | Field | Value | +------------------------+--------------------------------------+ - | allocation_pools | ::2-::ffff:ffff:ffff:ffff | + | allocation_pools | ::1-::ffff:ffff:ffff:ffff | | cidr | ::/64 | | created_at | 2017-01-25T19:31:53Z | | description | | | dns_nameservers | | + | dns_publish_fixed_ip | None | | enable_dhcp | True | - | gateway_ip | ::1 | + | gateway_ip | :: | | headers | | | host_routes | | | id | 1319510d-c92c-4532-bf5d-8bcf3da761a1 | @@ -710,9 +711,8 @@ First, create a network and IPv6 subnet: | revision_number | 2 | | service_types | | | subnetpool_id | prefix_delegation | - | tags | [] | + | tags | | | updated_at | 2017-01-25T19:31:53Z | - | use_default_subnetpool | True | +------------------------+--------------------------------------+ The subnet is initially created with a temporary CIDR before one can be diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index ddd856e94e3..0811f292a08 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -595,7 +595,7 @@ def _validate_ip_version(self, ip_version, addr, name): "the ip_version '%(ip_version)s'") % data raise exc.InvalidInput(error_message=msg) - def _validate_subnet(self, context, s, cur_subnet=None): + def _validate_subnet(self, context, s, cur_subnet=None, is_pd=False): """Validate a subnet spec.""" # This method will validate attributes which may change during @@ -613,6 +613,7 @@ def _validate_subnet(self, context, s, cur_subnet=None): has_cidr = False if validators.is_attr_set(s.get('cidr')): self._validate_ip_version(ip_ver, s['cidr'], 'cidr') + net = netaddr.IPNetwork(s['cidr']) has_cidr = True # TODO(watanabe.isao): After we found a way to avoid the re-sync @@ -621,15 +622,21 @@ def _validate_subnet(self, context, s, cur_subnet=None): dhcp_was_enabled = cur_subnet.enable_dhcp else: dhcp_was_enabled = False + # A subnet cidr of '::' is invalid, unless the caller has + # indicated they are doing Prefix Delegation, + # see https://bugs.launchpad.net/neutron/+bug/2028159 + if (has_cidr and ip_ver == constants.IP_VERSION_6 and + net.network == netaddr.IPAddress('::') and not + is_pd): + error_message = _("IPv6 subnet '::' is not supported") + raise exc.InvalidInput(error_message=error_message) if has_cidr and s.get('enable_dhcp') and not dhcp_was_enabled: - subnet_prefixlen = netaddr.IPNetwork(s['cidr']).prefixlen error_message = _("Subnet has a prefix length that is " "incompatible with DHCP service enabled") - if ((ip_ver == 4 and subnet_prefixlen > 30) or - (ip_ver == 6 and subnet_prefixlen > 126)): + if ((ip_ver == 4 and net.prefixlen > 30) or + (ip_ver == 6 and net.prefixlen > 126)): raise exc.InvalidInput(error_message=error_message) - net = netaddr.IPNetwork(s['cidr']) if net.is_multicast(): error_message = _("Multicast IP subnet is not supported " "if enable_dhcp is True") @@ -874,9 +881,11 @@ def _create_subnet_precommit(self, context, subnet): raise exc.BadRequest(resource='subnets', msg=msg) validate = True + is_pd = False if subnetpool_id: self.ipam.validate_pools_with_subnetpool(s) if subnetpool_id == constants.IPV6_PD_POOL_ID: + is_pd = True if has_cidr: # We do not currently support requesting a specific # cidr with IPv6 prefix delegation. Set the subnetpool_id @@ -894,7 +903,7 @@ def _create_subnet_precommit(self, context, subnet): raise exc.BadRequest(resource='subnets', msg=msg) if validate: - self._validate_subnet(context, s) + self._validate_subnet(context, s, is_pd=is_pd) with db_api.CONTEXT_WRITER.using(context): network = self._get_network(context, @@ -950,7 +959,8 @@ def _update_subnet_precommit(self, context, id, subnet): # Fill 'network_id' field with the current value since this is expected # by _validate_segment() in ipam_pluggable_backend. s['network_id'] = subnet_obj.network_id - self._validate_subnet(context, s, cur_subnet=subnet_obj) + is_pd = s['subnetpool_id'] == constants.IPV6_PD_POOL_ID + self._validate_subnet(context, s, cur_subnet=subnet_obj, is_pd=is_pd) db_pools = [netaddr.IPRange(p.start, p.end) for p in subnet_obj.allocation_pools] diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index b218b95b368..87701355b5f 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -397,7 +397,7 @@ def _create_subnet(self, fmt, net_id, cidr, data = {'subnet': {'network_id': net_id, 'ip_version': constants.IP_VERSION_4, 'tenant_id': self._tenant_id}} - if cidr: + if cidr and cidr is not constants.ATTR_NOT_SPECIFIED: data['subnet']['cidr'] = cidr for arg in ('ip_version', 'tenant_id', 'subnetpool_id', 'prefixlen', 'enable_dhcp', 'allocation_pools', 'segment_id', @@ -774,7 +774,9 @@ def subnet(self, network=None, set_context=False): if project_id: tenant_id = project_id - cidr = netaddr.IPNetwork(cidr) if cidr else None + if (cidr is not None and + cidr != constants.ATTR_NOT_SPECIFIED): + cidr = netaddr.IPNetwork(cidr) if (gateway_ip is not None and gateway_ip != constants.ATTR_NOT_SPECIFIED): gateway_ip = netaddr.IPAddress(gateway_ip) @@ -947,7 +949,7 @@ def _validate_resource(self, resource, keys, res_name): if (k == 'gateway_ip' and ipv6_zero_gateway and keys[k][-3:] == "::0"): self.assertEqual(keys[k][:-1], resource[res_name][k]) - else: + elif keys[k] != constants.ATTR_NOT_SPECIFIED: self.assertEqual(keys[k], resource[res_name][k]) @@ -3265,7 +3267,6 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): def _test_create_subnet(self, network=None, expected=None, **kwargs): keys = kwargs.copy() - keys.setdefault('cidr', '10.0.0.0/24') keys.setdefault('ip_version', constants.IP_VERSION_4) keys.setdefault('enable_dhcp', True) with self.subnet(network=network, **keys) as subnet: @@ -4005,6 +4006,7 @@ def test_create_subnet_ipv6_gw_values(self): @testtools.skipIf(tools.is_bsd(), 'bug/1484837') def test_create_subnet_ipv6_pd_gw_values(self): cidr = constants.PROVISIONAL_IPV6_PD_PREFIX + subnetpool_id = constants.IPV6_PD_POOL_ID # Gateway is last IP in IPv6 DHCPv6 Stateless subnet gateway = '::ffff:ffff:ffff:ffff' allocation_pools = [{'start': '::1', @@ -4012,10 +4014,17 @@ def test_create_subnet_ipv6_pd_gw_values(self): expected = {'gateway_ip': gateway, 'cidr': cidr, 'allocation_pools': allocation_pools} + # We do not specify a CIDR in the API call for a PD subnet, as it + # is unsupported. Instead we specify the subnetpool_id as + # "prefix_delegation" which is what happens via OSC's + # --use-prefix-delegation argument. But the expected result is a + # subnet object with the "::/64" PD prefix. Same comment applies below. self._test_create_subnet(expected=expected, gateway_ip=gateway, - cidr=cidr, ip_version=constants.IP_VERSION_6, + cidr=constants.ATTR_NOT_SPECIFIED, + ip_version=constants.IP_VERSION_6, ipv6_ra_mode=constants.DHCPV6_STATELESS, - ipv6_address_mode=constants.DHCPV6_STATELESS) + ipv6_address_mode=constants.DHCPV6_STATELESS, + subnetpool_id=subnetpool_id) # Gateway is first IP in IPv6 DHCPv6 Stateless subnet gateway = '::1' allocation_pools = [{'start': '::2', @@ -4024,18 +4033,22 @@ def test_create_subnet_ipv6_pd_gw_values(self): 'cidr': cidr, 'allocation_pools': allocation_pools} self._test_create_subnet(expected=expected, gateway_ip=gateway, - cidr=cidr, ip_version=constants.IP_VERSION_6, + cidr=constants.ATTR_NOT_SPECIFIED, + ip_version=constants.IP_VERSION_6, ipv6_ra_mode=constants.DHCPV6_STATELESS, - ipv6_address_mode=constants.DHCPV6_STATELESS) + ipv6_address_mode=constants.DHCPV6_STATELESS, + subnetpool_id=subnetpool_id) # If gateway_ip is not specified and the subnet is using prefix # delegation, until the CIDR is assigned, this value should be first # IP from the subnet expected = {'gateway_ip': str(netaddr.IPNetwork(cidr).network), 'cidr': cidr} self._test_create_subnet(expected=expected, - cidr=cidr, ip_version=constants.IP_VERSION_6, + cidr=constants.ATTR_NOT_SPECIFIED, + ip_version=constants.IP_VERSION_6, ipv6_ra_mode=constants.IPV6_SLAAC, - ipv6_address_mode=constants.IPV6_SLAAC) + ipv6_address_mode=constants.IPV6_SLAAC, + subnetpool_id=subnetpool_id) def test_create_subnet_gw_outside_cidr_returns_201(self): with self.network() as network: @@ -4132,14 +4145,21 @@ def test_create_subnet_with_v6_allocation_pool(self): allocation_pools=allocation_pools) @testtools.skipIf(tools.is_bsd(), 'bug/1484837') - def test_create_subnet_with_v6_pd_allocation_pool(self): + def test_create_subnet_with_v6_pd_allocation_pool_returns_400(self): gateway_ip = '::1' cidr = constants.PROVISIONAL_IPV6_PD_PREFIX allocation_pools = [{'start': '::2', 'end': '::ffff:ffff:ffff:fffe'}] - self._test_create_subnet(gateway_ip=gateway_ip, - cidr=cidr, ip_version=constants.IP_VERSION_6, - allocation_pools=allocation_pools) + # Creating a subnet object with the "::/64" PD prefix is invalid + # unless the subnetpool_id is also passed as "prefix_delegation" + with testlib_api.ExpectedException( + webob.exc.HTTPClientError) as ctx_manager: + self._test_create_subnet(gateway_ip=gateway_ip, + cidr=cidr, + ip_version=constants.IP_VERSION_6, + allocation_pools=allocation_pools) + self.assertEqual(webob.exc.HTTPClientError.code, + ctx_manager.exception.code) def test_create_subnet_with_large_allocation_pool(self): gateway_ip = '10.0.0.1' @@ -4350,10 +4370,10 @@ def _test_validate_subnet_ipv6_pd_modes(self, cur_subnet=None, for mode, value in modes.items(): new_subnet[mode] = value if expect_success: - plugin._validate_subnet(ctx, new_subnet, cur_subnet) + plugin._validate_subnet(ctx, new_subnet, cur_subnet, True) else: self.assertRaises(lib_exc.InvalidInput, plugin._validate_subnet, - ctx, new_subnet, cur_subnet) + ctx, new_subnet, cur_subnet, True) def test_create_subnet_ipv6_ra_modes(self): # Test all RA modes with no address mode specified diff --git a/neutron/tests/unit/db/test_ipam_pluggable_backend.py b/neutron/tests/unit/db/test_ipam_pluggable_backend.py index 69fc9fc5c60..3a9419f98de 100644 --- a/neutron/tests/unit/db/test_ipam_pluggable_backend.py +++ b/neutron/tests/unit/db/test_ipam_pluggable_backend.py @@ -374,6 +374,7 @@ def test_test_fixed_ips_for_port_pd_gateway(self): context = mock.Mock() pluggable_backend = ipam_pluggable_backend.IpamPluggableBackend() with self.subnet(cidr=constants.PROVISIONAL_IPV6_PD_PREFIX, + subnetpool_id=constants.IPV6_PD_POOL_ID, ip_version=constants.IP_VERSION_6) as subnet: subnet = subnet['subnet'] fixed_ips = [{'subnet_id': subnet['id'], From 1d2a1c68c0a869576e359f5245495c743a11c17c Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 19 Feb 2024 09:38:07 +0000 Subject: [PATCH 2/4] Retry ``set|get_link_attribute(s)`` if the interface is not present After some interface operations (in particular the ``IpLinkCommand.set_ns`` operation), the network interface is temporarily not present in the destination namespace. This patch retries the interface "ip link set|show" command in that case. Related-Bug: #1961740 Change-Id: I5a57cfc71ad59f1fe9ea65e19b1a32314d798729 (cherry picked from commit 016f5187a5503b835cd6803c5d8fe6f05091df12) --- neutron/privileged/agent/linux/ip_lib.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/neutron/privileged/agent/linux/ip_lib.py b/neutron/privileged/agent/linux/ip_lib.py index aa44b62c912..f889b795dd1 100644 --- a/neutron/privileged/agent/linux/ip_lib.py +++ b/neutron/privileged/agent/linux/ip_lib.py @@ -400,6 +400,10 @@ def set_link_flags(device, namespace, flags): _run_iproute_link("set", device, namespace, flags=new_flags) +@tenacity.retry( + retry=tenacity.retry_if_exception_type(NetworkInterfaceNotFound), + wait=tenacity.wait_exponential(multiplier=0.02, max=1), + stop=tenacity.stop_after_delay(3), reraise=True) @privileged.link_cmd.entrypoint def set_link_attribute(device, namespace, **attributes): _run_iproute_link("set", device, namespace, **attributes) @@ -430,7 +434,8 @@ def set_link_bridge_master(device, bridge, namespace=None): @tenacity.retry( retry=tenacity.retry_if_exception_type( - netlink_exceptions.NetlinkDumpInterrupted), + (netlink_exceptions.NetlinkDumpInterrupted, + NetworkInterfaceNotFound)), wait=tenacity.wait_exponential(multiplier=0.02, max=1), stop=tenacity.stop_after_delay(8), reraise=True) From b34911a6c938187c8554b39dfd0591d1cca94e73 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 7 Feb 2024 15:45:54 +0000 Subject: [PATCH 3/4] [OVN] A LRP in an external tunnelled network has no chassis A logical router port in an external tunnelled network won't be scheduled in any chassis. A tunnelled network has no physical provider network associated thus the logical router port cannot be bound to a specific chassis. Conflicts: neutron/tests/functional/services/ovn_l3/test_plugin.py Related-Bug: #2019217 Change-Id: I140c22899ea3b0240f8c30902fc2dc7055914a18 (cherry picked from commit d55c591ecde2f6cc4c2cea64fb21a75b6343cd5a) --- .../ovn/mech_driver/ovsdb/ovn_client.py | 31 +++++++------- .../functional/services/ovn_l3/test_plugin.py | 42 ++++++++++--------- .../ovn/mech_driver/test_mech_driver.py | 31 +++++++------- 3 files changed, 54 insertions(+), 50 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 20b7ad3b2c0..df61890ac9f 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -1486,11 +1486,13 @@ def get_candidates_for_scheduling(self, physnet, cms=None, """Return chassis for scheduling gateway router. Criteria for selecting chassis as candidates - 1) chassis from cms with proper bridge mappings - 2) if no chassis is available from 1) then, - select chassis with proper bridge mappings - 3) Filter the available chassis accordingly to the routers + 1) Chassis from cms with proper bridge mappings only (that means these + gateway chassis with the requested physical network). + 2) Filter the available chassis accordingly to the routers availability zone hints (if present) + + If the logical router port belongs to a tunnelled network, there won't + be any candidate. """ # TODO(lucasagomes): Simplify the logic here, the CMS option has # been introduced long ago and by now all gateway chassis should @@ -1499,15 +1501,13 @@ def get_candidates_for_scheduling(self, physnet, cms=None, cms = cms or self._sb_idl.get_gateway_chassis_from_cms_options() chassis_physnets = (chassis_physnets or self._sb_idl.get_chassis_and_physnets()) - cms_bmaps = [] - bmaps = [] + candidates = set() for chassis, physnets in chassis_physnets.items(): - if physnet and physnet in physnets: - if chassis in cms: - cms_bmaps.append(chassis) - else: - bmaps.append(chassis) - candidates = cms_bmaps or bmaps or cms + if (physnet and + physnet in physnets and + chassis in cms): + candidates.add(chassis) + candidates = list(candidates) # Filter for availability zones if availability_zone_hints: @@ -1518,11 +1518,8 @@ def get_candidates_for_scheduling(self, physnet, cms=None, if az in utils.get_chassis_availability_zones( self._sb_idl.lookup('Chassis', ch, None))] - if not cms_bmaps: - LOG.debug("No eligible chassis with external connectivity" - " through ovn-cms-options for %s", physnet) - LOG.debug("Chassis candidates for scheduling gateway router ports: %s", - candidates) + LOG.debug('Chassis candidates for scheduling gateway router ports ' + 'for "%s" physical network: %s', physnet, candidates) return candidates def _get_physnet(self, network): diff --git a/neutron/tests/functional/services/ovn_l3/test_plugin.py b/neutron/tests/functional/services/ovn_l3/test_plugin.py index 64f67d0ed06..ded05d76530 100644 --- a/neutron/tests/functional/services/ovn_l3/test_plugin.py +++ b/neutron/tests/functional/services/ovn_l3/test_plugin.py @@ -32,12 +32,14 @@ class TestRouter(base.TestOVNFunctionalBase): - def setUp(self): - super(TestRouter, self).setUp() + def setUp(self, **kwargs): + super().setUp(**kwargs) self.chassis1 = self.add_fake_chassis( - 'ovs-host1', physical_nets=['physnet1', 'physnet3']) + 'ovs-host1', physical_nets=['physnet1', 'physnet3'], + enable_chassis_as_gw=True, azs=[]) self.chassis2 = self.add_fake_chassis( - 'ovs-host2', physical_nets=['physnet2', 'physnet3']) + 'ovs-host2', physical_nets=['physnet2', 'physnet3'], + enable_chassis_as_gw=True, azs=[]) self.cr_lrp_pb_event = events.WaitForCrLrpPortBindingEvent() self.sb_api.idl.notify_handler.watch_event(self.cr_lrp_pb_event) @@ -95,12 +97,14 @@ def test_gateway_chassis_on_router_gateway_port(self): self.assertIn(rc, expected) def _check_gateway_chassis_candidates(self, candidates, - router_az_hints=None): + router_az_hints=None, + physnet='physnet1'): # In this test, fake_select() is called once from _create_router() # and later from schedule_unhosted_gateways() ovn_client = self.l3_plugin._ovn_client + net_type = 'vlan' if physnet else 'geneve' ext1 = self._create_ext_network( - 'ext1', 'vlan', 'physnet1', 1, "10.0.0.1", "10.0.0.0/24") + 'ext1', net_type, physnet, 1, "10.0.0.1", "10.0.0.0/24") # mock select function and check if it is called with expected # candidates. @@ -131,12 +135,11 @@ def fake_select(*args, **kwargs): def test_gateway_chassis_with_cms_and_bridge_mappings(self): # Both chassis1 and chassis3 are having proper bridge mappings, - # but only chassis3 is having enable-chassis-as-gw. - # Test if chassis3 is selected as candidate or not. + # but only chassis1 is having enable-chassis-as-gw. + # Test if chassis1 is selected as candidate or not. self.chassis3 = self.add_fake_chassis( - 'ovs-host3', physical_nets=['physnet1'], - other_config={'ovn-cms-options': 'enable-chassis-as-gw'}) - self._check_gateway_chassis_candidates([self.chassis3]) + 'ovs-host3', physical_nets=['physnet1'], azs=[]) + self._check_gateway_chassis_candidates([self.chassis1]) def test_gateway_chassis_with_cms_and_no_bridge_mappings(self): # chassis1 is having proper bridge mappings. @@ -170,12 +173,10 @@ def test_gateway_chassis_with_cms_and_azs(self): # Test if chassis3 is selected as candidate or not. self.chassis3 = self.add_fake_chassis( 'ovs-host3', physical_nets=['physnet1'], - other_config={'ovn-cms-options': 'enable-chassis-as-gw'}, - azs=['ovn']) + azs=['ovn'], enable_chassis_as_gw=True) self.chassis4 = self.add_fake_chassis( 'ovs-host4', physical_nets=['physnet1'], - other_config={'ovn-cms-options': 'enable-chassis-as-gw'}, - azs=['ovn2']) + azs=['ovn2'], enable_chassis_as_gw=True) self._check_gateway_chassis_candidates([self.chassis3], router_az_hints=['ovn']) @@ -185,11 +186,9 @@ def test_gateway_chassis_with_cms_and_not_match_azs(self): # AvailabilityZoneNotFound. after create will delete if. # add chassis4 is having azs [ovn2], not match routers az_hints [ovn] self.chassis3 = self.add_fake_chassis( - 'ovs-host3', physical_nets=['physnet1'], - other_config={'ovn-cms-options': 'enable-chassis-as-gw'}) + 'ovs-host3', physical_nets=['physnet1'], enable_chassis_as_gw=True) self.chassis4 = self.add_fake_chassis( - 'ovs-host4', physical_nets=['physnet1'], - other_config={'ovn-cms-options': 'enable-chassis-as-gw'}, + 'ovs-host4', physical_nets=['physnet1'], enable_chassis_as_gw=True, azs=['ovn2']) ovn_client = self.l3_plugin._ovn_client ext1 = self._create_ext_network( @@ -217,6 +216,11 @@ def test_gateway_chassis_with_bridge_mappings_and_no_cms(self): # Test if chassis1 is selected as candidate or not. self._check_gateway_chassis_candidates([self.chassis1]) + def test_gateway_chassis_no_physnet_tunnelled_network(self): + # The GW network is tunnelled, no physnet defined --> no possible + # candidates. + self._check_gateway_chassis_candidates([], physnet=None) + def _l3_ha_supported(self): # If the Gateway_Chassis table exists in SB database, then it # means that L3 HA is supported. diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 1a91b7e11c8..2fb7dba494a 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -2554,24 +2554,27 @@ def fake_lookup(table, chassis_name, default): return ch ovn_client._sb_idl.lookup = fake_lookup - # The target physnet and availability zones - physnet = 'public' - az_hints = ['az0', 'az2'] - + # List of chassis and chassis:physnet mappings. + physnet_name = 'public' cms = [ch0.name, ch1.name, ch2.name, ch3.name, ch4.name, ch5.name] - ch_physnet = {ch0.name: [physnet], ch1.name: [physnet], - ch2.name: [physnet], ch3.name: [physnet], + ch_physnet = {ch0.name: [physnet_name], ch1.name: [physnet_name], + ch2.name: [physnet_name], ch3.name: [physnet_name], ch4.name: ['another-physnet'], ch5.name: ['yet-another-physnet']} - candidates = ovn_client.get_candidates_for_scheduling( - physnet, cms=cms, chassis_physnets=ch_physnet, - availability_zone_hints=az_hints) - - # Only chassis ch0 and ch2 should match the availability zones - # hints and physnet we passed to get_candidates_for_scheduling() - expected_candidates = [ch0.name, ch2.name] - self.assertEqual(sorted(expected_candidates), sorted(candidates)) + # The target physnets, the availability zones and the expected + # candidates. + results = [{'physnet': physnet_name, 'az_hints': ['az0', 'az2'], + 'expected_candidates': [ch0.name, ch2.name]}, + {'physnet': None, 'az_hints': ['az0', 'az2'], + 'expected_candidates': []}, + ] + for result in results: + candidates = ovn_client.get_candidates_for_scheduling( + result['physnet'], cms=cms, chassis_physnets=ch_physnet, + availability_zone_hints=result['az_hints']) + self.assertEqual(sorted(result['expected_candidates']), + sorted(candidates)) def test__get_info_for_ha_chassis_group_as_extport(self): net_attrs = {az_def.AZ_HINTS: ['az0', 'az1', 'az2']} From 0dfe8dedd63aba2bf5b75ad8494b0ead4ba1b79f Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Thu, 22 Feb 2024 10:06:58 +0100 Subject: [PATCH 4/4] Ensure that haproxy spawned by the metadata agents is active In both neutron-metadata and neutron-ovn-metadata agents we should ensure that haproxy service spawned for network/router is actually active before moving on. This patch adds that check and this is similar to what was already implemented some time ago for the dnsmasq process spawned by the dhcp agent. Related-Bug: #2052787 Change-Id: Ic58640d89952fa03bd1059608ee6c9072fbaabf5 (cherry picked from commit 2f7f7c2fc29d0ac26b5ff9d82867952a40f0fa1b) --- neutron/agent/metadata/driver.py | 2 +- neutron/agent/ovn/metadata/driver.py | 2 +- neutron/tests/unit/agent/dhcp/test_agent.py | 3 ++- neutron/tests/unit/agent/metadata/test_driver.py | 7 ++++++- neutron/tests/unit/agent/ovn/metadata/test_driver.py | 7 ++++++- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/neutron/agent/metadata/driver.py b/neutron/agent/metadata/driver.py index b6ba2c6d8c2..c3ca3c05a03 100644 --- a/neutron/agent/metadata/driver.py +++ b/neutron/agent/metadata/driver.py @@ -270,7 +270,7 @@ def spawn_monitored_metadata_proxy(cls, monitor, ns_name, port, conf, ns_name=ns_name, callback=callback) try: - pm.enable() + pm.enable(ensure_active=True) except exceptions.ProcessExecutionError as exec_err: LOG.error("Encountered process execution error %(err)s while " "starting process in namespace %(ns)s", diff --git a/neutron/agent/ovn/metadata/driver.py b/neutron/agent/ovn/metadata/driver.py index 3c6daa23039..87771f6c94e 100644 --- a/neutron/agent/ovn/metadata/driver.py +++ b/neutron/agent/ovn/metadata/driver.py @@ -181,7 +181,7 @@ def spawn_monitored_metadata_proxy(cls, monitor, ns_name, port, conf, ns_name=ns_name, callback=callback) try: - pm.enable() + pm.enable(ensure_active=True) except exceptions.ProcessExecutionError as exec_err: LOG.error("Encountered process execution error %(err)s while " "starting process in namespace %(ns)s", diff --git a/neutron/tests/unit/agent/dhcp/test_agent.py b/neutron/tests/unit/agent/dhcp/test_agent.py index 978a36b0f2d..7917a005858 100644 --- a/neutron/tests/unit/agent/dhcp/test_agent.py +++ b/neutron/tests/unit/agent/dhcp/test_agent.py @@ -658,6 +658,7 @@ def test_dhcp_ready_ports_updates_after_enable_dhcp(self, *args): 'IpAddrCommand.wait_until_address_ready') as mock_wait: mock_wait.return_value = True dhcp = dhcp_agent.DhcpAgent(HOSTNAME) + dhcp.update_isolated_metadata_proxy = mock.Mock() self.assertEqual(set(), dhcp.dhcp_ready_ports) dhcp.configure_dhcp_for_network(fake_network) self.assertEqual({fake_port1.id}, dhcp.dhcp_ready_ports) @@ -854,7 +855,7 @@ def _enable_dhcp_helper(self, network, enable_isolated_metadata=False, is_ovn_network): process_instance.assert_has_calls([ mock.call.disable(sig=str(int(signal.SIGTERM))), - mock.call.enable()]) + mock.call.enable(ensure_active=True)]) else: process_instance.assert_has_calls([ mock.call.disable(sig=str(int(signal.SIGTERM)))]) diff --git a/neutron/tests/unit/agent/metadata/test_driver.py b/neutron/tests/unit/agent/metadata/test_driver.py index 32f64fbe1b7..f427ee4b61e 100644 --- a/neutron/tests/unit/agent/metadata/test_driver.py +++ b/neutron/tests/unit/agent/metadata/test_driver.py @@ -171,7 +171,12 @@ def _test_spawn_metadata_proxy(self, dad_failed=False): 'IpAddrCommand.wait_until_address_ready') as mock_wait,\ mock.patch( 'neutron.agent.linux.ip_lib.' - 'delete_ip_address') as mock_del: + 'delete_ip_address') as mock_del,\ + mock.patch( + 'neutron.agent.linux.external_process.' + 'ProcessManager.active', + new_callable=mock.PropertyMock, + side_effect=[False, True]): agent = l3_agent.L3NATAgent('localhost') agent.process_monitor = mock.Mock() cfg_file = os.path.join( diff --git a/neutron/tests/unit/agent/ovn/metadata/test_driver.py b/neutron/tests/unit/agent/ovn/metadata/test_driver.py index aca322e4a33..9519701947f 100644 --- a/neutron/tests/unit/agent/ovn/metadata/test_driver.py +++ b/neutron/tests/unit/agent/ovn/metadata/test_driver.py @@ -74,7 +74,12 @@ def test_spawn_metadata_proxy(self): return_value=test_utils.FakeUser(self.EUNAME)),\ mock.patch('grp.getgrnam', return_value=test_utils.FakeGroup(self.EGNAME)),\ - mock.patch('os.makedirs'): + mock.patch('os.makedirs'),\ + mock.patch( + 'neutron.agent.linux.external_process.' + 'ProcessManager.active', + new_callable=mock.PropertyMock, + side_effect=[False, True]): cfg_file = os.path.join( metadata_driver.HaproxyConfigurator.get_config_path( cfg.CONF.state_path),