Avoid interface name collisions in the amphora

After unpluging a network, the octavia-agent may try to plug a new
network to a new interface while another interface with the same name
already exists.

Closes-Bug: #2017894

Change-Id: I7f150c2c1c3da706055aba5e02b5ad7c032806f1

octavia/amphorae/backends/agent/api_server/plug.py

@@ -14,6 +14,7 @@
 # under the License.
 
 import ipaddress
+import itertools
 import os
 import socket
 import stat
@@ -212,13 +213,7 @@ class Plug(object):
 
         # We need to determine the interface name when inside the namespace
         # to avoid name conflicts
-        with pyroute2.NetNS(consts.AMPHORA_NAMESPACE,
-                            flags=os.O_CREAT) as netns:
-
-            # 1 means just loopback, but we should already have a VIP. This
-            # works for the add/delete/add case as we don't delete interfaces
-            # Note, eth0 is skipped because that is the VIP interface
-            netns_interface = 'eth{0}'.format(len(netns.get_links()))
+        netns_interface = self._netns_get_next_interface()
 
         LOG.info('Plugged interface %s will become %s in the namespace %s',
                  default_netns_interface, netns_interface,
@@ -295,3 +290,16 @@ class Plug(object):
 
     def _netns_interface_exists(self, mac_address):
         return self._netns_interface_by_mac(mac_address) is not None
+
+    def _netns_get_next_interface(self):
+        with pyroute2.NetNS(consts.AMPHORA_NAMESPACE,
+                            flags=os.O_CREAT) as netns:
+            existing_ifaces = [
+                dict(link['attrs']).get(consts.IFLA_IFNAME)
+                for link in netns.get_links()]
+            # find the first unused ethXXX
+            for idx in itertools.count(start=2):
+                iface_name = f"eth{idx}"
+                if iface_name not in existing_ifaces:
+                    break
+            return iface_name

octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py

@@ -1008,7 +1008,9 @@ class TestServerTestCase(base.TestCase):
 
         mock_int_exists.return_value = False
         netns_handle = mock_netns.return_value.__enter__.return_value
-        netns_handle.get_links.return_value = [0] * test_int_num
+        netns_handle.get_links.return_value = [
+            {'attrs': [['IFLA_IFNAME', f'eth{idx}']]}
+            for idx in range(test_int_num)]
         mock_isfile.return_value = True
 
         mock_check_output.return_value = b"1\n2\n3\n"
@@ -1396,7 +1398,7 @@ class TestServerTestCase(base.TestCase):
 
         netns_handle = mock_netns.return_value.__enter__.return_value
         netns_handle.get_links.return_value = [{
-            'attrs': [['IFLA_IFNAME', consts.NETNS_PRIMARY_INTERFACE]]}]
+            'attrs': [['IFLA_IFNAME', 'eth2']]}]
 
         port_info = {'mac_address': MAC, 'mtu': 1450, 'fixed_ips': [
             {'ip_address': IP, 'subnet_cidr': SUBNET_CIDR,
@@ -1405,8 +1407,7 @@ class TestServerTestCase(base.TestCase):
 
         flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC
         mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
-        file_name = '/etc/octavia/interfaces/{}.json'.format(
-            consts.NETNS_PRIMARY_INTERFACE)
+        file_name = '/etc/octavia/interfaces/eth3.json'
 
         m = self.useFixture(test_utils.OpenFixture(file_name)).mock_open
         with mock.patch('os.open') as mock_open, mock.patch.object(
@@ -1436,7 +1437,7 @@ class TestServerTestCase(base.TestCase):
             mock_fdopen.assert_any_call(123, 'r+')
 
             expected_dict = {
-                consts.NAME: consts.NETNS_PRIMARY_INTERFACE,
+                consts.NAME: 'eth3',
                 consts.MTU: 1450,
                 consts.ADDRESSES: [
                     {
@@ -1458,13 +1459,12 @@ class TestServerTestCase(base.TestCase):
                 consts.SCRIPTS: {
                     consts.IFACE_UP: [{
                         consts.COMMAND: (
-                            "/usr/local/bin/lvs-masquerade.sh add ipv4 "
-                            "{}".format(consts.NETNS_PRIMARY_INTERFACE))
+                            "/usr/local/bin/lvs-masquerade.sh add ipv4 eth3")
                     }],
                     consts.IFACE_DOWN: [{
                         consts.COMMAND: (
                             "/usr/local/bin/lvs-masquerade.sh delete ipv4 "
-                            "{}".format(consts.NETNS_PRIMARY_INTERFACE))
+                            "eth3")
                     }]
                 }
             }
@@ -1476,8 +1476,7 @@ class TestServerTestCase(base.TestCase):
 
             mock_check_output.assert_called_with(
                 ['ip', 'netns', 'exec', consts.AMPHORA_NAMESPACE,
-                 'amphora-interface', 'up',
-                 consts.NETNS_PRIMARY_INTERFACE], stderr=-2)
+                 'amphora-interface', 'up', 'eth3'], stderr=-2)
 
     def test_ubuntu_plug_VIP4(self):
         self._test_plug_VIP4(consts.UBUNTU)

octavia/tests/unit/amphorae/backends/agent/api_server/test_plug.py

@@ -285,13 +285,13 @@ class TestPlug(base.TestCase):
             self.test_plug.plug_network(FAKE_MAC_ADDRESS, fixed_ips, 1400)
 
         mock_write_port_interface.assert_called_once_with(
-            interface='eth0', fixed_ips=fixed_ips, mtu=mtu)
-        mock_if_up.assert_called_once_with('eth0', 'network')
+            interface='eth2', fixed_ips=fixed_ips, mtu=mtu)
+        mock_if_up.assert_called_once_with('eth2', 'network')
         mock_send_member_adv.assert_called_once_with(fixed_ips)
 
         mock_webob.Response.assert_any_call(
             json={'message': 'OK',
-                  'details': 'Plugged on interface eth0'},
+                  'details': 'Plugged on interface eth2'},
             status=202)
 
     @mock.patch.object(plug, "webob")
