From f7c776fdd96c7857506737e4c150413244368850 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Mon, 21 Mar 2016 19:43:48 +0000 Subject: [PATCH] Fixes failover when using a spares pool The failover flow was not plugging the ports back into the amphora if the failover used an amphora from the spares pool. This patch adds a task to plug the ports back into the amphora during failover Change-Id: Id7f0e60650ca2b35afb2695181897674abb9d8cf Closes-Bug: #1558934 --- .../controller/worker/flows/amphora_flows.py | 2 + .../controller/worker/tasks/network_tasks.py | 11 ++++ octavia/network/base.py | 11 ++++ .../drivers/neutron/allowed_address_pairs.py | 36 ++++++++++- octavia/network/drivers/noop_driver/driver.py | 9 +++ .../worker/tasks/test_network_tasks.py | 11 ++++ .../neutron/test_allowed_address_pairs.py | 61 ++++++++++++++++++- .../drivers/test_network_noop_driver.py | 10 +++ 8 files changed, 148 insertions(+), 3 deletions(-) diff --git a/octavia/controller/worker/flows/amphora_flows.py b/octavia/controller/worker/flows/amphora_flows.py index cf85469ff1..c029320c7a 100644 --- a/octavia/controller/worker/flows/amphora_flows.py +++ b/octavia/controller/worker/flows/amphora_flows.py @@ -341,6 +341,8 @@ class AmphoraFlows(object): requires=constants.LOADBALANCER, provides=constants.VIP)) failover_amphora_flow.add(amphora_driver_tasks.ListenersUpdate( requires=(constants.LOADBALANCER, constants.LISTENERS))) + failover_amphora_flow.add(network_tasks.PlugPorts( + requires=(constants.AMPHORA, constants.PORTS))) failover_amphora_flow.add(amphora_driver_tasks.AmphoraPostVIPPlug( requires=(constants.LOADBALANCER, constants.AMPHORAE_NETWORK_CONFIG))) diff --git a/octavia/controller/worker/tasks/network_tasks.py b/octavia/controller/worker/tasks/network_tasks.py index 3bec6da622..b9930c460e 100644 --- a/octavia/controller/worker/tasks/network_tasks.py +++ b/octavia/controller/worker/tasks/network_tasks.py @@ -416,3 +416,14 @@ class RetrievePortIDsOnAmphoraExceptLBNetwork(BaseNetworkTask): ports.append(port) return ports + + +class PlugPorts(BaseNetworkTask): + """Task to plug neutron ports into a compute instance.""" + + def execute(self, amphora, ports): + for port in ports: + LOG.debug('Plugging port ID: {port_id} into compute instance: ' + '{compute_id}.'.format(port_id=port.id, + compute_id=amphora.compute_id)) + self.network_driver.plug_port(amphora, port) diff --git a/octavia/network/base.py b/octavia/network/base.py index aea411ca34..e0907ae62c 100644 --- a/octavia/network/base.py +++ b/octavia/network/base.py @@ -224,3 +224,14 @@ class AbstractNetworkDriver(object): :raises: PortNotFound """ pass + + @abc.abstractmethod + def plug_port(self, compute_id, port): + """Plug a neutron port in to a compute instance + + :param compute_id: id of an amphora in the compute service + :param port: port to plug into the compute instance + :return: None + :raises: PlugNetworkException, AmphoraNotFound, NetworkNotFound + """ + pass diff --git a/octavia/network/drivers/neutron/allowed_address_pairs.py b/octavia/network/drivers/neutron/allowed_address_pairs.py index 7b2d087640..791772f2c4 100644 --- a/octavia/network/drivers/neutron/allowed_address_pairs.py +++ b/octavia/network/drivers/neutron/allowed_address_pairs.py @@ -431,9 +431,43 @@ class AllowedAddressPairsDriver(neutron_base.BaseNeutronDriver): ports.append(port) for port in ports: + try: + self.nova_client.servers.interface_detach( + server=amphora.compute_id, port=port.id) + except Exception as e: + LOG.warning(_LW('Failed to detach port %(portid)s from ' + 'instance %(compid)s. Error: ' + '%(error)s') % {'portid': port.id, + 'compid': amphora.compute_id, + 'error': str(e)}) try: self.neutron_client.update_port(port.id, - {'port': {'device_id': ''}}) + {'port': {'device_id': '', + 'dns_name': ''}}) except (neutron_client_exceptions.NotFound, neutron_client_exceptions.PortNotFoundClient): raise base.PortNotFound() + + def plug_port(self, amphora, port): + try: + self.nova_client.servers.interface_attach( + server=amphora.compute_id, net_id=None, + fixed_ip=None, port_id=port.id) + except nova_client_exceptions.NotFound as e: + if 'Instance' in e.message: + raise base.AmphoraNotFound(e.message) + elif 'Network' in e.message: + raise base.NetworkNotFound(e.message) + else: + raise base.PlugNetworkException(e.message) + except nova_client_exceptions.Conflict: + LOG.info(_LI('Port %(portid)s is already plugged, ' + 'skipping') % {'portid': port.id}) + except Exception: + message = _LE('Error plugging amphora (compute_id: ' + '{compute_id}) into port ' + '{port_id}.').format( + compute_id=amphora.compute_id, + port_id=port.id) + LOG.exception(message) + raise base.PlugNetworkException(message) diff --git a/octavia/network/drivers/noop_driver/driver.py b/octavia/network/drivers/noop_driver/driver.py index c5f2335b86..e4f012a716 100644 --- a/octavia/network/drivers/noop_driver/driver.py +++ b/octavia/network/drivers/noop_driver/driver.py @@ -125,6 +125,12 @@ class NoopManager(object): LOG.debug("failover %s no-op, failover_preparation, amphora id %s", self.__class__.__name__, amphora.id) + def plug_port(self, compute_id, port): + LOG.debug("Network %s no-op, plug_port compute_id %s, port_id " + "%s", self.__class__.__name__, compute_id, port.id) + self.networkconfigconfig[(compute_id, port)] = ( + compute_id, port, 'plug_port') + class NoopNetworkDriver(driver_base.AbstractNetworkDriver): def __init__(self): @@ -167,3 +173,6 @@ class NoopNetworkDriver(driver_base.AbstractNetworkDriver): def failover_preparation(self, amphora): self.driver.failover_preparation(amphora) + + def plug_port(self, compute_id, port): + return self.driver.plug_port(compute_id, port) diff --git a/octavia/tests/unit/controller/worker/tasks/test_network_tasks.py b/octavia/tests/unit/controller/worker/tasks/test_network_tasks.py index 27b342facb..6242898af5 100644 --- a/octavia/tests/unit/controller/worker/tasks/test_network_tasks.py +++ b/octavia/tests/unit/controller/worker/tasks/test_network_tasks.py @@ -482,3 +482,14 @@ class TestNetworkTasks(base.TestCase): mock_driver.get_port.return_value = port_mock ports = net_task.execute(amphora) self.assertEqual(1, len(ports)) + + def test_plug_ports(self, mock_driver): + amphora = mock.MagicMock() + port1 = mock.MagicMock() + port2 = mock.MagicMock() + + plugports = network_tasks.PlugPorts() + plugports.execute(amphora, [port1, port2]) + + mock_driver.plug_port.assert_any_call(amphora, port1) + mock_driver.plug_port.assert_any_call(amphora, port2) diff --git a/octavia/tests/unit/network/drivers/neutron/test_allowed_address_pairs.py b/octavia/tests/unit/network/drivers/neutron/test_allowed_address_pairs.py index 8e9059cabb..71346b035f 100644 --- a/octavia/tests/unit/network/drivers/neutron/test_allowed_address_pairs.py +++ b/octavia/tests/unit/network/drivers/neutron/test_allowed_address_pairs.py @@ -17,6 +17,7 @@ import copy import mock from neutronclient.common import exceptions as neutron_exceptions from novaclient.client import exceptions as nova_exceptions +from oslo_utils import uuidutils from octavia.common import clients from octavia.common import constants @@ -50,6 +51,7 @@ class TestAllowedAddressPairsDriver(base.TestCase): LB_NET_PORT_ID = "6" HA_PORT_ID = "8" HA_IP = "12.0.0.2" + PORT_ID = uuidutils.generate_uuid() def setUp(self): super(TestAllowedAddressPairsDriver, self).setUp() @@ -551,14 +553,22 @@ class TestAllowedAddressPairsDriver(base.TestCase): self.driver.neutron_client.show_port = mock.Mock( side_effect=self._failover_show_port_side_effect) port_update = self.driver.neutron_client.update_port + interface_detach = self.driver.nova_client.servers.interface_detach amphora = data_models.Amphora( id=self.AMPHORA_ID, load_balancer_id=self.LB_ID, compute_id=self.COMPUTE_ID, status=self.ACTIVE, lb_network_ip=self.LB_NET_IP, ha_port_id=self.HA_PORT_ID, ha_ip=self.HA_IP) self.driver.failover_preparation(amphora) - port_update.assert_called_once_with(ports["ports"][1].get("id"), - {'port': {'device_id': ''}}) + port_update.assert_called_once_with(ports['ports'][1].get('id'), + {'port': {'dns_name': '', + 'device_id': ''}}) + interface_detach.assert_called_once_with( + server=amphora.compute_id, port=ports['ports'][1].get('id')) + + # Test that the detach exception is caught and not raised + interface_detach.side_effect = Exception + self.driver.failover_preparation(amphora) def _failover_show_port_side_effect(self, port_id): if port_id == self.LB_NET_PORT_ID: @@ -569,3 +579,50 @@ class TestAllowedAddressPairsDriver(base.TestCase): return {"fixed_ips": [{"subnet_id": self.SUBNET_ID_2, "ip_address": self.IP_ADDRESS_2}], "id": self.FIXED_IP_ID_2, "network_id": self.NETWORK_ID_2} + + def test_plug_port(self): + port = mock.MagicMock() + port.id = self.PORT_ID + interface_attach = self.driver.nova_client.servers.interface_attach + amphora = data_models.Amphora( + id=self.AMPHORA_ID, load_balancer_id=self.LB_ID, + compute_id=self.COMPUTE_ID, status=self.ACTIVE, + lb_network_ip=self.LB_NET_IP, ha_port_id=self.HA_PORT_ID, + ha_ip=self.HA_IP) + + self.driver.plug_port(amphora, port) + interface_attach.assert_called_once_with(server=amphora.compute_id, + net_id=None, + fixed_ip=None, + port_id=self.PORT_ID) + + # NotFound cases + interface_attach.side_effect = nova_exceptions.NotFound( + 1, message='Instance') + self.assertRaises(network_base.AmphoraNotFound, + self.driver.plug_port, + amphora, + port) + interface_attach.side_effect = nova_exceptions.NotFound( + 1, message='Network') + self.assertRaises(network_base.NetworkNotFound, + self.driver.plug_port, + amphora, + port) + interface_attach.side_effect = nova_exceptions.NotFound( + 1, message='bogus') + self.assertRaises(network_base.PlugNetworkException, + self.driver.plug_port, + amphora, + port) + + # Already plugged case should not raise an exception + interface_attach.side_effect = nova_exceptions.Conflict(1) + self.driver.plug_port(amphora, port) + + # Unknown error case + interface_attach.side_effect = TypeError + self.assertRaises(network_base.PlugNetworkException, + self.driver.plug_port, + amphora, + port) diff --git a/octavia/tests/unit/network/drivers/test_network_noop_driver.py b/octavia/tests/unit/network/drivers/test_network_noop_driver.py index f82e3cd0a2..deb6c9a909 100644 --- a/octavia/tests/unit/network/drivers/test_network_noop_driver.py +++ b/octavia/tests/unit/network/drivers/test_network_noop_driver.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +import mock from oslo_utils import uuidutils from octavia.db import models as models @@ -27,7 +28,9 @@ class TestNoopNetworkDriver(base.TestCase): def setUp(self): super(TestNoopNetworkDriver, self).setUp() self.driver = driver.NoopNetworkDriver() + self.port = mock.MagicMock() self.port_id = 88 + self.port.id = self.port_id self.network_id = self.FAKE_UUID_3 self.ip_address = "10.0.0.2" self.load_balancer = models.LoadBalancer() @@ -117,3 +120,10 @@ class TestNoopNetworkDriver(base.TestCase): (self.port_id, 'get_port'), self.driver.driver.networkconfigconfig[self.port_id] ) + + def test_plug_port(self): + self.driver.plug_port(self.compute_id, self.port) + self.assertEqual( + (self.compute_id, self.port, 'plug_port'), + self.driver.driver.networkconfigconfig[self.compute_id, self.port] + )