Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-10-23
16:18:17 lyarwood yeah true, I started writing this while triaging the original downstream bug as a way of proving an exception from post_live_migration would leave the instance in an ERROR state pointing at the wrong host
16:18:18 mriedem well, at least a bad dream
16:18:59 mriedem that's possible without using and mocking the shit out of the libvirt driver here,
16:19:21 mriedem i.e. you could have a much much simpler functional test that just stubs the driver.post_live_migration method to raise some exception
16:19:30 mriedem bingo bango you cut out 80% of the gross of that test
16:20:03 lyarwood well that's one mock gone
16:20:08 efried mriedem: somebody had to do it :P
16:20:30 lyarwood okay maybe two
16:21:31 lyarwood that wouldn't even work anyway
16:21:42 mriedem lyarwood: i left a comment
16:21:45 lyarwood the pre migration checks are a PITA to workaround in a functional test sense
16:21:57 mriedem we have functional tests with the fake driver that all handle live migratoin
16:22:07 mriedem the only thing you have to do is stub out driver.post_live_migration to raise processutils.ProcessExecutionError
16:22:36 mriedem so if you made this much simpler and more generic i'd say cool with the functional test - and it removes your dep on artom's change - otherwise i'd say drop it
16:22:49 lyarwood that's only useful if I moved the fix up into the compute layer and out of the Libvirt driver
16:23:21 mriedem true, in the libvirt driver fix you'd have to just remove the stub which would be dumb
16:23:35 lyarwood I don't mind going that way
16:24:03 lyarwood but I was worried that other virt drivers might not want that behaviour tbh
16:26:20 mriedem i'd just drop the functional test - you've at least proven the issue with it so we can be comfortable with the driver fix on top
16:26:49 lyarwood yup that's fine, thanks for the review!
16:27:08 mriedem and now artom can go back to watching antiques roadshow
16:39:00 openstackgerrit Lee Yarwood proposed openstack/nova master: libvirt: Ignore volume exceptions during post_live_migration https://review.opendev.org/682622
16:40:51 melwitt mriedem: I dunno if you saw, but I proposed a devstack change to fix the vnc + tls thing https://review.opendev.org/690469
16:42:38 KeithMnemonic mriedem, would you expect --deleted --marker to work or is --marker only for active instances
16:43:03 mriedem melwitt: ooo cool
16:43:17 mriedem KeithMnemonic: i would expect it to work
16:43:36 mriedem KeithMnemonic: if you have 20 deleted instances and limit to 10 per page, then you'd get a marker
16:44:35 KeithMnemonic can you look at this., this is with your latest patch http://paste.openstack.org/show/785528/
16:46:06 KeithMnemonic even with --limit -1 you get the same message
16:46:30 mriedem hmm, we do look up deleted instances for markers so it's not that https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2283
16:46:59 mriedem which cell is c244e139-98fe-44eb-a971-c067954da026 in ?
16:47:03 mriedem cell1 or cell0?
16:47:16 KeithMnemonic for not deleted it works as expected. let me check one sec
16:49:15 openstackgerrit Matt Riedemann proposed openstack/nova master: Make nova-next multinode and drop tempest-slow-py3 https://review.opendev.org/683988
16:51:35 KeithMnemonic cell1 it seems
16:52:05 openstackgerrit Matt Riedemann proposed openstack/nova master: Make nova-next multinode and drop tempest-slow-py3 https://review.opendev.org/683988
16:52:52 mriedem hmm, i wonder if there is something going on with the multi-cell instance listing stuff that has some marker logic in it
16:53:08 KeithMnemonic let me try it with a market in cell0
16:53:13 KeithMnemonic i have one instance there
16:53:16 melwitt mriedem: uh oh https://github.com/openstack/nova/blob/master/nova/compute/instance_list.py#L59
16:53:34 melwitt oh nevermind
16:53:36 mriedem was just looking at that code,
16:53:41 mriedem but yeah a deleted instance will still have a mapping
16:53:43 mriedem until archived
16:53:43 melwitt there should still be an instance mapping? or no
16:53:45 melwitt yeah
16:53:52 mriedem and https://github.com/openstack/nova/blob/master/nova/compute/instance_list.py#L63
16:53:59 mriedem matches the db api to read deleted markers
16:54:05 melwitt yeah hm
16:54:58 KeithMnemonic nope same error with a deleted instance in cell0,
16:56:15 mriedem ok when i've had to debug these types of failures before i've needed to add a LOG.exception here in the API https://github.com/openstack/nova/blob/2718de6ed7c21f8ff8cf74164ae5054531fdbc30/nova/api/openstack/compute/servers.py#L331 to get a traceback
16:56:23 KeithMnemonic but just --deleted --limit -1 shows deleted from cell0 and cell1 so it is definitely something with the marker
16:56:25 mriedem KeithMnemonic: is that something you can do and then recreate to get the trace?
16:56:47 KeithMnemonic so this is interesting
16:57:01 KeithMnemonic "https://192.168.23.35:8774/v2.1/428982d4248a419a933668b6a4dd14a0/servers?name=c244e139-98fe-44eb-a971-c067954da026" will this work without deleted being passed?
16:57:18 melwitt I'm wondering what this get_marker_by_values is
16:57:28 mriedem KeithMnemonic: that's not a server name
16:57:43 KeithMnemonic that is what --debug shows
16:57:45 mriedem i'd expect you to get no results in that case
16:58:37 mriedem KeithMnemonic: hmm, ok so it's probably the cli trying to determine if the marker provided is a server name or id
16:58:46 KeithMnemonic i pass in --marker c244e139-98fe-44eb-a971-c067954da026
16:58:57 KeithMnemonic no option to specify name vs ID
16:58:59 mriedem --marker <server>¶ The last server of the previous page. Display list of servers after marker. Display all servers if not specified. (name or ID)
16:59:56 mriedem that might be a red herring if osc is just trying to determine if you gave a name or id
17:00:28 mriedem try to recreate with the LOG.exception injection i mentioned above to see where the MarkerNotFound is coming from
17:00:43 mriedem you know, we might not even be getting to the api
17:00:57 mriedem "No server with a name or ID of 'c244e139-98fe-44eb-a971-c067954da026' exists." sounds like an osc bug
17:01:06 mriedem ok i bet i know,
17:01:13 mriedem the osc code is trying to find the marker server by name or id,
17:01:19 mriedem but failing to include the deleted=True filter,
17:01:32 mriedem so the api says "nope a server that's not deleted does not exist for that"
17:01:38 KeithMnemonic but should it worked without specifying deleted in the api call?
17:02:10 mriedem meaning can you specify a marker that is a deleted server? yes
17:02:22 mriedem it seems the bug is in osc though
17:03:03 mriedem https://github.com/openstack/python-openstackclient/blob/4.0.0/openstackclient/compute/v2/server.py#L1415
17:03:09 melwitt wait, how do we know it's not including the deleted=True filter? it should show that in the --debug right
17:03:11 mriedem osc isn't looking for a deleted marker
17:03:44 KeithMnemonic it is osc, you can not do a show on a deleted sever, you get the same curl and same error
17:03:45 melwitt I mean, when you pass --deleted doesn't it just send deleted=True regardless? or no
17:03:45 mriedem https://192.168.23.35:8774/v2.1/428982d4248a419a933668b6a4dd14a0/servers?name=c244e139-98fe-44eb-a971-c067954da026 means osc is first looking to see if the marker server is given on the command line by name
17:03:50 melwitt oh
17:03:52 mriedem it's not, it gets a 404 and then tries by id,
17:04:08 mriedem which returns a 404 because the server is deleted and you're not including the ?deleted=true filter param
17:04:13 KeithMnemonic correct
17:04:23 KeithMnemonic but osc is called with --deleted
17:04:24 mriedem so it's a shell bug
17:04:30 KeithMnemonic it is just not passed to the api call
17:04:35 mriedem right, it's a bug
17:04:42 KeithMnemonic ok against osc
17:04:44 mriedem yes
17:05:17 KeithMnemonic i can file another today. i will reference this discussion and some debug output.
17:05:20 KeithMnemonic is that ok
17:05:31 mriedem sure
17:05:36 mriedem that goes against osc in storyboard though
17:05:52 mriedem where it will languish for 1000 years
17:06:26 KeithMnemonic ok let me test on something newer to see if it still exists is train for example
17:06:34 mriedem that code i linked above is train
17:06:38 mriedem so it's there
17:07:35 mriedem https://github.com/openstack/osc-lib/blob/1.14.1/osc_lib/utils/__init__.py#L169
17:07:49 mriedem looks like the fix is relatively simple:

Earlier   Later