| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-05-26 | |||
| 13:49:57 | superdan | yep, i see | |
| 13:50:13 | superdan | fungi: recede behind the curtain, I'm unblocked and we can discuss more later if needed :) | |
| 13:50:27 | fungi | glad i could help! | |
| 13:51:09 | superdan | yep thanks :) | |
| 13:52:02 | mriedem | if there is one thing i hate, it's needing to mock a context manager | |
| 13:52:28 | leakypipes | mriedem: just one thing, eh? | |
| 13:52:35 | mriedem | that's the only thing i hate | |
| 13:52:40 | mriedem | right this minute | |
| 14:00:22 | alex_xu | sdague: leakypipes yea, I can take a look at the trait log tomorrow, do you have any example? | |
| 14:00:39 | sdague | alex_xu: I was just running tox -e functional locally | |
| 14:00:52 | alex_xu | sdague: ok, got it, i will try | |
| 14:01:02 | sdague | it's warnings in the stream | |
| 14:01:11 | leakypipes | alex_xu: oh, it's all the same warning... just need to use six.text_type() for a few things. | |
| 14:01:41 | alex_xu | leakypipes: ah, got it, I will check that tomorrow | |
| 14:01:55 | leakypipes | coolio. | |
| 14:10:21 | danpawlik | mriedem: Hi. If you have some time can you reply me in my comment: https://review.openstack.org/#/c/459753/ ? Please | |
| 14:16:43 | mriedem | danpawlik: reply how? | |
| 14:19:08 | danpawlik | mriedem: maybe question is not directed to you, but it will be nice to know if we still wait for reply from Claudio or we can focus on merging it ;) | |
| 14:19:23 | mriedem | danpawlik: i understand that you're eager to get this change merged, | |
| 14:19:32 | mriedem | but we have ~72 proposed blueprints for this release, | |
| 14:19:37 | mriedem | and other things going on at the same time, | |
| 14:19:39 | mriedem | so it's on my list | |
| 14:19:43 | mriedem | but not top priority atm | |
| 14:20:04 | melwitt | mriedem: fyi it looks like this patch is working on the same thing you are about cell0 context https://review.openstack.org/#/c/468322 | |
| 14:20:45 | mriedem | melwitt: ah, it's also incomplete | |
| 14:21:00 | melwitt | here's the bug he opened too https://bugs.launchpad.net/nova/+bug/1693721 | |
| 14:21:00 | openstack | Launchpad bug 1693721 in OpenStack Compute (nova) "Error handling for HostMappingNotFound in conductor manager does not work properly" [Undecided,In progress] - Assigned to Takashi NATSUME (natsume-takashi) | |
| 14:21:21 | melwitt | if you want to pick which one to dupe | |
| 14:21:34 | mriedem | i'll dupe his | |
| 14:21:44 | melwitt | k | |
| 14:22:32 | danpawlik | mriedem: Ok, sorry for pushing you. I just want to move forward.I'll be patient | |
| 14:22:32 | mriedem | so given takashin is probably done for the weekend | |
| 14:22:35 | mriedem | and his patch is incomplete | |
| 14:22:41 | mriedem | i'm not sure if i'll just continue working on mine or clean his up | |
| 14:22:45 | mriedem | plus the trove patches are dependent on mine | |
| 14:29:08 | mriedem | sdague: melwitt: superdan: looks like the trove job failed but it's not failing on the bury in cell0 InstanceNotFound thing with my patch http://logs.openstack.org/47/467447/15/check/gate-trove-scenario-dsvm-mariadb-multi-ubuntu-xenial-nv/30d878e/logs/screen-n-cond.txt.gz?level=TRACE | |
| 14:29:27 | mriedem | now i just need to get unit tests fixed up | |
| 14:30:49 | openstackgerrit | Jay Pipes proposed openstack/nova master: integrate trait cache with TraitList and Trait https://review.openstack.org/462770 | |
| 14:31:14 | sdague | mriedem: nice | |
| 14:32:08 | efried | leakypipes Just saw a pop from an hour and a half ago - something about unicode mismatches? | |
| 14:33:07 | leakypipes | efried: ya, it's cool though. alex_xu is gonna do it tomorrow morning. | |
| 14:33:17 | efried | ight | |
| 14:48:08 | efried | sdague Not sure if you saw, but glanceclient plans to cut a release next Wed. (This is regarding global req id work items) | |
| 14:48:22 | sdague | efried: I did not see that | |
| 14:48:41 | sdague | efried: do glance unit tests run for you? | |
| 14:48:46 | efried | You're wanting https://review.openstack.org/#/c/467592/ to get in there? | |
| 14:48:47 | sdague | because they seem to be hanging for me | |
| 14:48:53 | sdague | efried: yes please | |
| 14:48:59 | efried | sdague They did last week. Which env? | |
| 14:49:52 | sdague | py27 | |
| 14:50:59 | efried | sdague oh, glance, not glanceclient. Haven't done that, possibly ever. | |
| 14:51:10 | sdague | I'm trying to run locally and http://paste.openstack.org/show/610745/ | |
| 14:51:22 | sdague | I guess it's all just timing out now | |
| 14:52:23 | efried | sdague It wouldn't surprise me if there are latent bugs; there were in glanceclient. And the last couple weeks have been pretty chaotic around there. | |
| 14:53:48 | efried | I'm pulling & running locally, will let you know what I find. | |
| 14:54:28 | mriedem | \o/ context manager is successfully mocked | |
| 14:57:06 | sdague | mriedem: sweet | |
| 14:57:19 | efried | sdague Okay, I hadn't gotten all the way through my emails - rosmaita called out your patch on the dev ML. | |
| 14:57:30 | openstackgerrit | Jeremy Liu proposed openstack/python-novaclient master: Fix reservation_id not supported by Nova API https://review.openstack.org/468444 | |
| 14:57:49 | sdague | efried: https://review.openstack.org/#/c/468443/ is the thing I'd like to also get in on the glance side, because today the pass just overwrites the real request_id | |
| 14:59:13 | efried | sdague Okay. (In case there's been some confusion, I'm not a glance guy - just jumped in there for the first time last week to get service-tokens work moving ;) | |
| 15:00:14 | mriedem | efried: you are now a glance guy | |
| 15:00:15 | mriedem | deal with it | |
| 15:00:20 | sdague | mriedem: ++ | |
| 15:00:25 | efried | dangit | |
| 15:07:27 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use targeted context when burying instances in cell0 https://review.openstack.org/468401 | |
| 15:07:28 | mriedem | sdague: melwitt: superdan: ^ now with tests | |
| 15:08:05 | mriedem | https://review.openstack.org/#/c/467447/ passed with that | |
| 15:12:11 | melwitt | mriedem: none of those tests repro the original problem, where the instance can't be deleted, do they? | |
| 15:13:32 | superdan | because they're not actually using the full cell fixture right? | |
| 15:14:16 | mriedem | they aren't using the cells fixtures no | |
| 15:14:24 | mriedem | well, | |
| 15:14:37 | mriedem | they would be via the base test case | |
| 15:15:12 | mriedem | if you want a full regression suite, that's probably a functional test, and probably the rest of my day | |
| 15:16:01 | mriedem | i'm not actually sure why the TestDeleteFromCell0CheckQuota functional test never caught this | |
| 15:16:13 | mriedem | since it's simulating a case where the instance fails to schedule and goes into ERROR state, and goes into cell0 | |
| 15:17:13 | mriedem | could have to do with not actually doing notifications in tests | |
| 15:17:14 | mriedem | checking | |
| 15:17:23 | melwitt | okay, just thinking about how this got past tests in the first place | |
| 15:18:16 | sdague | mriedem: any chance that it wasn't magicing the db enough? | |
| 15:18:26 | melwitt | because the problem is the InstanceMapping is pointing at a NULL cell_id | |
| 15:19:11 | melwitt | this is where the issue is right? https://review.openstack.org/#/c/468401/3/nova/conductor/manager.py@893 maybe we could at least assert the right context in that call in unit tests | |
| 15:19:36 | mriedem | melwitt: that's why the unit test changes do | |
| 15:19:52 | melwitt | I thought those were only changes about notifications | |
| 15:20:16 | efried | sdague FYI glance py35: Ran 625 tests in 1216.144s \ OK. py27 cranking up now. | |
| 15:20:17 | mriedem | it doesn't blow up there, it's before that, | |
| 15:20:48 | mriedem | maybe you're asking that the tests also assert InstanceMapping.get_by_instance_uuid | |
| 15:20:58 | mriedem | which is probably raising NotFound and we pass | |
| 15:21:43 | melwitt | maybe. I trust the fix, just saying none of the test changes are covering it. we can add coverage as a follow up if this is urgent | |
| 15:21:54 | mriedem | superdan: so i'm guessing the reason this didn't fail functional tests like TestDeleteFromCell0CheckQuota is because the CellDatabases fixture is patching target_cell? | |
| 15:22:10 | superdan | melwitt: no, the set_vm_state thing does actual changes to the instance right? | |
| 15:22:15 | sdague | melwitt: on the up side we have real tests in trove that at least expose it | |
| 15:22:23 | sdague | which is how we got here | |
| 15:22:55 | superdan | mriedem: I dunno, I need to look at that test.. are you using CellDatabases or SingleCellSimple there? | |
| 15:23:11 | mriedem | CellDatabases since USES_DB=True | |
| 15:23:18 | superdan | yeah okay | |
| 15:24:01 | mriedem | superdan: so for your question, | |
| 15:24:04 | mriedem | i asked the same thing in your series https://review.openstack.org/#/c/458634/13/nova/objects/host_mapping.py@227 | |
| 15:24:16 | melwitt | superdan: oh, yeah. set_vm_state makes the changes to the instance, so that explains the wonky states from the bug | |
| 15:24:39 | melwitt | but the failed deletion part is that the InstanceMapping isn't pointing at the cell where the instance is | |