Synchronise 2023.1 with upstream

neutron/agent/linux/interface.py

@@ -365,7 +365,7 @@ def _add_device_to_namespace(self, ip_wrapper, device, namespace):
         namespace_obj = ip_wrapper.ensure_namespace(namespace)
         for i in range(9):
             try:
-                namespace_obj.add_device_to_namespace(device)
+                namespace_obj.add_device_to_namespace(device, is_ovs_port=True)
                 break
             except ip_lib.NetworkInterfaceNotFound:
                 # NOTE(slaweq): if the exception was NetworkInterfaceNotFound

neutron/agent/linux/ip_lib.py

@@ -259,7 +259,22 @@ def ensure_namespace(self, name):
         return ip
 
     def namespace_is_empty(self):
-        return not self.get_devices()
+        try:
+            return not self.get_devices()
+        except OSError as e:
+            # This can happen if we previously got terminated in the middle of
+            # removing this namespace. In this case the bind mount of the
+            # namespace under /var/run/netns will be removed, but the namespace
+            # file is still there. As the bind mount is gone we can no longer
+            # access the namespace to validate that it is empty. But since it
+            # should have already been removed we are sure that the check has
+            # passed the last time and since the namespace is unuseable that
+            # can not have changed.
+            # Future calls to pyroute2 to remove that namespace will clean up
+            # the leftover file.
+            if e.errno == errno.EINVAL:
+                return True
+            raise e
 
     def garbage_collect_namespace(self):
         """Conditionally destroy the namespace if it is empty."""
@@ -269,9 +284,9 @@ def garbage_collect_namespace(self):
                 return True
         return False
 
-    def add_device_to_namespace(self, device):
+    def add_device_to_namespace(self, device, is_ovs_port=False):
         if self.namespace:
-            device.link.set_netns(self.namespace)
+            device.link.set_netns(self.namespace, is_ovs_port=is_ovs_port)
 
     def add_vlan(self, name, physical_interface, vlan_id):
         privileged.create_interface(name,
@@ -461,10 +476,15 @@ def set_down(self):
         privileged.set_link_attribute(
             self.name, self._parent.namespace, state='down')
 
-    def set_netns(self, namespace):
+    def set_netns(self, namespace, is_ovs_port=False):
         privileged.set_link_attribute(
             self.name, self._parent.namespace, net_ns_fd=namespace)
         self._parent.namespace = namespace
+        if is_ovs_port:
+            # NOTE(slaweq): because of the "shy port" which may dissapear for
+            # short time after it's moved to the namespace we need to wait
+            # a bit before checking if port really exists in the namespace
+            time.sleep(1)
         common_utils.wait_until_true(lambda: self.exists, timeout=5,
                                      sleep=0.5)
 

neutron/agent/ovn/metadata/agent.py

@@ -430,7 +430,10 @@ def sync(self, provision=True):
                              ns.startswith(NS_PREFIX) and
                              ns not in metadata_namespaces]
         for ns in unused_namespaces:
-            self.teardown_datapath(self._get_datapath_name(ns))
+            try:
+                self.teardown_datapath(self._get_datapath_name(ns))
+            except Exception:
+                LOG.exception('Error unable to destroy namespace: %s', ns)
 
         # resync all network namespaces based on the associated datapaths,
         # even those that are already running. This is to make sure

neutron/db/db_base_plugin_v2.py

@@ -643,32 +643,39 @@ def _validate_subnet(self, context, s, cur_subnet=None):
                                   "supported if enable_dhcp is True.")
                 raise exc.InvalidInput(error_message=error_message)
 
