| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-24 | |||
| 14:35:55 | melwitt | I see | |
| 14:36:47 | openstackgerrit | Eric Fried proposed openstack/nova master: Clarify --before help text in nova manage https://review.opendev.org/661289 | |
| 14:36:50 | mriedem | there is no whitelist | |
| 14:36:52 | mriedem | we talked about it | |
| 14:36:55 | mriedem | but it got out of hand | |
| 14:37:08 | mriedem | i had a wip patch for a bit for one type of exception but it was gross and dropped it | |
| 14:37:32 | mriedem | there are about a billion ways you can get an exception in that build path in the compute | |
| 14:37:41 | dansmith | yeah, that's what I mean by we thought it would be clearer which things are terminal failures or not, but it's not | |
| 14:37:48 | mriedem | and since it's python and not strictly typing any of that stuff, we can't really just catch one | |
| 14:38:10 | dansmith | well, I dunno about _that_ :) | |
| 14:39:02 | mriedem | well, meaning like in java where you have to declare and explicitly handle exceptions that get raised up during compiling | |
| 14:39:14 | dansmith | ah sure | |
| 14:39:27 | dansmith | i.e. "the most annoying part of java" | |
| 14:39:35 | mriedem | zvm driver could add a new MainframeKaputError to raise and we'd not handle it | |
| 14:39:43 | mriedem | if you're ocd like me it's not so bad :) | |
| 14:40:30 | mriedem | i'd like some credit for my witty use of a german word in that zvm joke please... | |
| 14:40:46 | melwitt | ok, this is sounding familiar now, I vaguely remember your wip patch about it | |
| 14:40:58 | melwitt | oh yeah, kaputt, nicely done | |
| 14:41:11 | mriedem | melwitt: so is your customer on just a release that is too old to have the weigher? | |
| 14:41:48 | melwitt | no, they have the weigher, it's just incrementing the counter for user-caused build failures we think | |
| 14:41:48 | efried | finucannot: If two out of three of melwitt, mriedem, and sean-k-mooney ack it, I'll be happy to approve it (nova-consoleauth removal bp) | |
| 14:42:02 | dansmith | that's the whole thing | |
| 14:42:07 | dansmith | that's why we moved it to a weigher | |
| 14:42:21 | dansmith | because we suck so bad at knowing why a thing failed that auto-disable isn't really sane | |
| 14:42:23 | mriedem | melwitt: malicious user? | |
| 14:42:41 | dansmith | so we made it a weigher so it doesn't take it out of rotation and you can adjust how much score you give to that decision | |
| 14:42:51 | melwitt | mriedem: I doubt it but not sure | |
| 14:43:22 | mriedem | so jed at the bank wrote a script to boot 100 vms at once and it's DoSing their private cloud | |
| 14:43:58 | melwitt | dansmith: got it. yeah, IIUC this customer is trying to pack instances and they see it start spreading instances when they don't want it to, because of the user-caused failures | |
| 14:44:21 | dansmith | melwitt: right so null out that weigher and move on | |
| 14:45:51 | melwitt | mdbooth: turns out I was mistaken. we had tried some exception whitelist patches but it got unwieldy, so that idea was abandoned. the recommended way to handle this scenario is to disable the failed build weigher | |
| 14:46:30 | mdbooth | melwitt: Ack, thanks. | |
| 14:48:27 | mriedem | once again let me state i dislike the non-border table rendering in our docs now https://docs.openstack.org/nova/latest/admin/configuration/schedulers.html#id17 | |
| 14:51:53 | gibi | mriedem: do you have opinion about the CLI syntaxt propsal in https://review.opendev.org/#/c/651783/3/osc_placement/resources/resource_provider.py@130 ? | |
| 14:54:13 | mriedem | sec | |
| 14:54:18 | gibi | thanks | |
| 15:02:42 | finucannot | mriedem: I'll trade you a fix for that table issue for your ack on my remove-consoleauth bp | |
| 15:03:04 | finucannot | been meaning to get to that for weeks now | |
| 15:17:09 | aspiers | efried: asking on #openstack-requirements about the contextlib2 issue, I have not much clue what's going on | |
| 15:17:20 | aspiers | bizarrely it's moaning about sphinx too | |
| 15:17:36 | aspiers | where's stephenfin when you need him? | |
| 15:18:06 | finucannot | aspiers: I'm here (it's Friday) | |
| 15:18:12 | aspiers | hah | |
| 15:18:59 | openstackgerrit | Arnaud Morin proposed openstack/nova master: Force refresh instance network info on deletion https://review.opendev.org/660761 | |
| 15:19:28 | finucannot | What's the specific Sphinx issue? | |
| 15:19:31 | finucannot | aspiers: ^ | |
| 15:19:43 | aspiers | finucannot: see #openstack-requirements | |
| 15:20:45 | finucannot | Man, I wish we'd drop Python 2 this cycle instead of in U :( | |
| 15:22:28 | amorin | hi efried, mriedem and others. I updated the patch to refresh instance network cache on deletion, thanks for commenting/reviewing! | |
| 15:23:48 | ganso | dansmith: Hi! When you have a few minutes, could you please take a look at and sign off https://review.opendev.org/#/c/659338/ if everything is ok? thanks in advance =) | |
| 15:24:34 | openstackgerrit | Adam Spiers proposed openstack/nova master: Move patch_exists() to nova.test.TestCase for reuse https://review.opendev.org/660500 | |
| 15:24:58 | dansmith | ganso: I was going to challenge the directness of your query, but I see tonyb threw me under the bus, so.. okay :) | |
| 15:25:15 | mriedem | efried: finucannot: i've put my ack in https://blueprints.launchpad.net/nova/+spec/remove-consoleauth | |
| 15:30:48 | mriedem | amorin: questions in there, | |
| 15:31:05 | mriedem | but without the force will this even fix the case in your bug? have you tested it without the force_refresh=True? | |
| 15:31:27 | mriedem | b/c the point of the force flag was to rebuild the cache from neutron rather than the cache itself | |
| 15:33:56 | amorin | mriedem: good point | |
| 15:34:03 | amorin | hum | |
| 15:34:33 | amorin | I need to test that, it worked in my dev infra, but I need to double check that | |
| 15:35:11 | amorin | I wont have time today, but I will do next week | |
| 15:35:43 | mriedem | "it" being the original patch with force_refresh=True? | |
| 15:35:50 | mriedem | or the current version of https://review.opendev.org/#/c/660761/3 ? | |
| 15:36:15 | mriedem | if terminate is just racing with build and the copy of the instance during terminate just doesn't have the info cache data from the db, then the refresh should fix your problem i think, | |
| 15:36:35 | mriedem | but if the info cache in the db is empty when you refresh, that bottom change isn't going to help | |
| 15:36:47 | mriedem | it won't make things worse of course | |
| 15:36:47 | amorin | the cache is populated in the DB | |
| 15:36:55 | mriedem | ok, well in that case you might be saved, | |
| 15:37:00 | amorin | but after the copy of instance is used for deletion | |
| 15:37:14 | amorin | yup, anyway I will double check that point | |
| 15:37:15 | melwitt | dansmith: do you remember where we landed when we were talking about disabling oslo.messaging heartbeats in nova-api, as far as whether we would recommend running that configuration in our upstream docs? | |
| 15:37:20 | mriedem | because https://github.com/openstack/nova/blob/976d1b89c2b754729903291050e01c9bf49704b9/nova/network/neutronv2/api.py#L1757 | |
| 15:37:33 | sean-k-mooney | efried: sorry was on a call for the last 90 mins | |
| 15:37:43 | amorin | _gather_port_ids_and_networks is using the nova cache DB right? | |
| 15:37:53 | sean-k-mooney | efried: did ye reach an agreement on the nova-console auth blueprint | |
| 15:37:56 | openstackgerrit | Adam Spiers proposed openstack/nova master: Move selective patching of open() to nova.test for reuse https://review.opendev.org/661266 | |
| 15:38:12 | melwitt | dansmith: this is about the idle nova-api wsgi app heartbeat error messages stuff | |
| 15:38:26 | mriedem | amorin: yeah | |
| 15:38:47 | amorin | mriedem, by the way, thanks for the tip about mocking in setup, I was pretty sure that this was possible | |
| 15:38:51 | mriedem | amorin: yeah so tl;dr if you're just racing to refresh from the db on delete then your fix is probably ok | |
| 15:38:55 | amorin | but I am beginning in that subject | |
| 15:38:57 | mriedem | np | |
| 15:39:03 | mriedem | i've only recently-ish started using that trick | |
| 15:39:09 | efried | sean-k-mooney: It sounds like all the deployment projects are either sorted or have patches in flight to sort. I was looking for you & others to vet and +1 basically so I could approve the bp | |
| 15:39:54 | melwitt | efried, finucannot: I'm also +1 on removing nova-consoleauth since it looks like everything is sorted with deployment tools | |
| 15:40:02 | sean-k-mooney | efried: kolla has an open bug but no progess in a while | |
| 15:40:22 | sean-k-mooney | if that is the only blocker i can go write the patches needed | |
| 15:40:47 | sean-k-mooney | it is already optional but it would jsut invovle deleting the container on upgrade an removing the deployment code | |
| 15:41:28 | efried | finucannot: do you have any further words about kolla ^ ? | |
| 15:41:57 | sean-k-mooney | so yes im +1 on removing it and can help the kolla folks as needed since i used to work on kolla and kolla ansibel a few release ago | |
| 15:42:06 | finucannot | sean-k-mooney: So there's more to it than this? https://review.opendev.org/#/c/661251/ | |
| 15:42:09 | finucannot | efried: ^ | |
| 15:42:33 | sean-k-mooney | that removed it from kolla | |
| 15:42:44 | sean-k-mooney | but they also need to remove it in kolla ansible | |
| 15:42:53 | sean-k-mooney | e.g. you just deleted teh container | |
| 15:43:16 | sean-k-mooney | but the upgrade playbook need to koll all running instance of the contain | |
| 15:43:21 | sean-k-mooney | *kill | |
| 15:43:54 | finucannot | Ah, I figured there was more to it than that. I can try drafting those patches based on what they did in OSA. I'd need one to kill the running instances (which would be backported to stable/stein) then a follow up to remove all references entirely | |
| 15:44:33 | sean-k-mooney | we need to kill this handeler https://github.com/openstack/kolla-ansible/blob/master/ansible/roles/nova/handlers/main.yml#L112-L136 | |
| 15:44:48 | efried | finucannot: Okay, mriedem, melwitt, and sean-k-mooney are all +1, I'm approving the bp assuming the above is fairly easy and will be taken care of before we pull the trigger. | |
| 15:44:54 | sean-k-mooney | and remove like 2 or 3 other things but its a fairly small patch | |