| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-09-01 | |||
| 15:18:16 | sean-k-mooney | yep i get that but we have many things that depend on the virt driver | |
| 15:18:20 | dansmith | I definitely think these kinds of pinpoint features that look generic but only work in libvirt are a problem | |
| 15:19:01 | bauzas | sean-k-mooney: well, do all the virt drivers support to create *or* update the configdrives ? | |
| 15:19:18 | dansmith | create yes | |
| 15:19:23 | sean-k-mooney | vmware ironic libvirt yes | |
| 15:19:23 | dansmith | nobody supports update today AFAIK | |
| 15:19:27 | bauzas | dansmith: that's my point, I wasn't seeing the dependency when reviewing the spec | |
| 15:19:29 | sean-k-mooney | for create | |
| 15:19:35 | sean-k-mooney | not sure about powervm | |
| 15:19:53 | dansmith | gibi: the problem with that is things like vmware where I'm not sure where the config drive actually gets created, or even if generating it on the compute node and passing it to the virt driver as a file would necessarily be reasonable | |
| 15:19:55 | dansmith | since it's a remote thing | |
| 15:20:01 | sean-k-mooney | i can take a look and see what would be required for other driver quickly | |
| 15:20:07 | sean-k-mooney | ironic is the hard one | |
| 15:20:29 | sean-k-mooney | they pass the config drive via ipa | |
| 15:21:02 | gibi | dansmith: I see | |
| 15:21:03 | dansmith | like I think ironic passes the configdrive as a string to the api, | |
| 15:21:07 | dansmith | never generates it on disk | |
| 15:21:43 | sean-k-mooney | ah that might be how nova passes it to ironci but then ironic connects it to the vm via ipa or mounting it | |
| 15:21:50 | dansmith | yeah | |
| 15:22:01 | dansmith | it serializes it and then ironic writes it remotely I think | |
| 15:22:22 | dansmith | now, if compute generated the file I guess the driver could read and send it | |
| 15:22:30 | dansmith | but still, those are big changes requiring lots of testing | |
| 15:22:52 | sean-k-mooney | so really we would want the regeneration logic to be here https://github.com/openstack/nova/blob/master/nova/virt/configdrive.py | |
| 15:22:57 | sean-k-mooney | in the generic shared part | |
| 15:23:26 | sean-k-mooney | althogh maybe not | |
| 15:23:32 | sean-k-mooney | that does not quite do what i was expecting | |
| 15:23:35 | bauzas | I tend to agree with dansmith, we're playing with guns | |
| 15:23:55 | dansmith | well, I like playing with guns, but maybe.. playing with fire? :) | |
| 15:24:00 | bauzas | if we try to update a configdrive and this doesn't work, then our user won't be happy at all | |
| 15:24:28 | bauzas | dansmith: what you prefer | |
| 15:24:32 | bauzas | or playing with teslas | |
| 15:25:06 | dansmith | as long as you mean "teslas, the unit of charge, meaning playing with dangerous high-voltage" .. then sure :D | |
| 15:25:15 | bauzas | point is, we need to only accept to update the userdata if the configdriver correctly regenerated | |
| 15:25:18 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L1060-L1092 | |
| 15:25:28 | sean-k-mooney | so ironci jsut tuns it into a string yes | |
| 15:25:48 | bauzas | dansmith: like "driving a tesla with a 10% battery for 100km" | |
| 15:25:55 | dansmith | oh I see | |
| 15:26:02 | dansmith | that *is* scary :) | |
| 15:26:10 | bauzas | this *may* work | |
| 15:26:19 | bauzas | but before driving, you need to ensure you can do it | |
| 15:26:34 | bauzas | the same goes with configdrives | |
| 15:26:50 | sean-k-mooney | and they pass it in spawn and rebuild https://github.com/openstack/nova/blob/bcdf5988f6ae902dba9b41144a7b4a60688b627c/nova/virt/ironic/driver.py#L1189-L1191 | |
| 15:26:54 | bauzas | you only accept the userdata to be updated once you're sure your configdrive is correctly regenerated | |
| 15:27:12 | dansmith | so the feeling I'm getting here is that there are a lot of open questions at this point | |
| 15:27:19 | bauzas | since the user doesn't have an idea whether the userdata is updated and only relies on the fact he/she restarted the instance | |
| 15:27:32 | sean-k-mooney | bauzas: well that is already a problem today | |
| 15:27:40 | dansmith | sean-k-mooney: no it's not | |
| 15:27:45 | bauzas | today the userdata is immutable | |
| 15:27:53 | dansmith | sean-k-mooney: if you can only provide userdata during create, it's clear what userdata the instance sees | |
| 15:28:03 | bauzas | that reminds me the discussions we had when reviewing server groups /PUT | |
| 15:28:06 | sean-k-mooney | it is the config drive is not updated today if you atach interface or volumes or update the server metadta | |
| 15:28:10 | sean-k-mooney | the user data is not statble | |
| 15:28:16 | sean-k-mooney | btu in general the config drive can be | |
| 15:28:34 | dansmith | configdrive != userdata | |
| 15:28:43 | sean-k-mooney | yep that is my point | |
| 15:28:51 | sean-k-mooney | the config drive can be stale today the user data is not | |
| 15:29:00 | sean-k-mooney | but that is only because its imutable | |
| 15:29:08 | dansmith | right | |
| 15:29:12 | dansmith | not ideal, but also not ambiguous | |
| 15:29:15 | dansmith | which is bauzas' point I think | |
| 15:29:18 | bauzas | yup | |
| 15:29:31 | bauzas | since it's immutable, this isn't a problem | |
| 15:29:36 | sean-k-mooney | sure but its expected that the user data script will likely depend on the other datta in the config drive | |
| 15:29:57 | sean-k-mooney | specificly the device role taggin info | |
| 15:29:59 | opendevreview | Dan Smith proposed openstack/nova master: WIP: Add recreate_configdrive to reboot https://review.opendev.org/c/openstack/nova/+/855529 | |
| 15:30:12 | dansmith | sean-k-mooney: here's the rpc stuff ^ passing tests, not wired into anything else | |
| 15:30:20 | dansmith | just so it's available | |
| 15:30:30 | sean-k-mooney | dansmith: but to your point it does sound like there is enough stuff that we wont have this mergable even with an FFE | |
| 15:30:32 | dansmith | needs more test coverage and an assert on soft I think | |
| 15:33:04 | bauzas | sean-k-mooney: my personal opinion is that the more I review this API change, the more I think I discover points we missed | |
| 15:33:10 | bauzas | hence my concern | |
| 15:33:15 | dansmith | so I should probably rebase that on your regen configdrive patch and I guess I can rebase the api bit on top of that and trivially wire it up | |
| 15:33:59 | bauzas | the configdrive regeneration is already a concern (making sure we validate it synchronously with the userdata update) | |
| 15:34:13 | bauzas | the fact that we leave other virt drivers in the weeds is another concern | |
| 15:34:21 | bauzas | both weren't discussed at the PTG | |
| 15:34:46 | dansmith | so on that, | |
| 15:34:53 | bauzas | and when I reviewed the spec, I was letting enough comments explaining how I was thinking the reboot solution was fragile | |
| 15:34:59 | dansmith | are we really expecting anyone to update vmware or hyperv to do this? | |
| 15:35:05 | bauzas | reasonably not | |
| 15:35:10 | dansmith | ironic perhaps, but for the above patch, | |
| 15:35:19 | dansmith | I was looking at those other drivers and wondering when they were last touched | |
| 15:35:35 | bauzas | surely, but we haven't even asked them to look | |
| 15:35:53 | dansmith | okay I guess we've had changes in 2022 for vmware | |
| 15:35:58 | dansmith | so maybe doable | |
| 15:36:31 | dansmith | not much for hyperv though | |
| 15:37:18 | dansmith | so I don't see any configdrive stuff in hyperv at first glance | |
| 15:37:48 | bauzas | fair enough, but then question | |
| 15:37:54 | bauzas | I'm an user | |
| 15:38:09 | bauzas | I wanna update my pet's userdata | |
| 15:38:13 | bauzas | I do it | |
| 15:38:33 | bauzas | and then I don't understand, my f*** script doesn't run as expected when I restart the guest | |
| 15:38:44 | bauzas | shall I open a ticket ? | |
| 15:39:08 | dansmith | oh it's in hyperv, just in vmops.py | |
| 15:39:24 | bauzas | oh, simple, your cloud was running some driver on that host that was having trouble with updating the configdrive | |
| 15:39:35 | bauzas | either because it was a legacy driver | |
| 15:39:44 | bauzas | or because something (like a perm error) expected | |
| 15:40:10 | bauzas | honestly, this spec was just about touching an internal Nova DB record | |
| 15:40:21 | bauzas | now, we're pulling way more than that | |
| 15:40:47 | dansmith | yeah, it's becoming more about configdrive than anything else | |
| 15:41:27 | sean-k-mooney | bauzas: in two converstaion but form my persepcitve i dont think we really have discoverd anythign bar the rpc change. i reveiw the spec with the understandin it was scoped to libvirt | |