| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-01-31 | |||
| 13:08:18 | sean-k-mooney | alreday have it open :) | |
| 13:08:33 | sean-k-mooney | i assume a 400 will be returned | |
| 13:08:36 | stephenfin | yup | |
| 13:08:51 | opendevreview | Dmitrii Shcherbakov proposed openstack/nova master: Add nova-ovs-hybrid-plug job https://review.opendev.org/c/openstack/nova/+/817303 | |
| 13:08:51 | stephenfin | I'm really hoping to avoid a microversion for that, since it's so obviously wrong as-is | |
| 13:09:00 | stephenfin | but I can't make that choice alone, of course :) | |
| 13:09:02 | sean-k-mooney | 400 is already a valid return code | |
| 13:09:07 | sean-k-mooney | so no microversion needed | |
| 13:09:16 | stephenfin | right, but it's currently returning a 201 | |
| 13:09:21 | stephenfin | since server create is async | |
| 13:09:34 | sean-k-mooney | its returning a 201 but the server end in error | |
| 13:09:38 | stephenfin | yeah | |
| 13:09:44 | sean-k-mooney | i think we have an excpetion for that | |
| 13:10:14 | stephenfin | so I'm arguing that since every other subsequent server op would fail, this is an acceptable change. We shouldn't break anyones automation | |
| 13:10:24 | sean-k-mooney | https://docs.openstack.org/nova/latest/contributor/microversions.html#when-do-i-need-a-new-microversion | |
| 13:10:37 | sean-k-mooney | so i would argue that thies is the first yes | |
| 13:10:46 | sean-k-mooney | did we silently fail to do what is asked | |
| 13:11:00 | opendevreview | Dmitrii Shcherbakov proposed openstack/nova master: Revert "Revert resize: wait for events according to hybrid plug" https://review.opendev.org/c/openstack/nova/+/819494 | |
| 13:11:20 | sean-k-mooney | we log the error for the duplciate keys in nova-compute but i think the user got a no valid host error in the event log | |
| 13:12:34 | stephenfin | we don't even get as far as scheduling | |
| 13:13:00 | stephenfin | but ultimately we silently fail to do what is asked, yes | |
| 13:13:25 | sean-k-mooney | oh we create teh entry in the virtual interfaces table in the api i gues so we can store any tag infor | |
| 13:13:52 | sean-k-mooney | ok well ya so im pro no microververion for this too and backporting it | |
| 13:14:19 | opendevreview | Stephen Finucane proposed openstack/nova master: api: Reject duplicate port IDs in server create https://review.opendev.org/c/openstack/nova/+/827070 | |
| 13:15:19 | sean-k-mooney | stephenfin: since your about can you quickly look at https://review.opendev.org/c/openstack/nova/+/817303/11 and the followup revert form artom | |
| 13:16:09 | sean-k-mooney | dmitriis: ping me later today when the full series is rebased and up and ill try and do a full pass form start to finish | |
| 13:16:34 | stephenfin | sean-k-mooney: Sure, I won't do it right now but I'll get to it straight after lunch | |
| 13:16:53 | sean-k-mooney | dmitriis: ill be busy for the next hour or so but ill try to get to it later | |
| 13:16:56 | sean-k-mooney | stephenfin: ack | |
| 13:17:08 | chateaulav | sean-k-mooney: what would be the best method for testing my ci job? | |
| 13:18:10 | dmitriis | sean-k-mooney: ack, on it now. I'll add a release note to the latest commit as well. I'm going to work on a doc change as well after I resubmit. | |
| 13:28:09 | sean-k-mooney | chateaulav: typically we submit a DNM patch at the end of a series, in that you can disable most of the intree ci jobs and then test your job by doing <ci>-recheck or whatever the comment is that you use for your ci | |
| 13:28:40 | sean-k-mooney | chateaulav: [DNM] means do not merge and typiclay do not review | |
| 13:29:08 | sean-k-mooney | we use it when ever we are doing things to see if they work or when we are workign on infra like the ci | |
| 13:29:38 | gibi | stephenfin: +2 on your fix. thanks | |
| 13:32:04 | chateaulav | gotcha | |
| 13:32:08 | sean-k-mooney | chateaulav: here is an example of working on a first party job https://review.opendev.org/c/openstack/nova/+/727228/1/.zuul.yaml you would do the same for third party just leave the -check-requiremetns template enabled and comment out the check an gate jobs then you can recheck usign your third party ci comment | |
| 13:32:36 | chateaulav | sean-k-mooney: thanks! | |
| 13:32:57 | sean-k-mooney | chateaulav: no problem | |
| 13:37:34 | plibeau4 | lyarwood: https://review.opendev.org/c/openstack/nova/+/820531 when you have time :) | |
| 13:43:52 | gibi | sean-k-mooney: filed bug about placement silently ignoring repeated required query params https://storyboard.openstack.org/#!/story/2009816 I know that you would like to fix it as a bugfix to return http400 (option a) in the bugreport) | |
| 13:44:02 | gibi | bauzas, gmann, melwitt: ^^ Do you agree? | |
| 13:44:18 | bauzas | sorry folks, I had a problem with my internet | |
| 13:44:39 | sean-k-mooney | gibi: that would be my perfernce yes but lets see how the rest feel | |
| 13:44:43 | bauzas | (not my internet, rather my router) | |
| 13:45:24 | sean-k-mooney | gibi: if we think we cant do that without a microverion im ok with what you have previously propsoed in the patch | |
| 13:46:32 | gibi | sean-k-mooney: ack, I will prepare a bugfix as I also think this should be fixed | |
| 13:47:27 | bauzas | gibi: hah, I finally saw the story | |
| 13:47:45 | bauzas | gibi: when I was calling the link, it wasn't giving me the story | |
| 13:48:19 | bauzas | gibi: looks to me a correct bug | |
| 13:48:45 | bauzas | for the solution, yeah, HTTP400 without needing a microversion I think | |
| 13:49:05 | bauzas | but let's wait for gmann's thoughts | |
| 13:50:23 | sean-k-mooney | gibi: oh thats an interesting edge case this also hides invlaid "standard" traits like that interesting. it makes sense but that is even more broken then just ignoring some of your requirements when selecting resouces | |
| 13:50:49 | gibi | yepp it hides invalid things | |
| 14:44:21 | gibi | incoming... | |
| 14:44:27 | opendevreview | Balazs Gibizer proposed openstack/placement master: Refactor trait normalization https://review.opendev.org/c/openstack/placement/+/825847 | |
| 14:44:27 | opendevreview | Balazs Gibizer proposed openstack/placement master: Extend the RP db query to support any-traits https://review.opendev.org/c/openstack/placement/+/825848 | |
| 14:44:28 | opendevreview | Balazs Gibizer proposed openstack/placement master: Reproduce bug story/2009816 https://review.opendev.org/c/openstack/placement/+/827114 | |
| 14:44:31 | opendevreview | Balazs Gibizer proposed openstack/placement master: Reject repeated required[N] param https://review.opendev.org/c/openstack/placement/+/827115 | |
| 14:44:35 | opendevreview | Balazs Gibizer proposed openstack/placement master: Extra tests around required traits https://review.opendev.org/c/openstack/placement/+/827116 | |
| 14:44:40 | opendevreview | Balazs Gibizer proposed openstack/placement master: DB layer should only depend on trait id not names https://review.opendev.org/c/openstack/placement/+/826490 | |
| 14:44:41 | opendevreview | Balazs Gibizer proposed openstack/placement master: Enhance doc of _get_trees_with_traits https://review.opendev.org/c/openstack/placement/+/825780 | |
| 14:44:50 | opendevreview | Balazs Gibizer proposed openstack/placement master: Extend the RP tree DB query to support any-traits https://review.opendev.org/c/openstack/placement/+/825849 | |
| 14:45:00 | opendevreview | Balazs Gibizer proposed openstack/placement master: Add any-traits support for listing resource providers https://review.opendev.org/c/openstack/placement/+/826491 | |
| 14:45:06 | opendevreview | Balazs Gibizer proposed openstack/placement master: Add any-traits support for allocation candidates https://review.opendev.org/c/openstack/placement/+/826492 | |
| 14:45:07 | opendevreview | Balazs Gibizer proposed openstack/placement master: Remove unused compatibility code https://review.opendev.org/c/openstack/placement/+/826493 | |
| 14:45:07 | opendevreview | Balazs Gibizer proposed openstack/placement master: Add microversion 1.39 to support any-trait queries https://review.opendev.org/c/openstack/placement/+/826719 | |
| 14:51:54 | gmann | sean-k-mooney: stephenfin +1 on changing return code from 201 to 400 without microversion in case of 'Reject duplicate port IDs in' because server goes in error at the end | |
| 14:51:58 | opendevreview | Balazs Gibizer proposed openstack/placement master: Extra tests around required traits https://review.opendev.org/c/openstack/placement/+/825846 | |
| 14:51:59 | opendevreview | Balazs Gibizer proposed openstack/placement master: Refactor trait normalization https://review.opendev.org/c/openstack/placement/+/825847 | |
| 14:51:59 | opendevreview | Balazs Gibizer proposed openstack/placement master: Extend the RP db query to support any-traits https://review.opendev.org/c/openstack/placement/+/825848 | |
| 14:52:05 | opendevreview | Balazs Gibizer proposed openstack/placement master: DB layer should only depend on trait id not names https://review.opendev.org/c/openstack/placement/+/826490 | |
| 14:52:06 | opendevreview | Balazs Gibizer proposed openstack/placement master: Enhance doc of _get_trees_with_traits https://review.opendev.org/c/openstack/placement/+/825780 | |
| 14:52:06 | opendevreview | Balazs Gibizer proposed openstack/placement master: Extend the RP tree DB query to support any-traits https://review.opendev.org/c/openstack/placement/+/825849 | |
| 14:52:07 | opendevreview | Balazs Gibizer proposed openstack/placement master: Add any-traits support for listing resource providers https://review.opendev.org/c/openstack/placement/+/826491 | |
| 14:52:29 | opendevreview | Balazs Gibizer proposed openstack/placement master: Add any-traits support for allocation candidates https://review.opendev.org/c/openstack/placement/+/826492 | |
| 14:52:30 | opendevreview | Balazs Gibizer proposed openstack/placement master: Remove unused compatibility code https://review.opendev.org/c/openstack/placement/+/826493 | |
| 14:52:43 | opendevreview | Balazs Gibizer proposed openstack/placement master: Add microversion 1.39 to support any-trait queries https://review.opendev.org/c/openstack/placement/+/826719 | |
| 14:52:53 | gmann | gibi: sean-k-mooney bauzas for placement API ignoring the extra param in query seems like API change which impact users. We should not do that without microversion because of interop. | |
| 14:53:28 | gibi | gmann: even if ignoring a repeated silently hides an error? | |
| 14:53:36 | gibi | *repeated param | |
| 14:54:35 | gmann | gibi: sean-k-mooney bauzas we fixed that in nova with microversion only - https://specs.openstack.org/openstack/nova-specs/specs/train/implemented/api-consistency-cleanup.html#proposed-change | |
| 14:54:54 | gibi | e.g. the user asked for an RP that has both trait A and trait B but because A is ignored it gets RPs with only trait B | |
| 14:55:12 | gibi | this is a logic error not just inconvinience | |
| 14:56:21 | gibi | so it is not about ignoring invalid query params | |
| 14:56:23 | gmann | gibi: but that is what user asked, 'trait B' at the end of query he mentioned. it can be used in confusion as I am mentioning two value in single field and API should accept it as both but that is wring query | |
| 14:57:01 | gmann | gibi: yeah so this is only if user query in for same field. | |
| 14:57:04 | gibi | I don't think that when the user said required=A&required=B she meant that only apply B filter | |
| 14:57:29 | gmann | how they can do multiple query ? for 'required' A and B both together? | |
| 14:58:03 | gibi | today with required=A,B | |
| 14:58:20 | gibi | after microversion 1.39 required=A&required=B will work too | |
| 14:58:39 | gibi | but today required=A&required=B is paresed as required=B by placement | |
| 15:00:18 | gmann | gibi: so you are fixing it with 400 or allow required=A&required=B to return A and B ? | |
| 15:00:46 | gibi | I have a bugfix without microversion bump that retuns http400 if required is repeated | |
| 15:01:04 | gibi | and I have a feature with a microversion bump that allows repeating and pareses it as A and B | |
| 15:01:14 | gibi | (and that microversion adds other things to required too) | |
| 15:01:52 | gibi | gmann: this is the series, the bottom is the bugifx https://review.opendev.org/q/topic:any-traits-support | |
| 15:02:38 | opendevreview | Stephen Finucane proposed openstack/nova stable/xena: api: Reject duplicate port IDs in server create https://review.opendev.org/c/openstack/nova/+/827120 | |
| 15:02:41 | gmann | ok so changing behavior of " required=A&required=B " with microversion but for older microversion we will change 200 -> 400 right ? | |
| 15:02:58 | sean-k-mooney | gmann: yes | |