Earlier  
Posted Nick Remark
#openstack-nova - 2017-12-05
19:49:11 sean-k-mooney2 gus: if the privsep entrypoint decorator that was used for func1 and func2 were from a different privsep context im guessing the answer would have to be yes as they may have different capablitys which woudld lead me to belive if they were the same context i would still invoke func2 via the socket.
19:54:44 sean-k-mooney2 mikal: ^^ any input on the above question regarding nested privsep calls?
19:58:19 mikal sean-k-mooney2: huh, we haven't done anything like that. I'm really not sure.
19:58:39 mikal sean-k-mooney2: I guess the decorators aren't magic, and would just apply the contexts in the order the decorators were executed
19:58:56 mikal sean-k-mooney2: and if you didn't get an error from privsep calling itself, then you'd get the context of the innermost call
19:59:09 sean-k-mooney2 mikal: actully we do it in os-vif but i belive its unintentional.
19:59:31 sean-k-mooney2 mikal: i think two decorators were acidentally swapped
20:00:21 sean-k-mooney2 mikal: in the os-vif case both decorators are form the same privsep context so both have the same permissions
20:00:45 mikal Oh, interesting. So its either crash, or work as expected?
20:01:00 sean-k-mooney2 basically i think https://github.com/openstack/os-vif/blob/23c71e083aeb3536c4cf4c46b5c7712b0f7442ee/vif_plug_linux_bridge/linux_net.py#L58 and https://github.com/openstack/os-vif/blob/23c71e083aeb3536c4cf4c46b5c7712b0f7442ee/vif_plug_linux_bridge/linux_net.py#L70 are swapped
20:01:24 sean-k-mooney2 mikal: i think it works as expected but just dispatches twice
20:02:31 sean-k-mooney2 mikal: https://github.com/openstack/os-vif/blob/23c71e083aeb3536c4cf4c46b5c7712b0f7442ee/vif_plug_linux_bridge/linux_net.py#L65 the call to _ensure_bridge_privileged is in a privesep context but _ensure_bridge_privileged is also privaldged
20:06:28 sean-k-mooney2 mikal: im going to try and un nest the calls tomorow but i realised while reviewing https://review.openstack.org/#/c/484386/26/os_vif/privsep.py that the functions that use this new privsep context are only ever invoked form privaldged function in the plugins which have the same capablites CAP_NET_ADMIN
20:07:55 sean-k-mooney2 so 1 that would result in nested context with the same capablites and 2 it would intoduce another instance of the privsep deamon to hanel that context both of which i think are not needed.
20:08:51 sean-k-mooney2 mikal: am im going to head home for the evening but if you have a chance could you take a look and see if it makes sense to intorduce this new privsep context or not?
20:19:39 openstackgerrit Matt Riedemann proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285
20:19:40 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: libvirt: Allow multiple volume attachments https://review.openstack.org/267587
20:21:30 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Allow multi-attach in compute api https://review.openstack.org/271047
20:21:52 mriedem ildikov: i touched up some things in https://review.openstack.org/#/c/267587/ and left some FIXMEs
20:22:57 ildikov Oh, I thought you'll fix up the new attach patch only
20:23:06 mriedem i wanted to rebase the series
20:24:10 ildikov Oh, ok, I left behind the API one on purpose, but will check what you did in a bit :)
20:24:17 ildikov Thanks!!!
20:24:51 ildikov I can look into the Cinder policies and client leftovers too a bit then...
20:41:13 mriedem i'm also going to add unit tests for the new flow to nova.tests.unit.virt.test_block_device in https://review.openstack.org/#/c/330285/170 - we might want to consider splitting out the virt.block_device changes too
20:42:46 mikal mriedem: you had a chance to read my comment on that privsep bug?
20:42:53 mriedem mikal: nope
20:46:03 mikal mriedem: good man. In summary, I have no idea what you people are talking about.
20:46:15 mriedem mikal: that's what you told me yesterday, so ok :)
20:46:32 mikal mriedem: oh, I was expecting you to call me a liar on the bug is all
20:47:04 jaypipes cdent: you have any idea what is causing this tempest.scenario.test_server_multinode.TestServerMultinode.test_schedule_to_all_nodes failure with the "Failed to get resource provider with UUID XXXX" issue that keeps occurring? I think you and efried_cya_wed may have talked aboiut it?
20:47:41 cdent yeah, there’s a bug for it, I’m working on it: https://bugs.launchpad.net/nova/+bug/1736385 basically placement is getting restarted
20:47:42 openstack Launchpad bug 1736385 in grenade "placement is not being properly restarted in grenade (pike to master)" [Undecided,In progress] - Assigned to Chris Dent (cdent)
20:48:11 cdent it’s apparently been a latent problem only exposed because exception handing that efried_cya_wed added
20:48:27 jaypipes cdent: k. is there anything I can do to assist?
20:49:20 cdent I don’t reckon so, the delay has been in getting it to repeat locally, but I’m nearly there, and from there should be able to do some poking
20:49:56 cdent but thanks for offering. If I stall out before the night is over, I’ll dump state on the bug and let you know
20:53:15 jaypipes cdent: k
20:53:33 openstackgerrit Brianna Poulos proposed openstack/nova master: Implement certificate_utils https://review.openstack.org/479949
21:12:17 ildikov mriedem: let me know when you had enough and want me to take over and screw up the half thing again before doing anything sane :)
21:13:03 ildikov mriedem: and I owe you a lot, I'm sure we can negotiate, lemme know
21:13:03 mriedem ildikov: i need to fix another functional test, and i'm working on adding the unit test coverage for the new flow changes in test_block_device
21:13:21 mriedem which makes me realize that in the new attach flow in block_device.py, if attachment_complete fails, we don't do any cleanup
21:13:24 mriedem unlike the old flow
21:14:35 ildikov yeah, we moved that around a bit
21:14:43 ildikov so I guess that's why it's uncovered
21:15:32 ildikov the volume is connected though, however the volume state would still be attaching if that call doesn't get to Cinder or fails before the db update
21:20:00 mriedem that's why we handle it in the legacy flow and rollback (terminate the connection and detach the volume so it's "available")
21:22:22 mriedem anyway, i'm just going to leave a note in the code
21:22:37 ildikov I know, was thinking out loud
21:43:13 mriedem we are going to want to break out these block_device.py changes with the unit tests - this gets pretty hairy in the tests
21:43:19 mriedem i'm down to 10 failures
21:45:05 ildikov we should find a superhero name for you
21:45:32 ildikov can do it after my next meeting and breaking out those changes
21:52:28 ildikov and sorry, there were two self.save() calls in the new attach call in block_device.py
21:52:57 ildikov I obviously deleted the wrong duplicate...
21:57:40 openstackgerrit Lance Bragstad proposed openstack/nova master: Add scope_types to server policies https://review.openstack.org/525772
21:59:07 mriedem ildikov: i'm actively working on the attach patch so don't make any changes there
21:59:21 lbragstad cc johnthetubaguy ^
21:59:44 ildikov mriedem: I didn't intend to until you explicitly say so
22:00:18 ildikov mriedem: otherwise things can get messy, practiced that with jgriffith not that long ago
22:29:02 faridda Hi guys, to Configure Hyper-V virtual switching, we have to specify an interface. Which one should be use (internal api, tenant network ...)?
22:55:29 melwitt mriedem: heads up, regression live-migrating with a volume that was backported to ocata. I'm gonna add stable/pike and stable/ocata to the bug https://bugs.launchpad.net/nova/+bug/1715569
22:55:30 openstack Launchpad bug 1715569 in OpenStack Compute (nova) "Live migration fails with an attached non-bootable Cinder volume (Pike)" [Low,In progress] - Assigned to Mike Lowe (jomlowe)
22:55:59 melwitt "nominate to series pike and ocata
22:56:44 mriedem faridda: ask claudiub or lpetrut when they are around
22:57:13 mriedem melwitt: you mean something was changed which introduced the regression and that change was backported to stable?
22:57:56 melwitt mriedem: yes, that
22:58:08 mriedem and https://review.openstack.org/#/c/459741 was the chnage
22:58:20 melwitt yes
22:58:28 mriedem super
22:58:33 melwitt yeah :(
22:59:13 melwitt the good news? is the fix looks correct to me and fixes the problem without undoing what the other change [that introduced the regression] fixed
23:02:43 melwitt so I think we can just roll forward and backport the fix that's up for review right now
23:20:42 mriedem gd finally got these unit tests passing
23:21:05 mriedem 2.5 hours
23:22:22 melwitt I know your pain
23:28:33 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: libvirt: Allow multiple volume attachments https://review.openstack.org/267587
23:28:33 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Implement new attach Cinder flow https://review.openstack.org/330285
23:28:34 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Allow multi-attach in compute api https://review.openstack.org/271047
23:29:27 ildikov mriedem: that would've taken me waaaaay more than that
23:29:45 ildikov I know it doesn't give you back the wasted time, but still
23:30:32 mriedem i'll see if i can split out block_device.py quick
23:34:16 melwitt mriedem: on this https://review.openstack.org/#/c/525622 how can we know we have cinder microversion 3.44 available? or does the check not hurt anything in the case that it's < 3.44?
23:34:37 ildikov sounds like a famous last sentence, but hearing from you makes it believable :)
23:37:06 openstackgerrit Takashi NATSUME proposed openstack/nova stable/pike: [placement] Fix foreign key constraint error https://review.openstack.org/525620
23:38:57 mriedem melwitt: at this point we won't check because bdm.attachment_id won't be set
23:39:21 mriedem bdm.attachment_id only gets set once (1) computes are all upgraded to support new flow and (2) cinder 3.44 is available
23:39:31 melwitt ah, cool. thanks
23:39:35 mriedem existing bdms won't have that attachment_id field set, so we'll just do the old stuff
23:45:07 mriedem here it comes
23:45:09 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: libvirt: Allow multiple volume attachments https://review.openstack.org/267587
23:45:09 openstackgerrit Matt Riedemann proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285
23:45:10 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Add new style volume attachment support to block_device.py https://review.openstack.org/525787
23:45:10 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Allow multi-attach in compute api https://review.openstack.org/271047
23:48:47 mriedem ildikov: jgriffith: i split this out https://review.openstack.org/#/c/525787/ - and left fixme/todo stuff in there from when i was adding the unit tests
23:48:55 mriedem too tired to really go over any of that now
23:50:44 mriedem that tempest test for attaching a volume to a shelved offloaded test is passing too, even after the revert
23:51:06 mriedem so at this point i think the questions in https://review.openstack.org/#/c/525787/ are going to be the sticking point, and hopefully those aren't too bad
23:55:08 mriedem melwitt: so in https://review.openstack.org/#/c/525622/3/nova/compute/api.py - you want a specific test for the case that we find a bdm and fail?

Earlier   Later