| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-06-16 | |||
| 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 | bauzas | dansmith: melwitt: which means something not possibly backported | |
| 14:44:23 | dansmith | yeah | |
| 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 | melwitt | mriedem: I thought you said it would 400 if someone tried to create the default group? | |
| 14:47:07 | sahid | thanks jaypipes - the patches are confusing i guess it's an effort to try understand the logic behind | |
| 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 | 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:39 | mriedem | bauzas: yeah that is nice, since it's namespaced to the driver CONF.filter_scheduler.enabled_filters | |
| 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 | |
| 14:49:32 | mriedem | host manager is the manager, caching and filter schedulers are drivers | |
| 14:49:32 | bauzas | mriedem: so, before providing a new opt for cachingscheduler, we would need to move the attribute | |
| 14:49:45 | mriedem | i don't want to move the attribute at this point really | |
| 14:49:52 | mriedem | since that would be another move - it's already been moved once | |
| 14:49:59 | mriedem | it used to just be in the [scheduler] group i think | |
| 14:50:49 | mriedem | let's just focus on getting placement to the point that we can ditch the caching scheduler :) | |
| 14:52:50 | openstackgerrit | Ed Leafe proposed openstack/nova master: Extract custom resource classes from flavors https://review.openstack.org/473627 | |
| 14:52:56 | figleaf | jaypipes: dtantsur: ^^ updated | |
| 14:53:05 | dtantsur | thanks! | |
| 14:56:55 | mriedem | eandersson: does https://review.openstack.org/#/c/474994/ fix the issue you're trying to solve with https://review.openstack.org/#/c/390984/ ? | |
| 15:01:32 | mriedem | heh, stable/mitaka is still open, maybe we should just continue backporting fixes to it :) | |
| 15:01:46 | mriedem | as i've said before, the LTS solution for openstack is just don't hire tony | |
| 15:05:11 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transform aggregate.add_host notification https://review.openstack.org/396210 | |
| 15:05:12 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transform aggregate.remove_host notification https://review.openstack.org/396211 | |
| 15:05:45 | mriedem | bottom 3 quotas changes are ready to go https://review.openstack.org/#/c/446241/ | |
| 15:07:22 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: use context mgr in instance.delete https://review.openstack.org/443764 | |
| 15:07:23 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transform missing delete notifications https://review.openstack.org/410297 | |
| 15:14:03 | bauzas | mriedem: superdan: I'm about to +W https://review.openstack.org/#/c/474994/ but I do wonder if we should duplicate the bug report | |
| 15:14:21 | bauzas | mriedem: superdan: oh, nvm, matt already commented on that | |
| 15:14:41 | bauzas | mriedem: I'll duplicate the bug, exact same problem | |
| 15:15:36 | bauzas | superdan: you don't need to amend the commit, launchpad will just follow-up | |
| 15:15:39 | mriedem | bauzas: maybe wait for the ironic job to finish on it first, but it should be fine | |
| 15:15:47 | mriedem | plus, i think this only shows up if a node is down right? | |
| 15:15:53 | mriedem | ironic i mean | |
| 15:18:27 | sdague | mriedem: ok, so what do you want the behavior to be on these stable bugs? | |
| 15:19:36 | sdague | like should I start converting them over to Fix Released? Or make a tool so you can do that when running the release? | |
| 15:19:37 | mriedem | sdague: if they are released then i guess the status should be fix released | |
| 15:19:58 | sdague | mriedem: right, well for dead branches, that's easy to determine | |
| 15:20:02 | sdague | for live ones, less so | |
| 15:20:03 | mriedem | we don't know if they are released though w/o pulling the change down and checking tags | |
| 15:20:21 | openstackgerrit | Baodong (Robert) Li proposed openstack/nova master: [WIP] Expose vlan trunking in metadata/configdrive https://review.openstack.org/471874 | |
| 15:20:25 | sdague | mriedem: right | |
| 15:20:35 | sdague | when are the next stable releases scheduled? | |
| 15:20:39 | mriedem | or we just punt and do like on master and say if it's committed it's released | |
| 15:20:45 | mriedem | sdague: they aren't scheduled | |
| 15:20:50 | sdague | mriedem: ok | |
| 15:20:50 | mriedem | they are more or less on demand | |
| 15:21:30 | sdague | ok, well that's kind of option B. I can put this into a tool that can be run on a branch when a release is done, and it would sweep everything to fix released then | |
| 15:26:01 | openstackgerrit | Ed Leafe proposed openstack/nova master: Extract custom resource classes from flavors https://review.openstack.org/473627 | |
| 15:26:31 | mriedem | sdague: we could just provide the release tag / version to the tool so it can leave a comment when switching the lp bug status to fix released too | |
| 15:26:35 | mriedem | to simplify | |
| 15:26:47 | mriedem | melwitt: i think there are a couple of issues in this one https://review.openstack.org/#/c/457861/ | |
| 15:26:54 | mriedem | for after your friday morning super meeting | |
| 15:26:57 | sdague | mriedem: sure | |
| 15:27:49 | mriedem | anyone fancy going through the bottom 4 quotas changes here? https://review.openstack.org/#/c/446241/ - i'm +2 on all, they are pretty mechanical | |
| 15:28:24 | melwitt | mriedem: super meeting was 7-8, but I'm digging into a bugzilla right now. will look when I can | |
| 15:33:29 | mriedem | cfriesen: looking at your novaclient change, the tests are much more readable now w/o the evals and stuff :) | |
| 15:43:56 | mriedem | cfriesen: i think you're missing handling --minimal for nova show | |
| 15:43:57 | mriedem | so -1 for that | |
| 15:50:50 | mriedem | alex_xu: if you're around | |
| 15:50:59 | mriedem | alex_xu: sdague: so this os-hypervisors API change i'm working on, | |
| 15:51:15 | mriedem | to fold the servers and search routes into the GET /os-hypervisors route using query parameters | |
| 15:51:35 | mriedem | https://developer.openstack.org/api-ref/compute/#hypervisors-os-hypervisors for context | |
| 15:51:49 | mriedem | i got to thinking last night, | |
| 15:52:16 | mriedem | search and servers were really just index with (1) hostname lookup pattern and (2) optional details on listing servers per host | |
| 15:52:31 | mriedem | folding those into index() as query parameters is natural | |
| 15:52:41 | mriedem | but then i got thinking last night, | |
| 15:52:57 | mriedem | if we add those as query parameters to index(), should we also add them as query parameters to GET /os-hypervisors/detail ? | |
| 15:53:09 | mriedem | because it would seem odd to be able to get per-host instance details from index, but not from detail | |
| 15:53:26 | mriedem | not that i'm looking for more work, but if i'm completely rewriting this API, we should probably do it properly | |
| 15:54:33 | sdague | yeh, well I've kind of thought the detail url pattern is a little bit broken. Because as far as I know no one users the main list | |
| 15:55:01 | sdague | but yes, any query parameters on index should also work on detail | |
| 15:55:02 | mriedem | what really got me thinking about this, | |
| 15:55:14 | mriedem | was in my change i'm using query parameter validation now, including on limit and marker for paging, | |
| 15:55:20 | mriedem | but didn't have that on detail() for the same microversion, which is odd | |
| 15:55:50 | mriedem | so you could do GET /os-hypervisors/detail?marker=uuid1&marker=uuid2 but not for GET /os-hypervisors | |
| 15:55:57 | mriedem | so yeah i guess i'm going to have to straighten that all out, | |
| 15:56:23 | mriedem | and i'll probably push an early change to consolidate all of the common code, since really index() and detail() are identical except for a boolean flag passed to show details | |
| 15:56:41 | mriedem | basically make it like how _get_servers works in the servers API | |