| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-21 | |||
| 14:09:10 | jroll | ironic reports everything used in get_available_resource, but the RT corrects it based on instance.flavor | |
| 14:09:56 | bauzas | jroll: self.driver.estimate_instance_overhead(instance) | |
| 14:10:22 | bauzas | mmm, nevermind | |
| 14:10:29 | bauzas | that one is not what I thought | |
| 14:11:35 | bauzas | looking at the limits now | |
| 14:13:09 | bauzas | yeah bingo | |
| 14:13:41 | bauzas | jroll: https://github.com/openstack/nova/blob/master/nova/scheduler/filters/exact_ram_filter.py#L38-L41 | |
| 14:15:01 | bauzas | jroll: which ends up in https://github.com/openstack/nova/blob/master/nova/compute/claims.py#L247 | |
| 14:15:06 | bauzas | on the compute side | |
| 14:15:37 | jroll | bauzas: neat, but that means people not running exact filters were still broken before this change | |
| 14:15:49 | jroll | bauzas: and means people running exact filters are still not broken after this change | |
| 14:16:05 | bauzas | jroll: so the filter returns a limit that is passed down to the conductor and then the compute which is used there https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L155-L156 | |
| 14:16:13 | jroll | meaning, my change does not introduce more broken behavior, just gets us back to consistent with old behavior | |
| 14:16:21 | mriedem | rfolco: congratulations you are the first pike bp implemented https://blueprints.launchpad.net/nova/+spec/delete-inventories-placement-api | |
| 14:16:31 | mriedem | you get a root beer flavored scratch and sniff sticker | |
| 14:16:38 | bauzas | jroll: lemme consider the impact | |
| 14:16:59 | mriedem | jroll: that's what i said earlier right? | |
| 14:17:11 | mriedem | your patch makes get_inventory the same as it was working for everything before | |
| 14:17:28 | mriedem | the "consume all allocation" is a latent problem | |
| 14:17:32 | mriedem | not introduced by get_inventory | |
| 14:17:39 | bauzas | mriedem: post-placement use yeah | |
| 14:17:48 | cdent | mriedem: I am _so_ motivated now, now that I know there might be scratch and sniff stickers | |
| 14:17:55 | jroll | mriedem: yes, I agree with you | |
| 14:18:09 | bauzas | okay, jroll and mriedem, you're right | |
| 14:18:11 | edleafe | rfolco: w00t! | |
| 14:18:28 | jroll | zomg, that never happens | |
| 14:18:38 | bauzas | we just leave placement returning more hosts | |
| 14:19:00 | bauzas | the allocation consumption problem is only there when you don't use the Exact | |
| 14:19:04 | bauzas | Exact* filters | |
| 14:19:09 | bauzas | okay, so... | |
| 14:23:22 | bauzas | jroll: EmilienM: +2, but holding my +W https://review.openstack.org/#/c/448098/2 | |
| 14:23:34 | bauzas | until we get more results, in particular with 3O job | |
| 14:23:53 | EmilienM | bauzas: ack, thx | |
| 14:25:03 | jroll | thanks bauzas | |
| 14:25:19 | bauzas | jroll: nah, you thanks for tackling that | |
| 14:25:52 | jroll | bauzas: welcome | |
| 14:26:02 | jroll | I promised I'd help with placement stuff, yay I did a thing | |
| 14:26:22 | bauzas | at least we discussed how scary was the IronicHostManager :) | |
| 14:26:35 | bauzas | now mriedem could just throw it under the bus too :p | |
| 14:26:42 | jroll | that's drilled into my head for years now | |
| 14:27:01 | bauzas | FWIW, I'm even not convinced by HostState objects | |
| 14:27:10 | sdague | sfinucan: on https://review.openstack.org/#/c/430912 ... if we put an idempotent id in the commit message, can we put the full one in there? If it's full gerrit makes it a link, the partial isn't | |
| 14:27:37 | bauzas | anyway, /me disappearing b/c paternal duties | |
| 14:28:16 | sfinucan | sdague: Weird. Partial commit refs _do_ work. I'll fix that now | |
| 14:28:46 | sdague | sfinucan: yeh, but not Idempotent ids | |
| 14:28:53 | sdague | I think it's gerrit being conservative | |
| 14:28:54 | sfinucan | sdague: It does in https://review.openstack.org/#/c/430911/5 though? | |
| 14:29:12 | mriedem | rpodolyaka: you've got to sort the projects.yaml file https://review.openstack.org/#/c/448115/1 | |
| 14:29:22 | sdague | sfinucan: only 7 in the unlinked one? | |
| 14:29:33 | sdague | it's still probably good form to do the whole thing though | |
| 14:30:13 | mriedem | rpodolyaka: let me know when we get to the point of needing to setup a launchpad project for bug tracking | |
| 14:30:21 | alex_xu | cdent: what is your expect when no content-length and content-type, 400 or 415? | |
| 14:30:25 | sfinucan | sdague: Aye, it does work with 8 chars | |
| 14:31:12 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Ignore 'allow_same_net_traffic' for port filters https://review.openstack.org/430912 | |
| 14:31:55 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Deprecate 'allow_same_net_traffic' https://review.openstack.org/430913 | |
| 14:32:03 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Final cleanups in conf/network https://review.openstack.org/431023 | |
| 14:32:35 | sfinucan | sdague: Thanks for reviewing those :) | |
| 14:32:42 | cdent | alex_xu: 400 | |
| 14:32:53 | cdent | 415 is only for when the value of content-type is not the one desired | |
| 14:33:07 | alex_xu | cdent: but the https://review.openstack.org/447625 will return 415 | |
| 14:33:08 | rpodolyaka | mriedem: oops, will fix that now | |
| 14:33:31 | openstackgerrit | Béla Vancsics proposed openstack/nova master: Transform instance.trigger_crash_dump notification https://review.openstack.org/396225 | |
| 14:34:00 | rpodolyaka | mriedem: hmm, I already created one - https://bugs.launchpad.net/placement-osc-plugin . Was that too early? | |
| 14:34:39 | cdent | alex_xu: sorry I misunderstood what you meant: I thought you mean no content-type and no content-length. For no content-length but with a content-type then 415 (assuming the content-type is wrong) | |
| 14:35:05 | alex_xu | cdent: yes, I mean no content-type and no content-length | |
| 14:36:16 | cdent | alex_xu: okay, ideally that would be a 400, but I've allowed it to fall through to a 415 because we don't want to do either of the following: | |
| 14:36:28 | cdent | * check for the presence of a body (before actually reading it) | |
| 14:36:42 | cdent | * limit things based on which method, since GET could send a body | |
| 14:36:49 | mriedem | rpodolyaka: less work for me | |
| 14:36:53 | rpodolyaka | :) | |
| 14:37:56 | mriedem | mdbooth: on https://review.openstack.org/#/c/441989/ i noticed there was no unit test, which, whatever, but if having a method defined is required to actually have things work, what are your thoughts on using ABCs in the imagebackend code? | |
| 14:37:59 | cdent | alex_xu: a thing to keep in mind is that is hard, without explicit intent, to not send a content-length header when you have a body | |
| 14:38:18 | mriedem | so if a method is required, like get_model, then define it as an abstractmethod | |
| 14:38:36 | cdent | alex_xu: we could change things back to check for the request method, but since we've had discussions about GET using bodies, I didn't want to build that in | |
| 14:38:53 | mriedem | i guess Image is an ABC | |
| 14:39:15 | mriedem | but get_model() isn't marked as an abstractmethod? that seems wrong | |
| 14:39:21 | cdent | alex_xu: we can change it like you suggest (all in the decorator) but that doesn't feel as robust to me | |
| 14:39:35 | mdbooth | mriedem: I vaguely recall considering that when I looked at it, but we don't use ABCs very consistently so I wasn't that bothered IIRC | |
| 14:39:42 | mriedem | mdbooth: i'll push a change quick | |
| 14:39:50 | alex_xu | cdent: sorry, I didn't get what relationship with GET using bodies | |
| 14:40:06 | mriedem | so when MyCoolImageEnterpriseBackend shows up, we don't have the same issue | |
| 14:40:43 | jroll | EmilienM: I rechecked your test patch, btw | |
| 14:40:49 | EmilienM | thx | |
| 14:40:55 | EmilienM | I'm still in mtg | |
| 14:41:15 | jroll | no worries, just a heads up :) | |
| 14:41:23 | alex_xu | emm...actually, i didn't get why that two points related to my suggestion. I may miss something | |
| 14:43:22 | edleafe | cdent: (reading scrollback) So no content-type with no content is not OK? | |
| 14:43:59 | cdent | alex_xu: there's two things going on in the patch: One is making sure that if there is no body (thus the content-length header is either 0 or not there) then a PUT or POST can work. Two is making sure that if there is a body (content-length header is present and has a value) then the content-type header is present, no matter what the request method is. | |
| 14:44:02 | cdent | edleafe: ^ | |
| 14:45:24 | cdent | Item number two should be true for every request and should lead to a 400 if not the case. Thus having it as part of a decorator is not ideal. | |
| 14:46:03 | cdent | (unless it was part of the wsgify decorator, but that's essentially the same thing as where it is now) | |
| 14:46:04 | edleafe | cdent: I was reading alex_xu's comment at http://p.anticdent.org/1sVC and your reply | |
| 14:46:45 | cdent | edleafe: if there is a body, and there is either no content-type or no content-length, that's a 400 | |
| 14:47:17 | cdent | edleafe: but we only have the content-length header as a reasonable proxy for knowing if there is a body | |
| 14:47:21 | openstackgerrit | Sujitha proposed openstack/nova master: Change os-server-tags default policy https://review.openstack.org/396420 | |
| 14:47:21 | edleafe | cdent: a body with no content-length? | |
| 14:48:22 | cdent | edleafe: a healthy client would never do that, but if, for example, you telnet to port 80 and write a PUT it's easy to do | |
| 14:48:37 | cdent | also, if chunked encoding is being used, all bets are off | |
| 14:48:45 | cdent | but we don't use that, so that's a relief | |
| 14:50:43 | cdent | alex_xu, edleafe: does that help clear things up or just make it more muddy? sadly we can't protect against all cases of "telnet to port 80" but we can cover some. | |