| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-12-12 | |||
| 17:12:22 | sean-k-mooney | to trigger the backfile or just od it with a script | |
| 17:13:22 | sean-k-mooney | i tought it might be simlpel to just have nova-manage call the function that normally generates the metatada we would set and call the ironic api with that | |
| 17:13:27 | JayF | A script, for sure, and I could see the shape of that script being such that nova-backfill is an option on it | |
| 17:13:31 | sean-k-mooney | but if that is not simple then no need to do it in nova | |
| 17:13:43 | JayF | Yeah but like I said, that's only like, half of the equation and alone is not super valuable | |
| 17:14:03 | sean-k-mooney | ack | |
| 17:14:27 | sean-k-mooney | for what its wroth you have remined me that i have wanted to extend the metadata in the libvirt xml for years | |
| 17:14:50 | sean-k-mooney | sepeicicly i want to have both the image and flagor (name and uuid) and have the image /falvor extra specs direclly in the metadata | |
| 17:14:52 | JayF | well that's like, another thing I wanted to say | |
| 17:15:07 | JayF | Why do we want a libvirt metadata list and an Ironic metadata list? | |
| 17:15:25 | JayF | can there not just be a "here's a big list of instance metadata that hypervisors that care about metadata cna have" | |
| 17:15:49 | sean-k-mooney | we coudld have this in common code yes | |
| 17:16:06 | JayF | John and I were looking at this, and that's where the bp came from | |
| 17:16:08 | sean-k-mooney | all the dirver need to do is transform the dic of metadta to where it stores it | |
| 17:16:29 | JayF | ++ that would be ideal | |
| 17:16:52 | sean-k-mooney | so the reaosn i want to add both the name and uuid is the image uses one and the flavor uese the other | |
| 17:16:57 | sean-k-mooney | both shoudl use both | |
| 17:17:20 | sean-k-mooney | and the extra_specs/image properties are so if you dont ahve a db dump | |
| 17:17:23 | JayF | the only fields I have strong feelings about are any-flavor-identifier-at-all and the project information | |
| 17:17:29 | sean-k-mooney | you can just look at the xml and know everything about it | |
| 17:17:30 | JayF | the rest of them are things that are situationally useful | |
| 17:18:00 | JayF | almost all of those metadata fields in the libvirt driver, I was able to think of a time troubleshooting when that in node.instance_info would've been useful :) | |
| 17:18:19 | sean-k-mooney | JayF: related question | |
| 17:18:28 | sean-k-mooney | out of interest why do you care about the flavor | |
| 17:18:50 | JayF | Super common support question for Ironic admins | |
| 17:18:52 | sean-k-mooney | what will it be used for? | |
| 17:18:57 | JayF | that's the answer | |
| 17:19:10 | JayF | 'what flavor did you use to boot this' often dictates some actions Ironic takes on spawn | |
| 17:19:14 | sean-k-mooney | oh ok so the resouce class on the node is not enough | |
| 17:19:17 | JayF | yep | |
| 17:19:23 | sean-k-mooney | ok makes sense | |
| 17:19:28 | sean-k-mooney | this is why its there in the libvirt xml | |
| 17:19:40 | sean-k-mooney | but often i get a comptue log with no flavor info | |
| 17:19:46 | JayF | because resource_class + capabilities (not sure I'm using right nova term ?) means you might have the same node | |
| 17:19:51 | sean-k-mooney | so that why the flavor detail like the extra spec are useful to me | |
| 17:19:57 | JayF | but it gets a different deployment template, giving it different bios config on spawn | |
| 17:20:09 | johnthetubaguy | sean-k-mooney: FWIW, I feel like this should be "similar" to what we do with libvirt xml, to get the extra info in there. | |
| 17:20:30 | JayF | so you might have, supersize-node-configa supersize-node-configb and both are node.resource_class=supersize-node | |
| 17:20:38 | sean-k-mooney | johnthetubaguy: ya so the libvirt part is just using the flavor and image + the project id i think | |
| 17:20:54 | JayF | johnthetubaguy: we were just talking about maybe that code becoming common | |
| 17:21:10 | JayF | johnthetubaguy: so the ironic/libvirt driver code would just be taking a dict of metadata and putting it in the right place | |
| 17:21:12 | johnthetubaguy | sean-k-mooney: yeah, I think it makes sense to be the same, or at least similar. | |
| 17:22:18 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/config.py#L3600-L3642 | |
| 17:22:55 | sean-k-mooney | well actuly https://github.com/openstack/nova/blob/master/nova/virt/libvirt/config.py#L3600-L3699 | |
| 17:23:23 | sean-k-mooney | i have been meing to extend that for a while but it was never a high enough priority | |
| 17:23:32 | sean-k-mooney | i think that ahs most of what you want already | |
| 17:24:14 | JayF | I'm pretty sure that's *exactly* the code we had up in the meeting when we made this bp | |
| 17:24:34 | johnthetubaguy | JayF: what is missing? yeah, that is the code I was thinking about before. | |
| 17:26:12 | JayF | as long as it has the project that spawned it and the flavor that's the primary things we care about | |
| 17:26:22 | JayF | and I see a self.owner and a self.flavor | |
| 17:28:00 | johnthetubaguy | JayF: +1 I think that has it all. Feels like a case of adding to_dict(), in a way. | |
| 17:28:06 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L5780 | |
| 17:28:15 | sean-k-mooney | so this is where we generate this today | |
| 17:28:17 | johnthetubaguy | ah, that was it. | |
| 17:28:32 | sean-k-mooney | we could move that up to the base virt dirver | |
| 17:28:57 | sean-k-mooney | it just taks the instance and network info today | |
| 17:29:10 | sean-k-mooney | ok so not quite | |
| 17:29:16 | sean-k-mooney | its currently building livbirt objects | |
| 17:29:39 | johnthetubaguy | Yeah, if we have something generating a dict we can both use, that would have everything we need | |
| 17:29:41 | sean-k-mooney | but we coudl build a dict or other object that is not libvirt specific | |
| 17:31:25 | JayF | and then the MVP for the Ironic change becomes "toss that metadata into the instance_info patch on spawn" and "do the proper $whatever on rebuild/other non-spawn instance modifying actions" | |
| 17:31:42 | sean-k-mooney | basically ya | |
| 17:31:45 | JayF | then we'll have an Ironic side change to respect that metadata for automatic_lessee and to add a backfill script | |
| 17:31:58 | sean-k-mooney | the libvirt driver just generate it every time we generate an xml | |
| 17:32:03 | JayF | (which the backfill script will also backfill node.lessee if configured to do so, which is why having it in Ironic is great) | |
| 17:32:09 | sean-k-mooney | so on reboot as well | |
| 17:32:13 | sean-k-mooney | im not sure if you just want to do that | |
| 17:32:21 | sean-k-mooney | and say reboot the node to backfile or not | |
| 17:32:44 | sean-k-mooney | that is how we handlel it when we add more metadata for libvirt | |
| 17:33:01 | sean-k-mooney | just tell people reboot, cold migate or anything else that updates the xml | |
| 17:33:19 | JayF | What kind of actions cause metadata to change? | |
| 17:33:24 | JayF | Let me ask that differently. | |
| 17:33:37 | JayF | What kind of non-API-initiated-actions can cause metadata to change? | |
| 17:33:42 | JayF | ^ user | |
| 17:33:50 | sean-k-mooney | interface attach/detach, resize, rebuild and evcauate | |
| 17:34:23 | sean-k-mooney | technially it wont change on reboot but we just do that anyway because its simpelr since we deltee and recreate the xml on every reboot | |
| 17:34:28 | JayF | we'd may want to consider filtering interface metadata in ironic instance_info, because ironic has it's own network info | |
| 17:34:34 | JayF | and rebuild is the only other case that impacts Ironic | |
| 17:35:07 | sean-k-mooney | i think its ok for a virt direr to profject a view of the metdata that only has the filed that makes sense to it | |
| 17:35:18 | JayF | so I think we should be able to get it to a case where Ironic instnace metadata would only change meaningfully on rebuild/spawn/destroy | |
| 17:35:38 | sean-k-mooney | ya that sounds about right | |
| 17:36:02 | johnthetubaguy | sean-k-mooney: yeah, I think the common code can provide the set of metadata allowed, Ironic can pick a subset of that, I think skipping the network bits makes sense. | |
| 17:36:22 | sean-k-mooney | with that said | |
| 17:36:38 | sean-k-mooney | on the network side i know ironci suport portgroups | |
| 17:36:56 | sean-k-mooney | but when an ironic node is assocated with a nova instance | |
| 17:37:08 | sean-k-mooney | the only addtion or remoavl of interface | |
| 17:37:13 | sean-k-mooney | should be done via nova api | |
| 17:38:10 | sean-k-mooney | im aware that vif plug in the ironic driver calls ironic which does thign with neutron | |
| 17:38:47 | sean-k-mooney | btu it shoudl not really affect which ports/port groups are assocated with the instance form a nutron point of view | |
| 17:39:43 | JayF | Yes. The feeling behind not wanting to pass thru the network metadata is much, much more basic than that: there's already like, three views to networks when people come to Ironic with support: Nova's view, Neutron's view, Ironic's view (port object)... not even counting various bifrost/whatever type of configs | |
| 17:40:07 | JayF | I just don't want to see any bugs with "here's the node.instance_info['meta']['network']" or pollute the DB with more redundant info :) | |
| 17:40:08 | sean-k-mooney | :) | |
| 17:40:35 | sean-k-mooney | well the metadta does not ahve the full network object | |
| 17:40:42 | sean-k-mooney | just the ip s | |
| 17:40:46 | sean-k-mooney | assocated with an instance | |
| 17:41:05 | sean-k-mooney | but if you already have that no need to duplciate it as you said | |
| 17:41:06 | JayF | which is actually information that people might want | |
| 17:41:14 | JayF | IP is in neutron, we don't have it in Ironic | |
| 17:41:36 | JayF | Ironic holds more of the information about the physical port: mac address, binding profile/metadata about how it's physically connected to switches, etc | |
| 17:45:06 | sean-k-mooney | the ip stuff was a relitivly late addtion | |