| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-16 | |||
| 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 | |
| 14:43:12 | finucannot | They weren't there before though | |
| 14:44:03 | finucannot | If '[testenv:venv] deps' wasn't defined, it would defer to '[testenv] deps' https://github.com/openstack/nova/blob/master/tox.ini#L20 | |
| 14:44:09 | mriedem | right, hmm | |
| 14:44:22 | openstackgerrit | Merged openstack/nova master: docs: Disable smartquotes https://review.openstack.org/553730 | |
| 14:44:22 | finucannot | mriedem: We use 'usedevelop' | |
| 14:44:36 | finucannot | which would install the package and all its runtime dependencies | |
| 14:44:39 | finucannot | so we're good there | |
| 14:44:48 | mriedem | ok, +2 | |
| 14:44:57 | finucannot | (y) | |
| 14:45:31 | mriedem | leakypipes: can you put a procedural -2 on https://review.openstack.org/#/c/553597/ until the spec is approved? | |
| 14:49:56 | leakypipes | mriedem: -W'd it. | |
| 14:50:24 | leakypipes | mriedem: that was mostly me just experimenting in code to see what will work best for the mirroring. basically using it to inform the spec... | |
| 14:50:31 | leakypipes | mriedem: working on the spec changes now. | |
| 14:50:33 | mriedem | yeah i understand | |
| 14:59:32 | cdent | mriedem: you recall how you imported oslo_service to get a single log_options config thing: https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/wsgi.py#L60 ? | |
| 14:59:54 | cdent | how horribly dirty is it to use a local config opt (that duplicates the oslo_service one) instead? | |
| 15:01:38 | lyaaaaaaaaaaaaar | mnaser: https://review.openstack.org/#/q/topic:bug/1744361+status:open - btw the stable changes still reference different change-id's , happy to resubmit these if you don't have time. | |
| 15:04:46 | mriedem | cdent: given i can't figure out how to add a hook into oslo.service for that, it's probably not the worst thing in the world | |
| 15:05:01 | mnaser | lyaaaaaaaaaaaaar: appreciate if you do, I’m out of the office and running around with a bunch of stuff | |
| 15:05:10 | mriedem | i think at one point when doing that i asked in the -oslo channel if i should even be doing this, i.e. piggyback on their option, and got no response | |
| 15:05:20 | lyaaaaaaaaaaaaar | mnaser: np, happy to help | |
| 15:05:42 | cdent | mriedem: in nova it's not a big deal since oslo_service is already there, but in placement it sets off a cascade of imports and requirments that is sad making. | |
| 15:06:25 | mriedem | only thing i'm not sure about is how to signal a config option change here, | |
| 15:06:29 | mriedem | because the old option isn't deprecated, | |
| 15:06:31 | mriedem | only our usage of it, | |
| 15:06:39 | mriedem | so i guess we could just log something on startup if/when it's used | |
| 15:06:48 | mriedem | to say, 'move to this other nova-specific option' | |
| 15:07:17 | mnaser | lyaaaaaaaaaaaaar: thank you very much | |
| 15:07:57 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/queens: Revert "Refine waiting for vif plug events during _hard_reboot" https://review.openstack.org/553817 | |
| 15:08:45 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/pike: Revert "Refine waiting for vif plug events during _hard_reboot" https://review.openstack.org/553818 | |
| 15:09:07 | cdent | mriedem: I'm not sure I'm following? The option (in the config file) stays the same. What will be different is how the process has the code which makes the conf option legit and validates its value? | |
| 15:09:44 | mriedem | cdent: i don't think you can (should) re-define an option in nova.conf that is already in oslo.service, | |
| 15:09:51 | mriedem | because the oslo.service options are imported into nova.conf | |
| 15:09:56 | mriedem | s/imported/registered/ | |
| 15:10:13 | johnthetubaguy | superdan: mriedem: do we need the placeholder migrations for queens now? | |
| 15:10:16 | cdent | right, but in placement itself there is no import of oslo_service | |
| 15:10:35 | mriedem | cdent: i understand, but placement is still in nova | |
| 15:10:42 | mriedem | and it's using nova's config | |
| 15:10:43 | mriedem | so.. | |
| 15:10:52 | cdent | so I'm wondering about a sort of hack in placement that says "this is a stub for a thing that is normall in olso_service, but since it's in default it's okay" | |
| 15:11:03 | cdent | sure, but it is an indepedent process, so it doesn't really care, does it? | |
| 15:11:22 | mriedem | i defer to melwitt | |
| 15:11:40 | cdent | it's not a big deal, it just seems wasteful to import oslo_service | |
| 15:11:49 | mriedem | and i'm saying you don't have to, | |
| 15:11:53 | mriedem | if we define our own option | |
| 15:12:06 | mriedem | which is what placement would have to do anyway once it's split out | |
| 15:12:09 | mriedem | to keep using this | |
| 15:12:21 | cdent | maybe we are talking about the same thing | |