| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-12-07 | |||
| 17:04:15 | bauzas | not letting the instance action appearing at the API level ? | |
| 17:04:30 | bauzas | but given you need an instance UUID, I don't get the problem | |
| 17:04:42 | sean-k-mooney | bauzas: you can list delete instanes | |
| 17:04:46 | sean-k-mooney | to get the uuid | |
| 17:05:04 | bauzas | sure, and then you can get the instance actions of that deleted UUID | |
| 17:05:09 | bauzas | which is expected behaviour | |
| 17:05:13 | sean-k-mooney | but pslestang can you confrim when you delete an instance today are the isntance actions deleted from the db | |
| 17:05:20 | bauzas | in order to know why this disappeared | |
| 17:05:34 | gmann | yeah | |
| 17:05:40 | pslestang | the use case is to have the table uniformly soft-deleted because we use a custom tool to archive the data | |
| 17:06:12 | sean-k-mooney | bauzas: i think pslestang is suggeting that is not what happens today | |
| 17:06:21 | sean-k-mooney | pslestang: yes i think i have seen your tool | |
| 17:06:29 | sean-k-mooney | its now in the ops tools repo right | |
| 17:06:37 | bauzas | again, what's the current behaviour ? | |
| 17:06:40 | pslestang | sean-k-mooney: it should be I guess | |
| 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" | |