| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-12-07 | |||
| 17:15:34 | sean-k-mooney | and if you use an old microverion for instnace action it should ignore the deleted field | |
| 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: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 | bauzas | OVH wants their clouds to stop reporting instance actions by default | |
| 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: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 | bauzas | period. | |
| 17:19:39 | sean-k-mooney | bauzas: i know but i dont think we can make there change they way they want | |
| 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 | |
| 17:24:24 | sean-k-mooney | in 2.100 or whatever you would not | |
| 17:24:30 | sean-k-mooney | unless you pass --deleted | |
| 17:24:35 | gmann | yes | |
| 17:24:44 | bauzas | sean-k-mooney: what's the difference with what I proposed ? | |
| 17:25:01 | bauzas | you would also have semantically incorrect values for 2.1 | |
| 17:25:04 | sean-k-mooney | in 2.100 you would only see non soft deleted ones by default | |
| 17:25:21 | sean-k-mooney | no you would not | |
| 17:25:43 | sean-k-mooney | in 2.1 we are reflecting the fact that instnace action have a sperate lifetime form the instnace they refer too | |
| 17:25:49 | sean-k-mooney | which were the sematics in 2.1 | |
| 17:25:55 | gmann | with 2.100 with deleted it shows all otherwise only non-soft deleted one | |
| 17:26:00 | sean-k-mooney | in 2.100 we are saying they share the same lifetime | |
| 17:26:30 | sean-k-mooney | in 2.100 soft deleting the instance will soft delete the actions | |
| 17:27:03 | sean-k-mooney | that is a semantic change in the behavior of the api hence need for new microverion | |
| 17:27:07 | sean-k-mooney | does that make sense | |
| 17:27:07 | bauzas | I can understand this | |
| 17:27:19 | bauzas | no config but a change for all | |
| 17:27:26 | sean-k-mooney | yes | |
| 17:27:46 | bauzas | just to be sure, default of 2.100 is 'delete' be always True, right? | |
| 17:27:57 | sean-k-mooney | no | |
| 17:28:10 | sean-k-mooney | default fo server delete will be to soft delete the instance actions | |
| 17:28:10 | bauzas | then, we need to discuss this with ops | |
| 17:28:21 | sean-k-mooney | default for instance_action show would be deleted=false | |
| 17:28:26 | gmann | and we cannot fix it as bug as it is changing API behaviour and so does effect existing users | |
| 17:28:40 | bauzas | I'm pretty sure I can find a handful of operators who wouldn't want to opt-in for v2.100 | |
| 17:29:04 | bauzas | the problem is that if operators want v2.101, they're stuck with shipping 2.100 too | |
| 17:29:13 | bauzas | which they dislike | |
| 17:29:33 | sean-k-mooney | for which endpoint | |
| 17:29:44 | bauzas | the os-instance-actions one, of course | |
| 17:30:03 | sean-k-mooney | if we have a 2.101 for that endpoint | |
| 17:30:10 | sean-k-mooney | the client already needs to be updated to use it | |
| 17:30:16 | sean-k-mooney | so they can just pass deleted=false | |
| 17:30:20 | bauzas | honestly, the more I think, the less I consider it as something good for Nova if we make this general for ops | |
| 17:30:21 | sean-k-mooney | *deleted=true | |
| 17:30:23 | gmann | well that is drawback but applicable for all the changes via microversion | |
| 17:30:51 | bauzas | while on the other hand, the archive problem is solvable with a smarter script | |
| 17:31:05 | sean-k-mooney | the fact we are adding a query arg in 2.100 means they can choose what behvior they want in 2.101 | |
| 17:31:32 | bauzas | that's one thing to let OVH do what they want with their own APIs | |
| 17:31:34 | gmann | sean-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 actions | |
| 17:31:51 | bauzas | that's another thing to consider that ops don't care about getting information why their instance is deleted | |
| 17:31:55 | sean-k-mooney | gmann: we could do that yes | |
| 17:32:12 | sean-k-mooney | but i think it makes sense to change the default we coudl defer that to a later microverion | |
| 17:32:15 | bauzas | sean-k-mooney: this was my proposal... default be "show-deleted" | |
| 17:32:27 | sean-k-mooney | bauzas: right | |
| 17:32:45 | bauzas | either way, this is a spec | |
| 17:32:49 | sean-k-mooney | but if we are making this change i was suggesting changing the bhavior now instead of having 2 microverions for it | |