From 0a8254e04e6c7536564e0497ef6ebe7ce2b3f1ec Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Wed, 21 Oct 2020 18:40:46 -0700 Subject: [PATCH] Fix provider driver quota handling When provider drivers registered a load balancer object delete, the driver agent was not decrementing the project quota. This patch corrects that by decrementing the proper quota when a DELETED status is received from the provider driver. Change-Id: I7d705c9f4f0217c6fbe332f45b15892bf1d4a90b Story: 2008268 Task: 41133 (cherry picked from commit fc8ee42dfccfd4b0735185896c30b0899daa4c55) --- .../drivers/driver_agent/driver_updater.py | 43 ++++++++++-- .../driver_agent/test_driver_updater.py | 66 ++++++++++++++++++- ...gent-decrement-quota-27486d9fa0bdeb89.yaml | 5 ++ 3 files changed, 107 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/fix-driver-agent-decrement-quota-27486d9fa0bdeb89.yaml diff --git a/octavia/api/drivers/driver_agent/driver_updater.py b/octavia/api/drivers/driver_agent/driver_updater.py index 0d8521b907..912e392a7e 100644 --- a/octavia/api/drivers/driver_agent/driver_updater.py +++ b/octavia/api/drivers/driver_agent/driver_updater.py @@ -16,6 +16,8 @@ import time from octavia_lib.api.drivers import exceptions as driver_exceptions from octavia_lib.common import constants as lib_consts +from oslo_log import log as logging +from oslo_utils import excutils from octavia.common import constants as consts from octavia.common import data_models @@ -24,10 +26,13 @@ from octavia.db import api as db_apis from octavia.db import repositories as repo from octavia.statistics import stats_base +LOG = logging.getLogger(__name__) + class DriverUpdater(object): def __init__(self, **kwargs): + self.repos = repo.Repositories() self.loadbalancer_repo = repo.LoadBalancerRepository() self.listener_repo = repo.ListenerRepository() self.pool_repo = repo.PoolRepository() @@ -50,6 +55,28 @@ class DriverUpdater(object): network_driver = utils.get_network_driver() network_driver.deallocate_vip(vip) + def _decrement_quota(self, repo, object_name, record_id): + lock_session = db_apis.get_session(autocommit=False) + db_object = repo.get(lock_session, id=record_id) + try: + if db_object.provisioning_status == consts.DELETED: + LOG.info('%(name)s with ID of %(id)s is already in the ' + 'DELETED state. Skipping quota update.', + {'name': object_name, 'id': record_id}) + lock_session.rollback() + return + self.repos.decrement_quota(lock_session, + repo.model_class.__data_model__, + db_object.project_id) + lock_session.commit() + except Exception: + with excutils.save_and_reraise_exception(): + LOG.error('Failed to decrement %(name)s quota for ' + 'project: %(proj)s the project may have excess ' + 'quota in use.', {'proj': db_object.project_id, + 'name': object_name}) + lock_session.rollback() + def _process_status_update(self, repo, object_name, record, delete_record=False): # Zero it out so that if the ID is missing from a record we do not @@ -60,12 +87,16 @@ class DriverUpdater(object): record_kwargs = {} prov_status = record.get(consts.PROVISIONING_STATUS, None) if prov_status: - if (prov_status == consts.DELETED and - object_name == consts.LOADBALANCERS): - self._check_for_lb_vip_deallocate(repo, record_id) - elif prov_status == consts.DELETED and delete_record: - repo.delete(self.db_session, id=record_id) - return + if prov_status == consts.DELETED: + if object_name == consts.LOADBALANCERS: + self._check_for_lb_vip_deallocate(repo, record_id) + + self._decrement_quota(repo, object_name, record_id) + + if delete_record and object_name != consts.LOADBALANCERS: + repo.delete(self.db_session, id=record_id) + return + record_kwargs[consts.PROVISIONING_STATUS] = prov_status op_status = record.get(consts.OPERATING_STATUS, None) if op_status: diff --git a/octavia/tests/unit/api/drivers/driver_agent/test_driver_updater.py b/octavia/tests/unit/api/drivers/driver_agent/test_driver_updater.py index 986dbf3b88..5645a3d441 100644 --- a/octavia/tests/unit/api/drivers/driver_agent/test_driver_updater.py +++ b/octavia/tests/unit/api/drivers/driver_agent/test_driver_updater.py @@ -17,9 +17,11 @@ from unittest.mock import call from octavia_lib.api.drivers import exceptions as driver_exceptions from octavia_lib.common import constants as lib_consts +from oslo_utils import uuidutils from octavia.api.drivers.driver_agent import driver_updater from octavia.common import data_models +from octavia.common import exceptions import octavia.tests.unit.base as base @@ -64,6 +66,15 @@ class TestDriverUpdater(base.TestCase): self.driver_updater = driver_updater.DriverUpdater() self.ref_ok_response = {lib_consts.STATUS_CODE: lib_consts.DRVR_STATUS_CODE_OK} + mock_lb = mock.MagicMock() + self.lb_id = uuidutils.generate_uuid() + self.lb_project_id = uuidutils.generate_uuid() + mock_lb.id = self.lb_id + mock_lb.project_id = self.lb_project_id + mock_lb.provisioning_status = lib_consts.ACTIVE + self.lb_data_model = 'FakeLBDataModel' + self.mock_lb_repo.model_class.__data_model__ = self.lb_data_model + self.mock_lb_repo.get.return_value = mock_lb @mock.patch('octavia.common.utils.get_network_driver') def test_check_for_lb_vip_deallocate(self, mock_get_net_drvr): @@ -84,9 +95,56 @@ class TestDriverUpdater(base.TestCase): self.driver_updater._check_for_lb_vip_deallocate(mock_repo, 'bogus_id') mock_net_drvr.deallocate_vip.assert_not_called() + @mock.patch('octavia.db.repositories.Repositories.decrement_quota') + @mock.patch('octavia.db.api.get_session') + def test_decrement_quota(self, mock_get_session, mock_dec_quota): + mock_session = mock.MagicMock() + mock_get_session.return_value = mock_session + mock_dec_quota.side_effect = [mock.DEFAULT, + exceptions.OctaviaException('Boom')] + + self.driver_updater._decrement_quota(self.mock_lb_repo, + 'FakeName', self.lb_id) + mock_dec_quota.assert_called_once_with( + mock_session, self.mock_lb_repo.model_class.__data_model__, + self.lb_project_id) + mock_session.commit.assert_called_once() + mock_session.rollback.assert_not_called() + + # Test exception path + mock_dec_quota.reset_mock() + mock_session.reset_mock() + self.assertRaises(exceptions.OctaviaException, + self.driver_updater._decrement_quota, + self.mock_lb_repo, 'FakeName', self.lb_id) + mock_dec_quota.assert_called_once_with( + mock_session, self.mock_lb_repo.model_class.__data_model__, + self.lb_project_id) + mock_session.commit.assert_not_called() + mock_session.rollback.assert_called_once() + + # Test already deleted path + mock_dec_quota.reset_mock() + mock_session.reset_mock() + # Create a local mock LB and LB_repo for this test + mock_lb = mock.MagicMock() + mock_lb.id = self.lb_id + mock_lb.provisioning_status = lib_consts.DELETED + mock_lb_repo = mock.MagicMock() + mock_lb_repo.model_class.__data_model__ = self.lb_data_model + mock_lb_repo.get.return_value = mock_lb + self.driver_updater._decrement_quota(mock_lb_repo, + 'FakeName', self.lb_id) + mock_dec_quota.assert_not_called() + mock_session.commit.assert_not_called() + mock_session.rollback.assert_called_once() + + @mock.patch('octavia.api.drivers.driver_agent.driver_updater.' + 'DriverUpdater._decrement_quota') @mock.patch('octavia.api.drivers.driver_agent.driver_updater.' 'DriverUpdater._check_for_lb_vip_deallocate') - def test_process_status_update(self, mock_deallocate): + def test_process_status_update(self, mock_deallocate, + mock_decrement_quota): mock_repo = mock.MagicMock() list_dict = {"id": 2, lib_consts.PROVISIONING_STATUS: lib_consts.ACTIVE, @@ -131,6 +189,7 @@ class TestDriverUpdater(base.TestCase): self.mock_session, 2, provisioning_status=lib_consts.DELETED, operating_status=lib_consts.ONLINE) mock_repo.delete.assert_not_called() + mock_decrement_quota.assert_called_once_with(mock_repo, 'FakeName', 2) # Test with an empty update mock_repo.reset_mock() @@ -140,17 +199,22 @@ class TestDriverUpdater(base.TestCase): mock_repo.delete.assert_not_called() # Test with deleted and delete_record True + mock_decrement_quota.reset_mock() mock_repo.reset_mock() self.driver_updater._process_status_update( mock_repo, 'FakeName', list_deleted_dict, delete_record=True) mock_repo.delete.assert_called_once_with(self.mock_session, id=2) mock_repo.update.assert_not_called() + mock_decrement_quota.assert_called_once_with(mock_repo, 'FakeName', 2) # Test with LB Delete + mock_decrement_quota.reset_mock() mock_repo.reset_mock() self.driver_updater._process_status_update( mock_repo, lib_consts.LOADBALANCERS, list_deleted_dict) mock_deallocate.assert_called_once_with(mock_repo, 2) + mock_decrement_quota.assert_called_once_with( + mock_repo, lib_consts.LOADBALANCERS, 2) # Test with an exception mock_repo.reset_mock() diff --git a/releasenotes/notes/fix-driver-agent-decrement-quota-27486d9fa0bdeb89.yaml b/releasenotes/notes/fix-driver-agent-decrement-quota-27486d9fa0bdeb89.yaml new file mode 100644 index 0000000000..f6b67f5fba --- /dev/null +++ b/releasenotes/notes/fix-driver-agent-decrement-quota-27486d9fa0bdeb89.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes an issue where provider drivers may not decrement the load + balancer objects quota on delete.