| *** armax has joined #openstack-stable | 00:04 | |
| *** armax has quit IRC | 00:07 | |
| *** mriedem has quit IRC | 04:18 | |
| *** rcernin has joined #openstack-stable | 06:02 | |
| *** armax has joined #openstack-stable | 06:16 | |
| *** kashyap has joined #openstack-stable | 06:23 | |
| kashyap | Morning folks. can someone review this CVE fix for stable/liberty? https://review.openstack.org/#/c/327624/ | 06:23 |
|---|---|---|
| *** pcaruana has joined #openstack-stable | 06:38 | |
| *** armax has quit IRC | 06:44 | |
| ttx | kashyap: +2 | 06:56 |
| kashyap | ttx: Excellent, thank you. | 07:09 |
| *** tesseract- has joined #openstack-stable | 07:59 | |
| -openstackstatus- NOTICE: setuptools 24.0.0 broke dsvm tests, we've gone back to old images, it's safe to recheck now if you had a failure related to setuptools 24.0.0 (processor_architecture) - see bug 1598525 | 08:15 | |
| *** kzaitsev_ws has quit IRC | 08:48 | |
| *** kzaitsev_ws has joined #openstack-stable | 08:49 | |
| *** Kiall has quit IRC | 09:55 | |
| *** Kiall has joined #openstack-stable | 09:57 | |
| *** apevec has joined #openstack-stable | 10:04 | |
| *** apevec has quit IRC | 10:22 | |
| *** amrith has quit IRC | 10:27 | |
| *** amrith has joined #openstack-stable | 10:28 | |
| *** apevec has joined #openstack-stable | 10:28 | |
| kashyap | apevec or anyone else would like to give the final ACK on this - https://review.openstack.org/#/c/327624/ | 10:48 |
| apevec | kashyap, thanks for persiting on that backport :) | 10:49 |
| apevec | persisting even! | 10:49 |
| kashyap | apevec: Heh, thanks for noticing | 10:49 |
| kashyap | Not just me though, there are others too, who pushed for it and chimed in. | 10:49 |
| kashyap | So, the only addition here is that I added an explicit check for the ProcessLimits attribute -- if it's not present error out wtih a nice message. | 10:50 |
| kashyap | apevec: ^ | 10:50 |
| apevec | yep, that was a compromise vs bumping min.ver | 10:51 |
| kashyap | Yeah, seemed reasonable. I agree (after some convincing by others) that doing a global-req bump might be a bit too disruptive. | 10:52 |
| apevec | kashyap, hmm, late comment from danpb ? | 11:04 |
| apevec | I've pulled +W for now | 11:05 |
| kashyap | apevec: Looking | 11:05 |
| apevec | it's already in gate queue, not sure if removing +w will prevent merge | 11:07 |
| kashyap | apevec: init_host() where? | 11:07 |
| apevec | dunno, ask danpb | 11:08 |
| kashyap | I'll check on -nova | 11:08 |
| apevec | but probably he meant compute.manager ? | 11:08 |
| apevec | or add conditional and not use that attribute if missing | 11:09 |
| apevec | in qemu_img_info | 11:09 |
| kashyap | apevec: Yeah, I'm looking at the source to see where this could be it | 11:10 |
| apevec | kashyap, I'm not sure what woud relocating to init_host bring | 11:11 |
| apevec | it would still need conditional in qemu_img_info | 11:11 |
| kashyap | Yeah, I don't have that code in my 'cache' in the head | 11:11 |
| kashyap | So, add another conditional in qemu_img_info() for QEMU_IMG_LIMITS? | 11:12 |
| apevec | yeah, that would avoid crash&burn danpb pointed out | 11:14 |
| apevec | try block is catching only processutils.ProcessExecutionError | 11:15 |
| apevec | and this would be NameError | 11:16 |
| apevec | NameError: name 'QEMU_IMG_LIMITS' is not defined | 11:16 |
| kashyap | apevec: So wrap the 'try' block in qemu_img_info() also under a similar the hasattr() check? | 11:17 |
| * kashyap tries | 11:18 | |
| apevec | kashyap, or just set QEMU_IMG_LIMITS = 0 in else? keyword param should be ignored in older oslo? | 11:25 |
| kashyap | apevec: You mean: QEMU_IMG_LIMITS = None? | 11:27 |
| * kashyap will check | 11:29 | |
| apevec | nm, it won't work | 11:32 |
| apevec | you'll get UnknownArgumentError | 11:32 |
| apevec | kashyap, ^ | 11:32 |
| kashyap | apevec: Thanks for confirming. Did you also see the cpu_time attribute | 11:33 |
| kashyap | s/also see/also see Dan's comment on/ | 11:33 |
| kashyap | Oh, it's taken care by the first check anyway. | 11:34 |
| apevec | damn oslo incompatibilities :) | 11:34 |
| kashyap | Yeah, I'm not so involved in Oslo stuff so far :-) | 11:35 |
| * kashyap back in a few, lunch | 11:37 | |
| apevec | kashyap, what danpb is saying, that hasattr(processutils, 'ProcessLimits') is not enough, cpu_time was added only in 3.8.0 : https://github.com/openstack/oslo.concurrency/commit/8af826953d1ad2cab2ecf360e0c794de70a367c3 | 11:38 |
| apevec | ah np, it was backported | 11:39 |
| apevec | https://github.com/openstack/oslo.concurrency/commit/d65d931da8490576f0abf30f124ca3a032b481c7 | 11:39 |
| apevec | so issue is if someone is running some older versions from master, not latest stable/liberty | 11:40 |
| kashyap | apevec: Back now. Reading your comment | 11:51 |
| kashyap | Yeah, it was backported indeed | 11:51 |
| apevec | right, so issue would be for someone mixing liberty nova and certain range of master/mitaka oslo lib versions | 11:52 |
| kashyap | apevec: You mean "newer versions from master"? Or some version that is not latest stable/liberty | 11:52 |
| kashyap | Oh, dear. Yeah... | 11:52 |
| apevec | that would be under "unsupported" but still might want to catch that edge case | 11:52 |
| apevec | afaict ProcessLimits would throw ValueError | 11:53 |
| kashyap | apevec: Okay, thought this would be a straightforward one. Now here I am... :-) | 11:57 |
| kashyap | Thanks for your comments, so far! | 11:57 |
| apevec | I've added comment in review for danpb to see it | 11:58 |
| kashyap | apevec: Yeah, just noticed. Thanks for adding it there (better place than here) too | 11:59 |
| apevec | kashyap, confirmed, oslo_concurrency 3.4.0 throws ValueError: invalid limits: cpu_time | 12:00 |
| kashyap | apevec: You just tested it? | 12:01 |
| apevec | yeah, in git checkout 3.4.0 | 12:01 |
| apevec | it is silly edge case, but let's make danpb happy ;) | 12:01 |
| kashyap | apevec: :-) Noted | 12:02 |
| apevec | import oslo_concurrency.processutils as utils | 12:03 |
| apevec | utils.ProcessLimits(cpu_time=1) | 12:03 |
| kashyap | apevec: The error message while catching the ValueError would be would be: | 12:03 |
| apevec | kashyap, ^ works >= 3.6.0 | 12:03 |
| kashyap | "Please use oslo.concurrency" version 3.6.0 or above" | 12:04 |
| kashyap | ? | 12:04 |
| apevec | "dude, Oslo and Nova branches do not mix well" | 12:04 |
| kashyap | LOL | 12:04 |
| apevec | kashyap, 3.8.0 or higher | 12:04 |
| kashyap | Okido, let me try the code locally | 12:05 |
| apevec | sorry, works >= 3.6.0 before was wrong! | 12:05 |
| kashyap | Yeah, noted. | 12:06 |
| apevec | 3.6.0 still fails, cpu_time came only in 3.8 | 12:06 |
| kashyap | apevec: Oh, right. It's a mess | 12:07 |
| kashyap | So: 3.6.1 or higher | 12:08 |
| apevec | no | 12:08 |
| kashyap | Sigh, you said 3.8.0. /me makes a proper note of what came in where first, to avoid confusing self | 12:09 |
| apevec | there isn't 3.6.1 :) | 12:09 |
| apevec | 3.8.0 is what worked | 12:10 |
| apevec | on master | 12:10 |
| kashyap | Yeah | 12:10 |
| kashyap | Thanks for testing! | 12:11 |
| apevec | 3.7.1 was stable/mitaka backport which works too | 12:11 |
| apevec | so safest is to say >= 3.8.0 | 12:11 |
| kashyap | apevec: Yeah, makese sense | 12:15 |
| *** mriedem has joined #openstack-stable | 12:43 | |
| kashyap | apevec: When you get a sec, any quick comment on this diff? - http://paste.openstack.org/show/525592/ | 12:46 |
| kashyap | New change (the try block is now around 'prlimit') http://paste.openstack.org/show/525595/ | 12:58 |
| kashyap | Disregard the above, needs a little more rework. | 13:12 |
| *** mriedem has quit IRC | 13:45 | |
| apevec | kashyap, you need to catch valueerror above, where setting cpu_time | 13:45 |
| kashyap | apevec: In the new change, I just used the try/except block around the creation of QEMU_IMG_LIMITS (and not checking on any attributes): https://review.openstack.org/#/c/327624/ | 13:45 |
| kashyap | apevec: Please look at this new change (just using try/except at the creation of 'prlimit') without any explicit attribute check -- https://review.openstack.org/#/c/327624/7/nova/virt/images.py | 13:46 |
| kashyap | apevec: The above way, no need to explicitly check for attributes. I hope it looks reasonable now. Will let Jenkins run | 13:49 |
| apevec | kashyap, so that will only log error on init, but later will produce UnknownArgumentError in qemu_img_info | 13:49 |
| apevec | with incompat oslo.concurrecy | 13:49 |
| apevec | you still need conditional there | 13:50 |
| kashyap | apevec: Okay, I'll add an explicit check around what you said there then | 13:50 |
| kashyap | I'll make a quick post here, if you don't mind. (Instead of waiting on yet another CI run.) | 13:50 |
| kashyap | apevec: Does this look okay? - http://paste.openstack.org/show/525611/ | 13:53 |
| kashyap | apevec: The conditional you request | 13:53 |
| kashyap | s/request/requested/ | 13:53 |
| apevec | that will still produce NameError: name 'QEMU_IMG_LIMITS' is not defined | 13:55 |
| apevec | unless you set NameError: QEMU_IMG_LIMITS = None is exception block | 13:55 |
| apevec | above | 13:55 |
| kashyap | apevec: I see | 13:56 |
| apevec | also not sure about raising value error | 13:56 |
| kashyap | apevec: I'll just raise regular exception | 13:56 |
| *** tristanC has joined #openstack-stable | 13:56 | |
| kashyap | apevec: Please comment when you have a sec: https://review.openstack.org/#/c/327624/8/nova/virt/images.py | 14:08 |
| kashyap | Thanks for your patience. Feel free to ignore this and get to something else :-) | 14:11 |
| *** armax has joined #openstack-stable | 14:28 | |
| *** catintheroof has joined #openstack-stable | 14:33 | |
| apevec | kashyap, almost there, just move one line :) | 14:48 |
| kashyap | apevec: Yeah, was out for a break | 14:48 |
| kashyap | apevec: Responding to Dan, _why_ updating global-req. is not an option, with the rationale pointer to the discussion here: https://review.openstack.org/#/c/333403/ | 14:48 |
| apevec | yeah, I linked to that in my reply too | 14:50 |
| kashyap | Oh, cool. | 14:53 |
| *** armax has quit IRC | 14:57 | |
| *** amrith has quit IRC | 15:06 | |
| kashyap | apevec: Before I upload, a quick once-over, does this address sahid/your comments: http://paste.openstack.org/show/525621/ | 15:09 |
| apevec | kashyap, yes, that will work | 15:09 |
| kashyap | apevec: Thank you, let me submit that. | 15:09 |
| *** amrith has joined #openstack-stable | 15:10 | |
| apevec | lol, so we have declared madness now :) | 15:12 |
| kashyap | apevec: Haha, indeed | 15:13 |
| kashyap | apevec: I was just telling Dan on -nova that I wasn't disregarding his comment, but uploaded that PS9 _before_ noticing his comment | 15:14 |
| kashyap | apevec: Should I raise that thread to the mailing list? | 15:18 |
| kashyap | With some thread like "Qorum on XYZ bug" with some context? | 15:19 |
| *** arif-ali has quit IRC | 15:20 | |
| kashyap | I'll just let it sit. I'm done with it for today. | 15:20 |
| kashyap | (Until Matt comments with his stance.) | 15:21 |
| apevec | reqs change is already merged with u-c change only, we'll need new change with g-r bump | 15:24 |
| kashyap | apevec: I'll post that and let it continue there, maybe | 15:24 |
| apevec | I've commented in 333403 | 15:25 |
| * kashyap looks | 15:26 | |
| kashyap | Thanks for putting up with this | 15:26 |
| kashyap | apevec: https://review.openstack.org/337277 Bump 'global-requirements' for 'oslo.concurrency' to 2.6.1 | 15:37 |
| kashyap | Hope the commit message gives some context to those just catching up | 15:37 |
| apevec | +2 | 15:42 |
| kashyap | Thanks! | 15:45 |
| *** rcernin has quit IRC | 16:03 | |
| *** arif-ali has joined #openstack-stable | 16:03 | |
| *** number80 has quit IRC | 16:20 | |
| *** number80 has joined #openstack-stable | 16:22 | |
| *** rcernin has joined #openstack-stable | 17:08 | |
| *** tesseract- has quit IRC | 17:09 | |
| *** pcaruana has quit IRC | 17:34 | |
| *** number80 has quit IRC | 17:37 | |
| *** number80 has joined #openstack-stable | 17:41 | |
| *** armax has joined #openstack-stable | 18:07 | |
| *** armax has quit IRC | 18:13 | |
| *** armax has joined #openstack-stable | 18:13 | |
| *** armax has quit IRC | 18:29 | |
| *** e0ne has joined #openstack-stable | 19:07 | |
| *** e0ne has quit IRC | 19:11 | |
| *** arif-ali has quit IRC | 19:31 | |
| *** arif-ali has joined #openstack-stable | 19:50 | |
| *** e0ne has joined #openstack-stable | 20:08 | |
| *** e0ne has quit IRC | 20:21 | |
| *** e0ne_ has joined #openstack-stable | 20:22 | |
| *** e0ne_ has quit IRC | 20:40 | |
| tonyb | stable meeting in ~5mins | 20:54 |
| *** rcernin has quit IRC | 22:15 | |
| *** apevec has quit IRC | 22:24 | |
| *** catintheroof has quit IRC | 22:24 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!