Thursday, 2017-02-09

*** gouthamr has quit IRC02:16
*** openstackstatus has quit IRC02:58
*** openstackstatus has joined #openstack-shade03:02
*** ChanServ sets mode: +v openstackstatus03:02
*** TheJulia_ has joined #openstack-shade06:29
*** TheJulia has quit IRC06:33
*** TheJulia_ is now known as TheJulia06:33
*** abregman has joined #openstack-shade06:35
*** yfried has joined #openstack-shade07:21
*** ioggstream has joined #openstack-shade07:21
*** ioggstream has quit IRC07:37
*** ioggstream has joined #openstack-shade08:27
*** ioggstream has quit IRC08:46
*** abregman has quit IRC09:01
*** openstackgerrit has quit IRC09:03
*** abregman has joined #openstack-shade09:34
*** cdent has joined #openstack-shade09:54
*** cdent has quit IRC10:37
*** cdent has joined #openstack-shade11:09
*** ioggstream has joined #openstack-shade13:08
mordredShrews: could I bother you good sir for reviews of https://review.openstack.org/429925 and https://review.openstack.org/429911 and https://review.openstack.org/429328 ? (the first is the most important, I'd like to cut a release once it's in)13:54
*** gouthamr has joined #openstack-shade13:56
Shrewsmordred: sure14:08
*** jlk has quit IRC14:22
Shrewsmordred: 429328 is probably going to merge conflict once the COMPUTE_ENDPOINT change merges (just approved)14:35
Shrewsoh, nm. it's before it14:35
Shrewsduh14:35
mordredShrews: I do agree though - they would totally otherwise conflict :)14:36
mordredShrews: I had to stop halfway through on that one because doing the port test (where I added some dicts from citycloud) just about killed me14:36
Shrewsmordred: umm... how do those tests pass if we aren't yet using a raw neutron client?14:40
mordredShrews: requests_mock overrides the underlying requests.Session object14:40
mordredShrews: so they're mocking out the requests calls that neutronclient is making14:40
Shrewsmordred: ah right14:41
mordredwhen we make the switch to raw_neutron_client, if we did things right, there should just about no changes to the tests14:41
mordredit's magic :)14:41
Shrewshard to keep the new stuff straight in my head14:41
* mordred hugs jamielennox and morgan for requests_mock14:41
Shrewstoo much context switching14:42
mordredShrews: ++14:42
*** openstackgerrit has joined #openstack-shade15:06
openstackgerritMerged openstack-infra/shade master: Pass task to post_task_run hook  https://review.openstack.org/42992515:06
mordredShrews: thank you!15:10
openstackgerritMerged openstack-infra/shade master: Transition half of test_floating_ip_neutron to requests_mock  https://review.openstack.org/42932815:17
openstackgerritMerged openstack-infra/shade master: Rename ENDPOINT to COMPUTE_ENDPOINT  https://review.openstack.org/42991115:21
*** abregman is now known as abregman|afk15:29
mordredthingee: you'll enjoy https://github.com/ansible/ansible/issues/2119115:39
mordredthingee: for some values of enjoy15:39
*** abregman|afk has quit IRC16:20
*** yolanda_ is now known as yolanda16:41
*** yfried has quit IRC16:46
*** cdent has quit IRC17:03
*** jlk has joined #openstack-shade17:29
*** ioggstream has quit IRC17:31
* morgan starts writing requests_mock for keystonein shade17:41
*** cdent has joined #openstack-shade17:42
mordredyay!17:43
mordredmorgan: you probably see it in the suite, but I've got a v2 and a v3 catalog and fixtures/helpers for getting those set up17:44
morganoh nice17:44
morgani hadn't gotten that far17:44
morgani literally was just starting with "test_users" and converting it over to start17:44
* mordred looks forward to the death of keystoneclient17:44
mordredmorgan: yah17:44
morganand since today is a nice rainy-day in seattle, and I am loaded up with coffee...17:45
morganwooooooo17:45
mordredwell - if you base from RequestsMockTestCase it'll get you set up with the catalog stuff without you needing to do much anyway17:45
morgancool17:45
morgangood to know we have a simple starting place17:45
morganalso yes, requests_mock is awesome17:45
mordred+10017:45
morganjamielennox did great working getting it started17:45
morgani just do some limited maintenance on it17:46
morganin case we have bugs17:46
morgan> 1 core = good plan17:46
mordredmorgan: I just verified - RequestMockTestCase gets you a v3 catalog and stuff added for the token exchange and discovery and whatnot - if you want do test things against v2 auth instead, there is a "use_keystone_v2" method on the test case that will add the v2 versions instead17:47
morgannice.17:47
* morgan nods.17:47
morgani was just looking at it.17:47
morganmordred: sooooo i hear the "retro" thinkpad is a "this year" thing17:47
mordredmorgan: OOOOOOOO17:48
morganmordred: fyi.17:48
morganmordred: rumor mills and whatnot17:48
* mordred likes that rumor17:48
morganhehehe17:48
morganright?!17:48
mordredhere's hoping it has the quality17:48
mordredI'm getting sick of the keys on this thing sticking17:48
morganhonestly, I have zero issues with the X1C keys sticking17:49
morganit's why i've stuck with the X1Cs17:49
morganeven with the other odd issues (but i seem ot just get lucky on that front)17:49
morganhmm.17:50
morganmordred: do you mind if i get rid of the @patch object stuff for say OCC as well?17:50
* morgan has never liked the @patch decorators17:50
morganalways context managers17:50
morganand/or setup/cleanups17:50
morganmordred: but i don't want to remove the decorator if that is the "general" way you like to patch OCC.17:51
morganoh wait. derp17:51
morganthat may not be needed17:51
morgansince we have a real catalog now.17:51
mordredmorgan: the eventual goal is to not use  mock.patch in any way17:56
mordredmorgan: so removing it is awesome17:57
morgancool17:57
morganwe'll see how this works.17:57
mordredthat said - the decorator is the 'general' way we patch things when we patch them (although there are a few instances of context manager)... but I'd rather just get rid of mock patch completely than align in either direction :)17:57
mordredand yah - having real catalog makes many of the patch things just unneeded (again, yay requests_mock!)17:58
morganmordred: my coffee did not stay warm enough18:05
morganmordred: it means i didn't drink it fast enough18:05
mordredmorgan: that is terrible!18:07
mordredmorgan: I got a carafe recently which I've been putting the coffee in to so that it stays warm between mugs18:07
jlkmorgan: so, K5 eh?18:07
morganmordred: i have an insulated french press, it's just the coffee in the cup is cold18:07
morganjlk: k5?18:08
mordredjlk: yah man18:08
jlkfujitsu cloud that is OpenStack, except for where it isn't.18:08
morganoooh18:08
mordredjlk: I support lots of crazy things in shade ... I'm going to draw the line at a divergent nova api that changes semantics of things18:08
jlkmordred: your response was remarkably civil18:08
mordredjlk: well, it's not that dude's fault. I feel bad for him18:08
mordredmorgan: fujitsu k5 apparently makes _everything_ az aware and many resources per-az that are not per-az in normal openstack18:09
mordredthe example in question is keypairs, which are apparently a per-az resource in the k5 cloud18:09
mordredthat COMPLETELY changes the cloud interaction18:10
morganoh wow18:11
morganhmm. ooooo, need to implement a token mock now.18:12
morganor.. wait.. wtf.18:13
morganmordred: use_keystone_v2() is doing something weird still. digging, but i'm still getting requests sent to v3/auth/tokens18:13
morganooh aha, your discovery.json is getting in the way.18:19
morganmordred: how did you select 192.168.0.19 as the keystone endpoint?18:21
morganlooking at use_keystone_v3 and use_keystone_v218:21
morganmordred: and the values from discovery.json?18:21
mordredmorgan: I think it was just a payload taken from a devstack or something18:24
morganmordred: ah ok because trying to use requests_mock and keystoneclient is resulting in all sorts of "get tokens from example.com"18:24
mordredexcellent. well - fixes to the catalog/discovery are more than welcome :)18:25
mordredmorgan: (don't miss that there are also clouds.yaml files in the fixtures dir which point to the 192.168.0.19 as an auth_url too)18:26
morganyeah i think i'm inclined to make it all run through example.com unless we want to change the discovery.json18:26
morganyeah.18:26
mordredI think making it all sane sounds great!18:26
morgani mean long term we shouldn't need specific overrides except where we are explicitly overriding18:26
morganand testing that18:26
morgani'll poke at magical use of clouds.yaml for now18:27
morganthough18:27
morgans/magical/useful18:27
morganhuh18:27
morganweird. this actually looks like something that should work as setup18:27
* morgan digs more...18:27
morganstupid rabbit hole18:27
mordredyay rabbit holes!18:28
mordredwell - also, as you mentioned a little while ago ...18:29
mordredkeystoneclient is ... special18:29
morganyeah... *sigh*18:29
morganmordred: self.magic_fixes(config) you scare me sometimes18:29
mordredmorgan: hell yes I do18:29
mordredmorgan: this is your daily reminder that within these projects lay all of the dirty secrets and evil misdeeds of the past 7 years18:30
mordredto gaze directly upon them is to gaze into the gaping maw of insanity18:31
morganoh18:31
morganso, looks like _make_test_cloud is always pulling _test_cloud18:31
morganunfortunately.... somehow keystoneclient is ignoring all this and actually doing discovery18:31
mordredlike - it's doing discovery again even though ksa already did discovery?18:32
morganwell no, it's KSA doing discovery18:32
morganbut it seems to be just ignoring the clouds.yaml auth_url18:32
morganso it is actually doing discovery and trying to post to example.com/v3/auth/tokens18:33
morganit also seems ot be ignoring identity_api_version18:33
mordredit's entirely possible that we're not plumbing identity_api_version properly18:33
mordredwe _do_ expect a POST to example.com/v3/auth/tokens though18:33
morganthat is actually what it is looking like18:33
morganwell we do, except we don't in this case18:34
mordredah18:34
morgani think the correct answer is to pass a cloud to make_test_cloud and use _test_cloud_v218:34
morganvs _test_cloud18:34
morgansomehow the passing of idenitty_api_version isn't being processed deep in the cloud creation18:34
morganuse_keystone_v3() works because _test_cloud already sets that value18:35
morgani bet if i didn't specify identity_api_version it would break in the same way18:35
morganin clouds.yaml that is18:35
mordredI'm confused ...18:36
morganso am i.18:36
mordred:)18:36
morgani'm digging through this and seeing why the rest class stuff works w/ KSA just fine when v3 is used but it doesn't for v218:36
mordredso we have a test that's using v2 ...18:37
morganspecifying an auth url should prevent the need to do discovery18:37
morganfor the token.18:37
mordrednot if it's an unversioned auth_url18:37
morganah that might be the issue18:37
morganyep, lookie there18:38
morganidentity_api_version=2 is not being set18:38
morganeverything else should be "fine"18:38
mordredit should be being set in use_keystone_v2() though18:38
morganok something is wonky in passing identity_api_version to get_one_cloud18:38
mordredAWESOME18:38
morganno, i think it is not being set in get_one_cloud, it is being passed into geT_one_cloud by _make_test_cloud18:38
mordredyah. and things passed as kwargs _should_ override values that come from config18:39
morganso we're still trying to hit keystone v3 for tokens even when keystone v2 is used.18:39
mordredblerg18:39
morgannow... short term fix18:39
morganwe could make use_keystone_v2() use _test_cloud_v218:39
mordredwait - hang on - why is this working in shade/tests/unit/test_caching.py ?18:39
morganlet me see if i can fix the kwarg prossessing though18:39
morganlooking18:39
mordred(test_modify_user_invalidates_cache)18:40
morganbecause you mock keystone_client18:40
mordredso in test_modify_user_invalidates_cache ksa does v2 discovery but then if we pass that session to ksc ksc doesn't get the values we picked up so it does discovery again itself?18:41
morganno, you explicitly mock .keystone_client18:41
morganwith @patch object18:41
morgandiscovery doesn't occur18:41
morganyou hit a mock object on cloud.keystone_client18:41
mordredah. gotcha18:41
morgan    @mock.patch.object(shade.OpenStackCloud, 'keystone_client')18:41
morganso that property never calls ksa.18:42
morganand we never hit discovery18:42
morgan.use_keystone_v2() in that test is pointless ;)18:42
mordred\o/18:42
morgani mean, it does register_uri, just doesn't ever get used.18:42
mordredyah - and I don't have a self.assert_calls() there which I'm guessing would explode18:42
morganyup18:43
morgani'm not going to add that18:43
morgan:P18:43
morgani'm going to see why shade is not working right18:43
mordredso two thoughts:  long term setting identity_api_version = 2 should make v2 get selected even if there is a v3 available18:43
mordredEXCEPT18:43
mordredthis is weird because it's keystone18:43
morgani think setting identity_api_version is busted here18:44
mordredand there is _also_ inference about v2 and v3 based on whether or not one has domain params or not18:44
mordredyup. I agree, it almost certainly is :)18:44
morganand we should always adhere to explicit setting over inferred18:44
mordredbut the question is - which should win, identity_api_version or someone passing in domain params18:44
morgani don't care if someone passes domain params18:44
morganif they pass or lean on a v2 cloud config18:44
morganv218:44
mordredsince one could argue that identity_api_version is _really_ about keystone crud18:44
morganyou can always pass identity_api_version as well as domain data18:45
morgannow... the sticky bit18:45
morganwe break behavior if we change this18:45
morgan*sigh*18:45
morgansooooooooooooo18:45
morganthe correct answer is inferred wins18:45
morganso shade doesn't break behavior18:45
mordredyah. we choose the auth plugin based on whether or not you have domain params18:45
* morgan grumbles.18:45
mordredso if you have domain params, we make a passwordv3 object18:45
morganyep18:45
morganregardless if you say "do v2 only"18:46
mordredyup18:46
morganmaybe we need an "identity_auth_version"18:46
mordredthis is where your longstanding complaint about combining the auth and the crud really shines18:46
morganparam18:46
mordred++18:46
* morgan backlogs that18:46
*** jlk has quit IRC18:46
mordredyah - let's say that identity_api_version is only for identity crud (since that's all it does now anyway)18:46
morgani'm going to make use_keystone_v2() *also* set token auth18:46
morganfor v318:46
morganand i'm going to make it so you can specify a cloud in .make_test_cloud18:47
mordredI'm confused by that last thing ... can you say that differently?18:47
morganso we could use _test_cloud_v218:47
mordred(the token auth part)18:47
morgan.use_keystone_v2 will also patch example.com/v3/auth/token18:47
morgansince .use_keystone_v2 is strictly a CRUD directive18:47
morganfor now18:47
morgans/patch/mock18:47
mordredgotcha. works for me18:47
morgan3 fixes, 2 short term, 1 long18:48
mordredyup18:48
morganshort term (test only): patch v3 for .use_keystone_v2() && allow specifing a cloud in .make_test_cloud18:48
mordred++18:48
morganlong term, allow a identity_auth_version18:48
morganwhich overrides any inference on domain etc18:48
morganthat is a much bigger change18:48
mordredit is - and we'll have to be careful about openstackclient too18:49
morganand i kindof want to attack keystone's crud+auth bit (I have a spec) so we can make _plain_auth a reality there too18:49
morganunrelateD: my coffee is warm again18:49
morganyay insulated french press18:49
* morgan is onto ~.8L of coffee so far today18:50
morganoh gross18:50
mordredyou'll be saying that a lot18:51
morganthis may be more broken than thought18:51
morgantesting something18:51
* dtroyer catching up18:52
morgandtroyer: :) it's thankfully 100% internal to shade behavior18:52
dtroyermordred, morgan: my thought has always that user/caller setting xx_api_version wins18:52
dtroyerdefault xx_api_version is last resort18:52
morgandtroyer: right. except we treat auth separate from CRUD18:53
dtroyerok, cool, it's nice if we're on the same page for expectations too :)18:53
morgandtroyer: yep.18:53
dtroyergotcha18:53
morgandtroyer: the hard part is ... we already behave this way18:53
* dtroyer retreats to "mordred mode" (waiting for airplanes)18:53
dtroyerah18:53
morgani would rather have api_version dictate both18:53
morganbut since shade already uses values from auth: to select plugin, regardless of api version...18:54
mordredmorgan: it's possible our behavior is broken enough that fixing it won't break people18:54
morgannot something changable w/o potentially breaking people18:54
morganmordred: i doubt it is that broken18:54
dtroyerso if we change lets all do it together :)18:54
morgani actually... sadly... expect people are using it JUST this way18:54
morganmordred: auth with V3, but do V2 actions18:54
morgannotably,, because service accounts18:54
morgana v3 admin account can do V2 things (don't get me started on the security BS here)18:55
mordredluckily - and changes to that can get caught by devstack18:55
morgan(i lost that battle and api contracts blah blah blah)18:55
morganthis mostly all goes away when V2 dies18:55
morganand we're realistically ~1.5yrs from that18:55
mordredwe're infinity away from that18:56
mordredbecause v2 doesn't die from shade/occ18:56
* mordred ducks18:56
morganno i mean where it becomes a tonne less important to worry about the oddness here18:56
mordredindeed18:56
morganand we can just maintain behavior for most cases18:56
morganand anyone using v2 gets told "yeah, it's how v2 works"18:56
morgan"use v3, please"18:57
mordredmorgan: WHY DO DIFFERENT CLOUDS HAVE DIFFERENT FORMATTED VERSION DISCOVERY FOR KEYSTONE?????19:02
morganmordred: because we suck.19:02
morganmordred: actually more to the point keystone team was not opinionated enough and didn't set a format and allowed anything into the catalog19:03
morganand discovery followed that pattern19:03
mordredmorgan: I'm doing a keystone version of the nova script I made a little while ago19:03
mordredto run the version discovery on all of my clouds and print the versions found19:03
morgannow... to be fair19:03
morgani actually think by the time we used discovery for anything it was a fixed set of values from keystone19:03
morganaka, json-home19:04
morganwhich means invalid formatting isn't keystone19:04
morganwhich means someone is not using keystone to serve that data19:04
morganwhich means... not a cloud?19:04
* morgan snarks a little at that last one.19:04
morganmordred: do you have explicit examples? I'd like to see the differences19:04
morganmake sure we didn't screw up here.19:04
morganbut it should be pretty much standard.19:05
Shrewsmordred: ooh, that sounds like a handy little script. that should be included in that user debugging script we keep talking about and never implementing19:06
* morgan head-desks.19:06
morganwhy does keystoneclient require explicit session ot be passed to .get_endpoint() !? *ugh*19:07
mordredmorgan, Shrews: http://paste.openstack.org/show/598284/19:08
morganmordred: can you show me dreamhost vs one that works?19:09
morganjust since you have the raw data. i mean... i could go look it up too19:09
morganmost of those look correct19:09
mordredhttp://paste.openstack.org/show/598285/ is the script19:10
mordredmorgan: dreamhost failed becuse my account is borked19:10
mordredmorgan: the script now accounts for all discovery docs19:10
mordredmorgan: I can also pprint the discovery if you like19:10
morganmordred: uh.19:11
morganhmm.19:11
morgani am looking at the raw data19:11
morganthings look correct at dreamhost19:11
morganah ok so dreamohst account issue19:11
mordredmorgan: http://paste.openstack.org/show/598286/19:11
mordredso - there seem to be some with version and some with versions19:12
morganoh... i wonder..19:12
mordredBUT - some of the version docs are for v3 - and some for v2 - so it's not just like it's an old cloud thing19:13
morganmordred: v2 is "version" afaict19:14
morgan"v3" is "versions19:14
openstackgerritMonty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info  https://review.openstack.org/43172019:14
mordredmorgan: but look at line 14419:14
mordredand 17319:15
mordredand 286 :(19:15
morganoh ffs19:17
morgansomeone "fixed" plurality19:17
morganat somepointr19:17
morgan.... *screams*19:17
mordred:)19:18
morganor19:18
morganno19:18
morgannot...19:18
morganwtf.19:18
morgani know why19:19
morgan...19:19
morganbecause the "get_versions" doesn't wrap the extra versions layer when there is 1 version19:19
morganor they are only exposing the v3 you're hitting the v3 explicit endpoint19:20
morganvs the /19:20
morganwhich would then have the extra layer19:20
openstackgerritMonty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info  https://review.openstack.org/43172019:20
morganmordred: yep: https://controller00.vanilla.ic.openstack.org:5000 <<---- unversioned19:20
morganmordred: vs https://identity1.citycloud.com:5000/v3 <<---- versioned19:20
mordredmorgan: holy ...19:21
morganmordred: not keystone issue, clouds.yaml issue / config issue19:21
mordredwell - also some clouds do not allow unversioned endpoint19:21
dtroyermorgan: I've always thought those two should either be superst/subset or identical…19:21
* mordred looks at rax19:21
mordreddtroyer: identical is my vote19:21
mordrednova does identical and it works very nicely19:21
morgandtroyer: i'd be fine with identical...19:21
dtroyermee too, especially in a v2 -> v3 world19:21
morgango tell someone to make microversions for keystone happen so we cna fix it =/19:21
morganthough..19:21
morganugh19:21
morganhow does that work at the root?19:22
morganbad implementation is bad.19:22
morgankeystone is behaving to it's contract... it is a crappy api contract though in this case19:22
*** gouthamr has quit IRC19:23
morgannot sure if we can fix this.19:23
morganmordred: this is likely to break ksa in awful ways if we "fix" it... short of making a new version keystone can have requested and new ksa to rquest it19:24
dtroyermorgan: can the fix simply be a new (aditional) top-level wrapper key so new and old can co-exist19:25
dtroyer((it;s been a while since I looked at this closely))19:25
morgandtroyer: that would break old clients, since they wouldn't know how to handle it... i think19:25
dtroyersome maybe, I have a feeling that many only pick out the wrapper key they want19:25
morgandtroyer: versioned discovery has a specific code path... i am not sure where the tipping point between that would occur19:26
dtroyerI know I've seen code that does that19:26
morganright19:26
morganit's a question of how far back that discovery code cna handle either/or19:26
morgandtroyer: we also have been yelled at for adding top-level keys19:27
morgan:(19:27
morgandtroyer: in the past "OMG MY NON OPENSTACK CODE THING BROKE"19:27
* dtroyer may have done that himself once upon a time19:27
morgani would be 100% fine adding in a top-level identical key for keystones... but ... it doesn't solve the "well we need conditional code for this"19:28
morganin shade19:28
morganor anywhere else =/19:28
* morgan thinks any cloud not offering an unversioned endpoint (*cough*) is doing it wrong.19:28
dtroyerright, but the longer we punt on drawing another line in the sane the longer we wait until the new way can be the default19:28
dtroyers/sane/sand/19:28
morgani'd like to see defcore require that19:29
dtroyeralthough sanity is involved here19:29
morgantherfore we can start leaning on the unversioned endpoint discovery only for keystone and say "versioned is maintained but deprecated"19:29
dtroyerok, boarding call… laters gators…19:29
morgancheers19:29
morgansafe flight19:30
dtroyerthat's a good start for sure morgan19:30
morgandtroyer: that way we only maintain conditional code, but going forward unversioned discovery can do it all.19:30
mordredmorgan: ok - I've got a much richer version of the code19:42
*** gouthamr has joined #openstack-shade19:52
mordredmorgan: this new script is CRAZY and produces the following: http://paste.openstack.org/show/598293/19:58
openstackgerritMonty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info  https://review.openstack.org/43172020:02
mordredmorgan: there's the updated script20:02
mordredmorgan: turns out there were a few more formats - and also I added in trying unversioned if I got a single thing from a versioned endpoint20:02
morganmordred: ugh... well this is ugly... and is somehow giving me bad data back20:03
morgani think we have a bug in ksa20:03
morganmordred:         endpoint = self.cloud.keystone_session.auth.get_endpoint(self.cloud.keystone_session, service_type='identity')20:03
morganmordred: 'https://identity.example.comv2.0'20:03
morgani think there is a bug...20:04
morganbut i mean... maybe thats just me :P20:04
mordredhahaha20:04
mordrednice20:04
mordredmorgan: http://paste.openstack.org/show/598297/ <-- there is http://paste.openstack.org/show/598293/ but with the actual discovery payloads added too20:05
openstackgerritMonty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info  https://review.openstack.org/43172020:09
mordredmorgan: today I have learned a new way to hate everything20:09
Shrewsneat20:11
*** jlk has joined #openstack-shade20:11
morganyep. this looks like a bug in either the catalog or ksa20:12
morgan*great*20:12
morganlol mordred your catalog is busted.20:13
* morgan fixes20:13
mordredmorgan: \o/20:13
openstackgerritMonty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info  https://review.openstack.org/43172020:13
mordredmorgan, Shrews: ok. that ^^ is now good to go - supports -v flag for adding the pprint of the raw discovery json20:14
mordredenjoy20:14
mordredactually - I lied - one more rev coming20:16
openstackgerritMonty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info  https://review.openstack.org/43172020:17
-openstackstatus- NOTICE: Restarting gerrit due to performance problems20:21
mordredwow. that just took 1.5 hours of my life20:27
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: First keystone test using request_mock  https://review.openstack.org/43174320:40
morganmordred: ^20:40
morganfirst test actually using requests_mock instead of keystoneclient20:40
morganmock20:40
mordredwoot!20:41
morganalso fixes a chunk of the issues with discovery, catalog, and eliminates the random devstack addr20:41
morganthat was a LOT of work... and the way to get the catalog endpoint sucks :(20:41
morganbut whatever.20:41
morganthe next bulk of these should be a ton easier.20:41
* mordred reads and enjoys20:42
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: First keystone test using request_mock  https://review.openstack.org/43174320:44
morganminor typo fix20:44
morganmordred: it should also add an assert_calls in20:46
morganbut... that can be next larger round of fixes.20:46
*** jlk has quit IRC20:50
mordredyup20:50
mordredmorgan: cool - one inline suggestion that can also go into the next patch20:51
*** jlk has joined #openstack-shade20:51
mordredmorgan: and ... minor style point that I figure I'll mention now before you get too far in to things ... prevailing style in shade is to not use visual indentation but instead to wrap after { or ( ... it's not worth redoing anything in this patch, but it is a conscious style choice20:56
morganmordred: the response_json is going to be rolled up into a function shortly20:59
morganotherwise we're copy/pasta-ing a lot of stuff20:59
mordredmorgan: cool. did my validate= comment make sense?21:01
morganhaven't looked21:02
morganoh yeah21:02
morganthat makes sense21:02
morganthe validate will be more important when it's not keystoneclient21:03
*** cdent has quit IRC21:13
jamielennoxi didn't read all of that but glad requests_mock is useful, what i think we should provide in ksa is a fixture that mocks out the auth steps21:14
jamielennoxbecause it's a little complex with fetching versions and catalogs and such, that most people shouldn't care about21:14
jamielennoxalso ksc has a whole lot of options that are now dead, but if you treat it just like every other client it should work21:14
mordredjamielennox: yah - we've got a pile of fixture for that in shade - once morgan gets done hitting it with a sledgehammer and fixing my mistakes, it could be the beginning of a fixture perhaps21:16
mordred(it's a bit non-generalized at the moment)21:16
jamielennoxso there are fixtures for the actual responses21:16
jamielennoxbut yea, ideally you'd be mocking out the whole auth part than rely on you correctly stubbing out the auth responses21:17
jamielennoxmorgan: knows about those so i assume he's using htem21:17
mordredwell - right now he's just coming to terms with teh pile of crack I've made so far :)21:17
jamielennoxheh21:17
jamielennoxso i was going to remind you that mocker.request_uri('GET', ...) == mocker.get(...) but it looks like you've got some of your own stuff in there21:18
jamielennoxwhat does validate do/21:19
mordredvalidate lets you put in a payload that is what the code would send to the call - then we've got a single assert_calls helper that asserts that all of the calls were made in the right order and that if any validate payloads were specified, that they matched21:20
mordredjamielennox: https://github.com/openstack-infra/shade/blob/master/shade/tests/unit/test_flavors.py#L23 is a good simple example21:21
jamielennoxoh, ok, asserts that the body sent to the mock is what you'd expect21:22
jamielennoxnice21:22
mordredyah. hopefully it's keeping us honest when we swap a python-*client for the rest calls to make sure we're sending the same payload that the client lib was originally21:23
jamielennoxmakes sense, i mostly like to check how people are using requests_mock for what i should include as part of the library21:24
mordredjamielennox: a few days ago I used it to drive myself insane :)21:25
mordredbut please don't add that to the library21:26
jamielennoxheh, yea, i don't think i would add it as part of the register_uri method, IMO i like the validate against history21:26
mordred(trying to get the requests right in support of some of the neutron ports and floating ips magic that we do almost made me give up on computers)21:26
morganjamielennox: atm no21:27
jamielennoxoh, heh, yea - nothing can help you there21:27
morganjamielennox: i'm just using what mordred wrote21:27
morganjamielennox: i was not interested in converting it until i had a working test case to fall back on21:27
morganjamielennox: make sure i didn't eff it up :P21:28
mordredmorgan: fun how working tests are helpful isn't it?21:28
morganmordred: right?!21:28
jamielennoxit looks good, again mostly i was just interested in how requests_mock was being used, and it looks fine to me21:29
jamielennoxand a big agreement with dumping the clients21:29
mordredjamielennox: zomg. as much as it's work, the results are so super satisfying21:30
mordredjamielennox: this: https://github.com/openstack-infra/shade/commit/7878a9daf60d5b65115fb930ff6ffcda3f5eb07b is also worth checking out- ported it in from the zuul codebase - but it allows us to run with ksa http debugging turned on and not blow out the subunit stream since it only actually attaches the debug log if the test failed21:31
morganmordred: we should look at adding that elsewhere....21:32
mordredyah21:33
mordredit needs a little work on the generalization front - but it changed _everything_ for me when I was hacking on the hard neutron interaction21:33
jamielennoxinteresting, it could probably go to the oslo test thing but i can see it being useful21:33
morganaye21:34
morganon the topic of ksa, i am tired of arguing with folks about including "oslo" or other edge-case libraries with it21:34
jamielennoxksa dependency on something in oslo?21:34
jamielennoxyea not doing that21:34
*** Shrews has quit IRC21:35
jamielennoxi'm still thinking about the oslo.context <-> ksa relationship though, not sure how to make that right21:35
morganjamielennox: make oslo_context depend on ksa21:36
*** Shrews has joined #openstack-shade21:36
morganand implement a base class that oslo_context subclasses :P21:36
jamielennoxmorgan: yea, the other option is just dont do a dep at all and have them pass objects between each other21:36
morganthat way we can make ksa depending on anything oslo a circular dependency21:36
morganand we can short circuit the argument :P21:37
morgan>.>21:37
mordredheh21:37
morgan<.<21:37
morgannah, passing objects is more sane21:37
jamielennoxright, we need a way to make an auth plugin from a context object but it's going to be painful21:37
morgan(>^_^)>21:37
mordredwhat's a context object?21:37
morganthat sounds like an oslo_context problem21:37
morganhonestly21:37
morgannot a ksa problem21:37
*** gouthamr has quit IRC21:38
morganand i'd put the code there21:38
jamielennoxan oslo.context:Context object is created per request in most services and has auth information and other request info on it21:38
jamielennoxit's what they did instead of just subclassing webob.Request21:38
morganmordred: i'm going to be making mickey mouse go away in shade tests btw21:38
jamielennoxbut it has some useful things like parsing all the HTTP_X_ headers from auth_token middleware and putting them in logical places so that policy and such doesn't need to be re-invented each time21:39
morganmordred: going to do random generated values for everything, that way no one can "fudge" the result by it being a known quantity21:39
morganmordred: just a warning21:39
morgan;)21:39
jamielennox(it has these useful things because i added them, but not many services are using them unless i did it)21:39
morganjamielennox: hehe21:39
mordredmorgan: yes please21:41
mordredmorgan: you know about the unique string thing in testtools yeah?21:41
jamielennoxmorgan: yea  i think it's a context problem, but that doesn't make it easier21:41
morganmordred: nope, i just use uuid.uuid4()21:41
morganmordred: ;)21:41
mordredmorgan: self.getUniqueString('image')21:41
morganoh interesting21:42
mordredmorgan: is a part of the general testtools api21:42
mordredwe use it a bit in the functional tests21:42
morganwill still need uuid4.hex because.... well ids should be uuid4.hex (for keystone)21:42
mordredtotes21:42
morganjamielennox: honestly, i'm sad we .hex the uuid4's in keystone21:42
morganjamielennox: i wish we had just used the uuid w/ the '-' in it21:43
jamielennoxmorgan: why21:43
morganjamielennox: because easier to compare/read visually when needed21:43
jamielennoxhmm, ok21:43
mordredhttp://testtools.readthedocs.io/en/latest/api.html#testtools.TestCase.getUniqueString - I believe the reasoning for the uniqueString method is that it gives you a unique string that also includes the identifier so if you're looking at output you can visually validate that it's the string you thought it should be21:43
morganfor operator, user, and dev human interactions is nice.21:43
morganfor programatic, it doesn't matter21:43
jamielennoxi don't actually know, can you just use uuid.uuid4() because that's not a string21:44
mordredmorgan: but totally agree on the uiid.hex for things that are supposed to be uuid21:44
morganjamielennox: yeah you can just look at it, or print it21:44
morganjamielennox: the __str__ method is smart21:44
morganas is __repr__21:44
jamielennoxok - i really don't mind :)21:44
morganbut we can't fix it now21:44
morganAPI contracts and all that stuff21:44
jamielennoxoh - right, you mean the APIs and not the tests21:45
morganjamielennox: yes. like... what keystone emits.21:45
morgani wish was not .hex21:45
jamielennoxmaybe, -'s in urls can be weird21:45
jamielennoxi'd restrict it to A-Za-z0-9 and i can see whoever thought of .hex saw it an easy way to solve a problem21:46
morganjamielennox: nah '-' is fine21:50
morganjamielennox: *shrug* i dunno, and don't feel like asking termie :P21:52
*** gouthamr has joined #openstack-shade22:17
jamielennoxmordred: do you have a link handy to the explaining zuul animation?22:31
jamielennoxi can't find it22:31
jamielennoxnvm22:32
mordredyes!22:33
morganmordred: i think your assertCalls has a bug in it22:50
morganmordred: it seems we can't have duplicate calls to the same resource in it.22:51
mordredmorgan: you totally can22:51
mordredmorgan: you just have to register them more than once22:51
morganright, i try that and i keep getting failure(s) in odd ways22:51
mordredmorgan: paste what you've got somewhere?22:52
morganoh... ugh no it's that we make 3 round trip calls to identity.example.come22:52
morgancom*22:52
morganwow...22:53
morganwow........22:53
mordredyah22:53
morgansorry 422:53
morganget, post, get, get22:53
mordredthis is actually a REALLY fun exercise, because you realize just how chatty everything is22:53
morganbecause of the way endpoint collection goes22:53
morganugh22:53
mordredmorgan: on a plus note, when I swapped nova flavors to REST - I was actually able to remove a GET call that was not necessary that novaclient was doing behind the scenes :)22:53
mordredthat felt nice22:54
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: Cleanup new requests_mock stuff for test_users  https://review.openstack.org/43176622:54
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: Cleanup new requests_mock stuff for test_users  https://review.openstack.org/43176622:54
morgan^22:54
morganthat is.. wow22:54
morganactually that might need to be stop_after=5?22:54
morganoh nope, it passes22:54
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: Cleanup new requests_mock stuff for test_users  https://review.openstack.org/43176622:55
morganthere22:55
morganmordred: fwiw, v3 also needs the extra calls added22:56
morganbut that can come when i start using it22:56
mordredmorgan: looks great22:59
mordredmorgan: oh - also - to put onto your todo list ...22:59
mordredusers has a normalize method in shade/_utils - but that should get moved to shade/_normalize and brought in line with the other things (have a location property added at least) and docs added to doc/source/model.rst23:00
mordred(old style - normalization in _utils - new style normalization in _normalize)23:00
morganoh god. i think v3 adds another explicit round-trip *sigh*23:13
morganoh god. i think v3 adds another explicit round-trip *sigh*23:15
morganthis is so sloppy23:15
mordredmorgan: it's the gift that keeps on giving23:17
clarkbmorgan: was it you that was saying the devs don't use the APIs? :)23:18
morganclarkb: yep, and i have also been only tangentially a dev on keystone in the last 2 cycles23:19
mordredclarkb: it was all of us23:19
morganat best23:19
morganbut god23:21
morgan....23:21
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: Convert first V3 keystone test to requests_mock  https://review.openstack.org/43177923:44
morganmordred: it's slow, but making progress. i think i have all the tools now to do the rest of the conversion.23:44
mordredmorgan: doesn't it feel good to remove mock.patch calls?23:45
mordredmorgan: oh - also, don't know if you saw, but in the log-on-error patch thing, I added an env var thign - so you can say "OS_ALWAYS_LOG=true ttrun -epy27 shade.tests.unit.blah.blah" and it'll print all the http logging stuff even if the test succeeds ... just in case that's useful to you23:47
morgancool23:47
morganmordred: i dunno if this "feels good"23:47
morganbut.. it at least is more representative of what the client expects vs blind mocking23:47
mordredmorgan: :)23:47
mordredyah23:47
mordredmorgan: also - yah  that failure on your first patch in the functional test is one that happens sometimes ... not frequently enough to have caused anyone to rage enough to go debug it23:48
morganyou know the next level of insanity with the tests is... right?23:48
mordredbut pretty much always in the realm of boot from volume23:48
mordredno - what is it?23:49
morganforget requests_mock... betamax from a devstack :P23:49
* morgan will never be that insane for unit tests.23:49
mordredwell.....23:49
morgan#NOPE23:49
mordredso I started poking at that for the _functional_ tests a little while ago23:49
mordredand my thinking was to make a betamax of them so that betamax functional ran as part of unit23:50
morganit's a royal PITA.23:50
mordredbut then functional ran live during functional23:50
morganthat is what it is23:50
mordredyah23:50
mordredit is23:50
mordredand produces GIANT fixtures23:50
mordredI think it would be AWESOME to have - and that we're unlikely to ever have it23:50
morganbut it's "representative" of a "real" cloud23:50
morgan:P23:50
mordredyup23:50
mordredwell - next step after betamax devstack is to have betamax cassettes for every cloud I have creds on for all of the functional tests23:51
morgansolution, unit tests spin up a devstack, record the beta max, then run the unit tests with the programatically generated unit tests..... *shiftyeyes*23:51
mordredso that in unit tests we could run all of the functional tests against rax/vexxhost/ovh/citycloud/bluebox23:51
morganand if you don't have a local devstack, it downloads a root-kit, installs it, pivots from windows to linux, and pwns your whole laptop23:51
mordred:)23:51
morganyou gave it sudo, right?!23:52
mordredhahahaha23:52
morganfwiw, my head hurts.23:52
mordredmorgan: if you keep hitting it against the wall eventually the pain subsides23:55
morganmordred: on the plus side, a lot of the icky work will be done with this because ksc does so much stuff.23:55
mordredyup23:56
morganand a lot behind the scenes23:56
morganthen we do the "more" "fun" part.23:56
morgandrop keystoneclient to the floor and kick it a few times23:56
mordredyup23:56
mordredthat's actualy where it feels really satisfying23:56

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