Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-06
14:24:24 melwitt :(
14:25:06 mriedem alex_xu: yeah that's probably fine, there is no real contract on support for out of tree scheduler drivers
14:25:34 mriedem alex_xu: see https://opendev.org/openstack/nova/src/branch/master/nova/scheduler/driver.py#L35
14:27:41 efried dansmith: risk on https://review.opendev.org/#/c/677736/ I really have no idea I'm afraid. If cdent thinks it's okay to merge-and-watch, that's good enough for me. Remember FF is FF so we have a little bit of time before the release to flush out issues; so I don't think current proximity to FF is a reason to block it.
14:28:33 dansmith efried: ack
14:33:36 mriedem dansmith: before artom moves forward with this rpc/object change in the numa lm series https://review.opendev.org/#/c/634827/57//COMMIT_MSG@12 might want to see if you are on board
14:33:56 mriedem i'm basically asking that we're consistent in the flags saying both source and dest are ready for action
14:33:57 alex_xu mriedem: i see now, thanks
14:34:05 dansmith mriedem: ack looking
14:34:15 artom mriedem, actually yeah, thanks. I think it should be fine, but dansmith's lack of veto would boost confidence
14:35:16 dansmith artom: mriedem you're thinking set that field on the dest using the same can-do-numa thing in rpcapi? because without that, the only way to know is to pass a new param and have it *not* show up at the other end
14:35:34 dansmith we resolved that first, and then artom added the can-do check to rpcapi for the source, who has nobody to call
14:35:48 dansmith so if you're saying make the source and dest both call that, and set their corresponding field, yeah I guess that's okay
14:37:22 artom dansmith, not quite - I'm saying the dest doesn't need to actually send RPC 5.3, so check_can_live_migrate_source doesn't need that numa_live_migraiton flag, the dest can just shove it in migrate_data
14:38:17 aspiers Just asked in #openstack-qa but figured worth asking here too: Is it considered good/bad practice for tempest tests to check the message string of nova exceptions returned by assertRaises?
14:38:37 dansmith artom: but the dest needs to support the 5.3 inbound call, but you're saying actually report the conductor side? meaning the did-i-get-the-migration-from-conductor flag?
14:38:59 aspiers I'm thinking that if we wanted to reword an exception's message string, this could break tempest, and then you'd need a circular Depends-On across repos to fix it
14:39:10 artom dansmith, right, the dest needs to receive 5.3 - specifically migration and limits form the conductor
14:39:20 artom So if gets those, it can set the flag inside migrate_data
14:39:24 dansmith artom: and the new drop call
14:39:27 dansmith from source
14:39:31 artom dansmith, yeah
14:39:35 artom No, not from source
14:39:40 artom The source still needs to send 5.3
14:39:43 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix incorrect invocation of openstacksdk's baremetal.nodes() https://review.opendev.org/680684
14:39:51 mriedem dtantsur: +2 now ^
14:39:52 mriedem efried: ^
14:39:53 dansmith artom: the source calls the new drop-claim against the dest right?
14:40:02 artom dansmith, yeah
14:40:04 artom So if the dest sets the flag in migrate_data, these things could happen on the source:
14:40:09 dansmith artom: that's what I mean by inbound 5.3
14:40:32 dansmith anyway, yeah I think that's okay
14:41:11 mriedem aspiers: error messages in the api aren't a contract
14:41:20 mriedem only error codes like placement uses, but nova doesn't
14:42:00 mriedem dansmith: yeah basically remove the numa_live_migration flag that goes from dest to source and instead put that as an attribute in migrate_data instead
14:42:01 efried dustinc: I'm merging https://review.opendev.org/#/c/680684/ but please have a look at it when you get in.
14:42:03 mriedem for consistency
14:42:04 artom dansmith, right, dest needs to receive 5.3
14:42:12 artom But nothing needs to actually send it
14:42:15 aspiers mriedem: OK thanks, so I'll just check for HTTPBadRequest and leave it there
14:42:19 efried mriedem, dtantsur: +A
14:42:23 artom So if it sets the flag in migrate_data, we know it's the new code that can receive 5.3
14:42:26 artom And then on the source:
14:42:37 artom Either it's old code and that flag gets dropped by the conductor
14:42:47 dansmith mriedem: right I was just confirming how we calculate that because right now the flag making it through the dest's rpcapi to the source is the sentinel
14:43:06 artom Either it's new code but pinned, in which case the can_send check will fail
14:43:15 artom Or it's new code unpinned, and we proceed with the NUMA LM
14:44:03 mriedem dansmith: yup
14:44:59 mriedem i didn't want to start a pattern of putting random feature flags on those compute rpc api methods, like sev=True, vpmems=True, suchandsuch=True
14:45:56 dansmith mriedem: yeah, but we needed it to figure out if the dest could *send* 5.3, but that's actually not a thing we need to know
14:46:11 dansmith and now we have that can-do method to tell us without actually trying to make the call
14:46:34 artom dansmith, that's the thing, I don't think dest actually needs to send 5.3
14:46:42 artom The changes are all in objects
14:46:45 artom From its POV
14:48:07 dansmith artom: no it doesn't, I'm agreeing
14:48:31 dansmith artom: I think we just kinda munged the "does it support" with "is it configured to allow sending" ... 5.3 when we first discussed it
14:48:40 artom dansmith, yep
14:48:54 dansmith if we were adding new calls on both sides, they'd both need to be configured for 5.3, but in this case, we can cheat
14:50:56 artom dansmith, ack, looks like we're all on the same page, I'll continue down that route and with the rest of mriedem's feedback after the bug call
14:51:07 dansmith cool
14:52:26 mriedem stephenfin: alex_xu: yonglihe: on that topology cli change, this is wrong, correct? https://review.opendev.org/#/c/670790/7/doc/source/cli/nova.rst@3663
14:53:04 mriedem compute:server:topology:host:index is admin-only by default and used to expose host fields
14:54:12 mriedem https://review.opendev.org/#/c/621476/62/nova/api/openstack/compute/server_topology.py@58
14:54:14 mriedem yeah looks like it
14:56:17 donnyd sean-k-mooney: looks like we are still getting a node failure
14:58:06 stephenfin looking
14:59:55 openstackgerrit Balazs Gibizer proposed openstack/nova master: Add min service level check for migrate with bandwidth https://review.opendev.org/680394
15:01:13 stephenfin mriedem: Yup, good catch
15:01:14 mriedem efried: i'm +2 on the other ironic fix https://review.opendev.org/#/c/680542/ but we can wait until the test patch passes https://review.opendev.org/#/c/680643/ - given the state of the gate that might be sometime this weekend though, so maybe you want to +2 and one of us can approve once we have results?
15:01:58 sean-k-mooney donnyd: it might be because its requesting two vms getting one and timeing out getting the second?
15:02:47 mriedem actually that ironic patch already got test nodes, it must be nice to not be nova
15:02:49 efried mriedem: I was thinking along the same lines. But iiuc dtantsur followed up later and said the grenade fix wasn't actually related to this bug. We (still) don't have CI coverage that will hit it. I just proposed https://review.opendev.org/680718 in tripleo which will hopefully hit the job that originally uncovered the problem.
15:02:58 openstackgerrit Balazs Gibizer proposed openstack/nova master: migrate: Add bw min service level check of source compute https://review.opendev.org/680395
15:03:06 efried s/grenade fix/grenade failure/
15:03:38 mriedem alright then
15:03:40 mriedem moving on
15:04:05 efried mriedem: but given how simple and obviously-right the fix is, and that dtantsur says it is legit, I think we might want to merge it anyway.
15:04:05 sean-k-mooney donnyd: ill check it at the weekend agian when the ci does not have as high a load
15:04:13 dtantsur efried: I'd make your tripleo patch depending on both fixes. I'm not sure which one of them is most breaking.
15:04:22 mriedem efried: i'm good with that
15:04:34 mriedem i definitely trust dtantsur here and these are simple fixes
15:04:39 efried dtantsur: the symptoms would be different
15:06:00 openstackgerrit Balazs Gibizer proposed openstack/nova master: resize: Add bw min service level check of source compute https://review.opendev.org/680396
15:06:00 dtantsur okie
15:08:32 efried dtantsur, mriedem: +A
15:08:53 dtantsur now let's see if you'll regret trusting me :D
15:09:01 openstackgerrit Balazs Gibizer proposed openstack/nova master: prepare func test env for moving servers with bandwidth https://review.opendev.org/655109
15:09:18 efried It's not clear that the tripleo patch will work as is; I don't understand the job chain that would get us there, or whether nova needs to be added to required_projects *somewhere* (but where??). So it may take a few iterations to figure that out.
15:10:43 mriedem dtantsur: please don't break my achy breaky heart
15:11:13 dtantsur okay
15:11:21 mriedem i'm assuming you get early 90s billy ray cyrus references of course
15:11:58 openstackgerrit Balazs Gibizer proposed openstack/nova master: Func test for migrate server with ports having resource request https://review.opendev.org/655113
15:14:58 openstackgerrit Balazs Gibizer proposed openstack/nova master: Make _rever_allocation nested allocation aware https://review.opendev.org/676138
15:18:01 openstackgerrit Balazs Gibizer proposed openstack/nova master: Support reverting migration / resize with bandwidth https://review.opendev.org/676140
15:18:57 gibi mriedem, bauzas: the next 5 patches in the bw series is up-to-date. first patch: https://review.opendev.org/#/c/680394/ last up-to-date patch: https://review.opendev.org/#/c/655113
15:19:09 mriedem gibi: yup, on my list for today
15:20:53 gibi mriedem: thanks. I still have to work on comments in th second half of the patch series and also need to do a follow up for the already merged https://review.opendev.org/#/c/656422
15:21:00 openstackgerrit Balazs Gibizer proposed openstack/nova master: Func test for migrate re-schedule with bandwidth https://review.opendev.org/676972
15:24:03 openstackgerrit Balazs Gibizer proposed openstack/nova master: Support migrating SRIOV port with bandwidth https://review.opendev.org/676980
15:25:11 mriedem stephenfin: is it not possible to deprecate an entire group of config options?

Earlier   Later