openstack/neutron-lib
Code Issues Proposed changes

Make update_qos_minbw_allocation() more generic

Browse Source 
update_qos_minbw_allocation() function is used to update Placement
allocation. Initially, only minimum bandwidth rule allocated resources
in Placement, but with the introduction of minimum packet rate rule
that's changed. Because of that, we should rename the function to make
it more generic and to avoid confusion.

The second problem with this function is that it allows to update
resources only of a single RP at a time, even if multiple RPs are
associated with the same consumer UUID. With introduction of a
minimum packet rate rule it makes sense to allow to update resources of
multiple RPs in a single API call. To accommodate this, we need to
slightly modify arguments this function takes, and embed RP UUID
in alloc_diff, rather than pass it as a separate parameter.

Addresses TODO comment from [1].

[1] https://opendev.org/openstack/neutron/src/branch/master/neutron/services/qos/qos_plugin.py#L68

Partial-Bug: #1943724
Related-Bug: #1922237
See-Also: https://review.opendev.org/785236
Change-Id: Ie28b95e8ed351ab88db1fc75c83a02c474582e0b
This commit is contained in:
Przemyslaw Szczerbik 2021-09-07 15:37:07 +02:00
parent b35ba2fe95
commit d2bd7760ef
2 changed files with 97 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
neutron_lib/placement/client.py
View File

@ -752,39 +752,40 @@ class PlacementAPIClient(object):
url = '/allocations/%s' % consumer_uuid
return self._get(url).json()
def update_qos_minbw_allocation(self, consumer_uuid, minbw_alloc_diff,
rp_uuid):
def update_qos_allocation(self, consumer_uuid, alloc_diff):
"""Update allocation for QoS minimum bandwidth consumer
:param consumer_uuid: The uuid of the consumer, in case of bound port
owned by a VM, the VM uuid.
:param minbw_alloc_diff: A dict which contains the fields to update
for the allocation under the given resource
provider.
:param rp_uuid: uuid of the resource provider for which the
allocations are to be updated.
:param alloc_diff: A dict which contains RP UUIDs as keys and
corresponding fields to update for the allocation
under the given resource provider.
"""
for i in range(GENERATION_CONFLICT_RETRIES):
body = self.list_allocations(consumer_uuid)
if not body['allocations']:
raise n_exc.PlacementAllocationRemoved(consumer=consumer_uuid)
if rp_uuid not in body['allocations']:
raise n_exc.PlacementAllocationRpNotExists(
resource_provider=rp_uuid, consumer=consumer_uuid)
# Count new min_kbps values based on the diff in alloc_diff
for drctn, min_kbps_diff in minbw_alloc_diff.items():
orig_kbps = body['allocations'][rp_uuid]['resources'][drctn]
new_kbps = orig_kbps + min_kbps_diff
if new_kbps > 0:
body['allocations'][rp_uuid]['resources'][drctn] = new_kbps
else:
# Remove the resource class if the new value for min_kbps
# is 0
resources = body['allocations'][rp_uuid]['resources']
if len(resources) > 1:
resources.pop(drctn, None)
# Count new values based on the diff in alloc_diff
for rp_uuid, diff in alloc_diff.items():
if rp_uuid not in body['allocations']:
raise n_exc.PlacementAllocationRpNotExists(
resource_provider=rp_uuid, consumer=consumer_uuid)
for drctn, value in diff.items():
orig_value = body['allocations'][rp_uuid][
'resources'].get(drctn, 0)
new_value = orig_value + value
if new_value > 0:
body['allocations'][rp_uuid]['resources'][
drctn] = new_value
else:
body['allocations'].pop(rp_uuid)
# Remove the resource class if the new value is 0
resources = body['allocations'][rp_uuid]['resources']
resources.pop(drctn, None)
# Remove RPs without any resources
body['allocations'] = {
rp: alloc for rp, alloc in body['allocations'].items()
if alloc.get('resources')}
try:
# Update allocations has no return body, but leave the loop
return self.update_allocation(consumer_uuid, body)

114
neutron_lib/tests/unit/placement/test_client.py
View File

