Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-21
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.
14:51:22 cdent on the other hand it is entirely possible I'm putting too much trust in the existing gabbi tests, but since they pass, I feel pretty okay
14:51:50 alex_xu I think i get your point now
14:55:30 openstackgerrit Stephen Finucane proposed openstack/nova master: Add description to policies in assisted_volume_snapshots.py https://review.openstack.org/445238
14:55:44 openstackgerrit Stephen Finucane proposed openstack/nova master: Add description to policies in admin_password.py https://review.openstack.org/443957
14:55:45 edleafe cdent: you mean there are other ways to use OpenStack other than telnet?
14:56:16 rfolco mriedem, edleafe: re: first pike bp implemented: nice! thanks to edleafe, cdent and jaypipes for great guidance :)
14:56:17 cdent edleafe: only if you pass the awkward test
14:56:35 cdent \o/ rfolco
14:59:00 alex_xu if we check the presence the body, that means we have to read the body first

Earlier   Later