| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-19 | |||
| 15:53:04 | fried_rice | Thanks giblet! It's all downhill from here. | |
| 15:53:59 | cfriesen | anyone feel like a fairly quick code review? https://review.openstack.org/#/c/484381/ has been sitting with a single +2 for a couple weeks now | |
| 15:54:24 | cfriesen | ^ this is related to instance evacuation with PCI devices | |
| 15:54:33 | leakypipes | cfriesen: evacuate? PCI devices? oh hell, sign me up! | |
| 15:54:49 | cfriesen | leakypipes: it's under 20 lines. :) | |
| 15:54:57 | leakypipes | cfriesen: I'm looking at it :) | |
| 15:55:05 | cfriesen | sweet, thanks | |
| 15:55:48 | giblet | fried_rice: I have left like 15 minutes then boardgame night... | |
| 15:56:01 | fried_rice | ooo :) | |
| 15:56:18 | fried_rice | cfriesen How does this actually fix anything? | |
| 15:56:44 | fried_rice | cfriesen Oh, it passes in a thing that's being checked by something that happened in a different patch? | |
| 15:57:20 | fried_rice | got it. | |
| 15:57:24 | cfriesen | fried_rice: not actually my patch, but I think it's passing in a thing that something else already in the code is looking for | |
| 15:58:13 | finucannot | fried_rice: Yup. Something we already do for one code path (cold migration, resize) should now being done for another (evacuation) | |
| 15:58:20 | fried_rice | cfriesen Yeah, I see _update_port_binding_for_instance takes a migration kwarg | |
| 15:58:36 | leakypipes | cfriesen: +Wallaby'd | |
| 15:58:49 | cfriesen | leakypipes: awesome, thanks | |
| 15:59:08 | fried_rice | cfriesen Left a passive-aggressive +0 with a nit. | |
| 15:59:49 | cfriesen | lol | |
| 16:00:28 | leakypipes | jackiet: how about "fullmetaljackie"? ;) | |
| 16:00:47 | leakypipes | jackiet: though that might be too long for irc. | |
| 16:01:52 | fried_rice | ++! | |
| 16:01:53 | fullmetaljackiet | leakypipes: niceeee I like it | |
| 16:01:57 | leakypipes | heh | |
| 16:02:00 | leakypipes | :) | |
| 16:02:12 | leakypipes | fullmetaljackiet: glad to be of service. | |
| 16:02:13 | cfriesen | leakypipes: want me to quick make the change suggested by fried_rice? | |
| 16:02:59 | leakypipes | cfriesen: meh? the way the gate is I'd prefer to just leave it for now. | |
| 16:03:08 | cfriesen | works for me | |
| 16:03:15 | cfriesen | figured I'd offer | |
| 16:03:16 | leakypipes | fried_rice is right, of course. | |
| 16:03:22 | leakypipes | but it's a nit | |
| 16:03:49 | fried_rice | Not even worth a followon. | |
| 16:04:56 | leakypipes | fried_rice: so, your comment on this one unit test "_get_provider_aggregates and _traits were called thrice" just makes me think of the monty python holy hand grenade scene. | |
| 16:05:13 | fried_rice | leakypipes Five is RIGHT OUT. | |
| 16:05:19 | leakypipes | "The number shall be THREE!" | |
| 16:05:28 | leakypipes | :) | |
| 16:05:33 | fried_rice | Then lobbest thou the holy hand grenade... | |
| 16:05:37 | fried_rice | okay okay. | |
| 16:05:40 | leakypipes | hehe | |
| 16:05:54 | fried_rice | Back to Airplane! quotes. | |
| 16:06:20 | openstackgerrit | Matthew Edmonds proposed openstack/nova master: Fix bad autospec usage https://review.openstack.org/535806 | |
| 16:07:17 | edmondsw | claudiub I would prefer ^ to the oslotest fixture | |
| 16:07:33 | edmondsw | but it's at least a first step even if you want to continue to push use of that fixture | |
| 16:07:42 | edmondsw | (which could be done subsequently) | |
| 16:08:58 | bauzas | ildikov: I don't see any change in the series that does things at the driver level with https://review.openstack.org/#/c/531244/10/nova/compute/manager.py@5190 | |
| 16:09:16 | bauzas | ildikov: I mean, you and matt made the RPC changes for asking for multiattach, sure | |
| 16:09:31 | bauzas | but then, how do we pass that down to libvirt? I'm confused | |
| 16:10:03 | bauzas | oh f***, nevermind | |
| 16:10:05 | ildikov | bauzas: it's in the capabilities | |
| 16:10:26 | ildikov | bauzas: I guess you found it :) | |
| 16:10:30 | bauzas | ildikov: yeah, so basically the RPC change is just for asking whether the compute driver supports it | |
| 16:10:37 | ildikov | bauzas: yes | |
| 16:10:40 | bauzas | if not, raise | |
| 16:10:45 | ildikov | exactly | |
| 16:10:51 | bauzas | if yes, let's the volume say whether it's multiattached | |
| 16:10:54 | bauzas | gotcha | |
| 16:10:59 | claudiub | edmondsw: sure, that works too, but i've had nightmares with mock.create_autospec | |
| 16:11:18 | bauzas | ildikov: sorry, I'm in general used to see things happening at the manager level once you pass a new attribute | |
| 16:11:27 | ildikov | bauzas: yeah, many pieces to put together | |
| 16:11:27 | bauzas | a new *RPC* attribute | |
| 16:11:34 | claudiub | I've had run times increase exponentially because of mock.create_autospec | |
| 16:11:43 | bauzas | ildikov: yeah, https://review.openstack.org/#/c/532928/4/nova/compute/manager.py does that | |
| 16:12:26 | edmondsw | claudiub hmm | |
| 16:12:33 | ildikov | bauzas: we tried to look into failing as quick as possible if either bit is not there | |
| 16:13:06 | claudiub | at one time, i've had an execution time of 4000 seconds for ~300 unit tests. :) | |
| 16:13:18 | claudiub | aprox. | |
| 16:13:21 | ildikov | bauzas: as we don't have all the info at the API we needed some tricks :) | |
| 16:14:40 | bauzas | ildikov: I'll +W it, but I think you should add a NOTE in the manager explaining why you don't really care doing things after the RPC call | |
| 16:15:08 | bauzas | ildikov: just because it took me some time to understand that, some other people wanting to contribute could be confused by that new attribute we don't really use | |
| 16:15:15 | bauzas | it's just a flag | |
| 16:15:53 | ildikov | bauzas: sure, I guess you added a comment already, so I will do it in a follow up patch | |
| 16:16:00 | ildikov | bauzas: thank you | |
| 16:16:04 | bauzas | ildikov: yup, no rush | |
| 16:16:12 | bauzas | ildikov: that can totally be a follow-up | |
| 16:16:23 | ildikov | bauzas: cool :) | |
| 16:16:26 | bauzas | but I'd be happy if we could keep sort of the logic in the code | |
| 16:16:41 | bauzas | matt did a good job doing that in the conductor | |
| 16:16:58 | ildikov | yeah, it always starts simple and then we give it more and more thoughts... | |
| 16:17:19 | bauzas | ildikov: for example https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L896-L903 | |
| 16:17:26 | ildikov | the tricks are all on him here too and I mean it the best way possible | |
| 16:17:45 | bauzas | and https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L913-L921 | |
| 16:18:00 | bauzas | ildikov: sure, I don't deny it | |
| 16:18:02 | ildikov | nice, good examples | |
| 16:18:13 | bauzas | ildikov: I'm just saying, we somehow need to remember what we did | |
| 16:18:28 | ildikov | bauzas: I completely agree | |
| 16:18:44 | bauzas | anyway, I'm calling it a day | |
| 16:19:02 | bauzas | the API change is hairy and not my expertise, so it'll require a bit more thoughts | |
| 16:19:11 | bauzas | not for a friday thing | |
| 16:19:15 | bauzas | ++ folks | |
| 16:19:34 | ildikov | bauzas: makes sense, thanks for looking into the series | |
| 16:20:24 | ildikov | bauzas: have a great weekend! | |
| 16:21:33 | lyarwood | mdbooth: http://logs.openstack.org/58/523958/16/check/openstack-tox-py35/db3ef07/testr_results.html.gz - if you have time, any thoughts on these failures? hasattr is failing on o.vo objects on py35, works on py27. | |
| 16:22:41 | mdbooth | lyarwood: Yeah, will look now. | |
| 16:27:04 | mdbooth | lyarwood: I'm hacking on it right now. Ping me if you fix it before I do. | |
| 16:27:15 | lyarwood | mdbooth: the race is on | |
| 16:27:21 | mdbooth | Hehe | |
| 16:27:30 | mdbooth | If you've got other stuff to do, feel free :) | |
| 16:27:31 | lyarwood | mdbooth: but given I pinged as I was stuck I think you might win | |
| 16:42:49 | edmondsw | claudiub I ran the UTs for test_compute (which take the longest of the files changed) for both master and my proposal with create_autospec. No significant perf impact | |