| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-26 | |||
| 12:39:44 | openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: Enable driver configuring PMEM namespaces https://review.opendev.org/678453 | |
| 12:39:44 | openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: report VPMEM resources by provider tree https://review.opendev.org/678454 | |
| 12:39:45 | openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: Support VM creation with vpmems and vpmems cleanup https://review.opendev.org/678455 | |
| 12:39:45 | openstackgerrit | Luyao Zhong proposed openstack/nova master: Parse vpmem related flavor extra spec https://review.opendev.org/678456 | |
| 12:39:46 | openstackgerrit | Luyao Zhong proposed openstack/nova master: Add functional tests for virtual persistent memory https://review.opendev.org/678470 | |
| 14:20:36 | efried | mriedem, melwitt, dansmith: Could I please get eyes on https://review.opendev.org/#/c/678237/ to unwedge u-c https://review.opendev.org/#/c/678207/ | |
| 14:21:19 | mriedem | " We get to stop doing this soon, I promise." fool me once, shame on you, fool me twice... | |
| 14:21:38 | efried | but mordred *also* promises | |
| 14:22:19 | sean-k-mooney | did we "fix" something like this recently | |
| 14:22:27 | efried | sean-k-mooney: you mean https://review.opendev.org/#/c/676495/ ? | |
| 14:22:40 | sean-k-mooney | yes | |
| 14:22:43 | efried | You mean, every time we release a new sdk we have to re-fix our fixture that stubs out something private in sdk? Yes. | |
| 14:22:58 | efried | the long term solution is to not stub out private sdk stuff. | |
| 14:23:05 | mordred | srrsly | |
| 14:23:12 | mordred | yes | |
| 14:23:13 | efried | sdk is going to expose a fixture that we can simply import and use | |
| 14:23:15 | sean-k-mooney | yep seams like what we should be doing | |
| 14:23:20 | mordred | the sdk is going to give you the happy fixture | |
| 14:23:22 | mordred | and you will use that | |
| 14:23:25 | mordred | and it will be tested in the sdk | |
| 14:23:29 | mordred | so we won't break you | |
| 14:23:36 | efried | right, and then as they change internals, they'll change the fixture, and we'll automatically "keep up". | |
| 14:23:48 | mriedem | i might as well just fast approve this | |
| 14:23:55 | mriedem | it's test only and holding up the u-c change | |
| 14:24:05 | efried | yes, the u-c patch proves it works | |
| 14:24:12 | sean-k-mooney | efried: we try to do that with plamcent but we have been broken with the placement fixture in the past | |
| 14:24:12 | efried | and the nova patch proves it still works at 0.34.0 | |
| 14:24:24 | efried | sure, but less likely | |
| 14:24:39 | sean-k-mooney | we will need to similarly pull the sdk out of requirements and install it spereatly in tox right | |
| 14:24:48 | sean-k-mooney | e.g. to consume it form master? | |
| 14:24:51 | efried | eh? Why would we need to do that? | |
| 14:25:02 | sean-k-mooney | the same reason we do it for the placement fixture | |
| 14:25:13 | efried | oh, no, tracking against real releases should be fine. | |
| 14:25:22 | sean-k-mooney | ok | |
| 14:25:34 | efried | When we've needed to pre-test a feature, we twiddle the project list in .zuul.yaml | |
| 14:25:34 | sean-k-mooney | i guess for placement we do it to get eraly acess to features | |
| 14:25:50 | efried | yes, which is actually probably not necessary anymore | |
| 14:25:55 | efried | actually probably not advisable. | |
| 14:26:01 | efried | we should bring that up with cdent when he gets back. | |
| 14:26:04 | sean-k-mooney | yes the second one | |
| 14:26:07 | efried | (which I thought was today, but I guess not) | |
| 14:26:22 | sean-k-mooney | i think it is still nessacary for some feature but we have a backlog of feature to catch up on | |
| 14:26:40 | openstackgerrit | Merged openstack/nova master: Document archive_deleted_rows return codes https://review.opendev.org/677819 | |
| 14:27:03 | efried | sean-k-mooney: I actually don't remember the last time we did something in nova that *needed* to track against unreleased placement. | |
| 14:27:35 | efried | and iiuc having it set up this way allows us to do things like... merge a nova feature before placement is released accordingly, thereby breaking nova (cf. the aforementioned fixture issue). | |
| 14:27:35 | sean-k-mooney | well all of the nested stuff would have needed it if we did that this cycle. | |
| 14:27:43 | efried | yeah, but we can just release placement | |
| 14:27:50 | efried | releases are pretty cheap. | |
| 14:28:18 | sean-k-mooney | maybe. i guess depends on is really the way to test this stuff | |
| 14:29:15 | efried | right, I'm saying, I'm not sure there's a good reason for us to have placement in required projects by default. | |
| 14:29:40 | sean-k-mooney | i wasnt refering to required proejcts specifcally | |
| 14:30:05 | efried | that's how we end up with nova running against master placement in the gate, nah? | |
| 14:30:40 | mriedem | you need placement in required projects in the gate jobs b/c it's a service not a library | |
| 14:30:55 | mriedem | if you want to freeze the fixture or something, yo'ud need to make it a library i think | |
| 14:31:17 | openstackgerrit | Merged openstack/nova master: Change nova-manage unexpected error return code to 255 https://review.opendev.org/677832 | |
| 14:31:20 | efried | do we have keystone/glance/ironic/neutron/cinder in required projects too? | |
| 14:31:21 | mriedem | iow, if placement isn't in required-projects, gate jobs aren't going to run with placement from master | |
| 14:31:24 | mriedem | yes | |
| 14:31:25 | openstackgerrit | Merged openstack/nova master: Document map_instances return codes in table format https://review.opendev.org/677835 | |
| 14:31:50 | efried | okay, maybe I'm thinking of placement as more of a lib | |
| 14:32:34 | dansmith | artom: you working on the test failures? I haven't looked to see what they are, but I assume they're relevant since all of them seem to be failing | |
| 14:32:40 | mriedem | https://github.com/openstack/devstack/blob/master/.zuul.yaml#L368 | |
| 14:32:44 | artom | dansmith, yeo | |
| 14:32:46 | artom | *yep | |
| 14:33:03 | artom | dansmith, wanted to ask you, we can't fully remove an RPC param until the next major version bump, right? | |
| 14:33:38 | artom | I wanted to remove destroy_disks from the rollback_live_migration call and have the method on the destination do all the deciding, since _live_migration_flags isn't host-dependant, it only cares about migrate_data. | |
| 14:33:53 | artom | But we have to keep compatibility code until RPC 6.0, right? | |
| 14:34:21 | dansmith | artom: fully or otherwise, yeah | |
| 14:35:30 | artom | dansmith, wait, otherwise? So not at all? RPC params are only additive? | |
| 14:36:13 | dansmith | artom: yeah, meaning if you supported the letter and intent of the law in the current version, you can't stop honoring either until the next bump | |
| 14:36:16 | mriedem | what does destroy_disks have to do with numa? | |
| 14:36:31 | dansmith | artom: in that, you can't just accept and ignore some flag that has some meaning | |
| 14:37:19 | artom | mriedem, it's convoluted, but basically, we need to call rollback_live_migration_at_destination if we did a claim and we need to drop it | |
| 14:37:35 | artom | Except now, it's called only the do_cleanup is True (as determined by _live_migration_flags) | |
| 14:38:08 | mriedem | yeah i remmeber that | |
| 14:38:22 | artom | There's a bunch of possibilities around that, but the end result it, everything becomes easier if we just always call rollback_live_migration_at_destination, and let it decide what cleanup needs to be done | |
| 14:38:23 | mriedem | you mean rollback_live_migration_at_destination right? | |
| 14:38:36 | artom | Instead of passing it booleans over RPC | |
| 14:38:53 | artom | Except doing that would technically require removing the destroy_disks param | |
| 14:39:47 | dansmith | artom: don't we pass that flag because there are cases where the other side can't decide what to do? | |
| 14:41:02 | artom | dansmith, might have been the case at one time, but doesn't look like it now. https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6946-L6979 doesn't depend on anything but migrate_data, which both sides have | |
| 14:41:11 | artom | Unless I'm really blind | |
| 14:41:26 | artom | I guess it was done to save an RPC call if we didn't need it? | |
| 14:43:55 | artom | sean-k-mooney, could you weigh in please: https://review.opendev.org/#/c/635669/39/nova/compute/resource_tracker.py@307 | |
| 14:44:10 | dansmith | hmm, okay.. I really thought we had serious issues with that logic, | |
| 14:44:23 | dansmith | but perhaps the passing of migrate data to more places made that better or something | |
| 14:44:25 | dansmith | it's been a while | |
| 14:44:34 | dansmith | mriedem probably remembers better | |
| 14:44:52 | mriedem | i would have to dig, _live_migration_cleanup_flags sucks | |
| 14:44:57 | dansmith | yeah | |
| 14:45:40 | mriedem | i had to workaround the same kind of thing where do_cleanup was False but i still needed to do network_api.setup_networks_on_host for the dest in the neutron case with port bindings | |
| 14:46:01 | mriedem | https://review.opendev.org/#/q/I658e0a749e842163ed74f82c975bcaf19f9f7f07 | |
| 14:47:24 | mriedem | artom: so you just need the source to make a call to the dest to rollback the claim on failure right? regardless of shared storage or not | |
| 14:47:32 | artom | mriedem, yep | |
| 14:47:45 | artom | I could always just add a new RPC method | |
| 14:47:48 | mriedem | dansmith probably won't like what i'm about to suggest, but it might be better to just write a new rpc method, | |
| 14:47:56 | artom | Seems weird to do that since we already have a thing called "rollback" | |
| 14:47:57 | mriedem | rather than munge it into this steaming pile o shit | |
| 14:48:08 | artom | You don't like wobbly shit castles? | |
| 14:48:26 | artom | If dansmith's onboard I'm all for that | |