Remove broken config_drive image_href support.

image_href support has not been working since at least shortly before
Folsom release.  This is a good indication that this functionality is not
used.  As far as I can tell, the docs also do not match what was
supported.  An image ID was required, but docs show examples with full
hrefs.

DocImpact

http://docs.openstack.org/developer/nova/api_ext/ext_config_drive.html

References to supporting image_hrefs should be removed.

This patch also removes the hack that passed a config_drive_id via
the Instance dictionary when config_drive_id is not a valid Column for
Instance.

Fixes bug 1186401

Change-Id: Iced7bc8e278cb9f208183f1dbb7a293675a47eae

9 files changed, 56 insertions, 95 deletions

diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py
index b9e18b501..8c329d12b 100644
--- a/nova/api/openstack/compute/servers.py
+++ b/nova/api/openstack/compute/servers.py
@@ -919,6 +919,9 @@ class Controller(wsgi.Controller):
         except exception.KeypairNotFound as error:
             msg = _("Invalid key_name provided.")
             raise exc.HTTPBadRequest(explanation=msg)
+        except exception.ConfigDriveInvalidValue:
+            msg = _("Invalid config_drive provided.")
+            raise exc.HTTPBadRequest(explanation=msg)
         except rpc_common.RemoteError as err:
             msg = "%(err_type)s: %(err_msg)s" % {'err_type': err.exc_type,
                                                  'err_msg': err.value}
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 70e205dc2..48d404148 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -496,21 +496,20 @@ class API(base.Base):
                 instance['uuid'], updates)
         return instance
 
-    def _check_config_drive(self, context, config_drive):
-        try:
-            bool_like = strutils.bool_from_string(config_drive, strict=True)
-        except ValueError:
-            bool_like = False
-
-        if config_drive is None:
-            return None, None
-        elif bool_like:
-            return None, bool_like
+    def _check_config_drive(self, config_drive):
+        if config_drive:
+            try:
+                bool_val = strutils.bool_from_string(config_drive,
+                                                     strict=True)
+            except ValueError:
+                raise exception.ConfigDriveInvalidValue(option=config_drive)
         else:
-            cd_image_service, config_drive_id = \
-                glance.get_remote_image_service(context, config_drive)
-            cd_image_service.show(context, config_drive_id)
-            return config_drive_id, None
+            bool_val = False
+        # FIXME(comstud):  Bug ID 1193438 filed for this. This looks silly,
+        # but this is because the config drive column is a String.  False
+        # is represented by using an empty string.  And for whatever
+        # reason, we rely on the DB to cast True to a String.
+        return True if bool_val else ''
 
     def _check_requested_image(self, context, image_id, image, instance_type):
         if not image:
@@ -598,8 +597,7 @@ class API(base.Base):
         kernel_id, ramdisk_id = self._handle_kernel_and_ramdisk(
                 context, kernel_id, ramdisk_id, image)
 
