Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-10-23
15:02:11 mriedem yup
15:02:21 dansmith meh this doesn't seem worth fixing
15:02:34 dansmith hah
15:02:59 dansmith how has this not been a thing before? like, doesn't this break almost everyone's pagination?
15:03:39 mriedem i'm surprised it hasn't come up earlier, so idk,
15:03:54 mriedem it's a combination of using that --limit -1 semantic in the CLI and filtering on --deleted servers
15:04:27 mriedem maybe admins don't use --limit -1 often, idk
15:04:51 dansmith oh right right, forgot about the deleted thing
15:06:43 openstackgerrit Matt Riedemann proposed openstack/nova master: doc: note the need to configure cinder auth in reclaim_instance_interval https://review.opendev.org/690602
15:17:04 openstackgerrit Adrian Chiris proposed openstack/nova stable/train: Revert "vif: Resolve a TODO and update another" https://review.opendev.org/690126
15:25:47 mriedem gibi_off: looks like we need https://review.opendev.org/#/c/687550/ updated to keep that series moving,
15:25:54 mriedem elod: ^ if gibi is out maybe you can do that?
15:26:31 mriedem doesn't have to be today since i know it's late in the day there, but would be good to wrap that up tomorrow
15:28:15 openstackgerrit Matt Riedemann proposed openstack/nova stable/train: Revert "vif: Resolve a TODO and update another" https://review.opendev.org/690126
15:30:11 mriedem need another stable core on ^, that's a fix for a regression in train
15:30:11 openstackgerrit Merged openstack/python-novaclient master: Switch to Ussuri jobs https://review.opendev.org/690013
15:30:43 mriedem efried: you'll like this https://review.opendev.org/#/q/owner:kangyufei+status:open
15:34:32 mriedem lyarwood: the stein version of this is merged now https://review.opendev.org/#/c/686292/
15:35:11 lyarwood mriedem: ack thanks,
15:35:24 mriedem dansmith: mind hitting this in stable/rocky? https://review.opendev.org/#/c/686276/
15:36:14 dansmith yar
15:57:59 openstackgerrit Matt Riedemann proposed openstack/nova stable/queens: rt: only map compute node if we created it https://review.opendev.org/676285
16:02:44 lyarwood mriedem: https://review.opendev.org/#/q/topic:bug/1843639 - would you mind taking a swing at this if you have time this week btw? The functional test might be a little over the top for a chance like this but it felt like the best way of verifying this at the time.
16:02:55 lyarwood change*
16:05:26 mriedem i see that depends on https://review.opendev.org/#/c/686832/ which has had a -1 sitting for awhile
16:05:30 mriedem is that going to be addressed?
16:06:44 lyarwood mriedem: I was going to talk to artom about that today once he's around
16:06:56 lyarwood mriedem: I'm happy to address it if he doesn't have time
16:11:55 mriedem lyarwood: ok i'll get out the red pen for your functional test
16:12:53 lyarwood thanks
16:13:31 mriedem man there is a lot of mocking in this func test
16:13:34 mriedem usually a red flag
16:16:20 lyarwood yeah given I'm trying to get to post_live_migration within the Libvirt driver itself I had to mock out the initial checks and migration itself
16:17:06 mriedem i'm not sure the functional test is worth it honestly,
16:17:14 mriedem especially given the simple nature of the fix in the driver
16:18:12 mriedem trying to backport that functional test is also going to be a nightmare
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

Earlier   Later