Lookup interfaces by MAC directly

Currently the amphora agent will lookup interfaces using the
interface name determined earlier in the plug method. This can
lead to a race condition with the udev interface renaming rule.
This patch changes the interface lookup to use the MAC address
directly and not rely on the interface name.

Story: 2006300
Task: 36013

Change-Id: I5bc21d5abdeb67a3a8ae88456735643463f15694
This commit is contained in:
Michael Johnson 2019-07-29 09:49:24 -07:00
parent 6e4da85064
commit 2529fa33ab
3 changed files with 97 additions and 97 deletions

View File

@ -26,7 +26,6 @@ import six
import webob
from werkzeug import exceptions
import netifaces
from octavia.common import constants as consts
@ -82,8 +81,8 @@ class Plug(object):
return webob.Response(
json=dict(message="Interface already exists"), status=409)
# This is the interface prior to moving into the netns
default_netns_interface = self._interface_by_mac(mac_address)
# Check that the interface has been fully plugged
self._interface_by_mac(mac_address)
# Always put the VIP interface as eth1
primary_interface = consts.NETNS_PRIMARY_INTERFACE
@ -143,7 +142,7 @@ class Plug(object):
with pyroute2.IPRoute() as ipr:
# Move the interfaces into the namespace
idx = ipr.link_lookup(ifname=default_netns_interface)[0]
idx = ipr.link_lookup(address=mac_address)[0]
ipr.link('set', index=idx, net_ns_fd=consts.AMPHORA_NAMESPACE,
IFLA_IFNAME=primary_interface)
@ -213,7 +212,7 @@ class Plug(object):
with pyroute2.IPRoute() as ipr:
# Move the interfaces into the namespace
idx = ipr.link_lookup(ifname=default_netns_interface)[0]
idx = ipr.link_lookup(address=mac_address)[0]
ipr.link('set', index=idx,
net_ns_fd=consts.AMPHORA_NAMESPACE,
IFLA_IFNAME=netns_interface)
@ -227,12 +226,17 @@ class Plug(object):
interface=netns_interface)), status=202)
def _interface_by_mac(self, mac):
for interface in netifaces.interfaces():
if netifaces.AF_LINK in netifaces.ifaddresses(interface):
for link in netifaces.ifaddresses(
interface)[netifaces.AF_LINK]:
if link.get('addr', '').lower() == mac.lower():
return interface
try:
with pyroute2.IPRoute() as ipr:
idx = ipr.link_lookup(address=mac)[0]
addr = ipr.get_links(idx)[0]
for attr in addr['attrs']:
if attr[0] == 'IFLA_IFNAME':
return attr[1]
except Exception as e:
LOG.info('Unable to find interface with MAC: %s, rescanning '
'and returning 404. Reported error: %s', mac, str(e))
# Poke the kernel to re-enumerate the PCI bus.
# We have had cases where nova hot plugs the interface but
# the kernel doesn't get the memo.

View File

