Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-04-03
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 mordred mriedem: https://bugs.launchpad.net/nova/+bug/1679285
19:46:45 openstack Launchpad bug 1679285 in OpenStack Compute (nova) "Create Server Snapshot returns incorrect Image Location" [Undecided,New]
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 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:02 mriedem it doesn't have v1 in it either
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
20:09:30 mordred raj_singh: hey - will you point me to it as well?
20:09:37 raj_singh pumaranikar: is working on it
20:09:49 mordred pumaranikar, raj_singh: I have some thoughts and a half-done patch for improving that area as well :)
20:09:53 raj_singh mriedem: Sure I will ping you once ready
20:10:09 mordred sweet
20:10:16 sdague mriedem: sounds great
20:31:08 melwitt mriedem: I thought you might want to look at this docs change, makes a new page for release notes how-to and links from the main page and the code review page https://review.openstack.org/#/c/451870
20:33:06 mriedem sdague: mordred: funny, but we emit notifications for snapshot start/end, but don't actually include the snapshot image id in those
20:33:11 mriedem kind of useless
20:33:30 mriedem gibi: ^
20:33:35 sdague so, question, in log messages, sometimes we use instance= and sometimes we use instance_uuid=
20:33:43 mriedem sdague: no different
20:33:45 mriedem *difference
20:33:56 sdague is there a reason we couldn't abolish instance= and do the uuid dereference thre?
20:33:56 mriedem oslo.log checks to see if it's 'instance' and pulls the uuid from it
20:34:13 sdague mriedem: yes, I realize it, but it actually adds a reasonable amount of gorp in oslo.log
20:34:15 mriedem it would be a bunch of churn
20:35:51 sdague yeh, though the dual fields do make doing some additional stuff with exposing variables to systemd journal a big gorpier

Earlier   Later