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
(cherry picked from commit 8aae3e39e5
)
This commit is contained in:
parent
a1db4507ac
commit
3285210add
|
@ -571,8 +571,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.
|
||||
|
@ -665,6 +664,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
|
||||
|
@ -3623,6 +3646,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)
|
||||
|
@ -3630,12 +3654,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)
|
||||
|
@ -3668,10 +3695,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': []}
|
||||
|
||||
|
|
|
@ -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')
|
||||
|
|
|
@ -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,
|
||||
|
|
Loading…
Reference in New Issue