@ -21,7 +21,6 @@ import subprocess
import fixtures
import mock
import netifaces
from oslo_config import fixture as oslo_fixture
from oslo_serialization import jsonutils
from oslo_utils import uuidutils
@ -41,6 +40,7 @@ import octavia.tests.unit.base as base
AMP_AGENT_CONF_PATH = '/etc/octavia/amphora-agent.conf'
RANDOM_ERROR = b'random error'
OK = dict(message='OK')
FAKE_INTERFACE = 'eth33'
class TestServerTestCase(base.TestCase):
@ -966,8 +966,6 @@ class TestServerTestCase(base.TestCase):
self._test_plug_network(consts.CENTOS)
@mock.patch('os.chmod')
@mock.patch('netifaces.interfaces')
@mock.patch('netifaces.ifaddresses')
@mock.patch('pyroute2.IPRoute', create=True)
@mock.patch('pyroute2.NetNS', create=True)
@mock.patch('subprocess.check_output')
@ -976,7 +974,16 @@ class TestServerTestCase(base.TestCase):
@mock.patch('os.path.isfile')
def _test_plug_network(self, distro, mock_isfile, mock_int_exists,
mock_check_output, mock_netns, mock_pyroute2,
mock_ifaddress, mock_interfaces, mock_os_chmod):
mock_os_chmod):
mock_ipr = mock.MagicMock()
mock_ipr_instance = mock.MagicMock()
mock_ipr_instance.link_lookup.side_effect = [
[], [], [33], [33], [33], [33], [33], [33], [33], [33]]
mock_ipr_instance.get_links.return_value = ({
'attrs': [('IFLA_IFNAME', FAKE_INTERFACE)]},)
mock_ipr.__enter__.return_value = mock_ipr_instance
mock_pyroute2.return_value = mock_ipr
self.assertIn(distro, [consts.UBUNTU, consts.CENTOS])
port_info = {'mac_address': '123'}
test_int_num = random.randint(0, 9999)
@ -1006,7 +1013,6 @@ class TestServerTestCase(base.TestCase):
mock_int_exists.return_value = False
# No interface at all
mock_interfaces.side_effect = [[]]
file_name = '/sys/bus/pci/rescan'
m = self.useFixture(test_utils.OpenFixture(file_name)).mock_open
with mock.patch('os.open') as mock_open, mock.patch.object(
@ -1031,8 +1037,6 @@ class TestServerTestCase(base.TestCase):
# No interface down
m().reset_mock()
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_INET]]
with mock.patch('os.open') as mock_open, mock.patch.object(
os, 'fdopen', m) as mock_fdopen:
mock_open.return_value = 123
@ -1052,13 +1056,8 @@ class TestServerTestCase(base.TestCase):
self.assertEqual(404, rv.status_code)
self.assertEqual(dict(details="No suitable network interface found"),
jsonutils.loads(rv.data.decode('utf-8')))
mock_ifaddress.assert_called_once_with('blah')
# One Interface down, Happy Path
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
if self.conf.conf.amphora_agent.agent_server_network_file:
@ -1128,9 +1127,6 @@ class TestServerTestCase(base.TestCase):
# fixed IPs happy path
port_info = {'mac_address': '123', 'mtu': 1450, 'fixed_ips': [
{'ip_address': '10.0.0.5', 'subnet_cidr': '10.0.0.0/24'}]}
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
@ -1206,9 +1202,6 @@ class TestServerTestCase(base.TestCase):
# fixed IPs happy path IPv6
port_info = {'mac_address': '123', 'mtu': 1450, 'fixed_ips': [
{'ip_address': '2001:db8::2', 'subnet_cidr': '2001:db8::/32'}]}
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
@ -1280,9 +1273,6 @@ class TestServerTestCase(base.TestCase):
# fixed IPs, bogus IP
port_info = {'mac_address': '123', 'fixed_ips': [
{'ip_address': '10005', 'subnet_cidr': '10.0.0.0/24'}]}
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC
mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
@ -1307,9 +1297,6 @@ class TestServerTestCase(base.TestCase):
# same as above but ifup fails
port_info = {'mac_address': '123', 'fixed_ips': [
{'ip_address': '10.0.0.5', 'subnet_cidr': '10.0.0.0/24'}]}
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
mock_check_output.side_effect = [subprocess.CalledProcessError(
7, 'test', RANDOM_ERROR), subprocess.CalledProcessError(
7, 'test', RANDOM_ERROR)]
@ -1369,15 +1356,19 @@ class TestServerTestCase(base.TestCase):
self._test_plug_network_host_routes(consts.CENTOS)
@mock.patch('os.chmod')
@mock.patch('netifaces.interfaces')
@mock.patch('netifaces.ifaddresses')
@mock.patch('pyroute2.IPRoute', create=True)
@mock.patch('pyroute2.NetNS', create=True)
@mock.patch('subprocess.check_output')
def _test_plug_network_host_routes(self, distro, mock_check_output,
mock_netns, mock_pyroute2,
mock_ifaddress, mock_interfaces,
mock_os_chmod):
mock_ipr = mock.MagicMock()
mock_ipr_instance = mock.MagicMock()
mock_ipr_instance.link_lookup.return_value = [33]
mock_ipr_instance.get_links.return_value = ({
'attrs': [('IFLA_IFNAME', FAKE_INTERFACE)]},)
mock_ipr.__enter__.return_value = mock_ipr_instance
mock_pyroute2.return_value = mock_ipr
self.assertIn(distro, [consts.UBUNTU, consts.CENTOS])
@ -1398,9 +1389,6 @@ class TestServerTestCase(base.TestCase):
{'ip_address': IP, 'subnet_cidr': SUBNET_CIDR,
'host_routes': [{'destination': DEST1, 'nexthop': NEXTHOP},
{'destination': DEST2, 'nexthop': NEXTHOP}]}]}
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC
mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
@ -1495,8 +1483,6 @@ class TestServerTestCase(base.TestCase):
@mock.patch('pyroute2.NSPopen', create=True)
@mock.patch('octavia.amphorae.backends.agent.api_server.'
'plug.Plug._netns_interface_exists')
@mock.patch('netifaces.interfaces')
@mock.patch('netifaces.ifaddresses')
@mock.patch('pyroute2.IPRoute', create=True)
@mock.patch('pyroute2.netns.create', create=True)
@mock.patch('pyroute2.NetNS', create=True)
@ -1506,9 +1492,16 @@ class TestServerTestCase(base.TestCase):
@mock.patch('os.path.isfile')
def _test_plug_VIP4(self, distro, mock_isfile, mock_makedirs,
mock_copytree, mock_check_output, mock_netns,
mock_netns_create, mock_pyroute2, mock_ifaddress,
mock_interfaces, mock_int_exists, mock_nspopen,
mock_copy2, mock_os_chmod):
mock_netns_create, mock_pyroute2, mock_int_exists,
mock_nspopen, mock_copy2, mock_os_chmod):
mock_ipr = mock.MagicMock()
mock_ipr_instance = mock.MagicMock()
mock_ipr_instance.link_lookup.side_effect = [[], [], [33], [33], [33],
[33], [33], [33]]
mock_ipr_instance.get_links.return_value = ({
'attrs': [('IFLA_IFNAME', FAKE_INTERFACE)]},)
mock_ipr.__enter__.return_value = mock_ipr_instance
mock_pyroute2.return_value = mock_ipr
mock_isfile.return_value = True
@ -1560,7 +1553,6 @@ class TestServerTestCase(base.TestCase):
mock_int_exists.return_value = False
# No interface at all
mock_interfaces.side_effect = [[]]
file_name = '/sys/bus/pci/rescan'
m = self.useFixture(test_utils.OpenFixture(file_name)).mock_open
with mock.patch('os.open') as mock_open, mock.patch.object(
@ -1586,8 +1578,6 @@ class TestServerTestCase(base.TestCase):
# Two interfaces down
m().reset_mock()
mock_interfaces.side_effect = [['blah', 'blah2']]
mock_ifaddress.side_effect = [['blabla'], ['blabla']]
with mock.patch('os.open') as mock_open, mock.patch.object(
os, 'fdopen', m) as mock_fdopen:
mock_open.return_value = 123
@ -1622,10 +1612,6 @@ class TestServerTestCase(base.TestCase):
'nexthop': '203.0.113.5'}]
}
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
if self.conf.conf.amphora_agent.agent_server_network_file:
@ -1755,10 +1741,6 @@ class TestServerTestCase(base.TestCase):
mock_nspopen.assert_has_calls(calls, any_order=True)
# One Interface down, Happy Path IPv4
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
if self.conf.conf.amphora_agent.agent_server_network_file:
@ -1843,10 +1825,6 @@ class TestServerTestCase(base.TestCase):
['ip', 'netns', 'exec', consts.AMPHORA_NAMESPACE,
'ifup', '{netns_int}:0'.format(
netns_int=consts.NETNS_PRIMARY_INTERFACE)], stderr=-2)
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
mock_check_output.side_effect = [
'unplug1',
subprocess.CalledProcessError(
@ -1880,8 +1858,6 @@ class TestServerTestCase(base.TestCase):
@mock.patch('os.chmod')
@mock.patch('shutil.copy2')
@mock.patch('pyroute2.NSPopen', create=True)
@mock.patch('netifaces.interfaces')
@mock.patch('netifaces.ifaddresses')
@mock.patch('pyroute2.IPRoute', create=True)
@mock.patch('pyroute2.netns.create', create=True)
@mock.patch('pyroute2.NetNS', create=True)
@ -1891,9 +1867,16 @@ class TestServerTestCase(base.TestCase):
@mock.patch('os.path.isfile')
def _test_plug_vip6(self, distro, mock_isfile, mock_makedirs,
mock_copytree, mock_check_output, mock_netns,
mock_netns_create, mock_pyroute2, mock_ifaddress,
mock_interfaces, mock_nspopen, mock_copy2,
mock_os_chmod):
mock_netns_create, mock_pyroute2, mock_nspopen,
mock_copy2, mock_os_chmod):
mock_ipr = mock.MagicMock()
mock_ipr_instance = mock.MagicMock()
mock_ipr_instance.link_lookup.side_effect = [[], [], [33], [33], [33],
[33], [33], [33]]
mock_ipr_instance.get_links.return_value = ({
'attrs': [('IFLA_IFNAME', FAKE_INTERFACE)]},)
mock_ipr.__enter__.return_value = mock_ipr_instance
mock_pyroute2.return_value = mock_ipr
mock_isfile.return_value = True
@ -1933,7 +1916,6 @@ class TestServerTestCase(base.TestCase):
self.assertEqual(400, rv.status_code)
# No interface at all
mock_interfaces.side_effect = [[]]
file_name = '/sys/bus/pci/rescan'
m = self.useFixture(test_utils.OpenFixture(file_name)).mock_open
with mock.patch('os.open') as mock_open, mock.patch.object(
@ -1958,8 +1940,6 @@ class TestServerTestCase(base.TestCase):
# Two interfaces down
m().reset_mock()
mock_interfaces.side_effect = [['blah', 'blah2']]
mock_ifaddress.side_effect = [['blabla'], ['blabla']]
with mock.patch('os.open') as mock_open, mock.patch.object(
os, 'fdopen', m) as mock_fdopen:
mock_open.return_value = 123
@ -1993,10 +1973,6 @@ class TestServerTestCase(base.TestCase):
'nexthop': '2001:db8::5'}]
}
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC
mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
@ -2128,10 +2104,6 @@ class TestServerTestCase(base.TestCase):
mock_nspopen.assert_has_calls(calls, any_order=True)
# One Interface down, Happy Path IPv6
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC
mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
if distro == consts.UBUNTU:
@ -2222,10 +2194,6 @@ class TestServerTestCase(base.TestCase):
['ip', 'netns', 'exec', consts.AMPHORA_NAMESPACE,
'ifup', '{netns_int}'.format(
netns_int=consts.NETNS_PRIMARY_INTERFACE)], stderr=-2)
mock_interfaces.side_effect = [['blah']]
mock_ifaddress.side_effect = [[netifaces.AF_LINK],
{netifaces.AF_LINK: [{'addr': '123'}]}]
mock_check_output.side_effect = [
'unplug1',
subprocess.CalledProcessError(

View File

@ -16,9 +16,9 @@ import os
import subprocess
import mock
import netifaces
from oslo_config import cfg
from oslo_config import fixture as oslo_fixture
from werkzeug import exceptions as wz_exceptions
from octavia.amphorae.backends.agent.api_server import osutils
from octavia.amphorae.backends.agent.api_server import plug
@ -33,40 +33,64 @@ FAKE_GATEWAY_IPV6 = '2001:db8::1'
FAKE_IP_IPV6 = '2001:db8::2'
FAKE_IP_IPV6_EXPANDED = '2001:0db8:0000:0000:0000:0000:0000:0002'
FAKE_MAC_ADDRESS = 'ab:cd:ef:00:ff:22'
FAKE_INTERFACE = 'eth0'
FAKE_INTERFACE = 'eth33'
class TestPlug(base.TestCase):
def setUp(self):
super(TestPlug, self).setUp()
self.mock_netifaces = mock.patch.object(plug, "netifaces").start()
self.mock_platform = mock.patch("distro.id").start()
self.mock_platform.return_value = "ubuntu"
self.osutil = osutils.BaseOS.get_os_util()
self.test_plug = plug.Plug(self.osutil)
self.addCleanup(self.mock_netifaces.stop)
self.addCleanup(self.mock_platform.stop)
# Set up our fake interface
self.mock_netifaces.AF_LINK = netifaces.AF_LINK
self.mock_netifaces.interfaces.return_value = [FAKE_INTERFACE]
self.mock_netifaces.ifaddresses.return_value = {
netifaces.AF_LINK: [
{'addr': FAKE_MAC_ADDRESS.lower()}
]
}
@mock.patch('pyroute2.IPRoute.__enter__', create=True)
def test__interface_by_mac_case_insensitive_ubuntu(self, mock_ipr):
mock_ipr_instance = mock.MagicMock()
mock_ipr_instance.link_lookup.return_value = [33]
mock_ipr_instance.get_links.return_value = ({
'attrs': [('IFLA_IFNAME', FAKE_INTERFACE)]},)
mock_ipr.return_value = mock_ipr_instance
def test__interface_by_mac_case_insensitive_ubuntu(self):
interface = self.test_plug._interface_by_mac(FAKE_MAC_ADDRESS.upper())
self.assertEqual(FAKE_INTERFACE, interface)
mock_ipr_instance.get_links.assert_called_once_with(33)
@mock.patch('pyroute2.IPRoute.__enter__', create=True)
def test__interface_by_mac_not_found(self, mock_ipr):
mock_ipr_instance = mock.MagicMock()
mock_ipr_instance.link_lookup.return_value = []
mock_ipr.return_value = mock_ipr_instance
fd_mock = mock.mock_open()
open_mock = mock.Mock()
with mock.patch('os.open', open_mock), mock.patch.object(
os, 'fdopen', fd_mock):
self.assertRaises(wz_exceptions.HTTPException,
self.test_plug._interface_by_mac,
FAKE_MAC_ADDRESS.upper())
open_mock.assert_called_once_with('/sys/bus/pci/rescan', os.O_WRONLY)
fd_mock().write.assert_called_once_with('1')
@mock.patch('pyroute2.IPRoute.__enter__', create=True)
def test__interface_by_mac_case_insensitive_rh(self, mock_ipr):
mock_ipr_instance = mock.MagicMock()
mock_ipr_instance.link_lookup.return_value = [33]
mock_ipr_instance.get_links.return_value = ({
'attrs': [('IFLA_IFNAME', FAKE_INTERFACE)]},)
mock_ipr.return_value = mock_ipr_instance
def test__interface_by_mac_case_insensitive_rh(self):
with mock.patch('distro.id', return_value='centos'):
osutil = osutils.BaseOS.get_os_util()
self.test_plug = plug.Plug(osutil)
interface = self.test_plug._interface_by_mac(
FAKE_MAC_ADDRESS.upper())
self.assertEqual(FAKE_INTERFACE, interface)
mock_ipr_instance.get_links.assert_called_once_with(33)
@mock.patch('octavia.amphorae.backends.agent.api_server.plug.Plug.'
'_interface_by_mac', return_value=FAKE_INTERFACE)
@mock.patch('pyroute2.NSPopen', create=True)
@mock.patch.object(plug, "webob")
@mock.patch('pyroute2.IPRoute', create=True)
@ -77,7 +101,8 @@ class TestPlug(base.TestCase):
@mock.patch('os.makedirs')
def test_plug_vip_ipv4(self, mock_makedirs, mock_copytree,
mock_check_output, mock_netns, mock_netns_create,
mock_pyroute2, mock_webob, mock_nspopen):
mock_pyroute2, mock_webob, mock_nspopen,
mock_by_mac):
m = mock.mock_open()
with mock.patch('os.open'), mock.patch.object(os, 'fdopen', m):
self.test_plug.plug_vip(
@ -103,6 +128,8 @@ class TestPlug(base.TestCase):
stdout=subprocess.PIPE)]
mock_nspopen.assert_has_calls(calls, any_order=True)
@mock.patch('octavia.amphorae.backends.agent.api_server.plug.Plug.'
'_interface_by_mac', return_value=FAKE_INTERFACE)
@mock.patch('pyroute2.NSPopen', create=True)
@mock.patch.object(plug, "webob")
@mock.patch('pyroute2.IPRoute', create=True)
@ -113,7 +140,8 @@ class TestPlug(base.TestCase):
@mock.patch('os.makedirs')
def test_plug_vip_ipv6(self, mock_makedirs, mock_copytree,
mock_check_output, mock_netns, mock_netns_create,
mock_pyroute2, mock_webob, mock_nspopen):
mock_pyroute2, mock_webob, mock_nspopen,
mock_by_mac):
conf = self.useFixture(oslo_fixture.Config(cfg.CONF))
conf.config(group='controller_worker',
loadbalancer_topology=constants.TOPOLOGY_ACTIVE_STANDBY)