Commit 86e3eb4c authored by Carlos Goncalves's avatar Carlos Goncalves

Fix setting of VIP QoS policy

Load balancers were going in to ERROR when updating vip_qos_policy_id in
two different cases:

- QoS extension enabled: the VIP DB data model was incorrectly
  constructed ('vip_qos_policy_id' where it should have been
  'qos_policy_id')
- QoS extension disabled: setting an UUID or None would fail in the LB
  update flow as the extension is disabled, and the API would return
  HTTP 202 to the user.

Story: 2004602
Task: 28512

Change-Id: Ie974afa52fe70cbab72b7e7f75bf7ee1015e148c
(cherry picked from commit e0c45ce4d288aa0a8c855754245cd3949e54fa3d)
parent c2afddfd
...@@ -95,7 +95,7 @@ class AmphoraProviderDriver(driver_base.ProviderDriver): ...@@ -95,7 +95,7 @@ class AmphoraProviderDriver(driver_base.ProviderDriver):
# expects # expects
vip_qos_policy_id = lb_dict.pop('vip_qos_policy_id', None) vip_qos_policy_id = lb_dict.pop('vip_qos_policy_id', None)
if vip_qos_policy_id: if vip_qos_policy_id:
vip_dict = {"vip_qos_policy_id": vip_qos_policy_id} vip_dict = {"qos_policy_id": vip_qos_policy_id}
lb_dict["vip"] = vip_dict lb_dict["vip"] = vip_dict
payload = {consts.LOAD_BALANCER_ID: lb_id, payload = {consts.LOAD_BALANCER_ID: lb_id,
......
...@@ -547,11 +547,12 @@ class LoadBalancersController(base.BaseController): ...@@ -547,11 +547,12 @@ class LoadBalancersController(base.BaseController):
self._auth_validate_action(context, db_lb.project_id, self._auth_validate_action(context, db_lb.project_id,
constants.RBAC_PUT) constants.RBAC_PUT)
if (load_balancer.vip_qos_policy_id and if not isinstance(load_balancer.vip_qos_policy_id, wtypes.UnsetType):
not isinstance(load_balancer.vip_qos_policy_id, network_driver = utils.get_network_driver()
wtypes.UnsetType) and validate.qos_extension_enabled(network_driver)
db_lb.vip.qos_policy_id != load_balancer.vip_qos_policy_id): if load_balancer.vip_qos_policy_id is not None:
validate.qos_policy_exists(load_balancer.vip_qos_policy_id) if db_lb.vip.qos_policy_id != load_balancer.vip_qos_policy_id:
validate.qos_policy_exists(load_balancer.vip_qos_policy_id)
# Load the driver early as it also provides validation # Load the driver early as it also provides validation
driver = driver_factory.get_driver(db_lb.provider) driver = driver_factory.get_driver(db_lb.provider)
......
...@@ -342,6 +342,7 @@ def subnet_exists(subnet_id): ...@@ -342,6 +342,7 @@ def subnet_exists(subnet_id):
def qos_policy_exists(qos_policy_id): def qos_policy_exists(qos_policy_id):
network_driver = utils.get_network_driver() network_driver = utils.get_network_driver()
qos_extension_enabled(network_driver)
try: try:
qos_policy = network_driver.get_qos_policy(qos_policy_id) qos_policy = network_driver.get_qos_policy(qos_policy_id)
except Exception: except Exception:
...@@ -350,6 +351,12 @@ def qos_policy_exists(qos_policy_id): ...@@ -350,6 +351,12 @@ def qos_policy_exists(qos_policy_id):
return qos_policy return qos_policy
def qos_extension_enabled(network_driver):
if not network_driver.qos_enabled():
raise exceptions.ValidationException(detail=_(
"VIP QoS policy is not allowed in this deployment."))
def network_exists_optionally_contains_subnet(network_id, subnet_id=None): def network_exists_optionally_contains_subnet(network_id, subnet_id=None):
"""Raises an exception when a network does not exist. """Raises an exception when a network does not exist.
......
...@@ -349,3 +349,10 @@ class AbstractNetworkDriver(object): ...@@ -349,3 +349,10 @@ class AbstractNetworkDriver(object):
:param subnet: The subnet to plug the aap into :param subnet: The subnet to plug the aap into
""" """
pass pass
@abc.abstractmethod
def qos_enabled(self):
"""Whether QoS is enabled
:return: Boolean
"""
...@@ -27,6 +27,7 @@ from octavia.network.drivers.neutron import utils ...@@ -27,6 +27,7 @@ from octavia.network.drivers.neutron import utils
LOG = logging.getLogger(__name__) LOG = logging.getLogger(__name__)
DNS_INT_EXT_ALIAS = 'dns-integration' DNS_INT_EXT_ALIAS = 'dns-integration'
SEC_GRP_EXT_ALIAS = 'security-group' SEC_GRP_EXT_ALIAS = 'security-group'
QOS_EXT_ALIAS = 'qos'
CONF = cfg.CONF CONF = cfg.CONF
...@@ -46,6 +47,7 @@ class BaseNeutronDriver(base.AbstractNetworkDriver): ...@@ -46,6 +47,7 @@ class BaseNeutronDriver(base.AbstractNetworkDriver):
self.sec_grp_enabled = self._check_extension_enabled(SEC_GRP_EXT_ALIAS) self.sec_grp_enabled = self._check_extension_enabled(SEC_GRP_EXT_ALIAS)
self.dns_integration_enabled = self._check_extension_enabled( self.dns_integration_enabled = self._check_extension_enabled(
DNS_INT_EXT_ALIAS) DNS_INT_EXT_ALIAS)
self._qos_enabled = self._check_extension_enabled(QOS_EXT_ALIAS)
self.project_id = self.neutron_client.get_auth_info().get( self.project_id = self.neutron_client.get_auth_info().get(
'auth_tenant_id') 'auth_tenant_id')
...@@ -248,3 +250,6 @@ class BaseNeutronDriver(base.AbstractNetworkDriver): ...@@ -248,3 +250,6 @@ class BaseNeutronDriver(base.AbstractNetworkDriver):
def get_qos_policy(self, qos_policy_id): def get_qos_policy(self, qos_policy_id):
return self._get_resource('qos_policy', qos_policy_id) return self._get_resource('qos_policy', qos_policy_id)
def qos_enabled(self):
return self._qos_enabled
...@@ -26,6 +26,7 @@ class NoopManager(object): ...@@ -26,6 +26,7 @@ class NoopManager(object):
def __init__(self): def __init__(self):
super(NoopManager, self).__init__() super(NoopManager, self).__init__()
self.networkconfigconfig = {} self.networkconfigconfig = {}
self._qos_extension_enabled = True
def allocate_vip(self, loadbalancer): def allocate_vip(self, loadbalancer):
LOG.debug("Network %s no-op, allocate_vip loadbalancer %s", LOG.debug("Network %s no-op, allocate_vip loadbalancer %s",
...@@ -260,6 +261,9 @@ class NoopManager(object): ...@@ -260,6 +261,9 @@ class NoopManager(object):
self.networkconfigconfig[(qos_id, port_id)] = ( self.networkconfigconfig[(qos_id, port_id)] = (
qos_id, port_id, 'apply_qos_on_port') qos_id, port_id, 'apply_qos_on_port')
def qos_enabled(self):
return self._qos_extension_enabled
class NoopNetworkDriver(driver_base.AbstractNetworkDriver): class NoopNetworkDriver(driver_base.AbstractNetworkDriver):
def __init__(self): def __init__(self):
...@@ -338,3 +342,6 @@ class NoopNetworkDriver(driver_base.AbstractNetworkDriver): ...@@ -338,3 +342,6 @@ class NoopNetworkDriver(driver_base.AbstractNetworkDriver):
def unplug_aap_port(self, vip, amphora, subnet): def unplug_aap_port(self, vip, amphora, subnet):
self.driver.unplug_aap_port(vip, amphora, subnet) self.driver.unplug_aap_port(vip, amphora, subnet)
def qos_enabled(self):
return self.driver.qos_enabled()
...@@ -1590,6 +1590,20 @@ class TestLoadBalancer(base.BaseAPITest): ...@@ -1590,6 +1590,20 @@ class TestLoadBalancer(base.BaseAPITest):
self.put(self.LB_PATH.format(lb_id=lb_dict.get('id')), self.put(self.LB_PATH.format(lb_id=lb_dict.get('id')),
lb_json, status=400) lb_json, status=400)
def test_update_with_qos_ext_disabled(self):
project_id = uuidutils.generate_uuid()
lb = self.create_load_balancer(uuidutils.generate_uuid(),
name='lb1',
project_id=project_id)
lb_dict = lb.get(self.root_tag)
self.set_lb_status(lb_dict.get('id'))
vip_qos_policy_id = uuidutils.generate_uuid()
lb_json = self._build_body({'vip_qos_policy_id': vip_qos_policy_id})
with mock.patch("octavia.network.drivers.noop_driver.driver"
".NoopManager.qos_enabled", return_value=False):
self.put(self.LB_PATH.format(lb_id=lb_dict.get('id')),
lb_json, status=400)
def test_update_bad_lb_id(self): def test_update_bad_lb_id(self):
path = self.LB_PATH.format(lb_id='SEAN-CONNERY') path = self.LB_PATH.format(lb_id='SEAN-CONNERY')
self.put(path, body={}, status=404) self.put(path, body={}, status=404)
......
...@@ -112,7 +112,7 @@ class TestAmphoraDriver(base.TestRpc): ...@@ -112,7 +112,7 @@ class TestAmphoraDriver(base.TestRpc):
provider_lb = driver_dm.LoadBalancer( provider_lb = driver_dm.LoadBalancer(
loadbalancer_id=self.sample_data.lb_id, loadbalancer_id=self.sample_data.lb_id,
vip_qos_policy_id=qos_policy_id) vip_qos_policy_id=qos_policy_id)
lb_dict = {'vip': {'vip_qos_policy_id': qos_policy_id}} lb_dict = {'vip': {'qos_policy_id': qos_policy_id}}
self.amp_driver.loadbalancer_update(old_provider_lb, provider_lb) self.amp_driver.loadbalancer_update(old_provider_lb, provider_lb)
payload = {consts.LOAD_BALANCER_ID: self.sample_data.lb_id, payload = {consts.LOAD_BALANCER_ID: self.sample_data.lb_id,
consts.LOAD_BALANCER_UPDATES: lb_dict} consts.LOAD_BALANCER_UPDATES: lb_dict}
......
...@@ -374,6 +374,18 @@ class TestValidations(base.TestCase): ...@@ -374,6 +374,18 @@ class TestValidations(base.TestCase):
validate.qos_policy_exists, validate.qos_policy_exists,
qos_policy_id) qos_policy_id)
def test_qos_extension_enabled(self):
network_driver = mock.Mock()
network_driver.qos_enabled.return_value = True
self.assertIsNone(validate.qos_extension_enabled(network_driver))
def test_qos_extension_disabled(self):
network_driver = mock.Mock()
network_driver.qos_enabled.return_value = False
self.assertRaises(exceptions.ValidationException,
validate.qos_extension_enabled,
network_driver)
def test_check_session_persistence(self): def test_check_session_persistence(self):
valid_cookie_name_dict = {'type': 'APP_COOKIE', valid_cookie_name_dict = {'type': 'APP_COOKIE',
'cookie_name': 'chocolate_chip'} 'cookie_name': 'chocolate_chip'}
......
---
fixes:
- Fixed an issue where trying to set a QoS policy on a VIP while the QoS
extension is disabled would bring the load balancer to ERROR. Should the
QoS extension be disabled, the API will now return HTTP 400 to the user.
- Fixed an issue where setting a QoS policy on the VIP would bring the load
balancer to ERROR when the QoS extension is enabled.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment