From 2c76209003d5ca09a302881a40a35fd14762db55 Mon Sep 17 00:00:00 2001 From: Adam Harwell Date: Wed, 29 Jan 2020 22:49:52 -0800 Subject: [PATCH] Select the right lb_network_ip interface using AZ If the AZ system is being used and there is also a default boot network set in the main config, the nova driver would not pick the right interface when translating the amphora info for amps in custom AZs. This affects the step that fetches the lb_network_ip (the management address) from nova, immediately before the step that updates the database with the correct IP. That IP is then used in the rest of the amphora creation flow, which pulls in that address and uses it to connect to the amp. Change-Id: Ie4869035a557ebcddea2fce693067c82fbd2d2a9 --- octavia/compute/compute_base.py | 5 ++-- octavia/compute/drivers/noop_driver/driver.py | 13 +++++----- octavia/compute/drivers/nova_driver.py | 22 +++++++++------- .../worker/v1/flows/amphora_flows.py | 3 ++- .../worker/v1/tasks/compute_tasks.py | 8 ++++-- .../worker/v2/flows/amphora_flows.py | 3 ++- .../worker/v2/tasks/compute_tasks.py | 16 ++++++++++-- .../drivers/test_compute_noop_driver.py | 10 ++++--- .../worker/v1/tasks/test_compute_tasks.py | 26 ++++++++++++------- .../worker/v2/flows/test_amphora_flows.py | 15 +++++++++-- .../worker/v2/tasks/test_compute_tasks.py | 26 ++++++++++++------- 11 files changed, 99 insertions(+), 48 deletions(-) diff --git a/octavia/compute/compute_base.py b/octavia/compute/compute_base.py index a27a8f4455..75e2689ff8 100644 --- a/octavia/compute/compute_base.py +++ b/octavia/compute/compute_base.py @@ -69,10 +69,11 @@ class ComputeBase(object): """ @abc.abstractmethod - def get_amphora(self, compute_id): + def get_amphora(self, compute_id, management_network_id=None): """Retrieve an amphora object - :param compute_id: the id of the desired amphora + :param compute_id: the compute id of the desired amphora + :param management_network_id: ID of the management network :returns: the amphora object :returns: fault message or None """ diff --git a/octavia/compute/drivers/noop_driver/driver.py b/octavia/compute/drivers/noop_driver/driver.py index 6eadf86779..015503a919 100644 --- a/octavia/compute/drivers/noop_driver/driver.py +++ b/octavia/compute/drivers/noop_driver/driver.py @@ -63,10 +63,11 @@ class NoopManager(object): self.computeconfig[compute_id] = (compute_id, 'status') return constants.UP - def get_amphora(self, compute_id): - LOG.debug("Compute %s no-op, compute_id %s", - self.__class__.__name__, compute_id) - self.computeconfig[compute_id] = (compute_id, 'get_amphora') + def get_amphora(self, compute_id, management_network_id=None): + LOG.debug("Compute %s no-op, compute_id %s, management_network_id %s", + self.__class__.__name__, compute_id, management_network_id) + self.computeconfig[(compute_id, management_network_id)] = ( + compute_id, management_network_id, 'get_amphora') return data_models.Amphora( compute_id=compute_id, status=constants.ACTIVE, @@ -143,8 +144,8 @@ class NoopComputeDriver(driver_base.ComputeBase): def status(self, compute_id): return self.driver.status(compute_id) - def get_amphora(self, compute_id): - return self.driver.get_amphora(compute_id) + def get_amphora(self, compute_id, management_network_id=None): + return self.driver.get_amphora(compute_id, management_network_id) def create_server_group(self, name, policy): return self.driver.create_server_group(name, policy) diff --git a/octavia/compute/drivers/nova_driver.py b/octavia/compute/drivers/nova_driver.py index 5f9a44269c..3383f1025d 100644 --- a/octavia/compute/drivers/nova_driver.py +++ b/octavia/compute/drivers/nova_driver.py @@ -218,10 +218,11 @@ class VirtualMachineManager(compute_base.ComputeBase): raise exceptions.ComputeStatusException() return constants.DOWN - def get_amphora(self, compute_id): + def get_amphora(self, compute_id, management_network_id=None): '''Retrieve the information in nova of a virtual machine. - :param amphora_id: virtual machine UUID + :param compute_id: virtual machine UUID + :param management_network_id: ID of the management network :returns: an amphora object :returns: fault message or None ''' @@ -231,12 +232,13 @@ class VirtualMachineManager(compute_base.ComputeBase): except Exception: LOG.exception("Error retrieving nova virtual machine.") raise exceptions.ComputeGetException() - return self._translate_amphora(amphora) + return self._translate_amphora(amphora, management_network_id) - def _translate_amphora(self, nova_response): + def _translate_amphora(self, nova_response, management_network_id=None): '''Convert a nova virtual machine into an amphora object. :param nova_response: JSON response from nova + :param management_network_id: ID of the management network :returns: an amphora object :returns: fault message or None ''' @@ -246,19 +248,19 @@ class VirtualMachineManager(compute_base.ComputeBase): lb_network_ip = None availability_zone = None image_id = None - fault = None + + if management_network_id: + boot_networks = [management_network_id] + else: + boot_networks = CONF.controller_worker.amp_boot_network_list try: inf_list = nova_response.interface_list() - no_boot_networks = ( - not CONF.controller_worker.amp_boot_network_list) for interface in inf_list: net_id = interface.net_id - is_boot_network = ( - net_id in CONF.controller_worker.amp_boot_network_list) # Pick the first fixed_ip if this is a boot network or if # there are no boot networks configured (use default network) - if is_boot_network or no_boot_networks: + if net_id in boot_networks or not boot_networks: lb_network_ip = interface.fixed_ips[0]['ip_address'] break try: diff --git a/octavia/controller/worker/v1/flows/amphora_flows.py b/octavia/controller/worker/v1/flows/amphora_flows.py index eb19bf021d..956788c617 100644 --- a/octavia/controller/worker/v1/flows/amphora_flows.py +++ b/octavia/controller/worker/v1/flows/amphora_flows.py @@ -193,7 +193,8 @@ class AmphoraFlows(object): requires=(constants.AMPHORA_ID, constants.COMPUTE_ID))) create_amp_for_lb_subflow.add(compute_tasks.ComputeActiveWait( name=sf_name + '-' + constants.COMPUTE_WAIT, - requires=(constants.COMPUTE_ID, constants.AMPHORA_ID), + requires=(constants.COMPUTE_ID, constants.AMPHORA_ID, + constants.AVAILABILITY_ZONE), provides=constants.COMPUTE_OBJ)) create_amp_for_lb_subflow.add(database_tasks.UpdateAmphoraInfo( name=sf_name + '-' + constants.UPDATE_AMPHORA_INFO, diff --git a/octavia/controller/worker/v1/tasks/compute_tasks.py b/octavia/controller/worker/v1/tasks/compute_tasks.py index cf8291accf..77a04e44b8 100644 --- a/octavia/controller/worker/v1/tasks/compute_tasks.py +++ b/octavia/controller/worker/v1/tasks/compute_tasks.py @@ -204,14 +204,18 @@ class ComputeDelete(BaseComputeTask): class ComputeActiveWait(BaseComputeTask): """Wait for the compute driver to mark the amphora active.""" - def execute(self, compute_id, amphora_id): + def execute(self, compute_id, amphora_id, availability_zone): """Wait for the compute driver to mark the amphora active :raises: Generic exception if the amphora is not active :returns: An amphora object """ + if availability_zone: + amp_network = availability_zone.get(constants.MANAGEMENT_NETWORK) + else: + amp_network = None for i in range(CONF.controller_worker.amp_active_retries): - amp, fault = self.compute.get_amphora(compute_id) + amp, fault = self.compute.get_amphora(compute_id, amp_network) if amp.status == constants.ACTIVE: if CONF.haproxy_amphora.build_rate_limit != -1: self.rate_limit.remove_from_build_req_queue(amphora_id) diff --git a/octavia/controller/worker/v2/flows/amphora_flows.py b/octavia/controller/worker/v2/flows/amphora_flows.py index 2d29ebbef0..a0746c42c8 100644 --- a/octavia/controller/worker/v2/flows/amphora_flows.py +++ b/octavia/controller/worker/v2/flows/amphora_flows.py @@ -200,7 +200,8 @@ class AmphoraFlows(object): requires=(constants.AMPHORA_ID, constants.COMPUTE_ID))) create_amp_for_lb_subflow.add(compute_tasks.ComputeActiveWait( name=sf_name + '-' + constants.COMPUTE_WAIT, - requires=(constants.COMPUTE_ID, constants.AMPHORA_ID), + requires=(constants.COMPUTE_ID, constants.AMPHORA_ID, + constants.AVAILABILITY_ZONE), provides=constants.COMPUTE_OBJ)) create_amp_for_lb_subflow.add(database_tasks.UpdateAmphoraInfo( name=sf_name + '-' + constants.UPDATE_AMPHORA_INFO, diff --git a/octavia/controller/worker/v2/tasks/compute_tasks.py b/octavia/controller/worker/v2/tasks/compute_tasks.py index bb35f1b30b..fedb404a68 100644 --- a/octavia/controller/worker/v2/tasks/compute_tasks.py +++ b/octavia/controller/worker/v2/tasks/compute_tasks.py @@ -59,6 +59,8 @@ class ComputeCreate(BaseComputeTask): availability_zone=None): """Create an amphora + :param availability_zone: availability zone metadata dictionary + :returns: an amphora """ ports = ports or [] @@ -156,6 +158,8 @@ class CertComputeCreate(ComputeCreate): availability_zone=None): """Create an amphora + :param availability_zone: availability zone metadata dictionary + :returns: an amphora """ @@ -211,14 +215,22 @@ class ComputeDelete(BaseComputeTask): class ComputeActiveWait(BaseComputeTask): """Wait for the compute driver to mark the amphora active.""" - def execute(self, compute_id, amphora_id): + def execute(self, compute_id, amphora_id, availability_zone): """Wait for the compute driver to mark the amphora active + :param compute_id: virtual machine UUID + :param amphora_id: id of the amphora object + :param availability_zone: availability zone metadata dictionary + :raises: Generic exception if the amphora is not active :returns: An amphora object """ + if availability_zone: + amp_network = availability_zone.get(constants.MANAGEMENT_NETWORK) + else: + amp_network = None for i in range(CONF.controller_worker.amp_active_retries): - amp, fault = self.compute.get_amphora(compute_id) + amp, fault = self.compute.get_amphora(compute_id, amp_network) if amp.status == constants.ACTIVE: if CONF.haproxy_amphora.build_rate_limit != -1: self.rate_limit.remove_from_build_req_queue(amphora_id) diff --git a/octavia/tests/unit/compute/drivers/test_compute_noop_driver.py b/octavia/tests/unit/compute/drivers/test_compute_noop_driver.py index 27e4cf4ddf..9efb1c9596 100644 --- a/octavia/tests/unit/compute/drivers/test_compute_noop_driver.py +++ b/octavia/tests/unit/compute/drivers/test_compute_noop_driver.py @@ -88,10 +88,12 @@ class TestNoopComputeDriver(base.TestCase): self.amphora_id]) def test_get_amphora(self): - self.driver.get_amphora(self.amphora_id) - self.assertEqual((self.amphora_id, 'get_amphora'), - self.driver.driver.computeconfig[ - self.amphora_id]) + management_network_id = uuidutils.generate_uuid() + self.driver.get_amphora(self.amphora_id, management_network_id) + self.assertEqual( + (self.amphora_id, management_network_id, 'get_amphora'), + self.driver.driver.computeconfig[ + self.amphora_id, management_network_id]) def test_create_server_group(self): self.driver.create_server_group(self.server_group_name, diff --git a/octavia/tests/unit/controller/worker/v1/tasks/test_compute_tasks.py b/octavia/tests/unit/controller/worker/v1/tasks/test_compute_tasks.py index 79f88eebd8..966d6e7b36 100644 --- a/octavia/tests/unit/controller/worker/v1/tasks/test_compute_tasks.py +++ b/octavia/tests/unit/controller/worker/v1/tasks/test_compute_tasks.py @@ -434,15 +434,23 @@ class TestComputeTasks(base.TestCase): mock_driver.get_amphora.return_value = _amphora_mock, None computewait = compute_tasks.ComputeActiveWait() - computewait.execute(COMPUTE_ID, AMPHORA_ID) - mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID) + # Test with no AZ + computewait.execute(COMPUTE_ID, AMPHORA_ID, None) + mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID, None) + # Test with AZ + mock_driver.reset_mock() + az = {constants.MANAGEMENT_NETWORK: uuidutils.generate_uuid()} + computewait.execute(COMPUTE_ID, AMPHORA_ID, az) + mock_driver.get_amphora.assert_called_once_with( + COMPUTE_ID, az[constants.MANAGEMENT_NETWORK]) + + # Test with deleted amp _amphora_mock.status = constants.DELETED - self.assertRaises(exceptions.ComputeWaitTimeoutException, computewait.execute, - _amphora_mock, AMPHORA_ID) + _amphora_mock, AMPHORA_ID, None) @mock.patch('octavia.controller.worker.amphora_rate_limit' '.AmphoraBuildRateLimit.remove_from_build_req_queue') @@ -461,15 +469,15 @@ class TestComputeTasks(base.TestCase): mock_driver.get_amphora.return_value = _amphora_mock, None computewait = compute_tasks.ComputeActiveWait() - computewait.execute(COMPUTE_ID, AMPHORA_ID) + computewait.execute(COMPUTE_ID, AMPHORA_ID, None) - mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID) + mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID, None) _amphora_mock.status = constants.ERROR self.assertRaises(exceptions.ComputeBuildException, computewait.execute, - _amphora_mock, AMPHORA_ID) + _amphora_mock, AMPHORA_ID, None) @mock.patch('octavia.controller.worker.amphora_rate_limit' '.AmphoraBuildRateLimit.remove_from_build_req_queue') @@ -486,9 +494,9 @@ class TestComputeTasks(base.TestCase): mock_driver.get_amphora.return_value = _amphora_mock, None computewait = compute_tasks.ComputeActiveWait() - computewait.execute(COMPUTE_ID, AMPHORA_ID) + computewait.execute(COMPUTE_ID, AMPHORA_ID, None) - mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID) + mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID, None) mock_remove_from_build_queue.assert_not_called() @mock.patch('stevedore.driver.DriverManager.driver') diff --git a/octavia/tests/unit/controller/worker/v2/flows/test_amphora_flows.py b/octavia/tests/unit/controller/worker/v2/flows/test_amphora_flows.py index 4d1dd572ae..e5383bb0b3 100644 --- a/octavia/tests/unit/controller/worker/v2/flows/test_amphora_flows.py +++ b/octavia/tests/unit/controller/worker/v2/flows/test_amphora_flows.py @@ -54,10 +54,15 @@ class TestAmphoraFlows(base.TestCase): self.assertIn(constants.AMPHORA, amp_flow.provides) self.assertIn(constants.AMPHORA_ID, amp_flow.provides) self.assertIn(constants.COMPUTE_ID, amp_flow.provides) + self.assertIn(constants.COMPUTE_OBJ, amp_flow.provides) self.assertIn(constants.SERVER_PEM, amp_flow.provides) + self.assertIn(constants.BUILD_TYPE_PRIORITY, amp_flow.requires) + self.assertIn(constants.FLAVOR, amp_flow.requires) + self.assertIn(constants.AVAILABILITY_ZONE, amp_flow.requires) + self.assertEqual(5, len(amp_flow.provides)) - self.assertEqual(2, len(amp_flow.requires)) + self.assertEqual(3, len(amp_flow.requires)) def test_get_create_amphora_flow_cert(self, mock_get_net_driver): self.AmpFlow = amphora_flows.AmphoraFlows() @@ -69,9 +74,15 @@ class TestAmphoraFlows(base.TestCase): self.assertIn(constants.AMPHORA, amp_flow.provides) self.assertIn(constants.AMPHORA_ID, amp_flow.provides) self.assertIn(constants.COMPUTE_ID, amp_flow.provides) + self.assertIn(constants.COMPUTE_OBJ, amp_flow.provides) + self.assertIn(constants.SERVER_PEM, amp_flow.provides) + + self.assertIn(constants.BUILD_TYPE_PRIORITY, amp_flow.requires) + self.assertIn(constants.FLAVOR, amp_flow.requires) + self.assertIn(constants.AVAILABILITY_ZONE, amp_flow.requires) self.assertEqual(5, len(amp_flow.provides)) - self.assertEqual(2, len(amp_flow.requires)) + self.assertEqual(3, len(amp_flow.requires)) def test_get_create_amphora_for_lb_flow(self, mock_get_net_driver): diff --git a/octavia/tests/unit/controller/worker/v2/tasks/test_compute_tasks.py b/octavia/tests/unit/controller/worker/v2/tasks/test_compute_tasks.py index 977abccc40..2e89cba6f6 100644 --- a/octavia/tests/unit/controller/worker/v2/tasks/test_compute_tasks.py +++ b/octavia/tests/unit/controller/worker/v2/tasks/test_compute_tasks.py @@ -447,15 +447,23 @@ class TestComputeTasks(base.TestCase): mock_driver.get_amphora.return_value = _db_amphora_mock, None computewait = compute_tasks.ComputeActiveWait() - computewait.execute(COMPUTE_ID, AMPHORA_ID) - mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID) + # Test with no AZ + computewait.execute(COMPUTE_ID, AMPHORA_ID, None) + mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID, None) + # Test with AZ + mock_driver.reset_mock() + az = {constants.MANAGEMENT_NETWORK: uuidutils.generate_uuid()} + computewait.execute(COMPUTE_ID, AMPHORA_ID, az) + mock_driver.get_amphora.assert_called_once_with( + COMPUTE_ID, az[constants.MANAGEMENT_NETWORK]) + + # Test with deleted amp _db_amphora_mock.status = constants.DELETED - self.assertRaises(exceptions.ComputeWaitTimeoutException, computewait.execute, - _db_amphora_mock, AMPHORA_ID) + _amphora_mock, AMPHORA_ID, None) @mock.patch('octavia.controller.worker.amphora_rate_limit' '.AmphoraBuildRateLimit.remove_from_build_req_queue') @@ -474,15 +482,15 @@ class TestComputeTasks(base.TestCase): mock_driver.get_amphora.return_value = _db_amphora_mock, None computewait = compute_tasks.ComputeActiveWait() - computewait.execute(COMPUTE_ID, AMPHORA_ID) + computewait.execute(COMPUTE_ID, AMPHORA_ID, None) - mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID) + mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID, None) _db_amphora_mock.status = constants.ERROR self.assertRaises(exceptions.ComputeBuildException, computewait.execute, - _db_amphora_mock, AMPHORA_ID) + _db_amphora_mock, AMPHORA_ID, None) @mock.patch('octavia.controller.worker.amphora_rate_limit' '.AmphoraBuildRateLimit.remove_from_build_req_queue') @@ -499,9 +507,9 @@ class TestComputeTasks(base.TestCase): mock_driver.get_amphora.return_value = _db_amphora_mock, None computewait = compute_tasks.ComputeActiveWait() - computewait.execute(COMPUTE_ID, AMPHORA_ID) + computewait.execute(COMPUTE_ID, AMPHORA_ID, None) - mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID) + mock_driver.get_amphora.assert_called_once_with(COMPUTE_ID, None) mock_remove_from_build_queue.assert_not_called() @mock.patch('stevedore.driver.DriverManager.driver')