| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-12-07 | |||
| 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 | |
| 17:33:06 | sean-k-mooney | if its less contoversion to not change the defualt behavior im not agaisnt that | |
| 17:33:17 | bauzas | honestly, as I said, I'm not super happy with changing the default behaviour | |
| 17:33:22 | sean-k-mooney | provided we never change the default behavior or atleast not in the near future | |
| 17:33:27 | bauzas | (from an ops perspective) | |
| 17:33:39 | sean-k-mooney | bauzas: ok then lets not | |
| 17:33:40 | gmann | yeah. let's do spec. keeping existing behavior as default will be useful | |
| 17:33:49 | sean-k-mooney | ok | |
| 17:34:29 | bauzas | gmann: 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-specific | |
| 17:34:30 | sean-k-mooney | the thing i want to avoid is two microverions to 1 allow filtering and 2 change the default close toghter | |
| 17:34:49 | sean-k-mooney | if we do the first part and oepraters give feedback we can reconsider the second | |
| 17:35:00 | bauzas | gmann: 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 param | |
| 17:35:04 | bauzas | looks unfair, no ? | |
| 17:35:08 | sean-k-mooney | bauzas: actully they do not use our archive feature at all | |
| 17:35:27 | sean-k-mooney | bauzas: no only if they use the latest microverion | |
| 17:35:41 | sean-k-mooney | if they are using microverions correctly that is not going to affect them | |
| 17:35:45 | gmann | yeah and nobody complained on current default behavior so good to keep it | |
| 17:35:52 | sean-k-mooney | its why osc orginally didn not defualt to latest microverion | |
| 17:36:01 | sean-k-mooney | so that the cli would be stable | |
| 17:36:20 | bauzas | gmann: even OVH hasn't complained about the current behaviour, they just complain about the fact their tooling doesn't work with our DB | |
| 17:36:26 | sean-k-mooney | anyway i think we are agred. needs a spec and keep current behviaor by default | |
| 17:36:30 | gmann | bauzas: yeah. | |
| 17:36:36 | gmann | sean-k-mooney: +1 | |
| 17:37:02 | bauzas | sean-k-mooney: even with a spec, I think this is taking a hammer for chasing a firefly | |
| 17:37:05 | sean-k-mooney | bauzas: well as i said ovh do not use our showdown tables anda archiving mechanium | |
| 17:37:23 | bauzas | sean-k-mooney: that's the whole point, they don't use what we provide | |
| 17:37:32 | bauzas | why should we make modifications for them so ? | |
| 17:37:38 | sean-k-mooney | right because not all service implement it | |
| 17:37:44 | bauzas | if it's all about DB persistency | |
| 17:37:49 | sean-k-mooney | there tool work for neutorn glance cinder consitently | |
| 17:38:13 | sean-k-mooney | bauzas: i think ovh and i woudl both be happy if we fully remove the shadow tables | |
| 17:39:29 | bauzas | I still think we're ending into a weird state | |
| 17:39:53 | bauzas | we said in the past "this is expected behaviour, as actions table is here for recording user actions" | |
| 17:40:23 | bauzas | now we're about to publicly express that the actions API is just usable by default for non-deleted instances | |
| 17:40:33 | bauzas | (if we change the default) | |
| 17:40:40 | sean-k-mooney | bauzas: this is what they use https://github.com/ovh/osarchiver/ | |
| 17:41:01 | bauzas | if we don't change the default, we just provide a new microversion that won't change the behaviour by default, which is also weird | |