Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-01-17
15:19:35 bauzas you need to either provide no dict or if you provide the dict, you need to add the AZ key
15:19:46 artom ... which could be None :P
15:20:00 bauzas so, when adding a new argument like "host", we would also need to add AZ
15:20:10 sean-k-mooney yep but there is no other property you can currently pass in the dict
15:20:32 bauzas yeah, that's why I'm asking to remove this for the new API microversion also adding the new argument
15:20:41 bauzas sean-k-mooney: correct, for the moment
15:20:43 sean-k-mooney artom it would have to be the litral value null potentailly but this is not the only place where its slightly odd
15:20:55 gibi I think both unshelve to an AZ or unshelve to a specific host make sense
15:21:16 bauzas sean-k-mooney: but once Uggla will add a new key like "host", that would mean that the AZ key should be required
15:21:29 sean-k-mooney yep likely you dont need to sepcify both but if you did we shoudl vlaidated the host to az relationship
15:21:43 artom No...? Having to specify both host and AZ seems weird
15:21:51 artom I'd say one or the other, but not both?
15:21:53 bauzas that's why I'm saying we should remove the requiring when we modify this API
15:22:11 bauzas in a new microversion of course
15:22:15 sean-k-mooney bauzas: well as currently specified but in generally unshele:{"host":"my-host"}
15:22:20 sean-k-mooney i think shoudl be valid
15:22:21 artom To my mind what would be clearest is: either empty body, or AZ with a non-null value, or host with a non-null value
15:22:22 bauzas anyway, let's wait Uggla to provide his spec for Z :)
15:22:32 sean-k-mooney i dont think we shoudl required Az if the host is passed
15:22:38 gibi artom: ++
15:22:43 bauzas sean-k-mooney: agreed, that's my concern
15:22:54 bauzas sean-k-mooney: https://github.com/openstack/nova/blob/1ddb8f83adef964a8ca050994a43adc6175994f1/nova/api/openstack/compute/schemas/shelve.py#L31 doesn't accept it for the moment
15:23:03 bauzas so we need to remove this line
15:23:16 bauzas with a new microversion and when adding a new argument
15:23:17 sean-k-mooney yes in the new microverion
15:23:26 sean-k-mooney in the current one it shoudl remain
15:23:32 bauzas ok, anyway, I think we have a consensus
15:23:45 bauzas sean-k-mooney: of course, I know it
15:24:00 sean-k-mooney yep just make sure Uggla captures it in the spec for next cycle
15:24:01 bauzas I was asking whether it was ok to remove the line at the same time we add a new argument
15:24:11 sean-k-mooney i assume they will start working on the implemantion in parallel
15:24:26 bauzas sean-k-mooney: correct, I discussed that with him :)
15:24:36 bauzas anyway, consensus, that's it
15:24:39 bauzas it was just a question
15:24:42 bauzas a simple one
15:25:53 sean-k-mooney yep i broght that up as we have not created the new spec dir yet but Uggla can totally do that and we can review the spec
15:26:26 Uggla cool, I'll work on this way.
15:26:32 sean-k-mooney but ya my vote is for make it not required and supprot null as well
15:30:33 Uggla passing both option, do you think that passing host should "discard" az
15:30:40 Uggla passing both options, do you think that passing host should "discard" az
15:31:25 Uggla I mean whatever we will have in az will be ignored if host is passed.
15:31:42 Uggla I mean whatever we will have in az will be ignored if host is passed ?
15:32:13 gibi Uggla: no, I think we should not ignore input. we either forbid both to be passed at once, or we should validate that the host is in the az
15:32:36 gibi I more with artom, to forbid to pass both in the same request
15:33:19 sean-k-mooney well either we validate that the host is in the az or it should be an error to pass both in my view
15:33:37 gibi sean-k-mooney: I agree
15:34:02 Uggla ok probably simpler to make it an error.
15:34:12 sean-k-mooney yep simple 400
15:34:24 sean-k-mooney no existing client will pass host
15:34:43 sean-k-mooney and when optinign into the new microversion they can ensure that only one is generated
15:35:34 Uggla Is it possible to do this kind of exclusive or parameters in the schema validation ?
15:35:48 sean-k-mooney yes
15:36:03 artom Right, but why do more work when less work is enough? :)
15:37:13 sean-k-mooney im pretty sure there is a way to defeint them as a mutualy exclucive group
15:37:30 sean-k-mooney i just cant think of a place off the top of my head whwere we actully do that
15:37:43 sean-k-mooney i suspect this si normally check after the scheme validation
15:39:32 Uggla Is making both options ok in schema validation and then doing the exclusive part in the code, sounds ok ?
15:39:38 sean-k-mooney https://github.com/openstack/nova/blob/1ddb8f83adef964a8ca050994a43adc6175994f1/nova/api/openstack/compute/schemas/servers.py#L172-L173
15:39:45 sean-k-mooney oneOf
15:40:09 Uggla sean-k-mooney, nice thank you !
15:40:42 sean-k-mooney as i said we rearly enforce that in teh scheme but no one will object if you do it either
15:41:40 sean-k-mooney there are some other exampels in the samee file
15:41:41 sean-k-mooney https://github.com/openstack/nova/blob/1ddb8f83adef964a8ca050994a43adc6175994f1/nova/api/openstack/compute/schemas/servers.py#L281-L298
15:42:58 bauzas sean-k-mooney: I'll create the specs directory once we know the Z name :)
15:43:26 bauzas unless I name it 'zombie' directly :p
15:45:24 sean-k-mooney oh ya good point
15:46:29 sean-k-mooney i still quite like "zenith" cause its all down hill from there
15:53:16 gmann bauzas: gibi sean-k-mooney Uggla artom yeah, we can remove the AZ from being 'required' from schema now as it will ne extended to accepting 'host' too. AZ was added as 'required' because this API only accept None or AZ and no other field but that is handled by additionalProperties=False so 'required' is not needed as such.
15:54:09 gmann bauzas: I think you will be right about 'zombie' :) everyone liking it :)
15:56:36 sean-k-mooney if its is zombie we shoudl make it a base for a downstream realse
15:57:01 sean-k-mooney because a decade form now we will still have once custoemr that refuses to move off it and it will never die
15:57:10 sean-k-mooney like any good zombie :)
15:58:02 gmann :)
15:59:52 sean-k-mooney osp 18 actuly should either be the z release of a release depending on timeing so maybe that will happen anyway
16:00:12 sean-k-mooney *Z release or A release
16:00:59 bauzas A = Awesome release I guess
16:01:17 bauzas easy peasy
16:01:28 sean-k-mooney B = Buffy to help with the zombie problem ?
16:02:01 bauzas C = Chilly
16:02:14 bauzas D = Draughful
16:03:51 Uggla I like B especially if Z is zombie !
16:14:36 Uggla Sarah Michelle Gellar was so cute.
16:15:55 opendevreview Merged openstack/nova master: ensure samples folder exists for microversion https://review.opendev.org/c/openstack/nova/+/813672
17:39:16 opendevreview Merged openstack/nova master: libvirt: Ensure all volume drivers log the instance whenever possible https://review.opendev.org/c/openstack/nova/+/780260
18:17:22 opendevreview Merged openstack/nova master: functional: Add reproducer for #1907775 https://review.opendev.org/c/openstack/nova/+/766771
19:44:05 ade_lee sean-k-mooney, lyarwood hey guys, I updated the fips job to include the full set of tempest tests and there were quite a few more failures -- can you guys take a look?
19:44:12 ade_lee https://review.opendev.org/c/openstack/nova/+/790519
19:45:44 ade_lee cinder folks are looking into other potential problems with volumes / cryptsetup here -- https://review.opendev.org/c/openstack/cinder/+/790535 so some of the failures may be related to that.
19:46:30 sean-k-mooney sure
19:47:05 sean-k-mooney have the general centos8 gate issue been fixed
19:47:17 sean-k-mooney i.e. the fact they broke ping
19:47:23 sean-k-mooney making it require root to run
19:47:55 ade_lee sean-k-mooney, not sure .. that woud certainly muddy things
19:48:14 ade_lee fungi, clarkb ^^ any idea?
19:49:01 fungi sean-k-mooney: ade_lee: i don't know if red hat has fixed centos stream 8's packages yet
19:49:42 sean-k-mooney the fips failrues might be unrealtaed but that is breaking all centos8 stream jobs currently right
19:49:45 fungi technically, centos stream 8 was (maybe still is) broken/regressed. i recommend not relying on it since it doesn't seem like they actually test it like they did the old centos
19:50:18 sean-k-mooney fungi: for what its worth i agree with not working aroudn this in our jobs
19:50:21 fungi i think there's work underway to get rocky linux added, and we have openeuler though it sounds like it may have a slightly too old kernel for the fips testing
19:50:50 sean-k-mooney fungi: really centos 8 stream should be fixed and maintianed as if it was rhel

Earlier   Later