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
This commit is contained in:
parent
d62e708889
commit
b013d80ff7
|
@ -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}
|
||||
|
|
|
@ -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'],
|
||||
|
|
|
@ -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")
|
||||
|
|
|
@ -3253,7 +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):
|
||||
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'
|
||||
|
@ -3275,34 +3276,6 @@ class ServersControllerCreateTest(test.TestCase):
|
|||
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):
|
||||
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': 'asdf',
|
||||
},
|
||||
}
|
||||
|
||||
req = fakes.HTTPRequest.blank('/fake/servers')
|
||||
req.method = 'POST'
|
||||
req.body = jsonutils.dumps(body)
|
||||
req.headers["content-type"] = "application/json"
|
||||
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.create, req, body)
|
||||
|
||||
|
|
|
@ -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))
|
||||
self.compute_api._check_config_drive(config_drive))
|
||||
|
||||
def test_value_is_none(self):
|
||||
self.assertFalse(self.called['show'])
|
||||
self.assertCheck((None, None), None)
|
||||
self.assertFalse(self.called['show'])
|
||||
def _assertInvalid(self, config_drive):
|
||||
self.assertRaises(exception.ConfigDriveInvalidValue,
|
||||
self.compute_api._check_config_drive,
|
||||
config_drive)
|
||||
|
||||
def test_bool_string_or_id(self):
|
||||
self.assertCheck((None, True), "true")
|
||||
self.assertCheck((None, True), 1)
|
||||
self.assertCheck((None, True), 't')
|
||||
def test_config_drive_false_values(self):
|
||||
self._assertCheck('', None)
|
||||
self._assertCheck('', '')
|
||||
self._assertCheck('', 'False')
|
||||
self._assertCheck('', 'f')
|
||||
self._assertCheck('', '0')
|
||||
|
||||
def test_value_is_image_id(self):
|
||||
self.assertCheck(("fake-uuid", None), "fake-uuid")
|
||||
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):
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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')
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue