Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-06
14:18:56 melwitt oh yeah, good memory
14:19:37 melwitt https://review.opendev.org/528036
14:19:50 mriedem melwitt: i'm pretty sure task log is related to https://docs.openstack.org/api-ref/compute/#server-usage-audit-log-os-instance-usage-audit-log which is related to ceilometer which no one uses
14:19:57 mriedem i'd like to burn out all of that code
14:20:35 melwitt thanks. I was surprised in all my time working on nova, I've never heard of the task_log table before now
14:21:04 melwitt so I'd support burning the code that no one uses if indeed no one uses it
14:21:08 efried dtantsur: I looked briefly at the ironic CI job to see why it didn't hit that bug; I think it's just because those methods are used in the course of specific operations that tempest doesn't cover. We should add that, for sure. I'm not completely sure how to get started with that though.
14:21:30 dtantsur yeah
14:21:39 dtantsur the 2nd bug is hit when you have >1 nodes
14:21:47 mriedem rackspace probably had internal scripts or something to purge task log
14:22:02 mriedem melwitt: well clearly your customer is using it
14:22:09 cdent thanks melwitt, I think that corresponds to something I was looking at when poking at the problem, but doesn't address the issue with the exception name being included in the log even though it was "elsewhere". I'm inclined to ignore it if nobody is complaining, since it gets better with python3. I only noticed becasue somebody asked why there was a "NotImplemted" at the end of rp generation conflict message
14:22:46 melwitt mriedem: oh, you think table grow means they are using the audit APIs? I had assumed task_log entries get added regardless (like on any instance action)
14:22:49 openstackgerrit Dmitry Tantsur proposed openstack/nova master: Fix incorrect invocation of openstacksdk's baremetal.nodes() https://review.opendev.org/680684
14:22:49 dtantsur mriedem: better ^^?
14:23:15 alex_xu mriedem: I'm talking to luyao, we should stop the instance booting if there is no allocation, since vgpu and vpmem should depend on allocation
14:23:22 melwitt I need to check
14:23:25 mriedem melwitt: it's a periodic task in the compute service which you have to enable, and it's disabled by default
14:23:32 mriedem so if they have task log table entries they are configured to run that task
14:23:34 melwitt oh, dang
14:23:42 mriedem https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.instance_usage_audit
14:23:56 mriedem and they are probably using ceilometer to hit that compute api
14:24:03 mriedem if they aren't, then they should disable the task and purge the db
14:24:03 melwitt thanks. I shouldn't have assumed
14:24:13 mriedem please see topic new contributor :P
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

Earlier   Later