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

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,

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)

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.
 

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
+        """

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

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()

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)

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}

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'}

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.