Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-17
21:23:19 efried I'm sayin, are we calling it from API even if we don't require it?
21:23:22 mriedem and i'm trying to make those graceful for upgrades
21:23:36 dansmith I feel like we'd start or stop talking to cinder or neutron from api without thinking about it
21:23:56 dansmith but also,
21:24:08 dansmith I don't think we have to have safe_connect at the lowest layer just because api wants it
21:24:30 mriedem i agree with the last thing you said
21:24:31 dansmith api can do the placement mirroring stuff and except Exception
21:24:52 dansmith or we can move safe_connect to a context manager and you can do:
21:25:00 dansmith with safe_placement_stuff: do_things;
21:25:02 mriedem ^ is what i'm thinking
21:25:16 dansmith and it will catch and ignore or log or whatever is appropriate for up there
21:26:19 efried dansmith: Would you use that context manager from RT or from methods within report client?
21:26:40 dansmith you'd use it in the api methods getting ready to talk to placement
21:26:53 dansmith or anywhere else that still wants safe_connect behavior
21:27:17 dansmith I would expect report client to catch things it needs, bubble unexpected things (like this auth thing) up to the caller
21:27:33 dansmith or translate them into something like PlacementNoWorky or whatever
21:27:53 efried okay. Cause I think the major hole with what we're doing now is that we're calling it from within report client, often in ways that may cause us to be ignoring an exception on step 3 of a 10-step process and then proceeding with the rest.
21:28:23 dansmith right
21:28:24 dansmith which is why we shouldn't have it underneath report client
21:28:28 efried ++
21:28:42 efried unless it's doing a reconnect-and-retry thing.
21:28:47 dansmith sure
21:28:56 cfriesen mriedem: I'm confused why it's not failing...it was trivially easy to reproduce in devstack by adding a single row to the services table.
21:29:09 efried In which case it should probably decorate the get/put/post/delete primitives and nothing else.
21:33:36 lyarwood Does anyone have a simple way to enable debug logging within functional tests? self.flags doesn't appear to work.
21:34:53 mriedem OS_DEBUG=True
21:35:14 mriedem https://github.com/openstack/nova/blob/ad249d8994067b1c255823cb7c1ff4c32593e5ef/tox.ini#L24
21:35:36 lyarwood thanks! :)
21:43:39 cfriesen mriedem: when I run your testcase locally it's failing in _migrate_server()
21:44:57 cfriesen http://paste.openstack.org/show/719417/
21:45:16 mriedem cfriesen: in pike?
21:45:53 mriedem because pike doesn't have 204 in this list https://github.com/openstack/nova/blob/87ea686f9f2cc706205d188922bb14272625e7be/nova/tests/functional/api/client.py#L223
21:46:04 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Make safe_connect retry or raise https://review.openstack.org/562060
21:46:09 efried dansmith, mriedem: ^
21:46:48 efried dansmith, mriedem: There would be a separate, unrelated thingy for the n-api side of the world.
21:46:55 cfriesen mriedem: yes, in pike
21:48:13 cfriesen retrying with that
21:49:59 cfriesen is there a way to stick a pdb breakpoint in there right before the last line?
21:52:35 efried cfriesen: Are you asking how to pdb via stestr?
21:52:56 efried cause I have that answer
21:53:22 cfriesen efried: I want to pdb in a functional test
21:53:41 efried cfriesen: Same as in a unit test.
21:53:54 efried cfriesen: You have to run stestr directly, and with the -n option and the full name of the test case.
21:54:13 efried or you can use remote_pdb
21:54:44 openstackgerrit Jackie Truong proposed openstack/python-novaclient master: Microversion 2.62 - Add trusted_image_certificates https://review.openstack.org/500396
21:55:54 efried stestr run -n nova.tests.functional.regressions.test_bug_1746509.InstanceListWithServicesTestCase.test_instance_list_deleted_service_with_no_uuid
21:55:54 efried source .tox/func/bin/activate
21:55:54 efried cfriesen: e.g.
21:56:11 efried s/func/functional/ I guess - whatever the venv name is for functional.
21:59:59 cfriesen efried: thanks, running ostestr directly worked
22:00:20 efried cfriesen: ostestr? Are you not in nova? But cool.
22:00:43 cfriesen efried: nova, but stable/pike. it's what tox was using
22:00:46 efried ahh
22:01:08 cfriesen mriedem: running db.service_get_all_by_binary() righte before the call to self.admin_api.get_servers() shows that both services have uuid entries
22:01:27 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix docs for confirmResize action https://review.openstack.org/562062
22:01:59 mriedem cfriesen: yes, which they both will for the non-deleted services
22:02:13 mriedem because host1.start() creates a new service record with a uuid
22:02:25 mriedem simulating that you've upgraded to pike and started that compute host with pike code
22:02:38 mriedem after deleting the "newton" compute service / node which didn't have the uuid set
22:03:53 cfriesen mriedem: right, I need the mutated context to read the deleted one.
22:04:20 openstackgerrit Jackie Truong proposed openstack/python-novaclient master: Microversion 2.62 - Add trusted_image_certificates https://review.openstack.org/500396
22:06:26 mriedem cfriesen: only other thing i could think of is like melwitt was saying yesterday and the backref joinedload on the services table is getting lucky and pulling the non-deleted service when listing instances
22:06:49 mriedem this https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/models.py#L103
22:07:03 cfriesen mriedem: the deleted service in the DB has a uuid
22:07:30 melwitt mriedem, cfriesen: I was wrong about that. the instance.services is all services whose host match the instance, and it will try to add a UUID to both of them, so should reliably fail I think
22:08:46 mriedem cfriesen: ok i don't know how that is per https://review.openstack.org/#/c/562041/2/nova/tests/functional/regressions/test_bug_1764556.py@149
22:09:09 mriedem cfriesen: i guess there needs to be an assertion after the 2nd _migrate_server that the deleted service still doesn't have a uuid
22:10:48 cfriesen mriedem: if I set the uuid to None right before the final line then it fails
22:12:29 melwitt something in the migrate is loading instance.services and generating UUID I guess?
22:12:44 cfriesen seems like...I'm guessing wait_for_state_change()
22:15:39 mriedem idk, at what point is the test mocked out so hard that it masks whether or not we have a bug?
22:15:45 mriedem or that i trust the recreate of it?
22:17:30 mriedem here are some updates, but i've got to run - the 2nd migrate is indeed migrating the deleted service record
22:17:30 openstackgerrit Matt Riedemann proposed openstack/nova master: DNM: Revert "Lazy-load instance attributes with read_deleted=yes" https://review.openstack.org/562051
22:17:30 openstack bug 1764556 in OpenStack Compute (nova) ""nova list" fails with exception.ServiceNotFound if service is deleted and has no UUID" [Undecided,New] https://launchpad.net/bugs/1764556
22:17:30 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Add functional recreate test for bug 1764556 https://review.openstack.org/562041
22:18:21 melwitt yeah, wouldn't want to mock it. I guess have to determine what exactly is the func test doing differently than the devstack repro and see if we make it the same as the actions needed with devstack
22:19:15 cfriesen melwitt: I wonder if we need to do the delete/migrate/recreate/migrate first and then do the upgrade to pike
22:19:27 cfriesen so that the migrate happens without uuids on the service
22:20:24 SamYaple looking for love in getting the +3 to get this backport merged. https://review.openstack.org/#/c/560088/
22:21:07 melwitt looking for love in all the wrong places
22:21:27 melwitt dansmith might be able to help you with the backport review ^
22:21:56 SamYaple story of my life melwitt
22:22:02 melwitt heh
22:22:05 cfriesen melwitt: guess not, my bug report specifically says they added the new compute was added after the upgrade
22:22:38 melwitt cfriesen: yeah, I don't yet understand how we're simulating an upgrade in the func test, that's probably what's messing it up
22:22:52 melwitt (I haven't looked in detail because I'm mired in other stuff right now)
22:23:01 cfriesen melwitt: we're simulating the upgrade by just deleting the uuid on the service
22:23:12 melwitt k, that makes sense
22:23:57 cfriesen melwitt: mriedem_away: I suspect I know what's going on....initially we didn't have the other two fixes from stable/pike so I suspect that the migration after the upgrade wouldn't have updated the UUID, thus leaving the DB in the wonky state.
22:24:25 cfriesen melwitt: mriedem_away: I bet that upgrading to stable/pike probably *would* update the service UUID on the migration.
22:25:09 melwitt cfriesen: but earlier didn't you recreate the bug in devstack with the two fixes already applied?
22:25:41 openstackgerrit Merged openstack/nova master: Remove redundant _do_check_can_live_migrate_destination https://review.openstack.org/561948
22:26:18 cfriesen melwitt: yes, by manually editing the DB to make an entry without the UUID. and then trying to list servers. I didn't do the migration step.
22:26:30 melwitt ah
22:26:31 cfriesen melwitt: I suspect that the migration step would update the UUID.
22:26:39 melwitt I see
22:27:05 cfriesen if I'm right, then anyone upgrading to current stable/pike wouldn't see this
22:27:13 melwitt yeah
22:34:28 dansmith melwitt: I think SamYaple means "dansmith mergin' my stuff in a timely manner is the story of my life"

Earlier   Later