| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-12 | |||
| 18:45:09 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add notification support for trusted_certs https://review.openstack.org/563269 | |
| 18:45:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add certificate validation docs https://review.openstack.org/560158 | |
| 19:13:23 | openstackgerrit | Jay Pipes proposed openstack/nova master: add consumers generation field https://review.openstack.org/557958 | |
| 19:13:24 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: Allocation.consumer field https://review.openstack.org/565405 | |
| 19:13:25 | openstackgerrit | Jay Pipes proposed openstack/nova master: rework allocation handler _allocations_dict() https://review.openstack.org/565407 | |
| 19:13:26 | openstackgerrit | Jay Pipes proposed openstack/nova master: Add a microversion for consumer generation support https://review.openstack.org/565604 | |
| 19:13:27 | openstackgerrit | Jay Pipes proposed openstack/nova master: Fix nits from change Id609789ef6b4a4c745550cde80dd49cabe03869a https://review.openstack.org/574324 | |
| 19:13:44 | jaypipes | mriedem: ^ rebased and merge-conflict-fixed. | |
| 19:16:28 | mriedem | jaypipes: https://review.openstack.org/#/c/557958/29..30/nova/db/sqlalchemy/api_models.py | |
| 19:24:35 | melwitt | mriedem: great, thanks for swapping the runway | |
| 19:28:14 | openstackgerrit | Jay Pipes proposed openstack/nova master: add consumers generation field https://review.openstack.org/557958 | |
| 19:28:15 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: Allocation.consumer field https://review.openstack.org/565405 | |
| 19:28:16 | openstackgerrit | Jay Pipes proposed openstack/nova master: rework allocation handler _allocations_dict() https://review.openstack.org/565407 | |
| 19:28:17 | openstackgerrit | Jay Pipes proposed openstack/nova master: Add a microversion for consumer generation support https://review.openstack.org/565604 | |
| 19:28:18 | openstackgerrit | Jay Pipes proposed openstack/nova master: Fix nits from change Id609789ef6b4a4c745550cde80dd49cabe03869a https://review.openstack.org/574324 | |
| 19:28:20 | jaypipes | mriedem: sorry about that. fixed. | |
| 19:53:14 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: DNM: Depends on intel-nfv-ci-tests to test NUMA LM https://review.openstack.org/574872 | |
| 20:00:20 | openstackgerrit | Merged openstack/nova master: Refactor libvirt get_memory_used_mb() https://review.openstack.org/571030 | |
| 20:31:59 | mriedem | dansmith: i'm assuming you could have cert validation setup on kernel and ramdisk images...? | |
| 20:32:12 | mriedem | because this libvirt driver change isn't passing the trusted certs through in that case | |
| 20:32:19 | mriedem | not sure if that was intentional | |
| 20:32:22 | dansmith | mriedem: um | |
| 20:32:37 | dansmith | mriedem: if you use that kind of image, each would have its own signature | |
| 20:32:40 | mriedem | in general, i assume any image we download from glance could be signed and verified | |
| 20:32:50 | dansmith | aren't we buried deep in the image code enough that we'd verify those like anything else/ | |
| 20:33:16 | mriedem | i think so https://review.openstack.org/#/c/561262/27/nova/virt/libvirt/driver.py@7432 | |
| 20:33:28 | mriedem | this is the trouble part that's missing passing through trusted_certs | |
| 20:34:20 | dansmith | mriedem: you know I didn't write any of that code right? | |
| 20:34:49 | mriedem | IT'S YOUR GD CODE NOW SMITH | |
| 20:34:59 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Add InstanceNUMATopology to LibvirtLiveMigrateData https://review.openstack.org/566398 | |
| 20:35:00 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: libvirt live migration: fit NUMA topology on dest https://review.openstack.org/567242 | |
| 20:35:01 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Service version check for NUMA live migration https://review.openstack.org/566723 | |
| 20:35:02 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: DNM: Depends on intel-nfv-ci-tests to test NUMA LM https://review.openstack.org/574872 | |
| 20:35:05 | mriedem | your prints are all over this | |
| 20:35:17 | mriedem | it's 1/8 my baby now too | |
| 20:35:22 | dansmith | I really have lost most of my context on it at this point | |
| 20:35:28 | dansmith | I just split this off from the main patch for them | |
| 20:35:32 | mriedem | i know | |
| 20:35:38 | mriedem | and this change could be split into 2 now too | |
| 20:35:43 | mriedem | virt changes and then compute manager changes | |
| 20:35:45 | mriedem | but i'm not going to do that | |
| 20:37:05 | dansmith | yeah, so I guess you're right, since they don't pass trusted_certs there, | |
| 20:37:18 | dansmith | but if they did what you suggest (which I'm pretty sure I had suggested already) then they'd auto-verify | |
| 20:39:30 | mriedem | yeah i moved the low-level calls to libvirt_utils.fetch_image to just use the in-scope instance variable and pass instance.trusted_certs rather than add a new parameter to the various _try_fetch_image* methods | |
| 20:39:37 | mriedem | that's how i found this bug | |
| 20:45:12 | mriedem | gdi, i can't really tell how to assert anything that is mocked out from ImageBackendFixture | |
| 20:50:52 | efried | mriedem: example? | |
| 20:51:31 | mriedem | don't have the energy | |
| 20:51:46 | efried | you want to assert that the _create_ephemeral mock was called? | |
| 20:51:46 | efried | self.mock_create_ephemeral)) | |
| 20:51:46 | efried | 'nova.virt.libvirt.driver.LibvirtDriver._create_ephemeral', | |
| 20:51:46 | efried | self.useFixture(fixtures.MonkeyPatch( | |
| 20:51:46 | efried | mock.create_autospec(driver.LibvirtDriver._create_ephemeral) | |
| 20:51:46 | efried | self.mock_create_ephemeral = \ | |
| 20:51:46 | efried | mriedem: You mean from something like | |
| 20:52:06 | mriedem | more or less yes, | |
| 20:52:07 | mriedem | was doing: | |
| 20:52:10 | efried | mriedem: You have to make the fixture save off the actual mock | |
| 20:52:13 | mriedem | img_backend_fixture = snaz | |
| 20:52:16 | mriedem | ... | |
| 20:52:29 | mriedem | img_backend_fixture.mocked_thing.assert_called_once_with(...) | |
| 20:52:42 | mriedem | and tried mocked_thing.return_value | |
| 20:53:02 | mriedem | i thought the autospeced mock was being saved in those variables | |
| 20:53:09 | efried | mriedem: So like: | |
| 20:53:10 | efried | self.mriedem_mock = fx.mock | |
| 20:53:10 | efried | self.mock_create_ephemeral)) | |
| 20:53:10 | efried | 'nova.virt.libvirt.driver.LibvirtDriver._create_ephemeral', | |
| 20:53:10 | efried | fx = self.useFixture(fixtures.MonkeyPatch( | |
| 20:53:10 | efried | mock.create_autospec(driver.LibvirtDriver._create_ephemeral) | |
| 20:53:10 | efried | self.mock_create_ephemeral = \ | |
| 20:53:36 | mriedem | ok, well... | |
| 20:53:43 | mriedem | i'm past caring really | |
| 20:55:04 | efried | mriedem: You're right, though, that create_autospec ought to be a mock you can assert on. | |
| 20:55:28 | mriedem | i just hate the libvirt driver unit tests | |
| 20:55:31 | mriedem | but that's not news | |
| 20:55:40 | efried | mriedem: I've got +2s on the bottom nine patches in the neutron port binding series. | |
| 20:55:52 | mriedem | hot diggity dog | |
| 20:56:00 | mriedem | how many patches are there? 25? | |
| 20:56:12 | efried | mriedem: I'm going to punt on the libvirt ones, at least unless someone begs. | |
| 20:56:25 | efried | looks like 15 | |
| 20:56:26 | mriedem | https://review.openstack.org/#/q/topic:bp/neutron-new-port-binding-api+(status:open+OR+status:merged) | |
| 20:56:52 | efried | yeah, those all show up in the series | |
| 20:57:08 | mriedem | thanks for hitting those, even before it was in a runway | |
| 20:57:15 | efried | yahyoubetcha. | |
| 20:57:25 | mriedem | despite me wanting to murder you for awhile on the ksa stuff... :) | |
| 21:00:50 | efried | mriedem: If I had a shiny nickel for every time someone wanted to murder me... | |
| 21:02:17 | efried | mriedem: Here's some great news on ksa adapter, btw. Once https://review.openstack.org/#/c/574784/ is in a release, we can stop sending raise_exc=False with every request! | |
| 21:05:39 | mriedem | small victories | |
| 21:07:13 | mriedem | you know, | |
| 21:07:18 | mriedem | this trusted certs thing, | |
| 21:07:28 | mriedem | doesn't seem like something that should be plumbed through the various virt drivers at all | |
| 21:07:32 | mriedem | should/needs to be | |
| 21:07:47 | mriedem | but that's how we eventually get from compute manager to image api download() | |
| 21:07:56 | mriedem | kind of sucks though | |
| 21:08:18 | mriedem | it doesn't have anything specific to do with the virt drivers at all | |
| 21:08:20 | mriedem | nor the guest | |
| 21:10:00 | mriedem | like, could we do something like monkey patch or partial the image download api method from the compute manager flows that call driver.spawn() and pass the instance.trusted_certs right there to download()? | |
| 21:10:06 | mriedem | so the virt drivers don't need to plumb all that in | |
| 21:10:23 | mriedem | dansmith: ^? | |
| 21:10:57 | dansmith | I mean | |