19:00:44 <lbragstad> #startmeeting keystone-office-hours
19:00:47 <openstack> Meeting started Tue Aug  8 19:00:44 2017 UTC and is due to finish in 60 minutes.  The chair is lbragstad. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:48 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
19:00:50 <openstack> The meeting name has been set to 'keystone_office_hours'
19:00:52 <cmurphy> o/
19:01:02 <mordred> cmurphy, morgan, lbragstad: I agree with what I see in the summary - adding a microversion header without actually supporting microversions seems like a very bad idea
19:01:16 <knikolla> o/
19:01:30 <lbragstad> mordred: ack
19:01:47 <lbragstad> mordred: we've punted that until we can discuss our approach to microversions at the PTG
19:02:49 <mordred> ++
19:02:54 <mordred> lbragstad: I think that's a great plan
19:04:08 <lbragstad> cmurphy: i think https://bugs.launchpad.net/keystone/+bug/1692090 needs more info
19:04:08 <openstack> Launchpad bug 1692090 in OpenStack Identity (keystone) "_dn_to_id ignores user_id_attribute" [Low,In progress] - Assigned to Boris Kudryavtsev (bkudryavtsev)
19:04:20 <lbragstad> cmurphy: based on your comment - i'm inclined to think you agree
19:05:04 <cmurphy> lbragstad: yes i think that might be solveable in config
19:05:34 <lbragstad> cmurphy: ack - removed from rc1 and marked as Incomplete
19:06:04 <cmurphy> lbragstad: also it seemed like the solution was making another round trip to ldap which is :(
19:07:03 <lbragstad> yeah..
19:07:43 <lbragstad> morgan: your 410 gone patch addressed https://bugs.launchpad.net/keystone/+bug/1696308 ?
19:07:43 <openstack> Launchpad bug 1696308 in OpenStack Identity (keystone) "list revoked tokens API returns 500 when pki_setup is not run" [Wishlist,Triaged] - Assigned to Nisha Yadav (ynisha11)
19:08:39 <morgan> yeah it does
19:20:00 <ayoung> jeremyfreudberg, HTTP_X_USER_ID, HTTP_X_SERVICE_USER_ID
19:56:41 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: Attempt caching list_projects_for_user  https://review.openstack.org/487143
20:23:52 <lbragstad> cmurphy: ^ that passes tests now
20:23:59 <lbragstad> (at least locally)
20:24:14 <cmurphy> sweet
20:24:18 <lbragstad> i have a patch for my other todo from today's meeting
20:24:38 <lbragstad> running tests locally at the moment
20:24:40 <openstackgerrit> Eric Fried proposed openstack/keystoneauth master: WIP: Adapter.get_conf_options(deprecated_opts)  https://review.openstack.org/490895
20:25:47 <cmurphy> lbragstad: why did the hints arg get dropped?
20:25:54 <cmurphy> that seems not backwards compatible
20:26:37 <lbragstad> cmurphy: still working through that bit
20:26:42 <lbragstad> actually
20:27:20 <lbragstad> our caching decorator doesn't let us cache methods that accept kwargs
20:27:24 <lbragstad> :-/
20:28:42 <cmurphy> hrm :(
20:29:38 <lbragstad> cmurphy: oh!
20:29:48 <lbragstad> cmurphy: i remember now
20:29:59 <lbragstad> cmurphy: no only does it cause weird things with caching
20:30:08 <lbragstad> cmurphy: it's not even used
20:30:15 <lbragstad> https://review.openstack.org/#/c/487143/2/keystone/assignment/core.py
20:30:32 <cmurphy> oh you're right
20:30:53 <lbragstad> i should pull that out into it's own change
20:31:09 <cmurphy> yes please
20:35:06 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: WIP: Unset project ids for all identity backends  https://review.openstack.org/491916
20:57:28 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: Cache GET /v3/users/{user_id}/projects  https://review.openstack.org/487143
20:57:29 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: Remove hints from list_user_projects API  https://review.openstack.org/491921
20:57:34 <openstackgerrit> Eric Fried proposed openstack/keystoneauth master: Adapter.get_conf_options(deprecated_opts)  https://review.openstack.org/490895
20:59:18 <efried> ^ ready; closes bug https://bugs.launchpad.net/keystoneauth/+bug/1708673
20:59:18 <openstack> Launchpad bug 1708673 in keystoneauth "Register deprecated opts with Adapter.get_conf_options" [Undecided,In progress] - Assigned to Eric Fried (efried)
21:01:26 <openstackgerrit> Merged openstack/keystone master: Document required `type` mapping attribute  https://review.openstack.org/491478
21:07:37 <cmurphy> lbragstad: so with 491921 - do we need to worry about it breaking out-of-tree drivers?
21:07:45 <cmurphy> this isn't breaking an api contract at all?
21:09:18 <lbragstad> i don't believe it is? but i'll walk through how i understand it to be sure
21:09:58 <lbragstad> so before that change - that api will have attempted to extract things from the request and builds a hints object
21:10:13 <lbragstad> based on query strings and whatnot
21:10:45 <lbragstad> regardless of what the user passed in - keystone would always return the same list of assignments (which is arguably broken behavior)
21:11:12 <lbragstad> so - as far as what keystone returns, it should be the same before and after the patch
21:11:57 <lbragstad> from a driver perspective - the hints object was never passed to a driver so I don't think it should affect folks maintaining their own assignment backend
21:12:10 <cmurphy> okay
21:13:39 <lbragstad> cmurphy: call me out on it if that doesn't seem right though
21:14:15 <cmurphy> lbragstad: no that makes sense
21:14:26 <cmurphy> lbragstad: minor comment on the patch
21:14:34 <lbragstad> cmurphy: reading
21:15:28 <lbragstad> this might be worth investigating though? https://github.com/openstack/keystone/blob/de5efb234809c1af43f8d98c29759588c0333f29/keystone/assignment/controllers.py#L273
21:15:47 <lbragstad> just to see if wrap_collection does anything with hints in the response
21:16:10 <lbragstad> (which would mean it would be inconsistent with the actual response body since it was never passed to the backend)
21:17:05 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: WIP: Unset project ids for all identity backends  https://review.openstack.org/491916
21:17:06 <cmurphy> hmm iirc it does do things, like imposing list limits
21:17:14 <lbragstad> cmurphy: right
21:17:16 <lbragstad> ^
21:19:57 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: WIP: Unset project ids for all identity backends  https://review.openstack.org/491916
21:21:13 <clarkb> lbragstad: I think I have time now to look at that grenade thing again. Looks like the logs for the case I found have already been expired and deleted :/
21:22:14 <lbragstad> clarkb: yeah - cmurphy and i noticed that earlier
21:22:36 <clarkb> my memory of the original case was that tests were failing due to the bug so it wasn't just a warning. IIRC nova couldn't boot instances because some system user apparently did not exist
21:22:50 <clarkb> that said all of the hits for your logstash query are failed jobs
21:23:08 <clarkb> so I don't think its "normal" at least not during tempest runs
21:24:08 <clarkb> oh except those are all for the midonet job which likely is just broken
21:24:32 <clarkb> oh and that was only last 15 minutes derp
21:25:08 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: Remove hints when listing domains and project for users  https://review.openstack.org/491921
21:28:11 <openstackgerrit> Gage Hugo proposed openstack/keystone master: WIP - Add description for relationship links in api-ref  https://review.openstack.org/491934
21:28:46 <gagehugo> lbragstad: ^ WIP but let me know if that would be a good approach to take for describing the relationship links
21:29:10 <clarkb> lbragstad: I noticed http://logs.openstack.org/17/479517/20/check/gate-grenade-dsvm-neutron-ubuntu-xenial/94d3489/logs/apache/keystone.txt?level=WARNING#_2017-08-08_21_10_44_518 while digging into logs for the earlier issue, not sure if this is expected (maybe just a bad patch?)
21:30:25 <lbragstad> gagehugo: thanks
21:30:34 <lbragstad> clarkb: interesting - that seems consistent with our direction
21:31:36 <lbragstad> clarkb: https://github.com/openstack/keystone/blob/de5efb234809c1af43f8d98c29759588c0333f29/keystone/middleware/core.py#L51-L71
21:31:51 <lbragstad> might need to update the paste file for that service?
21:32:08 <openstackgerrit> Eric Fried proposed openstack/keystoneauth master: Protect against missing interface attribute  https://review.openstack.org/488568
21:32:12 <efried> ^ ready; closes bug https://bugs.launchpad.net/keystoneauth/+bug/1707273
21:32:12 <openstack> Launchpad bug 1707273 in keystoneauth "get_adapter_conf_options(include_deprecated=False) results in NoSuchOptError" [Undecided,In progress] - Assigned to Eric Fried (efried)
21:32:25 <clarkb> interesting that you chose to log that as an error... should be warning imo. Errors should be for fatal actions
21:32:55 <clarkb> lbragstad: its probably because in grenade we don't update the configs between versions so we write the old version then update install and start new version with old config
21:33:04 <clarkb> (but thats totally not an error)
21:33:55 <lbragstad> clarkb: here's the change https://review.openstack.org/#/c/427878/
21:34:06 <lbragstad> digging into it to see if there is history behind the reasoning
21:34:21 <clarkb> lbragstad: I've tried searching logstash tempest.txt on grenade jobs for timeouts based on the original bugs info, and I'm not finding anything so guessing this bug can be ignored/closed and we'll just have to debug it if it shows up again
21:35:25 <lbragstad> morgan: do you remember the context of why that ^ was an error instead of a warning?
21:35:55 <clarkb> lbragstad: cmurphy so ya I think I'd just mark that as incomplete or invalid until we have more infos
21:36:22 <morgan> uhm....
21:36:32 <cmurphy> clarkb: \o/ best kind of bug
21:36:40 <morgan> yes
21:36:44 <morgan> that is supposed to be an error
21:36:51 <morgan> don't have that in your paste-ini
21:36:56 <clarkb> but it isn't an error if the service is perfectly capable of functioning...
21:37:11 <morgan> it is going away as in it *will* break your cloud when it's deleted
21:37:16 <clarkb> sure definitely log it
21:37:17 <morgan> it's logged as an error because of that
21:37:21 <clarkb> warning would be appropriate
21:37:25 <morgan> i disagree.
21:37:35 <morgan> we did warning before and it wasn't high enough
21:37:45 <morgan> things broke people horribly
21:37:47 <clarkb> the problem with error is anytime I see an error in my logs I think fire
21:38:00 <clarkb> and the problem is lots of software doesn't actually log errors for fires and it leads to people ignoring errors
21:38:02 <clarkb> then you miss real fires
21:38:06 <morgan> this is a fire, if we remove it it errors and breaks the cloud in non-easy to diagnose ways
21:38:08 * clarkb looks at gerrit's logs and has a sad
21:38:21 <morgan> this *must* be removed this release.
21:38:32 <clarkb> morgan: thats not what grenade says
21:38:42 <lbragstad> morgan: you mean in Queens?
21:38:45 <clarkb> grenade says keystone is working fine despite the error
21:38:47 <morgan> before queens
21:39:08 <clarkb> next release then
21:39:10 <morgan> if it is not removed in queens you break. and break badly. paste-ini is many times CMS managed (sigh)
21:39:18 <clarkb> not this release
21:39:28 <morgan> it must be removed in this release, not there by next
21:39:37 <clarkb> (so grenade is doing the right thing)
21:39:49 <morgan> if it is still there next release, you are 100% broken and it is not a clear error
21:39:57 <morgan> paste errors are really unclear/unfun
21:40:00 <morgan> and confusing
21:40:19 <morgan> this is an error case. it is an operator must make a change.
21:40:20 <clarkb> anyways my point is it works fine in pike as evidenced by grenade
21:40:28 <lbragstad> this says if was deprecated *this* release and staged for removal in Queens https://github.com/openstack/keystone/blob/de5efb234809c1af43f8d98c29759588c0333f29/keystone/middleware/core.py#L55-L58
21:40:36 <clarkb> and there are better ways to address that (like the work to make paste data not config)
21:40:38 <morgan> lbragstad: correct
21:40:46 <morgan> clarkb: i tried, i lost that battle
21:40:53 <clarkb> if it was actually an error grenade should fail imo
21:40:58 <morgan> the way to do that is delete paste from our deps
21:41:04 <morgan> you can't make it not config otherwise
21:41:18 <morgan> lbragstad: when the code is deleted, paste fails if it's still there
21:41:51 <clarkb> as is you have a honeypot for people debugging real errors that will only cause confusion
21:41:54 <morgan> clarkb: i wanted to remove paste and make everything a simple wsgi app, i was told in no uncertain terms by other cores that that was a -2 because people use it as config and add elements to the pipeline
21:42:44 <morgan> well, then i guess we'll just disagree. in my experience, when a change is needed within the cycle that will totally hork your cloud next upgrade, it is worthy of an error
21:44:38 * morgan stands by the decision that it is an error.
21:45:00 <morgan> if the ptl wants to change it, he may. i'm not going to block a change like that.
21:45:15 <morgan> or ptl supporitng a change for it.
21:45:19 <clarkb> I'm just giving my opinion as a person that oeprates a ton of different software and reads a lot of openstack logs
21:45:36 <clarkb> using error too much leads to people ignoring it and also creates confusion when looking for causes of real failures
21:45:43 <morgan> the error log did exactly what it was supposed to do btw
21:45:45 <morgan> then
21:45:54 <morgan> it brought your attention to the paste-ini
21:46:11 <clarkb> yup, but if I'm debugging why nova can't boot an instance that isn't useful
21:46:27 <morgan> as an operator you'd see that and fix your config, no?
21:46:35 <morgan> early on. but it doesn't break your cloud *today*
21:46:38 <clarkb> yes, but not while I am firefighting
21:46:43 <clarkb> its just noise and not helpful
21:47:31 <morgan> anyway, i simply disagree here.
21:48:02 <morgan> this is telegraphing a "will break your cloud" [it's not critical, it is an error in the config] change
21:48:36 <openstackgerrit> Eric Fried proposed openstack/keystoneauth master: Protect against missing interface attribute  https://review.openstack.org/488568
21:48:37 <openstackgerrit> Eric Fried proposed openstack/keystoneauth master: Adapter.get_conf_options(deprecated_opts)  https://review.openstack.org/490895
21:49:11 <efried> okay NOW they're ready.
21:49:50 <clarkb> morgan: you might want to link to docs/release notes in that case
21:49:52 <morgan> clarkb: if it is a huge deal, propose a fix that downgrades it and have lbragstad approve it. i stand by this choice.
21:50:03 <clarkb> morgan: so that it is clear where the delineation is and why things aren't on fire now
21:50:20 <clarkb> as is the message says "you are broken cloud on fire"
21:53:25 <lbragstad> it doesn't look like it's in the cinder paste.ini
21:53:35 <morgan> this is only in the keystone paste-ini
21:56:14 <openstackgerrit> Morgan Fainberg proposed openstack/keystone master: Make an error state message more explicit  https://review.openstack.org/491938
21:56:17 <lbragstad> weird - so where is that getting set?
21:56:23 <lbragstad> oh...
21:56:30 <morgan> grenade doesn't update the paste-ini
21:56:36 <morgan> or use the new one
21:56:56 <morgan> so the code says "oh hey this is a bad config. this will break your cloud in the next release'
21:57:03 <morgan> 'fix your config'
21:57:29 <morgan> if the operator was using the default paste-ini from pike, no issue would occur
21:57:46 <morgan> but if they manage paste-ini as config (which they shouldn't, but i lost that argument as said before)
21:58:03 <morgan> they would need to fix it to prevent a future "omg totally broken"
22:01:50 <lbragstad> #endmeeting