| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-15 | |||
| 14:29:22 | giblet | mriedem: I left some comments in https://review.openstack.org/#/c/500396 nothing serious | |
| 14:30:36 | mriedem | giblet: ok i can address those quick | |
| 14:37:20 | openstackgerrit | Eric Fried proposed openstack/nova master: Enforce placement minimum in nova.cmd.status https://review.openstack.org/568965 | |
| 14:40:28 | openstackgerrit | Matt Riedemann proposed openstack/python-novaclient master: Microversion 2.63 - Add trusted_image_certificates https://review.openstack.org/500396 | |
| 14:40:29 | mriedem | giblet: done ^ | |
| 14:41:09 | mriedem | giblet: if you're ok with that now, can you also go back to the support matrix docs change which is now dependent on the novaclient change and i'm using the nova boot CLI instead of OSC CLI https://review.openstack.org/#/c/574890/ | |
| 14:42:54 | openstackgerrit | Merged openstack/nova stable/queens: mock utils.execute() in qemu-img unit test https://review.openstack.org/574255 | |
| 14:44:26 | fried_rice | Looks like I may need a second opinion on https://review.openstack.org/#/c/575277/ -- am I being a dick asking for UT there? | |
| 14:45:11 | superdan | without looking, no. | |
| 14:47:32 | giblet | mriedem: looking | |
| 14:47:36 | giblet | mriedem: at both | |
| 14:48:31 | melwitt | fried_rice: I think the author just doesn't understand the request for test coverage, is my guess | |
| 14:48:42 | melwitt | and I agree test coverage is definitely needed | |
| 14:48:58 | fried_rice | melwitt: Okay, thanks for sanity checking me. | |
| 14:49:17 | fried_rice | superdan also :) | |
| 14:49:26 | melwitt | I might be able to help with the test later | |
| 14:54:56 | zcorneli | mriedem, superdan: I think https://review.openstack.org/#/c/567876 should be ready for review now (after passing the tests yesterday) | |
| 14:55:42 | mriedem | zcorneli: have you started tinkering with a devstack change to enable / fake out file-backed memory? | |
| 14:55:51 | superdan | I have that | |
| 14:55:56 | superdan | and it's passing some things but failing others | |
| 14:56:02 | superdan | I haven't had a chance to look | |
| 14:56:15 | superdan | but we need to run a nova change against it to get the grenade live migration job yeah? | |
| 14:56:21 | superdan | https://review.openstack.org/#/c/574792/ | |
| 14:56:47 | mriedem | yeah i figured we could just put a DNM nova patch on top of https://review.openstack.org/#/c/567876 which depends on the devstack change | |
| 14:56:57 | superdan | yep | |
| 14:57:14 | superdan | I just got that devstack change passing that much yesterday | |
| 14:57:28 | superdan | the grenade multinode job passed all the test, but reported failure for some reason | |
| 14:57:29 | giblet | mriedem: I'm +2 on both. Thanks for your updates | |
| 14:57:35 | mriedem | giblet: thanks | |
| 14:58:20 | mriedem | superdan: this is the failure http://logs.openstack.org/92/574792/5/check/neutron-grenade/7ec989c/logs/screen-n-cpu.txt.gz?level=TRACE#_Jun_14_16_51_14_918632 | |
| 14:58:23 | superdan | oh, it never ran the new side | |
| 14:58:28 | mriedem | yeah | |
| 14:58:39 | superdan | ack | |
| 14:58:41 | superdan | zcorneli: ^ | |
| 14:58:48 | mriedem | so....queens / old side is not using the queens UCA... | |
| 14:58:49 | superdan | mriedem: so I thought you said we had new enough stuff? | |
| 14:58:55 | mriedem | we do, in rocky | |
| 14:58:57 | mriedem | so, | |
| 14:59:05 | mriedem | we can also do a dnm backport of the queens uca devstack patch | |
| 14:59:06 | mriedem | sec | |
| 14:59:10 | superdan | heh | |
| 15:00:12 | zcorneli | The check did its job? | |
| 15:00:19 | superdan | yup | |
| 15:00:59 | mriedem | done | |
| 15:01:40 | mriedem | zcorneli: can you roll up the DNM nova patch on top of yours that depends on https://review.openstack.org/#/c/574792/ ? | |
| 15:01:47 | mriedem | zcorneli: do those words mean anything? | |
| 15:02:31 | mriedem | just need a nova patch on top of yours that has a depends-on in the commit message to https://review.openstack.org/#/c/574792/ - see how the depends-on in that change itself work | |
| 15:03:24 | zcorneli | The words mean things, not sure yet what the patch would contain. | |
| 15:03:36 | mriedem | just change a comment in nova/compute/manager.py to trigger tests | |
| 15:03:51 | mriedem | it's just a patch to link other changes together in the CI system | |
| 15:03:59 | mriedem | since devstack itself doesn't run the job we need | |
| 15:05:43 | zcorneli | Ah, so no real content to the patch, just a token change to trigger tests, with a depends-on the Use Queens UCA patch here: https://review.openstack.org/#/c/575764/ ? | |
| 15:06:15 | zcorneli | With a depends-on my patch as well? | |
| 15:07:24 | zcorneli | (I think I had it, and now I'm pretty sure I've confused myself. Not enough caffeine yet.) | |
| 15:09:29 | mriedem | wrong devstack change | |
| 15:09:44 | mriedem | zcorneli: depend on this https://review.openstack.org/#/c/574792/ | |
| 15:09:48 | mriedem | which depends on that other one | |
| 15:10:09 | mriedem | the dnm on top of your change doesn't require a depends-on to your change since they are in the same series / topic branch | |
| 15:10:34 | zcorneli | Ah. Got it now. | |
| 15:10:44 | zcorneli | Will do. | |
| 15:16:39 | openstackgerrit | Zack Cornelius proposed openstack/nova master: DNM: Libvirt file backed memory https://review.openstack.org/575766 | |
| 15:16:48 | zcorneli | mriedem: ^ | |
| 15:19:22 | giblet | finucannot: I replied in https://review.openstack.org/#/c/564092 | |
| 15:19:38 | finucannot | giblet: You sure? :) | |
| 15:20:02 | giblet | finucannot: friday. forget to push the button. It is there now | |
| 15:20:33 | finucannot | giblet: Oh, yeah. I'd link to the Rocky version and assume it will be there | |
| 15:20:49 | finucannot | giblet: More importantly though, are you happy to just get it in as is and fix after? | |
| 15:21:35 | giblet | finucannot: I can fix it now to point to the soon to be rocky version. I guess when the release notes are published that link won't be dead any more | |
| 15:21:49 | mriedem | giblet: finucannot: link to https://docs.openstack.org/nova/latest/reference/notifications.html#versioned-notification-samples | |
| 15:22:11 | giblet | mriedem: I can do that but finucannot doesn't like it | |
| 15:22:24 | mriedem | why? | |
| 15:22:31 | finucannot | mriedem: It'll break at some point in the future | |
| 15:22:45 | finucannot | giblet: To be clear, I'd prefer not to but I'm not firmly against it :) | |
| 15:22:48 | mriedem | i link to latest in the docs all the time | |
| 15:23:01 | finucannot | and that's why we can't have nice things | |
| 15:23:01 | mriedem | if it breaks in the future it's because we failed to add a redirect | |
| 15:23:36 | finucannot | Sections of docs do get moved around too, though I guess that's not a concern here | |
| 15:23:42 | finucannot | Yeah, use latest so | |
| 15:23:52 | openstackgerrit | Eric Fried proposed openstack/os-vif master: doc: Fix arg specs and object types in docs https://review.openstack.org/575151 | |
| 15:24:15 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add full traceback to ExceptionPayload in versioned notifications https://review.openstack.org/564092 | |
| 15:24:18 | giblet | mriedem, finucannot: ^^ done | |
| 15:24:22 | mriedem | ack | |
| 15:25:37 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add policy rule to block image-backed servers with 0 root disk flavor https://review.openstack.org/561284 | |
| 15:25:41 | mriedem | i'll trade you or anyone for ^ which is a security related issue (B1 classification) that's been sitting for a couple of months, and we know is impacting public clouds | |
| 15:26:02 | giblet | mriedem: looking | |
| 15:26:10 | finucannot | mriedem: I can take that if you can offer insight on https://review.openstack.org/#/c/575487/ | |
| 15:26:43 | finucannot | tl;dr: Without that patch, I find the RequestSpec.numa_networks field is unset when I get it at the scheduler (after being sent over RPC, I assume) | |
| 15:27:13 | finucannot | I assume something is off in the prior patch but I can't figure out what | |
| 15:27:40 | mriedem | is conductor converting the request spec to a primitive and back? | |
| 15:27:44 | mriedem | or legacy format and back? | |
| 15:27:48 | mriedem | there is some of that old cruft in conductor | |
| 15:28:25 | giblet | finucannot: could it be that you missing a similar piece than this https://review.openstack.org/#/c/567267/15/nova/objects/request_spec.py@486 ? | |
| 15:28:46 | finucannot | giblet: Oh, that could be it | |
| 15:29:03 | finucannot | *after* reviewing mriedem's patch | |
| 15:29:22 | giblet | finucannot: when RequestSpec.create() or RequestSpec.save() is called the code rewrites the RequestSpec object based on the db data | |
| 15:29:39 | giblet | finucannot: and your field is missing from the db data obviously | |
| 15:29:58 | mriedem | yeah my guess is _from_db_object is setting it to None | |
| 15:31:04 | mriedem | yup, that's it | |
| 15:31:10 | mriedem | RequestSpec.save() calls _from_db_object which nulls it out | |
| 15:31:14 | mriedem | pretty sure that's it | |