| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-12-07 | |||
| 17:06:56 | sean-k-mooney | pslestang: i tought ye upstreamed it recently into one of the opendev repos | |
| 17:07:06 | sean-k-mooney | i rembere a mail thread about it | |
| 17:07:09 | pslestang | the current behaviour is: on instance soft delete, instance actions are not soft deleted | |
| 17:07:44 | sean-k-mooney | pslestang: the db rows in the instance action table are deleted? or just not marked as deleted | |
| 17:07:45 | pslestang | when running nova db-archive, all the data ar copied into shadow table | |
| 17:08:07 | bauzas | that | |
| 17:08:21 | pslestang | sean-k-mooney: the db rows in the instance action table are not marked as deleted | |
| 17:08:26 | bauzas | OK, that's then expected behavioour and not a bug | |
| 17:08:41 | sean-k-mooney | ok and you just want them to be marked as deleted when the new config opiton is set | |
| 17:08:49 | bauzas | given the records aren't marked as soft-deleted, you can retrieve them thru the API | |
| 17:09:07 | bauzas | sean-k-mooney: yeah he wants the records to not be showable | |
| 17:09:12 | sean-k-mooney | yes you should be able too | |
| 17:09:19 | pslestang | and 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:35 | pslestang | sean-k-mooney: yes that's it | |
| 17:09:40 | bauzas | he prefers data consistency over API | |
| 17:10:02 | pslestang | bauzas: exact | |
| 17:10:10 | bauzas | well, 'he' being OVH, not pslestang I guess :) | |
| 17:10:21 | sean-k-mooney | well its not really a consitency issue | |
| 17:10:33 | sean-k-mooney | they are two differnece reseoucces | |
| 17:10:40 | sean-k-mooney | the instnace and the instnace actions | |
| 17:10:51 | bauzas | pslestang: 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:55 | sean-k-mooney | the sate of the instnace does not modify the sate of the actions | |
| 17:11:01 | bauzas | sean-k-mooney: agreed | |
| 17:11:12 | bauzas | sean-k-mooney: this is two different models but, | |
| 17:11:20 | bauzas | OVH wants their script to be simplier | |
| 17:11:24 | sean-k-mooney | bauzas: the way around that would be to extend the api to allow a --delete | |
| 17:11:29 | sean-k-mooney | like we do for instnace list | |
| 17:11:34 | sean-k-mooney | in a new microverion | |
| 17:11:51 | pslestang | bauzas: 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:53 | bauzas | sean-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 API | |
| 17:12:15 | bauzas | the scope of this BP needs to be clarified | |
| 17:12:19 | sean-k-mooney | bauzas: right so if we want to change the api it would need to be a spec | |
| 17:12:49 | gmann | yeah, I think it is good to add spec and then we can discuss all API or DB change needed | |
| 17:13:27 | pslestang | bauzas: 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 BP | |
| 17:13:29 | bauzas | actually I said I was OK with a specless BP for just the config flag, but,n | |
| 17:13:34 | sean-k-mooney | ok 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 be | |
| 17:13:38 | bauzas | there are interop concerns | |
| 17:14:05 | bauzas | if one cloud stops reporting instance actions for a deleted instance and one reporting them | |
| 17:14:06 | sean-k-mooney | there are yes it woudl be config dirven api behaivor | |
| 17:14:08 | pslestang | the second will require a spec to add a --delete flag | |
| 17:14:34 | bauzas | we somehow need to make the API public that is a behavioural change, right? | |
| 17:14:37 | sean-k-mooney | i think we would have to either do this always when a server is delete with the new microverion or never | |
| 17:14:57 | bauzas | I'm not an API interop expert | |
| 17:15:10 | sean-k-mooney | so new microverion to make instnace action soft deleted and then allow you to list with --deleted | |
| 17:15:14 | bauzas | and I don't know how we treat config-driven API behaviours | |
| 17:15:34 | gmann | yeah, I am not sure if doing it with config is good idea. means API config based behavior is not good | |
| 17:15:34 | sean-k-mooney | and if you use an old microverion for instnace action it should ignore the deleted field | |
| 17:16:06 | sean-k-mooney | gmann: i think we need a microverion too | |
| 17:16:14 | sean-k-mooney | not a config option | |
| 17:16:17 | bauzas | gmann: this is more than a microversion problem | |
| 17:16:18 | gmann | yeah | |
| 17:16:40 | gmann | we should avoid config driven API behaviors | |
| 17:16:41 | sean-k-mooney | bauzas: well no for old microverion we would jsut ignore the deleted colum in the instance action table | |
| 17:16:41 | bauzas | OVH wants their clouds to stop reporting instance actions by default | |
| 17:16:52 | sean-k-mooney | so you would get consitent behavior | |
| 17:16:59 | bauzas | sean-k-mooney: the problem is not the API query | |
| 17:17:12 | bauzas | here, I see OVH wanting to change the DB | |
| 17:17:30 | sean-k-mooney | and 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 api | |
| 17:17:45 | bauzas | sean-k-mooney: the other way around | |
| 17:18:03 | bauzas | sean-k-mooney: by default, we need to report soft-deleted records even if config changes | |
| 17:18:04 | pslestang | bauzas: we do not want to change the DB just allow the operator do soft delete instance action on instance soft delete | |
| 17:18:21 | sean-k-mooney | bauzas: no you are missundestanding me | |
| 17:18:35 | sean-k-mooney | bauzas: im suggeting not having a config opiton at all | |
| 17:18:47 | gmann | yeah agree with sean-k-mooney on not to have config option | |
| 17:18:56 | bauzas | sean-k-mooney: I understand you, I'm playing devil's advocate with OVH wishes | |
| 17:18:57 | sean-k-mooney | with old micro verions we woudl report soft-deleted instanstnce actions | |
| 17:19:03 | gmann | and do it with new microversion only | |
| 17:19:14 | sean-k-mooney | with new microversion we would filter | |
| 17:19:19 | pslestang | I'm sorry I need to go, hope to be back in 20minutes if you are stille there | |
| 17:19:20 | bauzas | sean-k-mooney: I think pslestang's request is not an API change | |
| 17:19:38 | bauzas | sean-k-mooney: he wants the DB records be marked as "deleted" | |
| 17:19:39 | sean-k-mooney | bauzas: i know but i dont think we can make there change they way they want | |
| 17:19:39 | bauzas | period. | |
| 17:19:54 | sean-k-mooney | bauzas: yep and for the interop reason i dont think we can do that | |
| 17:20:04 | sean-k-mooney | but we can do it with a microverion for server delete | |
| 17:20:09 | gmann | bauzas: yeah but doping it via config option is not good | |
| 17:20:13 | sean-k-mooney | so all new isntance will always be soft deleted | |
| 17:20:18 | bauzas | sean-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:22 | gmann | *doing | |
| 17:20:39 | bauzas | so we wouldn't change the API behaviour | |
| 17:20:54 | sean-k-mooney | bauzas: i think that is not semanticly correct behavior in the api | |
| 17:21:04 | bauzas | the DB would change and mark the actions be deleted (if operator wants) but the API would continue to return those results | |
| 17:21:21 | bauzas | sean-k-mooney: that's a good point, I dunno | |
| 17:21:28 | sean-k-mooney | bauzas: i dont think we shoudl make this an operator choice | |
| 17:21:49 | bauzas | sean-k-mooney: in this case, the answer to pslestang is "no, we can't accept your BP" | |
| 17:22:03 | sean-k-mooney | bauzas: we can accpet somehting simiilar | |
| 17:22:05 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/main/models.py#L885-L909 | |
| 17:22:12 | sean-k-mooney | also there is no db change required | |
| 17:22:15 | gmann | specless BP right but we are ok to discuss to do with API change | |
| 17:22:22 | sean-k-mooney | its already marked as soft deleable | |
| 17:22:42 | bauzas | sean-k-mooney: yup, I know | |
| 17:22:48 | bauzas | sean-k-mooney: again, the problem is interop | |
| 17:22:57 | sean-k-mooney | not whith what i porpose | |
| 17:23:11 | sean-k-mooney | if we do this as a spec with an api change there is no interop issue | |
| 17:23:48 | bauzas | sean-k-mooneyso, we would soft-delete the actions in a new microversion, OK | |
| 17:23:54 | gmann | with --delete option right and default it shows deleted instance action ? | |
| 17:24:04 | bauzas | sean-k-mooney: but what with 2.1 ? | |
| 17:24:16 | bauzas | gmann: right, that's the problem we need to solve here | |
| 17:24:17 | gmann | so no change for existing users also | |
| 17:24:18 | sean-k-mooney | in 2.1 you would see soft-deleted instance actions | |