Thursday, 2018-10-18

*** markvoelker has joined #openstack-keystone00:05
*** markvoelker has quit IRC00:10
*** markvoelker has joined #openstack-keystone01:05
*** markvoelker has quit IRC01:09
*** Dinesh_Bhor has joined #openstack-keystone01:20
*** imacdonn has quit IRC01:22
*** imacdonn has joined #openstack-keystone01:22
*** Emine has quit IRC01:22
*** Emine has joined #openstack-keystone01:23
*** Dinesh_Bhor has quit IRC01:53
*** Dinesh_Bhor has joined #openstack-keystone02:03
openstackgerritayoung proposed openstack/keystone-specs master: Explicit Domain Ids  https://review.openstack.org/61120102:05
*** ayoung has joined #openstack-keystone02:11
*** felipemonteiro has quit IRC02:16
*** ayoung has quit IRC02:17
*** Emine has quit IRC02:23
*** Dinesh_Bhor has quit IRC02:38
openstackgerritVishakha Agarwal proposed openstack/keystone master: Set min and max length for resource_name  https://review.openstack.org/61148402:45
*** Dinesh_Bhor has joined #openstack-keystone02:47
*** dave-mccowan has quit IRC02:56
*** rcernin has quit IRC03:02
*** bnemec has joined #openstack-keystone03:05
*** bnemec has quit IRC03:10
*** deepak_mourya__ has joined #openstack-keystone03:12
*** rcernin has joined #openstack-keystone03:28
*** Dinesh_Bhor has quit IRC04:05
openstackgerritVishakha Agarwal proposed openstack/keystone master: Adding 'date' for trust_flush  https://review.openstack.org/60789704:32
*** Dinesh_Bhor has joined #openstack-keystone04:44
openstackgerritVishakha Agarwal proposed openstack/keystone master: Set min and max length for resource_name  https://review.openstack.org/61148404:51
vishakhaayoung : Hello. Regarding https://review.openstack.org/#/c/606912/05:03
vishakhaayoung: I have removed the return from cli.  As there are 3 prints in the mock_call containing Json object. It is difficult to match the expected with the actual as you can see in http://paste.openstack.org/show/731879/. Due to the different key positions the matching is failed.05:10
vishakhaSo I asserted a mock_count in code to confirm that all 3 prints are there but not able to validate stdout05:10
*** Dinesh_Bhor has quit IRC05:13
*** felipemonteiro has joined #openstack-keystone05:24
*** lbragstad_503 has quit IRC05:27
*** lbragstad_503 has joined #openstack-keystone05:27
*** ChanServ sets mode: +o lbragstad_50305:27
*** deepak_mourya__ has quit IRC05:30
*** felipemonteiro has quit IRC06:07
*** Emine has joined #openstack-keystone06:37
*** Emine has quit IRC06:42
openstackgerritJose Castro Leon proposed openstack/keystone master: Add caching on trust role validation to improve performance  https://review.openstack.org/60896306:56
*** rcernin has quit IRC07:07
*** xek has joined #openstack-keystone07:12
*** openstackgerrit has quit IRC07:35
*** rdopiera has joined #openstack-keystone07:52
*** xek has quit IRC07:58
*** Dinesh_Bhor has joined #openstack-keystone08:37
*** pooja-jadhav has joined #openstack-keystone08:46
*** pooja_jadhav has quit IRC08:46
*** Dinesh_Bhor has quit IRC09:03
*** Dinesh_Bhor has joined #openstack-keystone09:28
*** Dinesh_Bhor has quit IRC09:31
*** pooja-jadhav is now known as pooja_jadhav09:33
*** openstackgerrit has joined #openstack-keystone10:16
openstackgerritJens Harbott (frickler) proposed openstack/keystone master: DNM: Test jobs running on bionic instead of xenial  https://review.openstack.org/61156310:16
*** dave-mccowan has joined #openstack-keystone10:18
openstackgerritJens Harbott (frickler) proposed openstack/keystone master: DNM: Test jobs running on bionic instead of xenial  https://review.openstack.org/61156310:18
openstackgerritColleen Murphy proposed openstack/keystone-specs master: Update spec template  https://review.openstack.org/61158312:07
*** raildo has joined #openstack-keystone12:13
*** dims has quit IRC12:30
*** dims has joined #openstack-keystone12:33
*** jrist has quit IRC13:11
*** jrist has joined #openstack-keystone13:13
*** mvkr has quit IRC13:27
*** felipemonteiro has joined #openstack-keystone13:28
*** lbragstad_503 is now known as lbragstad13:32
*** bnemec has joined #openstack-keystone13:43
*** mchlumsky has joined #openstack-keystone13:52
openstackgerritLance Bragstad proposed openstack/keystone master: Implement scope_type checking for credentials  https://review.openstack.org/59454713:53
openstackgerritLance Bragstad proposed openstack/keystone master: Pass context objects to policy enforcement  https://review.openstack.org/60553913:53
openstackgerritLance Bragstad proposed openstack/keystone master: Remove obsolete credential policies  https://review.openstack.org/59718713:54
*** mvkr has joined #openstack-keystone13:57
*** mchlumsky has quit IRC13:57
*** mchlumsky has joined #openstack-keystone13:59
*** felipemonteiro has quit IRC14:42
*** jmlowe has quit IRC14:57
*** itlinux has quit IRC15:01
*** dklyle has joined #openstack-keystone15:06
openstackgerritLance Bragstad proposed openstack/oslo.policy master: Add domain scope support for scope types  https://review.openstack.org/61144315:17
*** gyee has joined #openstack-keystone15:43
*** rook has joined #openstack-keystone15:48
rookkmalloc: hey - curious w/ Rocky is there a way to disable the flask bits, so keystone only uses wsgi?15:48
*** itlinux has joined #openstack-keystone15:51
*** dklyle has quit IRC16:16
*** dklyle has joined #openstack-keystone16:16
*** dklyle has quit IRC16:37
lbragstadrook there isn't a way to disable flask16:55
rooklbragstad: ack - between Queens and Rocky where there any big chnages to keystone that could of impacted performance?16:58
rookthe reason I asked about flask is because it was flooding the log when debug is enabled.16:58
*** mvkr has quit IRC16:58
lbragstadrook you can disable debug if you'd like - which is what we recommend if you're not actively debugging an issue in production16:58
rooklbragstad: but I am activly debugging :)16:59
lbragstadrook ah - well in that case :)16:59
rooktrying to determine why we see a performance difference between rocky and queens :)16:59
lbragstadwe did land a pretty large refactor to the token API16:59
rookthe flask log puke is the only thing i see16:59
lbragstadand it did affect performance, we have a patch for it upstream that we're going to be backporting as soon as it lands in master16:59
rooklet me see if I can share what i am seeing16:59
rookooo got a link handy to the patchset?17:00
lbragstadhttps://review.openstack.org/#/c/608963/ is the fix - not sure if that impacts you17:00
lbragstadit's specific to trust scoped tokens17:00
lbragstadrook https://launchpad.net/bugs/1796887 is the bug report17:00
openstackLaunchpad bug 1796887 in OpenStack Identity (keystone) "Validation of tokens degraded after upgrade to Rocky" [High,In progress] - Assigned to Jose Castro Leon (jose-castro-leon)17:00
lbragstadif that's not what you're seeing, please share what exactly is underperforming and we can help debug17:01
rookhttps://docs.google.com/spreadsheets/d/1EAAT7lFQkJsxwGCboMv2p74_opmMVoLkNIU_thfaLjM/edit?usp=sharing17:01
rookcan you see that lbragstad ?17:01
lbragstadlooks like i need permission17:01
* lbragstad requests access17:01
rookgd google17:02
rooktry again :)17:02
rookso, this does seem relevant17:03
rooksince it is the validation code.17:03
lbragstadinteresting17:03
lbragstadlooks like neutron is the biggest offender?17:04
rookthe textual output is easier to read IMHO.17:04
rookRight17:04
rookvalidating neutron17:04
rookhowever, if you look at neutron (different tab) it seems to be performing better in rocky :)17:04
lbragstaddo you know what you're doing in neutron when that happens?17:04
rookthis is the rally scenario - authenicate.validate_neutron17:05
lbragstadhow easy is it for you to apply patches to your env?17:05
rookhm - this is against master :/ not sure how easy/hard it would be apply to rocky17:06
rooki would just monkey patch it in.17:06
rookunless hrybacki could help me17:06
lbragstadthat's what i would do (`git ready stable/rocky; git review -x 608963; pip install -e keystone`)17:09
openstackgerritMerged openstack/keystone master: Replace JSON Body middleware with flask-native func  https://review.openstack.org/60953517:10
lbragstadrook or i suppose you could do the same thing with master17:12
lbragstadmonkey patch master with the same patch and rerun the tests?17:12
rooklbragstad: ack - let me see what sort of pain that involves.17:13
*** rdopiera has quit IRC17:15
openstackgerritMerged openstack/keystone master: Cleanup keystone.server.flask.application  https://review.openstack.org/60954817:15
rooklbragstad: I must be missing how this patch is actually caching... i think ayoung asked the same question.17:16
lbragstadrook https://review.openstack.org/#/c/608963/5/keystone/assignment/core.py@13217:17
lbragstad^ that decorator implements caching17:17
rookack17:17
lbragstadprior to that patch, building roles for trust scoped tokens wasn't using a memoized method, so the result of the method wasn't being cached17:17
*** dklyle has joined #openstack-keystone17:30
*** mvkr has joined #openstack-keystone17:41
*** dklyle has quit IRC17:42
*** dklyle has joined #openstack-keystone17:50
openstackgerritMagnus Lööf proposed openstack/keystone master: Fix example for getting system scoped token  https://review.openstack.org/61168517:54
*** dklyle has quit IRC18:02
*** lbragstad has quit IRC18:17
*** dklyle has joined #openstack-keystone18:20
*** dklyle has quit IRC18:29
*** lbragstad has joined #openstack-keystone18:48
*** ChanServ sets mode: +o lbragstad18:48
*** dklyle has joined #openstack-keystone19:07
*** dklyle has quit IRC19:14
*** itlinux has quit IRC19:16
*** lbragstad has quit IRC19:21
*** lbragstad has joined #openstack-keystone19:24
*** ChanServ sets mode: +o lbragstad19:24
rooklbragstad: that patched helped slightly in my tests...19:29
rookhowever, i have re-run with newer Rocky bits, and keystone looks better overall..19:30
lbragstadnice!19:35
lbragstadare you still seeing performance degrade with neutron explicitly?19:35
rooknope19:35
rookactually performing better than Queens19:36
rook:D19:36
lbragstadwoot!19:36
lbragstadthat's awesome19:36
lbragstadjust by applying that one patch?19:36
rooknewer rocky bits + the patch19:40
rookI have multiple iterations19:40
rookI am also looking at something else too, might have impacted things19:40
rooknova changed (scheduler) so by default it deploys # of cores / # nova-scheduler workers...19:40
rookwhich also puts additonal pressure on keystone19:40
lbragstadinteresting19:49
rooklbragstad: yeah... i mean, these newer processors have tons of threads...19:54
rookso dual socket machines could easily have 96 threads19:54
rookand if you have ha, that is a ton of workers asking for a lot out of keystone19:54
rook:)19:54
openstackgerritLance Bragstad proposed openstack/keystone master: Implement scope_type checking for credentials  https://review.openstack.org/59454720:12
openstackgerritLance Bragstad proposed openstack/keystone master: Pass context objects to policy enforcement  https://review.openstack.org/60553920:12
openstackgerritLance Bragstad proposed openstack/keystone master: Implement scope_types for user API  https://review.openstack.org/61117920:12
*** openstackgerrit has quit IRC20:36
*** pcaruana has quit IRC20:44
*** itlinux has joined #openstack-keystone21:06
*** raildo has quit IRC21:09
*** bnemec is now known as bnemec-bbl21:20
*** dklyle has joined #openstack-keystone21:36
*** dklyle has quit IRC21:45
kmalloclbragstad: see my comment on https://review.openstack.org/#/c/611121/22:02
kmallocbasically... this is not something testable22:02
kmallocit's code flow.22:02
kmallocwe could try a decorator...but i think it'd be unwieldy22:02
lbragstadyeah - i figured22:03
kmalloclbragstad: any concerns on the current flask (final) stack?22:06
kmallocasking before i go dig into KSM adjustments (mostly extracting a couple things into isolated functions)22:06
*** mchlumsky has quit IRC22:21
*** openstackgerrit has joined #openstack-keystone22:25
openstackgerritMorgan Fainberg proposed openstack/keystone master: Support KWARGS in the cache key generating function(s).  https://review.openstack.org/61112022:25
*** rcernin has joined #openstack-keystone22:33
*** dnguyen has joined #openstack-keystone22:35
openstackgerritMerged openstack/keystone-specs master: Update spec template  https://review.openstack.org/61158322:42
openstackgerritAdrian Turjak proposed openstack/keystone master: Implement auth receipts spec  https://review.openstack.org/61123022:47
*** dnguyen has left #openstack-keystone22:47
adriantkmalloc: when you get a chance, can you give me a little bit of feedback on ^22:48
adriantat the very least around the new auth API bits with flask22:49
kmallocsure.22:49
kmalloclooking now.22:49
kmallocadriant: i hope flask made this somewhat easier22:49
adriantkmalloc: yes and no :P22:49
adriantkmalloc: because authenticate_for_token no longer returns a response, I need to handle some stuff elsewhere22:50
adriantand that's the part I'm not happy with yet or not sure of22:50
kmallochehe22:50
kmallocyeah, it's a bit more explicit22:50
adriantmostly my changes in keystone/api/auth.py22:50
openstackgerritAdrian Turjak proposed openstack/keystone master: Implement auth receipts spec  https://review.openstack.org/61123022:53
adriantkmalloc: are we doing any custom error handling for the flask APIs? As in, when we raise an Unauthorised error, where is that caught and returns a 401 to the user?22:56
adriantpart of my was thinking that rather than handling that error and returning the receipt in auth.py, we could do it in that same layer we catch the other errors22:56
adriantbut that may in turn making it harder to find/debug22:56
kmallocadriant: https://review.openstack.org/#/c/609796/22:58
kmallocadriant: it will merge *soon*ish to move to flask handling exceptions22:58
adriantOH, so until that is merged the exceptionss via flask aren't really being handled?23:01
adriantor did we have some weird flask+webob handling?23:02
adriantah yes23:02
adriant URL Normalizing Middleware23:02
adriantalright, that makes more sense to me. Just not sure if having auth receipt creation there is a good idea :/23:05
kmallocyou should add a specific handling case to the error handler, like we have for the Unauthorized.23:13
kmallocimo23:13
kmalloconce that merges.23:13
kmallocit makes sense to go in the handler23:14
adriantworks for me, and makes it nicer than making auth.py ugly23:14
kmallocit modifies the response in some cases rather than in the auth view.23:14
kmalloc++23:14
kmalloci have lots of comments on your code.23:14
kmallocbtw.23:14
adriantas expected! and it's still missing unit tests.23:15
kmallocalmost done with the first pass review, and should be enough to justify another patchset and enough changes to allow for another pass on a future patchset23:15
adriantplus I might rebase my change on the exception handling one so I can get that code written before it gets merged23:16
kmalloccomments on patchset 323:18
kmalloclots of comments, like i said ;)23:19
adriantkmalloc: I'm ok with lots of comments, it means someone at least cares about the code!23:20
kmalloc:)23:21
kmallocthe biggest comment is23:21
kmalloci would just lean on all the token invalidation queues23:21
adriantwill try and address them tonight and hopefully have a patch soon. Plus I need to rewrite the unit tests and port those over.23:21
kmallocif you need to invalidate the token cache, you need to invalidate the receipt cache (probably)23:21
* adriant nods23:21
kmalloc(global invalidate)23:21
adriantthat makes sense23:22
kmallocit's tough to cache the receipts since they aren't issued with concrete values like tokens e.g. user_id23:23
kmallocso you can't really invalidate tokens for a user23:23
kmallocor receipts23:23
kmallocyou might want to lean on the revocation events as well23:23
kmallocso user password change invalidates the receipts as well as the tokens.23:23
kmalloclbragstad: you here?23:24
kmallochave some questions on scope_type bits23:24
*** felipemonteiro has joined #openstack-keystone23:48
*** bnemec has joined #openstack-keystone23:50
lbragstadkmalloc go ahead and ask, i should be back a bit later23:50
kmalloclbragstad: commented23:50
lbragstadok23:50
kmalloca minor change needed really23:50
kmallocthe try/excepts can all be collapsed to a build_target function23:50
kmalloci gave you an example23:50
kmallockeeps the API / resource code cleaner23:51
kmalloclbragstad: beyond that, the code looks good, i need to more closely review the tests23:51
kmallocbut nothing looked broken that side23:52
*** bnemec-bbl has quit IRC23:52
openstackgerritMorgan Fainberg proposed openstack/keystone master: Support KWARGS in the cache key generating function(s).  https://review.openstack.org/61112023:54
*** markvoelker has joined #openstack-keystone23:57
*** gyee has quit IRC23:59

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!