@ -26,6 +26,7 @@ from neutron_lib.tests import _base as base
RESOURCE_PROVIDER_UUID = uuidutils.generate_uuid()
SECOND_RESOURCE_PROVIDER_UUID = uuidutils.generate_uuid()
CONSUMER_UUID = uuidutils.generate_uuid()
RESOURCE_PROVIDER_NAME = 'resource_provider_name'
RESOURCE_PROVIDER = {
@ -727,23 +728,20 @@ class TestPlacementAPIClient(base.BaseTestCase):
}}
)
def _get_allocation_response(self, resources):
def _get_allocation_response(self, allocation):
mock_rsp_get = mock.Mock()
mock_rsp_get.json = lambda: {
'allocations': {
RESOURCE_PROVIDER_UUID: resources
}
'allocations': allocation
}
return mock_rsp_get
def test_update_qos_minbw_allocation(self):
def test_update_qos_allocation(self):
mock_rsp_get = self._get_allocation_response(
{'resources': {'a': 3, 'b': 2}})
{RESOURCE_PROVIDER_UUID: {'resources': {'a': 3, 'b': 2}}})
self.placement_fixture.mock_get.side_effect = [mock_rsp_get]
self.placement_api_client.update_qos_minbw_allocation(
self.placement_api_client.update_qos_allocation(
consumer_uuid=CONSUMER_UUID,
minbw_alloc_diff={'a': 2, 'b': 2},
rp_uuid=RESOURCE_PROVIDER_UUID
alloc_diff={RESOURCE_PROVIDER_UUID: {'a': 2, 'b': 2}},
)
self.placement_fixture.mock_put.assert_called_once_with(
'/allocations/%s' % CONSUMER_UUID,
@ -753,32 +751,31 @@ class TestPlacementAPIClient(base.BaseTestCase):
}}
)
def test_update_qos_minbw_allocation_removed(self):
def test_update_qos_allocation_removed(self):
mock_rsp = mock.Mock()
mock_rsp.json = lambda: {'allocations': {}}
self.placement_fixture.mock_get.side_effect = [mock_rsp]
self.assertRaises(
n_exc.PlacementAllocationRemoved,
self.placement_api_client.update_qos_minbw_allocation,
self.placement_api_client.update_qos_allocation,
consumer_uuid=CONSUMER_UUID,
minbw_alloc_diff={'a': 1, 'b': 1},
rp_uuid=RESOURCE_PROVIDER_UUID
alloc_diff={RESOURCE_PROVIDER_UUID: {'a': 1, 'b': 1}},
)
def test_update_qos_minbw_allocation_rp_not_exists(self):
def test_update_qos_allocation_rp_not_exists(self):
mock_rsp = mock.Mock()
mock_rsp.json = lambda: {'allocations': {'other:rp:uuid': {'c': 3}}}
self.placement_fixture.mock_get.side_effect = [mock_rsp]
self.assertRaises(
n_exc.PlacementAllocationRpNotExists,
self.placement_api_client.update_qos_minbw_allocation,
self.placement_api_client.update_qos_allocation,
consumer_uuid=CONSUMER_UUID,
minbw_alloc_diff={'a': 1, 'b': 1},
rp_uuid=RESOURCE_PROVIDER_UUID
alloc_diff={RESOURCE_PROVIDER_UUID: {'a': 1, 'b': 1}},
)
def test_update_qos_minbw_allocation_max_retries(self):
mock_rsp_get = self._get_allocation_response({'c': 3})
def test_update_qos_allocation_max_retries(self):
mock_rsp_get = self._get_allocation_response(
{RESOURCE_PROVIDER_UUID: {'c': 3}})
self.placement_fixture.mock_get.side_effect = 10 * [mock_rsp_get]
mock_rsp_put = mock.Mock()
mock_rsp_put.json = lambda: {
@ -787,15 +784,15 @@ class TestPlacementAPIClient(base.BaseTestCase):
response=mock_rsp_put)
self.assertRaises(
n_exc.PlacementAllocationGenerationConflict,
self.placement_api_client.update_qos_minbw_allocation,
self.placement_api_client.update_qos_allocation,
consumer_uuid=CONSUMER_UUID,
minbw_alloc_diff={},
rp_uuid=RESOURCE_PROVIDER_UUID,
alloc_diff={RESOURCE_PROVIDER_UUID: {}},
)
self.assertEqual(10, self.placement_fixture.mock_put.call_count)
def test_update_qos_minbwallocation_generation_conflict_solved(self):
mock_rsp_get = self._get_allocation_response({'c': 3})
mock_rsp_get = self._get_allocation_response(
{RESOURCE_PROVIDER_UUID: {'c': 3}})
self.placement_fixture.mock_get.side_effect = 2 * [mock_rsp_get]
mock_rsp_put = mock.Mock()
mock_rsp_put.json = lambda: {
@ -804,15 +801,15 @@ class TestPlacementAPIClient(base.BaseTestCase):
ks_exc.Conflict(response=mock_rsp_put),
mock.Mock()
]
self.placement_api_client.update_qos_minbw_allocation(
self.placement_api_client.update_qos_allocation(
consumer_uuid=CONSUMER_UUID,
minbw_alloc_diff={},
rp_uuid=RESOURCE_PROVIDER_UUID
alloc_diff={RESOURCE_PROVIDER_UUID: {}},
)
self.assertEqual(2, self.placement_fixture.mock_put.call_count)
def test_update_qos_minbw_allocation_other_conflict(self):
mock_rsp_get = self._get_allocation_response({'c': 3})
def test_update_qos_allocation_other_conflict(self):
mock_rsp_get = self._get_allocation_response(
{RESOURCE_PROVIDER_UUID: {'c': 3}})
self.placement_fixture.mock_get.side_effect = 10*[mock_rsp_get]
mock_rsp_put = mock.Mock()
mock_rsp_put.text = ''
@ -822,34 +819,31 @@ class TestPlacementAPIClient(base.BaseTestCase):
response=mock_rsp_put)
self.assertRaises(
ks_exc.Conflict,
self.placement_api_client.update_qos_minbw_allocation,
self.placement_api_client.update_qos_allocation,
consumer_uuid=CONSUMER_UUID,
minbw_alloc_diff={},
rp_uuid=RESOURCE_PROVIDER_UUID,
alloc_diff={RESOURCE_PROVIDER_UUID: {}},
)
self.placement_fixture.mock_put.assert_called_once()
def test_update_qos_minbw_allocation_to_zero(self):
def test_update_qos_allocation_to_zero(self):
mock_rsp_get = self._get_allocation_response(
{'resources': {'a': 3, 'b': 2}})
{RESOURCE_PROVIDER_UUID: {'resources': {'a': 3, 'b': 2}}})
self.placement_fixture.mock_get.side_effect = [mock_rsp_get]
self.placement_api_client.update_qos_minbw_allocation(
self.placement_api_client.update_qos_allocation(
consumer_uuid=CONSUMER_UUID,
minbw_alloc_diff={'a': -3, 'b': -2},
rp_uuid=RESOURCE_PROVIDER_UUID
alloc_diff={RESOURCE_PROVIDER_UUID: {'a': -3, 'b': -2}},
)
self.placement_fixture.mock_put.assert_called_once_with(
'/allocations/%s' % CONSUMER_UUID,
{'allocations': {}})
def test_update_qos_minbw_allocation_one_class_to_zero(self):
def test_update_qos_allocation_one_class_to_zero(self):
mock_rsp_get = self._get_allocation_response(
{'resources': {'a': 3, 'b': 2}})
{RESOURCE_PROVIDER_UUID: {'resources': {'a': 3, 'b': 2}}})
self.placement_fixture.mock_get.side_effect = [mock_rsp_get]
self.placement_api_client.update_qos_minbw_allocation(
self.placement_api_client.update_qos_allocation(
consumer_uuid=CONSUMER_UUID,
minbw_alloc_diff={'a': -3, 'b': 1},
rp_uuid=RESOURCE_PROVIDER_UUID
alloc_diff={RESOURCE_PROVIDER_UUID: {'a': -3, 'b': 1}},
)
self.placement_fixture.mock_put.assert_called_once_with(
'/allocations/%s' % CONSUMER_UUID,
@ -857,3 +851,41 @@ class TestPlacementAPIClient(base.BaseTestCase):
RESOURCE_PROVIDER_UUID: {
'resources': {'b': 3}}
}})
def test_update_qos_allocation_one_class_to_zero_and_new_class(self):
mock_rsp_get = self._get_allocation_response(
{RESOURCE_PROVIDER_UUID: {'resources': {'a': 3}}})
self.placement_fixture.mock_get.side_effect = [mock_rsp_get]
self.placement_api_client.update_qos_allocation(
consumer_uuid=CONSUMER_UUID,
alloc_diff={RESOURCE_PROVIDER_UUID: {'a': -3, 'b': 1}},
)
self.placement_fixture.mock_put.assert_called_once_with(
'/allocations/%s' % CONSUMER_UUID,
{'allocations': {
RESOURCE_PROVIDER_UUID: {
'resources': {'b': 1}}
}})
def test_update_qos_allocation_multiple_rps(self):
mock_rsp_get = self._get_allocation_response({
RESOURCE_PROVIDER_UUID: {'resources': {'a': 3, 'b': 2}},
SECOND_RESOURCE_PROVIDER_UUID: {'resources': {'c': 1, 'd': 5}},
})
self.placement_fixture.mock_get.side_effect = [mock_rsp_get]
self.placement_api_client.update_qos_allocation(
consumer_uuid=CONSUMER_UUID,
alloc_diff={
RESOURCE_PROVIDER_UUID: {'a': -3, 'b': 2},
SECOND_RESOURCE_PROVIDER_UUID: {'e': 3, 'd': -5},
},
)
self.placement_fixture.mock_put.assert_called_once_with(
'/allocations/%s' % CONSUMER_UUID,
{'allocations': {
RESOURCE_PROVIDER_UUID: {
'resources': {'b': 4}},
SECOND_RESOURCE_PROVIDER_UUID: {
'resources': {'c': 1, 'e': 3}},
}}
)