| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-05-24 | |||
| 17:02:25 | bauzas | 3/ given 1/ and 2/, we still need a microversion to signal the behavioural change | |
| 17:02:27 | dansmith | and just silently changing that is just asking for a very angry customer | |
| 17:02:53 | sean-k-mooney | dansmith: so what is it doing in this case | |
| 17:03:00 | bauzas | sean-k-mooneypoint is, like it or not, we can't change the behaviour without signaling it | |
| 17:03:04 | sean-k-mooney | updating the image metadata | |
| 17:03:13 | dansmith | sean-k-mooney: now or after this spec merges? | |
| 17:03:16 | sean-k-mooney | if so that can break things and shoudl be blocked | |
| 17:03:20 | sean-k-mooney | now | |
| 17:03:37 | dansmith | sean-k-mooney: now it does all the rebuild machinery it just doesn't change your root disk at all | |
| 17:03:46 | dansmith | i.e. evacuate but without the move | |
| 17:04:02 | sean-k-mooney | well rebuild just does two things | |
| 17:04:03 | dansmith | I'm not so sure it's identical to a hard reboot, but maybe | |
| 17:04:13 | dansmith | it doesn't really matter though | |
| 17:04:17 | sean-k-mooney | erases epmeral storage unless its ironic and you use an api exteion | |
| 17:04:28 | sean-k-mooney | reimage the root disk and hard rebotos | |
| 17:05:10 | sean-k-mooney | dansmith: im really debating if we should be blocking rebuidl with the same image as a bug in older releases | |
| 17:05:21 | dansmith | so I think rebuild will refresh your stored image_meta if the meta has changed on your same image right? | |
| 17:05:22 | sean-k-mooney | it seam dangous to me to allow | |
| 17:05:26 | dansmith | so you could use that to get new numa settings or something | |
| 17:05:35 | sean-k-mooney | yep | |
| 17:05:35 | opendevreview | Elod Illes proposed openstack/placement stable/wallaby: Add periodic-stable-jobs template https://review.opendev.org/c/openstack/placement/+/843174 | |
| 17:05:39 | dansmith | not the intent, but could definitely be people using it that way | |
| 17:05:52 | sean-k-mooney | i only block that if the image chagnes | |
| 17:05:56 | dansmith | not that we need to allow that in the future, BUT it means they could be using it now not expecting destruction of data | |
| 17:06:05 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3647-L3648= | |
| 17:06:22 | sean-k-mooney | dansmith: if the image chagnes we validate the host in the schduler | |
| 17:06:36 | sean-k-mooney | dansmith: i belive we have an optimisation where we do not do that if its the same image | |
| 17:06:49 | sean-k-mooney | dansmith: or at least we used too but maybe that was removed | |
| 17:07:06 | dansmith | sean-k-mooney: but we update the image_meta stored with the instance and rebuild the pci device stuff | |
| 17:07:54 | whoami-rajat | with the new proposal, if opted in, we will be performing the reimage whether it is the same image or different | |
| 17:08:11 | whoami-rajat | if not opted in, we can still perform rebuild for same image but 400 for different image | |
| 17:08:21 | sean-k-mooney | no | |
| 17:08:33 | sean-k-mooney | i really dont think that is safe. i need to read the code | |
| 17:08:36 | dansmith | sean-k-mooney: no what? | |
| 17:08:50 | sean-k-mooney | but i dont think rebuidl to same image in the current case is safe in all cases | |
| 17:09:06 | whoami-rajat | we are keeping backward compatibility with the new microversion ? | |
| 17:09:33 | sean-k-mooney | im quetioning if the curren behavior in the old microverion is a bug | |
| 17:09:57 | sean-k-mooney | whoami-rajat: i tought we blocked it always | |
| 17:10:11 | sean-k-mooney | that is not the case and now im trying to assess if it currently safe as is | |
| 17:10:19 | dansmith | oh yeah rebuild also lets you add/change metadata, server name, keys, user data, hostname, certs, etc | |
| 17:10:35 | dansmith | so people could totes be using that on pets right now and expecting no data loss | |
| 17:10:37 | sean-k-mooney | yes it does now | |
| 17:10:48 | whoami-rajat | ok, I'm not too sure about it, when i started working on it I thought it wasn't supported at all but now I'm trying to impose the new behavior without differentiating with same or different image | |
| 17:11:00 | whoami-rajat | not sure if the old behavior makes sense from a nova perspective | |
| 17:11:08 | sean-k-mooney | whoami-rajat: i tought it was not supported at all too | |
| 17:12:44 | sean-k-mooney | so the resoltion for https://bugs.launchpad.net/nova/+bug/1482040 was just to note that the image is not replaced | |
| 17:12:48 | sean-k-mooney | by linking to the bug | |
| 17:13:07 | sean-k-mooney | where as teh correct fix liekly shoudl have been to block the operatio or implemtn your spec. | |
| 17:13:23 | dansmith | https://github.com/openstack/tempest/blob/44dac69eb77d78a0de8e68e63617099249345578/tempest/api/compute/servers/test_server_actions.py#L292-L329 | |
| 17:13:34 | sean-k-mooney | dansmith: so ya we likely need to actully have a microversion... | |
| 17:13:42 | dansmith | the comment there implies that the test is doing a rebuild of a volume-backed instance, but it's not, just volume-attached | |
| 17:13:49 | dansmith | but it does say "is common" FWIW :) | |
| 17:13:53 | dansmith | sean-k-mooney: yup | |
| 17:14:32 | sean-k-mooney | i really hate that but we need to fix the api ref to docuemnt this properly | |
| 17:15:00 | sean-k-mooney | i guess i can live with new microversion always reimages and old perserves current behaivor | |
| 17:15:04 | bauzas | ok, looks like we then have a consensus | |
| 17:15:07 | dansmith | "There is a known limitation where the root disk is not replaced for volume-backed instances during a rebuild." | |
| 17:15:11 | dansmith | ^ in the api-ref | |
| 17:15:14 | sean-k-mooney | dansmith: does that work for you. | |
| 17:15:27 | bauzas | dansmith: heh, I'm glad I remind this correctly | |
| 17:15:29 | sean-k-mooney | dansmith: ya but i was expecting an error in that case | |
| 17:15:44 | bauzas | sean-k-mooney: are you okay with the direction ? | |
| 17:15:51 | bauzas | personnally, I'm all good | |
| 17:15:58 | gibi | I'm ok with a new microversion | |
| 17:15:58 | dansmith | sean-k-mooney: yes, but I think that it would probably be prudent for the _client_ to have some flag if you provide the same image to make sure you really mean it | |
| 17:16:21 | dansmith | because most people are just going to be firing the client the same way and not realize the change | |
| 17:16:50 | sean-k-mooney | i dont really liek that | |
| 17:16:55 | bauzas | dansmith: that's the whole purpose of microversions, no ? | |
| 17:16:59 | sean-k-mooney | as every custoemr will then have to use a new microver and flag | |
| 17:17:04 | sean-k-mooney | and heat/ansibel ectra | |
| 17:17:17 | sean-k-mooney | will have to then always know if its a bfv instnace or not | |
| 17:17:28 | dansmith | bauzas: it is, but I think most people "opt into" microversions because they try to do something, google for why it's not working and someone says "oh pass this cryptic version flag" | |
| 17:17:50 | sean-k-mooney | BFV is not a flav show on instance show | |
| 17:17:54 | whoami-rajat | if you request the rebuild and it gets rejected, the message will clearly mention that you need to pass ``reimage_boot_volume`` with the request | |
| 17:17:57 | dansmith | but fine, if it's not palatable, then whatever, it's a risk | |
| 17:18:15 | sean-k-mooney | so you need to do some deep introspecation fo the instnace to fiture out if the root voluem is on cinder or not | |
| 17:18:16 | dansmith | whoami-rajat: they're asking for *not* that | |
| 17:18:20 | sean-k-mooney | and we dodnt provde the bdm either | |
| 17:18:40 | whoami-rajat | ok | |
| 17:18:53 | dansmith | sean-k-mooney: the client could always require that flag if the image is not changing, even for non-BFV instances, but I understand the concern | |
| 17:19:25 | dansmith | just saying, someone's going to lose their data.. granted because they weren't paying close enough attention, but .. it's a pet root volume so I just think it's worth being careful here | |
| 17:19:34 | dansmith | but the microversion is the important part to me for sure | |
| 17:19:34 | sean-k-mooney | so new microver alwasy require you to pass yes-i-really-really-mean-it | |
| 17:20:01 | dansmith | sean-k-mooney: that's what whoami-rajat has, at the API level (IIRC), but I was suggesting make that just a client shell behavior | |
| 17:20:26 | sean-k-mooney | dansmith: i think it was only for bfv instnaces in the spec currently | |
| 17:20:28 | dansmith | like, the client will refuse to use microversion 2.x *and* the same image_ref, unless you provide -yes-really | |
| 17:20:35 | dansmith | sean-k-mooney: yes, in the spec currently | |
| 17:20:53 | sean-k-mooney | if it was for all instance i think it woudl be more palletable | |
| 17:21:15 | sean-k-mooney | becasue as an end user i dont need to care if its bfv or not | |
| 17:21:19 | dansmith | right, I was suggesting making it consistent for all to avoid having to know if it's bfv or not | |
| 17:21:44 | sean-k-mooney | ya that im ok with even if it will break people that blindly use latest | |
| 17:21:54 | sean-k-mooney | it will break them by being safer | |
| 17:21:58 | dansmith | right | |
| 17:22:07 | sean-k-mooney | but that will mean rebuidl with nova client wont work anymore | |
| 17:22:20 | sean-k-mooney | since we are nolonger updatign the cli | |
| 17:22:31 | dansmith | sean-k-mooney: again I'm saying make it a client behavior, not an API one | |
| 17:22:47 | sean-k-mooney | so what woudl the api behavior be | |
| 17:22:56 | sean-k-mooney | new microversion just reimage | |
| 17:23:05 | sean-k-mooney | but gaurd in osc | |
| 17:23:07 | dansmith | the api behavior would be "if new, always destroy" | |