xenapi: add support for auto_disk_config=disabled

Previously, only True and False were supported. This change allows an
administrator to specify which images should reject any user requests
for DiskConfig=AUTO, i.e. reject the request to automatically resize
that instance's root disk.

To specify that an image's disk can't be resized, add the following
property to the image:
auto_disk_config=disabled

In addition, when uploading a snapshot we need to ensure that if
auto_disk_config is "disabled", that property is inherited by any
snapshot. Currently, the disk_config from the database, which is always
False in the case of "disabled", would be persisted instead. When
auto_disk_config is not "disabled", then the current True or False value
for that server should continue to be persisted in any snapshot, and not
the value stored in the source image for that server.

DocImpact
Implements blueprint auto-disk-config-disabled
Change-Id: Ifd41886b9bc7dff01cdf741a833946bed1bdddc3

nova/compute/api.py

@@ -505,31 +505,29 @@ class API(base.Base):
 
         return availability_zone, forced_host, forced_node
 
-    @staticmethod
-    def _inherit_properties_from_image(image, auto_disk_config):
+    def _ensure_auto_disk_config_is_valid(self, auto_disk_config_img,
+                                          auto_disk_config, image):
+        auto_disk_config_disabled = \
+                utils.is_auto_disk_config_disabled(auto_disk_config_img)
+        if auto_disk_config_disabled and auto_disk_config:
+            raise exception.AutoDiskConfigDisabledByImage(image=image)
+
+    def _inherit_properties_from_image(self, image, auto_disk_config):
         image_properties = image.get('properties', {})
-
-        def prop(prop_, prop_type=None):
-            """Return the value of an image property."""
-            value = image_properties.get(prop_)
-
-            if value is not None:
-                if prop_type == 'bool':
-                    value = strutils.bool_from_string(value)
-
-            return value
-
-        options_from_image = {'os_type': prop('os_type'),
-                              'architecture': prop('architecture'),
-                              'vm_mode': prop('vm_mode')}
-
-        # If instance doesn't have auto_disk_config overridden by request, use
-        # whatever the image indicates
+        auto_disk_config_img = \
+                utils.get_auto_disk_config_from_image_props(image_properties)
+        self._ensure_auto_disk_config_is_valid(auto_disk_config_img,
+                                               auto_disk_config,
+                                               image.get("id"))
         if auto_disk_config is None:
-            auto_disk_config = prop('auto_disk_config', prop_type='bool')
+            auto_disk_config = strutils.bool_from_string(auto_disk_config_img)
 
