| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-12-07 | |||
| 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 | bauzas | I can understand this | |
| 17:27:07 | sean-k-mooney | does that make sense | |
| 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 | bauzas | then, we need to discuss this with ops | |
| 17:28:10 | sean-k-mooney | default fo server delete will be to soft delete the instance actions | |
| 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 | |
| 17:41:23 | sean-k-mooney | we often add microverion that dont change default behavior | |
| 17:41:28 | bauzas | and why ? because we consider the current behaviour is enough good to not touch it | |
| 17:41:43 | bauzas | sean-k-mooney: we used microversions for "signaling", I know | |
| 17:41:51 | sean-k-mooney | no | |
| 17:42:05 | sean-k-mooney | the feature this would be interocudeing is the ablity to filter on the soft delete status | |
| 17:42:17 | sean-k-mooney | that does not require use to change the defualt behavior | |
| 17:42:28 | sean-k-mooney | so its not jsut signaling | |
| 17:44:19 | gmann | yeah, adding new filtering which is asked in current specless BP | |
| 17:44:52 | gmann | currently there is no way to filter soft deleted instance actions | |
| 17:45:39 | gmann | with new microversion, we solve both use case 1. existing keep working 2. way to filter the soft deleted one | |
| 17:45:45 | sean-k-mooney | so 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 change | |
| 17:47:56 | sean-k-mooney | the filtering is an api change as we are adding a new query arg even if the default behvior does not change. | |
| 17:48:21 | pslestang | sean-k-mooney: +1 for your proposition | |
| 17:49:28 | sean-k-mooney | pslestang: by the way what happend with your upstreaming efforts http://lists.openstack.org/pipermail/openstack-discuss/2021-February/020383.html | |
| 17:49:49 | sean-k-mooney | that was the last mail i could find on that trhead was there a tc desicion made | |
| 17:50:59 | sean-k-mooney | i dont see it in https://opendev.org/openstack/osops/src/branch/master so i guess it was not added to osops | |
| 17:51:15 | sean-k-mooney | and i dont see a new repo for https://github.com/ovh/osarchiver/ | |
| 17:51:27 | sean-k-mooney | did ye decide to just keep it in github in the end | |
| 17:53:33 | sean-k-mooney | pslestang: 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 happened | |
| 17:54:46 | sean-k-mooney | if 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 period | |
| 17:54:50 | pslestang | sean-k-mooney:this is what I was looking for, exact option 3 prefered but nothing done | |
| 17:57:18 | pslestang | We prefer to put it under opendev than keeping it in github, we really thing that other operators could take benefit of this tool | |