-        if validators.is_attr_set(s.get('gateway_ip')):
-            self._validate_ip_version(ip_ver, s['gateway_ip'], 'gateway_ip')
-            if has_cidr:
-                is_gateway_not_valid = (
-                    ipam.utils.check_gateway_invalid_in_subnet(
-                        s['cidr'], s['gateway_ip']))
-                if is_gateway_not_valid:
-                    error_message = _("Gateway is not valid on subnet")
-                    raise exc.InvalidInput(error_message=error_message)
-            # Ensure the gateway IP is not assigned to any port
-            # skip this check in case of create (s parameter won't have id)
+        gateway_ip = s.get('gateway_ip', constants.ATTR_NOT_SPECIFIED)
+        if validators.is_attr_set(gateway_ip) or gateway_ip is None:
+            # Validate the gateway IP, if defined in the request.
+            if s['gateway_ip']:
+                self._validate_ip_version(ip_ver, gateway_ip, 'gateway_ip')
+                if has_cidr:
+                    is_gateway_not_valid = (
+                        ipam.utils.check_gateway_invalid_in_subnet(
+                            s['cidr'], gateway_ip))
+                    if is_gateway_not_valid:
+                        error_message = _("Gateway is not valid on subnet")
+                        raise exc.InvalidInput(error_message=error_message)
+
+            # Ensure the current subnet gateway IP is not assigned to any port.
+            # The subnet gateway IP cannot be modified or removed if in use
+            # (assigned to a router interface).
+            # Skip this check in case of create (s parameter won't have id).
             # NOTE(salv-orlando): There is slight chance of a race, when
             # a subnet-update and a router-interface-add operation are
             # executed concurrently
-            s_gateway_ip = netaddr.IPAddress(s['gateway_ip'])
+            s_gateway_ip = (netaddr.IPAddress(gateway_ip) if gateway_ip else
+                            None)
             if (cur_subnet and
                     s_gateway_ip != cur_subnet['gateway_ip'] and
                     not ipv6_utils.is_ipv6_pd_enabled(s)):
-                gateway_ip = str(cur_subnet['gateway_ip'])
+                current_gateway_ip = str(cur_subnet['gateway_ip'])
                 alloc = port_obj.IPAllocation.get_alloc_routerports(
-                    context, cur_subnet['id'], gateway_ip=gateway_ip,
+                    context, cur_subnet['id'], gateway_ip=current_gateway_ip,
                     first=True)
 
                 if alloc and alloc.port_id:
                     raise exc.GatewayIpInUse(
-                        ip_address=gateway_ip,
+                        ip_address=current_gateway_ip,
                         port_id=alloc.port_id)
 
         if validators.is_attr_set(s.get('dns_nameservers')):

neutron/objects/router.py

@@ -10,8 +10,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import itertools
-
 import netaddr
 
 from neutron_lib.api.definitions import availability_zone as az_def
@@ -409,20 +407,11 @@ def get_scoped_floating_ips(cls, context, router_ids):
 
         # Filter out on router_ids
         query = query.filter(l3.FloatingIP.router_id.in_(router_ids))
-        return cls._unique_floatingip_iterator(context, query)
 
-    @classmethod
-    def _unique_floatingip_iterator(cls, context, query):
-        """Iterates over only one row per floating ip. Ignores others."""
-        # Group rows by fip id. They must be sorted by same.
-        q = query.order_by(l3.FloatingIP.id)
-        keyfunc = lambda row: row[0]['id']
-        group_iterator = itertools.groupby(q, keyfunc)
-
-        # Just hit the first row of each group
-        for key, value in group_iterator:
-            # pylint: disable=stop-iteration-return
-            row = list(next(value))
+        # Remove duplicate rows based on FIP IDs
+        query = query.group_by(l3.FloatingIP.id)
+
+        for row in query:
             yield (cls._load_object(context, row[0]), row[1])
 
     @classmethod

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py

@@ -670,6 +670,8 @@ def notify(self, event, row, updates=None):
 class BaseOvnSbIdl(Ml2OvnIdlBase):
     @classmethod
     def from_server(cls, connection_string, helper):
+        if 'Chassis_Private' in helper.schema_json['tables']:
+            helper.register_table('Chassis_Private')
         helper.register_table('Chassis')
         helper.register_table('Encap')
         helper.register_table('Port_Binding')

