| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-12-12 | |||
| 16:42:02 | artom | There might be operator intervention necessary to manually clean mdevs in certain cases | |
| 16:42:04 | bauzas | I don't see any upgrade concerns about it, the fact is that we will start persisting mdevs upon reboot by every compute that's upgraded | |
| 16:42:15 | artom | Because the mdevs would outlive their instances and host reboots | |
| 16:42:37 | artom | So for instance, changing the enabled mdev types (after draining the host), the operator would need to clean up the old mdevs | |
| 16:42:52 | sean-k-mooney | one thing you need to be careful of | |
| 16:42:56 | bauzas | artom: surely this will require a releasenote and some upstream docs, but this doesn't require to add a new DB model or anything about RPC | |
| 16:42:57 | gibi | as draining the host will not remove mdevs? | |
| 16:43:08 | sean-k-mooney | is on restating nova to a version with this support | |
| 16:43:19 | sean-k-mooney | if we have vms using mdevs created via sysfs | |
| 16:43:49 | sean-k-mooney | we need to supprot cretign the libvirt nodedev object to persist them | |
| 16:44:05 | bauzas | sean-k-mooney: I see the upgrade path for persisting the mdevs to restart the instances | |
| 16:44:12 | sean-k-mooney | i.e. we need to supprot upgrade in place without restarts | |
| 16:44:21 | sean-k-mooney | why? | |
| 16:44:33 | sean-k-mooney | we shoudl not need vm downtime or move operations | |
| 16:44:41 | bauzas | oh my bad, I was wrong | |
| 16:44:50 | bauzas | the mdev would already be created | |
| 16:44:54 | sean-k-mooney | yep | |
| 16:44:57 | sean-k-mooney | and in use by the vm | |
| 16:45:09 | sean-k-mooney | we jsut need to create the mdev with the same uuid in the libvirt api | |
| 16:45:12 | sean-k-mooney | to have it persited | |
| 16:45:19 | bauzas | so, I think this feature requires some upgrade doc that explains how to persist the mdecv | |
| 16:45:33 | sean-k-mooney | well nova can do it | |
| 16:45:34 | bauzas | I mean, an admin doc | |
| 16:45:42 | sean-k-mooney | but we can have n upgade doc to cover how this works | |
| 16:46:10 | sean-k-mooney | i would hope its litrally just update the nova-compute binary adn restart the compute agent | |
| 16:46:15 | bauzas | sean-k-mooney: do you think of a nova-compute startup method that would check every single mdev and would persist it ? | |
| 16:46:15 | sean-k-mooney | no other upgrade impact | |
| 16:46:29 | sean-k-mooney | init host can reconsile what we expect based on teh current xmls | |
| 16:46:41 | sean-k-mooney | ya | |
| 16:46:45 | sean-k-mooney | that is what i was thinking | |
| 16:46:48 | bauzas | that's an additional effort, sure, still specless I think | |
| 16:46:57 | sean-k-mooney | we can defer that to impelmation review if we like | |
| 16:47:13 | sean-k-mooney | but i woudl ike to see upgrade in place support with or without a hard reboot i guess | |
| 16:47:15 | bauzas | sounds acceptable o me | |
| 16:47:37 | bauzas | we already have a broken method that's run on init_host | |
| 16:47:48 | bauzas | we could amend it to persist the mdev instead | |
| 16:48:12 | bauzas | (ie. delete and recreate it using libvirt API with the same uuid) | |
| 16:48:38 | bauzas | artom: does that sound reasonable to you ? | |
| 16:49:06 | artom | Sorry, reading back, in multiple places at the same time | |
| 16:49:28 | bauzas | IRC meetings are good, but people do many things at the same time :) | |
| 16:49:39 | bauzas | I wish we could be more in sync somehow :) | |
| 16:50:34 | bauzas | given we only have 10 mins left and anther procedural approval to do | |
| 16:50:41 | bauzas | lemme summarize | |
| 16:51:06 | bauzas | 1/ the feature will ensure that every new mdev to create will use the libvirt API | |
| 16:51:38 | bauzas | 2/ at compute restart, the implementation will check every mdev that's created and will recreate it using the libvirt API | |
| 16:52:11 | artom | OK, yeah, I think that makes sense, though the mechanics of persisting existing transient mdevs are less obvious to me at this time | |
| 16:52:25 | bauzas | 3/ documentation will address the fact that operator needs some cleanup (unpersisting the mdevs) in case they want to change the type, additional to the fact they need to drain vgpu instances from that host | |
| 16:52:38 | sean-k-mooney | i would hope its just generateing an xml and asking livbirt to create it | |
| 16:52:54 | bauzas | artom: don't be afraid, I exactly see what, where and how to do it | |
| 16:52:57 | sean-k-mooney | it should see that it already exist and i hope just write the mdevctl file | |
| 16:53:07 | sean-k-mooney | we can do this at the end of the seriese | |
| 16:53:12 | sean-k-mooney | to not blcoke the over all feature | |
| 16:53:32 | bauzas | based on those 3 bullet points, I don't see anything that requires a spec | |
| 16:53:37 | bauzas | anyone disagreeing ? | |
| 16:53:58 | bauzas | looks not | |
| 16:54:13 | bauzas | and as a reminder, a specless approval isn't a blank check about design | |
| 16:54:28 | bauzas | if something controversial comes up, we could revisit that and ask for a spec | |
| 16:54:34 | bauzas | based on that | |
| 16:54:40 | gibi | looks OK to me | |
| 16:54:54 | bauzas | #agreed https://blueprints.launchpad.net/nova/+spec/persistent-mdevs accepted as a specless blueprint | |
| 16:55:08 | gibi | Im not clear about when manual cleanup is needed but we can discuss that in the review | |
| 16:55:15 | bauzas | #action artom to amend the blueprint description to note what we agreed | |
| 16:55:24 | bauzas | moving on | |
| 16:55:32 | bauzas | I really want the last item to be discussed | |
| 16:55:37 | bauzas | (JayF/johnthetubaguy) Specless blueprint for ironic guest metadata | |
| 16:55:40 | JayF | o/ | |
| 16:55:42 | bauzas | JayF: 'sup ? | |
| 16:55:48 | JayF | I am unsure if John will be here, but I am. | |
| 16:55:49 | bauzas | https://blueprints.launchpad.net/nova/+spec/ironic-guest-metadata | |
| 16:55:53 | bauzas | shot | |
| 16:55:57 | bauzas | shoot* even | |
| 16:56:44 | sean-k-mooney | reading it quickly it looks reasonable but i would use flavor uuid instead of name or both | |
| 16:56:59 | bauzas | ditto here | |
| 16:57:00 | JayF | Essentially, libvirt instances get a large amount of useful metadata that Ironic would like to get as well for various uses -- the primary case that drove us to this was implementing Ironic's "automatic_lessee" support, allowing Ironic to use the project that provisioned an instance some RBAC access to it | |
| 16:57:30 | JayF | but generally many of those metadata items map to previous feature requests / things that operators have asked for in node instance_info for troubleshooting in the past (like flavor) | |
| 16:57:33 | bauzas | I only care about the upgrade path | |
| 16:57:36 | JayF | so it seemed like a good fit/easy win | |
| 16:57:37 | sean-k-mooney | so i guess my request woudl be can we update the decription with the full list of things we want to be set | |
| 16:58:04 | bauzas | would we need some interim period to ensure all computes report that metadata ? | |
| 16:58:22 | JayF | Essentially this is just additional metadata you'd set on deploy | |
| 16:58:28 | JayF | it'd be Ironic's job to do the right thing if it's set/not set | |
| 16:58:30 | sean-k-mooney | so this is settign metadta on the ironic nodes right | |
| 16:58:32 | sean-k-mooney | not the compute nodes | |
| 16:58:34 | JayF | from a Nova standpoint, it should be 100% backwards compatible | |
| 16:58:41 | sean-k-mooney | as in when an instnace is shcudled ot a node | |
| 16:58:52 | sean-k-mooney | so for upgrade i guess we coudl have a nova-manage command | |
| 16:58:57 | sean-k-mooney | to set it for existing instnces | |
| 16:59:02 | JayF | oh, you mean for backfilling instance metadata, I understand | |
| 16:59:08 | sean-k-mooney | yep | |
| 16:59:10 | JayF | that's a case I hadn't even consideredd! | |
| 16:59:28 | sean-k-mooney | so i assume this would normally only be set on spawn | |
| 16:59:37 | sean-k-mooney | since ironci dose not support resize | |
| 16:59:45 | sean-k-mooney | well spawn or rebuild/evacuate | |
| 17:00:02 | JayF | Alright, looks like I have two actions: 1) List all the specific fields and 2) add details about migration path for preexisting instances and how/if they get metadata | |
| 17:00:08 | JayF | sean-k-mooney: we do rebuild, very common use case | |
| 17:00:19 | sean-k-mooney | ya so we would want to update it on rebuild right | |
| 17:00:33 | sean-k-mooney | so 3 actions. list the data to set, list when it will be set | |
| 17:00:39 | bauzas | sounds then an implementation detail to me | |
| 17:00:44 | sean-k-mooney | and then if we want to have a nova-manage command to backfile then detail that too | |
| 17:00:50 | JayF | yeah but one I don't mind enumerated in the blueprint to ensure I don't miss it | |