| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-16 | |||
| 14:32:50 | efried | tell | |
| 14:33:23 | stephenfin | https://review.opendev.org/#/c/671801/50/nova/scheduler/manager.py@191 | |
| 14:33:33 | stephenfin | we're using extend, which modifies a list in place | |
| 14:33:52 | stephenfin | and our mock is returning 'fakes.ALLOC_REQS' | |
| 14:34:15 | efried | oy vay | |
| 14:34:17 | stephenfin | so that's getting modified by the non-disabled fallback test | |
| 14:34:40 | efried | stephenfin: I think I saw those globals only used by this one test suite? | |
| 14:34:53 | stephenfin | correct | |
| 14:35:11 | efried | so make 'em instance vars, for future safety | |
| 14:35:44 | stephenfin | wdym? | |
| 14:35:57 | stephenfin | I was doing to do 'fakes.ALLOC_REQS[:]' | |
| 14:36:10 | stephenfin | though I could add a 'get_fake_alloc_reqs' helper too | |
| 14:36:11 | efried | I mean, you could deepcopy 'em to fix this problem, but it's just going to bite us in the ass again later, somewhere else. | |
| 14:36:26 | efried | global test artifacts bad | |
| 14:36:32 | stephenfin | v bad. | |
| 14:36:43 | efried | first 205 lines of fakes, bad. | |
| 14:37:59 | efried | stephenfin: I guess for now you could just make it a helper method that returns a fresh new copy every time | |
| 14:38:08 | efried | but if it were me, I would make it completely fresh | |
| 14:38:27 | efried | def get_fake_alloc_reqs(): | |
| 14:38:27 | efried | return { $everything } | |
| 14:38:55 | efried | rather than | |
| 14:38:55 | efried | EVIL_GLOBAL | |
| 14:38:55 | efried | def get_fake_alloc_reqs(): | |
| 14:38:55 | efried | return some_unreliable_copy_method(EVIL_GLOBAL) | |
| 14:39:05 | stephenfin | gotcha | |
| 14:39:07 | stephenfin | coming right up | |
| 14:39:17 | efried | this is gonna be partway down the series, yah? | |
| 14:39:29 | stephenfin | yeah, just before that patch. I'll do it separately | |
| 14:39:57 | efried | stephenfin: you're going to have to fix the patch anyway, so might as well do it in place, nah? | |
| 14:40:09 | efried | don't try to fix the race afterward | |
| 14:40:39 | stephenfin | I meant add the helper function in a precursor patch and modify the intermittently failing patch to use it | |
| 14:40:42 | efried | tbc, the patch in question is "Add support for translating CPU policy extra specs, image meta" | |
| 14:40:42 | stephenfin | but I can combine too | |
| 14:41:03 | efried | yeah, just combine, help me justify a fast approve | |
| 14:41:08 | stephenfin | ack | |
| 14:47:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add support for translating CPU policy extra specs, image meta https://review.opendev.org/671801 | |
| 14:47:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: fakelibvirt: Make 'Connection.getHostname' unique https://review.opendev.org/681060 | |
| 14:47:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Mock 'libvirt_utils.file_open' properly https://review.opendev.org/681061 | |
| 14:47:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add reshaper for PCPU https://review.opendev.org/674895 | |
| 14:47:46 | stephenfin | efried: ^ | |
| 14:48:06 | stephenfin | I'll remove the rest of those global fakes now (separate patch) | |
| 14:48:24 | efried | ++ | |
| 14:48:47 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Centralize volume create code during boot from volume https://review.opendev.org/682378 | |
| 14:48:51 | mriedem | ^ is a simple refactor split off from https://review.opendev.org/#/c/541420/ which has been around since february of 2018, | |
| 14:49:06 | mriedem | and is important if y'all ever want to drop the legacy volume attachment compat code | |
| 14:50:57 | efried | stephenfin: +A, and re+W up the pile. Nice work, thank you. | |
| 14:59:24 | stephenfin | mriedem: done | |
| 15:00:13 | efried | mriedem: one adjustment requested pls | |
| 15:00:17 | mriedem | doing it | |
| 15:00:28 | efried | alex_xu: yt? | |
| 15:06:32 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Centralize volume create code during boot from volume https://review.opendev.org/682378 | |
| 15:06:32 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Create volume attachment during boot from volume in compute https://review.opendev.org/541420 | |
| 15:09:30 | efried | mriedem: are we merging stuff like that ^ at this point or waiting for ussuri to fork? | |
| 15:09:51 | mriedem | which one? the refactor is trivial and i've added the latter to https://etherpad.openstack.org/p/nova-train-release-todo | |
| 15:10:06 | mriedem | as i said, it's been around forever without much core review outside melwitt | |
| 15:10:12 | mriedem | the mox->mock stuff in the tests blew it all up | |
| 15:10:29 | mriedem | but if we ever want to migrate off the legacy volume attach code, we need to be creating all volumes with the new style attachment stuff | |
| 15:10:56 | mriedem | iow, the longer we wait, the bigger the data migration is going to be | |
| 15:11:10 | mriedem | e.g. https://review.opendev.org/#/c/549130/ | |
| 15:11:40 | mriedem | i don't expect to get ^ into train at this point | |
| 15:11:46 | mriedem | nor is it probably the only way to skin that cat | |
| 15:12:29 | mriedem | at some point in the future we can add a nova-status upgrade check and fail to start if you haven't migrated old bdm records | |
| 15:12:50 | mriedem | and then drop all that compat code | |
| 15:15:44 | mriedem | melwitt: do you want to send this through? https://review.opendev.org/#/c/677736/ | |
| 15:16:19 | melwitt | I do. thanks | |
| 15:26:06 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Follow up for the bandwidth series https://review.opendev.org/682389 | |
| 15:26:29 | gibi | mriedem, efried: two smallish nits for the bandwidth series. I have two other testing enhancements on my TODO list that I plan to propose this week. None of these is critical, just nice to have. | |
| 15:27:17 | mriedem | gibi: i got the fup | |
| 15:27:37 | gibi | mriedem: thanks | |
| 15:27:55 | gibi | mriedem: and thanks for the review on the bug fix https://review.opendev.org/#/c/666857/ I will respin that | |
| 15:36:03 | bauzas | gibi: I can help you :) | |
| 15:56:29 | gibi | bauzas: thanks | |
| 17:26:10 | openstackgerrit | Eric Fried proposed openstack/nova master: objects: use all_things_equal from objects.base https://review.opendev.org/681397 | |
| 17:39:31 | openstackgerrit | Merged openstack/nova master: Deprecate the XenAPIDriver https://review.opendev.org/680732 | |
| 18:20:09 | openstackgerrit | Merged openstack/nova master: libvirt: Fix service-wide pauses caused by un-proxied libvirt calls https://review.opendev.org/677736 | |
| 18:25:33 | sean-k-mooney | cool that ^ will make mdbooth happy | |
| 18:27:04 | sean-k-mooney | by the way can i get a second +2+w on https://review.opendev.org/#/c/670585/ | |
| 18:27:39 | sean-k-mooney | i really want to ensure that is deprecated in train so we can remove it in Ussuri | |
| 18:27:52 | sean-k-mooney | as in like 2 weeks if not sooner | |
| 18:29:15 | sean-k-mooney | dansmith: mriedem: melwitt: could one of ye take a look when ye have time ^ | |
| 20:03:02 | mriedem | sean-k-mooney: commented, | |
| 20:03:08 | mriedem | and added moshe and adrianc to it | |
| 20:04:22 | sean-k-mooney | thanks. if stephen does not adress the docs bugs ill do them tomorrow | |
| 20:05:26 | sean-k-mooney | mriedem: assuming no objection from moshe or adrianc are you generally ok with this? | |
| 20:06:41 | mriedem | i can't say i'm very familiar with the pci passthrough whitelist dev name stuff | |
| 20:06:51 | mriedem | or how much it's used | |
| 20:07:15 | sean-k-mooney | its an alternitive to useing the pci addres of vendor id and product id | |
| 20:07:40 | sean-k-mooney | the issue with is if you restart the compute agnet while its passed to a vm the device wont be found | |
| 20:08:03 | sean-k-mooney | we recently prevented the compute agent from removeing in use device from the db | |
| 20:08:11 | efried | mriedem: guessing that az fail is yet another global test var race. We should go on a crusade to murder all of those. | |
| 20:08:45 | sean-k-mooney | but we didnt always, also the name can change after you release teh VF/PF form teh vm | |
| 20:09:30 | sean-k-mooney | so the main issue is its not realible and we keep getting bug downstream that we cant fix | |
| 20:09:39 | sean-k-mooney | so we just want to stop supproting it | |
| 20:10:19 | mriedem | efried: is this a recent failure? b/c i'm not sure what is global about this one and it's been around awhile (the test that is) | |
| 20:10:34 | efried | mriedem: from what I could tell, it started hitting on 9/10 | |
| 20:10:53 | efried | mriedem: I'm looking at the delta in test_aggregates.py for I9ab9d7d65378be564b3731b5227ede8cece71bef | |
| 20:11:06 | efried | https://review.opendev.org/#/c/671075/21/nova/tests/functional/test_aggregates.py | |
| 20:13:32 | mriedem | hmm, yeah probably related to that series | |
| 20:13:58 | artom | sean-k-mooney, https://review.opendev.org/#/c/682435/3 | |
| 20:14:04 | efried | never mind, that one only merged yesterday. | |