| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-06-16 | |||
| 13:32:25 | ttx | you got it. Makes more sense the way you summarized it :) | |
| 14:00:56 | sdague | mriedem: https://review.openstack.org/#/c/474961/ | |
| 14:01:54 | sdague | mriedem: in looking at the bug pile I noticed that bugs on stable series are remaining at "Fix Committed" not "Fix Released" - https://bugs.launchpad.net/nova/mitaka/+bugs | |
| 14:02:12 | sdague | mriedem: is that intentional, or did a tool not do the right thing there? | |
| 14:02:48 | mriedem | sdague: i thought the release tooling was supposed to flip those to fix released once a stable point release was done | |
| 14:04:14 | mriedem | sdague: my guess is ttx or doug would know which script in https://github.com/openstack-infra/release-tools does that | |
| 14:05:38 | efried | I thought "Fix Committed" was not used at all anymore. | |
| 14:05:54 | efried | Everything just flipped to "Fix Released" as soon as it merged. | |
| 14:06:08 | efried | Not saying I agree that's the way it *should* be done, esp. for stable branches, but... | |
| 14:08:50 | mriedem | efried: that's what happens on master yes | |
| 14:09:06 | sdague | yeh, I don't actually see where it's being done. | |
| 14:16:16 | sdague | mriedem: the process bugs tool only works for milestones | |
| 14:17:15 | mriedem | sdague: ok i figured it would be something like that | |
| 14:17:37 | mriedem | since we stopped using milestones for client/lib releases in LP long ago | |
| 14:18:37 | cfriesen | mriedem: https://review.openstack.org/435141 is ready for re-review, has the namespaced flavor output | |
| 14:19:00 | sdague | mriedem: ok, so I guess the question is, given that we've got a reasonable number of "still open bugs" for nova on stable branches, how would you like that handled? | |
| 14:19:44 | sdague | this is also complicated by the fact that you can't search on a series, so you actually just have to search all bugs, then look for tasks on a series | |
| 14:22:55 | mriedem | sdague: still open as in fix committed but not fixed relased? | |
| 14:22:58 | mriedem | *released? | |
| 14:23:48 | sdague | yes | |
| 14:24:26 | sdague | mriedem: so I took the liberty to bulk process them all on liberty, there were 73 bugs in "Fix Committed" | |
| 14:25:29 | sdague | there are 76 bugs in mitaka in "open" state, about 70 of those are fix committed | |
| 14:25:43 | mriedem | ok i didn't realize we considered fix committed as open | |
| 14:25:50 | sdague | launchpad does | |
| 14:25:56 | sdague | fix committed is an open state | |
| 14:25:58 | mriedem | melwitt: do you know if a user can create the "default" security group in nova? | |
| 14:26:35 | mriedem | sdague: ok so i guess we just mark them as fix released - although since we don't have milestones for stable branch releases we can't point them at a release milestone | |
| 14:26:52 | sdague | mriedem: - https://dl.dropboxusercontent.com/1/view/r7znbwgz2hg3qmm/Apps/Shutter/screenshot_253.png that's why the counts there are so high | |
| 14:27:01 | melwitt | mriedem: not sure. the only way they could is if they do it before they ever created even one instance. because there's a thing inline with instance create that creates the default group if it doesn't exist yet | |
| 14:27:33 | mriedem | melwitt: yeah that's what i'm looking at in your change | |
| 14:27:45 | mriedem | every time we create a security group with nova-net it ensures the default group exists | |
| 14:27:55 | melwitt | https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L1795 | |
| 14:27:59 | mriedem | i just was'nt sure if you could explicitly create the default group | |
| 14:28:23 | mriedem | melwitt: yeah also here https://review.openstack.org/#/c/457860/21/nova/compute/api.py@5047 | |
| 14:28:54 | melwitt | mriedem: let me check on that. I don't expect it would stop you but I don't know for sure if there's anything that prevents it | |
| 14:28:57 | mriedem | my thinking was, what if someone is creating the default group explicitly, hits a quota race and then we delete the default group because we aren't going through the SecurityGroupAPI.destroy() method, | |
| 14:29:02 | mriedem | although, | |
| 14:29:12 | melwitt | if you had a fresh deployment and hadn't created any instances yet, it might be possible | |
| 14:29:16 | mriedem | given the default group is created on the first secgroup create, you shouldn't hit overquota on the first one | |
| 14:29:22 | mriedem | unless your quota for security groups is like 1 | |
| 14:30:08 | mriedem | i think i'm mostly worrying about an edge case that may not ever happen | |
| 14:30:35 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add BDM to InstancePayload https://review.openstack.org/448779 | |
| 14:30:36 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add key_name field to InstancePayload https://review.openstack.org/463002 | |
| 14:30:37 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add separate instance.create payload type https://review.openstack.org/463001 | |
| 14:30:37 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add keypairs field to InstanceCreatePayload https://review.openstack.org/419730 | |
| 14:32:12 | melwitt | mriedem: ah, I see. thinking ... | |
| 14:39:32 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix regression preventing reporting negative resources for overcommit https://review.openstack.org/474994 | |
| 14:39:42 | mriedem | i don't see anything restricting someone from explicitly creating the 'default' security group with nova-net | |
| 14:39:45 | dansmith | jaypipes: melwitt ^ | |
| 14:39:49 | dansmith | bauzas: ^ | |
| 14:39:54 | mriedem | although | |
| 14:40:12 | jaypipes | dansmith: on it after sahid's patch and some food. | |
| 14:40:14 | mriedem | melwitt: looking at this code, i think what would happen if you tried to create the default group is we'd auto-create it, | |
| 14:40:22 | mriedem | and then try to create it again, and get a SecurityGroupExists | |
| 14:40:25 | dansmith | jaypipes: np, no real rush, just closing the loop | |
| 14:40:25 | mriedem | which would fail the request | |
| 14:40:52 | bauzas | dansmith: wfm, first glance looks good to me | |
| 14:40:55 | mriedem | melwitt: which is actually kind of funny since you're trying to create something, which we create, and then tell you it already exists | |
| 14:41:37 | melwitt | dansmith: here's the change I mentioned eandersson was working on which aims to re-enable overcommit https://review.openstack.org/#/c/390984/ | |
| 14:42:08 | dansmith | oh jeez | |
| 14:42:52 | dansmith | reworking the filters will be a lot harder to backport, and a lot of this can probably go away soon with placement | |
| 14:43:11 | dansmith | maybe we could already nuke the ram filter actually? | |
| 14:43:33 | mriedem | we've talked about at least taking the cpu, disk and ram filters out of the default enabled filters list | |
| 14:43:38 | mriedem | since we check those with placement since ocata | |
| 14:43:51 | melwitt | mriedem: huh. yeah, I was wondering about that, whether or not we'd end up deleting it or if something would prevent it | |
| 14:43:53 | mriedem | there was the weird ironic case, which i forget, that tripleo uses so yo'ud still need them | |
| 14:43:54 | bauzas | dansmith: melwitt: FWIW, I said in https://review.openstack.org/#/c/390984/ that it's a behavioural modification | |
| 14:44:05 | bauzas | dansmith: melwitt: so that's why I asked for a relnote | |
| 14:44:09 | dansmith | yeah | |
| 14:44:17 | melwitt | dansmith: yeah just bringing to attention if it needs to be -1ed in light of what you've found | |
| 14:44:23 | dansmith | yeah | |
| 14:44:23 | bauzas | dansmith: melwitt: which means something not possibly backported | |
| 14:45:27 | bauzas | mriedem: dansmith: nuking RamFilter needs first to nuke CachingScheduler | |
| 14:45:31 | mriedem | melwitt: so i guess in your change, if we hit a quota race, then you're going to destroy the group directly, bypassing the checks in the SecurityGroupAPI.destroy method | |
| 14:45:43 | bauzas | mriedem: dansmith: because folks use it | |
| 14:45:46 | mriedem | bauzas: i didn't say delete it, i said remove it from the default enabled filters list | |
| 14:45:58 | mriedem | bauzas: does caching scheduler user the filter_scheduler.enabled_filters option? | |
| 14:46:00 | bauzas | mriedem: unfortunately, CachingScheduler uses the same opt | |
| 14:46:04 | bauzas | yup | |
| 14:46:08 | bauzas | so, we're fucked | |
| 14:46:10 | mriedem | good thing we namespaced it to "filter_scheduler" then | |
| 14:46:34 | bauzas | wait, just verifying tho | |
| 14:47:00 | mriedem | melwitt: if we did race and cleanup the 'default' group, then the original one that we raced against is also gone | |
| 14:47:07 | sahid | thanks jaypipes - the patches are confusing i guess it's an effort to try understand the logic behind | |
| 14:47:07 | melwitt | mriedem: I thought you said it would 400 if someone tried to create the default group? | |
| 14:47:19 | mriedem | which is bad - but again, i don't think this can happen since default is auto-created on the first group/instance created, so unless your quota is 1, it shouldn't happen | |
| 14:47:27 | mriedem | melwitt: oh right yeah | |
| 14:47:35 | mriedem | so it shouldn't even create it | |
| 14:47:43 | mriedem | melwitt: let's just not ever worry about this :) | |
| 14:47:48 | mriedem | it's all code no one should be using anyway | |
| 14:47:57 | bauzas | mriedem: yeah, so... enabled_filters is related to the HostManager module, not the scheduler driver it uses | |
| 14:48:04 | bauzas | mriedem: nice, heh ? | |
| 14:48:23 | melwitt | mriedem: heh. I will think about it but it's early for me. I also have a fresh devstack I can try creating the group and see what happens | |
| 14:48:39 | mriedem | bauzas: yeah that is nice, since it's namespaced to the driver CONF.filter_scheduler.enabled_filters | |
| 14:48:39 | bauzas | I really don't know *why* we did set the list of filters in some internal module that's not related to the driver, but meh | |
| 14:48:55 | bauzas | mriedem: the fact is that HostManager is shared with CachingScheduler | |
| 14:48:56 | mriedem | melwitt: i wouldn't worry about it | |
| 14:49:10 | mriedem | melwitt: also, congrats on being up before 8am :) | |
| 14:49:23 | melwitt | -_- thanks | |
| 14:49:24 | mriedem | bauzas: sure | |