| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-16 | |||
| 14:00:14 | sahid | ok, i also working on backporting the change in one of our rhos version. so i hope we could do some internal tests | |
| 14:02:20 | stephenfin | lyarwood: Been looking at this oslo.config bug https://bugs.launchpad.net/nova/+bug/1755783 It seems nested_parse is getting confused with our output for that option http://paste.openstack.org/show/703014/ | |
| 14:02:22 | openstack | Launchpad bug 1755783 in OpenStack Compute (nova) "PCI configuration documentation renders with smart quotes" [Undecided,In progress] - Assigned to Stephen Finucane (stephenfinucane) | |
| 14:02:40 | stephenfin | lyarwood: So it's probably going to need a significant rewrite of that extension to unconfuse it :( | |
| 14:03:02 | stephenfin | (all that stuff in the <paragraph> tag should have been parsed itself but wasn't) | |
| 14:03:42 | dansmith | sahid: I thought you said you already tested this locally back in an earlier version | |
| 14:03:58 | dansmith | sahid: does that mean you haven't actually tested this manually yet? | |
| 14:04:27 | dansmith | specifically "I'm making tests locally and everything seemed to work" | |
| 14:04:46 | mnaser | are live migration job failures common-ish in gate? aka safe to recheck? | |
| 14:06:00 | fried_rice | figleaf: I'm sorry I'm just getting to this, but I've got some comments on that guy. | |
| 14:06:27 | figleaf | fried_rice: go for it | |
| 14:07:21 | sahid | dansmith: that was for a previous version of that patch | |
| 14:08:00 | dansmith | sahid: right, the version that _couldn't_ work...so you haven't tested the latest approach locally? | |
| 14:08:56 | fried_rice | figleaf, leakypipes: The thing so far that we might potentially want to hold up for is that the syntax isn't consistent with member_of in GET /resource_providers. Looks like this discussed earlier in the life of this patch, but not addressed. | |
| 14:09:16 | sahid | dansmith: no sure what you mean by the version that _counldn't work | |
| 14:09:23 | dansmith | heh | |
| 14:17:09 | figleaf | fried_rice: what are the differences that you see? | |
| 14:18:12 | fried_rice | figleaf: GET /resource_providers?member_of=in:<list of UUIDs> -- yours has no 'in:'. Personally I think 'in:' is silly and would like to see it go away. But it's inconsistent. But I don't like it. But it's inconsistent. I'm torn. | |
| 14:19:33 | openstackgerrit | Merged openstack/nova master: Updated from global requirements https://review.openstack.org/553211 | |
| 14:20:18 | figleaf | fried_rice: what do you mean? Because 'in:' is optional with a single UUID? | |
| 14:20:28 | leakypipes | fried_rice: I used to think I was indecisive. Now I'm not so sure. | |
| 14:20:35 | fried_rice | cdent: Do we have a blueprint for placement split? | |
| 14:20:43 | stephenfin | Can someone send this on its merry way through the gate? https://review.openstack.org/#/c/553751/ Fixes an issue I introduced with 'tox -e docs' locally (which the gate no longer runs) | |
| 14:20:45 | fried_rice | https://review.openstack.org/#/c/553149/ should be tagged with it. | |
| 14:20:45 | leakypipes | fried_rice: no, it's more a collection of things. | |
| 14:21:04 | mriedem | mdbooth: dansmith: Kevin_Zheng: cfriesen: re: the abort queued live migration spec, i did some digging and i think the futures library/module is what we'd want https://review.openstack.org/#/c/536722/ | |
| 14:21:20 | mriedem | throw futures into a thread pool executor and you can cancel them if they haven't already started | |
| 14:21:23 | fried_rice | figleaf: I haven't gotten all the way through the patch yet (in a meeting atm) but it looks to me like your GET /a_c member_of doesn't use 'in:' at all. | |
| 14:21:35 | leakypipes | stephenfin: gerrit is telling me it needs a rebase | |
| 14:21:42 | dansmith | mriedem: that's one way to do it yeah | |
| 14:21:43 | mriedem | reminds me of when i used to use https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/package-summary.html | |
| 14:21:45 | fried_rice | figleaf: In GET /r_p if I'm not mistaken, you *can't* specify a list without 'in:' | |
| 14:22:13 | stephenfin | leakypipes: It is? I'm not seeing anything in the UI | |
| 14:23:00 | cdent | fried_rice: no, there's no spec or blueprint at this stage, mostly because of what leakypipes says: there's a suite of vaguely connected lots of things. Do you think we should have something? | |
| 14:23:04 | mriedem | mnaser: yes we hit live migration failures in the gate intermittently | |
| 14:23:07 | figleaf | fried_rice: https://review.openstack.org/#/c/552098/8/nova/api/openstack/placement/util.py@333 | |
| 14:23:15 | mriedem | mnaser: there are some known issues with libvirt in the versions we use | |
| 14:23:27 | mnaser | mriedem: ah bummer, i'll throw a recheck then | |
| 14:23:29 | leakypipes | finucannot: it says Cannot Merge for me. | |
| 14:23:37 | mriedem | mnaser: e.g. http://status.openstack.org/elastic-recheck/#1706377 | |
| 14:24:16 | fried_rice | cdent: Seems like it would be nice. At least a common topic so we can see everything. I guess that would be sufficient. | |
| 14:24:48 | cdent | i'll make something | |
| 14:25:39 | fried_rice | figleaf: Okay; So far I had only read the explanation comment in microversion.py and it's not listed in there. I'll stay quiet til I finish the patch. | |
| 14:25:56 | mriedem | Kevin_Zheng: so i think we create a ThreadPoolExecutor, submit the live migration calls on that, which returns Future objects, and we map those to the migration id which we can later lookup to then call Future.cancel() | |
| 14:26:07 | fried_rice | I see it in the reno. | |
| 14:26:19 | finucannot | fried_rice: Oh, right. Weird that the UI didn't say that. | |
| 14:26:46 | superdan | mriedem: we need to make sure we don't .cancel() after it's started really running | |
| 14:26:54 | superdan | mriedem: does futures handle that or do we need to? | |
| 14:27:09 | openstackgerrit | Jay Pipes proposed openstack/nova master: mirror nova host aggregate members to placement https://review.openstack.org/553597 | |
| 14:27:27 | figleaf | fried_rice: if the comments can be made clearer, that can always be done in a follow-up patch, no? | |
| 14:27:29 | leakypipes | lyaaaaaaaaaaaaar: ahoy, matey. | |
| 14:27:39 | fried_rice | figleaf: Yes, certainly. | |
| 14:28:05 | Kevin_Zheng | mriedem: Thanks for reviewing, I’m not familiar with this lib, have to do some reading first | |
| 14:28:21 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Follow the new PTI for document build https://review.openstack.org/553751 | |
| 14:28:22 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Correct documentation for '[pci] passthrough_whitelist' https://review.openstack.org/552874 | |
| 14:28:53 | finucannot | johnthetubaguy, leakypipes: and fixed. A global requirements messed with the sphinx version, causing a merge conflict ^ | |
| 14:30:22 | mriedem | superdan: cancel() returns False if the task has already starte | |
| 14:30:24 | mriedem | *started | |
| 14:30:28 | superdan | mriedem: ack, col | |
| 14:30:30 | superdan | *cool | |
| 14:30:31 | mriedem | you can also add a callback function to the future | |
| 14:30:44 | mriedem | what i'm not totally sure about is if the executor removes futures from the pool once they are done | |
| 14:30:47 | mriedem | automatically | |
| 14:30:56 | mriedem | it should | |
| 14:31:04 | mriedem | but we'll need to track the futures in a dict, | |
| 14:31:06 | superdan | presumably we could remove it from the end of the thread ourselves if needed | |
| 14:31:16 | mriedem | and we could add a callback function so that when a future is done, we callback to cleanup our dict | |
| 14:31:50 | mriedem | or just at the end of a live migration run, either way | |
| 14:32:34 | superdan | that's what I mean | |
| 14:33:08 | mriedem | actually that's overthining it, | |
| 14:33:11 | mriedem | *thinking | |
| 14:33:27 | mriedem | i think we can just have _do_live_migration remove the entry from the dict when it runs and changes the migration status to 'preparing' | |
| 14:33:34 | mriedem | since at that point, you can't cancel the live migration | |
| 14:33:36 | leakypipes | finucannot: +Wallaby'd | |
| 14:34:02 | finucannot | leakypipes: Cheers :) | |
| 14:35:31 | cfriesen | what's the right channel for devstack questions? it seems to be installing the wrong version of packages for stable/queens | |
| 14:35:42 | mriedem | finucannot: leakypipes: https://review.openstack.org/#/c/553751/2 | |
| 14:35:53 | mriedem | cfriesen: -qa | |
| 14:36:05 | cfriesen | thx | |
| 14:36:28 | finucannot | mriedem: Good catch. I'll pull it out of the queue and rework | |
| 14:37:17 | finucannot | mriedem: Also, are you still using the 'tox -evenv' trick? Just install reno and save yourself a few minutes a week | |
| 14:37:36 | finucannot | ;pip install --user reno' if you don't want to pollute your system (I think) | |
| 14:37:48 | finucannot | *system Python install | |
| 14:37:54 | leakypipes | mriedem: sorry man, had no idea about that :( | |
| 14:37:55 | mriedem | i'll be damned if i'm going to change my reno creation workflow now | |
| 14:38:02 | finucannot | :D fair | |
| 14:39:12 | superdan | mriedem: ah yeah, true.. first thing it does is unqueue itself effectively.. makes sense | |
| 14:39:50 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Follow the new PTI for document build https://review.openstack.org/553751 | |
| 14:39:52 | finucannot | mriedem: as requested | |
| 14:41:27 | mriedem | finucannot: that's not using upper-constraints | |
| 14:41:30 | mriedem | see https://review.openstack.org/#/c/532971/1/tox.ini | |
| 14:41:43 | finucannot | mriedem: Neither is the standard [testenv] target | |
| 14:41:45 | finucannot | which I copied | |
| 14:42:03 | finucannot | Did I miss something? | |
| 14:42:24 | mriedem | the install_command does... | |
| 14:42:26 | mriedem | hmm | |
| 14:42:27 | finucannot | Yeah, we hack 'install_command' to do that | |
| 14:42:47 | mriedem | ok that's different from novaclient, but you're still missing runtime deps | |
| 14:42:58 | mriedem | venv is just a place to do whatever, so it should have all requs | |
| 14:42:59 | mriedem | *reqs | |