diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index f0f59124a40..3373395354e 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -536,9 +536,16 @@ class IpAddrCommand(IpDeviceCommandBase): add_ip_address(cidr, self.name, self._parent.namespace, scope, add_broadcast) + def add_multiple(self, cidrs, scope='global', add_broadcast=True): + add_ip_addresses(cidrs, self.name, self._parent.namespace, scope, + add_broadcast) + def delete(self, cidr): delete_ip_address(cidr, self.name, self._parent.namespace) + def delete_multiple(self, cidrs): + delete_ip_addresses(cidrs, self.name, self._parent.namespace) + def flush(self, ip_version): flush_ip_addresses(ip_version, self.name, self._parent.namespace) @@ -814,15 +821,27 @@ def add_ip_address(cidr, device, namespace=None, scope='global', """ net = netaddr.IPNetwork(cidr) broadcast = None - if add_broadcast and net.version == 4: - # NOTE(slaweq): in case if cidr is /32 net.broadcast is None so - # same IP address as cidr should be set as broadcast - broadcast = str(net.broadcast or net.ip) + if add_broadcast: + broadcast = common_utils.cidr_broadcast_address_alternative(cidr) privileged.add_ip_address( net.version, str(net.ip), net.prefixlen, device, namespace, scope, broadcast) +def add_ip_addresses(cidrs, device, namespace=None, scope='global', + add_broadcast=True): + """Add multiple IP addresses. + + :param cidrs: A list of IP addresses to add, in CIDR notation + :param device: Device name to use in adding address + :param namespace: The name of the namespace in which to add the address + :param scope: scope of address being added + :param add_broadcast: should broadcast address be added + """ + privileged.add_ip_addresses( + cidrs, device, namespace, scope, add_broadcast) + + def delete_ip_address(cidr, device, namespace=None): """Delete an IP address. @@ -835,6 +854,16 @@ def delete_ip_address(cidr, device, namespace=None): net.version, str(net.ip), net.prefixlen, device, namespace) +def delete_ip_addresses(cidrs, device, namespace=None): + """Delete multiple IP address. + + :param cidrs: A list of IP addresses to delete, in CIDR notation + :param device: Device name to use in deleting address + :param namespace: The name of the namespace in which to delete the address + """ + privileged.delete_ip_addresses(cidrs, device, namespace) + + def flush_ip_addresses(ip_version, device, namespace=None): """Flush all IP addresses. diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index a60ad26d964..e685b2b0955 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -483,16 +483,20 @@ class MetadataAgent(object): # Configure the IP addresses on the VETH pair and remove those # that we no longer need. current_cidrs = {dev['cidr'] for dev in dev_info} - for ipaddr in current_cidrs - metadata_port.ip_addresses: - ip2.addr.delete(ipaddr) - for ipaddr in metadata_port.ip_addresses - current_cidrs: - # NOTE(dalvarez): metadata only works on IPv4. We're doing this - # extra check here because it could be that the metadata port has - # an IPv6 address if there's an IPv6 subnet with SLAAC in its - # network. Neutron IPAM will autoallocate an IPv6 address for every - # port in the network. - if utils.get_ip_version(ipaddr) == 4: - ip2.addr.add(ipaddr) + cidrs_to_delete = list(current_cidrs - metadata_port.ip_addresses) + if cidrs_to_delete: + ip2.addr.delete_multiple(cidrs_to_delete) + + # NOTE(dalvarez): metadata only works on IPv4. We're doing this + # extra check here because it could be that the metadata port has + # an IPv6 address if there's an IPv6 subnet with SLAAC in its + # network. Neutron IPAM will autoallocate an IPv6 address for every + # port in the network. + ipv4_cidrs = [] + for cidr in metadata_port.ip_addresses - current_cidrs: + if utils.get_ip_version(cidr) == n_const.IP_VERSION_4: + ipv4_cidrs.append(cidr) + ip2.addr.add_multiple(ipv4_cidrs) # Check that this port is not attached to any other OVS bridge. This # can happen when the OVN bridge changes (for example, during a diff --git a/neutron/common/utils.py b/neutron/common/utils.py index 0f861338bb6..92921f648fc 100644 --- a/neutron/common/utils.py +++ b/neutron/common/utils.py @@ -311,14 +311,33 @@ def cidr_broadcast_address(cidr): return str(broadcast) +def cidr_broadcast_address_alternative(cidr): + """Wraps cidr_broadcast address and handles last cidr differently in + respect to netaddr + + :param cidr: (string, netaddr.IPNetwork, netaddr.IPAddress) either an ipv4 + or ipv6 cidr. + :returns: (string) broadcast address of the cidr, + IP of the cidr if IPv4 /32 CIDR, otherwise None + """ + broadcast = cidr_broadcast_address(cidr) + if broadcast: + return str(broadcast) + net = netaddr.IPNetwork(cidr) + if net.prefixlen == 32 and net.version == n_const.IP_VERSION_4: + return str(net.ip) + # NOTE(mtomaska): netaddr ver >= 0.9.0 will start returning None + # for IPv6 cidr /127 /128 + + def get_ip_version(ip_or_cidr): return netaddr.IPNetwork(ip_or_cidr).version def ip_version_from_int(ip_version_int): - if ip_version_int == 4: + if ip_version_int == n_const.IP_VERSION_4: return n_const.IPv4 - if ip_version_int == 6: + if ip_version_int == n_const.IP_VERSION_6: return n_const.IPv6 raise ValueError(_('Illegal IP version number')) diff --git a/neutron/privileged/agent/linux/ip_lib.py b/neutron/privileged/agent/linux/ip_lib.py index 7ad3af03349..71ace7e2586 100644 --- a/neutron/privileged/agent/linux/ip_lib.py +++ b/neutron/privileged/agent/linux/ip_lib.py @@ -14,6 +14,7 @@ import errno import os import socket +import netaddr from neutron_lib import constants from oslo_log import log as logging import pyroute2 @@ -27,6 +28,7 @@ from pyroute2 import netns # pylint: disable=no-name-in-module import tenacity from neutron._i18n import _ +from neutron.common import utils as common_utils from neutron import privileged from neutron.privileged.agent import linux as priv_linux @@ -272,6 +274,32 @@ def add_ip_address(ip_version, ip, prefixlen, device, namespace, scope, raise +@privileged.default.entrypoint +def add_ip_addresses(cidrs, device, namespace, scope, + add_broadcast=True): + for cidr in cidrs: + net = netaddr.IPNetwork(cidr) + ip = str(net.ip) + prefixlen = net.prefixlen + family = _IP_VERSION_FAMILY_MAP[net.version] + broadcast = None + if add_broadcast: + broadcast = common_utils.cidr_broadcast_address_alternative(cidr) + try: + _run_iproute_addr('add', + device, + namespace, + address=ip, + mask=prefixlen, + family=family, + broadcast=broadcast, + scope=get_scope_name(scope)) + except netlink_exceptions.NetlinkError as e: + if e.code == errno.EEXIST: + raise IpAddressAlreadyExists(ip=ip, device=device) + raise + + @privileged.default.entrypoint def delete_ip_address(ip_version, ip, prefixlen, device, namespace): family = _IP_VERSION_FAMILY_MAP[ip_version] @@ -292,6 +320,31 @@ def delete_ip_address(ip_version, ip, prefixlen, device, namespace): raise +@privileged.default.entrypoint +def delete_ip_addresses(cidrs, device, namespace): + for cidr in cidrs: + net = netaddr.IPNetwork(cidr) + ip = str(net.ip) + prefixlen = net.prefixlen + family = _IP_VERSION_FAMILY_MAP[net.version] + try: + _run_iproute_addr("delete", + device, + namespace, + address=ip, + mask=prefixlen, + family=family) + except netlink_exceptions.NetlinkError as e: + # when trying to delete a non-existent IP address, pyroute2 raises + # NetlinkError with code EADDRNOTAVAIL (99, 'Cannot assign + # requested address') + # this shouldn't raise an error + if e.code == errno.EADDRNOTAVAIL: + pass + else: + raise + + @privileged.default.entrypoint def flush_ip_addresses(ip_version, device, namespace): family = _IP_VERSION_FAMILY_MAP[ip_version] diff --git a/neutron/tests/functional/agent/linux/test_ip_lib.py b/neutron/tests/functional/agent/linux/test_ip_lib.py index 540d8a8dbd5..2e9f40aa0c1 100644 --- a/neutron/tests/functional/agent/linux/test_ip_lib.py +++ b/neutron/tests/functional/agent/linux/test_ip_lib.py @@ -613,6 +613,9 @@ class IpLibTestCase(IpLibTestFramework): cidr = netaddr.IPNetwork(ip_address['cidr']) self.assertNotEqual(ip_version, cidr.version) + def _get_cidrs_from_device(self, device_obj): + return [ip_info['cidr'] for ip_info in device_obj.addr.list()] + def test_add_ip_address(self): ip_addresses = [ (netaddr.IPNetwork("10.10.10.10/30"), "global", '10.10.10.11'), @@ -629,22 +632,78 @@ class IpLibTestCase(IpLibTestFramework): self.assertRaises(RuntimeError, device.addr.add, str(ip_address[0]), ip_address[1]) + def test_add_ip_addresses(self): + expected_cidrs = [ + "10.10.10.10/30", + "11.11.11.11/28", + "2801::1/120", + "fe80::/64" + ] + attr = self.generate_device_details(ip_cidrs=[]) + device = self.manage_device(attr) + + device.addr.add_multiple(expected_cidrs) + + self.assertListEqual( + expected_cidrs, + self._get_cidrs_from_device(device) + ) + def test_delete_ip_address(self): attr = self.generate_device_details() cidr = attr.ip_cidrs[0] device = self.manage_device(attr) - device_cidrs = [ip_info['cidr'] for ip_info in device.addr.list()] - self.assertIn(cidr, device_cidrs) + device_cidrs_before_delete = self._get_cidrs_from_device(device) + self.assertIn(cidr, device_cidrs_before_delete) device.addr.delete(cidr) - device_cidrs = [ip_info['cidr'] for ip_info in device.addr.list()] - self.assertNotIn(cidr, device_cidrs) + device_cidrs_after_delete = self._get_cidrs_from_device(device) + self.assertNotIn(cidr, device_cidrs_after_delete) # Try to delete not existing IP address, it should be just fine and # finish without any error raised device.addr.delete(cidr) + def test_delete_all_ip_addresses(self): + cidrs = [ + "10.10.10.10/30", + "11.11.11.11/28", + "2801::1/120", + "fe80::/64" + ] + attr = self.generate_device_details(ip_cidrs=cidrs) + device = self.manage_device(attr) + + device_cidrs_before_delete = self._get_cidrs_from_device(device) + self.assertCountEqual(cidrs, device_cidrs_before_delete) + + device.addr.delete_multiple(cidrs) + + self.assertEqual(0, len(device.addr.list())) + + def test_delete_some_ip_addresses(self): + cidrs = [ + "10.10.10.10/30", + "11.11.11.11/28", + "2801::1/120", + "fe80::/64" + ] + attr = self.generate_device_details(ip_cidrs=cidrs) + device = self.manage_device(attr) + + device_cidrs_before_delete = self._get_cidrs_from_device(device) + self.assertCountEqual(cidrs, device_cidrs_before_delete) + + # delete the last two cidrs + device.addr.delete_multiple(cidrs[-2:]) + + # confirm only remaining cidrs are present + self.assertCountEqual( + cidrs[:2], + self._get_cidrs_from_device(device) + ) + def test_flush_ip_addresses(self): ip_addresses = [ (netaddr.IPNetwork("10.10.10.10/30"), "global", '10.10.10.11'), diff --git a/neutron/tests/unit/agent/ovn/metadata/test_agent.py b/neutron/tests/unit/agent/ovn/metadata/test_agent.py index 041433a3758..31265eaa254 100644 --- a/neutron/tests/unit/agent/ovn/metadata/test_agent.py +++ b/neutron/tests/unit/agent/ovn/metadata/test_agent.py @@ -231,7 +231,8 @@ class TestMetadataAgent(base.BaseTestCase): mock.patch.object(ip_link, 'set_up') as link_set_up,\ mock.patch.object(ip_link, 'set_address') as link_set_addr,\ mock.patch.object(ip_addr, 'list', return_value=[]),\ - mock.patch.object(ip_addr, 'add') as ip_addr_add,\ + mock.patch.object( + ip_addr, 'add_multiple') as ip_addr_add_multiple,\ mock.patch.object( ip_wrap, 'add_veth', return_value=[ip_lib.IPDevice('ip1'), @@ -261,12 +262,9 @@ class TestMetadataAgent(base.BaseTestCase): 'br-int', 'veth_0') self.agent.ovs_idl.db_set.assert_called_once_with( 'Interface', 'veth_0', ('external_ids', {'iface-id': 'port'})) - # Check that the metadata port has the IP addresses properly - # configured and that IPv6 address has been skipped. - expected_calls = [mock.call('10.0.0.1/23'), - mock.call(n_const.METADATA_CIDR)] - self.assertEqual(sorted(expected_calls), - sorted(ip_addr_add.call_args_list)) + expected_call = [n_const.METADATA_CIDR, '10.0.0.1/23'] + self.assertCountEqual(expected_call, + ip_addr_add_multiple.call_args.args[0]) # Check that metadata proxy has been spawned spawn_mdp.assert_called_once_with( mock.ANY, 'namespace', 80, mock.ANY,