-        config_drive_id, config_drive = self._check_config_drive(
-            context, config_drive)
+        config_drive = self._check_config_drive(config_drive)
 
         if key_data is None and key_name:
             key_pair = self.db.key_pair_get(context, context.user_id,
@@ -619,8 +617,7 @@ class API(base.Base):
             'ramdisk_id': ramdisk_id or '',
             'power_state': power_state.NOSTATE,
             'vm_state': vm_states.BUILDING,
-            'config_drive_id': config_drive_id or '',
-            'config_drive': config_drive or '',
+            'config_drive': config_drive,
             'user_id': context.user_id,
             'project_id': context.project_id,
             'instance_type_id': instance_type['id'],
diff --git a/nova/exception.py b/nova/exception.py
index e5924b831..4fdb727d6 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -1124,6 +1124,10 @@ class InstanceIsLocked(InstanceInvalidState):
     message = _("Instance %(instance_uuid)s is locked")
 
 
+class ConfigDriveInvalidValue(Invalid):
+    message = _("Invalid value for Config Drive option: %(option)s")
+
+
 class ConfigDriveMountFailed(NovaException):
     message = _("Could not mount vfat config drive. %(operation)s failed. "
                 "Error: %(error)s")
diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py
index 8eebec613..82bb6b868 100644
--- a/nova/tests/api/openstack/compute/test_servers.py
+++ b/nova/tests/api/openstack/compute/test_servers.py
@@ -3253,34 +3253,8 @@ class ServersControllerCreateTest(test.TestCase):
         server = res['server']
         self.assertEqual(FAKE_UUID, server['id'])
 
-    def test_create_instance_with_config_drive_as_id(self):
-        self.ext_mgr.extensions = {'os-config-drive': 'fake'}
-        image_href = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6'
-        flavor_ref = 'http://localhost/v2/fake/flavors/3'
-        body = {
-            'server': {
-                'name': 'config_drive_test',
-                'imageRef': image_href,
-                'flavorRef': flavor_ref,
-                'metadata': {
-                    'hello': 'world',
-                    'open': 'stack',
-                },
-                'personality': {},
-                'config_drive': image_href,
-            },
-        }
-
-        req = fakes.HTTPRequest.blank('/fake/servers')
-        req.method = 'POST'
-        req.body = jsonutils.dumps(body)
-        req.headers["content-type"] = "application/json"
-        res = self.controller.create(req, body).obj
-
-        server = res['server']
-        self.assertEqual(FAKE_UUID, server['id'])
-
     def test_create_instance_with_bad_config_drive(self):
+        # Test with an image href as config drive value.
         self.ext_mgr.extensions = {'os-config-drive': 'fake'}
         image_href = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6'
         flavor_ref = 'http://localhost/v2/fake/flavors/3'
@@ -3294,7 +3268,7 @@ class ServersControllerCreateTest(test.TestCase):
                     'open': 'stack',
                 },
                 'personality': {},
-                'config_drive': 'asdf',
+                'config_drive': image_href,
             },
         }
 
@@ -3302,7 +3276,6 @@ class ServersControllerCreateTest(test.TestCase):
         req.method = 'POST'
         req.body = jsonutils.dumps(body)
         req.headers["content-type"] = "application/json"
-
         self.assertRaises(webob.exc.HTTPBadRequest,
                           self.controller.create, req, body)
 
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index e2783641c..00469ae30 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -9592,42 +9592,31 @@ class CheckConfigDriveTestCase(test.TestCase):
     def setUp(self):
         super(CheckConfigDriveTestCase, self).setUp()
         self.compute_api = compute.API()
-        self.context = context.RequestContext(
-                'fake_user_id', 'fake_project_id')
-
-        self.called = called = {'show': False}
-
-        def fake_get_remote_image_service(context, image_id):
-            class FakeGlance(object):
-                def show(self, context, image_id):
-                    called['show'] = True
-
-            return FakeGlance(), image_id
-
-        self.stubs.Set(glance, 'get_remote_image_service',
-                       fake_get_remote_image_service)
 
-    def tearDown(self):
-        self.stubs.UnsetAll()
-        super(CheckConfigDriveTestCase, self).tearDown()
-
-    def assertCheck(self, expected, config_drive):
+    def _assertCheck(self, expected, config_drive):
         self.assertEqual(expected,
-                         self.compute_api._check_config_drive(
-                             self.context, config_drive))
-
-    def test_value_is_none(self):
-        self.assertFalse(self.called['show'])
-        self.assertCheck((None, None), None)
-        self.assertFalse(self.called['show'])
-
-    def test_bool_string_or_id(self):
-        self.assertCheck((None, True), "true")
-        self.assertCheck((None, True), 1)
-        self.assertCheck((None, True), 't')
-
-    def test_value_is_image_id(self):
-        self.assertCheck(("fake-uuid", None), "fake-uuid")
+                         self.compute_api._check_config_drive(config_drive))
+
+    def _assertInvalid(self, config_drive):
+        self.assertRaises(exception.ConfigDriveInvalidValue,
+                          self.compute_api._check_config_drive,
+                          config_drive)
+
+    def test_config_drive_false_values(self):
+        self._assertCheck('', None)
+        self._assertCheck('', '')
+        self._assertCheck('', 'False')
+        self._assertCheck('', 'f')
+        self._assertCheck('', '0')
+
+    def test_config_drive_true_values(self):
+        self._assertCheck(True, 'True')
+        self._assertCheck(True, 't')
+        self._assertCheck(True, '1')
+
+    def test_config_drive_bogus_values_raise(self):
+        self._assertInvalid('asd')
+        self._assertInvalid(uuidutils.generate_uuid())
 
 
 class CheckRequestedImageTestCase(test.TestCase):
