| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-05-24 | |||
| 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" | |
| 17:23:09 | dansmith | right | |
| 17:23:15 | sean-k-mooney | ok i like that more | |
| 17:23:32 | dansmith | osc logic is: "if new_version and image_ref==server.image_ref and args.yes_really: then do_it" | |
| 17:23:55 | dansmith | maybe we don't already have the server there in osc I guess, but we do validations like that in other places right? | |
| 17:24:19 | dansmith | sorry, that's not the right logic, let me try again: | |
| 17:24:48 | dansmith | if image_ref==server.image_ref and new_version: if not args.yes_really: explode with warning | |
| 17:25:05 | dansmith | only require the flag if new version and the image is not changing | |
| 17:25:06 | sean-k-mooney | or just check for new version | |
| 17:25:09 | whoami-rajat | but if someone directly curls the API (not from client), don't we require the validation of additional parameter? | |
| 17:25:24 | dansmith | could do that, but then everyone always has to do that, and the image not changing is so niche | |
| 17:25:31 | dansmith | whoami-rajat: right | |
| 17:25:38 | sean-k-mooney | whoami-rajat: i really dont think that heat should have to do differnt thigns for bfv or not | |
| 17:26:00 | dansmith | sean-k-mooney: and also, heat had better know the impact of the new microversion if they opt into it | |
| 17:26:11 | sean-k-mooney | ya | |
| 17:26:37 | sean-k-mooney | this comes back to not blindly using latest | |
| 17:26:44 | dansmith | heh | |
| 17:27:15 | dansmith | people already blindly paste the shell code from the first answer on stackexchange into a root terminal, | |
| 17:27:26 | dansmith | we're pretty well sunk on making them carefully consider microversions :) | |
| 17:27:26 | sean-k-mooney | so from a tempest point of view we would want test for the new and old microverion too right | |
| 17:27:32 | dansmith | yes | |
| 17:28:28 | sean-k-mooney | am so do you want to summerise what you propsoe we do | |
| 17:29:46 | sean-k-mooney | new micorversion -> always reimage, old -> preseve data for bfv reimage for not bfv, evac-> alwasy preserve data if on share starge regardless of micoversion(no change) | |
| 17:29:46 | dansmith | I don't "want" to, but I will | |
| 17:30:33 | whoami-rajat | since it was mentioned, tempest test for new behavior https://review.opendev.org/c/openstack/tempest/+/831018 | |
| 17:30:41 | whoami-rajat | still in progress though | |
| 17:32:33 | bauzas | folks, btw. I forgot to remember that I'll off from tonight until Monday | |
| 17:32:34 | bauzas | thanks | |
| 17:34:53 | dansmith | sean-k-mooney: https://review.opendev.org/c/openstack/nova-specs/+/840155 | |
| 17:36:08 | sean-k-mooney | dansmith: ack just reading it that sounds good to me | |
| 17:36:15 | dansmith | cool | |
| 17:36:19 | sean-k-mooney | whoami-rajat: reading the tempest test its doing some thing i think shoudl not be in the test | |
| 17:36:34 | sean-k-mooney | and its not valdiating eveythign i think ti shoudl be validating | |
| 17:36:54 | dansmith | sean-k-mooney: which is what? | |
| 17:37:24 | dansmith | sean-k-mooney: I added the bit to create and file and assert that it's gone after the rebuild, because initially we were supposed to be rebuild and we weren't -- the file was still present after the rebuild | |
| 17:37:27 | dansmith | not sure if that is resolved now | |
| 17:37:36 | sean-k-mooney | dansmith: the old micoversion behavior | |
| 17:37:44 | dansmith | ah for sure | |
| 17:37:49 | sean-k-mooney | also https://review.opendev.org/c/openstack/tempest/+/831018/12/tempest/api/compute/servers/test_server_actions.py#917= | |
| 17:37:59 | whoami-rajat | sean-k-mooney, ack, happy to have feedback on the test | |
| 17:38:02 | sean-k-mooney | i dont think adding a cleanup that rebuild to the old image is a good idea | |
| 17:38:15 | sean-k-mooney | it just add another failure mode in the test cleanup | |
| 17:38:16 | dansmith | yeah, not sure what that's about | |
| 17:38:35 | dansmith | comment says "not needed" | |
| 17:38:52 | whoami-rajat | dansmith, I did some changes in the nova code so the errors from logs are gone but it still somehow is not able to do it, I tested manually and the file never exists after the rebuild but somehow in this test, it stays there | |
| 17:39:13 | whoami-rajat | I'm working on a new job with two different images and will take input from there to fix it | |
| 17:39:25 | dansmith | whoami-rajat: ack, well, glad to have that assertion in there then :) | |
| 17:39:35 | dansmith | it was not rebuilding when I tried locally - the file was still present | |
| 17:40:11 | whoami-rajat | sean-k-mooney, ack, yeah that i added from the original rebuild test, can remove that part | |
| 17:40:24 | whoami-rajat | i think it was for an instance that is shared so reverted back to original image | |
| 17:40:45 | whoami-rajat | dansmith, if you try with latest code, it should work, at least it works for me and i tried 3-4 times | |
| 17:40:55 | dansmith | whoami-rajat: but not in the gate right? | |
| 17:40:59 | whoami-rajat | from nova side (in-use) and also from cinder side (available volume) | |
| 17:41:07 | whoami-rajat | yep, not in gate | |
| 17:41:12 | dansmith | okay | |
| 17:42:28 | whoami-rajat | thanks sean-k-mooney and dansmith for your feedback, it's a pity that this parameter travels down from api->conductor->compute layer and would require plenty of rework in a cycle where I've less bandwidth | |
| 17:42:37 | whoami-rajat | but i agree with the concerns and issues, so i will try to get it done | |
| 17:42:48 | dansmith | whoami-rajat: we still have to have the parameter on the rpc side | |
| 17:42:58 | dansmith | so that's not a waste :) | |
| 17:43:04 | whoami-rajat | dansmith, i don't understand | |
| 17:43:12 | whoami-rajat | i thought it's not passed at all to the API? | |
| 17:43:39 | dansmith | whoami-rajat: only the api knows whether the client requested the old or new behavior, so you still have to communicate that down to the compute worker | |
| 17:45:07 | whoami-rajat | dansmith, do you mean if microversion >=2.91 then use the ``reimage_boot_volume`` parameter for telling it to conductor and compute ? | |
| 17:45:23 | dansmith | yes | |
| 17:45:48 | whoami-rajat | oh, that reduces huge amount of work then | |
| 17:46:30 | whoami-rajat | thanks for that | |
| 17:48:26 | sean-k-mooney | whoami-rajat: it will be based on the microversion used | |
| 17:48:26 | sean-k-mooney | whoami-rajat: but the way that paramter is set is not based on a new api parmater | |
| 17:48:26 | sean-k-mooney | so the conductor/compute chagne will still be used | |
| 17:48:26 | sean-k-mooney | with the old one it will be false | |
| 17:48:26 | sean-k-mooney | with the new microverion the partmer at teh rpc level wil always be true | |
| 17:48:40 | dansmith | right | |
| 17:48:43 | whoami-rajat | sean-k-mooney[m], yes | |
| 17:49:48 | sean-k-mooney | oh i had an irc diconnect | |
| 17:50:22 | sean-k-mooney | looking at my matix client i see i did not recive a bunch fo messages | |
| 17:51:05 | sean-k-mooney | well 2 or 3 messages i guess | |
| 17:51:33 | sean-k-mooney | anyway whoami-rajat are you ok to update the spec and i can re review | |
| 17:52:10 | whoami-rajat | <whoami-rajat> sean-k-mooney[m], yes | |
| 17:52:10 | whoami-rajat | <dansmith> right | |
| 17:52:10 | whoami-rajat | * sean-k-mooney has quit (Remote host closed the connection) | |
| 17:52:10 | whoami-rajat | <sean-k-mooney> whoami-rajat: it will be based on the microversion used | |
| 17:52:10 | sean-k-mooney | im going to call it a day however so ill review tomorrow | |
| 17:52:10 | whoami-rajat | <sean-k-mooney> whoami-rajat: but the way that paramter is set is not based on a new api parmater | |
| 17:52:17 | whoami-rajat | sean-k-mooney, just for reference ^ | |
| 17:52:23 | whoami-rajat | sean-k-mooney, sure, will do that | |
| 17:52:30 | dansmith | thanks whoami-rajat ! | |
| 17:53:18 | whoami-rajat | thanks dansmith and other nova folks for this discussion, I will sign out now since it's quite late my time. have a good day :) | |