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 commitfc8ee42dfc
) (cherry picked from commit0a8254e04e
)
This commit is contained in:
parent
198c7d9658
commit
dcce548ed6
|
@ -14,16 +14,21 @@
|
||||||
|
|
||||||
from octavia_lib.api.drivers import exceptions as driver_exceptions
|
from octavia_lib.api.drivers import exceptions as driver_exceptions
|
||||||
from octavia_lib.common import constants as lib_consts
|
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 constants as consts
|
||||||
from octavia.common import utils
|
from octavia.common import utils
|
||||||
from octavia.db import api as db_apis
|
from octavia.db import api as db_apis
|
||||||
from octavia.db import repositories as repo
|
from octavia.db import repositories as repo
|
||||||
|
|
||||||
|
LOG = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
class DriverUpdater(object):
|
class DriverUpdater(object):
|
||||||
|
|
||||||
def __init__(self, **kwargs):
|
def __init__(self, **kwargs):
|
||||||
|
self.repos = repo.Repositories()
|
||||||
self.loadbalancer_repo = repo.LoadBalancerRepository()
|
self.loadbalancer_repo = repo.LoadBalancerRepository()
|
||||||
self.listener_repo = repo.ListenerRepository()
|
self.listener_repo = repo.ListenerRepository()
|
||||||
self.pool_repo = repo.PoolRepository()
|
self.pool_repo = repo.PoolRepository()
|
||||||
|
@ -46,6 +51,28 @@ class DriverUpdater(object):
|
||||||
network_driver = utils.get_network_driver()
|
network_driver = utils.get_network_driver()
|
||||||
network_driver.deallocate_vip(vip)
|
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,
|
def _process_status_update(self, repo, object_name, record,
|
||||||
delete_record=False):
|
delete_record=False):
|
||||||
# Zero it out so that if the ID is missing from a record we do not
|
# 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 = {}
|
record_kwargs = {}
|
||||||
prov_status = record.get(consts.PROVISIONING_STATUS, None)
|
prov_status = record.get(consts.PROVISIONING_STATUS, None)
|
||||||
if prov_status:
|
if prov_status:
|
||||||
if (prov_status == consts.DELETED and
|
if prov_status == consts.DELETED:
|
||||||
object_name == consts.LOADBALANCERS):
|
if object_name == consts.LOADBALANCERS:
|
||||||
self._check_for_lb_vip_deallocate(repo, record_id)
|
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)
|
self._decrement_quota(repo, object_name, record_id)
|
||||||
return
|
|
||||||
|
if delete_record and object_name != consts.LOADBALANCERS:
|
||||||
|
repo.delete(self.db_session, id=record_id)
|
||||||
|
return
|
||||||
|
|
||||||
record_kwargs[consts.PROVISIONING_STATUS] = prov_status
|
record_kwargs[consts.PROVISIONING_STATUS] = prov_status
|
||||||
op_status = record.get(consts.OPERATING_STATUS, None)
|
op_status = record.get(consts.OPERATING_STATUS, None)
|
||||||
if op_status:
|
if op_status:
|
||||||
|
|
|
@ -16,10 +16,13 @@ from unittest import mock
|
||||||
|
|
||||||
from mock import call
|
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.api.drivers import exceptions as driver_exceptions
|
||||||
from octavia_lib.common import constants as lib_consts
|
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):
|
class TestDriverUpdater(base.TestCase):
|
||||||
|
@ -63,6 +66,15 @@ class TestDriverUpdater(base.TestCase):
|
||||||
self.driver_updater = driver_updater.DriverUpdater()
|
self.driver_updater = driver_updater.DriverUpdater()
|
||||||
self.ref_ok_response = {lib_consts.STATUS_CODE:
|
self.ref_ok_response = {lib_consts.STATUS_CODE:
|
||||||
lib_consts.DRVR_STATUS_CODE_OK}
|
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')
|
@mock.patch('octavia.common.utils.get_network_driver')
|
||||||
def test_check_for_lb_vip_deallocate(self, mock_get_net_drvr):
|
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')
|
self.driver_updater._check_for_lb_vip_deallocate(mock_repo, 'bogus_id')
|
||||||
mock_net_drvr.deallocate_vip.assert_not_called()
|
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.'
|
@mock.patch('octavia.api.drivers.driver_agent.driver_updater.'
|
||||||
'DriverUpdater._check_for_lb_vip_deallocate')
|
'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()
|
mock_repo = mock.MagicMock()
|
||||||
list_dict = {"id": 2,
|
list_dict = {"id": 2,
|
||||||
lib_consts.PROVISIONING_STATUS: lib_consts.ACTIVE,
|
lib_consts.PROVISIONING_STATUS: lib_consts.ACTIVE,
|
||||||
|
@ -130,6 +189,7 @@ class TestDriverUpdater(base.TestCase):
|
||||||
self.mock_session, 2, provisioning_status=lib_consts.DELETED,
|
self.mock_session, 2, provisioning_status=lib_consts.DELETED,
|
||||||
operating_status=lib_consts.ONLINE)
|
operating_status=lib_consts.ONLINE)
|
||||||
mock_repo.delete.assert_not_called()
|
mock_repo.delete.assert_not_called()
|
||||||
|
mock_decrement_quota.assert_called_once_with(mock_repo, 'FakeName', 2)
|
||||||
|
|
||||||
# Test with an empty update
|
# Test with an empty update
|
||||||
mock_repo.reset_mock()
|
mock_repo.reset_mock()
|
||||||
|
@ -139,17 +199,22 @@ class TestDriverUpdater(base.TestCase):
|
||||||
mock_repo.delete.assert_not_called()
|
mock_repo.delete.assert_not_called()
|
||||||
|
|
||||||
# Test with deleted and delete_record True
|
# Test with deleted and delete_record True
|
||||||
|
mock_decrement_quota.reset_mock()
|
||||||
mock_repo.reset_mock()
|
mock_repo.reset_mock()
|
||||||
self.driver_updater._process_status_update(
|
self.driver_updater._process_status_update(
|
||||||
mock_repo, 'FakeName', list_deleted_dict, delete_record=True)
|
mock_repo, 'FakeName', list_deleted_dict, delete_record=True)
|
||||||
mock_repo.delete.assert_called_once_with(self.mock_session, id=2)
|
mock_repo.delete.assert_called_once_with(self.mock_session, id=2)
|
||||||
mock_repo.update.assert_not_called()
|
mock_repo.update.assert_not_called()
|
||||||
|
mock_decrement_quota.assert_called_once_with(mock_repo, 'FakeName', 2)
|
||||||
|
|
||||||
# Test with LB Delete
|
# Test with LB Delete
|
||||||
|
mock_decrement_quota.reset_mock()
|
||||||
mock_repo.reset_mock()
|
mock_repo.reset_mock()
|
||||||
self.driver_updater._process_status_update(
|
self.driver_updater._process_status_update(
|
||||||
mock_repo, lib_consts.LOADBALANCERS, list_deleted_dict)
|
mock_repo, lib_consts.LOADBALANCERS, list_deleted_dict)
|
||||||
mock_deallocate.assert_called_once_with(mock_repo, 2)
|
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
|
# Test with an exception
|
||||||
mock_repo.reset_mock()
|
mock_repo.reset_mock()
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Fixes an issue where provider drivers may not decrement the load
|
||||||
|
balancer objects quota on delete.
|
Loading…
Reference in New Issue