neutron/tests/unit/agent/linux/test_interface.py

@@ -470,11 +470,11 @@ def device_exists(dev, namespace=None):
                 expected.extend(
                     [mock.call().ensure_namespace(namespace),
                      mock.call().ensure_namespace().add_device_to_namespace(
-                         mock.ANY),
+                         mock.ANY, is_ovs_port=True),
                      mock.call().ensure_namespace().add_device_to_namespace(
-                         mock.ANY),
+                         mock.ANY, is_ovs_port=True),
                      mock.call().ensure_namespace().add_device_to_namespace(
-                         mock.ANY)])
+                         mock.ANY, is_ovs_port=True)])
             expected.extend([
                 mock.call(namespace=namespace),
                 mock.call().device('tap0'),

neutron/tests/unit/agent/linux/test_ip_lib.py

@@ -357,6 +357,21 @@ def test_garbage_collect_namespace_existing_not_empty(self):
                 self.assertNotIn(mock.call().delete('ns'),
                                  ip_ns_cmd_cls.mock_calls)
 
+    def test_garbage_collect_namespace_existing_broken(self):
+        with mock.patch.object(ip_lib, 'IpNetnsCommand') as ip_ns_cmd_cls:
+            ip_ns_cmd_cls.return_value.exists.return_value = True
+
+            ip = ip_lib.IPWrapper(namespace='ns')
+
+            with mock.patch.object(ip, 'get_devices',
+                                   side_effect=OSError(errno.EINVAL, None)
+                                   ) as mock_get_devices:
+                self.assertTrue(ip.garbage_collect_namespace())
+
+                mock_get_devices.assert_called_once_with()
+                expected = [mock.call().delete('ns')]
+                ip_ns_cmd_cls.assert_has_calls(expected)
+
     @mock.patch.object(priv_lib, 'create_interface')
     def test_add_vlan(self, create):
         retval = ip_lib.IPWrapper().add_vlan('eth0.1', 'eth0', '1')
@@ -507,7 +522,8 @@ def fake_create_interface(ifname, namespace, kind, **kwargs):
     def test_add_device_to_namespace(self):
         dev = mock.Mock()
         ip_lib.IPWrapper(namespace='ns').add_device_to_namespace(dev)
-        dev.assert_has_calls([mock.call.link.set_netns('ns')])
+        dev.assert_has_calls(
+            [mock.call.link.set_netns('ns', is_ovs_port=False)])
 
     def test_add_device_to_namespace_is_none(self):
         dev = mock.Mock()

neutron/tests/unit/agent/ovn/metadata/test_agent.py

@@ -134,6 +134,31 @@ def test_sync_teardown_namespace(self):
             lnn.assert_called_once_with()
             tdp.assert_called_once_with('3')
 
+    def test_sync_teardown_namespace_does_not_crash_on_error(self):
+        """Test that sync tears down unneeded metadata namespaces.
+        Even if that fails it continues to provision other datapaths
+        """
+        with mock.patch.object(
+                self.agent, 'provision_datapath') as pdp,\
+                mock.patch.object(
+                    ip_lib, 'list_network_namespaces',
+                    return_value=['ovnmeta-1', 'ovnmeta-2', 'ovnmeta-3',
+                                  'ns1', 'ns2']) as lnn,\
+                mock.patch.object(
+                    self.agent, 'teardown_datapath',
+                    side_effect=Exception()) as tdp:
+            self.agent.sync()
+
+            pdp.assert_has_calls(
+                [
+                    mock.call(p.datapath)
+                    for p in self.ports
+                ],
+                any_order=True
+            )
+            lnn.assert_called_once_with()
+            tdp.assert_called_once_with('3')
+
     def test_get_networks_datapaths(self):
         """Test get_networks_datapaths returns only datapath objects for the
         networks containing vif ports of type ''(blank) and 'external'.

neutron/tests/unit/db/test_l3_db.py

@@ -252,27 +252,6 @@ def test__make_floatingip_dict_with_scope(self, make_fip_dict):
             'fixed_ip_address_scope': mock.sentinel.address_scope_id,
             'id': mock.sentinel.fip_ip}, result)
 
-    def test__unique_floatingip_iterator(self):
-        context = mock.MagicMock()
-        query = mock.MagicMock()
-        query.order_by().__iter__.return_value = [
-            ({'id': 'id1'}, 'scope1'),
-            ({'id': 'id1'}, 'scope1'),
-            ({'id': 'id2'}, 'scope2'),
-            ({'id': 'id2'}, 'scope2'),
-            ({'id': 'id2'}, 'scope2'),
-            ({'id': 'id3'}, 'scope3')]
-        query.reset_mock()
-        with mock.patch.object(
-                l3_obj.FloatingIP, '_load_object',
-                side_effect=({'id': 'id1'}, {'id': 'id2'}, {'id': 'id3'})):
-            result = list(
-                l3_obj.FloatingIP._unique_floatingip_iterator(context, query))
-            query.order_by.assert_called_once_with(l3_models.FloatingIP.id)
-            self.assertEqual([({'id': 'id1'}, 'scope1'),
-                              ({'id': 'id2'}, 'scope2'),
-                              ({'id': 'id3'}, 'scope3')], result)
-
     @mock.patch.object(directory, 'get_plugin')
     def test_prevent_l3_port_deletion_port_not_found(self, gp):
         # port not found doesn't prevent

neutron/tests/unit/objects/test_router.py

@@ -12,8 +12,11 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from itertools import chain
 from unittest import mock
 
+import netaddr
+
 from neutron_lib.db import api as db_api
 from oslo_utils import uuidutils
 
@@ -293,6 +296,51 @@ def test_v1_2_to_v1_1_drops_qos_network_policy_id(self):
         self.assertNotIn('qos_network_policy_id',
                          obj_v1_1['versioned_object.data'])
 
+    def test_get_scoped_floating_ips(self):
+        def compare_results(router_ids, original_fips):
+            self.assertCountEqual(
+                original_fips,
+                [
+                    fip[0].id
+                    for fip in router.FloatingIP.get_scoped_floating_ips(
+                        self.context, router_ids)
+                ]
+            )
+
+        # Setup three routers, networks and external networks
+        routers = {}
+        for i in range(3):
+            router_id = self._create_test_router_id(name=f'router-{i}')
+            routers[router_id] = []
+            net_id = self._create_test_network_id()
+            fip_net_id = self._create_external_network_id()
+
+            # Create three subnets and three FIPs using the
+            # aforementioned networks and routers
+            for j in range(3):
+                self._create_test_subnet_id(net_id)
+                fip = router.FloatingIP(
+                    self.context,
+                    floating_ip_address=netaddr.IPAddress(f'10.{i}.{j}.3'),
+                    floating_network_id=fip_net_id,
+                    floating_port_id=self._create_test_port_id(
+                        network_id=fip_net_id),
+                    fixed_port_id=self._create_test_port_id(
+                        network_id=net_id),
+                    router_id=router_id,
+                )
+                fip.create()
+                routers[router_id].append(fip.id)
+
+        # For each router we created, fetch the fips and ensure the
+        # results match what we originally created
+        for router_id, original_fips in routers.items():
+            compare_results([router_id], original_fips)
+
+        # Now try to fetch all the fips for all the routers at once
+        original_fips = list(chain.from_iterable(routers.values()))
+        compare_results(routers.keys(), original_fips)
+
 
 class DvrFipGatewayPortAgentBindingTestCase(
         obj_test_base.BaseObjectIfaceTestCase):

releasenotes/notes/forbid-subnet-gwip-deletion-router-interface-072a18373f920ed9.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    [`bug 2036423 <https://bugs.launchpad.net/neutron/+bug/2036423>`_]
+    Now it is not possible to delete a subnet gateway IP if that subnet has a
+    router interface; the subnet gateway IP modification was already forbidden.