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 commit 26f3de0f78
)
This commit is contained in:
parent
5c3f47b17e
commit
0f78c111e7
|
@ -154,6 +154,18 @@ class QosServiceDriverManager(object):
|
||||||
|
|
||||||
return False
|
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
|
@property
|
||||||
def supported_rule_types(self):
|
def supported_rule_types(self):
|
||||||
if not self._drivers:
|
if not self._drivers:
|
||||||
|
|
|
@ -60,11 +60,14 @@ class OVSDriver(base.DriverBase):
|
||||||
requires_rpc_notifications=True)
|
requires_rpc_notifications=True)
|
||||||
|
|
||||||
def validate_rule_for_port(self, context, rule, port):
|
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
|
# Minimum-bandwidth rule is only supported on networks whose
|
||||||
# first segment is backed by a physnet.
|
# first segment is backed by a physnet.
|
||||||
if rule.rule_type == qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH:
|
if rule.rule_type == qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH:
|
||||||
net = network_object.Network.get_object(
|
net = network_object.Network.get_object(
|
||||||
context, id=port.network_id)
|
context, id=network_id)
|
||||||
physnet = net.segments[0].physical_network
|
physnet = net.segments[0].physical_network
|
||||||
if physnet is None:
|
if physnet is None:
|
||||||
return False
|
return False
|
||||||
|
|
|
@ -49,6 +49,11 @@ from neutron.objects.qos import rule_type as rule_type_object
|
||||||
from neutron.services.qos.drivers import manager
|
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
|
@resource_extend.has_resource_extenders
|
||||||
class QoSPlugin(qos.QoSPluginBase):
|
class QoSPlugin(qos.QoSPluginBase):
|
||||||
"""Implementation of the Neutron QoS Service Plugin.
|
"""Implementation of the Neutron QoS Service Plugin.
|
||||||
|
@ -86,6 +91,10 @@ class QoSPlugin(qos.QoSPluginBase):
|
||||||
self._validate_update_network_callback,
|
self._validate_update_network_callback,
|
||||||
callbacks_resources.NETWORK,
|
callbacks_resources.NETWORK,
|
||||||
callbacks_events.PRECOMMIT_UPDATE)
|
callbacks_events.PRECOMMIT_UPDATE)
|
||||||
|
callbacks_registry.subscribe(
|
||||||
|
self._validate_create_network_callback,
|
||||||
|
callbacks_resources.NETWORK,
|
||||||
|
callbacks_events.PRECOMMIT_CREATE)
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
@resource_extend.extends([port_def.COLLECTION_NAME])
|
@resource_extend.extends([port_def.COLLECTION_NAME])
|
||||||
|
@ -252,6 +261,20 @@ class QoSPlugin(qos.QoSPluginBase):
|
||||||
|
|
||||||
self.validate_policy_for_port(context, policy, updated_port)
|
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,
|
def _validate_update_network_callback(self, resource, event, trigger,
|
||||||
payload=None):
|
payload=None):
|
||||||
context = payload.context
|
context = payload.context
|
||||||
|
@ -266,6 +289,9 @@ class QoSPlugin(qos.QoSPluginBase):
|
||||||
|
|
||||||
policy = policy_object.QosPolicy.get_object(
|
policy = policy_object.QosPolicy.get_object(
|
||||||
context.elevated(), id=policy_id)
|
context.elevated(), id=policy_id)
|
||||||
|
self.validate_policy_for_network(
|
||||||
|
context, policy, network_id=updated_network['id'])
|
||||||
|
|
||||||
ports = ports_object.Port.get_objects(
|
ports = ports_object.Port.get_objects(
|
||||||
context, network_id=updated_network['id'])
|
context, network_id=updated_network['id'])
|
||||||
# Filter only this ports which don't have overwritten policy
|
# 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,
|
raise qos_exc.QosRuleNotSupported(rule_type=rule.rule_type,
|
||||||
port_id=port['id'])
|
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):
|
def reject_min_bw_rule_updates(self, context, policy):
|
||||||
ports = self._get_ports_with_policy(context, policy)
|
ports = self._get_ports_with_policy(context, policy)
|
||||||
for port in ports:
|
for port in ports:
|
||||||
|
|
|
@ -81,7 +81,7 @@ class ClientFixture(fixtures.Fixture):
|
||||||
|
|
||||||
def create_network(self, tenant_id, name=None, external=False,
|
def create_network(self, tenant_id, name=None, external=False,
|
||||||
network_type=None, segmentation_id=None,
|
network_type=None, segmentation_id=None,
|
||||||
physical_network=None, mtu=None):
|
physical_network=None, mtu=None, qos_policy_id=None):
|
||||||
resource_type = 'network'
|
resource_type = 'network'
|
||||||
|
|
||||||
name = name or utils.get_rand_name(prefix=resource_type)
|
name = name or utils.get_rand_name(prefix=resource_type)
|
||||||
|
@ -96,6 +96,8 @@ class ClientFixture(fixtures.Fixture):
|
||||||
spec['provider:physical_network'] = physical_network
|
spec['provider:physical_network'] = physical_network
|
||||||
if mtu is not None:
|
if mtu is not None:
|
||||||
spec['mtu'] = mtu
|
spec['mtu'] = mtu
|
||||||
|
if qos_policy_id is not None:
|
||||||
|
spec['qos_policy_id'] = qos_policy_id
|
||||||
|
|
||||||
return self._create_resource(resource_type, spec)
|
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)
|
queues = '\nList of OVS Queue registers:\n%s' % '\n'.join(queues)
|
||||||
self.fail(queuenum + qoses + 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):
|
def test_min_bw_qos_port_removed(self):
|
||||||
"""Test if min BW limit config is properly removed when port removed.
|
"""Test if min BW limit config is properly removed when port removed.
|
||||||
|
|
||||||
|
|
|
@ -43,3 +43,5 @@ class TestOVSDriver(base.BaseQosTestCase):
|
||||||
return_value=net):
|
return_value=net):
|
||||||
test_method(self.driver.validate_rule_for_port(
|
test_method(self.driver.validate_rule_for_port(
|
||||||
mock.Mock(), rule, port))
|
mock.Mock(), rule, port))
|
||||||
|
test_method(self.driver.validate_rule_for_network(
|
||||||
|
mock.Mock(), rule, network_id=mock.Mock()))
|
||||||
|
|
|
@ -119,6 +119,29 @@ class TestQoSDriversRulesValidations(TestQosDriversManagerBase):
|
||||||
else:
|
else:
|
||||||
is_rule_supported_mock.assert_not_called()
|
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):
|
def test_validate_rule_for_port_rule_vif_type_supported(self):
|
||||||
port = self._get_port(
|
port = self._get_port(
|
||||||
portbindings.VIF_TYPE_OVS, portbindings.VNIC_NORMAL)
|
portbindings.VIF_TYPE_OVS, portbindings.VNIC_NORMAL)
|
||||||
|
|
|
@ -326,6 +326,8 @@ class TestQosPlugin(base.BaseQosTestCase):
|
||||||
'neutron.objects.qos.policy.QosPolicy.get_object',
|
'neutron.objects.qos.policy.QosPolicy.get_object',
|
||||||
return_value=policy_mock
|
return_value=policy_mock
|
||||||
) as get_policy, mock.patch.object(
|
) 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"
|
self.qos_plugin, "validate_policy_for_ports"
|
||||||
) as validate_policy_for_ports, mock.patch.object(
|
) as validate_policy_for_ports, mock.patch.object(
|
||||||
self.ctxt, "elevated", return_value=admin_ctxt
|
self.ctxt, "elevated", return_value=admin_ctxt
|
||||||
|
@ -337,6 +339,7 @@ class TestQosPlugin(base.BaseQosTestCase):
|
||||||
states=(kwargs['original_network'],)))
|
states=(kwargs['original_network'],)))
|
||||||
if policy_id is None or policy_id == original_policy_id:
|
if policy_id is None or policy_id == original_policy_id:
|
||||||
get_policy.assert_not_called()
|
get_policy.assert_not_called()
|
||||||
|
validate_policy_for_network.assert_not_called()
|
||||||
get_ports.assert_not_called()
|
get_ports.assert_not_called()
|
||||||
validate_policy_for_ports.assert_not_called()
|
validate_policy_for_ports.assert_not_called()
|
||||||
else:
|
else:
|
||||||
|
@ -384,6 +387,20 @@ class TestQosPlugin(base.BaseQosTestCase):
|
||||||
except qos_exc.QosRuleNotSupported:
|
except qos_exc.QosRuleNotSupported:
|
||||||
self.fail("QosRuleNotSupported exception unexpectedly raised")
|
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):
|
def test_create_min_bw_rule_on_bound_port(self):
|
||||||
policy = self._get_policy()
|
policy = self._get_policy()
|
||||||
policy.rules = [self.min_rule]
|
policy.rules = [self.min_rule]
|
||||||
|
@ -1236,6 +1253,35 @@ class TestQosPluginDB(base.BaseQosTestCase):
|
||||||
network.create()
|
network.create()
|
||||||
return network
|
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,
|
def _test_validate_create_port_callback(self, port_qos=False,
|
||||||
network_qos=False):
|
network_qos=False):
|
||||||
net_qos_obj = self._make_qos_policy()
|
net_qos_obj = self._make_qos_policy()
|
||||||
|
|
Loading…
Reference in New Issue