Tuesday, 2021-12-07

gibimelwitt: thanks for noticing that  https://review.opendev.org/c/openstack/nova/+/820381  and https://review.opendev.org/c/openstack/nova/+/820549 are related08:24
gibiI left comments in the former08:24
gibiWe need to figure out which direction is better 08:25
gibibauzas: you could also be interested in  ^^ 08:25
bauzasmorning08:30
bauzasgibi:sure, will look a bit later08:30
gibibauzas: o/08:30
opendevreviewVlad Gusev proposed openstack/nova stable/stein: only wait for plugtime events in pre-live-migration  https://review.opendev.org/c/openstack/nova/+/82068210:00
*** redrobot8 is now known as redrobot10:20
bauzasgibi: OK, so I briefly looked at both https://bugs.launchpad.net/nova/+bug/1952915 and https://bugs.launchpad.net/nova/+bug/195335910:27
bauzasI see you made a duplicate10:27
gibiI'm pretty sure the root cause of those bugs are the same10:27
gibihence the duplication10:27
gibiand we can choose from which fix we want10:28
bauzasso I guess for fixing https://bugs.launchpad.net/nova/+bug/1952915 we need to merge https://review.opendev.org/c/openstack/nova/+/820549 ?10:28
bauzasahah, I see the difference for both patches10:28
gibiI think both proposed fix fixes the same root cause10:28
gibibut in a different way10:29
gibiI tried to summ it up in https://review.opendev.org/c/openstack/nova/+/820381/4#message-122f30f371867edb0992d6260871f4cb65c047aa10:29
bauzasgibi: yeah I'd prefer your own change10:32
gibiit is a trade off really. Mine is less impact, but the other might fixes other currently unreported erros too, but maybe break something else in the process10:32
bauzasgibi: I provided my thoughts10:35
gibithanks10:36
bauzasgibi: I'll review your own change later today10:41
bauzasI just merged for the moment the functional test10:41
bauzas(with some comments)10:41
gibibauzas: OK, I try to extend the test a bit later 10:42
bauzasgibi: nah, just open thoughts10:42
gibinot due to your thoughts (havent checked them yet) but do to the fact that the other bug reports two visible issue 1) periodic fails 2) revert resize fails. My func test only covers 1) not 2)10:43
gibias I was only able to reliable reproduce 1) in my local env10:43
gibibut I saw the 2) too10:44
opendevreviewIlya Popov proposed openstack/nova master: Fix to implement 'pack' or 'spread' VM's NUMA cells  https://review.opendev.org/c/openstack/nova/+/80564910:47
opendevreviewSylvain Bauza proposed openstack/nova master: [doc] propose Review-Priority label for contribs  https://review.opendev.org/c/openstack/nova/+/81686110:50
opendevreviewMerged openstack/nova master: Reproduce bug 1953359  https://review.opendev.org/c/openstack/nova/+/82054012:10
opendevreviewBalazs Gibizer proposed openstack/nova stable/victoria: Extend the reproducer for 1953359 and 1952915  https://review.opendev.org/c/openstack/nova/+/82085612:24
gibibauzas, melwitt: ^^ extended my reproducer to cover both bugs 12:24
opendevreviewBalazs Gibizer proposed openstack/nova stable/victoria: [rt] Apply migration context for incoming migrations  https://review.opendev.org/c/openstack/nova/+/82055912:25
gibibajj12:26
gibibahh12:26
gibithe rebase went sideways12:26
opendevreviewBalazs Gibizer proposed openstack/nova stable/victoria: Extend the reproducer for 1953359 and 1952915  https://review.opendev.org/c/openstack/nova/+/82085612:29
gibiwhat am I doing /o\12:29
* gibi needs to slow down12:29
opendevreviewBalazs Gibizer proposed openstack/nova stable/victoria: [rt] Apply migration context for incoming migrations  https://review.opendev.org/c/openstack/nova/+/82055912:30
*** bhagyashris_ is now known as bhagyashris12:31
opendevreviewBalazs Gibizer proposed openstack/nova master: [rt] Apply migration context for incoming migrations  https://review.opendev.org/c/openstack/nova/+/82054912:32
opendevreviewBalazs Gibizer proposed openstack/nova master: Extend the reproducer for 1953359 and 1952915  https://review.opendev.org/c/openstack/nova/+/82085912:32
gibinow it is clean ^^ 12:34
opendevreviewBalazs Gibizer proposed openstack/nova stable/victoria: Reproduce bug 1953359  https://review.opendev.org/c/openstack/nova/+/82055812:43
opendevreviewBalazs Gibizer proposed openstack/nova stable/victoria: [rt] Apply migration context for incoming migrations  https://review.opendev.org/c/openstack/nova/+/82055912:43
opendevreviewVlad Gusev proposed openstack/nova stable/stein: only wait for plugtime events in pre-live-migration  https://review.opendev.org/c/openstack/nova/+/82068213:20
sean-k-mooneylyarwood: are we still seing issue with the qemu tb_size in the debian jobs?13:21
sean-k-mooneylyarwood: i have not had much time to work on https://review.opendev.org/c/openstack/devstack/+/817075 lately13:21
sean-k-mooneyjust wondering if that need to be work on again13:22
lyarwoodI think both Debian and CentOS stream are working around it by using additional swap 13:22
sean-k-mooneyi basically need to just uninstall apparmor  and i think it will work at that point13:22
sean-k-mooneyok i might see if i can make time to rework it this week but i wont priorties it if it snot currently urgent13:23
sean-k-mooneylyarwood: do we require the cherry-picked form lines for rdo backports? https://review.rdoproject.org/r/c/openstack/nova-distgit/+/3706913:32
lyarwoodsean-k-mooney: there's no hard requirement no13:32
sean-k-mooneyor is that just something we require in nova/openstack13:32
sean-k-mooneyok13:32
bauzasgibi: ack thanks for the FUP13:34
sean-k-mooneya simple stable backport if people are about https://review.opendev.org/c/openstack/nova/+/820682 this has already been merged all the way to train and this is just taking it to stein13:39
ricolinstephenfin, FYI, Pain point discussion meeting have scheduled on 1500UTC this Wednesday(Dec. 8th) in https://meet.google.com/xsx-inbw-mos13:45
opendevreviewPierre Libeau proposed openstack/nova master: Nova resize don't extend disk in one specific case  https://review.opendev.org/c/openstack/nova/+/82053114:03
bauzasgibi: I'll have to go off at 4:20pm and should be back hopefully at 5pm our time, in time for the meeting15:00
bauzasgibi: but in case I'm a bit late, could you start ?15:00
gibibauzas: sure15:02
bauzasthanks15:02
bauzasshouldn't arrive after 5:05pm15:03
stephenfinricolin: ack, thanks15:09
opendevreviewBelmiro Moreira proposed openstack/nova master: Over quota instance resize can give confusing error message  https://review.opendev.org/c/openstack/nova/+/82089815:14
bauzasreminder : nova meeting in 9 mins here at #openstack-nova15:51
bauzasgibi: I'm finally back ;)15:51
gibibauzas: you were fast :)15:51
bauzasmy daughter's teacher forgot about the appointment :p15:52
gibiups15:53
bauzaswell, she said that 41 was a prime number when I told her my age at my birthday :)15:58
bauzasso, she's fine. :p15:59
bauzasanyway, nova meeting starts in secs15:59
bauzas#startmeeting nova16:00
opendevmeetMeeting started Tue Dec  7 16:00:01 2021 UTC and is due to finish in 60 minutes.  The chair is bauzas. Information about MeetBot at http://wiki.debian.org/MeetBot.16:00
opendevmeetUseful Commands: #action #agreed #help #info #idea #link #topic #startvote.16:00
opendevmeetThe meeting name has been set to 'nova'16:00
gibio/16:00
bauzas#link https://wiki.openstack.org/wiki/Meetings/Nova#Agenda_for_next_meeting16:00
elodilleso/16:00
bauzashola everybody16:00
gmanno/16:01
pslestanghey16:01
bauzasok, looks like we can start16:02
bauzas#topic Bugs (stuck/critical) 16:02
bauzas#info No Critical bug16:02
bauzas#link https://bugs.launchpad.net/nova/+bugs?search=Search&field.status=New 24 new untriaged bugs (+1 since the last meeting)16:02
bauzas#help Nova bug triage help is appreciated https://wiki.openstack.org/wiki/Nova/BugTriage16:02
kashyapDo we have a lot of bugs coming in for triage?16:02
bauzasthe +1 is a new gate bug, but let's discuss it after16:03
kashyapOkay, 24...16:03
bauzaskashyap: we had 3 new bugs this week AFAIK16:03
bauzassometimes we have 1016:03
bauzas#link https://storyboard.openstack.org/#!/project/openstack/placement 25 open stories (+0 since the last meeting) in Storyboard for Placement 16:03
bauzasanything to discuss about bugs but not the gate ones ?16:04
bauzaslooks not16:05
bauzas#topic Gate status 16:05
bauzas#link https://bugs.launchpad.net/nova/+bugs?field.tag=gate-failure Nova gate bugs 16:05
bauzasso we now have https://bugs.launchpad.net/nova/+bug/195347816:05
bauzasI didn't had time to look at the CI 16:06
bauzashave someone looked at it ? ^16:06
bauzaslooks not16:07
bauzasokay, I'll try to see whether logstash continues to work16:07
bauzasand I'll tell it's a High bug16:07
bauzasbut ok, moving on16:08
bauzas#link https://zuul.openstack.org/builds?project=openstack%2Fplacement&pipeline=periodic-weekly Placement periodic job status 16:08
bauzasthis is fun, I got 503 ^16:09
kashyapbauzas: Sorry, just reading back - the CI issue seems intermittent16:09
kashyap(And I doubt there's much bandwidth for folks to debug intermittent issues :-()16:09
bauzasyeah no worries16:10
bauzasanyway, about placement jobs...16:10
bauzaslooks like we have a problem with the Zuul API16:10
gibiyeah zuul dashboard seems to be down16:10
bauzaswill ping the infra folks after the meeting16:10
bauzas#action bauzas to ask infra folks why Zuul dashboard returns HTTP503 16:11
bauzasmoving on then16:11
bauzas#info Please look at the gate failures, file a bug, and add an  elastic-recheck signature in the opendev/elastic-recheck repo (example: https://review.opendev.org/#/c/759967)16:12
bauzas#topic Release Planning 16:12
bauzas#info Yoga-2 is due Jan 6th16:12
bauzas#link https://releases.openstack.org/yoga/schedule.html#y-216:12
bauzas#info Next spec review day to happen next week on Dec 14th16:12
bauzassooooo16:12
bauzasspec, spec, spec, folks16:12
bauzasremember that we will have less reviews after Dec 20 16:13
bauzasas some folks are on holidays or off16:13
bauzasso, prepare your specs in advance16:13
bauzas#info Reminder that Spec Freeze deadline is at Yoga-216:14
bauzasanything to discuss about it ?16:14
bauzasheh, no16:15
kashyapbauzas: A quick one16:15
bauzas#topic Review priorities 16:15
bauzas#undo16:15
opendevmeetRemoving item from minutes: #topic Review priorities 16:15
bauzaskashyap: sure ?16:15
kashyapbauzas: Can the spec review days be moved somewhere in the 2nd week of Jan?16:15
kashyapMany will be on PTO from late next week...16:15
bauzaskashyap: that's why we have the spec review day on Dec 14th16:16
sean-k-mooneykashyap: yep most should be around on the 14th16:16
bauzasso people could provide new revisions after this day and cores could merge them after16:16
kashyapbauzas: Nod.  Okay, you can move on to reviw prio16:16
kashyapsean-k-mooney: I'm off from 14Dec myself16:16
bauzasI mean, before Dec 2016:16
bauzasbut, that said, I'm not against moving the spec freeze deadline after Jan 6th if people really want but,16:17
bauzaskeep in mind we already have a good number of accepted bps that we need to review 16:17
sean-k-mooneylets see if there is a need after m216:17
gibiyeah, we can get back to this on 11th of Jan meeting16:18
sean-k-mooneyya i think we likely will have enough on our plate but we can assess on the 11th16:18
bauzasfor the moment, those are the BPs we have for Yoga https://blueprints.launchpad.net/nova/yoga16:18
bauzasbut I need to add new BPs that specs were merged during this week16:18
sean-k-mooneyyep i also likly need to file 1-2 blueprints for heathchecks for one16:19
sean-k-mooneyill try and do that soon16:19
bauzasactually https://review.opendev.org/q/project:openstack/nova-specs+is:merged tells me that's it for the moment16:19
bauzasso Launchpad is OK16:19
sean-k-mooneyack16:19
bauzasbut ok, let's revisite this on Jan 4th and Jan 11th to see whether folks want some exceptions16:20
bauzasthis said, I have a point to discuss in this meeting about holidays, btw.16:20
bauzas(in the open discussion section)16:21
bauzasdo people agree with it ? we will at least see what's going on with specs after the review day16:21
gmann+116:22
sean-k-mooneyyep +116:22
*** artom__ is now known as artom16:22
bauzasok, moving on16:22
bauzas#topic Review priorities 16:23
bauzas#link https://review.opendev.org/q/status:open+(project:openstack/nova+OR+project:openstack/placement)+label:Review-Priority%252B116:23
bauzas#link https://review.opendev.org/c/openstack/nova/+/816861 bauzas proposing a documentation change for helping contributors to ask for reviews16:23
bauzasI'd appreciate a second core for https://review.opendev.org/c/openstack/nova/+/81537316:24
bauzasalso,16:24
gibibauzas: I will check that16:25
bauzasno update coming from https://review.opendev.org/c/openstack/nova/+/790519 since more than one month, can we consider to remove the prio ?16:25
* bauzas looks at some people he knows ;)16:25
gibiyepp drop it from prio16:25
gibiwe can add it back once the dependecies are sorted out16:25
bauzasyeah this is related to the centos9 devstack effort, right?16:26
gibiim not sure16:26
bauzasfrom adalee's last comment, looks like it16:27
bauzaslyarwood: around ?16:27
gibiahh I see, yes16:27
lyarwoodyup /me reads16:27
bauzaslyarwood: you could have more context than me on this FIPS job 16:27
lyarwoodI've not touched the review in a while16:28
bauzaslyarwood: well, looks like there are some deps issues16:28
lyarwoodkk well it's on Ade to push it forward tbh16:28
bauzasagreed16:30
bauzasso, I'll drop the review-prio flag16:30
bauzasbut I want to make sure there is progress on it16:30
bauzashopefully my gerrit update will remind Ade there is work to do16:30
gmannthere are few tempest one (in the series of those deps) also needs to merged before16:31
gmannwhich is under review16:31
bauzasyeah looks like there are deps16:32
bauzasquite a massive work16:32
gmannand this is goal proposal in case to know broader pic #link https://review.opendev.org/c/openstack/governance/+/81658716:32
bauzashaha16:32
bauzasgtk16:32
bauzasthanks gmann16:33
bauzasanyway, I think we discuss about all the subject, next one16:34
bauzasdiscussed* (dang)16:34
bauzasoh, my change16:34
bauzashttps://review.opendev.org/c/openstack/nova/+/816861 is ready for reviews16:34
bauzasI dropped the whole "I'm the owner, help me" thing16:34
bauzasso, this is now only about "I'm non-core and I like this change, so I'd appreciate if cores could review it given I commit myself to review it too"16:35
bauzasgood opportunity for review visibility, just sayin'16:35
sean-k-mooneyya i think that is a good chagne16:35
gibibauzas: I will get back to that too16:36
bauzasthanks16:37
sean-k-mooneyif we are more or less happy with it i can start drafting the project config change for it but ill waith until after the spec review day next week to work on it16:37
bauzasthe idea is to give visibility on review time for non-cores and giving them trust16:37
gmannI think this is good. and if every non core adding RP as +1 on their patches then we can re-iterate it16:37
bauzasso, if you're non-core and you'd appreciate some visibility on review time, this is for you16:38
sean-k-mooneyi think this is more or less like +1 ing your own patch and we can treat it as such16:38
gmannyeah, 'yes i reviewed it and it looks important to me so can core have a look'16:38
sean-k-mooneye.g. you should in general avoid it but you may still want to do it in some cases16:39
bauzassean-k-mooney: I punted this use case16:39
gmannmay be good to mention in that doc for more clarity  16:39
sean-k-mooneybauzas: ya that is ok16:39
bauzascomments are then appreciated on the change itself :)16:39
bauzasnext one16:39
bauzas#topic Stable Branches 16:39
bauzaselodilles: your dog.16:40
elodilleswell, this will be short as not so much happening around stable branches16:40
elodilles* stable gates should be OK (though no patches have been merged in the last couple of days)16:40
elodillesthat's it ^^^16:40
bauzashuzzah16:41
bauzasI remember I promised some reviews16:41
bauzasbut lacked my homework16:41
bauzasok, moving on then16:42
bauzas#topic Sub/related team Highlights 16:42
bauzasLibvirt (lyarwood)16:42
bauzasnothing special to mention ?16:42
lyarwoodNothing from me this week, FWIW we will need a new owner for this point in the future.16:42
lyarwoodI'll talk about that more later16:42
bauzasyou're right16:43
bauzasor,16:43
bauzasnot sure we still need to have a subteam16:43
bauzasgiven afaik only lee and me work on this :)16:43
lyarwoodEither or, it's up to the remaining team16:43
bauzaswho's teamed up ?16:43
bauzasanyway, we could make a call at next meeting16:44
lyarwoodI mean the wider Nova team ;)16:44
bauzasahah16:45
bauzasanyway, last topic16:46
bauzas#topic Open discussion 16:46
bauzas(bauzas) Holiday period and nova audience for both reviews and meetings16:46
bauzasso, we'll have less people around in the last weeks of Dec16:47
sean-k-mooneyi expect other then perhaps alex there will likely be no? cores around for the last 2 weeks16:48
bauzasI dunnoi16:48
gibiI will be off from 20th16:48
bauzaswe can discuss if people want16:48
gibiand back at 10th16:48
sean-k-mooney17th -> 4th in my case.16:49
bauzasok, Dec 21 -> Jan 3rd for me16:49
sean-k-mooneyi dont think we really need to spend much time on it 16:49
gmannI will be on and off during last week dec16:49
lyarwood17th -> 4th here also16:49
sean-k-mooneyi assume we are going to cancel meetings for the last 2 weeks16:49
sean-k-mooneythe 21th and 28th16:50
bauzassean-k-mooney: yeah I was about to propose to cancel the meetings16:50
gibiagree with cancelling those16:50
elodilles(20th -> 4th)16:50
gmann+1 on canceling 16:50
elodilles++16:51
sean-k-mooneyunless there is an object i would suggest just sending a mail to the list to inform those that are not here16:51
gmann21 and 28 one right?16:51
bauzascorrect16:51
gmannk16:51
bauzasok, looks like we have an agreement16:52
bauzas#agreed Cancel Dec 21th and Dec 28th nova meetings given there are less audience16:53
bauzasI'll write an email about it16:53
bauzasand thanks for providing your time-offs, appreciated.16:53
bauzaswe can look at this meeting' notes to remember when people are around16:54
bauzaslast item on our agenda,16:54
bauzas(pslestang) Specless BP approval request for soft deleting instance action  16:54
bauzaspslestang: your turn16:54
pslestangsure16:55
pslestangso at OVH we do not use the restore instance feature and we would like the instance action do be soft deleted when an instance is soft deleted, this is why we propose to add a configuration option to soft delete instance action on instance soft delete16:56
bauzasthis one, I'm OK with a specless BP16:56
bauzasbut,16:56
bauzas"We should also implement the possibility to retrieve deleted instance actions as we do for instances." seems like an API change, right?16:56
sean-k-mooneypslestang: so just to be clear the meaning of soft delete upstream is the row is kept in the db but marked as delete16:57
sean-k-mooneyso that it can be mared as not deleted again later16:57
pslestangbauzas: I did not dive into the API so need to be checked16:57
pslestangsean-k-mooney: yes16:57
bauzasare we sure that instance-actions table is soft-deletable already ?16:57
sean-k-mooneyi tought it was16:58
bauzasI dunno, hence my question16:58
pslestangbauzas: yes it is16:58
bauzasif yes, looks like a config option setting16:58
sean-k-mooneyso that if we resotred a soft deleted instance its action woudl be preserved16:58
bauzasif no, this is a db change hence a spec required16:58
sean-k-mooneypslestang: so today we shoudl be soft deleting them already16:59
pslestangthe instance_actions table is soft-deletable16:59
sean-k-mooneyand the new config option you would instead hard delete them16:59
bauzassean-k-mooney: yeah I'm assuming we would mark the records soft-deleted in the DB and we would also unmark them when undeleting 16:59
bauzasactually, we're at time16:59
bauzasbut we can continue off the meeting16:59
bauzas#endmeeting17:00
opendevmeetMeeting ended Tue Dec  7 17:00:09 2021 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)17:00
opendevmeetMinutes:        https://meetings.opendev.org/meetings/nova/2021/nova.2021-12-07-16.00.html17:00
opendevmeetMinutes (text): https://meetings.opendev.org/meetings/nova/2021/nova.2021-12-07-16.00.txt17:00
opendevmeetLog:            https://meetings.opendev.org/meetings/nova/2021/nova.2021-12-07-16.00.log.html17:00
gibiI think there is a reason to keep the actions visible after the instance is deleted17:00
lyarwoodfun, I did have another topic17:00
lyarwoodA quick heads for upstream folks who might not be aware, I'll be stepping away from OpenStack in the new year from ~Jan 28th, moving to a different project within Red Hat. My focus until then is to close out the various open specs, bugfixes etc that I currently have. 17:00
pslestangsean-k-mooney: today when we soft-delete an instance instance actions are not soft-deleted (I suppose to let the operator have an history on a deleted instane)17:00
gibilyarwood: I will miss you!17:00
gibilyarwood: but good luck!17:01
elodilleslyarwood: :-o17:01
lyarwoodgibi: thanks17:01
gibiand let me know if help needed to close up things17:01
sean-k-mooneypslestang: ah ok then i think that might be a bug then17:01
bauzaslyarwood: sorry, I stopped the meeting on time given i didn't see any other item17:01
sean-k-mooneypslestang: we should keep the records in the db for auit use until they are archived17:01
lyarwoodoh I'll be chasing for reviews don't worry about that gibi :)17:01
* gibi stopped worrying :D17:01
lyarwoodbauzas: yeah np I didn't post it in the agenda 17:01
elodilleslyarwood: same as gibi says: will miss you (especially at stable team!) :(17:01
gmannlyarwood: ohh. it is great help from you in many areas not just nova where we all will miss your contribution.  17:02
pslestangsean-k-mooney: agree with that17:02
elodilleslyarwood: good luck with the new position!17:02
bauzaspslestang: sean-k-mooney: I think instance actions be kept when soft-delete isn't a bug but rather made on purpose17:02
lyarwoodgmann / elodilles ; thanks both17:02
sean-k-mooneybauzas: pslestang i belive is reproting that currently they are not kept17:02
gibipslestang, sean-k-mooney: I need the instance actions to remind me that I deleted my instance yesterday hence not finding it17:02
bauzasbut I'm not opposed to let operators define this behaviour as opt-out17:02
bauzaspslestang: can you then clarify ?17:03
sean-k-mooneygibi: right so my expection is that the instacne action woudl remain until we do archive-delete-rows or whatever that is called17:03
gmannwhat is purpose of disabling it via config instead just ignore that action record if not needed ?17:03
sean-k-mooneyso that we could retrive them for deleted instance to see who deleted them17:03
bauzasgmann: good question, what's the use case ?17:04
gibiI need to drop, but I will read back tomorrow17:04
bauzasnot letting the instance action appearing at the API level ?17:04
bauzasbut given you need an instance UUID, I don't get the problem17:04
sean-k-mooneybauzas: you can list delete instanes17:04
sean-k-mooneyto get the uuid17:04
bauzassure, and then you can get the instance actions of that deleted UUID17:05
bauzaswhich is expected behaviour17:05
sean-k-mooneybut pslestang  can you confrim when you delete an  instance today are the isntance actions deleted from the db17:05
bauzasin order to know why this disappeared17:05
gmannyeah17:05
pslestangthe use case is to have the table uniformly soft-deleted because we use a custom tool to archive the data17:05
sean-k-mooneybauzas: i think pslestang  is suggeting that is not what happens today17:06
sean-k-mooneypslestang: yes i think i have seen your tool17:06
sean-k-mooneyits now in the ops tools repo right17:06
bauzasagain, what's the current behaviour ?17:06
pslestangsean-k-mooney: it should be I guess17:06
sean-k-mooneypslestang: i tought ye upstreamed it recently into one of the opendev repos17:06
sean-k-mooneyi rembere a mail thread about it17:07
pslestangthe current behaviour is: on instance soft delete, instance actions are not soft deleted17:07
sean-k-mooneypslestang: the db rows in the instance action table are deleted? or just not marked as deleted17:07
pslestangwhen running nova db-archive, all the data ar copied into shadow table17:07
bauzasthat17:08
pslestangsean-k-mooney:  the db rows in the instance action table are not marked as deleted17:08
bauzasOK, that's then expected behavioour and not a bug17:08
sean-k-mooneyok and you just want them to be marked as deleted when the new config opiton is set17:08
bauzasgiven the records aren't marked as soft-deleted, you can retrieve them thru the API17:08
bauzassean-k-mooney: yeah he wants the records to not be showable 17:09
sean-k-mooneyyes you should be able too17:09
pslestangand on nova db purge the data are remove from sahdow table and there is a if in the code for instance action to rely on updated_at instead of deleted_at 17:09
pslestangsean-k-mooney: yes that's it17:09
bauzashe prefers data consistency over API 17:09
pslestangbauzas: exact17:10
bauzaswell, 'he' being OVH, not pslestang I guess :)17:10
sean-k-mooneywell its not really a consitency issue17:10
sean-k-mooneythey are two differnece reseoucces17:10
sean-k-mooneythe instnace and the instnace actions17:10
bauzaspslestang: do you understand that you won't be able to investigate an instance deletion thru the API if you mark the action records as soft-deleted ?17:10
sean-k-mooneythe sate of the instnace does not modify the sate of the actions17:10
bauzassean-k-mooney: agreed17:11
bauzassean-k-mooney: this is two different models but, 17:11
bauzasOVH wants their script to be simplier17:11
sean-k-mooneybauzas: the way around that would be to extend the api to allow a --delete17:11
sean-k-mooneylike we do for instnace list17:11
sean-k-mooneyin a new microverion17:11
pslestangbauzas: yes we know but this is also the point you mention earlier to be able to retrieve instance action for a deleted instance 17:11
bauzassean-k-mooney: that's exactly why I said in the meeting that I'm opposed to this be a specless BP if we touch the API17:11
bauzasthe scope of this BP needs to be clarified17:12
sean-k-mooneybauzas: right so if we want to change the api it would need to be a spec17:12
gmannyeah, I think it is good to add spec and then we can discuss all API or DB change needed17:12
pslestangbauzas: ok, can we proceed in 2 steps? First one would be to add a config option to soft delete instance action if I understand well we could do it as a specless BP17:13
bauzasactually I said I was OK with a specless BP for just the config flag, but,n17:13
sean-k-mooneyok so i dont think we are against making this change at a high level but just want a spec to explain exactly what the behavior shoudl be17:13
bauzasthere are interop concerns17:13
bauzasif one cloud stops reporting instance actions for a deleted instance and one reporting them17:14
sean-k-mooneythere are yes it woudl be config dirven api behaivor17:14
pslestangthe second will require a spec to add a --delete flag17:14
bauzaswe somehow need to make the API public that is a behavioural change, right?17:14
sean-k-mooneyi think we would have to either do this always when a server is delete with the new microverion or never17:14
bauzasI'm not an API interop expert 17:14
sean-k-mooneyso new microverion to make instnace action soft deleted and then allow you to list with --deleted17:15
bauzasand I don't know how we treat config-driven API behaviours17:15
gmannyeah, I am not sure if doing it with config is good idea. means API config based behavior is not good 17:15
sean-k-mooneyand if you use an old microverion for instnace action it should ignore the deleted field17:15
sean-k-mooneygmann: i think we need a microverion too17:16
sean-k-mooneynot a config option17:16
bauzasgmann: this is more than a microversion problem17:16
gmannyeah17:16
gmannwe should avoid config driven API behaviors 17:16
sean-k-mooneybauzas: well no for old microverion we would jsut ignore the deleted colum in the instance action table17:16
bauzasOVH wants their clouds to stop reporting instance actions by default17:16
sean-k-mooneyso you would get consitent behavior17:16
bauzassean-k-mooney: the problem is not the API query17:16
bauzashere, I see OVH wanting to change the DB 17:17
sean-k-mooneyand with the new microversion for server delete and instance action show we will take it into account and intoduce the --deleted option to the isntance_action api17:17
bauzassean-k-mooney: the other way around17:17
bauzassean-k-mooney: by default, we need to report soft-deleted records even if config changes 17:18
pslestangbauzas: we do not want to change the DB just allow the operator do soft delete instance action on instance soft delete17:18
sean-k-mooneybauzas: no you are missundestanding me17:18
sean-k-mooneybauzas: im suggeting not having a config opiton at all17:18
gmannyeah agree with sean-k-mooney on not to have config option17:18
bauzassean-k-mooney: I understand you, I'm playing devil's advocate with OVH wishes17:18
sean-k-mooneywith old micro verions we woudl report soft-deleted instanstnce actions17:18
gmannand do it with new microversion only17:19
sean-k-mooneywith new microversion we would filter17:19
pslestangI'm sorry I need to go, hope to be back in 20minutes if you are stille there17:19
bauzassean-k-mooney: I think pslestang's request is not an API change17:19
bauzassean-k-mooney: he wants the DB records be marked as "deleted"17:19
sean-k-mooneybauzas: i know but i dont think we can make there change they way they want17:19
bauzasperiod.17:19
sean-k-mooneybauzas: yep and for the interop reason i dont think we can do that17:19
sean-k-mooneybut we can do it with a microverion for server delete17:20
gmannbauzas: yeah but doping it via config option is not good 17:20
sean-k-mooneyso all new isntance will always be soft deleted17:20
bauzassean-k-mooney: that's why I'm saying the only way to achieve this is to return the soft-deleted instance actions *either way*17:20
gmann*doing17:20
bauzasso we wouldn't change the API behaviour17:20
sean-k-mooneybauzas: i think that is not semanticly correct behavior in the api17:20
bauzasthe DB would change and mark the actions be deleted (if operator wants) but the API would continue to return those results17:21
bauzassean-k-mooney: that's a good point, I dunno17:21
sean-k-mooneybauzas: i dont think we shoudl make this an operator choice17:21
bauzassean-k-mooney: in this case, the answer to pslestang is "no, we can't accept your BP"17:21
sean-k-mooneybauzas: we can accpet somehting simiilar17:22
sean-k-mooneyhttps://github.com/openstack/nova/blob/master/nova/db/main/models.py#L885-L90917:22
sean-k-mooneyalso there is no db change required17:22
gmannspecless BP right but we are ok to discuss to do with API change17:22
sean-k-mooneyits already marked as soft deleable17:22
bauzassean-k-mooney: yup, I know17:22
bauzassean-k-mooney: again, the problem is interop17:22
sean-k-mooneynot whith what i porpose17:22
sean-k-mooneyif we do this as a spec with an api change there is no interop issue17:23
bauzassean-k-mooneyso, we would soft-delete the actions in a new microversion, OK17:23
gmannwith --delete option right and default it shows deleted instance action ?17:23
bauzassean-k-mooney: but what with 2.1 ?17:24
bauzasgmann: right, that's the problem we need to solve here17:24
gmannso no change for existing users also17:24
sean-k-mooneyin 2.1 you would see soft-deleted instance actions17:24
sean-k-mooneyin 2.100 or whatever you would not17:24
sean-k-mooneyunless you pass --deleted17:24
gmannyes17:24
bauzassean-k-mooney: what's the difference with what I proposed ?17:24
bauzasyou would also have semantically incorrect values for 2.117:25
sean-k-mooneyin 2.100 you would only see non soft deleted ones by default17:25
sean-k-mooneyno you would not17:25
sean-k-mooneyin 2.1 we are reflecting the fact that instnace action have a sperate lifetime form the instnace they refer too17:25
sean-k-mooneywhich were the sematics in 2.117:25
gmannwith 2.100 with deleted it shows all otherwise only non-soft deleted one17:25
sean-k-mooneyin 2.100 we are saying they share the same lifetime17:26
sean-k-mooneyin 2.100 soft deleting the instance will soft delete the actions17:26
sean-k-mooneythat is a semantic change in the behavior of the api hence need for new microverion17:27
bauzasI can understand this 17:27
sean-k-mooneydoes that make sense17:27
bauzasno config but a change for all17:27
sean-k-mooneyyes17:27
bauzasjust to be sure, default of 2.100 is 'delete' be always True, right?17:27
sean-k-mooneyno17:27
bauzasthen, we need to discuss this with ops17:28
sean-k-mooneydefault fo server delete will be to soft delete the instance actions17:28
sean-k-mooneydefault for instance_action show would be deleted=false17:28
gmannand we cannot fix it as bug as it is changing API behaviour and so does effect existing users 17:28
bauzasI'm pretty sure I can find a handful of operators who wouldn't want to opt-in for v2.10017:28
bauzasthe problem is that if operators want v2.101, they're stuck with shipping 2.100 too17:29
bauzaswhich they dislike17:29
sean-k-mooneyfor which endpoint17:29
bauzasthe os-instance-actions one, of course17:29
sean-k-mooneyif we have a 2.101 for that endpoint17:30
sean-k-mooneythe client already needs to be updated to use it17:30
sean-k-mooneyso they can just pass deleted=false17:30
bauzashonestly, the more I think, the less I consider it as something good for Nova if we make this general for ops17:30
sean-k-mooney*deleted=true17:30
gmannwell that is drawback but applicable for all the changes via microversion17:30
bauzaswhile on the other hand, the archive problem is solvable with a smarter script17:30
sean-k-mooneythe fact we are adding a query arg in 2.100 means they can choose what behvior they want in 2.10117:31
bauzasthat's one thing to let OVH do what they want with their own APIs17:31
gmannsean-k-mooney: I think we can return all deleted one like it is today so that other existing users are not effected and allow --deleted=false to hide deleted actions17:31
bauzasthat's another thing to consider that ops don't care about getting information why their instance is deleted17:31
sean-k-mooneygmann: we could do that yes17:31
sean-k-mooneybut i think it makes sense to change the default we coudl defer that to a later microverion17:32
bauzassean-k-mooney: this was my proposal... default be "show-deleted" 17:32
sean-k-mooneybauzas: right17:32
bauzaseither way, this is a spec17:32
sean-k-mooneybut if we are making this change i was suggesting changing the bhavior now instead of having 2 microverions for it17:32
sean-k-mooneyif its less contoversion to not change the defualt behavior im not agaisnt that17:33
bauzashonestly, as I said, I'm not super happy with changing the default behaviour17:33
sean-k-mooneyprovided we never change the default behavior or atleast not in the near future17:33
bauzas(from an ops perspective)17:33
sean-k-mooneybauzas: ok then lets not17:33
gmannyeah. let's do spec. keeping existing behavior as default will be useful 17:33
sean-k-mooneyok17:33
bauzasgmann: sean-k-mooney: the fun fact is that this whole conversation starts from a case from OVH coming because they don't wanna change their archive script to be model-specific17:34
sean-k-mooneythe thing i want to avoid is two microverions to 1 allow filtering and 2 change the default close toghter17:34
sean-k-mooneyif  we do the first part and oepraters give feedback we can reconsider the second17:34
bauzasgmann: sean-k-mooney: if we were changing the default, this would mean all ops but OVH would have to modify their own clients to use the new param17:35
bauzaslooks unfair, no ?17:35
sean-k-mooneybauzas: actully they do not use our archive feature at all17:35
sean-k-mooneybauzas: no only if they use the latest microverion17:35
sean-k-mooneyif they are using microverions correctly that is not going to affect them17:35
gmannyeah and nobody complained on current default behavior so good to keep it17:35
sean-k-mooneyits why osc orginally didn not defualt to latest microverion17:35
sean-k-mooneyso that the cli would be stable17:36
bauzasgmann: even OVH hasn't complained about the current behaviour, they just complain about the fact their tooling doesn't work with our DB17:36
sean-k-mooneyanyway i think we are agred. needs a spec and keep current behviaor by default17:36
gmannbauzas: yeah.17:36
gmannsean-k-mooney: +117:36
bauzassean-k-mooney: even with a spec, I think this is taking a hammer for chasing a firefly17:37
sean-k-mooneybauzas: well as i said ovh do not use our showdown tables anda archiving mechanium17:37
bauzassean-k-mooney: that's the whole point, they don't use what we provide17:37
bauzaswhy should we make modifications for them so ?17:37
sean-k-mooneyright because not all service implement it 17:37
bauzasif it's all about DB persistency17:37
sean-k-mooneythere tool work for neutorn glance cinder consitently17:37
sean-k-mooneybauzas: i think ovh and i woudl both be happy if we fully remove the shadow tables17:38
bauzasI still think we're ending into a weird state17:39
bauzaswe said in the past "this is expected behaviour, as actions table is here for recording user actions"17:39
bauzasnow we're about to publicly express that the actions API is just usable by default for non-deleted instances17:40
bauzas(if we change the default)17:40
sean-k-mooneybauzas: this is what they use https://github.com/ovh/osarchiver/17:40
bauzasif we don't change the default, we just provide a new microversion that won't change the behaviour by default, which is also weird17:41
sean-k-mooneywe often add microverion that dont change default behavior17:41
bauzasand why ? because we consider the current behaviour is enough good to not touch it17:41
bauzassean-k-mooney: we used microversions for "signaling", I know17:41
sean-k-mooneyno17:41
sean-k-mooneythe feature this would be interocudeing is the ablity to filter on the soft delete status17:42
sean-k-mooneythat does not require use to change the defualt behavior17:42
sean-k-mooneyso its not jsut signaling17:42
gmannyeah, adding new filtering which is asked in current specless BP 17:44
gmanncurrently there is no way to filter soft deleted instance actions 17:44
gmannwith new microversion, we solve both use case 1. existing keep working 2. way to filter the soft deleted one17:45
sean-k-mooneyso i think we agree that the current behaiovr is not broken or incorrect, that we could start marking the records as soft deleted when the instance is soft delete but only if we maintain the currnt behavior of returning both soft and not soft deleted records and we woudl also want to allow filtering if we made this change17:45
sean-k-mooneythe filtering is an api change as we are adding a new query arg even if the default behvior does not change.17:47
pslestangsean-k-mooney: +1 for your proposition17:48
sean-k-mooneypslestang: by the way what happend with your upstreaming efforts http://lists.openstack.org/pipermail/openstack-discuss/2021-February/020383.html17:49
sean-k-mooneythat was the last mail i could find on that trhead was there a tc desicion made17:49
sean-k-mooneyi dont see it in https://opendev.org/openstack/osops/src/branch/master so i guess it was not added to osops17:50
sean-k-mooneyand i dont see a new repo for https://github.com/ovh/osarchiver/17:51
sean-k-mooneydid ye decide to just keep it in github in the end17:51
sean-k-mooneypslestang: i think the prefernce was option 3 "Move it under its own repository under opendev and propose it as a new official OpenStack project" but i dont think that happened17:53
sean-k-mooneyif that was to happen i coudl see a day where nova could discontinue our current shadown tables eventually and rely on OSArchiver after a deprecation period17:54
pslestangsean-k-mooney:this is what I was looking for, exact option 3 prefered but nothing done17:54
pslestangWe prefer to put it under opendev than keeping it in github, we really thing that other operators could take benefit of this tool17:57
sean-k-mooneyyep and honestly i think its a better approch then we have today in nova17:57
sean-k-mooneyso if it was mature enouch and operators started to use it i think we coudl eventually depercante and remove nova archiveal supprot17:58
sean-k-mooneyand replace it with your external soltuion once we had time to update the installer tools and work with the differnt sake holders to adopt it17:58
sean-k-mooneythe fact it can archive the data to an entirly differnt db or a csv file can signifcalty help with db load on the main db17:59
sean-k-mooneyand help keep the main db small17:59
pslestangWe use it on a weekly basis (crontab) to clean our DB, and we do not have any issue (except with instance_actions_* ;-)) 17:59
sean-k-mooneyoh i know it works well for you but for use to ever remove the native support in nova we woudl need all the installaiton tools or at least most of them to supprot your alternivie and we would need an upgrade stragy for exsing clouds18:00
sean-k-mooneyso its more maturing that process that would need work18:01
sean-k-mooneycern were intersted in this too if i rememebr the mail thread18:01
pslestangsure I understand18:01
pslestangyes and I know also that ubisoft is using it18:03
bauzassean-k-mooney: distros would have to agree on it18:06
bauzasthis is shipping yet another dep18:06
sean-k-mooneybauzas: it would be yes although we have talked about removing shadow tables in the past and not providing a replacement too18:11
sean-k-mooneythis would just be a way to provide a replacement18:11
opendevreviewmitya-eremeev-2 proposed openstack/nova master: Delete bogus attachments.  https://review.opendev.org/c/openstack/nova/+/82093519:08
opendevreviewMerged openstack/nova stable/wallaby: Add functional regression test for bug 1853009  https://review.opendev.org/c/openstack/nova/+/81180522:31

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