Fix base (VRRP) port abandoned on revert

If the base (VRRP) port fails to attach to the amphora instance, the
port would not be deleted as part of the revert cleanup.
This patch splits the two plug VIP phases and attempts to clean up the
base (VRRP) port, that was created in the first phase, should the port
attach fail.

Change-Id: Ieab13c1152fed64da7390891b315b5e67513ce3e
Story: 2006468
Task: 36399

octavia/network/drivers/neutron/allowed_address_pairs.py

@@ -96,6 +96,13 @@ class AllowedAddressPairsDriver(neutron_base.BaseNeutronDriver):
             LOG.debug('Created vip port: %(port_id)s for amphora: %(amp)s',
                       {'port_id': new_port.id, 'amp': amphora.id})
 
+        except Exception:
+            message = _('Error creating the base (VRRP) port for the VIP with '
+                        'port details: {}').format(port)
+            LOG.exception(message)
+            raise base.PlugVIPException(message)
+
+        try:
             interface = self.plug_port(amphora, new_port)
         except Exception:
             message = _('Error plugging amphora (compute_id: {compute_id}) '
@@ -103,6 +110,16 @@ class AllowedAddressPairsDriver(neutron_base.BaseNeutronDriver):
                             compute_id=amphora.compute_id,
                             network_id=subnet.network_id)
             LOG.exception(message)
+            try:
+                if new_port:
+                    self.neutron_client.delete_port(new_port.id)
+                    LOG.debug('Deleted base (VRRP) port %s due to plug_port '
+                              'failure.', new_port.id)
+            except Exception:
+                LOG.exception('Failed to delete base (VRRP) port %s after '
+                              'plug_port failed. This resource is being '
+                              'abandoned and should be manually deleted when '
+                              'neutron is functional.', new_port.id)
             raise base.PlugVIPException(message)
         return interface
 

octavia/tests/unit/network/drivers/neutron/test_allowed_address_pairs.py

@@ -373,6 +373,43 @@ class TestAllowedAddressPairsDriver(base.TestCase):
                                     t_constants.MOCK_VRRP_IP2])
         self.assertEqual(lb.vip.ip_address, amp.ha_ip)
 
+    @mock.patch('octavia.network.drivers.neutron.utils.'
+                'convert_port_dict_to_model')
+    def test_plug_aap_port_create_fails(self, mock_convert):
+        lb = dmh.generate_load_balancer_tree()
+
+        subnet = network_models.Subnet(id=t_constants.MOCK_VIP_SUBNET_ID,
+                                       network_id=t_constants.MOCK_VIP_NET_ID)
+
+        list_ports = self.driver.neutron_client.list_ports
+        port1 = t_constants.MOCK_MANAGEMENT_PORT1['port']
+        port2 = t_constants.MOCK_MANAGEMENT_PORT2['port']
+        list_ports.side_effect = [{'ports': [port1]}, {'ports': [port2]}]
+        port_create = self.driver.neutron_client.create_port
+        port_create.side_effect = [Exception('Create failure')]
+        self.assertRaises(network_base.PlugVIPException,
+                          self.driver.plug_aap_port,
+                          lb, lb.vip, lb.amphorae[0], subnet)
+        mock_convert.assert_not_called()
+        self.driver.neutron_client.delete_port.assert_not_called()
+
+    def test_plug_aap_port_attach_fails(self):
+        lb = dmh.generate_load_balancer_tree()
+
+        subnet = network_models.Subnet(id=t_constants.MOCK_VIP_SUBNET_ID,
+                                       network_id=t_constants.MOCK_VIP_NET_ID)
+
+        list_ports = self.driver.neutron_client.list_ports
+        port1 = t_constants.MOCK_MANAGEMENT_PORT1['port']
+        port2 = t_constants.MOCK_MANAGEMENT_PORT2['port']
+        list_ports.side_effect = [{'ports': [port1]}, {'ports': [port2]}]
+        network_attach = self.driver.compute.attach_network_or_port
+        network_attach.side_effect = [Exception('Attach failure')]
+        self.assertRaises(network_base.PlugVIPException,
+                          self.driver.plug_aap_port,
+                          lb, lb.vip, lb.amphorae[0], subnet)
+        self.driver.neutron_client.delete_port.assert_called_once()
+
     def _set_safely(self, obj, name, value):
         if isinstance(obj, dict):
             current = obj.get(name)

releasenotes/notes/Fix-plug-vip-revert-abandoned-vrrp-port-efff14edce62ad75.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixes an issue where, if we were unable to attach the base (VRRP) port to
+    an amphora instance, the revert would not clean up the port in neutron.