Add minimum bw qos rule validation for network
Minimum bandwidth QoS rule is only applicable for the network which is backed by physical networks. It will raise exception when you want to set Minimum bandwidth QoS rule or update rule on network without ports. Closes-Bug: #1913180 Conflicts: neutron/services/qos/drivers/manager.py neutron/services/qos/qos_plugin.py Change-Id: I6ab945086b13730ad60957760bbc2eb5c321aca2 (cherry picked from commit26f3de0f78
) (cherry picked from commita21f30ca64
)
This commit is contained in:
parent
83fbb1db5a
commit
9539db17dc
|
@ -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:
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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:
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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.
|
||||
|
||||
|
|
|
@ -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()))
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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()
|
||||
|
|
Loading…
Reference in New Issue