| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-27 | |||
| 16:24:41 | sdague | we can always put a check like this back in later if it's happening at any frequency, and even backport it | |
| 16:24:57 | superdan | cdent: mriedem: scheduling patch just passed multinode grenade | |
| 16:25:00 | superdan | johnthetubaguy: ^ | |
| 16:25:26 | cdent | amazeballs | |
| 16:25:35 | superdan | sdague: it just doesn't seem responsible to not check for and catch a thing we know can happen when it's trivial to do, but.. the fight is all gone from me this week | |
| 16:26:26 | sdague | superdan: I just want to fix one thing at a time here and get people unblocked. We can fix these other things later. | |
| 16:26:47 | superdan | sdague: I'm not saying that has to be in front of this necessarily | |
| 16:26:48 | mriedem | superdan: how is that possible | |
| 16:26:53 | superdan | mriedem: cosmic rays | |
| 16:27:07 | sdague | because it's not like you'll get the wrong test results if it happens, it's just that figuring out what went wrong takes a little longer | |
| 16:27:14 | superdan | mriedem: cdent johnthetubaguy I will get a bite to eat and then do the grenade refactor sdague is asking for | |
| 16:27:23 | superdan | there's also pep8 and unit fails in the nova side patch | |
| 16:27:46 | johnthetubaguy | superdan: cool, I can roll back the patch so the hacks are removed, and the tests are passing? | |
| 16:27:54 | mriedem | jroll: throwaway hack yes please | |
| 16:27:54 | superdan | sdague: it does mean we run another 50m of tests that we know will fail instead of just bailing early, but yeah | |
| 16:28:16 | superdan | johnthetubaguy: remove the logging and such, but keep the *correct* min ver check yeah :) | |
| 16:28:26 | johnthetubaguy | cdent: sorry, I meant to ask you, did you have any more thoughts on this one: https://review.openstack.org/#/c/404472 | |
| 16:28:36 | johnthetubaguy | superdan: thats my *aim* anyways | |
| 16:28:46 | sdague | superdan: sure, but that makes it an optimization we can fix later, even deep in the freeze, once we see how often it happens | |
| 16:28:49 | superdan | johnthetubaguy: I wish you luck sir | |
| 16:29:12 | cdent | superdan, johnthetubaguy is the issue with making sure the service cache gets updated after each compute node update suitably okay? are we assuming the scheduler will be restarted every time some uprades a compute node? | |
| 16:30:11 | cdent | johnthetubaguy: sorry, I started looking and got distracted by a merge conflict on some logging fixes and lost my place. Is there a summary of what we're trying to resolve? | |
| 16:30:17 | johnthetubaguy | cdent: I was thinking you needed to SIG_HUP for the RPC API change anyways, but it might be the first time | |
| 16:30:27 | johnthetubaguy | cdent: the -1 comment on the patch should do it | |
| 16:30:32 | superdan | cdent: that's the assumption that my grenade service check ensures is safe, and which sdague wants to wait on since we haven't seen it yet, even though it's 100% possible | |
| 16:30:41 | cdent | sig hup won't do it, as there's no reset handler? | |
| 16:30:50 | superdan | cdent: so I'll pull that out and do it separately and if we start to break the world after this merges, we'll have it ready | |
| 16:30:56 | johnthetubaguy | cdent: basically, seems like the instance allocations will fail for ironic nodes that have resource_classes defined | |
| 16:31:16 | superdan | cdent: correct, there's no reset handler, which I did ask for, but we can do after as well, and won't help the gate anyway | |
| 16:31:24 | johnthetubaguy | cdent: ah, I missed that | |
| 16:31:33 | johnthetubaguy | superdan: +1, but lets keep that separate | |
| 16:31:40 | cdent | superdan: k, I wasn't think in terms of the gate, but you know, like real people ;) | |
| 16:32:52 | superdan | johnthetubaguy: yeah, I was going to pull it out regardless, but I was going to keep it in the path of this set, but I won't now | |
| 16:33:00 | superdan | cdent: yes, which is why I asked for the reset thing initially | |
| 16:34:14 | cdent | johnthetubaguy: tacking back to that allocations thing now | |
| 16:36:42 | mriedem | is someone fixing up the nova side scheduler change? because if they are, can they address the exception wording comment in https://review.openstack.org/#/c/425806/ ? | |
| 16:37:47 | johnthetubaguy | cdent: ack | |
| 16:38:06 | mriedem | snikitin: are you saying the diagnostics patches were all approved yesterday before nit fixing? | |
| 16:38:10 | mriedem | or just had +2s on them? | |
| 16:38:44 | openstackgerrit | Jim Rollenhagen proposed openstack/nova: DNM: hack ironic with resource providers https://review.openstack.org/426296 | |
| 16:38:49 | jroll | mriedem: enjoy | |
| 16:41:07 | mriedem | jroll: cool hopefully i can load that sometime today | |
| 16:41:24 | jroll | heh | |
| 16:48:53 | sdague | cdent: there are many ways that things can go wrong. My biggest concern is that we don't have a failed software stack that provides a passing result. As long as we don't have that, our cost is debug time and time until test completion. | |
| 16:49:33 | sdague | because we have a specific test that attempts to schedule to all nodes, and a min node count, we have one test that points in the right direction already, so debug time isn't hugely negatively impacted | |
| 16:49:46 | sdague | and test run time until failure is basically +15m here | |
| 16:49:47 | cdent | sdague: I'm not sure which of several context to put that response in? | |
| 16:50:01 | figleaf | johnthetubaguy: can you summarize the sticking points in https://review.openstack.org/#/c/404472 for your -1? | |
| 16:50:29 | cdent | johnthetubaguy: are you concerned about the situation of old style ironic allocations being deleted but then nothing allocation the new style? I'm still confused (apparently like figleaf too) | |
| 16:50:44 | sdague | cdent: about why I said don't both with this - https://review.openstack.org/#/c/424730/12/projects/60_nova/resources.sh@136 at the moment | |
| 16:51:17 | cdent | sdague: I hadn't made any comments with respect to that? | |
| 16:51:37 | sdague | cdent: ok, well a bunch of the superdan cross comments were about that | |
| 16:51:45 | johnthetubaguy | cdent: figleaf: the bit I don't understand, is who make allocations for ironic instances, making use of the new resource class | |
| 16:51:46 | sdague | anyway, lunch, biab | |
| 16:51:58 | cdent | johnthetubaguy: nobody | |
| 16:52:07 | cdent | which is why the old allocations will never be cleaned up | |
| 16:52:19 | cdent | because InventoryInUse will never get raised | |
| 16:52:26 | mriedem | jroll: CUSTOM_LOL eh | |
| 16:52:32 | johnthetubaguy | cdent: figleaf: it looks like we still trying to make the old style allocations in the resource tracker | |
| 16:53:05 | johnthetubaguy | cdent: figleaf: which should fail I assume, as that resource class isn't present for that resource provider any more | |
| 16:53:16 | superdan | sdague: if you mean cross in the "unhappy" sense, you're misinterpreting my exhaustion | |
| 16:53:38 | cdent | I think it was "cross" as in "interleaved" | |
| 16:53:42 | openstackgerrit | John Garbutt proposed openstack/nova: Scheduler calling the Placement API https://review.openstack.org/417961 | |
| 16:53:49 | superdan | sdague: I think that check should be in there given the requirement we're adding that it be right at scheduler start, but I'm okay to wait until later if you think it's best | |
| 16:53:52 | superdan | cdent: okay, cool | |
| 16:54:28 | cdent | johnthetubaguy: why are they not there anymore? (not asserting they are, just tracing your thought process) | |
| 16:55:00 | openstackgerrit | Stephen Finucane proposed openstack/nova: console: Move proxies to 'console/proxy' https://review.openstack.org/408192 | |
| 16:55:03 | johnthetubaguy | cdent: because the code now only creates inventory entires for the new custom resource class | |
| 16:56:05 | figleaf | johnthetubaguy: existing ironic nodes won't have the custom resource class set | |
| 16:56:09 | cdent | johnthetubaguy: so if a new ironic thing comes into existence, with a defined, resource class, that's when we run into trouble? | |
| 16:56:18 | figleaf | johnthetubaguy: so they'll get handled like they were vms | |
| 16:56:21 | johnthetubaguy | cdent: yes | |
| 16:56:34 | figleaf | johnthetubaguy: ah | |
| 16:56:59 | figleaf | johnthetubaguy: so your concern is that we don't have the ironic-aware bits to select a resource provider? | |
| 16:57:06 | mriedem | andreykurilin: your upper-constraints dreams have come true https://review.openstack.org/#/c/426116/5/upper-constraints.txt@332 | |
| 16:57:25 | openstackgerrit | Sarafraj Singh proposed openstack/nova: Adopts keystoneauth with glance client. https://review.openstack.org/412634 | |
| 16:57:25 | andreykurilin | mriedem: \o/ | |
| 16:57:46 | johnthetubaguy | figleaf: ish, its basically that this method will fail (looking for link) | |
| 16:58:45 | johnthetubaguy | figleaf: ah, so this will fail, when there is a new style ironic resource provider being used by a new instance that is created: https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L664 | |
| 16:58:50 | johnthetubaguy | figleaf: at least I think it will | |
| 16:58:51 | openstackgerrit | Sarafraj Singh proposed openstack/nova: Adopts keystoneauth with glance client. https://review.openstack.org/412634 | |
| 16:58:55 | sfinucan | Interesting problem here - I've backported a bugfix that's exposed a latent Python 3 issue in the code https://review.openstack.org/#/c/425087 | |
| 16:59:11 | johnthetubaguy | figleaf: cdent: maybe a quick google hangout would make this easier to talk through? | |
| 16:59:14 | sfinucan | Wonder if I should fix the Python 3 issue or drop the test? | |
| 17:01:48 | figleaf | johnthetubaguy: maybe, but I'm switching context to follow the code through the different parts | |
| 17:02:16 | figleaf | johnthetubaguy: I can't keep it all straight while working on something else :) | |
| 17:02:23 | johnthetubaguy | figleaf: no worries, it took me a good while to trace all that back too | |
| 17:05:45 | superdan | sdague: I have to merge something in devstack to make grenade consider a new projects/XX_thing plugin? | |
| 17:06:32 | cdent | johnthetubaguy: will objects.InstanceList.get_by_host_and_node return ironic things? | |
| 17:06:54 | johnthetubaguy | cdent: I believe it does | |
| 17:07:22 | superdan | cdent: it'll return ironic instances yeah | |
| 17:07:56 | cdent | k, thanks johnthetubaguy and superdan, trying to properly trace the code (as figleaf said, is meandering...) | |
| 17:09:01 | superdan | sdague: oh, nm I think i see | |
| 17:12:01 | openstackgerrit | John Garbutt proposed openstack/nova: Scheduler calling the Placement API https://review.openstack.org/417961 | |
| 17:12:02 | openstackgerrit | John Garbutt proposed openstack/nova: Block starting compute unless placement conf is provided https://review.openstack.org/425806 | |
| 17:13:13 | mriedem | johnthetubaguy: ah i see you updated it :) | |
| 17:13:19 | mriedem | was just leaving comments, will finish that up on the older PS | |
| 17:13:46 | johnthetubaguy | mriedem: hopefully, that was a rebase, but I think it was superdan 's change that failed to merge, but never mind | |
| 17:15:57 | mriedem | johnthetubaguy: just posted comments in PS36 | |
| 17:16:37 | jroll | mriedem: sure why not :P | |