Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-04-03
19:38:08 mriedem *driven
19:38:40 mordred \o/
19:38:52 mriedem were you surprised that it was a cold snapshot? i see we don't mention that in the api ref
19:39:30 mordred well - I shouldn't have been - I know we don't use snapshots for any purpose in infra because it used to make the servers unresponsive for potentially hours - but I think I had just assumed that was some Xen/Rackspace thing
19:39:44 mordred but yes - I was totally unprepared for it to be a COMPLETELY unsafe thing to run
19:40:08 mordred that left me with a VM in a down state for an unknowable period of time
19:40:28 mordred and with no way that I could find for me to say "oh, crap, dude, nevermind - I need the VM back please"
19:40:58 mriedem build a confirm prompt into shade :)
19:41:26 mriedem we could note this in the docs though, that by default it's a cold snapshot so you should expect the guest to be offline during the snapshot operation
19:41:36 mriedem i'm not really sure if that's true for the other virt drivers, but it is for libvirt
19:42:03 mriedem sdague: didn't you have a patch to undo disable_libvirt_livesnapshot?
19:42:06 mriedem or change it to False
19:42:10 mriedem maybe it was sfinucan
19:42:13 mordred mriedem: yah - I think a docs patch with a warning would be a good idea
19:42:14 openstackgerrit Sihan Wang proposed openstack/nova master: VMware:fix get VM root device conditional verify https://review.openstack.org/402256
19:42:32 mriedem mordred: we can also doc the image location header response as probably being bogus for most users, at least users of public clouds
19:42:36 mordred ++
19:43:44 sdague I did change it to false, I thought we reverted it though
19:44:33 sdague oh, I guess it's still off by default
19:44:47 sdague we should probably change that
19:46:45 openstack Launchpad bug 1679285 in OpenStack Compute (nova) "Create Server Snapshot returns incorrect Image Location" [Undecided,New]
19:46:45 mordred mriedem: https://bugs.launchpad.net/nova/+bug/1679285
19:47:00 mriedem thanks
19:51:00 mriedem so how to fix this, i'm not sure if we just do the service catalog thing for this right now, or change the response in a microversion (which we can't backport)
19:51:08 mriedem it's super latent so i'm not sure we need to backport a fix,
19:51:11 mriedem we could just doc it as always sucking
19:53:45 mriedem sdague: ^ thoughts on which way you'd lean?
19:54:14 mriedem i'm not as clear on the service catalog stuff but could probably figure it out by looking at similar things with placement
19:54:34 sdague well, it makes it clear that once we get to newton, we probably break people by accident if they used that field
19:54:57 mriedem because it doesn't have v2 in the url?
19:55:02 mriedem it doesn't have v1 in it either
19:55:02 mordred mriedem: for my money, I'd document it, and then add the microversion for a new response type ... I think the catalog thing is a _much_ longer fix, even though it's the right one long term
19:55:04 sdague the Location: field really is supposed to have the url
19:55:13 mriedem unless images is an alias for v1/images
19:55:24 mordred yah - I think there are too many variables for Location to be correct consistently
19:55:34 mordred because you might be an admin who _does_ want to use the internal url
19:55:44 mordred and you might have requested internal urls from keystone as a user
19:55:46 sdague Location is HTTP spec, if we are going to use it, it has to be a real url
19:56:06 mordred yah. I'm saying ignore the location header and just return the image id in a json doc
19:56:24 mordred that's consistent and can be consumed well
19:56:25 mriedem right it's a question of change the response, or fix the location header and use the catalog
19:56:32 mordred oh - I was suggesting neither ...
19:56:35 mriedem i can do the former easily
19:56:51 mordred just _Add_ a json return dict and document that the location header is likely broken
19:56:52 mriedem mordred: you're saying leave the busted location header?
19:56:57 mriedem oh
19:57:07 mriedem well it's still a microversion to return a body now
19:57:11 mordred yah
19:57:23 mordred but microversions are there for a reason, right?
19:57:51 mriedem yeah, i just don't know if it's better to do the quick fix with a microversion now, or invest in doing the catalog lookup, which is more obscure for me personally
19:58:04 mriedem ksa is just a big black box and i always ask sean what to do :)
19:58:19 mriedem or copy his code
19:58:21 mordred well - I have a patch coming to maybe make it friendlier
19:59:07 sdague mriedem: yeh, honestly, I would say a better response body is probably the right answer
19:59:13 sdague and drop the Location header when we do it
19:59:20 mriedem sdague: i can totally do that, today
19:59:38 mordred ++ - I still think fixing glance interactoins to use catalog long term is the correct and healthy thing to do - but it's not going to be quick
19:59:40 sdague the Location header is clever, and when it was an internal location for nova, maybe really sensible
19:59:45 sdague mordred: yeh
19:59:46 mordred agree
19:59:58 mordred but the world it has changed since then
20:00:03 sdague I think the only sucky part is that the fix isn't really backportable
20:00:14 sdague this is basically an edge case of the v1 -> v2 bit that we missed
20:00:33 mordred yah - but it's also luckily not TERRIBLE to workaround as an API consumer
20:00:35 mriedem does /images alias to /v1/images though?
20:00:48 mordred I mean id = "/".rsplit(location, 1)
20:00:49 sdague mriedem: honestly, I don't know
20:00:50 mordred isn't bad
20:00:56 mordred mriedem: I do not believe so, no
20:01:13 sdague mordred: yeh, sure. We should probably mention there are weirdnesses in the API call
20:01:30 mriedem ok, i wasn't sure looking at https://developer.openstack.org/api-ref/image/versions/index.html?expanded=list-api-versions-detail,id1-detail
20:01:34 mriedem and the api refs for glance
20:01:50 mriedem or https://developer.openstack.org/api-ref/image/v1/index.html#images
20:01:55 mriedem which has an explicit /v1/ in it
20:02:09 mordred mriedem: no, it doens't
20:02:15 mriedem maybe this has just been broken since glance became a thing birthed from nova? :P
20:02:20 mordred GET https://image-ca-ymq-1.vexxhost.net/images/05a02839-5eb2-4335-8869-fabe88601f18
20:02:27 mordred gets you the image version discovery document
20:02:39 mordred mriedem: yes - I believe it likely has been broken always
20:02:46 mordred mriedem: because of how novaclient consumes the value
20:03:23 mriedem mordred: to be clear, "no it doesn't" meaning /images doesn't alias /v1/images?
20:03:48 mriedem because yeah i see https://image-ca-ymq-1.vexxhost.net/v1/
20:04:03 mriedem ok, well i'll draft up a spec
20:04:07 sdague mordred: so... on other clouds is it returning a glance url or a nova one?
20:05:00 mordred sdague: I can check - but it'll be a little bit - I need to create some vms first
20:05:23 sdague mordred: yeh, no worries. Just curious, because it's been so long
20:05:32 sdague because, I think that might be part of what happened
20:05:40 sdague as we used to push through nova urls
20:06:33 mordred image = cls.compute_images_client.create_image(server_id, name=name)
20:06:35 mordred image_id = data_utils.parse_image_id(image.response['location'])
20:06:37 mordred cls.images.append(image_id)
20:06:41 mordred tempest ALSO employs the novaclient workaround
20:06:47 sdague heh
20:06:54 sdague yeh, ok, we've just been terrible forever
20:06:58 mordred yup
20:08:41 mriedem ok so we're a go for removing location header and return image_id in a response body dict in a microversion, yes?
20:08:45 mriedem if so, i'll start writing up a spec
20:08:54 raj_singh mriedem: sdague We are working up a spec to use catalog lookup for Glance. This spec will have details on making consistent use of keystone_auth in Neutron and Cinder client as well.
20:09:05 mordred mriedem: ++ I think that's a GREAT microversion
20:09:07 raj_singh and we will have it up by today

Earlier   Later