| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-26 | |||
| 15:52:26 | mriedem | without consumer types in the allocations response we have to rely on the resource class | |
| 15:52:58 | bauzas | exactly this, I was about to say which resource classes where nova-related | |
| 15:53:07 | bauzas | were* | |
| 15:53:53 | efried | ugh, relying on resource class... | |
| 15:54:04 | efried | this is where the concept of provider owner would be handy. | |
| 15:54:17 | bauzas | yeah I know | |
| 15:54:32 | efried | hopefully we're not allowing allocations from different owners against the same provider anywhere | |
| 15:54:39 | bauzas | we could also add an argument asking for the resource class we wanna check | |
| 15:54:49 | efried | no, we shouldn't do it by resource class | |
| 15:55:02 | efried | because same resource class may be managed by different owners in different providers | |
| 15:55:20 | efried | think VF (nova-PCI vs cyborg vs neutron) | |
| 15:55:53 | efried | but we (need to make sure we) have a rule that a provider as a whole is only managed by a single owner. | |
| 15:56:25 | bauzas | hmmm | |
| 15:57:10 | bauzas | actually, I'm checking consumer_id | |
| 15:57:39 | bauzas | so I guess all resource providers corresponding to compute nodes (and children associated) should have allocations against consumer_id that | |
| 15:57:53 | bauzas | that *is* either a migration object or a nova instance | |
| 15:58:05 | bauzas | even cyborg, right? | |
| 15:58:31 | openstackgerrit | Nate Johnston proposed openstack/nova stable/stein: [DNM] Test change to check for port/instance project mismatch https://review.opendev.org/667663 | |
| 15:59:20 | bauzas | efried: ^? | |
| 16:00:45 | efried | bauzas: If what you're looking to do is clean up allocations against orphaned instances, I think it's legit to remove all the allocations associated with that consumer, even if they're on providers you don't own. That's symmetrical with what we do when we schedule (we claim all of those atomically from nova). | |
| 16:00:51 | efried | and | |
| 16:01:14 | efried | if there's an allocation against a compute node RP, you can legitimately assume it's in that category | |
| 16:01:15 | efried | but | |
| 16:01:33 | efried | that will break eventually if we ever have resourceless roots | |
| 16:01:34 | efried | because | |
| 16:01:48 | efried | you can *not* assume that all children of the compute node RP *also* belong to nova. | |
| 16:01:51 | bauzas | baby steps here :) | |
| 16:02:06 | efried | yeah, just leave a note/todo I guess. | |
| 16:02:21 | bauzas | at least if I can support nested rps, it would be cool | |
| 16:02:58 | bauzas | because eg. VGPU allocations are still made *against* a consumer which is an instance, yeepee | |
| 16:03:54 | bauzas | but, that would mean I would look at all resource providers, not only the ones Nova owns | |
| 16:03:56 | efried | yeah, it would be 1) compute node => 2) compute node RP => 3) allocations against that RP => 4) consumer for that allocation => 5) filter down to orphan consumers => 6) allocations for those consumers => 7) delete all of those | |
| 16:03:59 | efried | no | |
| 16:04:00 | bauzas | and here comes pagination... | |
| 16:04:28 | efried | with the limitation noted above (stops working for resourceless roots, which we're a long way off of), the above process will get you there. | |
| 16:04:36 | efried | Step 1 done by paginating from the nova API. | |
| 16:04:50 | bauzas | cool then | |
| 16:05:04 | efried | this is in a nova-manage type utility? | |
| 16:05:14 | efried | So we don't care that it'll take FOREVER to run at cern? | |
| 16:05:30 | bauzas | a nova-manage placement audit thing | |
| 16:05:38 | efried | mm | |
| 16:05:39 | bauzas | so a cron job basically | |
| 16:05:49 | bauzas | marker and the likes | |
| 16:06:00 | efried | mm | |
| 16:06:04 | bauzas | zactly like heal_allocations | |
| 16:06:13 | efried | sure would be nice to find a way to make it more efficient then. | |
| 16:06:31 | mriedem | heal_allocations doesn't have a marker | |
| 16:06:38 | efried | but: make it, make it right, make it fast | |
| 16:06:41 | mriedem | it has a limit of things to process | |
| 16:06:55 | mriedem | nor does heal_allocations deal with nested allocations | |
| 16:08:42 | mriedem | the audit command could also take a --consumer option to just investigate what the operator thinks is a problem instance/migration | |
| 16:09:00 | mriedem | note that i added --instance to heal_allocations later for that reason | |
| 16:09:07 | bauzas | yup I saw | |
| 16:09:10 | mriedem | and --dry-run | |
| 16:09:31 | mriedem | depends on what the command will do though, if it's just reporting then you don't need a --dry-run | |
| 16:11:20 | bauzas | I was thinking of just telling the orphaned, but then later adding a --remove option | |
| 16:11:33 | bauzas | *later* | |
| 16:12:35 | bauzas | anyway, needs to go off and run by hot summer nights | |
| 16:13:03 | bauzas | I think I have everything I needed, thanks folks | |
| 16:14:57 | dansmith | hoo boy | |
| 16:15:23 | mriedem | strictly adult cheese, wine and things of that nature | |
| 16:16:35 | melwitt | now, for another fun topic | |
| 16:17:02 | melwitt | mriedem, dansmith: I was reading these comments on an old [unmerged] patch: https://review.opendev.org/#/c/462521/12/releasenotes/notes/resize-auto-revert-6e1648828aba16b2.yaml@5, | |
| 16:17:31 | melwitt | and it made me think of the [recently merged] patch: https://review.opendev.org/633227 again and how it changed ERROR state to ACTIVE (or STOPPED) state. now I'm worried that wasn't an ok thing to do (API change?) | |
| 16:18:12 | melwitt | for a failed cold migration to self | |
| 16:18:21 | mriedem | not the same | |
| 16:18:27 | mriedem | in my change, | |
| 16:18:35 | mriedem | we failed in prep_resize before we actually did anything to the guest | |
| 16:18:46 | mriedem | in that case, putting the instance in ERROR status makes no sense imo | |
| 16:19:08 | mriedem | as i said, the only way you can get it out of error then is to do something like rebuild, hard reboot and/or reset status to ACTIVE, | |
| 16:19:16 | dansmith | and was yours also resetting to ACTIVE if it was actually shutoff? | |
| 16:19:17 | dansmith | I forget | |
| 16:19:26 | mriedem | and if i started a resize or cold migration of a STOPPED instance, then resetting it to ACTIVE isn't what i want, nor is rebuild or hard reboot really | |
| 16:19:42 | mriedem | dansmith: that was the point of my fix | |
| 16:19:52 | mriedem | to reset to STOPPED if it was STOPPED | |
| 16:19:55 | dansmith | mriedem: right | |
| 16:19:57 | mriedem | well, in part, | |
| 16:20:02 | mriedem | the main point was don't put it in ERROR status | |
| 16:21:59 | melwitt | ok, I think I see. this is ok because the instance is actually ok (other than cosmetic), whereas for the first example, the instance was not ok and was proposed to auto-correct to an ok/healthy state | |
| 16:22:27 | dansmith | the auto-revert actually moved stuff back, IIRC | |
| 16:22:37 | dansmith | not just correcting state, but actual revert | |
| 16:22:39 | melwitt | yeah it did | |
| 16:23:09 | melwitt | I was zooming in on the vm_state part of it, how it appears to an external script like in your example in the comment | |
| 16:24:20 | melwitt | and then I was thinking, is that a problem, if we imagine an external script executing a cold migrate and it fails and the instance stays ACTIVE so the script doesn't know it didn't work. that sort of thing | |
| 16:25:08 | melwitt | I was wondering about that after I read the comments on the old auto-revert patch | |
| 16:26:00 | dansmith | but the difference is, | |
| 16:26:14 | mriedem | the external thing should be waiting for task_state to be None to know the operation is done (or the instance action is finished/error, or the migration status is 'finished' or whatever in this case) | |
| 16:26:18 | dansmith | the merged patch corrected state before it changed from $orig to MIGRATING or whatever, right? | |
| 16:26:20 | mriedem | polling the vm_state in the API is not sufficient | |
| 16:26:42 | dansmith | the auto-revert one has it go into all the migrating states and then pop back | |
| 16:27:16 | dansmith | specifically, potentially pop back to ACTIVE and not have moved, IIRC | |
| 16:27:43 | melwitt | yes, I believe it did prevent an ERROR state that occurred before going to MIGRATING | |
| 16:28:22 | mriedem | i'm getting lost in the "it" references here when talking about separate changes | |
| 16:28:32 | melwitt | heh, sorry. the merged change | |
| 16:28:35 | mriedem | booth's change was, | |
| 16:28:51 | mriedem | resize/cold migrated failed somewhere and somehow, and the instance was set to ERROR status, right? | |
| 16:29:14 | mriedem | and if you tried doing a revertResize API call on that ERROR instance, it would do the revert resize flow to go back from the dest to the source host | |
| 16:29:31 | dansmith | no, it did a full revert I think | |
| 16:29:37 | mriedem | even though what we could have failed on was maybe something in prep_resize or resize_instance before the guest / volumes / networking ever actually *got* to the dest host | |
| 16:30:10 | dansmith | so we get to the dest host, fail, auto-revert back to source, and go back to ACTIVE | |