@@ -411,3 +411,40 @@ class TestPlug(base.TestCase):
                   'details': 'Updated existing interface {}'.format(
                       FAKE_INTERFACE)},
             status=202)
+
+    @mock.patch('pyroute2.NetNS', create=True)
+    def test__netns_get_next_interface(self, mock_netns):
+        netns_handle = mock_netns.return_value.__enter__.return_value
+
+        netns_handle.get_links.return_value = [
+            {'attrs': [['IFLA_IFNAME', 'lo']]},
+        ]
+
+        ifname = self.test_plug._netns_get_next_interface()
+        self.assertEqual('eth2', ifname)
+
+        netns_handle.get_links.return_value = [
+            {'attrs': [['IFLA_IFNAME', 'lo']]},
+            {'attrs': [['IFLA_IFNAME', 'eth1']]},
+            {'attrs': [['IFLA_IFNAME', 'eth2']]},
+            {'attrs': [['IFLA_IFNAME', 'eth3']]},
+        ]
+
+        ifname = self.test_plug._netns_get_next_interface()
+        self.assertEqual('eth4', ifname)
+
+        netns_handle.get_links.return_value = [
+            {'attrs': [['IFLA_IFNAME', 'lo']]},
+            {'attrs': [['IFLA_IFNAME', 'eth1']]},
+            {'attrs': [['IFLA_IFNAME', 'eth3']]},
+        ]
+
+        ifname = self.test_plug._netns_get_next_interface()
+        self.assertEqual('eth2', ifname)
+
+        netns_handle.get_links.return_value = [
+            {'attrs': [['IFLA_IFNAME', f'eth{idx}']]}
+            for idx in range(2, 1000)]
+
+        ifname = self.test_plug._netns_get_next_interface()
+        self.assertEqual('eth1000', ifname)

releasenotes/notes/fix-network-interface-collision-939fd32587ea3344.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Fixed a potential error when plugging a member from a new network after
+    deleting another member and unplugging its network. Octavia may have tried
+    to plug the new network to a new interface but with an already existing
+    name.
+    This fix requires to update the Amphora image.