diff --git a/neutron/services/qos/drivers/manager.py b/neutron/services/qos/drivers/manager.py index e93fd557b59..13818065d59 100644 --- a/neutron/services/qos/drivers/manager.py +++ b/neutron/services/qos/drivers/manager.py @@ -154,6 +154,15 @@ 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) and + 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 c56db9c8975..b026b1042b7 100644 --- a/neutron/services/qos/qos_plugin.py +++ b/neutron/services/qos/qos_plugin.py @@ -101,6 +101,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]) @@ -343,6 +347,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 @@ -357,6 +375,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 @@ -380,6 +401,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 qos_exc.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 bd13a45f47e..55198d9a3d3 100644 --- a/neutron/tests/fullstack/resources/client.py +++ b/neutron/tests/fullstack/resources/client.py @@ -91,7 +91,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) @@ -106,6 +106,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 7a3b8306adb..cfaae13e15e 100644 --- a/neutron/tests/fullstack/test_qos.py +++ b/neutron/tests/fullstack/test_qos.py @@ -718,6 +718,31 @@ class TestMinBwQoSOvs(_TestMinBwQoS, base.BaseFullStackTestCase): qoses, queues = self._qos_info(vm.bridge) 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 5378dbbd926..6345cb82608 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 0a55c7feb96..b4c2b8d7986 100644 --- a/neutron/tests/unit/services/qos/drivers/test_manager.py +++ b/neutron/tests/unit/services/qos/drivers/test_manager.py @@ -119,6 +119,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 df93f6a10df..b254f952686 100644 --- a/neutron/tests/unit/services/qos/test_qos_plugin.py +++ b/neutron/tests/unit/services/qos/test_qos_plugin.py @@ -328,6 +328,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 @@ -339,6 +341,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: @@ -386,6 +389,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] @@ -1246,6 +1263,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()