| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-07 | |||
| 12:57:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove nova.compute.*API() shims https://review.opendev.org/660527 | |
| 12:57:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add reno for removed cells v1 policies https://review.opendev.org/662031 | |
| 12:57:16 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'InstanceUnknownCell' exception https://review.opendev.org/662411 | |
| 12:57:16 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Ensure controllers all call super https://review.opendev.org/660950 | |
| 12:58:12 | stephenfin | gibi++ | |
| 12:58:21 | stephenfin | Thanks for taking a look at those tests | |
| 13:28:55 | mriedem | stephenfin: gibi: comments inline on https://review.opendev.org/#/c/660774/ | |
| 13:29:05 | mriedem | for the notification sample test weirdness | |
| 13:29:47 | stephenfin | in a meeting but I'll look after | |
| 13:30:17 | stephenfin | mriedem: Actually, I did try to that. It failed miserably | |
| 13:31:02 | stephenfin | The issue that I pointed out previously is that I want the call to succeed in the other places it's called | |
| 13:31:08 | stephenfin | And just stub out that one caller | |
| 13:31:43 | gibi | stephenfin, mriedem: I can try to iterate on those tests sometime next week | |
| 13:40:33 | mriedem | i can play with it, maybe 3 sets of eyes can sort it out | |
| 13:42:53 | gibi | mriedem: one hint, in those resize sample tests there is only one compute (as far as I see) but still there is a successfull re-schedule attempt | |
| 13:43:40 | mriedem | which is totally unrealistic but ok | |
| 13:44:11 | mriedem | maybe it's not unrealistic i guess, hard to know how this flow really works | |
| 13:44:36 | mriedem | it would be unrealistic to reschedule and land back on the same host | |
| 13:44:43 | sean-k-mooney | gibi: a rescuderl would only work in that case if the retry filter is disabled | |
| 13:45:17 | gibi | mriedem: that two test are totally unrealistic :/ | |
| 13:45:18 | mriedem | we don't go back through the scheduler on a reschedule | |
| 13:45:21 | mriedem | since queens | |
| 13:45:40 | gibi | mriedem: fault injection is hard | |
| 13:45:51 | mriedem | in fact we could probably deprecate the RetryFilter | |
| 13:45:56 | mriedem | i don't think it's ever used anymore | |
| 13:46:05 | mriedem | anyway, bbiab | |
| 13:46:24 | sean-k-mooney | that worth looking into | |
| 13:46:43 | sean-k-mooney | ill file a bug so we dont forget | |
| 13:53:59 | stephenfin | mriedem: Yeah, if you've ideas I'd be happy to hear them. Those tests really aren't good but I'm out of ideas at this point | |
| 14:15:45 | mriedem | i'll tinker | |
| 14:21:39 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Deprecate RetryFilter https://review.opendev.org/663953 | |
| 14:21:40 | mriedem | sean-k-mooney: let's see what blows up ^ | |
| 14:21:48 | mriedem | edleafe: ^ | |
| 14:24:13 | stephenfin | mriedem: Would you be okay with the named kwargs thing if it was a separate patch? I think it's a nice readability improvement but it probably shouldn't be munged with other stuff alright | |
| 14:24:24 | stephenfin | Referring to https://review.opendev.org/#/c/651311/ | |
| 14:26:04 | mriedem | i disagree that it's a readability improvement... | |
| 14:26:23 | mriedem | by that logic we'd use named kwargs on everything | |
| 14:27:01 | mriedem | stephenfin: i'm going to pull the series down and tinker with those notification tests for reschedule and if i come up with someting cleaner i can push that up and remove the kwargs in the next patch in the series while i'm at it | |
| 14:27:13 | sean-k-mooney | mriedem: i have disabled the retry filter in one of my nfv job iteration and it more orless seam to be fine so i suspect it will not blow up or not blow up too badly | |
| 14:27:41 | stephenfin | mriedem: Not everything. Just things that have a lot of arguments (it's subjective but I'd say > 5) | |
| 14:28:05 | sean-k-mooney | im on a triage call but ill file the bug and coment on your patch when im done | |
| 14:28:13 | mriedem | sean-k-mooney: bug? | |
| 14:28:31 | mriedem | it's not really a bug that we forgot to deprecate the RetryFilter in Rocky | |
| 14:28:42 | sean-k-mooney | ok | |
| 14:29:06 | sean-k-mooney | in that case ill just keep an eye on the test results :) | |
| 14:30:48 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove cells v1 parameter from 'ComputeTaskAPI.resize_instance' https://review.opendev.org/651311 | |
| 14:30:48 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop passing 'kwargs' to 'rebuild_instance' https://review.opendev.org/651312 | |
| 14:30:49 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop passing 'delete_type' to 'terminate_instance' https://review.opendev.org/651313 | |
| 14:30:49 | openstackgerrit | Stephen Finucane proposed openstack/nova master: filters: Stop handling cells v1 https://review.opendev.org/651314 | |
| 14:30:50 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove nova.compute.*API() shims https://review.opendev.org/660527 | |
| 14:30:50 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add reno for removed cells v1 policies https://review.opendev.org/662031 | |
| 14:30:51 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'InstanceUnknownCell' exception https://review.opendev.org/662411 | |
| 14:30:51 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Ensure controllers all call super https://review.opendev.org/660950 | |
| 14:30:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Use named kwargs for resize_api https://review.opendev.org/663959 | |
| 14:36:32 | artom | bauzas, thanks for the note on https://review.opendev.org/#/c/644881/21, will address after this call. Also need to address mriedem's commit message feedback, which I completely forgot about the first time around | |
| 14:37:31 | mriedem | oh right you want to backport that.... | |
| 14:37:38 | mriedem | i was going to say we don't need a release note for a virt driver interface change, | |
| 14:37:42 | mriedem | but if we're backporting... | |
| 14:38:17 | mriedem | we did this recently for an ironic driver related fix and handled a TypeError in the backportable version and removed it in a follow up on master | |
| 14:39:04 | mriedem | https://github.com/openstack/nova/blob/stable/rocky/nova/compute/manager.py#L7648 | |
| 14:39:13 | mriedem | given that, we may want to follow the same pattern here | |
| 14:39:29 | mriedem | it is admittedly bending over backward for out of tree drivers which is being exceedingly nice | |
| 14:40:19 | artom | Wait, won't that leave vm_instance unset and blow up later? | |
| 14:40:52 | artom | Oh, actually no. | |
| 14:41:15 | artom | vm_power_state will be unset, but it's only used in an if | |
| 14:41:35 | artom | OK, I'll do something similar when I backport | |
| 14:41:39 | mriedem | for your change it would mean adding a try/except TypeError around the call to finish_revert_migration and if we get a TypeError, call again without the vifs_already_plugged kwarg | |
| 14:41:51 | mriedem | when you backport? if we do this, you'd do it on master, | |
| 14:41:57 | mriedem | and then follow up with a patch to remove that handling | |
| 14:42:06 | mriedem | otherwise your backport has new extra logic in it which sucks to review | |
| 14:42:25 | artom | Oh yeah *facepalm* | |
| 14:42:44 | artom | Why remove on master though? | |
| 14:43:11 | artom | That same method signature TypeError thing can also happen with oot drivers on master, no? | |
| 14:43:16 | artom | If they're really slow to update | |
| 14:44:54 | sean-k-mooney | artom: we have customers still running kilo | |
| 14:45:14 | sean-k-mooney | artom: i assume those customers are also slow to update there out of tree stuff | |
| 14:45:27 | artom | I doubt any of our customer have oot drivers :) | |
| 14:45:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop passing 'kwargs' to 'rebuild_instance' https://review.opendev.org/651312 | |
| 14:45:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop passing 'delete_type' to 'terminate_instance' https://review.opendev.org/651313 | |
| 14:45:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: filters: Stop handling cells v1 https://review.opendev.org/651314 | |
| 14:45:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove nova.compute.*API() shims https://review.opendev.org/660527 | |
| 14:45:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add reno for removed cells v1 policies https://review.opendev.org/662031 | |
| 14:45:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'InstanceUnknownCell' exception https://review.opendev.org/662411 | |
| 14:45:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Ensure controllers all call super https://review.opendev.org/660950 | |
| 14:46:04 | sean-k-mooney | well not any that we support in anycase | |
| 14:47:33 | efried | I know for sure PowerVM oot driver is in production for real customers | |
| 14:48:10 | bauzas | artom: cool with me | |
| 14:48:10 | efried | if you change the virt driver interface on stable, they'll need to patch. | |
| 14:48:29 | artom | efried, right, I'm sure they exist, just most likely not among our customers | |
| 14:48:42 | mriedem | artom: with your upstream hat on, you shouldn't care about your downstream customers and OOT issues | |
| 14:49:02 | mriedem | iow, saying "our customers won't care" isn't a valid reason to not do it | |
| 14:49:11 | efried | artom: "our" meaning RH? | |
| 14:49:18 | mriedem | the upstream argument would be, "oot? f'em" | |
| 14:49:23 | mriedem | efried: yes | |
| 14:50:01 | efried | I don't know what qualifies as a RH "customer", but the products that are based on the oot PowerVM driver are packaged as some form of RH distribution. | |
| 14:50:15 | bauzas | mriedem: okay, I was thinking we could be good citizens and notify OOT maintainers, like asked in https://review.opendev.org/#/c/589085/ | |
| 14:50:15 | efried | so whoever's using them is a RH "customer" in some capacity. | |
| 14:50:40 | bauzas | anyway, I need to leave | |
| 14:51:30 | stephenfin | Who's a good person to review resize flow bugfixes? https://review.opendev.org/#/c/662522/ | |
| 14:52:10 | mriedem | stephenfin: i want to say alex_xu already had a bug / patch for that | |