| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-09 | |||
| 15:54:28 | mriedem | smaller backports are much easier and nicer | |
| 15:55:03 | cdent | jaypipes: I had to -1 get_invetory, but it is a very soft -1 | |
| 15:55:50 | mriedem | dansmith: gibi: what's the cleanup? the context manager? | |
| 15:56:03 | dansmith | mriedem: yeah, and the use of it in a few places that aren't affected, AFAICT | |
| 15:57:13 | mriedem | jaypipes: https://review.openstack.org/#/c/442200/ - that's the bottom of the series and you were +2 yesterday | |
| 15:57:21 | gibi | mriedem: I think this is the only place where it wasn't needed to restore original behavior https://review.openstack.org/#/c/437222/1/nova/compute/api.py@1868 | |
| 15:58:08 | gibi | mriedem, dansmith: I can put this ^^ to a separate patch | |
| 15:58:18 | dansmith | gibi: that's PS1 | |
| 15:58:32 | gibi | ups sorry | |
| 15:58:55 | dansmith | gibi: there are only two places we're missing the notification, | |
| 15:58:56 | dansmith | and two places you convert to the new one, right? | |
| 15:59:09 | jaypipes | mriedem: k, on it. just +2'd https://review.openstack.org/#/c/345138 BTW | |
| 15:59:36 | mriedem | ok, haven't reviewed that yet at all | |
| 15:59:43 | dansmith | I have I'll circle back | |
| 15:59:55 | gibi | dansmith: true. I looked at the wrong ps | |
| 16:00:11 | gibi | dansmith: I can split the patch | |
| 16:00:14 | dansmith | gibi: I think splitting those up would be good for backport cleanliness if you're cool with it | |
| 16:00:28 | dansmith | gibi: sweet, thanks for finding, fixing :) | |
| 16:00:29 | gibi | dansmith: cool, I will do it | |
| 16:27:53 | nikeshm | hi | |
| 16:28:24 | nikeshm | i am frequently facing the quota issue in openstack master | |
| 16:28:26 | nikeshm | https://ask.openstack.org/en/question/494/how-to-reset-incorrect-quota-count/ | |
| 16:28:39 | nikeshm | is there any bug reported | |
| 16:28:42 | nikeshm | for this | |
| 16:33:38 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/nova-specs master: Enable SR-IOV NIC offload feature discovery https://review.openstack.org/435954 | |
| 16:34:11 | openstackgerrit | Chris Friesen proposed openstack/nova master: show flavor info in server details https://review.openstack.org/434514 | |
| 16:35:35 | jaypipes | sfinucan, sean-k-mooney: time for a hangout? is rodolfo available? (btw, what's rodolfo's IRC nick?) | |
| 16:35:57 | sfinucan | jaypipes: (y) | |
| 16:36:17 | sfinucan | As for IRC nick - not a clue | |
| 16:37:04 | sean-k-mooney | jaypipes: ralonsoh | |
| 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 | mriedem | dansmith: melwitt: Kevin_Zheng: ^ this is the regression test i was thinking of | |
| 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 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add regression test for bug 1670627 https://review.openstack.org/443745 | |
| 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 | 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:07 | nikeshm | smcginnis: https://bugs.launchpad.net/nova/+bug/1670627 :) | |
| 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 | mriedem | sec | |
| 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: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: WIP: Temporarily untarget context when deleting from cell0 https://review.openstack.org/443403 | |
| 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: Add regression test for bug 1670627 https://review.openstack.org/443745 | |
| 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 | dansmith | is what it looks like applied | |
| 17:20:55 | mriedem | i disagree with that | |
| 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? | |