Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-05-22
20:06:47 melwitt I don't think compute actually calls count. but if something old did, then the new code would have to work
20:07:33 mriedem i don't see anything calling it
20:08:06 mriedem besides the api,
20:08:08 mriedem which you've changed here
20:08:26 melwitt I don't think anything is, dansmith was saying because the remotable method is there, we can't just change the format of what it returns
20:08:45 mriedem i think that's fine, and you're not
20:09:05 mriedem you added count_as_dict and the api calls that now
20:09:19 melwitt yeah, but underneath there has to be something in nova/quota.py that returns the old format
20:09:21 melwitt right?
20:09:29 mriedem but i'm asking about https://review.openstack.org/#/c/446239/7/nova/quota.py@1914
20:09:34 melwitt if count() is to still be supported
20:09:45 melwitt Quotas.count() passes through to nova/quota.py
20:10:13 mriedem also this https://review.openstack.org/#/c/446239/7/nova/quota.py@1450
20:10:17 melwitt and what I have there is "if count() use old counting function, else use the new one that returns a dict"
20:10:59 mriedem where does ^ happen?
20:11:35 melwitt at L1450 I'm defining the old count function if one is provided
20:11:59 melwitt because a Quotas.count() would call self.count()
20:13:15 mriedem which otherwise would fail if you didn't have https://review.openstack.org/#/c/446239/7/nova/quota.py@1608 ?
20:13:23 mriedem Quotas.count() calls count on the resource objec
20:13:28 mriedem the engine doesn't have a count() method
20:13:31 dansmith mriedem: yeah, prep for the next
20:14:03 mriedem whether or not you're talking to the engine, the driver or the resource object is always a gamble when yo'ure talking quotas
20:14:05 dansmith bauzas: I'm not sure what you're asking.. currently we mutate global context for the temporary target context handler, which isn't what we want
20:14:35 mriedem melwitt: oh no you're right
20:14:44 mriedem engine does have a count() method which calls count() on the resource object
20:14:49 mriedem CountableResource in this case
20:14:57 bauzas dansmith: I'm trying to explain that I saw a weird behaviour where I stupidly forgot to target a cell for calling the compute list, and it worked
20:15:17 bauzas while it should have be given me no computes, right?
20:15:20 dansmith bauzas: well, that's probably because you weren't in a multicell environment
20:15:20 mriedem still, couldn't nova.objects.Quotas.count() call count_as_dict and return the int?
20:15:23 melwitt mriedem: yeah, those methods I added on the NoopQuotaDriver were a mistake, they would do nothing
20:15:37 bauzas dansmith: that's what I thought, since it was for functional tests
20:15:37 dansmith bauzas: without my (yet to be merged) patch to devstack, you're always single-cell
20:15:50 dansmith and in functional tests unless you specifically arrange for multicell
20:15:58 mriedem maybe the problem is if count called count_as_dict we wouldn't know which key to use in the dict
20:16:08 bauzas dansmith: ah right
20:16:09 mriedem but we could try keys until we found one that hits
20:16:32 melwitt mriedem: yeah, that might be possible.
20:17:28 melwitt I was thinking I should change security_group_rules to use the 'user' key since its limit can be scoped to a user
20:18:06 melwitt that could be the logic that determines which key it should be. whereas fixed_ips, floating_ips, and networks limit can only be scoped to a project
20:18:44 mriedem if figured just key = 'user' if 'user' in count else 'project'
20:18:52 mriedem you already have something like that in here
20:19:09 melwitt yeah. I was just saying as a tangent that I'm gonna change the key for security_group_rules
20:19:19 mriedem anywho, it's been raining here for about a week but we have sun today, so i'm going to go out and mow the lawn.
20:19:36 melwitt k. thanks for all of the good comments on the review
20:19:45 mriedem \o/ i'm hoping the rest of the series is easier than this one
20:20:14 melwitt it is, until the "count instances" patch, that one will be hard as all of the "reserve" change to "check_deltas"
20:25:28 mriedem plus it's multi-cell right?
20:25:34 mriedem until we get that info from placement?
20:26:12 melwitt mriedem: yeah, the count function isn't hard to understand. it's all of the changes in nova/compute/api and nova/compute/manager that might be tough
20:26:38 melwitt lots of deleted lines and changing the old reservation stuff to counting
20:27:42 melwitt and by that I mean, replacing the reserve/commit/rollback with the stuff defined in the first patch
20:28:00 mriedem dansmith: re your reply in the reschedule ML thread - you can configure the cell conductor to hit the api db to get the cell mappings, yes? we don't want you to, but you can.
20:28:28 dansmith mriedem: yes, but you can't configure it to talk to the compute and the scheduler at the same time, which is why I said MQ and not DB :)
20:28:52 mriedem yeah that was more clear
20:29:10 mriedem need diagrams in the devref at some point
20:29:13 mriedem but,
20:29:29 mriedem you probably wouldn't want to put a line between the cell conductor and the api db in a diagram anyway since you don't want people doing that
20:29:42 mriedem and we do'nt want code relying on that
20:30:04 dansmith and it wouldn't help without also building that line for the mq
20:30:08 mriedem i also get tripped up when talking about this because you don't need a super conductor today
20:31:35 mriedem i think it would be good to have something in the devref like a topology diagram for multi-cell cells v2, with some information about which config is needed for each service in which location, i.e. where do i need api_db or database set in nova.conf, which is pointing at cell0, et
20:32:07 mriedem i have a feeling people will just set api_db in nova.conf everywhere because they don't know, like we used to have in devstack
20:32:44 cburgess dansmith mriedem Wait what did I do?
20:33:05 mriedem huh?
20:33:09 cburgess dansmith mriedem Something about live migrations and extra specs (no regular users can't see the extra specs).
20:33:17 mriedem anyway, time for cutoffs and sleeveless t for the neighbors
20:33:17 dansmith mriedem: you don't if you only have one cell
20:33:35 cburgess Oh looks like an old message from a few days ago. Weird. Sorry
20:33:52 mriedem cburgess: regular users can see extra specs,
20:33:57 mriedem because we do'nt scope the check to the flavor
20:34:14 cburgess mriedem Oh ok.. well I meant, I don't change anything with regard to extra spec visibility
20:34:26 cburgess So whatever upstream does we do
20:34:28 mriedem https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/flavors_extraspecs.py#L53
20:34:55 mriedem cburgess: we all assumed that extra specs are hidden by default b/c they are admin or user in policy and the owner of a flavor/extra spec is the admin,
20:34:57 mriedem however,
20:35:13 mriedem we don't scope the policy check to the flavor, we scope it to the context, and the user context is the admin or owner - they are the owner of the request
20:35:25 mriedem so unless you change the policy to admin only, non-admins can see extra specs
20:35:30 cburgess Oh thats acute...
20:35:36 cburgess Cute even...
20:35:42 mriedem right, and johnthetubaguy has a spec to fix that
20:35:45 cburgess Like I said, I don't think we change that one.
20:35:53 cburgess Hmmm I wonder if that would be an issue..
20:35:59 cburgess Thats a pretty big change at this point.
20:36:01 mriedem https://review.openstack.org/#/c/433037/
20:36:13 mriedem it would break a bunch of shit i'm sure
20:36:22 mriedem b/c you'd start getting 401 or 403 or whatever
20:36:52 cburgess Yeah we have it default policy.
20:37:42 cburgess mriedem_afk What else is this spec going to break I wonder.
20:51:02 amrith mriedem_afk, when you get a chance would you please take a look at the email I sent you.
20:51:20 amrith wondering how best to make sure I get all of the gotcha's in one go.
21:18:58 openstackgerrit Dan Smith proposed openstack/nova master: De-complicate some of the instance delete path https://review.openstack.org/458636
21:18:59 openstackgerrit Dan Smith proposed openstack/nova master: Move to proper target_cell calling convention https://review.openstack.org/458634
21:19:00 openstackgerrit Dan Smith proposed openstack/nova master: Make target_cell() yield a new context https://review.openstack.org/458635
21:19:01 openstackgerrit Dan Smith proposed openstack/nova master: Fix live migration devstack hook for multicell environment https://review.openstack.org/458537
21:19:01 openstackgerrit Dan Smith proposed openstack/nova master: Target cell on local delete https://review.openstack.org/464059
21:19:11 dansmith mriedem_afk: this is only the object context change to see if it breaks anything ^
21:19:17 dansmith then I'll do the other things
21:49:59 efried mriedem_afk Can you please push https://review.openstack.org/#/c/391288/ ? Wanna get this pile moving again. Got some more stuff I want to add on top, features from previous review comments.
#openstack-nova - 2017-05-23
00:19:24 jaypipes artom: hi! hey, what parts of http://specs.openstack.org/openstack/nova-specs/specs/ocata/approved/virt-device-tagged-attach-detach.html were fully implemented and what's still left to do?

Earlier   Later