diff --git a/octavia/api/drivers/amphora_driver/driver.py b/octavia/api/drivers/amphora_driver/driver.py index c6ff69e443..6eb43530b3 100644 --- a/octavia/api/drivers/amphora_driver/driver.py +++ b/octavia/api/drivers/amphora_driver/driver.py @@ -95,7 +95,7 @@ class AmphoraProviderDriver(driver_base.ProviderDriver): # expects vip_qos_policy_id = lb_dict.pop('vip_qos_policy_id', None) 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 payload = {consts.LOAD_BALANCER_ID: lb_id, diff --git a/octavia/api/v2/controllers/load_balancer.py b/octavia/api/v2/controllers/load_balancer.py index 93013f0c7a..af52037b2f 100644 --- a/octavia/api/v2/controllers/load_balancer.py +++ b/octavia/api/v2/controllers/load_balancer.py @@ -547,11 +547,12 @@ class LoadBalancersController(base.BaseController): self._auth_validate_action(context, db_lb.project_id, constants.RBAC_PUT) - if (load_balancer.vip_qos_policy_id and - not isinstance(load_balancer.vip_qos_policy_id, - wtypes.UnsetType) and - db_lb.vip.qos_policy_id != load_balancer.vip_qos_policy_id): - validate.qos_policy_exists(load_balancer.vip_qos_policy_id) + if not isinstance(load_balancer.vip_qos_policy_id, wtypes.UnsetType): + network_driver = utils.get_network_driver() + validate.qos_extension_enabled(network_driver) + if load_balancer.vip_qos_policy_id is not None: + 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 driver = driver_factory.get_driver(db_lb.provider) diff --git a/octavia/common/validate.py b/octavia/common/validate.py index 16fbfe8453..947487b90b 100644 --- a/octavia/common/validate.py +++ b/octavia/common/validate.py @@ -342,6 +342,7 @@ def subnet_exists(subnet_id): def qos_policy_exists(qos_policy_id): network_driver = utils.get_network_driver() + qos_extension_enabled(network_driver) try: qos_policy = network_driver.get_qos_policy(qos_policy_id) except Exception: @@ -350,6 +351,12 @@ def qos_policy_exists(qos_policy_id): 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): """Raises an exception when a network does not exist. diff --git a/octavia/network/base.py b/octavia/network/base.py index 0266f3ac56..78b904927d 100644 --- a/octavia/network/base.py +++ b/octavia/network/base.py @@ -349,3 +349,10 @@ class AbstractNetworkDriver(object): :param subnet: The subnet to plug the aap into """ pass + + @abc.abstractmethod + def qos_enabled(self): + """Whether QoS is enabled + + :return: Boolean + """ diff --git a/octavia/network/drivers/neutron/base.py b/octavia/network/drivers/neutron/base.py index f9ec4f2b6a..f22a6cd8c4 100644 --- a/octavia/network/drivers/neutron/base.py +++ b/octavia/network/drivers/neutron/base.py @@ -27,6 +27,7 @@ from octavia.network.drivers.neutron import utils LOG = logging.getLogger(__name__) DNS_INT_EXT_ALIAS = 'dns-integration' SEC_GRP_EXT_ALIAS = 'security-group' +QOS_EXT_ALIAS = 'qos' CONF = cfg.CONF @@ -46,6 +47,7 @@ class BaseNeutronDriver(base.AbstractNetworkDriver): self.sec_grp_enabled = self._check_extension_enabled(SEC_GRP_EXT_ALIAS) self.dns_integration_enabled = self._check_extension_enabled( DNS_INT_EXT_ALIAS) + self._qos_enabled = self._check_extension_enabled(QOS_EXT_ALIAS) self.project_id = self.neutron_client.get_auth_info().get( 'auth_tenant_id') @@ -248,3 +250,6 @@ class BaseNeutronDriver(base.AbstractNetworkDriver): def get_qos_policy(self, qos_policy_id): return self._get_resource('qos_policy', qos_policy_id) + + def qos_enabled(self): + return self._qos_enabled diff --git a/octavia/network/drivers/noop_driver/driver.py b/octavia/network/drivers/noop_driver/driver.py index 8c961913e7..3352e2e115 100644 --- a/octavia/network/drivers/noop_driver/driver.py +++ b/octavia/network/drivers/noop_driver/driver.py @@ -26,6 +26,7 @@ class NoopManager(object): def __init__(self): super(NoopManager, self).__init__() self.networkconfigconfig = {} + self._qos_extension_enabled = True def allocate_vip(self, loadbalancer): LOG.debug("Network %s no-op, allocate_vip loadbalancer %s", @@ -260,6 +261,9 @@ class NoopManager(object): self.networkconfigconfig[(qos_id, port_id)] = ( qos_id, port_id, 'apply_qos_on_port') + def qos_enabled(self): + return self._qos_extension_enabled + class NoopNetworkDriver(driver_base.AbstractNetworkDriver): def __init__(self): @@ -338,3 +342,6 @@ class NoopNetworkDriver(driver_base.AbstractNetworkDriver): def unplug_aap_port(self, vip, amphora, subnet): self.driver.unplug_aap_port(vip, amphora, subnet) + + def qos_enabled(self): + return self.driver.qos_enabled() diff --git a/octavia/tests/functional/api/v2/test_load_balancer.py b/octavia/tests/functional/api/v2/test_load_balancer.py index fad027881c..b798c527f6 100644 --- a/octavia/tests/functional/api/v2/test_load_balancer.py +++ b/octavia/tests/functional/api/v2/test_load_balancer.py @@ -1590,6 +1590,20 @@ class TestLoadBalancer(base.BaseAPITest): self.put(self.LB_PATH.format(lb_id=lb_dict.get('id')), 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): path = self.LB_PATH.format(lb_id='SEAN-CONNERY') self.put(path, body={}, status=404) diff --git a/octavia/tests/unit/api/drivers/amphora_driver/test_amphora_driver.py b/octavia/tests/unit/api/drivers/amphora_driver/test_amphora_driver.py index 1804e9d1a7..856ca6995e 100644 --- a/octavia/tests/unit/api/drivers/amphora_driver/test_amphora_driver.py +++ b/octavia/tests/unit/api/drivers/amphora_driver/test_amphora_driver.py @@ -112,7 +112,7 @@ class TestAmphoraDriver(base.TestRpc): provider_lb = driver_dm.LoadBalancer( loadbalancer_id=self.sample_data.lb_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) payload = {consts.LOAD_BALANCER_ID: self.sample_data.lb_id, consts.LOAD_BALANCER_UPDATES: lb_dict} diff --git a/octavia/tests/unit/common/test_validations.py b/octavia/tests/unit/common/test_validations.py index c05828673f..8365deb894 100644 --- a/octavia/tests/unit/common/test_validations.py +++ b/octavia/tests/unit/common/test_validations.py @@ -374,6 +374,18 @@ class TestValidations(base.TestCase): validate.qos_policy_exists, 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): valid_cookie_name_dict = {'type': 'APP_COOKIE', 'cookie_name': 'chocolate_chip'} diff --git a/releasenotes/notes/fix-vip-qos-policy-extension-enabled-3e16e1c23a7d7ae5.yaml b/releasenotes/notes/fix-vip-qos-policy-extension-enabled-3e16e1c23a7d7ae5.yaml new file mode 100644 index 0000000000..3f210296d8 --- /dev/null +++ b/releasenotes/notes/fix-vip-qos-policy-extension-enabled-3e16e1c23a7d7ae5.yaml @@ -0,0 +1,7 @@ +--- +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.