-        options_from_image['auto_disk_config'] = auto_disk_config
-        return options_from_image
+        return {
+            'os_type': image_properties.get('os_type'),
+            'architecture': image_properties.get('architecture'),
+            'vm_mode': image_properties.get('vm_mode'),
+            'auto_disk_config': auto_disk_config
+        }
 
     def _apply_instance_name_template(self, context, instance, index):
         params = {
@@ -838,6 +836,9 @@ class API(base.Base):
                 self._get_bdm_image_metadata(context,
                     block_device_mapping, legacy_bdm)
 
+        self._check_auto_disk_config(image=boot_meta,
+                                     auto_disk_config=auto_disk_config)
+
         handle_az = self._handle_availability_zone
         availability_zone, forced_host, forced_node = handle_az(
                                                             availability_zone)
@@ -1222,6 +1223,30 @@ class API(base.Base):
 
         return dict(old_ref.iteritems()), dict(instance_ref.iteritems())
 
+    def _check_auto_disk_config(self, instance=None, image=None,
+                                **extra_instance_updates):
+        auto_disk_config = extra_instance_updates.get("auto_disk_config")
+        if auto_disk_config is None:
+            return
+        if not image and not instance:
+            return
+
+        if image:
+            image_props = image.get("properties", {})
+            LOG.error(image_props)
+            auto_disk_config_img = \
+                utils.get_auto_disk_config_from_image_props(image_props)
+            image_ref = image.get("id")
+        else:
+            sys_meta = utils.instance_sys_meta(instance)
+            image_ref = sys_meta.get('image_base_image_ref')
+            auto_disk_config_img = \
+                utils.get_auto_disk_config_from_instance(sys_meta=sys_meta)
+
+        self._ensure_auto_disk_config_is_valid(auto_disk_config_img,
+                                               auto_disk_config,
+                                               image_ref)
+
     def _delete(self, context, instance, delete_type, cb, **instance_attrs):
         if instance['disable_terminate']:
             LOG.info(_('instance termination disabled'),
@@ -1970,10 +1995,11 @@ class API(base.Base):
         orig_image_ref = instance['image_ref'] or ''
         files_to_inject = kwargs.pop('files_to_inject', [])
         metadata = kwargs.get('metadata', {})
-        instance_type = flavors.extract_flavor(instance)
 
         image_id, image = self._get_image(context, image_href)
+        self._check_auto_disk_config(image=image, **kwargs)
 
+        instance_type = flavors.extract_flavor(instance)
         self._checks_for_create_and_rebuild(context, image_id, image,
                 instance_type, metadata, files_to_inject)
 
@@ -2203,6 +2229,8 @@ class API(base.Base):
         the original flavor_id. If flavor_id is not None, the instance should
         be migrated to a new host and resized to the new flavor_id.
         """
+        self._check_auto_disk_config(instance, **extra_instance_updates)
+
         current_instance_type = flavors.extract_flavor(instance)
 
         # If flavor_id is not provided, only migrate the instance.

nova/exception.py

@@ -512,6 +512,11 @@ class InvalidImageRef(Invalid):
     msg_fmt = _("Invalid image href %(image_href)s.")
 
 
+class AutoDiskConfigDisabledByImage(Invalid):
+    msg_fmt = _("Requested image %(image)s "
+                "has automatic disk resize disabled.")
+
+
 class ImageNotFound(NotFound):
     msg_fmt = _("Image %(image_id)s could not be found.")
 

nova/tests/api/openstack/compute/contrib/test_disk_config.py

@@ -47,7 +47,7 @@ class DiskConfigTestCase(test.TestCase):
             osapi_compute_extension=[
                 'nova.api.openstack.compute.contrib.select_extensions'],
             osapi_compute_ext_list=['Disk_config'])
-        nova.tests.image.fake.stub_out_image_service(self.stubs)
+        self._setup_fake_image_service()
 
         fakes.stub_out_nw_api(self.stubs)
 
@@ -123,6 +123,24 @@ class DiskConfigTestCase(test.TestCase):
 
         self.app = compute.APIRouter(init_only=('servers', 'images'))
 
+    def _setup_fake_image_service(self):
+        self.image_service = nova.tests.image.fake.stub_out_image_service(
+                self.stubs)
+        timestamp = datetime.datetime(2011, 1, 1, 1, 2, 3)
+        image = {'id': '88580842-f50a-11e2-8d3a-f23c91aec05e',
+                 'name': 'fakeimage7',
+                 'created_at': timestamp,
+                 'updated_at': timestamp,
+                 'deleted_at': None,
+                 'deleted': False,
+                 'status': 'active',
+                 'is_public': False,
+                 'container_format': 'ova',
+                 'disk_format': 'vhd',
+                 'size': '74185822',
+                 'properties': {'auto_disk_config': 'Disabled'}}
+        self.image_service.create(None, image)
+
     def tearDown(self):
         super(DiskConfigTestCase, self).tearDown()
         nova.tests.image.fake.FakeImageService_reset()
@@ -242,6 +260,52 @@ class DiskConfigTestCase(test.TestCase):
         server_dict = jsonutils.loads(res.body)['server']
         self.assertDiskConfig(server_dict, 'AUTO')
 
+    def test_create_server_detect_from_image_disabled_goes_to_manual(self):
+        req = fakes.HTTPRequest.blank('/fake/servers')
+        req.method = 'POST'
+        req.content_type = 'application/json'
+        body = {'server': {
+                  'name': 'server_test',
+                  'imageRef': '88580842-f50a-11e2-8d3a-f23c91aec05e',
+                  'flavorRef': '1',
+               }}
+
+        req.body = jsonutils.dumps(body)
+        res = req.get_response(self.app)
+        server_dict = jsonutils.loads(res.body)['server']
+        self.assertDiskConfig(server_dict, 'MANUAL')
+
+    def test_create_server_errors_when_disabled_and_auto(self):
+        req = fakes.HTTPRequest.blank('/fake/servers')
+        req.method = 'POST'
+        req.content_type = 'application/json'
+        body = {'server': {
+                  'name': 'server_test',
+                  'imageRef': '88580842-f50a-11e2-8d3a-f23c91aec05e',
+                  'flavorRef': '1',
+                  API_DISK_CONFIG: 'AUTO'
+               }}
+
+        req.body = jsonutils.dumps(body)
+        res = req.get_response(self.app)
+        self.assertEqual(res.status_int, 400)
+
+    def test_create_server_when_disabled_and_manual(self):
+        req = fakes.HTTPRequest.blank('/fake/servers')
+        req.method = 'POST'
+        req.content_type = 'application/json'
+        body = {'server': {
+                  'name': 'server_test',
+                  'imageRef': '88580842-f50a-11e2-8d3a-f23c91aec05e',
+                  'flavorRef': '1',
+                  API_DISK_CONFIG: 'MANUAL'
+               }}
+
+        req.body = jsonutils.dumps(body)
+        res = req.get_response(self.app)
+        server_dict = jsonutils.loads(res.body)['server']
+        self.assertDiskConfig(server_dict, 'MANUAL')
+
     def test_update_server_invalid_disk_config(self):
         # Return BadRequest if user passes an invalid diskConfig value.
         req = fakes.HTTPRequest.blank(

nova/tests/compute/test_compute.py

@@ -252,7 +252,7 @@ class BaseTestCase(test.TestCase):
             params = {}
 
         def make_fake_sys_meta():
-            sys_meta = {}
+            sys_meta = params.pop("system_metadata", {})
             inst_type = flavors.get_flavor_by_name(type_name)
             for key in flavors.system_metadata_flavor_props:
                 sys_meta['instance_type_%s' % key] = inst_type[key]

nova/tests/compute/test_compute_api.py

@@ -35,6 +35,7 @@ from nova.openstack.common import timeutils
 from nova.openstack.common import uuidutils
 from nova import quota
 from nova import test
+from nova.tests.image import fake as fake_image
 from nova.tests.objects import test_migration
 
 
@@ -78,7 +79,7 @@ class _ComputeAPIUnitTestMixIn(object):
             flavor = self._create_flavor()
 
         def make_fake_sys_meta():
-            sys_meta = {}
+            sys_meta = params.pop("system_metadata", {})
             for key in flavors.system_metadata_flavor_props:
                 sys_meta['instance_type_%s' % key] = flavor[key]
             return sys_meta
@@ -1382,6 +1383,52 @@ class _ComputeAPIUnitTestMixIn(object):
         self.compute_api.volume_snapshot_delete(self.context, volume_id,
                 snapshot_id, {})
 
+    def _create_instance_with_disabled_disk_config(self):
+        sys_meta = {"image_auto_disk_config": "Disabled"}
+        params = {"system_metadata": sys_meta}
+        return obj_base.obj_to_primitive(self._create_instance_obj(
+                                                            params=params))
+
+    def _setup_fake_image_with_disabled_disk_config(self):
+        self.fake_image = {
+            'id': 1,
+            'name': 'fake_name',
+            'status': 'active',
+            'properties': {"auto_disk_config": "Disabled"},
+        }
+
+        def fake_show(obj, context, image_id):
+            return self.fake_image
+        fake_image.stub_out_image_service(self.stubs)
+        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        return self.fake_image['id']
+
+    def test_resize_with_disabled_auto_disk_config_fails(self):
+        fake_inst = self._create_instance_with_disabled_disk_config()
+
+        self.assertRaises(exception.AutoDiskConfigDisabledByImage,
+                          self.compute_api.resize,
+                          self.context, fake_inst,
+                          auto_disk_config=True)
+
+    def test_create_with_disabled_auto_disk_config_fails(self):
+        image_id = self._setup_fake_image_with_disabled_disk_config()
+
+        self.assertRaises(exception.AutoDiskConfigDisabledByImage,
+            self.compute_api.create, self.context,
+            "fake_flavor", image_id, auto_disk_config=True)
+
+    def test_rebuild_with_disabled_auto_disk_config_fails(self):
+        fake_inst = self._create_instance_with_disabled_disk_config()
+        image_id = self._setup_fake_image_with_disabled_disk_config()
+        self.assertRaises(exception.AutoDiskConfigDisabledByImage,
+                          self.compute_api.rebuild,
+                          self.context,
+                          fake_inst,
+                          image_id,
+                          "new password",
+                          auto_disk_config=True)
+
 
 class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
     def setUp(self):

nova/tests/image/fake.py

@@ -249,7 +249,9 @@ def get_valid_image_id():
 
 
 def stub_out_image_service(stubs):
+    image_service = FakeImageService()
     stubs.Set(nova.image.glance, 'get_remote_image_service',
-              lambda x, y: (FakeImageService(), y))
+              lambda x, y: (image_service, y))
     stubs.Set(nova.image.glance, 'get_default_image_service',
-              lambda: FakeImageService())
+              lambda: image_service)
+    return image_service

nova/tests/test_utils.py

@@ -946,3 +946,14 @@ class ValidateNeutronConfiguration(test.TestCase):
     def test_quantum(self):
         self.flags(network_api_class='nova.network.quantumv2.api.API')
         self.assertTrue(utils.is_neutron())
+
+
+class AutoDiskConfigUtilTestCase(test.TestCase):
+    def test_is_auto_disk_config_disabled(self):
+        self.assertTrue(utils.is_auto_disk_config_disabled("Disabled "))
+
+    def test_is_auto_disk_config_disabled_none(self):
+        self.assertFalse(utils.is_auto_disk_config_disabled(None))
+
+    def test_is_auto_disk_config_disabled_false(self):
+        self.assertFalse(utils.is_auto_disk_config_disabled("false"))

nova/tests/virt/xenapi/image/test_glance.py

@@ -86,15 +86,15 @@ class TestGlanceStore(stubs.XenAPITestBase):
 
         self.mox.VerifyAll()
 
-    def _get_upload_params(self):
+    def _get_upload_params(self, auto_disk_config=True):
         params = self._get_params()
         params['vdi_uuids'] = ['fake_vdi_uuid']
-        params['properties'] = {'auto_disk_config': True,
+        params['properties'] = {'auto_disk_config': auto_disk_config,
                                 'os_type': 'default'}
         return params
 
-    def test_upload_image(self):
-        params = self._get_upload_params()
+    def _test_upload_image(self, auto_disk_config):
+        params = self._get_upload_params(auto_disk_config)
 
         self.mox.StubOutWithMock(self.session, 'call_plugin_serialized')
         self.session.call_plugin_serialized('glance', 'upload_vhd', **params)
@@ -104,6 +104,14 @@ class TestGlanceStore(stubs.XenAPITestBase):
                                 ['fake_vdi_uuid'], 'fake_image_uuid')
         self.mox.VerifyAll()
 
+    def test_upload_image(self):
+        self._test_upload_image(True)
+
+    def test_upload_image_auto_config_disk_disabled(self):
+        sys_meta = [{"key": "image_auto_disk_config", "value": "Disabled"}]
+        self.instance["system_metadata"] = sys_meta
+        self._test_upload_image("disabled")
+
     def test_upload_image_raises_exception(self):
         params = self._get_upload_params()
 

nova/utils.py

@@ -1197,3 +1197,22 @@ def reset_is_neutron():
 
     _IS_NEUTRON_ATTEMPTED = False
     _IS_NEUTRON = False
+
+
+def is_auto_disk_config_disabled(auto_disk_config_raw):
+    auto_disk_config_disabled = False
+    if auto_disk_config_raw is not None:
+        adc_lowered = auto_disk_config_raw.strip().lower()
+        if adc_lowered == "disabled":
+            auto_disk_config_disabled = True
+    return auto_disk_config_disabled
+
+
+def get_auto_disk_config_from_instance(instance=None, sys_meta=None):
+    if sys_meta is None:
+        sys_meta = instance_sys_meta(instance)
+    return sys_meta.get("image_auto_disk_config")
+
+
+def get_auto_disk_config_from_image_props(image_properties):
+    return image_properties.get("auto_disk_config")

nova/virt/xenapi/image/glance.py

@@ -17,6 +17,7 @@ from oslo.config import cfg
 
 from nova import exception
 from nova.image import glance
+from nova import utils
 from nova.virt.xenapi import vm_utils
 
 CONF = cfg.CONF
@@ -64,6 +65,10 @@ class GlanceStore(object):
         if compression_level:
             props['xenapi_image_compression_level'] = compression_level
 
+        auto_disk_config = utils.get_auto_disk_config_from_instance(instance)
+        if utils.is_auto_disk_config_disabled(auto_disk_config):
+            props["auto_disk_config"] = "disabled"
+
         try:
             self._call_glance_plugin(session, 'upload_vhd', params)
         except exception.PluginRetriesExceeded: