Monday, 2022-10-24

*** han-guangyu is now known as Guest382903:30
*** Guest3829 is now known as han-guangyu03:32
*** han-guangyu is now known as Guest383003:34
*** Guest3830 is now known as han-guangyu03:35
*** Guest0 is now known as osmanlicilegi04:11
opendevreviewSahid Orentino Ferdjaoui proposed openstack/nova master: compute: enhance compute evacuate instance to support target state  https://review.opendev.org/c/openstack/nova/+/85838309:34
opendevreviewSahid Orentino Ferdjaoui proposed openstack/nova master: api: extend evacuate instance to support target state  https://review.opendev.org/c/openstack/nova/+/85838409:34
sean-k-mooney1sahid: ill try to summerise the ptg discussion on the spec review but when we discussed you feature we agreed on two high level things.09:54
sean-k-mooney11.) we wont ask you to fix all the drivers to not start the vms, we will file that as an existing bug and adress that later although your free to do that too once your feature is complete09:55
sean-k-mooney12.) we dont wat to add a new api parmater. instead we woudl prefer the new micro verion to just make evacuate always evacuate to powered off09:55
*** sean-k-mooney1 is now known as sean-k-mooney09:55
sean-k-mooneysahid: what that means for you is all the rpc changes you made are still correct and can be resused but you do not need to add a new api parmater, the new microversion means set the target state to stopped and old means dont pass it for the rpc call.09:57
sean-k-mooneyill summerise this in the spec later today 09:58
sean-k-mooneysahid: it was basicaly the last topic of the ptg https://etherpad.opendev.org/p/nova-antelope-ptg#L45109:58
sahid_sean-k-mooney: o/ I had some connectivty issues, sorry for that, could you copy/past your last messages addressed to me?09:59
fricklersahid_: unless you are referring to private messages, you could also check the channel log at https://meetings.opendev.org/irclogs/%23openstack-nova/latest.log.html10:17
sahidfrickler: ack thanks12:18
sahidsean-k-mooney: i see, so now the prefererenc for evacuate would be to always evacuate to shutoff, right?12:24
sahidbtw thank to give me the chance to not update all the virt drivers :-) Should I open the bug report or have you already assigned someone during PTG to open it?12:26
rm_workhey, hoping someone can help me understand how the fault reporting system works when retries are enabled... it seems like the "fault" field always just says "MaxRetriesExceeded" and gives that traceback, when I'd hope to be able to see the REASON for the failure (at least the latest failure)?12:45
rm_workI am not sure how that would work, but I'm sure there must be some way to make that happen (and maybe we messed it up internally) because otherwise the "fault" field seems like it'd be pretty useless?12:45
rm_workor maybe I'm expecting the wrong thing here, maybe that's right depending on the type of error?12:48
rm_workI guess it's storing each fault in the DB on the server object somehow when it happens, so it'd be the last failure recorded that gets shown in the `server show` return?12:52
sean-k-mooneysahid: we kind of felt it would be unfair to ask you do do that since its not actully required for your feature. its more just existing tech debt. sure please feel free to file one if not i can do it later. and yes the prefecne is to always evacuate to off when using the new microversion13:01
sean-k-mooneybaiscally because of a data integrety, power usage and encypted voluems we felt it would be better to alwasy evacuate as off13:02
sean-k-mooneysince evacuate is an admin only opeartion its not reasonable to assume the admin knows if its safe to restart the workload or not escially since there may be a posiablity of data curruption depending on howt eh host fails13:03
sean-k-mooneyso if we alwasy evacuate to off then the enduser who should know can decied if they want to start, rescue or delete the instace13:04
sean-k-mooneysahid: for the encypted volume case it would be niceif you could handel and ignore the excption we get during spawn but that can be a follow up patch13:05
sean-k-mooneysahid: evenutlaly we woudl like to split up rebuild/evacuate int 3 steps internally. 1 stop the instace, 2 rebuild the disk, 3 start it if requried.13:05
sahid_sean-k-mooney: if I understand, making it stop by default (the current spec), then in future have the internal clean that you are mentionning, and finally give users ability to choice active/stop, right?13:09
sean-k-mooneyno we dont want to give user a choice explictly. if they want the old behaivor they can use the old microverion but going forward we want evacuate to always mean evacuate to powerered off with the new micorversion13:11
sean-k-mooneywell its exiplcit in that its contorlled by the microversion but we dont want a new api parmater for it13:12
sean-k-mooneyso short term (A cycle) just allow evacuate to powered off13:12
sean-k-mooneymedeium term bug fix to not try and start the vm when the intended state is off as a non backporatble change to all drivers13:13
sean-k-mooneythat can be done in the A or later cycles13:14
sean-k-mooneysahid_: does that sound ok to you.13:14
sahid_sean-k-mooney: yes understood, sounds good13:26
sahid_thank you for your help on it, I will update the spec to reflect that13:26
sahid_also open an issue13:27
sean-k-mooneycool once updated ill be happy to re review13:27
sahid_++13:27
*** dasm|off is now known as dasm13:28
*** dasm is now known as dasm|rover13:28
sahid_are we agree with that I will still have to change the RPC and API to pass a targetState that will be based on the microversion, right?13:28
* sahid_ thinking about the impl13:29
sean-k-mooneyyep13:29
sean-k-mooneyso you can keep all that code that you already wrote13:30
sean-k-mooneyits jsut that instead of having a parmater in the payload to set targetState13:30
sean-k-mooneyyou will set it based on the micoversion13:30
sean-k-mooneyand with the new micorverison it will hardoced to off13:30
sean-k-mooneywith the old micorversion you dont need to pass it at all13:31
sahid_yeah i understand the whole process, perfect13:31
bauzas(just typing here again, I was in the wrong chan)14:22
bauzasman, my fingers bleed14:22
bauzasjust a bit of warning, I tried to summarize as much as I can for the PTG recap, but this will be a very long thread14:22
bauzasI'm at 80% of the writing but I need to caffeinate14:22
sean-k-mooney:)14:26
sean-k-mooneywell hopefully that means it was productive14:26
dansmithbauzas: we already have --purge for archive that does the purge immediately, right?17:26
sean-k-mooneydansmith: yep17:27
sean-k-mooneydansmith: but we would like to have --deleted for pruge17:28
sean-k-mooneyso we can eventrully remove the use of arcive and make our purge work like all the opter project17:28
sean-k-mooneythe other service dont have shadow tables so there perge remove the deleted rows17:28
dansmithso instead of archive --purge, purge --deleted ?17:28
sean-k-mooneyyep basically that17:29
sean-k-mooneythats a lower priority item but purge everywhere else is the command to remove the deleted rows17:29
dansmithmeh, but okay17:29
*** dasm|rover is now known as dasm22:13
*** dasm is now known as dasm|off22:14

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!