openstack
/
neutron
Code Issues Proposed changes

Add and delete multiple ip addresses in one priv call 

Created new add_ip_addresses privileged function
which takes an iterable of cidrs and adds them
in one privileged call. This is so we dont have to
take on additional priv overhead when calling
add_ip_address in a loop.
For parity, performed the same change on the
delete_ip_address function.

Closes-Bug: #1987281
Partial-Bug: #1981113
Change-Id: Ib1278af20c3b3b057712453cb249aba34b684a21
This commit is contained in:
Miro Tomaska 2022-09-02 10:26:11 -05:00
parent 12b21e235e
commit 81980146cb
6 changed files with 189 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
neutron/agent/linux/ip_lib.py
View File

@ -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.

24
neutron/agent/ovn/metadata/agent.py
View File

@ -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

23
neutron/common/utils.py
View File

@ -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'))

53
neutron/privileged/agent/linux/ip_lib.py
View File

@ -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]

67
neutron/tests/functional/agent/linux/test_ip_lib.py
View File

@ -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'),

12
neutron/tests/unit/agent/ovn/metadata/test_agent.py
View File

@ -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,