Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-28
17:56:23 mriedem to get the version doc
17:56:30 mriedem then we replace all of that with the KSA thing in master
17:56:49 mriedem then it's just a session.get()
17:57:15 efried cinderclient.Client.session doesn't exist until we've created the client though, does it?
17:57:50 efried that's the chicken/egg
18:03:14 efried mriedem: (I know you're busy, but when you're available...) Any reason not to move the microversion check to after the client construction?
18:03:25 mriedem i think that was my idea from the bug report
18:03:31 mriedem "The alternative is on the nova side, we just construct a cinderclient Client object and use it's internal client (session) to make a request, or use a keystoneauth1 adapter to make the request."
18:05:07 efried okey, I'll see if I can work that up.
18:29:30 openstackgerrit Dan Smith proposed openstack/nova master: Add aggregates list to Destination object https://review.openstack.org/544729
18:29:30 openstackgerrit Dan Smith proposed openstack/nova master: Make get_allocation_candidates() honor aggregate restrictions https://review.openstack.org/547990
18:29:31 openstackgerrit Dan Smith proposed openstack/nova master: Add an index on aggregate_metadata.value https://review.openstack.org/555851
18:29:31 openstackgerrit Dan Smith proposed openstack/nova master: Add AggregateList.get_by_metadata() query method https://review.openstack.org/544728
18:29:32 openstackgerrit Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002
18:29:33 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Honor availability_zone hint via placement https://review.openstack.org/546282
18:43:05 openstackgerrit Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002
19:02:10 openstackgerrit Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002
19:15:54 openstackgerrit Mathieu Gagné proposed openstack/nova-specs master: Multiple Fixed-IPs support in network information https://review.openstack.org/312626
19:17:30 openstackgerrit melanie witt proposed openstack/nova master: Convert websocketproxy to use db for token validation https://review.openstack.org/333990
19:17:31 openstackgerrit melanie witt proposed openstack/nova master: Update layout docs for running console proxies https://review.openstack.org/557489
19:20:46 openstackgerrit melanie witt proposed openstack/nova master: Convert websocketproxy to use db for token validation https://review.openstack.org/333990
19:20:46 openstackgerrit melanie witt proposed openstack/nova master: Update layout docs for running console proxies https://review.openstack.org/557489
19:21:25 dansmith mriedem: do you have a recent devstack running by chance?
19:21:28 dansmith (or anyone)
19:21:52 mriedem nope
19:23:36 dansmith okay I was asking you since I'm writing the docs you demand and I don't have a recent one myself
19:30:07 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Honor availability_zone hint via placement https://review.openstack.org/546282
19:30:08 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Documentation for tenant isolation with placement https://review.openstack.org/557490
19:33:49 openstackgerrit Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002
19:33:50 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Documentation for tenant isolation with placement https://review.openstack.org/557490
19:33:50 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Honor availability_zone hint via placement https://review.openstack.org/546282
19:39:34 efried mriedem, jgriffith: https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3964 Is this here to avoid a DB lookup?
19:40:35 efried It looks to me like the guts of _check_volume_already_attached_to_instance don't actually depend on cinder microversion 3.44 -- and we're just doing this because, if 3.44 isn't supported, we couldn't possibly have >1 volume attached, so there's no need to do _check_volume_already_attached_to_instance.
19:40:43 jgriffith efried: well, no.. not really; the cinder mv can be dynamic
19:41:42 jgriffith efried: yeah, once that attachment is made we use the bdm entry for determining which flow is in use
19:42:00 jgriffith efried: sorry, I might not understand completely what you're looking for
19:42:02 mriedem efried: the point is the comment in the else block below it
19:43:08 jgriffith Oh, specifically that call right there :)
19:44:23 mriedem i'm having a hard time remembering too
19:44:28 efried Lemme make sure I have the logic right:
19:44:28 efried If multiattach not supported by cinder, proceed.
19:44:28 efried Otherwise, fail if the volume is attached.
19:44:41 mriedem 3.44 isn't about multiattach
19:44:45 mriedem it starts with https://review.openstack.org/#/c/525622/
19:44:56 mriedem with the new attachments api stuff, cinder will let us create multiple attachments between the same volume and instance
19:45:09 mriedem to enable things like live migration where the instance is attached to the volume on different hosts
19:45:24 mriedem but because of that, we found we had a bug where you could attach the same volume to the instance via the api multiple times
19:45:30 mriedem so we had to add a specific check for that
19:46:07 jgriffith mriedem: yeah, the comment actually is pretty good :)
19:46:07 mriedem before using new style attachments, cinder would not allow this
19:46:18 mriedem that's why in https://review.openstack.org/#/c/525622/ we have to rely on the bdm table
19:47:54 efried Ultimately I was trying to figure out if I could get rid of that microversion check :) I think I now understand why I can't.
19:47:55 mriedem so going back to https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3964
19:48:20 efried Which kinda sucks, because it means I'm gonna have to build up a whole cinder client just to do that microversion check.
19:48:24 mriedem if 3.44 is supported, we know we're doing the new style attachment_create, and we have to make sure the instance isn't already attached to that volume, so we call _check_volume_already_attached_to_instance
19:48:34 mriedem if 3.44 isn't available, we'll do the old style reserve flow
19:48:49 mriedem which cinder enforces you can only have the instance attached to the volume once
19:49:01 mriedem efried: yeah but that is only temporary for the backport,
19:49:12 mriedem and we can replace that stuff with ksa in master right?
19:49:34 efried mriedem: Except I still haven't figured out how to get ksa stuff working for cinder, period.
19:49:41 mriedem heh
19:49:42 mriedem well,
19:49:47 mriedem it's just code right?!
19:49:56 efried Sigh. Right.
19:50:02 mriedem sylvain's daughter will have it done by tomorrow
19:50:19 efried I know, I can't let her show me up like that. Not again.
19:54:57 dansmith cripes
19:56:11 openstackgerrit Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002
20:12:46 mriedem dansmith: not sure if you care about any of the comments in https://review.openstack.org/#/c/544730/15
20:14:45 dansmith mriedem: sorry hadn't seen those
20:14:46 dansmith replied
20:18:25 mriedem i've got this nova-status check ready to go basically for the cells api service version check thing, i think i'll just munge the api log warning into the same patch
20:20:19 dansmith seems reasonable
20:38:30 openstackgerrit Dan Smith proposed openstack/nova master: Add request filter functionality to scheduler https://review.openstack.org/544730
20:38:31 openstackgerrit Dan Smith proposed openstack/nova master: Add aggregates list to Destination object https://review.openstack.org/544729
20:38:31 openstackgerrit Dan Smith proposed openstack/nova master: Make get_allocation_candidates() honor aggregate restrictions https://review.openstack.org/547990
20:38:32 openstackgerrit Dan Smith proposed openstack/nova master: Add an index on aggregate_metadata.value https://review.openstack.org/555851
20:38:32 openstackgerrit Dan Smith proposed openstack/nova master: Add AggregateList.get_by_metadata() query method https://review.openstack.org/544728
20:38:33 openstackgerrit Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002
20:38:33 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Documentation for tenant isolation with placement https://review.openstack.org/557490
20:38:34 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Honor availability_zone hint via placement https://review.openstack.org/546282
20:40:36 mikal mriedem: yeah, I can be around to interate on privsep for the next couple of weeks. Noting that both of the next two weeks are short because of easter.
20:41:26 openstackgerrit Merged openstack/nova stable/queens: Avoid exploding if guest refuses to detach a volume https://review.openstack.org/551948
20:41:27 mikal mriedem: what's blocking removing nova-net? The last couple of privsep patches aren't mergable until nova-net is gone, but I can WIP those or something. It would be nice to get them landed this cycle though.
20:43:34 mriedem mikal: nothing is blocking it
20:43:44 mriedem mikal: but,
20:43:57 mriedem i imagine it's not trivial since a ton of the test code in-tree assumes nova-net
20:44:05 mriedem and that's including non-nova-net things
20:44:20 mikal Is anyone working on it?
20:44:25 mriedem so all of that has to be unwound
20:44:28 mriedem mikal: not that i know of,
20:44:36 mriedem i got the cells v1 job running with neutron and that was step 1
20:46:53 mriedem which reminds me,
20:47:01 mriedem melwitt: dansmith: efried: i'm out on monday and tuesday
20:47:22 dansmith orly
20:47:22 melwitt k, thanks for the heads up
20:47:23 dansmith I'm out friday for "GENERIC SPRING HOLIDAY"
20:47:34 melwitt me too
20:47:40 efried roger that, mriedem. Means you have to review this bug fix tonight/tomorrow I guess.
20:49:18 openstackgerrit Matt Riedemann proposed openstack/nova master: Log a warning and add nova-status check for old API service versions https://review.openstack.org/557506

Earlier   Later