| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-09-01 | |||
| 14:09:03 | sean-k-mooney | elodilles: by the way can you add https://review.opendev.org/c/openstack/nova/+/833435 to your list whenever you have time | |
| 14:10:41 | elodilles | sean-k-mooney: ack, looking | |
| 14:11:02 | elodilles | bauzas: i'll answer to your mail as well :) | |
| 14:11:26 | sean-k-mooney | its just a backport making its way down to train so not urgent i came across the downstream bug and realise i should proably followup | |
| 14:11:59 | bauzas | dansmith: oh, right | |
| 14:12:16 | bauzas | dansmith: just exception handling at the API level, you're right | |
| 14:12:48 | bauzas | elodilles: don't get me wrong, I wasn't grumbling, I was just pointing out my problem for discussion | |
| 14:13:08 | bauzas | maybe the foundation folks don't exactly get what's happening at FF | |
| 14:13:16 | bauzas | in particular with the big projects | |
| 14:27:56 | elodilles | bauzas: i summarized what i think about this in my mail. let's see if someone from Marketing or Release team has different view o:) | |
| 14:28:11 | elodilles | sean-k-mooney: the patch looks valid, +2'd | |
| 14:33:29 | sean-k-mooney | ill slowly refersh those patches as they merged but give we are are FF im also trying to limit the ci usage | |
| 14:35:32 | elodilles | sean-k-mooney: ++ | |
| 14:53:42 | opendevreview | sean mooney proposed openstack/nova master: support configdrive rebuilding https://review.opendev.org/c/openstack/nova/+/855351 | |
| 14:53:43 | opendevreview | sean mooney proposed openstack/nova master: add support for updating server's user_data https://review.opendev.org/c/openstack/nova/+/816157 | |
| 14:54:19 | sean-k-mooney | dansmith: that wont work ^ but there in the correct order now and i can start on the tests | |
| 14:54:51 | dansmith | sean-k-mooney: okay but we can't land an RPC version that says it does a thing when it doesn't | |
| 14:55:04 | dansmith | sean-k-mooney: so I think your thing needs to be in front with "if False" or something | |
| 14:55:12 | dansmith | and I'll change that to "if the thing is enabled:" | |
| 14:55:46 | sean-k-mooney | dansmith: something like https://review.opendev.org/c/openstack/nova/+/855351/2/nova/virt/libvirt/driver.py#3879 | |
| 14:56:00 | sean-k-mooney | which is never set anywhere to True | |
| 14:56:19 | sean-k-mooney | and this if https://review.opendev.org/c/openstack/nova/+/855351/2/nova/virt/libvirt/driver.py#5011 | |
| 14:56:36 | dansmith | yeah | |
| 14:57:25 | sean-k-mooney | so i was thinking you could start a patch on top of the first one or if you want i can proably do it based on the termbin you provided | |
| 14:57:45 | sean-k-mooney | you will be quicker at that then me since its been a while since i toughed rpc but either way | |
| 14:58:19 | sean-k-mooney | im going to work on the base patch and get it to pass unit and func tests | |
| 14:58:34 | dansmith | yeah, I'm working on the rpc stuff already | |
| 14:58:50 | dansmith | I think that base patch will work now | |
| 14:59:43 | sean-k-mooney | it shoudl work in devstack im expecting the unit test to bitch about the extra paramter to _hard_reboot | |
| 15:04:15 | bauzas | hmm, are configdrivers only supported by libvirt N? | |
| 15:04:20 | bauzas | configdrives | |
| 15:04:30 | bauzas | I'm afraid of https://review.opendev.org/c/openstack/nova/+/816157/16/nova/virt/driver.py | |
| 15:04:36 | sean-k-mooney | techinially they are supproted by other drivers i think | |
| 15:04:38 | bauzas | trampling all our drivers but libvirt | |
| 15:04:39 | gibi | bauzas: nope, it is supported by other drivers too | |
| 15:04:59 | bauzas | that's super late for them to support them | |
| 15:05:07 | bauzas | s/them/this | |
| 15:05:08 | sean-k-mooney | they dont need too | |
| 15:05:13 | sean-k-mooney | they just wont be able to use this feature | |
| 15:05:26 | sean-k-mooney | i think that was in the spec | |
| 15:05:41 | sean-k-mooney | either way this was only planned to be implented by libvirt | |
| 15:05:50 | sean-k-mooney | at least for now | |
| 15:05:58 | bauzas | hmmmm | |
| 15:06:14 | gibi | so we need to detect the capability in the API and reject the reboot request | |
| 15:06:28 | bauzas | yup | |
| 15:06:29 | sean-k-mooney | thats in the patch currentlyyes | |
| 15:06:32 | gibi | if there is user_data update but the virt driver has no capability to regenerate | |
| 15:07:17 | bauzas | it wasn't stated a virt driver dependency in the spec https://review.opendev.org/c/openstack/nova-specs/+/816542/7/specs/zed/approved/update-userdata.rst | |
| 15:07:24 | sean-k-mooney | by the way we can now drop the object changes | |
| 15:07:42 | bauzas | this change seems to me more and more fragile | |
| 15:07:45 | sean-k-mooney | bauzas: well im not going to have time to work on ironic by monday | |
| 15:07:57 | bauzas | sean-k-mooney: I'm not asking you to do such things | |
| 15:08:04 | gibi | (it is not just ironice) | |
| 15:08:05 | sean-k-mooney | so hehe i think we were reving this as libvirt only or at least i was | |
| 15:08:10 | bauzas | but I point out how fragile this is | |
| 15:08:17 | sean-k-mooney | gibi: ya i know | |
| 15:08:33 | bauzas | and again, this wasn't explained in the spec | |
| 15:08:34 | sean-k-mooney | ironic would just be the most painful | |
| 15:08:55 | sean-k-mooney | bauzas: orgianly in the spec they planned to only do it for the metadta api | |
| 15:08:55 | bauzas | I was having concerns with this spec because I thought it wasn't really a needed usecase | |
| 15:09:07 | sean-k-mooney | but then we pointd out config drive exsited and need to also work | |
| 15:09:07 | bauzas | sean-k-mooney: no, they said about configdrives too | |
| 15:09:16 | sean-k-mooney | bauzas: no i asked them to add that | |
| 15:09:25 | sean-k-mooney | they wanted api only | |
| 15:09:35 | bauzas | sean-k-mooney: yes, but I also said it was a problem | |
| 15:09:35 | sean-k-mooney | id did not want this to not work if you used config drive | |
| 15:10:06 | sean-k-mooney | so as it stands any driver that does not supprot the new metond will not report the trait | |
| 15:10:19 | sean-k-mooney | that need to be called out in the api ref | |
| 15:10:26 | sean-k-mooney | or other docs for this | |
| 15:10:39 | sean-k-mooney | we could put it in the driver suport matirx i guess | |
| 15:10:56 | sean-k-mooney | that proably beter long term | |
| 15:14:07 | sean-k-mooney | dansmith: only 6 failure for the extra paramater | |
| 15:14:28 | sean-k-mooney | i actully need the driver api change in the first patch too so ill add that | |
| 15:15:12 | gibi | I'm wondering about the virt driver api change | |
| 15:15:26 | dansmith | but bauzas' point about this not being supported by other drivers is legit | |
| 15:15:37 | dansmith | having features that look like swiss cheese is very confusing for users | |
| 15:15:47 | sean-k-mooney | gibi: i could drop that now actully | |
| 15:15:49 | gibi | I asked for it originally as I thought the reboot + regenerate logic can be orchestrated from the compute manager | |
| 15:15:52 | sean-k-mooney | if we just have the parmater | |
| 15:16:03 | dansmith | even if we reject for the api caller, it's still annoying to say "reboot works, but not reboot with userdata, but only if you're using configdrive" | |
| 15:16:09 | gibi | but the actualy implementation calls the regenerate call from the virt driver | |
| 15:16:22 | dansmith | gibi: reboot is a single call to virt, | |
| 15:16:22 | gibi | so it is less useful to have a virt driver method | |
| 15:16:26 | dansmith | and the regenerate has to happen in the middle | |
| 15:16:31 | gibi | dansmith: I see now | |
| 15:16:50 | dansmith | it's pretty unideal in general for sure | |
| 15:16:59 | dansmith | just saying, I think that's why it was done this way | |
| 15:17:05 | gibi | this is why I pointing out that the separate virt driver method is probably not needed | |
| 15:17:05 | sean-k-mooney | so we dont need a new public methond in the virt driver api | |
| 15:17:20 | sean-k-mooney | its just a new parmater to hard_reboot right | |
| 15:17:22 | dansmith | no, the publicness isn't the concern I think | |
| 15:17:36 | sean-k-mooney | right i know its the uniformatiy of the api | |
| 15:17:43 | sean-k-mooney | and not having it depend on the backend | |
| 15:17:51 | bauzas | agreed with dansmith my concern isn't the publicness | |
| 15:18:05 | bauzas | this is about the fact we depend on virt drivers | |
| 15:18:07 | gibi | it would be nice to do the reboot in 3 generic steps from the compute manager: stop, regenerate, start. But this is a big change | |
| 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 | |