| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-09 | |||
| 14:44:38 | mriedem | ildikov: i posted it at 11pm and had fixed what i knew about, but haven't looked at the results yet this morning | |
| 14:45:12 | ildikov | mriedem: http://logs.openstack.org/47/271047/48/check/openstack-tox-py27/991a4d8/testr_results.html.gz | |
| 14:45:30 | mriedem | i'll look later, in the middle of something | |
| 14:45:51 | ildikov | mriedem: I can fix these, upload and then we can agree who does what onwards, if that helps | |
| 14:45:56 | mriedem | sure | |
| 14:46:04 | ildikov | mriedem: ok, will do that then, tnx | |
| 14:46:29 | ildikov | mriedem: I just wanted to ask to avoid overwriting each others' versions... | |
| 14:47:34 | mriedem | yup | |
| 14:53:02 | mriedem | mdbooth: clearly this has to be wrong right? https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L1669 | |
| 14:53:06 | mriedem | and source_type not in ('lvm') | |
| 14:54:42 | mriedem | i guess it does work as a substring match | |
| 14:54:48 | mriedem | probably by accident | |
| 14:55:38 | mdbooth | mriedem: I hadn't even spotted the 'strings are a list' thing... | |
| 14:56:17 | openstackgerrit | Ameed Ashour proposed openstack/nova master: change instance_system_metadata column type https://review.openstack.org/526900 | |
| 14:56:40 | mdbooth | Yeah, looks like it probably works accidentally | |
| 14:57:45 | mdbooth | >>> 'foo' in ('foobar',) | |
| 14:57:46 | mdbooth | False | |
| 14:58:36 | mriedem | yeah, just getting another set of eyes, just noticed it while looking at something else | |
| 15:02:31 | alex_xu_ | sdague: as I remember we forze the '/os-migrations' API, then we want to add links in the '/os-migrations' API to the '/servers/{uuid}/migrations', is it right? https://review.openstack.org/#/c/459483/ | |
| 15:03:10 | mriedem | alex_xu_: yes, but that was pretty short sighted | |
| 15:03:24 | mriedem | as that change was specifically for the change to abort a running live migration for a given instance | |
| 15:03:33 | mriedem | there is still use in the os-migrations API | |
| 15:03:55 | mriedem | for example, it's the only one that you can use to list not in progress migrations | |
| 15:06:58 | openstackgerrit | Lee Yarwood proposed openstack/nova master: rbd: flatten images when creating/unshelving an instance https://review.openstack.org/457886 | |
| 15:11:27 | mriedem | dansmith: i'm +2 on the migration paging/filtering series https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/add-pagination-and-change-since-for-migration-list | |
| 15:11:34 | mriedem | that includes the cross-cell lister | |
| 15:11:43 | dansmith | mriedem: okay | |
| 15:19:56 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: don't attempt to live snapshot paused instances https://review.openstack.org/532214 | |
| 15:20:04 | mriedem | sdague: does this look familiar? ^ i now remember why i added the PAUSED condition to the patch to enable libvirt live snapshot by default | |
| 15:20:08 | mriedem | it doesn't work on older libvirt/qemu | |
| 15:25:20 | openstackgerrit | sahid proposed openstack/nova master: hardware: only take into account small pages https://review.openstack.org/532168 | |
| 15:29:59 | mriedem | stephenfin: simple reno update https://review.openstack.org/#/c/531963/ | |
| 15:30:10 | mriedem | i was reading the release notes last night and when you put them together, the context is lost on some of them | |
| 15:30:18 | kashyap | mriedem: A quick aside: Follow up from your test that the 'shareable' disk flag "doesn't work when using raw images either http://paste.openstack.org/show/639185/" | |
| 15:30:32 | kashyap | It was a real bug apparently and fixed in libvirt 3.10 and above. (All the messy dependent bugs are linked here: https://bugzilla.redhat.com/show_bug.cgi?id=1378242) (libvirt patch series: "[PATCH v2 00/11] qemu: Fix |
|
| 15:30:33 | openstack | bugzilla.redhat.com bug 1378242 in libvirt "QEMU image file locking (libvirt)" [Unspecified,On_qa] - Assigned to pkrempa | |
| 15:31:52 | mriedem | kashyap: ok just reading https://bugzilla.redhat.com/show_bug.cgi?id=1378242#c21 | |
| 15:31:53 | openstack | bugzilla.redhat.com bug 1378242 in libvirt "QEMU image file locking (libvirt)" [Unspecified,On_qa] - Assigned to pkrempa | |
| 15:32:12 | mriedem | kashyap: but yeah, for now the change for multiattach in the libvirt driver only supports multiattach if qemu<2.10 or libvirt>=3.10 | |
| 15:32:23 | kashyap | mriedem: BTW, about that snapshots and paused instances; I already thought it was fixed earlier, maybe I'm hallucinating | |
| 15:32:40 | mriedem | https://review.openstack.org/#/c/267587/83/nova/virt/libvirt/driver.py | |
| 15:32:44 | kashyap | But see the version dependancy you document in the commit message. | |
| 15:33:01 | kashyap | mriedem: Noted (about the multi-attach change) | |
| 15:33:37 | mriedem | as for this test https://review.openstack.org/#/c/267587/83/nova/tests/unit/virt/libvirt/test_driver.py@19520 - i can't sort out the global mock nature that makes it fail when running the entire test module | |
| 15:33:56 | mriedem | https://review.openstack.org/#/c/267587/83/nova/tests/unit/virt/libvirt/test_driver.py@19520 is a challenge for mdbooth | |
| 15:35:21 | mdbooth | mriedem: Have you done the bisect thing? | |
| 15:35:25 | mriedem | mdbooth: nope | |
| 15:35:45 | mdbooth | I believe the instructions were originally written by sdague | |
| 15:35:46 | mriedem | i thought that splitting the tests out into it's own class would help so they weren't running with the other tests in LibvirtConnTestCase but that didn't help | |
| 15:36:49 | mdbooth | mriedem: This one: https://wiki.openstack.org/wiki/Testr#Reproducing_Failures | |
| 15:37:39 | mriedem | oh cool, i'll try that | |
| 15:37:41 | mdbooth | The output of that process is a minimal test ordering which causes the failure. | |
| 15:37:49 | mdbooth | Then you just squint at it for a bit. | |
| 15:38:09 | mriedem | johnthetubaguy: can you drop the -2 on this? https://review.openstack.org/#/c/140733/ | |
| 15:38:47 | mriedem | johnthetubaguy: also, i plumbed in the multiattach API functional changes last night - https://review.openstack.org/#/c/271047/ - still need to cleanup some test fallout from some refactor that was needed, but it should be ok to start reviewing the flow | |
| 15:38:48 | johnthetubaguy | mriedem: done | |
| 15:39:09 | mriedem | you have to use a new microversion to bfv or attach with a multiattach volume, | |
| 15:39:11 | johnthetubaguy | mriedem: ah cool, I am chasing some ironic traits stuff today, but will try get to that soon | |
| 15:39:16 | mriedem | and then we have min compute version checks and such | |
| 15:39:27 | johnthetubaguy | mriedem: all sounds sensible | |
| 15:39:43 | mriedem | it got a bit gross because i had to change a volume_id param to a volume dict in one of the lower down methods | |
| 15:40:03 | mriedem | to avoid multiple volume GET calls | |
| 15:41:30 | ildikov | mriedem: johnthetubaguy: the functional tests run into a KeyError on the volume which should be an easy fix if I can finally find the spot where it actually goes wrong... :) | |
| 15:41:49 | mriedem | ildikov: it's probably a GET response from the CinderFixture | |
| 15:41:55 | ildikov | mriedem: was it multiple GET calls? | |
| 15:42:09 | ildikov | mriedem: the fakes returns a volume dict with 'multiattach' | |
| 15:42:09 | breton | hi | |
| 15:42:24 | ildikov | mriedem: or I found another volume dict that contained it... | |
| 15:42:32 | mriedem | ildikov: https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L1332 | |
| 15:42:38 | mriedem | ^ doesn't contain multiattach=False | |
| 15:43:02 | mriedem | nor do the ones below | |
| 15:43:15 | mriedem | same thing in CinderFixtureNewAttachFlow | |
| 15:43:57 | mriedem | ildikov: i could have made the API code a lot of easier on tests if i just did volume.get('multiattach') but that's a cop out since the volume response from cinder's API should always have that key set | |
| 15:44:38 | breton | Is liberty affected by https://bugs.launchpad.net/nova/+bug/1664931? Is there any sense in porting https://review.openstack.org/#/c/519684/7 ? | |
| 15:44:40 | openstack | Launchpad bug 1664931 in OpenStack Compute (nova) pike "[OSSA-2017-005] nova rebuild ignores all image properties and scheduler filters (CVE-2017-16239)" [High,Fix committed] - Assigned to Matt Riedemann (mriedem) | |
| 15:45:09 | ildikov | mriedem: the test calls this: https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/fakes.py#L618 | |
| 15:45:16 | mriedem | breton: the OSSA-2017-005 calls out the affected versions | |
| 15:45:27 | ildikov | mriedem: which then goes here: https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/fakes.py#L568 | |
| 15:45:52 | mriedem | ildikov: i thought you were asking about functional tests | |
| 15:46:16 | mriedem | https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/fakes.py#L586 should have probably never been True | |
| 15:46:39 | breton | mriedem: so do i understand it right that all versions <=14.0.10 are affected? | |
| 15:46:43 | ildikov | mriedem: I do | |
| 15:47:35 | ildikov | mriedem: https://github.com/openstack/nova/blob/master/nova/tests/functional/api_sample_tests/test_volumes.py#L247 | |
| 15:48:22 | ildikov | mriedem: and just the old flow tests fail | |
| 15:48:41 | mriedem | breton: https://github.com/openstack/nova/commit/d6ca1cc02ab58615a6bb1b337db34dd06525112e introduced the regression, | |
| 15:48:42 | breton | mriedem: i am unsure because in liberty request specs were not used for instance rebuild | |
| 15:48:45 | mriedem | in 12.0.0 which was liberty | |
| 15:48:59 | ildikov | mriedem: and I'm surely just blind and hate all the fake stuff... | |
| 15:49:38 | mriedem | ildikov: oh i didn't realize the functional tests were using unit test stubs.... | |
| 15:49:46 | mriedem | they should be using the CinderFixture, but that's not a change to make here | |
| 15:50:12 | ildikov | I got surprised too :) | |
| 15:50:54 | breton | mriedem: understood, thanks | |
| 15:51:41 | mriedem | ildikov: https://github.com/openstack/nova/blob/master/nova/tests/functional/api_sample_tests/test_volumes.py#L368 | |
| 15:51:54 | mriedem | the 2.49 test is using the CinderFixture as well as the fakes/stubs | |
| 15:52:14 | mriedem | and the fixture is loaded after the stubs in the parent class setup | |
| 15:52:19 | mriedem | so the fixture overrides the GET call | |
| 15:52:24 | mriedem | and the fixture doesn't have multiattach in the response | |
| 15:52:30 | mriedem | so just fix the fixtures since you have to do that anyway | |
| 15:52:49 | ildikov | the test seemed to call it explicitely | |
| 15:52:54 | ildikov | anyway, will fix that | |