| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-01-31 | |||
| 18:45:38 | opendevreview | Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| 18:49:53 | opendevreview | Merged openstack/nova master: Add nova-ovs-hybrid-plug job https://review.opendev.org/c/openstack/nova/+/817303 | |
| 19:35:27 | opendevreview | Merged openstack/nova master: Deprecate the powervm driver https://review.opendev.org/c/openstack/nova/+/819366 | |
| 19:42:19 | chateaulav | can i get a second look on https://review.opendev.org/c/openstack/nova/+/822053, not sure what happened but it seems to be wanting to merge... i think im stuck and not sure what i broke | |
| 19:56:23 | melwitt | chateaulav: it show it's in merge conflict, should just need a rebase | |
| 19:57:01 | melwitt | *shows | |
| 19:57:31 | chateaulav | melwitt: yeah, tried, i think i got it. had to hard reset the branch. gonna add all the changes back and see if it resolves now | |
| 19:57:33 | chateaulav | thanks | |
| 20:04:04 | melwitt | ack | |
| 20:43:07 | opendevreview | Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| 20:48:44 | opendevreview | Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| 20:50:57 | opendevreview | Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| 20:53:19 | opendevreview | Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| 20:55:36 | opendevreview | Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| 21:11:26 | opendevreview | Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| 21:17:00 | opendevreview | Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| #openstack-nova - 2022-02-01 | |||
| 00:16:52 | gmann | gibi: ack | |
| 02:30:43 | gmann | gibi: sean-k-mooney[m] melwitt just tested that nova API query param also allow duplicate query param and take latest one. I remember somewhere we convert that to python dict that is why the last one is considered. | |
| 02:31:13 | melwitt | well that's fun | |
| 02:31:52 | sean-k-mooney[m] | there are some query parmaters that we all to be repeated like field that can be used to generate an array | |
| 02:32:19 | sean-k-mooney[m] | but unless we expressly document that as a supported usecase in teh api ref using it should be considerd a bug | |
| 02:32:49 | sean-k-mooney[m] | if its not in the api-ref or in the orginal spec it is not part of the public api contract | |
| 02:33:16 | sean-k-mooney[m] | gmann which nova api query parmater were you testing? | |
| 02:33:26 | gmann | list server | |
| 02:33:40 | gmann | limit filter | |
| 02:37:54 | sean-k-mooney[m] | no user should ever depend on the last instance of a parmater being used in the nova api we are free to change that at any point unless there is api documenation that expressless states that for any given parmater. | |
| 02:39:39 | sean-k-mooney[m] | put another way i do not think we should ever accpeat a bug if we change that behaivor without a microversion as it is an out of contract use fo the api and governed by the micorversion contract as a result | |
| 02:48:42 | sean-k-mooney[m] | ack i would consider depending on that behavior to be unsupported, https://docs.openstack.org/api-ref/compute/?expanded=list-servers-detail#list-servers does not declare you are allowed to repate the args as part of the filter support | |
| 02:49:46 | sean-k-mooney[m] | we should be retrunnign a 400 but we are not validateing that. if we change the behavior in the future it should not require a microversion as this is not part of the api specification today | |
| 02:52:35 | gmann | gibi: melwitt sean-k-mooney[m] we validate it by converting it to set which will add multiple passed value in query param - https://github.com/openstack/nova/blob/master/nova/api/validation/__init__.py#L176 | |
| 02:52:41 | gmann | which is good. | |
| 02:52:54 | gmann | but here this guy just pick latest one https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L179 | |
| 02:53:34 | gmann | sean-k-mooney[m]: additional param in query param was also not written behavior in api-ref but we change that with microversion so that we do not break users | |
| 02:54:13 | sean-k-mooney[m] | i really think this raised to the level of a v3 api change if we dont reject it without a microversion change | |
| 02:54:34 | sean-k-mooney[m] | i really dont think this is something we shoudl require a microversion change for | |
| 02:54:34 | gmann | I consider API usage as what code allow than what is there in api-ref. api-ref are not complete set of 'how not to use' its just 'how to use' | |
| 02:55:20 | sean-k-mooney[m] | to me the is a unaccpable burden of mantaince to contineu to supprot. the api is not defiend by the implemantion its defiend by the specification and this is not part of the specificaiont | |
| 02:55:21 | gmann | it is like we allowed our API to be used that wrong way and for many users it was working that as successful case. | |
| 02:55:56 | gmann | sean-k-mooney[m]: not our API :) api -ref was written based on what implementation was. | |
| 02:56:33 | gmann | we try to match that but if api-ref mention something and code does not behave that way then yes we consider as bug | |
| 02:57:03 | gmann | but if api-ref which is not complete does not mention anything does not mean code is wrong and we can change that without versioning | |
| 02:57:34 | gmann | in past we used to document that as known bug/behavior and fix in new microversion | |
| 02:57:51 | sean-k-mooney[m] | i disagree if the api and spec for the feature do not refernce the behaivor it is not part of the public contract | |
| 02:58:32 | sean-k-mooney[m] | i dont think we should leak the impleation into the public contract | |
| 02:58:40 | gmann | it should if we have designed our API that way but it is not. it was always what implementation was and that is being used. | |
| 02:58:52 | sean-k-mooney[m] | we did design our apis that way | |
| 02:59:02 | sean-k-mooney[m] | its why we require a spec for all api changes | |
| 02:59:40 | sean-k-mooney[m] | the really issue i guess is some fo the apis are form pre microversion and did not have a spec | |
| 02:59:59 | gmann | that is fine. I am saying for old and existing behavior. that is why microversion was introduced. for new APis/feature I agree as per spec driven. | |
| 03:00:01 | sean-k-mooney[m] | for example the regex support today in list servers is directly tied to the db you use | |
| 03:00:54 | sean-k-mooney[m] | i really think it would reduce the quality of our software to not fix what in my mind is broken validation code | |
| 03:01:04 | gmann | even rejecting unknown and invalid query param (which we ignored silently in code) was also changed with microversion so that we do not break users | |
| 03:01:48 | gmann | I am not saying that is good and should not be fixed. it should be but with microversion | |
| 03:03:14 | sean-k-mooney[m] | i guess with my downstream hat on i would be much less willing to supprot this. upstream we might be able to allow invalid input but form a downstream perspecitive i dont think i would accpete this form a customer bug. the api contract exits for a reason and not enforcing that contract exposes us to suppporting thing we never intended that is harmful to the healt of the product in my view | |
| 03:04:29 | gmann | my 1 month old saying to stop arguing on APIs :) its time to feed him. :P | |
| 03:04:41 | sean-k-mooney[m] | :) | |
| 03:04:52 | sean-k-mooney[m] | go do that | |
| 03:05:35 | sean-k-mooney[m] | but if we cant fix this without a microverion or validation middleware i think we have to reconsider raising the min microversion to fix this | |
| 08:04:56 | gibi | the more I think about API contract the more I feel that we are on the wrong path. Saying that we should not change the behavior of our APIs without a microversion could be abused to the extent that forbid any kind of change of the implementation behind the API. | |
| 08:06:52 | gibi | say we have a placement sql bug that breaks resource acconting today in a certain situation. As this is how the service works today one can say that a client might depend on the buggy behavior of the resource accounting hence we cannot fix the accounting outside of a microversion so placement servers out on the field will have the buggy behavior until everybody upgrades to future mastyer | |
| 08:07:17 | gibi | so imagine two customers | |
| 08:07:41 | gibi | i) customer depending on the buggy behavior and getting a stable bugfix would break their usage of placement | |
| 08:07:57 | gibi | ii) customer is facing the buggy behavior and their system is broken until the buggy behavior is removed | |
| 08:08:22 | gibi | clearly we cannot support both customer | |
| 08:08:54 | gibi | so it boils down to which customer we would like to support more? i) who depends on a bug ii) who are hit by the same bug | |
| 08:36:45 | bauzas | gibi: agreed with you | |
| 08:37:08 | bauzas | I wonder why we should support the i) customer if they already have an issue | |
| 08:38:26 | gibi | i) has no issue it is happy who the api behaves today | |
| 08:38:55 | gibi | s/who/how/ | |
| 08:42:37 | bauzas | gibi: well, I'm not sure this customer knows about the behaviour actually | |
| 08:42:52 | bauzas | maybe he's just happy because it works | |
| 08:44:40 | gibi | yeah, it can be accidental use | |
| 08:51:18 | bauzas | just a thought, we always said to avoid as much as possible to directly pass a placement request by flavors | |
| 08:52:33 | bauzas | gibi: so that would mean that customers doing this (and passing two same request params) looks to me not a bug | |
| 08:55:34 | gibi | I don't see why repeating required param today is a good idea as it does not give a consistent result. E.g. you say required=A&required=B but A is ignored. Why would you do that intentionally? | |
| 08:56:01 | gibi | so next time you add required=C to the list, B also gets ignored | |
| 08:56:36 | gibi | so whathever client does this that client is wrong and a real bug is about to happen with that client | |
| 08:57:41 | gibi | (btw openstack client is good, --required A --required B creates a REST request with required=A,B) | |
| 08:59:53 | gibi | also imagine that the client today does required=A,B&required=A,B and that produce a good result today | |
| 09:00:19 | gibi | then a new requirement came to add C to the request in that client | |
| 09:00:42 | gibi | the dev looking at the current client code can assume that required parameters can be repeated | |
| 09:00:57 | gibi | as it is repeated today and works | |
| 09:01:13 | gibi | but he might decide not to add C to both instance of required | |
| 09:01:20 | gibi | then he just implemented a bug | |
| 09:06:43 | bauzas | gibi: when I say "not a bug" I mean that's something we can close without a microversion honestly | |
| 10:32:34 | opendevreview | Rajat Dhasmana proposed openstack/nova master: WIP: Add support for volume backed server rebuild https://review.opendev.org/c/openstack/nova/+/820368 | |
| 11:24:57 | sean-k-mooney[m] | so i really think we should fix this however if we cant agree to just do it i would like to add a new default middleway to reject repated args outside a allowed set | |
| 11:28:41 | sean-k-mooney[m] | by the way changes to policy i confider much more likely to break peole then this. we dont reqiure micoro versions for default policy changes for some reason which to me feels like a feautre yet the assertion is we cant fix broken behavior without one feels wrong | |
| 11:37:15 | gibi | sean-k-mooney[m]: I don't see how we would be able to agree on the middleware being a fix without a microversion. Or do you propose that as a microversion bump? | |
| 12:01:44 | sean-k-mooney[m] | no | |
| 12:01:57 | sean-k-mooney[m] | middleware is not part of the versioned api | |
| 12:02:16 | sean-k-mooney[m] | its operator configurable via paste.ini | |
| 12:03:28 | sean-k-mooney[m] | so my proposal in order of preference would 1.) fix without a microversion, 2.) fix with a micorversion and provide middleware for older microverion that returns 400 if there is an unsupported repeated argument | |
| 12:04:18 | sean-k-mooney[m] | 3.) fix with a microverion and serioiusly consider raising min micorversion over the next few cycle until we finally get to this one | |
| 12:05:28 | sean-k-mooney[m] | gibi at some point i stongly feel this needs to be fixed by default for everyone that uses nova and placment since the same broken behavior affects both | |
| 12:06:03 | sean-k-mooney[m] | so if we force a microverion for this we need to look at starting the process of raising or min micorversion | |
| 12:09:55 | gibi | sean-k-mooney[m]: ahh I see, so the middleware would be optional therefore does not need a microversion. Yeah that could be tried. Probably on can say that becomes a config driven API but paste.ini is created exactly for that kind of things | |
| 12:10:37 | sean-k-mooney1 | ya i was lookign are our exising middelware it does not look like it would be that hard to create one that did this | |
| 12:11:14 | sean-k-mooney1 | basically using https://github.com/openstack/nova/blob/master/nova/api/openstack/identity.py as a template | |
| 12:12:07 | sean-k-mooney | we would then add a new filter lin and add that filter to the pipeline https://github.com/openstack/nova/blob/master/etc/nova/api-paste.ini#L68-L81 | |