diff --git a/nova/tests/integrated/test_api_samples.py b/nova/tests/integrated/test_api_samples.py
index 380b69079..fb7e197cc 100644
--- a/nova/tests/integrated/test_api_samples.py
+++ b/nova/tests/integrated/test_api_samples.py
@@ -3412,18 +3412,18 @@ class ConfigDriveSampleJsonTest(ServersSampleBase):
         response = self._do_get('servers/%s' % uuid)
         subs = self._get_regexes()
         subs['hostid'] = '[a-f0-9]+'
-        # config drive can be an uuid or empty value
-        subs['cdrive'] = '(%s)?' % subs['uuid']
+        # config drive can be a string for True or empty value for False
+        subs['cdrive'] = '.*'
         self._verify_response('server-config-drive-get-resp', subs,
                               response, 200)
 
     def test_config_drive_detail(self):
-        uuid = self._post_server()
+        self._post_server()
         response = self._do_get('servers/detail')
         subs = self._get_regexes()
         subs['hostid'] = '[a-f0-9]+'
-        # config drive can be an uuid or empty value
-        subs['cdrive'] = '(%s)?' % subs['uuid']
+        # config drive can be a string for True or empty value for False
+        subs['cdrive'] = '.*'
         self._verify_response('servers-config-drive-details-resp',
                               subs, response, 200)
 
diff --git a/nova/tests/virt/libvirt/test_libvirt.py b/nova/tests/virt/libvirt/test_libvirt.py
index 3c658a7f5..d3b842c0b 100644
--- a/nova/tests/virt/libvirt/test_libvirt.py
+++ b/nova/tests/virt/libvirt/test_libvirt.py
@@ -589,8 +589,8 @@ class LibvirtConnTestCase(test.TestCase):
         conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
         instance_ref = db.instance_create(self.context, self.test_instance)
 
-        # make configdrive.enabled_for() return True
-        instance_ref['config_drive'] = 'ANY_ID'
+        # make configdrive.required_by() return True
+        instance_ref['config_drive'] = True
 
         disk_info = blockinfo.get_disk_info(CONF.libvirt_type,
                                             instance_ref)
@@ -4945,10 +4945,9 @@ class LibvirtDriverTestCase(test.TestCase):
         inst['host'] = 'host1'
         inst['root_gb'] = 10
         inst['ephemeral_gb'] = 20
-        inst['config_drive'] = 1
+        inst['config_drive'] = True
         inst['kernel_id'] = 2
         inst['ramdisk_id'] = 3
-        inst['config_drive_id'] = 1
         inst['key_data'] = 'ABCDEFG'
         inst['system_metadata'] = sys_meta
 
diff --git a/nova/virt/configdrive.py b/nova/virt/configdrive.py
index 173dd457b..d4038457f 100644
--- a/nova/virt/configdrive.py
+++ b/nova/virt/configdrive.py
@@ -179,7 +179,3 @@ class ConfigDriveBuilder(object):
 
 def required_by(instance):
     return instance.get('config_drive') or CONF.force_config_drive
-
-
-def enabled_for(instance):
-    return required_by(instance) or instance.get('config_drive_id')
diff --git a/nova/virt/libvirt/blockinfo.py b/nova/virt/libvirt/blockinfo.py
index aabcef964..55bf4fcd1 100644
--- a/nova/virt/libvirt/blockinfo.py
+++ b/nova/virt/libvirt/blockinfo.py
@@ -411,7 +411,7 @@ def get_disk_mapping(virt_type, instance,
                                         'dev': disk_dev,
                                         'type': 'disk'}
 
-    if configdrive.enabled_for(instance):
+    if configdrive.required_by(instance):
         config_info = get_next_disk_info(mapping,
                                          disk_bus,
                                          last_device=True)