| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-01-23 | |||
| 09:57:18 | kashyap | (See the first one: test_get_guest_config_ppc64_through_image_meta_spice_enabled) | |
| 09:57:28 | kashyap | [...] | |
| 09:57:28 | kashyap | raise mismatch_error | |
| 09:57:28 | kashyap | testtools.matchers._impl.MismatchError: '<nova.virt.libvirt.config.LibvirtConfigMemoryBalloon object at 0x7f10ac940150>' is not an instance of LibvirtConfigGuestVideo | |
| 09:57:31 | kashyap | [...] | |
| 09:57:43 | openstackgerrit | caoxufeng proposed openstack/nova master: oslo.config==6.8.0 requires rfc3986>=1.2.0 and requirements project had made rfc3986==1.2.0 in upper-constraints.txt,so modify it . https://review.openstack.org/632391 | |
| 09:58:55 | mdbooth | kashyap: Looking at your patch | |
| 10:02:14 | mdbooth | kashyap: Ok, I can reproduce the failure locally | |
| 10:03:07 | kashyap | Nod | |
| 10:03:51 | kashyap | (There's a typo in the commit message: 1.3.1 --> 3.0.0) | |
| 10:04:01 | kashyap | (But that's besides the point.) | |
| 10:05:35 | mdbooth | kashyap: Ok, to start off with this is just a bad test. | |
| 10:05:53 | mdbooth | self._test_get_guest_config_ppc64(8) | |
| 10:06:17 | mdbooth | 8 is an uncommented bare constant, the best kind of constant | |
| 10:07:01 | mdbooth | It appears to describe the expected location of the video device in the ordered list of devices generates by _get_guest_config | |
| 10:07:17 | mdbooth | I can't imagine why that's a good idea | |
| 10:07:39 | kashyap | Yeah, there's many of those undocumented constants :-( | |
| 10:08:12 | kashyap | There are in total 33 failures, though. Still ploughing through | |
| 10:08:23 | mdbooth | So my expectation is that if we go dig in _get_guest_config, something in there is guarding the creation of devices based on libvirt or qemu version number | |
| 10:08:37 | mdbooth | Which breaks this test, which expects its device to be at exactly position 8 | |
| 10:08:53 | mdbooth | It's a stupid test, but unfortunately you have to fix it now | |
| 10:08:58 | kashyap | Yeah, I have that open, and looking at it | |
| 10:09:28 | kashyap | Had the same thing last year -- spent weeks fixing all kinds of unit test failures due to the bump | |
| 10:10:26 | kashyap | mdbooth: Thanks for the hint | |
| 10:10:31 | mdbooth | It's almost as bad as the 'code diff' tests we used to write, but fortunately seem to be going out of fashion now. | |
| 10:23:45 | kashyap | mdbooth: So if I changed the position from 8 --> 7 "it succeeds": | |
| 10:23:46 | kashyap | - self._test_get_guest_config_ppc64(8) | |
| 10:23:46 | kashyap | + self._test_get_guest_config_ppc64(7) | |
| 10:23:57 | kashyap | That "solution" makes me feel physically stupid. | |
| 10:24:55 | mdbooth | kashyap: If you do that: https://goo.gl/images/6ehtJv | |
| 10:25:23 | kashyap | :D | |
| 10:26:16 | mdbooth | kashyap: Although it might be the pragmatic thing to do. The problem is that the correct thing to do here is to determine what the test was actually trying to test, and rewrite it so that it tests it. | |
| 10:26:35 | mdbooth | That would require mental effort from the reviewer which you may find hard to come by. | |
| 10:26:44 | kashyap | Yeah, I will freely admit that I'm "still in the process of grokking it all" | |
| 10:26:58 | mdbooth | From experience, I can say that nobody likes reviewing long series of test fixes. | |
| 10:27:04 | kashyap | mdbooth: Yeah, I don't think anyone gives a damn thing about it | |
| 10:27:17 | kashyap | mdbooth: I will for now do the "pragmatic thing" (I feel really dirty doing it) | |
| 10:27:38 | mdbooth | At least write a comment containing an apology and blaming somebody else ;) | |
| 10:27:41 | kashyap | Because I hate that I don't understand the root cause entirely. I probably will if I spend my whole day duking around various files | |
| 10:27:46 | kashyap | mdbooth: Yeah, good point. | |
| 10:27:56 | kashyap | "Leave the trace for the poor sucker who comes later." | |
| 10:42:09 | openstackgerrit | caoxufeng proposed openstack/nova master: oslo.config==6.8.0 requires rfc3986>=1.2.0 and requirements project had made rfc3986==1.2.0 in upper-constraints.txt,so modify it . https://review.openstack.org/632391 | |
| 11:39:54 | openstackgerrit | Theodoros Tsioutsias proposed openstack/nova master: Add requested_networks to RequestSpec https://review.openstack.org/570201 | |
| 11:39:55 | openstackgerrit | Theodoros Tsioutsias proposed openstack/nova master: Add instance hard delete https://review.openstack.org/570202 | |
| 11:39:55 | openstackgerrit | Theodoros Tsioutsias proposed openstack/nova master: [WIP] Enable rebuild for instances in cell0 https://review.openstack.org/570203 | |
| 12:47:55 | kashyap | mdbooth: Tiny progress! | |
| 12:48:27 | kashyap | I at least see a "pattern" that some of these tests seem to fail only when I update FAKE_QEMU_VERSION | |
| 12:48:34 | kashyap | If that is unchanged, then the test succeeds. | |
| 12:50:01 | mdbooth | kashyap: But you need to change it... | |
| 12:50:32 | kashyap | Yes, yes. I was isolating the problem | |
| 12:50:47 | kashyap | mdbooth: For example, see this small PDB session w/ & without my patch for guest.devices: http://paste.openstack.org/show/743172/ | |
| 12:51:08 | kashyap | This is a different test that also fails due to: | |
| 12:51:10 | kashyap | self.assertEqual(2, len(guest.devices)) | |
| 12:51:19 | kashyap | [...] | |
| 12:51:24 | kashyap | testtools.matchers._impl.MismatchError: 2 != 1 | |
| 12:52:06 | kashyap | The fakelibvirt.py is "misbehaving" due to the version change. | |
| 12:59:59 | mdbooth | kashyap: There are way too many version checks for me to audit quickly. Honestly I'm not sure it's worth it, though. Could you not just change the test to scan the list of the devices for the one it's looking for? | |
| 13:05:47 | kashyap | I think I've got a lead; will update once done. | |
| 13:05:55 | kashyap | mdbooth: Please disregard me for now. | |
| 13:07:28 | mdbooth | kashyap: If I wanted to track it down quickly, I'd modify the unit test to spit out the device list, compare the generated devices with and without the bumped version number, then go find the guard in that code. | |
| 13:08:40 | kashyap | mdbooth: I don't know how to get the device list. | |
| 13:08:52 | kashyap | That's why I went the PDB route with and without my patch | |
| 13:09:16 | kashyap | And got that: dir(guest.devices[0]) output at the bottom: http://paste.openstack.org/raw/743168/ | |
| 13:09:35 | mdbooth | kashyap: Right, so you've worked out what device was removed? | |
| 13:10:07 | kashyap | 'port' seems to be missing on the second iteration | |
| 13:10:19 | kashyap | mdbooth: And ... weirdly enough, see this comment from migi on the review: | |
| 13:10:24 | kashyap | https://review.openstack.org/#/c/632507/2/nova/virt/libvirt/driver.py | |
| 13:11:16 | kashyap | He says updating MIN_QEMU_VIRTLOGD from 2.7.0 --> 2.9.0 "fixes" it. | |
| 13:11:31 | kashyap | But...that constant will also be removed, as the new MIN satisfies the condition. | |
| 13:11:45 | kashyap | If you're on something else, ignore it. Don't split your attention | |
| 13:29:15 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/rocky: Convert port to str when validate console port https://review.openstack.org/632720 | |
| 13:51:10 | kashyap | Haha, I was wondering why on _earth_ a particular console code behaviour is the way it is ... and the code comment points to a RHT Bugzilla. | |
| 13:51:37 | kashyap | I click it ... and it points to a bug I filed in Jan-2012! -- https://bugzilla.redhat.com/show_bug.cgi?id=781467 | |
| 13:51:38 | openstack | bugzilla.redhat.com bug 781467 in libvirt "RFE: virsh: 'console' support for non-PTY (unix, tcp)" [Medium,New] - Assigned to libvirt-maint | |
| 14:12:39 | openstackgerrit | Maciej Jozefczyk proposed openstack/nova master: Add fill_virtual_interface_list online_data_migration script https://review.openstack.org/614167 | |
| 14:12:39 | openstackgerrit | Maciej Jozefczyk proposed openstack/nova master: Force refresh instance info_cache during heal https://review.openstack.org/591607 | |
| 14:13:11 | jaypipes | belmoreira: good afternoon, sir. just wondering if you'd found any time to test out that single-query-per-cell instance info fetcher patch (https://review.openstack.org/#/c/623558/)? no worries if not, I know you're super busy | |
| 14:28:14 | kashyap | It is so cathartic to "remove" code... | |
| 14:28:32 | kashyap | Reminds me of the quote from UNIX hero, Douglas Mcilroy: https://en.wikipedia.org/wiki/Douglas_McIlroy#Views_on_computing | |
| 14:31:10 | cdent | kashyap: quite: https://review.openstack.org/#/c/618215/ | |
| 14:31:38 | kashyap | Also as Elements of Style says: "Omit needless words" | |
| 14:31:48 | kashyap | cdent: Whoa ... /me bows | |
| 14:31:56 | kashyap | It is nearing merging | |
| 14:32:17 | cdent | that's the smaller of two. the one to remove the placement tests (already merged) was more lines (but much of it yaml) | |
| 14:33:11 | kashyap | That `diff stat` ... nice work. I guess the new repo is shaping up nice and well | |
| 14:34:11 | cdent | yeah, external placement is what runs in the intergrated gate now | |
| 14:35:28 | kashyap | Naice | |
| 14:50:50 | openstackgerrit | Eric Fried proposed openstack/nova master: ksa auth conf and client for cyborg access https://review.openstack.org/631242 | |
| 15:09:45 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: [WIP] libvirt: Bump MIN_{LIBVIRT,QEMU}_VERSION for "Stein" https://review.openstack.org/632507 | |
| 15:46:27 | efried | cdent, melwitt: I probably won't make the nova IRC meeting tomorrow; will be on the road again. Not much worth reporting from the n-sch meeting. | |
| 15:47:03 | cdent | ✔ | |
| 15:47:44 | sean-k-mooney | cdent: do you keep that tick copy pasted or is there a keyboard sort cut for it | |
| 15:48:29 | cdent | i've got my keyboard setup to allow inputting unicode codepoints and have that one memorize opt/alt+2714 | |
| 15:59:00 | melwitt | efried: ack | |
| 16:02:30 | prometheanfire | new libvirt-python (5.0.0) has been proposed to upperconstaints, just an fyi | |
| 16:11:27 | kashyap | prometheanfire: Got the change URL handy? | |
| 16:11:51 | kashyap | The next min libvirt version for "Train" release is 4.0.0. | |
| 16:12:13 | kashyap | IIRC, we should stick to matching libvirt-python binding. | |
| 16:14:21 | prometheanfire | https://www.libvirt.org/news.html ? | |
| 16:14:56 | prometheanfire | kashyap: iirc libvirt-python maintains backwards compat | |