diff --git a/neutron/services/qos/drivers/manager.py b/neutron/services/qos/drivers/manager.py index e93fd557b59..8a0001fe57d 100644 --- a/neutron/services/qos/drivers/manager.py +++ b/neutron/services/qos/drivers/manager.py @@ -154,6 +154,18 @@ class QosServiceDriverManager(object): return False + def validate_rule_for_network(self, context, rule, network_id): + for driver in self._drivers: + if driver.is_rule_supported(rule): + # https://review.opendev.org/c/openstack/neutron-lib/+/774083 + # is not present, in this release, in neutron-lib. + if hasattr(driver, 'validate_rule_for_network'): + return driver.validate_rule_for_network(context, rule, + network_id) + return True + + return False + @property def supported_rule_types(self): if not self._drivers: diff --git a/neutron/services/qos/drivers/openvswitch/driver.py b/neutron/services/qos/drivers/openvswitch/driver.py index b1e599749f5..b12a0d75ed7 100644 --- a/neutron/services/qos/drivers/openvswitch/driver.py +++ b/neutron/services/qos/drivers/openvswitch/driver.py @@ -60,11 +60,14 @@ class OVSDriver(base.DriverBase): requires_rpc_notifications=True) def validate_rule_for_port(self, context, rule, port): + return self.validate_rule_for_network(context, rule, port.network_id) + + def validate_rule_for_network(self, context, rule, network_id): # Minimum-bandwidth rule is only supported on networks whose # first segment is backed by a physnet. if rule.rule_type == qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH: net = network_object.Network.get_object( - context, id=port.network_id) + context, id=network_id) physnet = net.segments[0].physical_network if physnet is None: return False diff --git a/neutron/services/qos/qos_plugin.py b/neutron/services/qos/qos_plugin.py index c8e18dd1489..cfd3950a8fd 100644 --- a/neutron/services/qos/qos_plugin.py +++ b/neutron/services/qos/qos_plugin.py @@ -49,6 +49,11 @@ from neutron.objects.qos import rule_type as rule_type_object from neutron.services.qos.drivers import manager +class QosRuleNotSupportedByNetwork(lib_exc.Conflict): + message = _("Rule %(rule_type)s is not supported " + "by network %(network_id)s") + + @resource_extend.has_resource_extenders class QoSPlugin(qos.QoSPluginBase): """Implementation of the Neutron QoS Service Plugin. @@ -86,6 +91,10 @@ class QoSPlugin(qos.QoSPluginBase): self._validate_update_network_callback, callbacks_resources.NETWORK, callbacks_events.PRECOMMIT_UPDATE) + callbacks_registry.subscribe( + self._validate_create_network_callback, + callbacks_resources.NETWORK, + callbacks_events.PRECOMMIT_CREATE) @staticmethod @resource_extend.extends([port_def.COLLECTION_NAME]) @@ -252,6 +261,20 @@ class QoSPlugin(qos.QoSPluginBase): self.validate_policy_for_port(context, policy, updated_port) + def _validate_create_network_callback(self, resource, event, trigger, + **kwargs): + context = kwargs['context'] + network_id = kwargs['network']['id'] + network = network_object.Network.get_object(context, id=network_id) + + policy_id = network.qos_policy_id + if policy_id is None: + return + + policy = policy_object.QosPolicy.get_object( + context.elevated(), id=policy_id) + self.validate_policy_for_network(context, policy, network_id) + def _validate_update_network_callback(self, resource, event, trigger, payload=None): context = payload.context @@ -266,6 +289,9 @@ class QoSPlugin(qos.QoSPluginBase): policy = policy_object.QosPolicy.get_object( context.elevated(), id=policy_id) + self.validate_policy_for_network( + context, policy, network_id=updated_network['id']) + ports = ports_object.Port.get_objects( context, network_id=updated_network['id']) # Filter only this ports which don't have overwritten policy @@ -289,6 +315,13 @@ class QoSPlugin(qos.QoSPluginBase): raise qos_exc.QosRuleNotSupported(rule_type=rule.rule_type, port_id=port['id']) + def validate_policy_for_network(self, context, policy, network_id): + for rule in policy.rules: + if not self.driver_manager.validate_rule_for_network( + context, rule, network_id): + raise QosRuleNotSupportedByNetwork( + rule_type=rule.rule_type, network_id=network_id) + def reject_min_bw_rule_updates(self, context, policy): ports = self._get_ports_with_policy(context, policy) for port in ports: diff --git a/neutron/tests/fullstack/resources/client.py b/neutron/tests/fullstack/resources/client.py index 9d14e5d00a5..42ae4427b74 100644 --- a/neutron/tests/fullstack/resources/client.py +++ b/neutron/tests/fullstack/resources/client.py @@ -81,7 +81,7 @@ class ClientFixture(fixtures.Fixture): def create_network(self, tenant_id, name=None, external=False, network_type=None, segmentation_id=None, - physical_network=None, mtu=None): + physical_network=None, mtu=None, qos_policy_id=None): resource_type = 'network' name = name or utils.get_rand_name(prefix=resource_type) @@ -96,6 +96,8 @@ class ClientFixture(fixtures.Fixture): spec['provider:physical_network'] = physical_network if mtu is not None: spec['mtu'] = mtu + if qos_policy_id is not None: + spec['qos_policy_id'] = qos_policy_id return self._create_resource(resource_type, spec) diff --git a/neutron/tests/fullstack/test_qos.py b/neutron/tests/fullstack/test_qos.py index 7f0e25d4c91..3aa628acdc5 100644 --- a/neutron/tests/fullstack/test_qos.py +++ b/neutron/tests/fullstack/test_qos.py @@ -707,6 +707,31 @@ class TestMinBwQoSOvs(_TestMinBwQoS, base.BaseFullStackTestCase): queues = '\nList of OVS Queue registers:\n%s' % '\n'.join(queues) self.fail(queuenum + qoses + queues) + def test_min_bw_qos_create_network_vxlan_not_supported(self): + qos_policy = self._create_qos_policy() + qos_policy_id = qos_policy['id'] + self.safe_client.create_minimum_bandwidth_rule( + self.tenant_id, qos_policy_id, MIN_BANDWIDTH, self.direction) + network_args = {'network_type': 'vxlan', + 'qos_policy_id': qos_policy_id} + self.assertRaises( + exceptions.Conflict, + self.safe_client.create_network, + self.tenant_id, name='network-test', **network_args) + + def test_min_bw_qos_update_network_vxlan_not_supported(self): + network_args = {'network_type': 'vxlan'} + network = self.safe_client.create_network( + self.tenant_id, name='network-test', **network_args) + qos_policy = self._create_qos_policy() + qos_policy_id = qos_policy['id'] + self.safe_client.create_minimum_bandwidth_rule( + self.tenant_id, qos_policy_id, MIN_BANDWIDTH, self.direction) + self.assertRaises( + exceptions.Conflict, + self.client.update_network, network['id'], + body={'network': {'qos_policy_id': qos_policy_id}}) + def test_min_bw_qos_port_removed(self): """Test if min BW limit config is properly removed when port removed. diff --git a/neutron/tests/unit/services/qos/drivers/openvswitch/test_driver.py b/neutron/tests/unit/services/qos/drivers/openvswitch/test_driver.py index f68145d7be8..cf77ba5eb83 100644 --- a/neutron/tests/unit/services/qos/drivers/openvswitch/test_driver.py +++ b/neutron/tests/unit/services/qos/drivers/openvswitch/test_driver.py @@ -43,3 +43,5 @@ class TestOVSDriver(base.BaseQosTestCase): return_value=net): test_method(self.driver.validate_rule_for_port( mock.Mock(), rule, port)) + test_method(self.driver.validate_rule_for_network( + mock.Mock(), rule, network_id=mock.Mock())) diff --git a/neutron/tests/unit/services/qos/drivers/test_manager.py b/neutron/tests/unit/services/qos/drivers/test_manager.py index 51ee1e4072e..64c757e38d6 100644 --- a/neutron/tests/unit/services/qos/drivers/test_manager.py +++ b/neutron/tests/unit/services/qos/drivers/test_manager.py @@ -118,6 +118,29 @@ class TestQoSDriversRulesValidations(TestQosDriversManagerBase): else: is_rule_supported_mock.assert_not_called() + def test_validate_rule_for_network(self): + driver_manager = self._create_manager_with_drivers({ + 'driver-A': { + 'is_loaded': True, + 'rules': { + qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH: { + "min_kbps": {'type:values': None}, + 'direction': { + 'type:values': lib_consts.VALID_DIRECTIONS} + } + } + } + }) + rule = rule_object.QosMinimumBandwidthRule( + self.ctxt, id=uuidutils.generate_uuid()) + + is_rule_supported_mock = mock.Mock() + is_rule_supported_mock.return_value = True + driver_manager._drivers[0].is_rule_supported = is_rule_supported_mock + self.assertTrue(driver_manager.validate_rule_for_network( + mock.Mock(), rule, mock.Mock())) + is_rule_supported_mock.assert_called_once_with(rule) + def test_validate_rule_for_port_rule_vif_type_supported(self): port = self._get_port( portbindings.VIF_TYPE_OVS, portbindings.VNIC_NORMAL) diff --git a/neutron/tests/unit/services/qos/test_qos_plugin.py b/neutron/tests/unit/services/qos/test_qos_plugin.py index 577d2c44448..54ac1c23a21 100644 --- a/neutron/tests/unit/services/qos/test_qos_plugin.py +++ b/neutron/tests/unit/services/qos/test_qos_plugin.py @@ -326,6 +326,8 @@ class TestQosPlugin(base.BaseQosTestCase): 'neutron.objects.qos.policy.QosPolicy.get_object', return_value=policy_mock ) as get_policy, mock.patch.object( + self.qos_plugin, "validate_policy_for_network" + ) as validate_policy_for_network, mock.patch.object( self.qos_plugin, "validate_policy_for_ports" ) as validate_policy_for_ports, mock.patch.object( self.ctxt, "elevated", return_value=admin_ctxt @@ -337,6 +339,7 @@ class TestQosPlugin(base.BaseQosTestCase): states=(kwargs['original_network'],))) if policy_id is None or policy_id == original_policy_id: get_policy.assert_not_called() + validate_policy_for_network.assert_not_called() get_ports.assert_not_called() validate_policy_for_ports.assert_not_called() else: @@ -384,6 +387,20 @@ class TestQosPlugin(base.BaseQosTestCase): except qos_exc.QosRuleNotSupported: self.fail("QosRuleNotSupported exception unexpectedly raised") + def test_validate_policy_for_network(self): + network = uuidutils.generate_uuid() + with mock.patch.object( + self.qos_plugin.driver_manager, "validate_rule_for_network", + return_value=True + ): + self.policy.rules = [self.rule] + try: + self.qos_plugin.validate_policy_for_network( + self.ctxt, self.policy, network_id=network) + except qos_exc.QosRuleNotSupportedByNetwork: + self.fail("QosRuleNotSupportedByNetwork " + "exception unexpectedly raised") + def test_create_min_bw_rule_on_bound_port(self): policy = self._get_policy() policy.rules = [self.min_rule] @@ -1236,6 +1253,35 @@ class TestQosPluginDB(base.BaseQosTestCase): network.create() return network + def _test_validate_create_network_callback(self, network_qos=False): + net_qos_obj = self._make_qos_policy() + net_qos_id = net_qos_obj.id if network_qos else None + network = self._make_network(qos_policy_id=net_qos_id) + kwargs = {"context": self.context, + "network": network} + + with mock.patch.object(self.qos_plugin, + 'validate_policy_for_network') \ + as mock_validate_policy: + self.qos_plugin._validate_create_network_callback( + 'NETWORK', 'precommit_create', 'test_plugin', **kwargs) + + qos_policy = None + if network_qos: + qos_policy = net_qos_obj + + if qos_policy: + mock_validate_policy.assert_called_once_with( + self.context, qos_policy, network.id) + else: + mock_validate_policy.assert_not_called() + + def test_validate_create_network_callback(self): + self._test_validate_create_network_callback(network_qos=True) + + def test_validate_create_network_callback_no_qos(self): + self._test_validate_create_network_callback(network_qos=False) + def _test_validate_create_port_callback(self, port_qos=False, network_qos=False): net_qos_obj = self._make_qos_policy()