Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-01-31
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
15:03:39 sean-k-mooney today if you are repeating the required parmater you have a bug in the code that is calling placement
15:03:57 sean-k-mooney with the new version there will be a vaild meanign for that and that will be supproted with the new microverion
15:04:15 gibi gmann: yes
15:04:28 gmann 'changing behavior of " required=A&required=B " with microversion' is all good, I am thinking on for old microverison change
15:04:29 gibi btw efried is on gmann's side in the code review...
15:04:36 gmann which break users
15:04:47 sean-k-mooney gmann: it wont break users
15:04:57 sean-k-mooney gmann: user that have a duplciat request are already broken
15:04:58 gibi gmann: it breaks uses if the user query was wrong in the first place
15:05:03 efried ^
15:05:07 gmann sean-k-mooney: 200 to 400. when user asking with wring query
15:05:31 sean-k-mooney yes it will only be 400 for an invild query
15:05:38 sean-k-mooney where one of the requiremnt was being ignored
15:06:57 sean-k-mooney nova wont generate this but if we were to consider the isolated aggreates feature
15:06:59 sean-k-mooney https://docs.openstack.org/nova/latest/reference/isolate-aggregates.html
15:07:19 gibi I can imagine one case when we break a valid query. required=A,B&required=A,B is valid today and has a good meaning but it will be HTTP 400 after the fix
15:07:36 sean-k-mooney if we had &required:CUSTOM_LICENSED_WINDOWS&required:CUSTOM_SOMETHING_ELSE
15:07:43 sean-k-mooney the the isolated aggreate part would be lost
15:07:55 opendevreview Stephen Finucane proposed openstack/nova stable/wallaby: api: Reject duplicate port IDs in server create https://review.opendev.org/c/openstack/nova/+/827121
15:08:19 sean-k-mooney gibi: two idential values i guess we could check for and allow that
15:08:39 opendevreview Stephen Finucane proposed openstack/nova stable/victoria: api: Reject duplicate port IDs in server create https://review.opendev.org/c/openstack/nova/+/827122
15:08:51 gibi sean-k-mooney: we coudl but that gets complicate if we need to allow required=A,B&required=B,A as well
15:09:10 sean-k-mooney gibi: it does and i would still consider that to be invalid today
15:09:19 opendevreview Stephen Finucane proposed openstack/nova stable/ussuri: api: Reject duplicate port IDs in server create https://review.opendev.org/c/openstack/nova/+/827123
15:09:29 sean-k-mooney it makes logical sense but the parmater is not intended to be repatable today
15:09:30 efried IMO going into the weeds to allow special corner cases is not The Way. I think we should fix this, but it would be best to do it in a microversion.

Earlier   Later