From dcce548ed6a85b0a17dbcba78a14e0166dd282cd 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. Conflicts: octavia/tests/unit/api/drivers/driver_agent/test_driver_updater.py Change-Id: I7d705c9f4f0217c6fbe332f45b15892bf1d4a90b Story: 2008268 Task: 41133 (cherry picked from commit fc8ee42dfccfd4b0735185896c30b0899daa4c55) (cherry picked from commit 0a8254e04e6c7536564e0497ef6ebe7ce2b3f1ec) --- .../drivers/driver_agent/driver_updater.py | 43 +++++++++-- .../driver_agent/test_driver_updater.py | 71 ++++++++++++++++++- ...gent-decrement-quota-27486d9fa0bdeb89.yaml | 5 ++ 3 files changed, 110 insertions(+), 9 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 c672a2a618..7cf290c571 100644 --- a/octavia/api/drivers/driver_agent/driver_updater.py +++ b/octavia/api/drivers/driver_agent/driver_updater.py @@ -14,16 +14,21 @@ 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 utils from octavia.db import api as db_apis from octavia.db import repositories as repo +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() @@ -46,6 +51,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 @@ -56,12 +83,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 46c9a27298..dbcc188cc4 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 @@ -16,10 +16,13 @@ from unittest import mock from mock import call -from octavia.api.drivers.driver_agent import driver_updater -import octavia.tests.unit.base as base 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 exceptions +import octavia.tests.unit.base as base class TestDriverUpdater(base.TestCase): @@ -63,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): @@ -83,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, @@ -130,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() @@ -139,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.