| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-11-28 | |||
| 16:18:31 | sean-k-mooney | in that case they are likely specifyign the host as they are freeing up old host to upgrade | |
| 16:18:37 | bauzas | operators do a lot of migrations when they upgrade | |
| 16:18:48 | dansmith | artom: live migration during an upgrade is the primary mechanism that most people use | |
| 16:19:07 | dansmith | artom: *lots* of people refuse to upgrade a compute node until they've moved everything off of it | |
| 16:19:14 | bauzas | tbh, I'm fine with dropping a release note saying there will be an impact | |
| 16:19:21 | artom | dansmith, fair enough | |
| 16:19:49 | bauzas | given worloads should move from old to new | |
| 16:19:56 | bauzas | and not from new to old | |
| 16:19:59 | sean-k-mooney | bauzas: we could do what we did for multiple port bindings and just fall back to the old behavior | |
| 16:20:12 | bauzas | oh shit, the problem is with old to new | |
| 16:20:17 | bauzas | not the contrary, my bad | |
| 16:20:41 | artom | I just don't see how we can realistically handle that | |
| 16:20:44 | bauzas | so, yeah, we could shoot operators in the foot | |
| 16:20:58 | bauzas | artom: we could make a flag during the upgrade | |
| 16:21:03 | artom | Keep the old code paths intact while somehow adding claims and all the new XML stuff | |
| 16:21:13 | bauzas | and say 'if you need so, it's on you, folks" | |
| 16:21:21 | sean-k-mooney | artom: for old to new we can fallback to the old behvior | |
| 16:21:38 | bauzas | new to old isn't a problem | |
| 16:21:41 | bauzas | new to new isn't a problem | |
| 16:21:42 | artom | sean-k-mooney, can we? How do we even implement that? | |
| 16:21:45 | bauzas | old to new is the problem | |
| 16:22:00 | sean-k-mooney | e.g. claim nothing and tell people to migrate again after migrate to fix everything | |
| 16:22:02 | bauzas | so yeah, we somehow need to keep the broken behaviour | |
| 16:22:05 | artom | It's going to be way ugly with a whole bunch of conditionals all over the place | |
| 16:22:15 | bauzas | artom: I know, it's freaking ugly | |
| 16:22:46 | artom | Unless we do the new stuff in entirely new RPC calls/casts | |
| 16:22:46 | bauzas | but I don't see operators be happy with what you propose :p | |
| 16:22:55 | openstack | Launchpad bug 1805659 in OpenStack Compute (nova) "nova notifications hammering the message bus" [Undecided,New] | |
| 16:22:55 | gibi | mriedem, jmlowe: I reported https://bugs.launchpad.net/nova/+bug/1805659 | |
| 16:22:58 | sean-k-mooney | artom: its what we did for multiple port bindings. we check if a field exitsing in the migrate data if not we do the old stuff if its there we do the new way | |
| 16:23:04 | artom | bauzas, yeah, probably true | |
| 16:23:05 | dansmith | artom: that's how we do things gracefully | |
| 16:23:14 | bauzas | couldn't you drop the move claim if the RPC argument is not there ? | |
| 16:23:18 | dansmith | artom: i.e. pass a flag, and if it's missing, assume it's an old compute and do the old thing | |
| 16:23:19 | artom | dansmith, what is? New RPC version? | |
| 16:23:29 | dansmith | you can't see the rpc version on the receiving end | |
| 16:23:32 | bauzas | what dansmith said and me :) | |
| 16:23:32 | dansmith | you need a flag | |
| 16:23:36 | dansmith | do_the_new_thing=True | |
| 16:23:46 | bauzas | that's what we generally do | |
| 16:23:49 | artom | dansmith, I don't disagree, but it's going to be a mess | |
| 16:23:58 | bauzas | artom: I can point you some code I wrote that does the signaling | |
| 16:24:18 | dansmith | artom: it's how you have to do it | |
| 16:24:19 | dansmith | it's how everything we do that involves old/new services works | |
| 16:24:30 | bauzas | artom: sure, but we somehow need to know that the live migration comes from an old compute hence us releasing the check | |
| 16:24:41 | bauzas | and not doing the claim | |
| 16:25:05 | mriedem | if you have an old dest compute, the migrate_data won't have the numa stuff from the claim right? | |
| 16:25:24 | mriedem | so the source (if new) can't rely on it | |
| 16:25:35 | artom | mriedem, other way around, new dest, old source | |
| 16:25:44 | artom | So source won't send the updated XML, but dest will have claimed for it | |
| 16:25:55 | artom | So we need to do conditional claiming based on source compute version | |
| 16:26:15 | mriedem | i think that's what the file-backed memory live migration does... | |
| 16:26:16 | bauzas | you could make the migrate_data parameter a sentinel | |
| 16:26:22 | mriedem | it checks the compute service version for the source from the dest | |
| 16:26:47 | artom | Alright, lemme update the spec | |
| 16:27:02 | bauzas | artom: just change this | |
| 16:27:06 | bauzas | other things were nits | |
| 16:27:16 | bauzas | just for documenting the spec | |
| 16:27:18 | artom | Alright if I keep it kinda high-level? So "conditional claim", but without specifying what that condition will look like (flag, version checking, etc)? | |
| 16:27:20 | bauzas | later when we review | |
| 16:27:28 | dansmith | artom: you can't check the version | |
| 16:27:42 | mriedem | artom: this is the code i'm thinking of https://github.com/openstack/nova/blob/62245235bc15da6abcdfd3df1c24bd856d69fbb4/nova/virt/libvirt/driver.py#L6636 | |
| 16:27:51 | artom | dansmith, I guess not on the compute, eh? | |
| 16:27:59 | artom | Only conductor, so it has to be flag | |
| 16:28:07 | bauzas | mriedem: we don't want to fail | |
| 16:28:16 | dansmith | artom: I'm not sure what you mean | |
| 16:28:19 | bauzas | mriedem: we want to blindly accept the migration and not claim | |
| 16:28:23 | mriedem | bauzas: you don't want to claim on the dest if the source isn't going to use it | |
| 16:28:24 | mriedem | right? | |
| 16:28:38 | bauzas | that's right | |
| 16:28:39 | artom | dansmith, can the dest compute check the source compute version? | |
| 16:28:47 | artom | And do the new thing only if the source compute is hew? | |
| 16:28:48 | artom | *new | |
| 16:28:49 | dansmith | artom: don't do it that way | |
| 16:28:50 | mriedem | my point is, ^ is how the dest checks the source compute version today - that pattern could be re-used, not the specific failure thing | |
| 16:29:04 | dansmith | artom: because version pinning might mean you have a new compute, but it didn't send something new | |
| 16:29:23 | artom | dansmith, oh right | |
| 16:29:46 | artom | Wait up though | |
| 16:29:57 | bauzas | artom: https://github.com/openstack/nova/blob/master/nova/scheduler/manager.py#L93 | |
| 16:30:11 | bauzas | artom: that's one example of a flag (the _sentinel value) | |
| 16:30:23 | dansmith | the file-backed stuff chooses not to migrate at all if the target isn't going to support it I think, but in your case you have to allow it, you just need to do the old thing | |
| 16:30:40 | mriedem | if the source isnt going to suppor it | |
| 16:30:43 | mriedem | *support | |
| 16:30:49 | bauzas | shit, I need to bail out | |
| 16:30:58 | bauzas | I just dropped my poop and then I leave | |
| 16:30:59 | bauzas | excellent | |
| 16:31:08 | dansmith | bauzas: um... | |
| 16:31:10 | artom | You panda | |
| 16:31:17 | artom | Eats poops and leaves | |
| 16:31:21 | mriedem | dansmith: i think the point is, from the dest, if the source is old, we don't claim on the dest and we don't put the numa things in migrate_data, | |
| 16:31:21 | artom | No wait, it's "shoots" | |
| 16:31:22 | artom | Dammit >< | |
| 16:31:26 | bauzas | context is https://www.meetup.com/fr-FR/Groupe-dutilisateurs-Python-Grenoble/events/256520367/ | |
| 16:31:38 | mriedem | because and old source isn't going to use those anyway, and also wouldn't know to rollback the claim or whatever | |
| 16:31:43 | bauzas | for once we have a meetup that talks OpenStack here... | |
| 16:32:16 | dansmith | mriedem: it really needs to work both ways, whether the source is old or new | |
| 16:32:25 | mriedem | if the dest is old, | |
| 16:32:29 | mriedem | and the source is new, | |
| 16:32:35 | mriedem | migrate_data won't have the new numa field in it, | |
| 16:32:42 | artom | If the source is new it's fine, dest will just ignore the new field | |