| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-30 | |||
| 15:09:26 | artom | To make sure the new field got through RPC version pinning to the dest | |
| 15:09:43 | dansmith | the field will make it through regardless of the rpc pin | |
| 15:09:45 | dansmith | they're unrelated | |
| 15:09:46 | dansmith | entirely | |
| 15:09:53 | dansmith | that's my point | |
| 15:10:03 | artom | Oh. | |
| 15:10:06 | dansmith | you can't use the presence of a field in an object to know what the version pin is | |
| 15:10:32 | artom | Wait, so how does the conductor know when to backlevel an object? | |
| 15:11:33 | dansmith | artom: conductor backlevels an object when asked. when an object arrives at a compute node and that compute node sees it's too new, and asks conductor to backlevel it to a specific version (i.e. its latest for that object) | |
| 15:12:27 | artom | dansmith, aha, so I had it completely wrong. OK, need to rethink and update then. | |
| 15:12:29 | dansmith | but compute->compute there is no conductor involvement for the actual rpc call, and the version pin is not related to object versions | |
| 15:12:35 | dansmith | artom: yep | |
| 15:16:31 | mriedem | artom: left my comments in the claims patch as well https://review.opendev.org/#/c/635669/47 | |
| 15:16:39 | artom | mriedem, ack, much thanks. | |
| 15:17:49 | artom | And we want to see https://review.opendev.org/#/c/678887/ passing before we start the +W Train (zing!) going, right? | |
| 15:18:10 | mriedem | speaking of that, | |
| 15:18:11 | artom | (OK, train may have been optimistic - snail?) | |
| 15:18:15 | mriedem | why did you pull it away from the nova series? | |
| 15:18:24 | mriedem | it's a nova change that depends on a devstack change that depends on a nova change...but why? | |
| 15:18:38 | artom | It became a PITA to work on both at the same time in the same stack? | |
| 15:18:49 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add extra spec parameter and image property for memory encryption https://review.opendev.org/664420 | |
| 15:18:58 | artom | Specifically, it needed a rebase, but no the stack below it | |
| 15:19:01 | artom | *not | |
| 15:19:06 | mriedem | oh yeah ok | |
| 15:19:13 | dansmith | artom: I would really like to see that working, unless there's some fundamental reason it can't | |
| 15:19:22 | dansmith | artom: it looks like it's still not even able to start an instance though right? | |
| 15:19:22 | mriedem | as for the +W train yeah ^ what dan said | |
| 15:19:49 | artom | sean-k-mooney would know more about the fundamental reasons, so if thinks it can work, let's keep trying. | |
| 15:20:19 | artom | I removed some lines I thought were no longer needed (beause of the devstack nuke) when I rebased, looks like I'll have to put them back int | |
| 15:20:40 | artom | And put it on top again, why not, I'm respinning the series anyways | |
| 15:21:56 | mriedem | you don't have to put it back on top, | |
| 15:22:17 | mriedem | but can if you want | |
| 15:22:32 | sean-k-mooney | artom: the temp job? we jsut don have the right setting in the config. | |
| 15:23:12 | sean-k-mooney | dansmith: mriedem do ye just want to see a gate job using numa migrtaion | |
| 15:23:18 | sean-k-mooney | or does it have to be that gate job | |
| 15:23:58 | artom | *snerk* let's try and hack something in grenade! | |
| 15:24:01 | dansmith | well, especially given the upgrade issues, it'd also be nice to run the grenade LM job against this, even though it probably wouldn't have covered the situation I found earlier | |
| 15:24:01 | sean-k-mooney | because at this point if adding the options artom remvoed does not work it would be simpler form me to just make my nfv job work | |
| 15:24:23 | dansmith | sean-k-mooney: if it runs some LMs then that's better than noting | |
| 15:24:43 | dansmith | I'm also going to be tempted to hack the job to pin the rpc version at 5.1 to check his work | |
| 15:24:52 | sean-k-mooney | dansmith: well i have my https://review.opendev.org/#/c/652197/ nfv job | |
| 15:24:55 | dansmith | because breaking LM across versions is icky | |
| 15:24:59 | sean-k-mooney | that i was going to rework | |
| 15:25:16 | sean-k-mooney | so i was going to stop using fedora and jsut use ubunut with nested vert | |
| 15:25:29 | sean-k-mooney | the multinode version runs live migratio tests | |
| 15:26:00 | dansmith | the closer it is to a standard job, the easier it will be to verify, but anything is better than nothing | |
| 15:26:10 | sean-k-mooney | sure | |
| 15:26:21 | sean-k-mooney | whcih is why im swapping ot ubuntu | |
| 15:26:39 | sean-k-mooney | but we also need to move the nova-migration job to non legacy at some point | |
| 15:27:17 | sean-k-mooney | anyway i was planning to finish up early today given how late i have been working lately | |
| 15:28:02 | dansmith | understandable, and holiday weekend here for lots of people | |
| 15:28:47 | sean-k-mooney | dansmith: donnyd provide a mulit numa flavor we can use on FN and we can also use vexhost. so ill create a job on monday | |
| 15:29:14 | sean-k-mooney | assuming ye dont get the other one working | |
| 15:29:15 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: DNM: Run LM integration tests with NUMA flavor https://review.opendev.org/678887 | |
| 15:34:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Rename Claims resources to compute_node https://review.opendev.org/679470 | |
| 15:37:10 | mriedem | artom: i thought this was funny https://review.opendev.org/#/c/635669/47/nova/compute/resource_tracker.py@399 | |
| 15:37:29 | mriedem | "because the fake objects in the tests aren't correct, we can't do anything about the actual code looking weird" | |
| 15:38:17 | artom | mriedem, heh - so, either we create a migration for the test that already has the accepted status set, which is close to real like but then... | |
| 15:38:20 | artom | Well | |
| 15:38:23 | artom | No, it'd work | |
| 15:38:30 | artom | Assert it's still accepted | |
| 15:38:34 | artom | Thanks! | |
| 15:38:40 | artom | *real life | |
| 15:38:41 | mriedem | yeah.... | |
| 15:38:47 | mriedem | i think you're overthinking all of this :) | |
| 15:39:26 | artom | Like a lot of things in life :( | |
| 15:59:27 | mriedem | efried: as for the ironic + sdk series, i'll wait for your ack on https://review.opendev.org/#/c/656027/ since you brought up CERN performance anxiety | |
| 16:12:06 | openstackgerrit | Merged openstack/nova master: Follow up for specifying az to unshelve https://review.opendev.org/679164 | |
| 16:13:44 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Handle VirtDriverNotReady in _cleanup_running_deleted_instances https://review.opendev.org/679473 | |
| 16:14:23 | artom | dansmith, sanity check with you before I go off haxing - if in https://review.opendev.org/#/c/634606/62/nova/compute/manager.py@6481 I check for the presense of the migration kwarg, which is new in RPC 5.2 and is not None from the conductor, is that a good way to make sure we're not pinned to < 5.2? | |
| 16:14:25 | dustinc | mriedem, efried: thanks for help and reviews, checking out comments now | |
| 16:14:30 | dustinc | (on ironic sdk series) | |
| 16:15:53 | mriedem | artom: wouldn't https://review.opendev.org/#/c/634606/62/nova/compute/manager.py@6511 already be False in that case? | |
| 16:16:01 | dansmith | artom: isn't that the destination not the source? | |
| 16:16:37 | artom | Doh, right | |
| 16:16:47 | dansmith | artom: I quickly looked to see if you had an easy indication on the source like that and didn't see one | |
| 16:17:05 | dansmith | but yeah, that's a little obscure, but as long as it's commented you should be able to use something like that | |
| 16:17:13 | dansmith | but, not sure there is one for the source :/ | |
| 16:18:01 | artom | Right, it's set by the libvirt driver during the source check: https://review.opendev.org/#/c/634606/62/nova/virt/libvirt/driver.py | |
| 16:18:23 | artom | That's the bit that has to be conditional on RPC 5.2 | |
| 16:19:48 | artom | dansmith, feels ugly, but add a new sentinel param to check_can_live_migrate_source when it's called from check_can_live_migrate_destination? | |
| 16:19:58 | artom | Or just have check_can_live_migrate_source manually check the RPC version | |
| 16:20:48 | dansmith | it's always called from there no? | |
| 16:21:24 | artom | You mean _source from _destination? Yeah, I was saying it from context | |
| 16:21:49 | artom | *for | |
| 16:22:05 | dansmith | it's always called there, so you mean when it's called from a new-enough node to pass that flag, yeah? | |
| 16:22:57 | artom | dansmith, yeah. If the node has the new code, it'll set the sentinel, and if the RPC is new enough, it'll actually reach the source | |
| 16:23:22 | dansmith | yeah, I don | |
| 16:23:27 | dansmith | I don't love it, but... | |
| 16:25:38 | artom | Or. | |
| 16:26:14 | artom | When the destination gets the migrate_data back from the source, pop the src_supports_numa_live_migration sentinel if the destination did not receive a migration from the conductor | |
| 16:28:28 | dansmith | the migration from conductor doesn't tell the whole story, so I don't think it's legit to use that as your sentinel | |
| 16:28:42 | dansmith | conductor's rpc version may be pinned differently | |
| 16:29:11 | artom | Differently.... from the computes? | |
| 16:29:24 | artom | I thought it was a deployment-wide thing | |
| 16:29:58 | dansmith | differently from anything, sure | |
| 16:30:07 | dansmith | config is per-node | |
| 16:30:36 | dansmith | in general you'd kinda assume they're the same, but especially when doing the slide-puzzle migration approch, | |
| 16:30:52 | dansmith | you might need to pin some computes differently to be able to communicate while things are old | |
| 16:31:13 | artom | Hrmm, so in addition to this whole thing, I need to check that the migration from the conductor is not None | |