| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-12-12 | |||
| 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 | sean-k-mooney | no other upgrade impact | |
| 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: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 | |
| 17:01:27 | bauzas | nova-manage what ? fill my ironic stuff for that instance ? | |
| 17:02:05 | sean-k-mooney | bauzas: ya set the metadtaa on the corresponding ironc node for an existing instance | |
| 17:02:19 | bauzas | couldn't it be some ironic script that would gather the details from the nova API ? | |
| 17:02:23 | sean-k-mooney | that shoudl be pretty simple to do its just a ironic api call | |
| 17:02:40 | sean-k-mooney | it could btu that feels liek a worse soultion to me | |
| 17:02:49 | JayF | So I'll note | |
| 17:02:52 | bauzas | sean-k-mooney: I'm not 100% happy with a very specific virt driver method in our nova-manage command | |
| 17:03:00 | JayF | From an Ironic standpoint, having the data backfilled is not super awesome | |
| 17:03:07 | sean-k-mooney | im pretty sure it woould not be the first | |
| 17:03:09 | JayF | we aren't going to do much with it | |
| 17:03:12 | bauzas | this would require nova-manage to be able to speak the ironic language | |
| 17:03:20 | sean-k-mooney | but this feels like the volume refesh commands to me | |
| 17:03:32 | sean-k-mooney | bauzas: it already can talk to ironci | |
| 17:03:33 | JayF | so while I'm happy to implement it, and I'm sure someone would find a use for it, I don't think it's in the primary path for enabling the sorta features we want | |
| 17:03:56 | sean-k-mooney | so i dont really mind where it ends up i guess but i think it woudl be nice to have | |