openstack
/
nova
Code Issues Proposed changes

Allow resizing server with port resource request 

This patch adds functional coverage for such resize. No extra code
changes were needed on top of the migrate patches to make resize work.

Now that the nova code supports the resize this patch removes the
check from the API that rejected the operation.

Note that in the spec [1] we agreed not to introduce new microversion
for this change but treat it as a bugfix. The current change also makes
it possible to accept the resize of these servers with _any_
microversion.

[1] https://specs.openstack.org/openstack/nova-specs/specs/train/approved/support-move-ops-with-qos-ports.html#rest-api-impact

blueprint: support-move-ops-with-qos-ports

Change-Id: Id0ee10e8d323786f4d79c82e3f05b48e76bd0956
This commit is contained in:
Balazs Gibizer 2019-08-27 15:05:55 +02:00 committed by Matt Riedemann
parent fee98b7147
commit a412d72dec
7 changed files with 123 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
api-guide/source/port_with_resource_request.rst
View File

@ -25,7 +25,11 @@ version of nova:
shelve offload) servers with ports having resource request is not yet
supported.
As of 20.0.0 (Train), nova supports cold migrating servers with neutron ports
having resource requests if both the source and destination compute services
are upgraded to 20.0.0 (Train) and the ``[upgrade_levels]/compute``
configuration does not prevent the computes from using the latest RPC version.
As of 20.0.0 (Train), nova supports cold migrating and resizing servers with
neutron ports having resource requests if both the source and destination
compute services are upgraded to 20.0.0 (Train) and the
``[upgrade_levels]/compute`` configuration does not prevent the computes from
using the latest RPC version.
See :nova-doc:`the admin guide <admin/port_with_resource_request.html>` for
administrative details.

10
nova/api/openstack/compute/servers.py
View File

