Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-09-12
16:15:12 rybridges mriedom: Thanks for the reply. IF we use the old classloader stuff, what are our chances in the future of getting hosed by a full deprecation?
16:15:22 openstackgerrit Eric Berglund proposed openstack/nova-specs master: PowerVM Driver Integration (Queens) https://review.openstack.org/503061
16:15:30 rybridges Main thing is I dont want to do a bunch of work to get a driver working which is planned for deprecation next release
16:16:41 mriedem rybridges: that stuff is still around in pike, but it's being removed in queens
16:17:00 mriedem rybridges: https://review.openstack.org/#/c/397835/
16:17:13 efried mikal https://review.openstack.org/#/c/471972/31/etc/nova/rootwrap.d/compute.filters <== looks like it removes the chown rootwrap filter... but nova.utils.temporary_chown wasn't flipped over to using the dac_admin wrapper for chown. Was that on purpose, or an oversight? Handled in a followup? (<== esberglu)
16:17:38 efried edmondsw_ ^
16:18:25 mriedem rybridges: novajoin is an example of a project that's using a vendordata v2 REST API service https://github.com/openstack/novajoin
16:18:30 mriedem if that helps
16:18:45 melwitt mriedem: if we count build_requests and instances there, there could still be in-flight instances that have both a build_request and an instance object right? but in that case your suggestion to remove dupe UUIDs seems like it would work
16:19:20 efried esberglu Can you propose a patch in nova to flip temporary_chown over to using the dac_admin wrapper for chown; then patch that sucker into our CI and see if it fixes our world?
16:20:27 mriedem melwitt: count them where? conductor?
16:20:27 efried esberglu It's possible mikal has that changed somewhere in the series (https://review.openstack.org/#/q/owner:%22Michael+Still+%253Cmikal%2540stillhq.com%253E%22+topic:hurrah-for-privsep+status:open) but I wasn't about to paw through all of those to find out.
16:20:29 openstackgerrit Eric Berglund proposed openstack/nova-specs master: PowerVM Driver Integration (Queens) https://review.openstack.org/503061
16:21:00 mriedem melwitt: there are two calls - api and conductor. i'm saying count build_requests *and* instances in cells in the api, but only instances in cells when calling from conductor
16:21:08 mriedem i don't think we even need dupe uuid filtering
16:21:21 mriedem because when checking qouta via the api, the instance doesn't exist for a given build request
16:21:23 melwitt mriedem: no, in the API where it is now. some other instances might be at the conductor stage and have both a build_request and an instance, right?
16:21:36 mriedem oh yeah, good point
16:21:41 mriedem separate concurrent request
16:21:45 melwitt yeah
16:21:45 mriedem getting double counted
16:21:51 openstackgerrit Eric Berglund proposed openstack/nova-specs master: PowerVM Driver Integration (Queens) https://review.openstack.org/503061
16:26:17 rybridges mriedem: I realize there are examples. We would really like to just keep our classloader based approach though. It doesnt make a lot of sense to us why we are getting rid of classloaders. This message from bloomberg sum up my thoughts pretty well -> http://lists.openstack.org/pipermail/openstack-operators/2016-April/010179.html
16:26:53 rybridges Why not just keep both approaches around and allow deployes to choose what works best for them?
16:27:13 mikal efried: looking
16:28:55 mikal efried: you're right, I've screwed that up. mriedem is standing next to me right now, I'll talk to him about it.
16:29:20 efried mikal I asked esberglu to propose a patch for it -- unless you'd rather own it.
16:29:31 openstackgerrit Eric Berglund proposed openstack/nova-specs master: PowerVM Driver Integration (Queens) https://review.openstack.org/503061
16:29:57 mikal efried: it depends on if mriedem wants to roll forwards or backwards. It would be quick for me to roll it forwards though.
16:30:02 mikal efried: which I'm happy to do the work for
16:30:15 edmondsw better fix test_temporary_chown... that should have failed
16:30:44 mikal It would only fail in a tempest test
16:30:48 mikal That's all mocked in unit tests
16:31:18 efried mikal Either way, it'd be neat to do it outside of the massive chain of privsep changes so we can get it in relatively quickly. It's blocking PowerVM CI at the moment :(
16:31:26 mikal Yeah, we can do that
16:31:42 efried mikal Okay, thanks. esberglu See ^^. mikal Do you want a LP bug?
16:31:58 mikal Yes please
16:32:02 efried rgr
16:33:26 openstackgerrit Dan Smith proposed openstack/nova master: Add nova-manage db command for ironic flavor migrations https://review.openstack.org/501025
16:35:56 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM Driver: config drive https://review.openstack.org/409404
16:36:52 mriedem rybridges: because classloading everything isn't a contract and we break it and then people that used hooks and classloaders complain when we change something internal to nova that breaks their unversioned API
16:37:51 esberglu mikal: efried: I can open the LP bug
16:38:37 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM Driver: config drive https://review.openstack.org/409404
16:38:55 mikal esberglu: ta, I have a fix now but need to work through the unit test fallout
16:42:49 mikal efried: do you guys have heaps of users of temporary_chown in your code? Because I kind of want to remove that method now that you've brought it to my attention.
16:42:57 mikal efried: because it makes me throw up in my mouth
16:44:34 openstackgerrit Balazs Gibizer proposed openstack/nova master: cover migration cases with functional tests https://review.openstack.org/493865
16:45:06 efried mikal We only use it in image snapshot.
16:45:26 efried mikal If you took it away, we would essentially have to duplicate the logic ourselves.
16:46:23 efried mikal https://github.com/openstack/nova-powervm/blob/fae3f96edb0c257468a93100796b338207a4cfc5/nova_powervm/virt/powervm/image.py#L45-L47
16:47:27 mikal efried: I'd hand hold you through uswing privsep there instead
16:47:36 efried Sorry, s/image snapshot/snapshot/. "image snapshot" doesn't make a lot of sense.
16:48:08 efried mikal Mm, that's another idea, I suppose. Cause it's really the open that needs privs raised, eh?
16:48:53 mikal efried: yeah, although I don't think privsep supp0orts passing off file descriptors (yet)
16:49:06 mikal efried: but yeah, I'd find a way to drag you guys forward before nuking the thing
16:49:07 efried mikal We could essentially just decorate that whole method with @nova.privsep.dac_admin_pctxt.entrypoint and remove the temporary_chown context manager.
16:49:16 efried right?
16:49:29 mikal efried: yes, but the method needs to move into the nova.privsep namespace as part of that decoration
16:49:36 efried oh
16:49:38 mikal efried: but yes, you could have nova.privsep.powervm
16:49:39 efried uhm
16:49:58 efried This is our out-of-tree driver.
16:49:59 mikal Let's fix the immediate problem and then talk the future thorugh
16:50:06 mikal You at the PTG?
16:50:24 efried We do have a toe in the nova.* namespaces, but at the moment its only purpose is redirecting to our virt driver entrypoint.
16:50:35 efried mikal Yeah, I talked to you yesterday morning in the bmvm room :)
16:51:07 efried You want to meet up?
16:51:17 esberglu mikal: https://bugs.launchpad.net/nova/+bug/1716718
16:51:18 openstack Launchpad bug 1716718 in OpenStack Compute (nova) "chown commands failing (no rootwrap filter)" [Undecided,New]
16:51:28 mikal LOL, I am an old man
16:51:38 mikal Let's talk tomorrow in the nova thing, that way I can get the thing fixed first
16:51:45 efried mikal ack
16:54:16 efried mikal Would you guys go for me adding the nova.privsep.powervm module in nova proper? We're in the process of integrating our driver in-tree, so we could argue it's for "future support of the snapshot operation".
16:54:31 prometheanfire anyone around to review https://review.openstack.org/#/c/501533/ for the new castellan release
16:54:32 efried Though tbh, we may not get snapshot into Queens.
16:58:43 mikal efried: so, I'm not a core, I'm basically a homeless guy no one has worked out how to get rid of
16:58:54 mikal efried: but I would be surprised if my betters would accept that thing
16:59:09 mikal efried: you're driver install process could add a file in that directory though...
16:59:27 efried mikal That *may* not be necessary.
16:59:38 efried Ima play with it and see if I can make it work.
16:59:44 mikal Cool
16:59:48 mikal Let me know if you need a hand
17:09:12 dansmith mdbooth: where you at?
17:10:30 efried mikal Like-a-this: https://review.openstack.org/503078
17:13:01 openstackgerrit Michael Still proposed openstack/nova master: Fix missed chown call https://review.openstack.org/503079
17:14:11 mikal efried: https://review.openstack.org/#/c/503079/ is your fix
17:14:17 mikal efried: can you test it with your driver please?
17:19:57 sean-k-mooney stephenfin: are you free for https://etherpad.openstack.org/p/placement-nova-neutron-queens-ptg, i think you would be interested in this
17:21:05 efried mikal Roger wilco. esberglu_lunch When you're back, please patch in https://review.openstack.org/#/c/503079/ and see if it resolves our snapshot snafu
17:22:31 efried esberglu_lunch It will also be instructive to see whether https://review.openstack.org/503078 passes *without* ^
17:30:18 openstackgerrit Balazs Gibizer proposed openstack/nova master: Send soft_delete from context manager https://review.openstack.org/476459
17:30:19 openstackgerrit Balazs Gibizer proposed openstack/nova master: Transform missing delete notifications https://review.openstack.org/410297
17:30:19 openstackgerrit Balazs Gibizer proposed openstack/nova master: use context mgr in instance.delete https://review.openstack.org/443764
17:38:17 openstackgerrit OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/502700
17:39:12 mriedem dansmith: there you go
17:39:50 dansmith mriedem: what now?
17:39:58 mriedem https://review.openstack.org/#/c/501025/
17:40:15 dansmith oh thanks
17:42:12 mriedem sure doll

Earlier   Later