| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-04-12 | |||
| 11:58:26 | sean-k-mooney | schema chagnes are seperate | |
| 11:58:39 | sean-k-mooney | the online migation are just data migrations | |
| 11:59:07 | noonedeadpunk | ok, then will see how CI will fail dramatically with my changes :D | |
| 12:04:49 | bauzas | noonedeadpunk: as it was written in the docs I passed, db sync are only changing the schemas | |
| 12:05:42 | bauzas | noonedeadpunk: then either we change the data internally (either by a lazy call, or when restarting the service), or we ask operators to run online_data_migration | |
| 12:06:26 | bauzas | but online_data_migration can run during all the cycle, until you want to upgrade to a new release | |
| 12:07:34 | bauzas | then, once you want to upgrade, you can before run nova-status upgrade check for verifying that you're done with the online data migrations | |
| 12:08:38 | noonedeadpunk | with N-1 online data migrations, right? | |
| 12:10:07 | noonedeadpunk | so running upgrade check on N will verify that N-1 migrations are done | |
| 12:10:22 | noonedeadpunk | or well, N-2 with slurp, but that's different topic :D | |
| 12:13:40 | bauzas | yup | |
| 12:14:08 | noonedeadpunk | ok, awesome, thanks for your time and patience :) | |
| 13:35:58 | dansmith | noonedeadpunk: yeah, can't run online migrations until you're past the point where conductors, api, scheduler are upgraded (or stopped before upgrade | |
| 13:36:12 | dansmith | noonedeadpunk: the idea is that online migrations should be run *after* everything is done and back up | |
| 13:36:34 | dansmith | services will migrate what they need on-demand, and the online migrations are just there to push things that don't get migrated on demand | |
| 13:38:53 | noonedeadpunk | yup, thanks for confirming that! | |
| 13:41:10 | noonedeadpunk | will go bug cinder folks with the same question then :-) | |
| 13:45:37 | dansmith | ack, I didn't mean to repeat, just wasn't sure there was a clear summary of that convo, but sounds like you got it :) | |
| 13:46:56 | noonedeadpunk | yeah, I've pushed change as a result - I should have posted it https://review.opendev.org/c/openstack/openstack-ansible-os_nova/+/880147 | |
| 13:57:41 | dansmith | noonedeadpunk: I think running the status check after online migrations also makes sense as there are cases where it will tell you that there are still pending migrations to do if they're not complete | |
| 13:57:51 | dansmith | but regardless, the meat of that change sounds right yeah | |
| 14:52:29 | bauzas | dansmith: thanks for having explained it better than me :) | |
| 14:52:53 | bauzas | your summary is far easier :) | |
| 15:12:49 | dansmith | bauzas: when you're done with the current call you're on I want to chat about some resource tracker stuff | |
| 15:54:38 | noonedeadpunk | between not that long ago (on PTG?) we've discussed enable_new_services option. And it should be applied not for computes at the end of the day, but somewhere on conductor/scheduler/api - not sure where exactly as I have them combined at same place (with same nova.conf) | |
| 15:55:34 | noonedeadpunk | so while it's defined in nova.conf.compute - it's not compute option | |
| 15:55:51 | noonedeadpunk | (https://opendev.org/openstack/nova/src/branch/master/nova/conf/compute.py#L1456-L1474) | |
| 15:57:08 | dansmith | conductor | |
| 15:57:11 | dansmith | but we could also fix that | |
| 16:00:09 | bauzas | dansmith: I'm here | |
| 16:00:29 | dansmith | bauzas: so... the RT has this notion of "disabled compute nodes" | |
| 16:00:51 | dansmith | which seems to cover things actually disabled (via ironic) as well as "no such compute node on this host" | |
| 16:01:06 | dansmith | we do some weird things in move claims if we think the compute node is missing, | |
| 16:01:13 | dansmith | like accept the migration but do no claim | |
| 16:01:15 | bauzas | iirc, this was done for the latter, not the former case | |
| 16:01:41 | dansmith | the latter meaning a missing compute node? | |
| 16:01:46 | dansmith | I can't think of any reason why that would make sense, as we'll just mess up our accounting | |
| 16:02:24 | dansmith | https://review.opendev.org/c/openstack/nova/+/879682/2/nova/compute/resource_tracker.py#296 | |
| 16:02:31 | dansmith | so I'm looking at that ^ specifically | |
| 16:02:46 | dansmith | where we just do a nopclaim and return from move_claim | |
| 16:03:44 | dansmith | which means, AFAICT, we would have created the migration object on the target machine, claimed no resources, and agreed to accept the new incoming instance | |
| 16:03:55 | dansmith | even though it's for a node that we don't have or know anything about? | |
| 16:04:28 | bauzas | I'm just trying to understand the intent | |
| 16:04:37 | dansmith | also note this from mr. pipes: https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L154 | |
| 16:04:51 | dansmith | in the instance claim, which we'll also do weird stuff for new instances with no such node | |
| 16:05:08 | dansmith | the earlier part of the comment says it shouldn't happen | |
| 16:05:24 | dansmith | so, like, I'm confused and feel like this is probably some very old cruft | |
| 16:05:43 | dansmith | like, surely we can't have these things happen in a placement world and not get everything messed up, right? | |
| 16:06:12 | bauzas | yeah, found the commit https://github.com/openstack/nova/commit/1c967593fbb0ab8b9dc8b0b509e388591d32f537 | |
| 16:07:10 | bauzas | ok, so now I think I remember where it comes | |
| 16:07:25 | bauzas | the virt drivers are responsible for creating the RTs | |
| 16:08:06 | bauzas | at compute startup, we were originally setting one RT per compute node resources dict that was returned by the virt driver | |
| 16:08:21 | bauzas | and then mr jay factorized that | |
| 16:08:27 | bauzas | by having one single RT instance | |
| 16:08:30 | dansmith | ...right | |
| 16:09:09 | bauzas | so the disabled property was originally coming from the fact there could be a race where the RT was instanciated but the virt resources not fully set up | |
| 16:10:41 | dansmith | that may be the case for startup, but we should never accept an instance boot or migration if we can't account for the resources right? | |
| 16:10:54 | bauzas | https://github.com/openstack/nova/commit/1c967593fbb0ab8b9dc8b0b509e388591d32f537#diff-ed9525d7ae319fd575249ed72daf634d27182e08fea7a4f740cb3164233612b7L435 | |
| 16:12:00 | bauzas | well, that whole thing predates placement | |
| 16:12:00 | bauzas | https://github.com/openstack/nova/commit/1c967593fbb0ab8b9dc8b0b509e388591d32f537#diff-ed9525d7ae319fd575249ed72daf634d27182e08fea7a4f740cb3164233612b7L412 | |
| 16:12:07 | dansmith | right | |
| 16:12:14 | dansmith | (those links do not link to a hunk for me, btw) | |
| 16:12:21 | bauzas | ah shit | |
| 16:12:28 | bauzas | so, the fact is, before placement, | |
| 16:12:38 | bauzas | we were relying on the ComputeNode records for scheduling | |
| 16:13:09 | bauzas | if there were no ComputeNode records, then the HostStates from the scheduler weren't generated | |
| 16:13:23 | bauzas | and no instances were able to schedule into them | |
| 16:13:47 | dansmith | your point being that we could (should) never get into that case where we do the NopClaims because of that? | |
| 16:14:05 | dansmith | I think that's probably also not true because of forced migrations | |
| 16:14:35 | bauzas | the NopClaims were there for saying 'meh' | |
| 16:14:53 | bauzas | if no compute record was existing then basically we were tracking nothing | |
| 16:14:56 | dansmith | right but they also cause us to skip all the other stuff like PCI and migration objects, etc | |
| 16:15:10 | dansmith | but we allow the instance to be sent there | |
| 16:15:40 | bauzas | if the scheduler isn't having a compute node record, then it shouldn't be able to send an instance to it | |
| 16:15:57 | bauzas | and I think this assumption is still valid | |
| 16:16:14 | dansmith | except for times where we didn't go through the scheduler, which I think back when this was done, was the case | |
| 16:16:26 | dansmith | but if you're right, why would we not raise an exception here instead of silently "meh" ? | |
| 16:16:42 | bauzas | good question | |
| 16:16:52 | dansmith | and also, | |
| 16:17:06 | dansmith | transient changes could result in the scheduler thinking a compute node was valid for a given host, send a node there, | |
| 16:17:09 | bauzas | the NopClaims even predates Mr Jay's work on RT | |
| 16:17:18 | bauzas | and even my presence on OpenStack since this decade :) | |
| 16:17:21 | dansmith | and the RT would literally create a migration record with a destination node name that does not match anything it knows about | |
| 16:18:39 | dansmith | the nopclaim stuff itself predates tons of stuff, yeah, but the usage of it in this case is much newer | |
| 16:18:46 | dansmith | here's my problem: | |
| 16:19:07 | bauzas | oh | |
| 16:19:10 | dansmith | in order to do the strict tying of instance->compute->service, we need migrations to have a strict tie from migration->compute | |
| 16:19:19 | dansmith | which means we need the compute id in the migration | |
| 16:19:21 | bauzas | I missed the fact we record a migration before we return the NopClaim | |
| 16:19:29 | dansmith | right | |
| 16:19:31 | bauzas | technically this isn't a noop | |
| 16:19:31 | dansmith | crazy right? | |
| 16:19:54 | bauzas | dansmith: I wonder something, lemme check | |
| 16:19:59 | dansmith | so my code moves that to below the disabled check and refuses to create a migration record for a destination compute node we don't know anything about (and thus don't have a node id) | |
| 16:20:11 | bauzas | my guess is that someone added the migration checks *after* the nopclaim implementation | |
| 16:20:21 | bauzas | without thinking about the implication | |
| 16:20:23 | dansmith | here, the nodename loose affiliation is us just lying about the destination, which is easy if the nodename is just a name, but the id requires stricter adherence | |
| 16:20:29 | bauzas | (gosh hope it isn't me :) ) | |
| 16:22:15 | dansmith | what "migration checks" ? | |
| 16:22:24 | dansmith | those lines I'm moving are the creation of the migration object | |