Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-03-06
16:32:56 stephenfin I checked and graceful_shutdown_timeout defaults to a minute
16:32:58 dansmith no, it's not
16:33:16 dansmith actually, nevermind what I said about the resource update, because that does drop and re-acquire
16:33:19 stephenfin so at worst we're introducing an extra 60 second delay
16:33:36 dansmith stephenfin: where's the 60 second limit?
16:34:02 stephenfin dansmith: https://github.com/openstack/oslo.service/blob/master/oslo_service/_options.py#L51
16:34:06 dansmith is that something the service does?
16:34:26 stephenfin yeah, if this comment is to be believed https://review.opendev.org/#/c/666245/16/nova/compute/manager.py@1511
16:34:28 dansmith where's the implementation of that though?
16:34:50 dansmith https://github.com/openstack/oslo.service/blob/master/oslo_service/service.py#L333
16:35:21 dansmith does it with sigalrm so should break us out of that otherwise infinite wait
16:37:25 gibi OK, so it is really just an extra 60 seconds delay
16:37:37 stephenfin that's my understanding, yes
16:37:50 stephenfin I think that's reasonable. We have a reno too
16:38:15 gibi I feel better that none of you freaked out about that patch.
16:38:23 dansmith I haven't really reviewed
16:38:27 dansmith can I have a few minutes?
16:38:32 gibi dansmith: sure
16:38:47 gibi dansmith: but you already know the basic idea of the patch
16:38:56 dansmith yeah
16:39:05 dansmith we already have a build semaphore and config for it
16:39:14 dansmith this would seem to complicate that a bit
16:40:06 stephenfin Does this need a microversion? https://review.opendev.org/#/c/709431/
16:40:46 stephenfin tl;dr: We've been using '/hypervisors' in our 'links' pagination field, rather than '/os-hypervisors'
16:41:11 gibi stephenfin: I think we don't want API users to opt into a bugfix
16:41:30 stephenfin yes, that's my thinking too
16:41:40 gibi this feels a clear bugfix to me
16:42:28 dansmith gibi: stephenfin: https://review.opendev.org/#/c/666245/16
16:43:36 stephenfin oh, I never even thought of that /o\ good spot
16:43:47 gibi dansmith: good catch
16:44:00 dansmith the change should be small.. just remove the new conf and the old semaphore, wire the group to the old conf
16:44:13 gibi yeah, make sense
16:45:15 gibi stephenfin: I'm +2 on the os-hypervisors bugfix
16:46:49 stephenfin gibi: Just checked and looks like os-keypairs is also wrong
16:47:13 gibi stephenfin: soo you won a bug to fix ;)
16:47:37 gibi dansmith: if you are in a reviewing mood then the last pieces of the qos bp needs a second core https://review.opendev.org/#/q/status:open+bp/support-move-ops-with-qos-ports-ussuri
16:48:14 dansmith gibi: I'm in a -1ing mood.. do you want to take that gamble? :D
16:49:05 gibi dansmith: if it is as valid -1 as on the graceful shutdown patch then please go ahead
17:03:15 openstackgerrit Stephen Finucane proposed openstack/nova master: Fix os-keypairs pagination links https://review.opendev.org/711662
17:03:16 openstackgerrit Stephen Finucane proposed openstack/nova master: trivial: Rename directory for os-keypairs samples https://review.opendev.org/711663
17:03:17 stephenfin gibi: ^
17:03:28 stephenfin (second one's just a nice-to-have)
17:03:43 gibi stephenfin: thanks, looking
17:04:33 openstackgerrit Stephen Finucane proposed openstack/nova stable/train: Fix hypervisors paginted collection_name. https://review.opendev.org/711664
17:05:00 openstackgerrit Stephen Finucane proposed openstack/nova stable/train: Fix hypervisors paginted collection_name. https://review.opendev.org/711664
17:05:08 dansmith gibi: https://review.opendev.org/#/c/705475/4/nova/api/openstack/common.py
17:08:02 openstackgerrit Stephen Finucane proposed openstack/nova stable/stein: Fix hypervisors paginted collection_name. https://review.opendev.org/711669
17:08:48 openstackgerrit Stephen Finucane proposed openstack/nova stable/rocky: Fix hypervisors paginted collection_name. https://review.opendev.org/711670
17:11:59 gibi dansmith: so you mean we need a service version bump even without RPC change.
17:12:29 dansmith gibi: you're depending on a behavior change on the compute nodes from the api node.. even though there's not an RPC signature change, you don't know when/if those computes can do what you're asking
17:14:45 gibi dansmith: yeah. you are right. without the compute side change the port binding won't be updated during unshelve.
17:15:18 dansmith this is really *why* we have service version.. the tie to RPC just makes it easier to ensure we can also look up rpc versions for auto pinning
17:16:40 gibi dansmith: in theory it would be the goal of the scheduler to select a host during unshelve that is capable of supporting the server with qos port
17:17:09 dansmith yeah, you could enforce it there too, but that seems messy-er
17:17:13 gibi dansmith: would it be a compute host capability?
17:17:26 dansmith you'd basically need a new filter or a trait
17:17:42 dansmith we're doing the trait approach for cyborg because it's more than just newness that we're looking for
17:18:04 dansmith so yeah, if you want to go that route that's fine, but it'd be a lot simpler to just do the service version :)
17:18:12 gibi dansmith: another option is global compute service level check in the api
17:18:24 gibi which do seems easier
17:18:33 dansmith yeah, that's what I was suggesting.. gobal check
17:18:49 dansmith meaning, api keeps it disabled until everything is upgraded, which is the more common way we do this kind of thing
17:18:57 gibi OK. I get it. global check it is
17:19:09 dansmith basically replace your "return False" with "return $version >= $minimum"
17:19:37 gibi yepp
17:20:39 gibi regarding microversion bump. I think that ship is sailed when we enabled migrate, resize, evac and live migrate support for qos without microversion bump
17:21:03 gibi it would be equally weird to bump now
17:24:13 dansmith ack, that's why I didn't -1
17:24:18 dansmith I figured there was context I was missing
17:24:26 dansmith and you did say in a comment "if we decide to do so"
17:24:34 dansmith so I figured....you (all) decided
17:25:02 openstackgerrit John Garbutt proposed openstack/nova master: WIP: Enforce unified limits using oslo.limit https://review.opendev.org/615180
17:26:20 gibi dansmith: it was the train PTG where we decided to go for the move support without microversion bumps. I made a mistake not change the error messages to not forcast such bum
17:26:24 gibi p
17:26:32 gibi anyhow I will work on the service level check
17:26:36 gibi thanks for the review
17:26:36 dansmith ack
17:38:42 openstackgerrit Lee Yarwood proposed openstack/nova master: WIP images: Make JSON the default output format of calls to qemu-img info https://review.opendev.org/711679
17:44:50 openstackgerrit melanie witt proposed openstack/nova master: Add logging of ironic driver hash ring to aid in debugging https://review.opendev.org/711680
17:47:38 melwitt dansmith: do you think ^ is a reasonable or did you think lower()ing names in the hash ring is something we should also do?
17:48:07 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add new default roles in os-atttach-inerfaces policies https://review.opendev.org/706672
17:49:18 dansmith melwitt: I think we probably need to lower() the things we put into the hash ring
17:49:36 dansmith potentially checking for conflicts that would only arise due to case might be prudent,
17:49:58 dansmith but I can't imagine differing-only-by-case is anything other than a point of confusion for people
17:50:32 melwitt yeah, that's what I was wondering
17:50:53 melwitt wondering/thinking
17:50:58 melwitt thanks
17:54:21 gmann johnthetubaguy: brinzhang this is issue, you used same policy rule with two different check_str - https://review.opendev.org/#/c/706470/8/nova/policies/instance_actions.py@31
17:55:00 gmann so basically event policy has deprecation of admin_or_owner depends on order it goes in oslo policy
18:18:39 openstackgerrit melanie witt proposed openstack/nova master: Lowercase ironic driver hash ring hosts and add logging https://review.opendev.org/711680
18:19:27 melwitt dansmith: updated ^
18:26:22 melwitt also TheJulia or jroll fyi ^
18:27:00 TheJulia melwitt: I'll take a look after I'm done cooking my wifey some lunch
18:27:09 melwitt heh k
18:29:07 openstackgerrit Ghanshyam Mann proposed openstack/nova master: [Trivial] Fix code comment of admin password tests https://review.opendev.org/711734
19:00:06 jroll melwitt: nice, thanks
19:00:21 melwitt thanks for reviewing!
19:02:19 openstackgerrit Merged openstack/nova master: Name Enums https://review.opendev.org/708707
19:32:01 melwitt thanks for the review TheJulia !
19:32:39 TheJulia So turns out, someone renaming a conductor would also do some... bad things

Earlier   Later