| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-05-22 | |||
| 19:41:05 | mriedem | hold up | |
| 19:41:08 | mriedem | still going through this | |
| 19:41:53 | dansmith | I need food anyway | |
| 19:43:09 | mriedem | this is probably tied to get_instance stashing the targeted context on the instance object | |
| 19:43:17 | mriedem | that's probably something that deserves a comment in the code then | |
| 19:49:12 | jaypipes | mriedem: and it's that "get me alternate hosts" thing that I am really against. | |
| 19:49:24 | jaypipes | mriedem: in the conductor that is.. | |
| 19:52:09 | bauzas | sorry folks, was afk for dinner purposes | |
| 19:52:51 | jaypipes | dansmith: the diaper exception. nice. | |
| 19:53:36 | dansmith | jaypipes: that | |
| 19:53:48 | melwitt | mriedem: replied to the rest of the comments | |
| 19:53:49 | dansmith | is definitely not my term, but someone used it around me recently and I made note to use it again | |
| 19:54:22 | bauzas | jaypipes: dansmith: unfortunatly, I think it's related to the thread mriedem created about the pendulum switching | |
| 19:54:41 | bauzas | ie. should nova be tolerant to misconfig? | |
| 19:55:11 | bauzas | if we assume the answer be no, then I agree, case c) is probably not worth it for rescheduling | |
| 19:55:56 | mriedem | bauzas: that's not really why i started that thread | |
| 19:56:04 | bauzas | mriedem: that I know | |
| 19:56:10 | bauzas | mriedem: but I think the outcome is related | |
| 19:57:16 | mriedem | i think eventually disabling reschedules by default will make things that cause (c) to get exposed more clearly so that we can determine if they are things we should reschedule on, or handle as just bug fixes, i.e. some race we can handle | |
| 19:57:40 | mriedem | but it's a wild west of who knows | |
| 19:57:46 | bauzas | yup | |
| 19:57:51 | mriedem | b/c it's python and anything raised up from below goes in there | |
| 19:58:27 | mriedem | dansmith: left some comments/questions in https://review.openstack.org/#/c/458634/ | |
| 19:58:34 | dansmith | mriedem: thanks | |
| 19:58:36 | mriedem | dansmith: am i correct in stating that's all prep work for the next change in the series? | |
| 20:00:19 | mriedem | melwitt: ok looking | |
| 20:01:04 | bauzas | mriedem: dansmith: mmm, that https://review.openstack.org/#/c/458634/ is something I wondered when I was looking at my branch | |
| 20:02:00 | bauzas | mriedem: dansmith: correct me if I'm misunderstanding, but the problem is that we mutate the context so that when we exit the contextmanager, it's still having the new MQ ? | |
| 20:02:48 | bauzas | because if so, that explains what I discovered, ie. the fact that I was able to ComputeNodeList.get_all() without needing to scope my target | |
| 20:05:50 | mriedem | melwitt: replied to your replies, i don't get the point about old compute code calling count() | |
| 20:05:55 | mriedem | because it's not a separate service, | |
| 20:06:00 | mriedem | the old compute code will call the old count() code | |
| 20:06:17 | melwitt | mriedem: the count() function is accessible through the Quotas.count() remotable method | |
| 20:06:21 | mriedem | because it's all bundled together i think, unless maybe you have like a nova-common package and that gets updated before this or something? | |
| 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 | mriedem | still, couldn't nova.objects.Quotas.count() call count_as_dict and return the int? | |
| 20:15:20 | dansmith | bauzas: well, that's probably because you weren't in a multicell environment | |
| 20:15:23 | melwitt | mriedem: yeah, those methods I added on the NoopQuotaDriver were a mistake, they would do nothing | |
| 20:15:37 | dansmith | bauzas: without my (yet to be merged) patch to devstack, you're always single-cell | |
| 20:15:37 | bauzas | dansmith: that's what I thought, since it was for functional tests | |
| 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). | |