| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-09 | |||
| 16:37:15 | sean-k-mooney | and ya im free ill ask him | |
| 16:37:19 | jaypipes | cool. | |
| 16:38:20 | ralonsoh | jaypipes, sean-k-mooney: hi, now I'm free | |
| 16:39:14 | jaypipes | sean-k-mooney, sfinucan, ralonsoh: https://hangouts.google.com/call/qqze37pjs5f4lnatg7owxiaklqe | |
| 16:41:49 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add regression test for bug 1670627 https://review.openstack.org/443745 | |
| 16:41:49 | openstack | bug 1670627 in OpenStack Compute (nova) "quota is always in-use after delete the ERROR instances " [Critical,In progress] https://launchpad.net/bugs/1670627 - Assigned to Dan Smith (danms) | |
| 16:41:49 | mriedem | dansmith: melwitt: Kevin_Zheng: ^ this is the regression test i was thinking of | |
| 16:42:41 | dansmith | mriedem: ack, and it works? | |
| 16:42:58 | mriedem | it passes | |
| 16:43:06 | mriedem | i'm going to rebase the fixes on top of it | |
| 16:43:53 | dansmith | it passes on master? | |
| 16:44:21 | dansmith | oh, I see it validates the broken behavior currently | |
| 16:47:36 | mriedem | right, that's generally how these types of tests work, | |
| 16:47:42 | mriedem | we write the recreate scenario, | |
| 16:47:49 | mriedem | then roll the fix on top and change the assertion in the test when it's fixed | |
| 16:49:16 | mriedem | ok, first victim is context.db_connection == None blows up the CellDatabases fixture | |
| 16:50:20 | dansmith | mriedem: yeah you'll have to make the change there since that fixture replaces target_cell | |
| 16:51:07 | nikeshm | smcginnis: https://bugs.launchpad.net/nova/+bug/1670627 :) | |
| 16:51:07 | openstack | Launchpad bug 1670627 in OpenStack Compute (nova) "quota is always in-use after delete the ERROR instances " [Critical,In progress] - Assigned to Dan Smith (danms) | |
| 16:51:22 | dansmith | mriedem: if it's None, just yield without calling real_target_cell | |
| 16:51:46 | dansmith | mriedem: but inside the lock | |
| 16:52:22 | mriedem | i was just going to use self._default_ctxt_mgr | |
| 16:52:55 | dansmith | not sure what you mean | |
| 16:53:26 | dansmith | mriedem: before this, you need to yield if cell_mapping is None and then return https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L402-L402 | |
| 16:53:26 | mriedem | sec | |
| 16:54:35 | mriedem | here it comes | |
| 16:55:00 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Decrement quota when deleting an instance in cell0 https://review.openstack.org/443395 | |
| 16:55:02 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add regression test for bug 1670627 https://review.openstack.org/443745 | |
| 16:55:02 | openstack | bug 1670627 in OpenStack Compute (nova) "quota is always in-use after delete the ERROR instances " [Critical,In progress] https://launchpad.net/bugs/1670627 - Assigned to Dan Smith (danms) | |
| 16:55:02 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Temporarily untarget context when deleting from cell0 https://review.openstack.org/443403 | |
| 16:56:25 | dansmith | mriedem: I'd prefer you didn't do that, | |
| 16:56:52 | mriedem | i'm fine changing it | |
| 16:56:58 | dansmith | mriedem: not only because it ends up mutating the context unlike the real live scenario, but it also is database-specific, so when my mq one rolls around, it will target the mq but not the db | |
| 16:58:41 | mriedem | hmm, so that makes the test fail | |
| 17:01:50 | mriedem | dansmith: wouldn't the default context be pointing at the main cell db? | |
| 17:02:01 | mriedem | which is what we want when we create the reservation | |
| 17:02:22 | dansmith | mriedem: it points at the one that was default=true, but in real life you'll end up taking the global one instead of having your cell targeted at the other one | |
| 17:02:30 | dansmith | mriedem: which means it depends on their config, and won't look like it does in real life | |
| 17:02:55 | dansmith | mriedem: so someone will come along and want to assert that db_connection=None there because that is how target_cell works, but it will be different when using the fixture | |
| 17:04:13 | arne_r | Calling attention to https://review.openstack.org/#/c/436868/ which, if passed, would make our CI folks really happy | |
| 17:04:37 | mriedem | dansmith: ok, in the nova/cinder meeting now, you want to tinker with that locally to change it? | |
| 17:04:52 | mriedem | because i'm a bit lost in that fixture | |
| 17:04:55 | dansmith | mriedem: sure | |
| 17:05:26 | dansmith | it's complicated | |
| 17:05:40 | dansmith | mriedem: you said the test failed.. which one did you mean? your regression test should fail yes? | |
| 17:06:16 | mriedem | https://review.openstack.org/443403 is passing | |
| 17:06:23 | mriedem | when i changed to what you wanted in the fixture, it failed | |
| 17:06:42 | mriedem | so to do what you want in the fixture, i need you to take a look at why it's failing | |
| 17:06:43 | dansmith | mriedem: but which test failed, the regression test? | |
| 17:06:46 | mriedem | yes | |
| 17:06:51 | mriedem | i'm just running that one | |
| 17:07:06 | dansmith | oh I see you changed it there, gotcha | |
| 17:07:31 | dansmith | waiting for a py27 run to finish and then I'll pull it down | |
| 17:08:46 | mriedem | py27 for something else right? | |
| 17:09:54 | dansmith | yes | |
| 17:11:20 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: fix missing instance.delete notification https://review.openstack.org/437222 | |
| 17:12:47 | mriedem | ok, once i'm done with this meeting i'll fix up the unit tests on the middle patch | |
| 17:18:37 | dansmith | mriedem: actually, I changed my mind, this is okay | |
| 17:19:03 | dansmith | I'm not sure it's as obvious, but it's less fiddling, and the context still looks like it should because we call real_target_cell anyway | |
| 17:19:13 | dansmith | mriedem: for completeness let me show you what I was thinking: | |
| 17:19:39 | dansmith | mriedem: http://pastebin.com/raw/isCJMvhG | |
| 17:20:06 | dansmith | mriedem: that works, but it may be more confusing for other people | |
| 17:20:11 | dansmith | if you agree, then just stick with what you've got | |
| 17:20:52 | mriedem | "controllering"? | |
| 17:20:54 | dansmith | http://pastebin.com/raw/pNv5smz8 | |
| 17:20:55 | mriedem | i disagree with that | |
| 17:20:55 | dansmith | is what it looks like applied | |
| 17:21:05 | dansmith | hah I just saw that | |
| 17:21:12 | dansmith | unintentional, but sounds like me | |
| 17:22:14 | mriedem | so, your change, it's basically just a pass through to the real unwrapped thing, right? | |
| 17:22:29 | dansmith | skipping the global db state switch, yeah | |
| 17:22:42 | mriedem | ok i think it makes sense to me, enough ayway | |
| 17:22:43 | mriedem | *anyway | |
| 17:22:55 | dansmith | yours does the switch, but .. to the main one | |
| 17:23:54 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transform missing delete notifications https://review.openstack.org/410297 | |
| 17:23:55 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: use context mgr in instance.delete https://review.openstack.org/443764 | |
| 17:24:17 | mriedem | ok i'll just take your change and update my local copy | |
| 17:24:36 | gibi | mriedem, dansmith: I splitted the instance.delete bugfix patch as we discussed https://review.openstack.org/#/c/437222 | |
| 17:24:55 | dansmith | mriedem: totally your call | |
| 17:25:24 | gibi | mriedem, dansmith: but I have to run now. thanks for the reviews! | |
| 17:25:25 | dansmith | gibi: I was going to suggest that first patch not adding/using the contextmanager | |
| 17:25:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: introduce basic framework for security proxying https://review.openstack.org/345396 | |
| 17:25:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: introduce framework for RFB authentication https://review.openstack.org/345397 | |
| 17:25:40 | dansmith | gibi: but if it's not going to cause conflicts, it's not as much of a problem I guess | |
| 17:25:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: introduce the VeNCrypt RFB authentication scheme https://review.openstack.org/345398 | |
| 17:25:41 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: provide an RFB security proxy implementation https://review.openstack.org/345399 | |
| 17:26:11 | gibi | dansmith: it didn't caused conflict a couple of days ago when I cherry-picked the patch to stable/ocata | |
| 17:26:55 | dansmith | gibi: not now for sure, I just mean in terms of backporting less code. the context manager add is more code, but it's contained | |
| 17:26:56 | dansmith | gibi: if mriedem is cool with it then it's fine | |
| 17:26:56 | gibi | dansmith: here is the cherry-pick https://review.openstack.org/#/c/441171/ I will update that as well | |
| 17:27:08 | gibi | dansmith: ahh I see | |
| 17:27:23 | gibi | dansmith: I agree that the ctx manager is quite self contained solution | |
| 17:27:43 | dansmith | gibi: anyway, I'll look at this in a few, thanks! | |
| 17:27:52 | gibi | dansmith: thanks a lot! | |
| 17:28:05 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Fix formatting of network options https://review.openstack.org/443086 | |
| 17:28:58 | mriedem | catch him! | |
| 17:29:47 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Final cleanups in conf/network https://review.openstack.org/431023 | |
| 17:30:16 | openstackgerrit | John Garbutt proposed openstack/nova-specs master: Add spec to use cinder's new attachment API https://review.openstack.org/373203 | |
| 17:34:19 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Temporarily untarget context when deleting from cell0 https://review.openstack.org/443403 | |
| 17:41:51 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/nova-specs master: Enable SR-IOV NIC offload feature discovery https://review.openstack.org/435954 | |