@ -940,18 +940,8 @@ class ServersController(wsgi.Controller):
target={'user_id': instance.user_id,
'project_id': instance.project_id})
# We could potentially move this check to conductor and avoid the
# extra API call to neutron when we support move operations with ports
# having resource requests.
if common.instance_has_port_with_resource_request(
context, instance_id, self.network_api):
if not common.supports_port_resource_request_during_move(req):
msg = _("The resize action on a server with ports having "
"resource requests, like a port with a QoS minimum "
"bandwidth policy, is not supported with this "
"microversion")
raise exc.HTTPBadRequest(explanation=msg)
# TODO(gibi): Remove when nova only supports compute newer than
# Train
source_service = objects.Service.get_by_host_and_binary(

13
nova/compute/manager.py
View File

@ -4284,7 +4284,8 @@ class ComputeManager(manager.Manager):
instance.save()
try:
self._revert_allocation(context, instance, migration)
source_allocations = self._revert_allocation(
context, instance, migration)
except exception.AllocationMoveFailed:
LOG.error('Reverting allocation in placement for migration '
'%(migration_uuid)s failed. The instance '
@ -4295,16 +4296,12 @@ class ComputeManager(manager.Manager):
raise
if request_spec:
# TODO(gibi): the _revert_allocation() call above already
# fetched the original allocation of the instance so we could
# avoid this second call to placement
# NOTE(gibi): We need to re-calculate the resource provider -
# port mapping as we have to have the neutron ports allocate
# from the source compute after revert.
allocs = self.reportclient.get_allocations_for_consumer(
context, instance.uuid)
scheduler_utils.fill_provider_mapping_based_on_allocation(
context, self.reportclient, request_spec, allocs)
context, self.reportclient, request_spec,
source_allocations)
provider_mappings = self._get_request_group_mapping(
request_spec)
else:
@ -4391,7 +4388,7 @@ class ComputeManager(manager.Manager):
# TODO(cdent): Should we be doing anything with return values here?
self.reportclient.move_allocations(context, migration.uuid,
instance.uuid)
return True
return orig_alloc
def _prep_resize(self, context, image, instance, instance_type,
filter_properties, node, migration, request_spec,

4
nova/tests/functional/integrated_helpers.py
View File

@ -433,10 +433,10 @@ class ProviderUsageBaseTestCase(test.TestCase, InstanceHelperMixin):
placement = self.useFixture(func_fixtures.PlacementFixture())
self.placement_api = placement.api
api_fixture = self.useFixture(nova_fixtures.OSAPIFixture(
self.api_fixture = self.useFixture(nova_fixtures.OSAPIFixture(
api_version='v2.1'))
self.admin_api = api_fixture.admin_api
self.admin_api = self.api_fixture.admin_api
self.admin_api.microversion = self.microversion
self.api = self.admin_api

160
nova/tests/functional/test_servers.py
View File

@ -5795,31 +5795,6 @@ class UnsupportedPortResourceRequestBasedSchedulingTest(
"until microversion 2.72.",
six.text_type(ex))
def test_resize_server_with_port_resource_request_old_microversion(self):
server = self._create_server(
flavor=self.flavor,
networks=[{'port': self.neutron.port_1['id']}])
self._wait_for_state_change(self.admin_api, server, 'ACTIVE')
# We need to simulate that the above server has a port that has
# resource request; we cannot boot with such a port but legacy servers
# can exist with such a port.
self._add_resource_request_to_a_bound_port(self.neutron.port_1['id'])
resize_req = {
'resize': {
'flavorRef': self.flavor['id']
}
}
ex = self.assertRaises(
client.OpenStackApiException,
self.api.post_server_action, server['id'], resize_req)
self.assertEqual(400, ex.response.status_code)
self.assertIn(
'The resize action on a server with ports having resource '
'requests', six.text_type(ex))
def test_live_migrate_server_with_port_resource_request_old_microversion(
self):
server = self._create_server(
@ -6348,9 +6323,22 @@ class ServerMoveWithPortResourceRequestTest(
self.compute2_service_id = self.admin_api.get_services(
host='host2', binary='nova-compute')[0]['id']
# create a bigger flavor to use in resize test
self.flavor_with_group_policy_bigger = self.admin_api.post_flavor(
{'flavor': {
'ram': self.flavor_with_group_policy['ram'],
'vcpus': self.flavor_with_group_policy['vcpus'],
'name': self.flavor_with_group_policy['name'] + '+',
'disk': self.flavor_with_group_policy['disk'] + 1,
}})
self.admin_api.post_extra_spec(
self.flavor_with_group_policy_bigger['id'],
{'extra_specs': {'group_policy': 'isolate'}})
def _check_allocation(
self, server, compute_rp_uuid, non_qos_port, qos_port,
qos_sriov_port, migration_uuid=None, source_compute_rp_uuid=None):
qos_sriov_port, flavor, migration_uuid=None,
source_compute_rp_uuid=None, new_flavor=None):
updated_non_qos_port = self.neutron.show_port(
non_qos_port['id'])['port']
@ -6361,13 +6349,18 @@ class ServerMoveWithPortResourceRequestTest(
allocations = self.placement_api.get(
'/allocations/%s' % server['id']).body['allocations']
# if there is new_flavor then we either have an in progress resize or
# a confirmed resize. In both cases the instance allocation should be
# according to the new_flavor
current_flavor = (new_flavor if new_flavor else flavor)
# We expect one set of allocations for the compute resources on the
# compute rp and two sets for the networking resources one on the ovs
# bridge rp due to the qos_port resource request and one one the
# sriov pf2 due to qos_sriov_port resource request
self.assertEqual(3, len(allocations))
self.assertComputeAllocationMatchesFlavor(
allocations, compute_rp_uuid, self.flavor_with_group_policy)
allocations, compute_rp_uuid, current_flavor)
ovs_allocations = allocations[
self.ovs_bridge_rp_per_host[compute_rp_uuid]]['resources']
self.assertPortMatchesAllocation(qos_port, ovs_allocations)
@ -6399,8 +6392,7 @@ class ServerMoveWithPortResourceRequestTest(
# the sriov pf2 due to qos_sriov_port resource request
self.assertEqual(3, len(migration_allocations))
self.assertComputeAllocationMatchesFlavor(
migration_allocations, source_compute_rp_uuid,
self.flavor_with_group_policy)
migration_allocations, source_compute_rp_uuid, flavor)
ovs_allocations = migration_allocations[
self.ovs_bridge_rp_per_host[
source_compute_rp_uuid]]['resources']
@ -6445,7 +6437,7 @@ class ServerMoveWithPortResourceRequestTest(
# check that the server allocates from the current host properly
self._check_allocation(
server, self.compute1_rp_uuid, non_qos_normal_port,
qos_normal_port, qos_sriov_port)
qos_normal_port, qos_sriov_port, self.flavor_with_group_policy)
orig_get_service = nova.objects.Service.get_by_host_and_binary
@ -6471,7 +6463,7 @@ class ServerMoveWithPortResourceRequestTest(
# check that the server still allocates from the original host
self._check_allocation(
server, self.compute1_rp_uuid, non_qos_normal_port,
qos_normal_port, qos_sriov_port)
qos_normal_port, qos_sriov_port, self.flavor_with_group_policy)
# but the migration allocation is gone
migration_uuid = self.get_migration_uuid_for_instance(server['id'])
@ -6501,7 +6493,7 @@ class ServerMoveWithPortResourceRequestTest(
# check that the server allocates from the current host properly
self._check_allocation(
server, self.compute1_rp_uuid, non_qos_normal_port,
qos_normal_port, qos_sriov_port)
qos_normal_port, qos_sriov_port, self.flavor_with_group_policy)
orig_get_service = nova.objects.Service.get_by_host_and_binary
@ -6530,14 +6522,14 @@ class ServerMoveWithPortResourceRequestTest(
# check that server allocates from host3
self._check_allocation(
server, compute3_rp_uuid, non_qos_normal_port, qos_normal_port,
qos_sriov_port, migration_uuid,
qos_sriov_port, self.flavor_with_group_policy, migration_uuid,
source_compute_rp_uuid=self.compute1_rp_uuid)
self._confirm_resize(server)
# check that allocation is still OK
self._check_allocation(
server, compute3_rp_uuid, non_qos_normal_port,
qos_normal_port, qos_sriov_port)
qos_normal_port, qos_sriov_port, self.flavor_with_group_policy)
# but the migration allocation is gone
migration_allocations = self.placement_api.get(
'/allocations/%s' % migration_uuid).body['allocations']
@ -6546,7 +6538,7 @@ class ServerMoveWithPortResourceRequestTest(
self._delete_server_and_check_allocations(
qos_normal_port, qos_sriov_port, server)
def test_migrate_server_with_qos_ports(self):
def _test_resize_or_migrate_server_with_qos_ports(self, new_flavor=None):
non_qos_normal_port = self.neutron.port_1
qos_normal_port = self.neutron.port_with_resource_request
qos_sriov_port = self.neutron.port_with_sriov_resource_request
@ -6557,9 +6549,14 @@ class ServerMoveWithPortResourceRequestTest(
# check that the server allocates from the current host properly
self._check_allocation(
server, self.compute1_rp_uuid, non_qos_normal_port,
qos_normal_port, qos_sriov_port)
qos_normal_port, qos_sriov_port, self.flavor_with_group_policy)
if new_flavor:
self.api_fixture.api.post_server_action(
server['id'], {'resize': {"flavorRef": new_flavor['id']}})
else:
self.api.post_server_action(server['id'], {'migrate': None})
self.api.post_server_action(server['id'], {'migrate': None})
self._wait_for_state_change(self.api, server, 'VERIFY_RESIZE')
migration_uuid = self.get_migration_uuid_for_instance(server['id'])
@ -6567,15 +6564,17 @@ class ServerMoveWithPortResourceRequestTest(
# check that server allocates from the new host properly
self._check_allocation(
server, self.compute2_rp_uuid, non_qos_normal_port,
qos_normal_port, qos_sriov_port, migration_uuid,
source_compute_rp_uuid=self.compute1_rp_uuid)
qos_normal_port, qos_sriov_port, self.flavor_with_group_policy,
migration_uuid, source_compute_rp_uuid=self.compute1_rp_uuid,
new_flavor=new_flavor)
self._confirm_resize(server)
# check that allocation is still OK
self._check_allocation(
server, self.compute2_rp_uuid, non_qos_normal_port,
qos_normal_port, qos_sriov_port)
qos_normal_port, qos_sriov_port, self.flavor_with_group_policy,
new_flavor=new_flavor)
migration_allocations = self.placement_api.get(
'/allocations/%s' % migration_uuid).body['allocations']
self.assertEqual({}, migration_allocations)
@ -6583,7 +6582,14 @@ class ServerMoveWithPortResourceRequestTest(
self._delete_server_and_check_allocations(
qos_normal_port, qos_sriov_port, server)
def test_migrate_revert_with_qos_port(self):
def test_migrate_server_with_qos_ports(self):
self._test_resize_or_migrate_server_with_qos_ports()
def test_resize_server_with_qos_ports(self):
self._test_resize_or_migrate_server_with_qos_ports(
new_flavor=self.flavor_with_group_policy_bigger)
def _test_resize_or_migrate_revert_with_qos_ports(self, new_flavor=None):
non_qos_port = self.neutron.port_1
qos_port = self.neutron.port_with_resource_request
qos_sriov_port = self.neutron.port_with_sriov_resource_request
@ -6594,9 +6600,14 @@ class ServerMoveWithPortResourceRequestTest(
# check that the server allocates from the current host properly
self._check_allocation(
server, self.compute1_rp_uuid, non_qos_port, qos_port,
qos_sriov_port)
qos_sriov_port, self.flavor_with_group_policy)
if new_flavor:
self.api_fixture.api.post_server_action(
server['id'], {'resize': {"flavorRef": new_flavor['id']}})
else:
self.api.post_server_action(server['id'], {'migrate': None})
self.api.post_server_action(server['id'], {'migrate': None})
self._wait_for_state_change(self.api, server, 'VERIFY_RESIZE')
migration_uuid = self.get_migration_uuid_for_instance(server['id'])
@ -6604,8 +6615,9 @@ class ServerMoveWithPortResourceRequestTest(
# check that server allocates from the new host properly
self._check_allocation(
server, self.compute2_rp_uuid, non_qos_port, qos_port,
qos_sriov_port, migration_uuid,
source_compute_rp_uuid=self.compute1_rp_uuid)
qos_sriov_port, self.flavor_with_group_policy, migration_uuid,
source_compute_rp_uuid=self.compute1_rp_uuid,
new_flavor=new_flavor)
self.api.post_server_action(server['id'], {'revertResize': None})
self._wait_for_state_change(self.api, server, 'ACTIVE')
@ -6613,7 +6625,7 @@ class ServerMoveWithPortResourceRequestTest(
# check that allocation is moved back to the source host
self._check_allocation(
server, self.compute1_rp_uuid, non_qos_port, qos_port,
qos_sriov_port)
qos_sriov_port, self.flavor_with_group_policy)
# check that the target host allocation is cleaned up.
self.assertRequestMatchesUsage(
@ -6627,7 +6639,15 @@ class ServerMoveWithPortResourceRequestTest(
self._delete_server_and_check_allocations(
qos_port, qos_sriov_port, server)
def test_migrate_server_with_qos_port_reschedule_success(self):
def test_migrate_revert_with_qos_ports(self):
self._test_resize_or_migrate_revert_with_qos_ports()
def test_resize_revert_with_qos_ports(self):
self._test_resize_or_migrate_revert_with_qos_ports(
new_flavor=self.flavor_with_group_policy_bigger)
def _test_resize_or_migrate_server_with_qos_port_reschedule_success(
self, new_flavor=None):
self._start_compute('host3')
compute3_rp_uuid = self._get_provider_uuid_by_host('host3')
self._create_networking_rp_tree('host3', compute3_rp_uuid)
@ -6642,7 +6662,7 @@ class ServerMoveWithPortResourceRequestTest(
# check that the server allocates from the current host properly
self._check_allocation(
server, self.compute1_rp_uuid, non_qos_port, qos_port,
qos_sriov_port)
qos_sriov_port, self.flavor_with_group_policy)
# Yes this isn't great in a functional test, but it's simple.
original_prep_resize = compute_manager.ComputeManager._prep_resize
@ -6664,7 +6684,11 @@ class ServerMoveWithPortResourceRequestTest(
with mock.patch.object(
compute_manager.ComputeManager, '_prep_resize',
new=fake_prep_resize):
self.api.post_server_action(server['id'], {'migrate': None})
if new_flavor:
self.api_fixture.api.post_server_action(
server['id'], {'resize': {"flavorRef": new_flavor['id']}})
else:
self.api.post_server_action(server['id'], {'migrate': None})
self._wait_for_state_change(self.api, server, 'VERIFY_RESIZE')
# ensure that resize is tried on two hosts, so we had a re-schedule
@ -6676,13 +6700,16 @@ class ServerMoveWithPortResourceRequestTest(
# the migration holds the allocation on the source host
self._check_allocation(
server, compute3_rp_uuid, non_qos_port, qos_port, qos_sriov_port,
migration_uuid, source_compute_rp_uuid=self.compute1_rp_uuid)
self.flavor_with_group_policy, migration_uuid,
source_compute_rp_uuid=self.compute1_rp_uuid,
new_flavor=new_flavor)
self._confirm_resize(server)
# check that allocation is still OK
self._check_allocation(
server, compute3_rp_uuid, non_qos_port, qos_port, qos_sriov_port)
server, compute3_rp_uuid, non_qos_port, qos_port, qos_sriov_port,
self.flavor_with_group_policy, new_flavor=new_flavor)
migration_allocations = self.placement_api.get(
'/allocations/%s' % migration_uuid).body['allocations']
self.assertEqual({}, migration_allocations)
@ -6690,7 +6717,15 @@ class ServerMoveWithPortResourceRequestTest(
self._delete_server_and_check_allocations(
qos_port, qos_sriov_port, server)
def test_migrate_server_with_qos_port_reschedule_failure(self):
def test_migrate_server_with_qos_port_reschedule_success(self):
self._test_resize_or_migrate_server_with_qos_port_reschedule_success()
def test_resize_server_with_qos_port_reschedule_success(self):
self._test_resize_or_migrate_server_with_qos_port_reschedule_success(
new_flavor=self.flavor_with_group_policy_bigger)
def _test_resize_or_migrate_server_with_qos_port_reschedule_failure(
self, new_flavor=None):
non_qos_port = self.neutron.port_1
qos_port = self.neutron.port_with_resource_request
qos_sriov_port = self.neutron.port_with_sriov_resource_request
@ -6701,7 +6736,7 @@ class ServerMoveWithPortResourceRequestTest(
# check that the server allocates from the current host properly
self._check_allocation(
server, self.compute1_rp_uuid, non_qos_port, qos_port,
qos_sriov_port)
qos_sriov_port, self.flavor_with_group_policy)
# The patched compute manager on host2 will raise from _prep_resize.
# Then the migration is reschedule but there is no other host to
@ -6710,7 +6745,11 @@ class ServerMoveWithPortResourceRequestTest(
compute_manager.ComputeManager, '_prep_resize',
side_effect=test.TestingException(
'Simulated prep_resize failure.')):
self.api.post_server_action(server['id'], {'migrate': None})
if new_flavor:
self.api_fixture.api.post_server_action(
server['id'], {'resize': {"flavorRef": new_flavor['id']}})
else:
self.api.post_server_action(server['id'], {'migrate': None})
self._wait_for_server_parameter(
self.api, server,
{'OS-EXT-SRV-ATTR:host': 'host1',
@ -6728,7 +6767,14 @@ class ServerMoveWithPortResourceRequestTest(
# and the instance allocates from the source host
self._check_allocation(
server, self.compute1_rp_uuid, non_qos_port, qos_port,
qos_sriov_port)
qos_sriov_port, self.flavor_with_group_policy)
def test_migrate_server_with_qos_port_reschedule_failure(self):
self._test_resize_or_migrate_server_with_qos_port_reschedule_failure()
def test_resize_server_with_qos_port_reschedule_failure(self):
self._test_resize_or_migrate_server_with_qos_port_reschedule_failure(
new_flavor=self.flavor_with_group_policy_bigger)
def test_migrate_server_with_qos_port_pci_update_fail_not_reschedule(self):
self._start_compute('host3')
@ -6745,7 +6791,7 @@ class ServerMoveWithPortResourceRequestTest(
# check that the server allocates from the current host properly
self._check_allocation(
server, self.compute1_rp_uuid, non_qos_port, qos_port,
qos_sriov_port)
qos_sriov_port, self.flavor_with_group_policy)
# The patched compute manager on host2 will raise from
# _update_pci_request_spec_with_allocated_interface_name which will
@ -6790,7 +6836,7 @@ class ServerMoveWithPortResourceRequestTest(
# and the instance allocates from the source host
self._check_allocation(
server, self.compute1_rp_uuid, non_qos_port, qos_port,
qos_sriov_port)
qos_sriov_port, self.flavor_with_group_policy)
class PortResourceRequestReSchedulingTest(

10
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -7591,8 +7591,6 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
'_finish_revert_resize_network_migrate_finish')
@mock.patch('nova.scheduler.utils.'
'fill_provider_mapping_based_on_allocation')
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
'get_allocations_for_consumer')
@mock.patch('nova.compute.manager.ComputeManager._revert_allocation')
@mock.patch.object(objects.Instance, 'save')
@mock.patch('nova.compute.manager.ComputeManager.'
@ -7604,20 +7602,18 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
def test_finish_revert_resize_recalc_group_rp_mapping(
self, mock_get_bdms, mock_notify_action, mock_notify_usage,
mock_set_instance_info, mock_instance_save, mock_revert_allocation,
mock_get_allocations, mock_fill_provider_mapping,
mock_network_migrate_finish, mock_drop_migration_context):
mock_fill_provider_mapping, mock_network_migrate_finish,
mock_drop_migration_context):
mock_get_bdms.return_value = objects.BlockDeviceMappingList()
request_spec = objects.RequestSpec()
mock_get_allocations.return_value = mock.sentinel.allocation
mock_revert_allocation.return_value = mock.sentinel.allocation
with mock.patch.object(
self.compute.network_api, 'get_instance_nw_info'):
self.compute.finish_revert_resize(
self.context, self.instance, self.migration, request_spec)
mock_get_allocations.assert_called_once_with(
self.context, self.instance.uuid)
mock_fill_provider_mapping.assert_called_once_with(
self.context, self.compute.reportclient, request_spec,
mock.sentinel.allocation)

4
releasenotes/notes/support-cold-migrating-neutron-ports-with-resource-request-6d23be654a253625.yaml
View File

@ -1,8 +1,8 @@
---
features:
- |
Cold migration is now supported for servers with neutron ports having
resource requests. E.g. ports that have QoS minimum bandwidth rules
Cold migration and resize is now supported for servers with neutron ports
having resource requests. E.g. ports that have QoS minimum bandwidth rules
attached. Note that the migration is only supported if both the source and
the destination compute services are upgraded to Train and the
``[upgrade_levels]/compute`` configuration does not prevent the computes