openstack
/
nova
Code Issues Proposed changes

Fix volume-backed resize with a smaller disk flavor 

Change I06fad233006c7bab14749a51ffa226c3801f951b in Stein started
calling _validate_flavor_image_nostatus during resize but did not
pass the root_bdm since the underlying volume does not change. This
can, however, lead to FlavorDiskSmallerThanMinDisk being raised
if the new flavor has a disk size smaller than the current flavor.
This is wrong in the case of a volume-backed server because we don't
care about the root disk size in the flavor in that case.

This fixes the bug by splitting the pci/numa validation logic out
of _validate_flavor_image_nostatus into a separate method so that
the resize method can call that directly for a volume-backed server
rather than deal with the complicated disk size logic in
_validate_flavor_image_nostatus (see bug 1646740 for details, but
tl;dr if the image min_disk is less than the flavor root_gb, the
min_disk stored in the instance image system metadata is the
flavor['root_gb'] which could be larger than the root volume size).
To see why trying to use _validate_flavor_image_nostatus during
resize for a volume-backed server is a mess, look at PS1 of this
change.

Change-Id: I91c9c1e88afa035c757f692c78ad72d69cc3c431
Closes-Bug: #1825020
This commit is contained in:
Matt Riedemann 2019-04-16 23:53:03 -04:00
parent a40c7f3e8d
commit 8aae3e39e5
3 changed files with 61 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
nova/compute/api.py
View File

@ -570,8 +570,7 @@ class API(base.Base):
with each other.
:param context: A context.RequestContext
:param image: a dict representation of the image including properties,
enforces the image status is active.
:param image: a dict representation of the image including properties
:param instance_type: Flavor object
:param root_bdm: BlockDeviceMapping for root disk. Will be None for
the resize case.
@ -664,6 +663,30 @@ class API(base.Base):
servers_policies.ZERO_DISK_FLAVOR, fatal=False):
raise exception.BootFromVolumeRequiredForZeroDiskFlavor()
API._validate_flavor_image_numa_pci(
image, instance_type, validate_numa=validate_numa,
validate_pci=validate_pci)
@staticmethod
def _validate_flavor_image_numa_pci(image, instance_type,
validate_numa=True,
validate_pci=False):
"""Validate the flavor and image NUMA/PCI values.
This is called from the API service to ensure that the flavor
extra-specs and image properties are self-consistent and compatible
with each other.
:param image: a dict representation of the image including properties
:param instance_type: Flavor object
:param validate_numa: Flag to indicate whether or not to validate
the NUMA-related metadata.
:param validate_pci: Flag to indicate whether or not to validate
the PCI-related metadata.
:raises: Many different possible exceptions. See
api.openstack.compute.servers.INVALID_FLAVOR_IMAGE_EXCEPTIONS
for the full list.
"""
image_meta = _get_image_meta_obj(image)
# Only validate values of flavor/image so the return results of
@ -3603,6 +3626,7 @@ class API(base.Base):
current_instance_type = instance.get_flavor()
# If flavor_id is not provided, only migrate the instance.
volume_backed = None
if not flavor_id:
LOG.debug("flavor_id is None. Assuming migration.",
instance=instance)
@ -3610,12 +3634,15 @@ class API(base.Base):
else:
new_instance_type = flavors.get_flavor_by_flavor_id(
flavor_id, read_deleted="no")
# Check to see if we're resizing to a zero-disk flavor which is
# only supported with volume-backed servers.
if (new_instance_type.get('root_gb') == 0 and
current_instance_type.get('root_gb') != 0 and
not compute_utils.is_volume_backed_instance(context,
instance)):
reason = _('Resize to zero disk flavor is not allowed.')
raise exception.CannotResizeDisk(reason=reason)
current_instance_type.get('root_gb') != 0):
volume_backed = compute_utils.is_volume_backed_instance(
context, instance)
if not volume_backed:
reason = _('Resize to zero disk flavor is not allowed.')
raise exception.CannotResizeDisk(reason=reason)
if not new_instance_type:
raise exception.FlavorNotFound(flavor_id=flavor_id)
@ -3648,10 +3675,23 @@ class API(base.Base):
if not same_instance_type:
image = utils.get_image_from_system_metadata(
instance.system_metadata)
# Can skip root_bdm check since it will not change during resize.
self._validate_flavor_image_nostatus(
context, image, new_instance_type, root_bdm=None,
validate_pci=True)
# Figure out if the instance is volume-backed but only if we didn't
# already figure that out above (avoid the extra db hit).
if volume_backed is None:
volume_backed = compute_utils.is_volume_backed_instance(
context, instance)
# If the server is volume-backed, we still want to validate numa
# and pci information in the new flavor, but we don't call
# _validate_flavor_image_nostatus because how it handles checking
# disk size validation was not intended for a volume-backed
# resize case.
if volume_backed:
self._validate_flavor_image_numa_pci(
image, new_instance_type, validate_pci=True)
else:
self._validate_flavor_image_nostatus(
context, image, new_instance_type, root_bdm=None,
validate_pci=True)
filter_properties = {'ignore_hosts': []}

16
nova/tests/functional/regressions/test_bug_1825020.py
View File

@ -10,11 +10,8 @@
# License for the specific language governing permissions and limitations
# under the License.
import six
from nova import test
from nova.tests import fixtures as nova_fixtures
from nova.tests.functional.api import client as api_client
from nova.tests.functional import fixtures as func_fixtures
from nova.tests.functional import integrated_helpers
from nova.tests.unit.image import fake as fake_image
@ -73,10 +70,11 @@ class VolumeBackedResizeDiskDown(test.TestCase,
self._wait_for_state_change(self.api, server, 'ACTIVE')
# Now try to resize the server with the flavor that has smaller disk.
# This should be allowed since the server is volume-backed and the
# disk size in the flavor shouldn't matter.
data = {'resize': {'flavorRef': flavor1['id']}}
# FIXME(mriedem): This will raise FlavorDiskSmallerThanMinDisk as a 500
# error until bug 1825020 is fixed.
ex = self.assertRaises(api_client.OpenStackApiException,
self.api.post_server_action, server['id'], data)
self.assertEqual(500, ex.response.status_code)
self.assertIn('FlavorDiskSmallerThanMinDisk', six.text_type(ex))
self.api.post_server_action(server['id'], data)
self._wait_for_state_change(self.api, server, 'VERIFY_RESIZE')
# Now confirm the resize just to complete the operation.
self.api.post_server_action(server['id'], {'confirmResize': None})
self._wait_for_state_change(self.api, server, 'ACTIVE')

4
nova/tests/unit/compute/test_compute_api.py
View File

@ -1932,6 +1932,8 @@ class _ComputeAPIUnitTestMixIn(object):
self.context, fake_inst['uuid'], 'finished')
mock_inst_save.assert_called_once_with(expected_task_state=[None])
@mock.patch('nova.compute.utils.is_volume_backed_instance',
return_value=False)
@mock.patch('nova.compute.api.API._validate_flavor_image_nostatus')
@mock.patch('nova.objects.Migration')
@mock.patch.object(compute_api.API, '_record_action_start')
@ -1945,7 +1947,7 @@ class _ComputeAPIUnitTestMixIn(object):
def _test_resize(self, mock_get_all_by_host,
mock_get_by_instance_uuid, mock_get_flavor, mock_upsize,
mock_inst_save, mock_count, mock_limit, mock_record,
mock_migration, mock_validate,
mock_migration, mock_validate, mock_is_vol_backed,
flavor_id_passed=True,
same_host=False, allow_same_host=False,
project_id=None,