| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-09 | |||
| 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 | |
| 15:53:18 | mriedem | the test calling fakes.stub_volume_get and the actual api runtime code calling it via the fixture are different thigns | |
| 15:53:19 | mriedem | *things | |
| 15:53:51 | ildikov | or in other words it's a mess :) | |
| 15:54:19 | mriedem | yeah, the functional tests shouldn't be using stubs from the unit tests, they should use the fixture long-term | |
| 15:54:32 | mriedem | it's ultimately trading one set of stubs for another | |
| 15:57:19 | smcginnis | mriedem: Are these Cinder or Nova tests you are referring to? | |
| 15:57:26 | mriedem | nova | |
| 15:57:44 | Roamer` | johnthetubaguy, now that mriedem seems to be kinda sorta okay with the StorPool libvirt volume driver (thanks!) and it passed the Zuul checks (yay!), could you remove your procedural -2? thanks in advance! | |
| 15:57:52 | smcginnis | OK, good. | |
| 15:57:56 | mriedem | Roamer`: he already did | |
| 15:58:20 | Roamer` | mriedem, johnthetubaguy, ahhhhh, right... he did indeed... thanks! | |
| 15:58:26 | Roamer` | (today is NOT my day for reading and parsing stuff) | |
| 16:02:19 | ildikov | smcginnis: got scared for a moment? :) | |
| 16:04:38 | smcginnis | ildikov: Yep. ;) | |
| 16:04:59 | smcginnis | ildikov: Our tests already need a lot of work. I was just hoping that it wasn't worse than I already knew. | |
| 16:05:24 | ildikov | smcginnis: I know that part :) | |
| 16:05:39 | ildikov | smcginnis: but you know no one is perfect, even Nova has it's flows :) | |
| 16:05:46 | ildikov | s/it's/its/ | |
| 16:05:53 | smcginnis | ildikov: Shh, you can't say that over here. :D | |
| 16:06:37 | ildikov | smcginnis: they all know I'm just expressing my love :) | |
| 16:11:12 | tovin07 | mriedem, can you take a quick look at this small fix (already had +2 from stephenfin): https://review.openstack.org/#/c/519664/ | |
| 16:13:19 | stephenfin | mriedem: Done (the release note) | |
| 16:13:57 | openstackgerrit | Merged openstack/nova master: Move aggregates from report client to ProviderTree https://review.openstack.org/521685 | |
| 16:19:17 | bauzas | is it me or the gate is in the weeds ? | |
| 16:19:36 | bauzas | I'm still waiting a single change to be merged since this morning | |
| 16:26:30 | mriedem | edleafe: comments on the alternate hosts patch for resize https://review.openstack.org/#/c/526436/21 | |
| 16:27:01 | mriedem | bauzas: thankfully you can take this opportunity to review some patches | |
| 16:27:37 | hrw | guys: where 'default' VM is defined? Nova decides what to include or does it just use defaults from libvirt? | |
| 16:27:43 | efried | bauzas Yeah, https://review.openstack.org/#/c/521685/ took 8.5h to merge this morning. | |
| 16:28:18 | hrw | I mean things like networking, graphics, usb, storage. not amount of ram/vcpu/storage | |
| 16:29:13 | openstackgerrit | Merged openstack/nova master: Fix race condition in retrying migrations https://review.openstack.org/531022 | |
| 16:30:15 | bauzas | mriedem: yup, I'm just working on a new PS for the vGPU change, but I hope to look at reviewing changes by tomorrow | |
| 16:32:34 | stephenfin | lyarwood: Have you got this the right way round? https://review.openstack.org/#/c/530744/ | |
| 16:33:47 | stephenfin | Wait - O comes before P | |
| 16:34:27 | lyarwood | :D | |
| 16:37:56 | mriedem | tovin07: done | |
| 16:38:09 | edleafe | mriedem: thx. Found a bug in my own code, so I'm working on fixing it | |
| 16:39:12 | cdent | functional tests love you more | |
| 16:42:18 | mriedem | efried: powervm ci is going to use SEA rather than OVS for networking right? | |
| 16:42:35 | efried | For the immediate future, yes. esberglu ^ | |
| 16:42:52 | mriedem | so what networking is this running against? https://review.openstack.org/#/c/422512/ | |
| 16:42:55 | mriedem | if SEA isn't yet there | |
| 16:43:35 | efried | I forget that answer. esberglu is on the way... | |
| 16:43:51 | mriedem | Configured mechanism driver names: ['openvswitch', 'linuxbridge'] | |
| 16:43:58 | openstackgerrit | Lee Yarwood proposed openstack/nova master: compute: Detach volumes on _rebuild_default_impl failure https://review.openstack.org/442105 | |
| 16:43:58 | mriedem | so maybe just the defaults for ML2? | |
| 16:45:17 | lyarwood | stephenfin / mdbooth; if you have any review time left today https://review.openstack.org/#/c/460243/ has been respun to include the fix from https://review.openstack.org/#/c/531233/ | |
| 16:46:38 | esberglu | mriedem: That's just running the CI we've been using since last release with no networking implementation | |
| 16:46:55 | esberglu | About to merge a patch that fixes SEA CI | |
| 16:48:18 | efried | esberglu IOW, neither network patch is yet running a CI that tests networking, but soon the SEA patch will run CI with SEA networking? | |
| 16:48:40 | efried | esberglu And... if we were to run the OVS patch once that is done, it would what, fail? | |
| 16:49:11 | esberglu | efried: No it would pass, but it would be testing SEA networking | |
| 16:49:24 | efried | esberglu How, without the SEA patch? | |
| 16:49:41 | esberglu | efried: PowerVM is running SEA CI on all nova changes as of yesterday | |
| 16:49:54 | efried | esberglu How, without the SEA patch? | |
| 16:50:07 | esberglu | It is applying the SEA patch | |
| 16:50:10 | efried | ahh | |
| 16:50:39 | esberglu | Hence the issue applying the SEA patch to the OVS and SEA changes, which we just fixed | |
| 16:50:43 | efried | that's what we were *just* working on, yeah :) | |
| 16:50:56 | efried | pieces come together. | |
| 16:51:04 | efried | mriedem Does this answer your question? | |
| 16:52:28 | mriedem | huh | |
| 16:52:29 | mriedem | ok | |
| 16:52:52 | mriedem | typically applying patches to the CI results for code that's not yet merged is a bit a of a no-no | |
| 16:53:34 | openstackgerrit | Claudiu Belu proposed openstack/nova master: tests: fixes mock autospec usage https://review.openstack.org/447505 | |
| 16:54:19 | efried | mriedem We have a chicken/egg here | |
| 16:55:25 | mriedem | well, that egg wasn't necessary in pike was it? | |
| 16:55:34 | esberglu | mriedem: No we did this in pike as well | |
| 16:55:40 | mriedem | or is the chicken the os-vif stuff added with the ovs change, which the SEA change relies on? | |
| 16:55:43 | mriedem | oh fun | |
| 16:57:55 | efried | mriedem Yeah, the SEA change is on top of the OVS change. Technically we would probably be able to wait to switch on the patching-and-SEA-networking part of the CI until after the OVS patch merges, but for the sake of getting both patches in the review pipeline as quickly as possible... | |
| 17:03:22 | gibi | mriedem: have you planned to join to the notification subteam meeting? | |
| 17:04:17 | stephenfin | claudiub: Want to look at https://review.openstack.org/#/c/447505/ so I can sign off on it? | |
| 17:05:15 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: WIP: [api] Allow multi-attach in compute api https://review.openstack.org/271047 | |
| 17:07:02 | gibi | mriedem: as nobody joined, I've just closed the meeting, we can discuss thing separatly if neede | |
| 17:08:06 | mriedem | gibi: sorry missed your ping | |
| 17:08:36 | stephenfin | lyarwood: Looked, but my head is gone. I'll hit it tomorrow morning | |
| 17:08:52 | lyarwood | stephenfin: no issues | |
| 17:18:59 | claudiub | stephenfin: done. just fyi. the patch merged in oslotest, and it is included in oslotest 3.2.0. right now, I'm still waiting for the global-requirements bump for oslotest, and then for the OpenStack Bot requirements update to be proposed to nova and be merged, Then finally, that patch is mergeable. :) | |
| 17:19:45 | efried | bauzas You still working on series https://review.openstack.org/#/c/528832/ locally? I see "Done" comments from ~8h ago but no new patch set. | |
| 17:20:09 | openstackgerrit | Claudiu Belu proposed openstack/nova master: tests: fixes mock autospec usage https://review.openstack.org/447505 | |
| 17:27:36 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: test to make sure volume_use_multipath is properly used https://review.openstack.org/532270 | |
| 17:27:53 | bauzas | efried: yup, I'm in there | |
| 17:28:15 | efried | k. | |
| 17:29:19 | bauzas | dansmith: around ? I have a thought | |
| 17:29:45 | bauzas | dansmith: I'm just building a dictionary of assigned mdevs but looking up the guests by init_host | |
| 17:30:05 | bauzas | dansmith: when I'm assigning a mdev to a guest, I'm also amending that dict | |