Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-09-12
15:39:03 mriedem right so if i create 20 instances in concurrent requests, i'll probably get 10 in ERROR state
15:39:10 mriedem rather than 10 plus 10 failed requests with 409s
15:40:12 mriedem Kevin_Zheng: we should start with a bug report, can you open one?
15:40:27 Kevin_Zheng mriedem: sure, I will
15:40:51 mriedem check_num_instances_quota is just iterating the cells looking for instances, but at this point we don't have instances in the cells, we have build_requests in the api db
15:41:01 mriedem so we'd need something similar
15:42:44 mriedem and that method is called from the api and conductor, and in the case of conductor we'd have to not count build_requests because at that point that conductor calls check_num_instances_quota we have the instances created in the cells
15:43:11 openstackgerrit Ghanshyam Mann proposed openstack/nova-specs master: Spec to remove the hide server address config options Partial implement blueprint remove-configurable-hide-server-address-feature https://review.openstack.org/502516
15:47:21 mriedem i see one issue with counting via build_requests is that when we count instances in the api, we also count cores and ram, which are fields on the instance in the db,
15:47:44 mriedem with a build_request, we have a serialized instance record, so we can count total instances via build_requests, but not cores/ram...unless we used placement
15:48:09 mriedem we want to use placement for counting quotas anyway...which we are going to talk about tomorrow morning
15:50:15 rybridges Hello. I had a question or 2 about vendordata in the Ocata release
15:50:21 rybridges Are we still able to write our vendordata logic stuff into a driver? I am noticing this line in the ocata code base -> https://github.com/openstack/nova/blob/stable/ocata/nova/api/metadata/base.py#L745 Is vendordata not doable through a driver anymore?
15:51:31 mriedem rybridges: you should be able to use the old style driver stuff
15:51:35 mriedem in ocata
15:58:19 Kevin_Zheng https://bugs.launchpad.net/nova/+bug/1716706
15:58:20 openstack Launchpad bug 1716706 in OpenStack Compute (nova) "Should count instances in build requests when check quotas" [Undecided,New] - Assigned to Zhenyu Zheng (zhengzhenyu)
16:01:45 openstackgerrit Balazs Gibizer proposed openstack/nova master: Test resource allocation during soft delete https://review.openstack.org/495159
16:03:12 mriedem Kevin_Zheng: thanks, added it here https://etherpad.openstack.org/p/nova-ptg-queens-cells
16:03:15 mriedem L22
16:03:26 mriedem also, i realized we can't use placement in the api to count cores/ram
16:03:32 mriedem since those allocations aren't created by then
16:04:46 Kevin_Zheng could use flavor instead
16:09:35 melwitt mriedem: we talked about counting build_requests but the consensus was to not because they're in a separate DB from instances and it would be possible for both to exist at the same time and result in a wrong count (doubled)
16:11:20 mriedem melwitt: when we check in the API they shouldn't exist at the same time, although that might be hard to know...
16:11:33 mriedem couldn't we exclude by uuid or something?
16:11:42 openstackgerrit Merged openstack/nova master: fake_notifier: Refactor wait_for_versioned_notification https://review.openstack.org/489637
16:12:10 Kevin_Zheng mriedem, records of other existing instances, might be doubled
16:12:16 mriedem select count(build_requests) where user/project; build a list of uuids; then iterate cells getting a count of instances where uuid is not in that list?
16:12:20 melwitt mriedem: we check in conductor though (check in API is for cells v1 I believe)
16:12:32 mriedem melwitt: yeah in conductor we wouldn't include the check for build_requests
16:12:40 mriedem because we know they both exist at the time of that check
16:12:53 mriedem melwitt: i'm not talking about the recheck
16:12:56 melwitt then how is this happening in the API?
16:13:06 melwitt let me just look
16:13:37 mriedem here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L888
16:14:02 mriedem so if i'm creating 20 instances concurrently, ^ will tell me there is one per request
16:14:23 mriedem then the recheck in conductor https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L1001 will fail
16:14:33 efried mikal Did you merge rootwrap changes related to chown?
16:14:49 melwitt mriedem: ah, right. thanks. I had forgotten what I did
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 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:27 mriedem melwitt: count them where? conductor?
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 mriedem getting double counted
16:21:45 melwitt yeah
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

Earlier   Later