*** gouthamr has quit IRC | 02:16 | |
*** openstackstatus has quit IRC | 02:58 | |
*** openstackstatus has joined #openstack-shade | 03:02 | |
*** ChanServ sets mode: +v openstackstatus | 03:02 | |
*** TheJulia_ has joined #openstack-shade | 06:29 | |
*** TheJulia has quit IRC | 06:33 | |
*** TheJulia_ is now known as TheJulia | 06:33 | |
*** abregman has joined #openstack-shade | 06:35 | |
*** yfried has joined #openstack-shade | 07:21 | |
*** ioggstream has joined #openstack-shade | 07:21 | |
*** ioggstream has quit IRC | 07:37 | |
*** ioggstream has joined #openstack-shade | 08:27 | |
*** ioggstream has quit IRC | 08:46 | |
*** abregman has quit IRC | 09:01 | |
*** openstackgerrit has quit IRC | 09:03 | |
*** abregman has joined #openstack-shade | 09:34 | |
*** cdent has joined #openstack-shade | 09:54 | |
*** cdent has quit IRC | 10:37 | |
*** cdent has joined #openstack-shade | 11:09 | |
*** ioggstream has joined #openstack-shade | 13:08 | |
mordred | Shrews: 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-shade | 13:56 | |
Shrews | mordred: sure | 14:08 |
*** jlk has quit IRC | 14:22 | |
Shrews | mordred: 429328 is probably going to merge conflict once the COMPUTE_ENDPOINT change merges (just approved) | 14:35 |
Shrews | oh, nm. it's before it | 14:35 |
Shrews | duh | 14:35 |
mordred | Shrews: I do agree though - they would totally otherwise conflict :) | 14:36 |
mordred | Shrews: I had to stop halfway through on that one because doing the port test (where I added some dicts from citycloud) just about killed me | 14:36 |
Shrews | mordred: umm... how do those tests pass if we aren't yet using a raw neutron client? | 14:40 |
mordred | Shrews: requests_mock overrides the underlying requests.Session object | 14:40 |
mordred | Shrews: so they're mocking out the requests calls that neutronclient is making | 14:40 |
Shrews | mordred: ah right | 14:41 |
mordred | when we make the switch to raw_neutron_client, if we did things right, there should just about no changes to the tests | 14:41 |
mordred | it's magic :) | 14:41 |
Shrews | hard to keep the new stuff straight in my head | 14:41 |
* mordred hugs jamielennox and morgan for requests_mock | 14:41 | |
Shrews | too much context switching | 14:42 |
mordred | Shrews: ++ | 14:42 |
*** openstackgerrit has joined #openstack-shade | 15:06 | |
openstackgerrit | Merged openstack-infra/shade master: Pass task to post_task_run hook https://review.openstack.org/429925 | 15:06 |
mordred | Shrews: thank you! | 15:10 |
openstackgerrit | Merged openstack-infra/shade master: Transition half of test_floating_ip_neutron to requests_mock https://review.openstack.org/429328 | 15:17 |
openstackgerrit | Merged openstack-infra/shade master: Rename ENDPOINT to COMPUTE_ENDPOINT https://review.openstack.org/429911 | 15:21 |
*** abregman is now known as abregman|afk | 15:29 | |
mordred | thingee: you'll enjoy https://github.com/ansible/ansible/issues/21191 | 15:39 |
mordred | thingee: for some values of enjoy | 15:39 |
*** abregman|afk has quit IRC | 16:20 | |
*** yolanda_ is now known as yolanda | 16:41 | |
*** yfried has quit IRC | 16:46 | |
*** cdent has quit IRC | 17:03 | |
*** jlk has joined #openstack-shade | 17:29 | |
*** ioggstream has quit IRC | 17:31 | |
* morgan starts writing requests_mock for keystonein shade | 17:41 | |
*** cdent has joined #openstack-shade | 17:42 | |
mordred | yay! | 17:43 |
mordred | morgan: you probably see it in the suite, but I've got a v2 and a v3 catalog and fixtures/helpers for getting those set up | 17:44 |
morgan | oh nice | 17:44 |
morgan | i hadn't gotten that far | 17:44 |
morgan | i literally was just starting with "test_users" and converting it over to start | 17:44 |
* mordred looks forward to the death of keystoneclient | 17:44 | |
mordred | morgan: yah | 17:44 |
morgan | and since today is a nice rainy-day in seattle, and I am loaded up with coffee... | 17:45 |
morgan | wooooooo | 17:45 |
mordred | well - if you base from RequestsMockTestCase it'll get you set up with the catalog stuff without you needing to do much anyway | 17:45 |
morgan | cool | 17:45 |
morgan | good to know we have a simple starting place | 17:45 |
morgan | also yes, requests_mock is awesome | 17:45 |
mordred | +100 | 17:45 |
morgan | jamielennox did great working getting it started | 17:45 |
morgan | i just do some limited maintenance on it | 17:46 |
morgan | in case we have bugs | 17:46 |
morgan | > 1 core = good plan | 17:46 |
mordred | morgan: 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 instead | 17:47 |
morgan | nice. | 17:47 |
* morgan nods. | 17:47 | |
morgan | i was just looking at it. | 17:47 |
morgan | mordred: sooooo i hear the "retro" thinkpad is a "this year" thing | 17:47 |
mordred | morgan: OOOOOOOO | 17:48 |
morgan | mordred: fyi. | 17:48 |
morgan | mordred: rumor mills and whatnot | 17:48 |
* mordred likes that rumor | 17:48 | |
morgan | hehehe | 17:48 |
morgan | right?! | 17:48 |
mordred | here's hoping it has the quality | 17:48 |
mordred | I'm getting sick of the keys on this thing sticking | 17:48 |
morgan | honestly, I have zero issues with the X1C keys sticking | 17:49 |
morgan | it's why i've stuck with the X1Cs | 17:49 |
morgan | even with the other odd issues (but i seem ot just get lucky on that front) | 17:49 |
morgan | hmm. | 17:50 |
morgan | mordred: 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 decorators | 17:50 | |
morgan | always context managers | 17:50 |
morgan | and/or setup/cleanups | 17:50 |
morgan | mordred: but i don't want to remove the decorator if that is the "general" way you like to patch OCC. | 17:51 |
morgan | oh wait. derp | 17:51 |
morgan | that may not be needed | 17:51 |
morgan | since we have a real catalog now. | 17:51 |
mordred | morgan: the eventual goal is to not use mock.patch in any way | 17:56 |
mordred | morgan: so removing it is awesome | 17:57 |
morgan | cool | 17:57 |
morgan | we'll see how this works. | 17:57 |
mordred | that 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 |
mordred | and yah - having real catalog makes many of the patch things just unneeded (again, yay requests_mock!) | 17:58 |
morgan | mordred: my coffee did not stay warm enough | 18:05 |
morgan | mordred: it means i didn't drink it fast enough | 18:05 |
mordred | morgan: that is terrible! | 18:07 |
mordred | morgan: I got a carafe recently which I've been putting the coffee in to so that it stays warm between mugs | 18:07 |
jlk | morgan: so, K5 eh? | 18:07 |
morgan | mordred: i have an insulated french press, it's just the coffee in the cup is cold | 18:07 |
morgan | jlk: k5? | 18:08 |
mordred | jlk: yah man | 18:08 |
jlk | fujitsu cloud that is OpenStack, except for where it isn't. | 18:08 |
morgan | oooh | 18:08 |
mordred | jlk: I support lots of crazy things in shade ... I'm going to draw the line at a divergent nova api that changes semantics of things | 18:08 |
jlk | mordred: your response was remarkably civil | 18:08 |
mordred | jlk: well, it's not that dude's fault. I feel bad for him | 18:08 |
mordred | morgan: fujitsu k5 apparently makes _everything_ az aware and many resources per-az that are not per-az in normal openstack | 18:09 |
mordred | the example in question is keypairs, which are apparently a per-az resource in the k5 cloud | 18:09 |
mordred | that COMPLETELY changes the cloud interaction | 18:10 |
morgan | oh wow | 18:11 |
morgan | hmm. ooooo, need to implement a token mock now. | 18:12 |
morgan | or.. wait.. wtf. | 18:13 |
morgan | mordred: use_keystone_v2() is doing something weird still. digging, but i'm still getting requests sent to v3/auth/tokens | 18:13 |
morgan | ooh aha, your discovery.json is getting in the way. | 18:19 |
morgan | mordred: how did you select 192.168.0.19 as the keystone endpoint? | 18:21 |
morgan | looking at use_keystone_v3 and use_keystone_v2 | 18:21 |
morgan | mordred: and the values from discovery.json? | 18:21 |
mordred | morgan: I think it was just a payload taken from a devstack or something | 18:24 |
morgan | mordred: ah ok because trying to use requests_mock and keystoneclient is resulting in all sorts of "get tokens from example.com" | 18:24 |
mordred | excellent. well - fixes to the catalog/discovery are more than welcome :) | 18:25 |
mordred | morgan: (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 |
morgan | yeah i think i'm inclined to make it all run through example.com unless we want to change the discovery.json | 18:26 |
morgan | yeah. | 18:26 |
mordred | I think making it all sane sounds great! | 18:26 |
morgan | i mean long term we shouldn't need specific overrides except where we are explicitly overriding | 18:26 |
morgan | and testing that | 18:26 |
morgan | i'll poke at magical use of clouds.yaml for now | 18:27 |
morgan | though | 18:27 |
morgan | s/magical/useful | 18:27 |
morgan | huh | 18:27 |
morgan | weird. this actually looks like something that should work as setup | 18:27 |
* morgan digs more... | 18:27 | |
morgan | stupid rabbit hole | 18:27 |
mordred | yay rabbit holes! | 18:28 |
mordred | well - also, as you mentioned a little while ago ... | 18:29 |
mordred | keystoneclient is ... special | 18:29 |
morgan | yeah... *sigh* | 18:29 |
morgan | mordred: self.magic_fixes(config) you scare me sometimes | 18:29 |
mordred | morgan: hell yes I do | 18:29 |
mordred | morgan: this is your daily reminder that within these projects lay all of the dirty secrets and evil misdeeds of the past 7 years | 18:30 |
mordred | to gaze directly upon them is to gaze into the gaping maw of insanity | 18:31 |
morgan | oh | 18:31 |
morgan | so, looks like _make_test_cloud is always pulling _test_cloud | 18:31 |
morgan | unfortunately.... somehow keystoneclient is ignoring all this and actually doing discovery | 18:31 |
mordred | like - it's doing discovery again even though ksa already did discovery? | 18:32 |
morgan | well no, it's KSA doing discovery | 18:32 |
morgan | but it seems to be just ignoring the clouds.yaml auth_url | 18:32 |
morgan | so it is actually doing discovery and trying to post to example.com/v3/auth/tokens | 18:33 |
morgan | it also seems ot be ignoring identity_api_version | 18:33 |
mordred | it's entirely possible that we're not plumbing identity_api_version properly | 18:33 |
mordred | we _do_ expect a POST to example.com/v3/auth/tokens though | 18:33 |
morgan | that is actually what it is looking like | 18:33 |
morgan | well we do, except we don't in this case | 18:34 |
mordred | ah | 18:34 |
morgan | i think the correct answer is to pass a cloud to make_test_cloud and use _test_cloud_v2 | 18:34 |
morgan | vs _test_cloud | 18:34 |
morgan | somehow the passing of idenitty_api_version isn't being processed deep in the cloud creation | 18:34 |
morgan | use_keystone_v3() works because _test_cloud already sets that value | 18:35 |
morgan | i bet if i didn't specify identity_api_version it would break in the same way | 18:35 |
morgan | in clouds.yaml that is | 18:35 |
mordred | I'm confused ... | 18:36 |
morgan | so am i. | 18:36 |
mordred | :) | 18:36 |
morgan | i'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 v2 | 18:36 |
mordred | so we have a test that's using v2 ... | 18:37 |
morgan | specifying an auth url should prevent the need to do discovery | 18:37 |
morgan | for the token. | 18:37 |
mordred | not if it's an unversioned auth_url | 18:37 |
morgan | ah that might be the issue | 18:37 |
morgan | yep, lookie there | 18:38 |
morgan | identity_api_version=2 is not being set | 18:38 |
morgan | everything else should be "fine" | 18:38 |
mordred | it should be being set in use_keystone_v2() though | 18:38 |
morgan | ok something is wonky in passing identity_api_version to get_one_cloud | 18:38 |
mordred | AWESOME | 18:38 |
morgan | no, i think it is not being set in get_one_cloud, it is being passed into geT_one_cloud by _make_test_cloud | 18:38 |
mordred | yah. and things passed as kwargs _should_ override values that come from config | 18:39 |
morgan | so we're still trying to hit keystone v3 for tokens even when keystone v2 is used. | 18:39 |
mordred | blerg | 18:39 |
morgan | now... short term fix | 18:39 |
morgan | we could make use_keystone_v2() use _test_cloud_v2 | 18:39 |
mordred | wait - hang on - why is this working in shade/tests/unit/test_caching.py ? | 18:39 |
morgan | let me see if i can fix the kwarg prossessing though | 18:39 |
morgan | looking | 18:39 |
mordred | (test_modify_user_invalidates_cache) | 18:40 |
morgan | because you mock keystone_client | 18:40 |
mordred | so 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 |
morgan | no, you explicitly mock .keystone_client | 18:41 |
morgan | with @patch object | 18:41 |
morgan | discovery doesn't occur | 18:41 |
morgan | you hit a mock object on cloud.keystone_client | 18:41 |
mordred | ah. gotcha | 18:41 |
morgan | @mock.patch.object(shade.OpenStackCloud, 'keystone_client') | 18:41 |
morgan | so that property never calls ksa. | 18:42 |
morgan | and we never hit discovery | 18:42 |
morgan | .use_keystone_v2() in that test is pointless ;) | 18:42 |
mordred | \o/ | 18:42 |
morgan | i mean, it does register_uri, just doesn't ever get used. | 18:42 |
mordred | yah - and I don't have a self.assert_calls() there which I'm guessing would explode | 18:42 |
morgan | yup | 18:43 |
morgan | i'm not going to add that | 18:43 |
morgan | :P | 18:43 |
morgan | i'm going to see why shade is not working right | 18:43 |
mordred | so two thoughts: long term setting identity_api_version = 2 should make v2 get selected even if there is a v3 available | 18:43 |
mordred | EXCEPT | 18:43 |
mordred | this is weird because it's keystone | 18:43 |
morgan | i think setting identity_api_version is busted here | 18:44 |
mordred | and there is _also_ inference about v2 and v3 based on whether or not one has domain params or not | 18:44 |
mordred | yup. I agree, it almost certainly is :) | 18:44 |
morgan | and we should always adhere to explicit setting over inferred | 18:44 |
mordred | but the question is - which should win, identity_api_version or someone passing in domain params | 18:44 |
morgan | i don't care if someone passes domain params | 18:44 |
morgan | if they pass or lean on a v2 cloud config | 18:44 |
morgan | v2 | 18:44 |
mordred | since one could argue that identity_api_version is _really_ about keystone crud | 18:44 |
morgan | you can always pass identity_api_version as well as domain data | 18:45 |
morgan | now... the sticky bit | 18:45 |
morgan | we break behavior if we change this | 18:45 |
morgan | *sigh* | 18:45 |
morgan | sooooooooooooo | 18:45 |
morgan | the correct answer is inferred wins | 18:45 |
morgan | so shade doesn't break behavior | 18:45 |
mordred | yah. we choose the auth plugin based on whether or not you have domain params | 18:45 |
* morgan grumbles. | 18:45 | |
mordred | so if you have domain params, we make a passwordv3 object | 18:45 |
morgan | yep | 18:45 |
morgan | regardless if you say "do v2 only" | 18:46 |
mordred | yup | 18:46 |
morgan | maybe we need an "identity_auth_version" | 18:46 |
mordred | this is where your longstanding complaint about combining the auth and the crud really shines | 18:46 |
morgan | param | 18:46 |
mordred | ++ | 18:46 |
* morgan backlogs that | 18:46 | |
*** jlk has quit IRC | 18:46 | |
mordred | yah - let's say that identity_api_version is only for identity crud (since that's all it does now anyway) | 18:46 |
morgan | i'm going to make use_keystone_v2() *also* set token auth | 18:46 |
morgan | for v3 | 18:46 |
morgan | and i'm going to make it so you can specify a cloud in .make_test_cloud | 18:47 |
mordred | I'm confused by that last thing ... can you say that differently? | 18:47 |
morgan | so we could use _test_cloud_v2 | 18:47 |
mordred | (the token auth part) | 18:47 |
morgan | .use_keystone_v2 will also patch example.com/v3/auth/token | 18:47 |
morgan | since .use_keystone_v2 is strictly a CRUD directive | 18:47 |
morgan | for now | 18:47 |
morgan | s/patch/mock | 18:47 |
mordred | gotcha. works for me | 18:47 |
morgan | 3 fixes, 2 short term, 1 long | 18:48 |
mordred | yup | 18:48 |
morgan | short term (test only): patch v3 for .use_keystone_v2() && allow specifing a cloud in .make_test_cloud | 18:48 |
mordred | ++ | 18:48 |
morgan | long term, allow a identity_auth_version | 18:48 |
morgan | which overrides any inference on domain etc | 18:48 |
morgan | that is a much bigger change | 18:48 |
mordred | it is - and we'll have to be careful about openstackclient too | 18:49 |
morgan | and i kindof want to attack keystone's crud+auth bit (I have a spec) so we can make _plain_auth a reality there too | 18:49 |
morgan | unrelateD: my coffee is warm again | 18:49 |
morgan | yay insulated french press | 18:49 |
* morgan is onto ~.8L of coffee so far today | 18:50 | |
morgan | oh gross | 18:50 |
mordred | you'll be saying that a lot | 18:51 |
morgan | this may be more broken than thought | 18:51 |
morgan | testing something | 18:51 |
* dtroyer catching up | 18:52 | |
morgan | dtroyer: :) it's thankfully 100% internal to shade behavior | 18:52 |
dtroyer | mordred, morgan: my thought has always that user/caller setting xx_api_version wins | 18:52 |
dtroyer | default xx_api_version is last resort | 18:52 |
morgan | dtroyer: right. except we treat auth separate from CRUD | 18:53 |
dtroyer | ok, cool, it's nice if we're on the same page for expectations too :) | 18:53 |
morgan | dtroyer: yep. | 18:53 |
dtroyer | gotcha | 18:53 |
morgan | dtroyer: the hard part is ... we already behave this way | 18:53 |
* dtroyer retreats to "mordred mode" (waiting for airplanes) | 18:53 | |
dtroyer | ah | 18:53 |
morgan | i would rather have api_version dictate both | 18:53 |
morgan | but since shade already uses values from auth: to select plugin, regardless of api version... | 18:54 |
mordred | morgan: it's possible our behavior is broken enough that fixing it won't break people | 18:54 |
morgan | not something changable w/o potentially breaking people | 18:54 |
morgan | mordred: i doubt it is that broken | 18:54 |
dtroyer | so if we change lets all do it together :) | 18:54 |
morgan | i actually... sadly... expect people are using it JUST this way | 18:54 |
morgan | mordred: auth with V3, but do V2 actions | 18:54 |
morgan | notably,, because service accounts | 18:54 |
morgan | a v3 admin account can do V2 things (don't get me started on the security BS here) | 18:55 |
mordred | luckily - and changes to that can get caught by devstack | 18:55 |
morgan | (i lost that battle and api contracts blah blah blah) | 18:55 |
morgan | this mostly all goes away when V2 dies | 18:55 |
morgan | and we're realistically ~1.5yrs from that | 18:55 |
mordred | we're infinity away from that | 18:56 |
mordred | because v2 doesn't die from shade/occ | 18:56 |
* mordred ducks | 18:56 | |
morgan | no i mean where it becomes a tonne less important to worry about the oddness here | 18:56 |
mordred | indeed | 18:56 |
morgan | and we can just maintain behavior for most cases | 18:56 |
morgan | and anyone using v2 gets told "yeah, it's how v2 works" | 18:56 |
morgan | "use v3, please" | 18:57 |
mordred | morgan: WHY DO DIFFERENT CLOUDS HAVE DIFFERENT FORMATTED VERSION DISCOVERY FOR KEYSTONE????? | 19:02 |
morgan | mordred: because we suck. | 19:02 |
morgan | mordred: actually more to the point keystone team was not opinionated enough and didn't set a format and allowed anything into the catalog | 19:03 |
morgan | and discovery followed that pattern | 19:03 |
mordred | morgan: I'm doing a keystone version of the nova script I made a little while ago | 19:03 |
mordred | to run the version discovery on all of my clouds and print the versions found | 19:03 |
morgan | now... to be fair | 19:03 |
morgan | i actually think by the time we used discovery for anything it was a fixed set of values from keystone | 19:03 |
morgan | aka, json-home | 19:04 |
morgan | which means invalid formatting isn't keystone | 19:04 |
morgan | which means someone is not using keystone to serve that data | 19:04 |
morgan | which means... not a cloud? | 19:04 |
* morgan snarks a little at that last one. | 19:04 | |
morgan | mordred: do you have explicit examples? I'd like to see the differences | 19:04 |
morgan | make sure we didn't screw up here. | 19:04 |
morgan | but it should be pretty much standard. | 19:05 |
Shrews | mordred: ooh, that sounds like a handy little script. that should be included in that user debugging script we keep talking about and never implementing | 19:06 |
* morgan head-desks. | 19:06 | |
morgan | why does keystoneclient require explicit session ot be passed to .get_endpoint() !? *ugh* | 19:07 |
mordred | morgan, Shrews: http://paste.openstack.org/show/598284/ | 19:08 |
morgan | mordred: can you show me dreamhost vs one that works? | 19:09 |
morgan | just since you have the raw data. i mean... i could go look it up too | 19:09 |
morgan | most of those look correct | 19:09 |
mordred | http://paste.openstack.org/show/598285/ is the script | 19:10 |
mordred | morgan: dreamhost failed becuse my account is borked | 19:10 |
mordred | morgan: the script now accounts for all discovery docs | 19:10 |
mordred | morgan: I can also pprint the discovery if you like | 19:10 |
morgan | mordred: uh. | 19:11 |
morgan | hmm. | 19:11 |
morgan | i am looking at the raw data | 19:11 |
morgan | things look correct at dreamhost | 19:11 |
morgan | ah ok so dreamohst account issue | 19:11 |
mordred | morgan: http://paste.openstack.org/show/598286/ | 19:11 |
mordred | so - there seem to be some with version and some with versions | 19:12 |
morgan | oh... i wonder.. | 19:12 |
mordred | BUT - some of the version docs are for v3 - and some for v2 - so it's not just like it's an old cloud thing | 19:13 |
morgan | mordred: v2 is "version" afaict | 19:14 |
morgan | "v3" is "versions | 19:14 |
openstackgerrit | Monty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info https://review.openstack.org/431720 | 19:14 |
mordred | morgan: but look at line 144 | 19:14 |
mordred | and 173 | 19:15 |
mordred | and 286 :( | 19:15 |
morgan | oh ffs | 19:17 |
morgan | someone "fixed" plurality | 19:17 |
morgan | at somepointr | 19:17 |
morgan | .... *screams* | 19:17 |
mordred | :) | 19:18 |
morgan | or | 19:18 |
morgan | no | 19:18 |
morgan | not... | 19:18 |
morgan | wtf. | 19:18 |
morgan | i know why | 19:19 |
morgan | ... | 19:19 |
morgan | because the "get_versions" doesn't wrap the extra versions layer when there is 1 version | 19:19 |
morgan | or they are only exposing the v3 you're hitting the v3 explicit endpoint | 19:20 |
morgan | vs the / | 19:20 |
morgan | which would then have the extra layer | 19:20 |
openstackgerrit | Monty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info https://review.openstack.org/431720 | 19:20 |
morgan | mordred: yep: https://controller00.vanilla.ic.openstack.org:5000 <<---- unversioned | 19:20 |
morgan | mordred: vs https://identity1.citycloud.com:5000/v3 <<---- versioned | 19:20 |
mordred | morgan: holy ... | 19:21 |
morgan | mordred: not keystone issue, clouds.yaml issue / config issue | 19:21 |
mordred | well - also some clouds do not allow unversioned endpoint | 19:21 |
dtroyer | morgan: I've always thought those two should either be superst/subset or identical… | 19:21 |
* mordred looks at rax | 19:21 | |
mordred | dtroyer: identical is my vote | 19:21 |
mordred | nova does identical and it works very nicely | 19:21 |
morgan | dtroyer: i'd be fine with identical... | 19:21 |
dtroyer | mee too, especially in a v2 -> v3 world | 19:21 |
morgan | go tell someone to make microversions for keystone happen so we cna fix it =/ | 19:21 |
morgan | though.. | 19:21 |
morgan | ugh | 19:21 |
morgan | how does that work at the root? | 19:22 |
morgan | bad implementation is bad. | 19:22 |
morgan | keystone is behaving to it's contract... it is a crappy api contract though in this case | 19:22 |
*** gouthamr has quit IRC | 19:23 | |
morgan | not sure if we can fix this. | 19:23 |
morgan | mordred: 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 it | 19:24 |
dtroyer | morgan: can the fix simply be a new (aditional) top-level wrapper key so new and old can co-exist | 19:25 |
dtroyer | ((it;s been a while since I looked at this closely)) | 19:25 |
morgan | dtroyer: that would break old clients, since they wouldn't know how to handle it... i think | 19:25 |
dtroyer | some maybe, I have a feeling that many only pick out the wrapper key they want | 19:25 |
morgan | dtroyer: versioned discovery has a specific code path... i am not sure where the tipping point between that would occur | 19:26 |
dtroyer | I know I've seen code that does that | 19:26 |
morgan | right | 19:26 |
morgan | it's a question of how far back that discovery code cna handle either/or | 19:26 |
morgan | dtroyer: we also have been yelled at for adding top-level keys | 19:27 |
morgan | :( | 19:27 |
morgan | dtroyer: in the past "OMG MY NON OPENSTACK CODE THING BROKE" | 19:27 |
* dtroyer may have done that himself once upon a time | 19:27 | |
morgan | i 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 |
morgan | in shade | 19:28 |
morgan | or anywhere else =/ | 19:28 |
* morgan thinks any cloud not offering an unversioned endpoint (*cough*) is doing it wrong. | 19:28 | |
dtroyer | right, but the longer we punt on drawing another line in the sane the longer we wait until the new way can be the default | 19:28 |
dtroyer | s/sane/sand/ | 19:28 |
morgan | i'd like to see defcore require that | 19:29 |
dtroyer | although sanity is involved here | 19:29 |
morgan | therfore we can start leaning on the unversioned endpoint discovery only for keystone and say "versioned is maintained but deprecated" | 19:29 |
dtroyer | ok, boarding call… laters gators… | 19:29 |
morgan | cheers | 19:29 |
morgan | safe flight | 19:30 |
dtroyer | that's a good start for sure morgan | 19:30 |
morgan | dtroyer: that way we only maintain conditional code, but going forward unversioned discovery can do it all. | 19:30 |
mordred | morgan: ok - I've got a much richer version of the code | 19:42 |
*** gouthamr has joined #openstack-shade | 19:52 | |
mordred | morgan: this new script is CRAZY and produces the following: http://paste.openstack.org/show/598293/ | 19:58 |
openstackgerrit | Monty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info https://review.openstack.org/431720 | 20:02 |
mordred | morgan: there's the updated script | 20:02 |
mordred | morgan: turns out there were a few more formats - and also I added in trying unversioned if I got a single thing from a versioned endpoint | 20:02 |
morgan | mordred: ugh... well this is ugly... and is somehow giving me bad data back | 20:03 |
morgan | i think we have a bug in ksa | 20:03 |
morgan | mordred: endpoint = self.cloud.keystone_session.auth.get_endpoint(self.cloud.keystone_session, service_type='identity') | 20:03 |
morgan | mordred: 'https://identity.example.comv2.0' | 20:03 |
morgan | i think there is a bug... | 20:04 |
morgan | but i mean... maybe thats just me :P | 20:04 |
mordred | hahaha | 20:04 |
mordred | nice | 20:04 |
mordred | morgan: http://paste.openstack.org/show/598297/ <-- there is http://paste.openstack.org/show/598293/ but with the actual discovery payloads added too | 20:05 |
openstackgerrit | Monty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info https://review.openstack.org/431720 | 20:09 |
mordred | morgan: today I have learned a new way to hate everything | 20:09 |
Shrews | neat | 20:11 |
*** jlk has joined #openstack-shade | 20:11 | |
morgan | yep. this looks like a bug in either the catalog or ksa | 20:12 |
morgan | *great* | 20:12 |
morgan | lol mordred your catalog is busted. | 20:13 |
* morgan fixes | 20:13 | |
mordred | morgan: \o/ | 20:13 |
openstackgerrit | Monty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info https://review.openstack.org/431720 | 20:13 |
mordred | morgan, Shrews: ok. that ^^ is now good to go - supports -v flag for adding the pprint of the raw discovery json | 20:14 |
mordred | enjoy | 20:14 |
mordred | actually - I lied - one more rev coming | 20:16 |
openstackgerrit | Monty Taylor proposed openstack/os-client-config master: Add helper scripts to print version discovery info https://review.openstack.org/431720 | 20:17 |
-openstackstatus- NOTICE: Restarting gerrit due to performance problems | 20:21 | |
mordred | wow. that just took 1.5 hours of my life | 20:27 |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: First keystone test using request_mock https://review.openstack.org/431743 | 20:40 |
morgan | mordred: ^ | 20:40 |
morgan | first test actually using requests_mock instead of keystoneclient | 20:40 |
morgan | mock | 20:40 |
mordred | woot! | 20:41 |
morgan | also fixes a chunk of the issues with discovery, catalog, and eliminates the random devstack addr | 20:41 |
morgan | that was a LOT of work... and the way to get the catalog endpoint sucks :( | 20:41 |
morgan | but whatever. | 20:41 |
morgan | the next bulk of these should be a ton easier. | 20:41 |
* mordred reads and enjoys | 20:42 | |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: First keystone test using request_mock https://review.openstack.org/431743 | 20:44 |
morgan | minor typo fix | 20:44 |
morgan | mordred: it should also add an assert_calls in | 20:46 |
morgan | but... that can be next larger round of fixes. | 20:46 |
*** jlk has quit IRC | 20:50 | |
mordred | yup | 20:50 |
mordred | morgan: cool - one inline suggestion that can also go into the next patch | 20:51 |
*** jlk has joined #openstack-shade | 20:51 | |
mordred | morgan: 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 choice | 20:56 |
morgan | mordred: the response_json is going to be rolled up into a function shortly | 20:59 |
morgan | otherwise we're copy/pasta-ing a lot of stuff | 20:59 |
mordred | morgan: cool. did my validate= comment make sense? | 21:01 |
morgan | haven't looked | 21:02 |
morgan | oh yeah | 21:02 |
morgan | that makes sense | 21:02 |
morgan | the validate will be more important when it's not keystoneclient | 21:03 |
*** cdent has quit IRC | 21:13 | |
jamielennox | i 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 steps | 21:14 |
jamielennox | because it's a little complex with fetching versions and catalogs and such, that most people shouldn't care about | 21:14 |
jamielennox | also ksc has a whole lot of options that are now dead, but if you treat it just like every other client it should work | 21:14 |
mordred | jamielennox: 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 perhaps | 21:16 |
mordred | (it's a bit non-generalized at the moment) | 21:16 |
jamielennox | so there are fixtures for the actual responses | 21:16 |
jamielennox | but yea, ideally you'd be mocking out the whole auth part than rely on you correctly stubbing out the auth responses | 21:17 |
jamielennox | morgan: knows about those so i assume he's using htem | 21:17 |
mordred | well - right now he's just coming to terms with teh pile of crack I've made so far :) | 21:17 |
jamielennox | heh | 21:17 |
jamielennox | so 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 there | 21:18 |
jamielennox | what does validate do/ | 21:19 |
mordred | validate 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 matched | 21:20 |
mordred | jamielennox: https://github.com/openstack-infra/shade/blob/master/shade/tests/unit/test_flavors.py#L23 is a good simple example | 21:21 |
jamielennox | oh, ok, asserts that the body sent to the mock is what you'd expect | 21:22 |
jamielennox | nice | 21:22 |
mordred | yah. 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 originally | 21:23 |
jamielennox | makes sense, i mostly like to check how people are using requests_mock for what i should include as part of the library | 21:24 |
mordred | jamielennox: a few days ago I used it to drive myself insane :) | 21:25 |
mordred | but please don't add that to the library | 21:26 |
jamielennox | heh, yea, i don't think i would add it as part of the register_uri method, IMO i like the validate against history | 21: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 |
morgan | jamielennox: atm no | 21:27 |
jamielennox | oh, heh, yea - nothing can help you there | 21:27 |
morgan | jamielennox: i'm just using what mordred wrote | 21:27 |
morgan | jamielennox: i was not interested in converting it until i had a working test case to fall back on | 21:27 |
morgan | jamielennox: make sure i didn't eff it up :P | 21:28 |
mordred | morgan: fun how working tests are helpful isn't it? | 21:28 |
morgan | mordred: right?! | 21:28 |
jamielennox | it looks good, again mostly i was just interested in how requests_mock was being used, and it looks fine to me | 21:29 |
jamielennox | and a big agreement with dumping the clients | 21:29 |
mordred | jamielennox: zomg. as much as it's work, the results are so super satisfying | 21:30 |
mordred | jamielennox: 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 failed | 21:31 |
morgan | mordred: we should look at adding that elsewhere.... | 21:32 |
mordred | yah | 21:33 |
mordred | it needs a little work on the generalization front - but it changed _everything_ for me when I was hacking on the hard neutron interaction | 21:33 |
jamielennox | interesting, it could probably go to the oslo test thing but i can see it being useful | 21:33 |
morgan | aye | 21:34 |
morgan | on the topic of ksa, i am tired of arguing with folks about including "oslo" or other edge-case libraries with it | 21:34 |
jamielennox | ksa dependency on something in oslo? | 21:34 |
jamielennox | yea not doing that | 21:34 |
*** Shrews has quit IRC | 21:35 | |
jamielennox | i'm still thinking about the oslo.context <-> ksa relationship though, not sure how to make that right | 21:35 |
morgan | jamielennox: make oslo_context depend on ksa | 21:36 |
*** Shrews has joined #openstack-shade | 21:36 | |
morgan | and implement a base class that oslo_context subclasses :P | 21:36 |
jamielennox | morgan: yea, the other option is just dont do a dep at all and have them pass objects between each other | 21:36 |
morgan | that way we can make ksa depending on anything oslo a circular dependency | 21:36 |
morgan | and we can short circuit the argument :P | 21:37 |
morgan | >.> | 21:37 |
mordred | heh | 21:37 |
morgan | <.< | 21:37 |
morgan | nah, passing objects is more sane | 21:37 |
jamielennox | right, we need a way to make an auth plugin from a context object but it's going to be painful | 21:37 |
morgan | (>^_^)> | 21:37 |
mordred | what's a context object? | 21:37 |
morgan | that sounds like an oslo_context problem | 21:37 |
morgan | honestly | 21:37 |
morgan | not a ksa problem | 21:37 |
*** gouthamr has quit IRC | 21:38 | |
morgan | and i'd put the code there | 21:38 |
jamielennox | an oslo.context:Context object is created per request in most services and has auth information and other request info on it | 21:38 |
jamielennox | it's what they did instead of just subclassing webob.Request | 21:38 |
morgan | mordred: i'm going to be making mickey mouse go away in shade tests btw | 21:38 |
jamielennox | but 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 time | 21:39 |
morgan | mordred: going to do random generated values for everything, that way no one can "fudge" the result by it being a known quantity | 21:39 |
morgan | mordred: just a warning | 21: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 |
morgan | jamielennox: hehe | 21:39 |
mordred | morgan: yes please | 21:41 |
mordred | morgan: you know about the unique string thing in testtools yeah? | 21:41 |
jamielennox | morgan: yea i think it's a context problem, but that doesn't make it easier | 21:41 |
morgan | mordred: nope, i just use uuid.uuid4() | 21:41 |
morgan | mordred: ;) | 21:41 |
mordred | morgan: self.getUniqueString('image') | 21:41 |
morgan | oh interesting | 21:42 |
mordred | morgan: is a part of the general testtools api | 21:42 |
mordred | we use it a bit in the functional tests | 21:42 |
morgan | will still need uuid4.hex because.... well ids should be uuid4.hex (for keystone) | 21:42 |
mordred | totes | 21:42 |
morgan | jamielennox: honestly, i'm sad we .hex the uuid4's in keystone | 21:42 |
morgan | jamielennox: i wish we had just used the uuid w/ the '-' in it | 21:43 |
jamielennox | morgan: why | 21:43 |
morgan | jamielennox: because easier to compare/read visually when needed | 21:43 |
jamielennox | hmm, ok | 21:43 |
mordred | http://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 be | 21:43 |
morgan | for operator, user, and dev human interactions is nice. | 21:43 |
morgan | for programatic, it doesn't matter | 21:43 |
jamielennox | i don't actually know, can you just use uuid.uuid4() because that's not a string | 21:44 |
mordred | morgan: but totally agree on the uiid.hex for things that are supposed to be uuid | 21:44 |
morgan | jamielennox: yeah you can just look at it, or print it | 21:44 |
morgan | jamielennox: the __str__ method is smart | 21:44 |
morgan | as is __repr__ | 21:44 |
jamielennox | ok - i really don't mind :) | 21:44 |
morgan | but we can't fix it now | 21:44 |
morgan | API contracts and all that stuff | 21:44 |
jamielennox | oh - right, you mean the APIs and not the tests | 21:45 |
morgan | jamielennox: yes. like... what keystone emits. | 21:45 |
morgan | i wish was not .hex | 21:45 |
jamielennox | maybe, -'s in urls can be weird | 21:45 |
jamielennox | i'd restrict it to A-Za-z0-9 and i can see whoever thought of .hex saw it an easy way to solve a problem | 21:46 |
morgan | jamielennox: nah '-' is fine | 21:50 |
morgan | jamielennox: *shrug* i dunno, and don't feel like asking termie :P | 21:52 |
*** gouthamr has joined #openstack-shade | 22:17 | |
jamielennox | mordred: do you have a link handy to the explaining zuul animation? | 22:31 |
jamielennox | i can't find it | 22:31 |
jamielennox | nvm | 22:32 |
mordred | yes! | 22:33 |
morgan | mordred: i think your assertCalls has a bug in it | 22:50 |
morgan | mordred: it seems we can't have duplicate calls to the same resource in it. | 22:51 |
mordred | morgan: you totally can | 22:51 |
mordred | morgan: you just have to register them more than once | 22:51 |
morgan | right, i try that and i keep getting failure(s) in odd ways | 22:51 |
mordred | morgan: paste what you've got somewhere? | 22:52 |
morgan | oh... ugh no it's that we make 3 round trip calls to identity.example.come | 22:52 |
morgan | com* | 22:52 |
morgan | wow... | 22:53 |
morgan | wow........ | 22:53 |
mordred | yah | 22:53 |
morgan | sorry 4 | 22:53 |
morgan | get, post, get, get | 22:53 |
mordred | this is actually a REALLY fun exercise, because you realize just how chatty everything is | 22:53 |
morgan | because of the way endpoint collection goes | 22:53 |
morgan | ugh | 22:53 |
mordred | morgan: 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 |
mordred | that felt nice | 22:54 |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: Cleanup new requests_mock stuff for test_users https://review.openstack.org/431766 | 22:54 |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: Cleanup new requests_mock stuff for test_users https://review.openstack.org/431766 | 22:54 |
morgan | ^ | 22:54 |
morgan | that is.. wow | 22:54 |
morgan | actually that might need to be stop_after=5? | 22:54 |
morgan | oh nope, it passes | 22:54 |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: Cleanup new requests_mock stuff for test_users https://review.openstack.org/431766 | 22:55 |
morgan | there | 22:55 |
morgan | mordred: fwiw, v3 also needs the extra calls added | 22:56 |
morgan | but that can come when i start using it | 22:56 |
mordred | morgan: looks great | 22:59 |
mordred | morgan: oh - also - to put onto your todo list ... | 22:59 |
mordred | users 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.rst | 23:00 |
mordred | (old style - normalization in _utils - new style normalization in _normalize) | 23:00 |
morgan | oh god. i think v3 adds another explicit round-trip *sigh* | 23:13 |
morgan | oh god. i think v3 adds another explicit round-trip *sigh* | 23:15 |
morgan | this is so sloppy | 23:15 |
mordred | morgan: it's the gift that keeps on giving | 23:17 |
clarkb | morgan: was it you that was saying the devs don't use the APIs? :) | 23:18 |
morgan | clarkb: yep, and i have also been only tangentially a dev on keystone in the last 2 cycles | 23:19 |
mordred | clarkb: it was all of us | 23:19 |
morgan | at best | 23:19 |
morgan | but god | 23:21 |
morgan | .... | 23:21 |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: Convert first V3 keystone test to requests_mock https://review.openstack.org/431779 | 23:44 |
morgan | mordred: it's slow, but making progress. i think i have all the tools now to do the rest of the conversion. | 23:44 |
mordred | morgan: doesn't it feel good to remove mock.patch calls? | 23:45 |
mordred | morgan: 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 you | 23:47 |
morgan | cool | 23:47 |
morgan | mordred: i dunno if this "feels good" | 23:47 |
morgan | but.. it at least is more representative of what the client expects vs blind mocking | 23:47 |
mordred | morgan: :) | 23:47 |
mordred | yah | 23:47 |
mordred | morgan: 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 it | 23:48 |
morgan | you know the next level of insanity with the tests is... right? | 23:48 |
mordred | but pretty much always in the realm of boot from volume | 23:48 |
mordred | no - what is it? | 23:49 |
morgan | forget requests_mock... betamax from a devstack :P | 23:49 |
* morgan will never be that insane for unit tests. | 23:49 | |
mordred | well..... | 23:49 |
morgan | #NOPE | 23:49 |
mordred | so I started poking at that for the _functional_ tests a little while ago | 23:49 |
mordred | and my thinking was to make a betamax of them so that betamax functional ran as part of unit | 23:50 |
morgan | it's a royal PITA. | 23:50 |
mordred | but then functional ran live during functional | 23:50 |
morgan | that is what it is | 23:50 |
mordred | yah | 23:50 |
mordred | it is | 23:50 |
mordred | and produces GIANT fixtures | 23:50 |
mordred | I think it would be AWESOME to have - and that we're unlikely to ever have it | 23:50 |
morgan | but it's "representative" of a "real" cloud | 23:50 |
morgan | :P | 23:50 |
mordred | yup | 23:50 |
mordred | well - next step after betamax devstack is to have betamax cassettes for every cloud I have creds on for all of the functional tests | 23:51 |
morgan | solution, unit tests spin up a devstack, record the beta max, then run the unit tests with the programatically generated unit tests..... *shiftyeyes* | 23:51 |
mordred | so that in unit tests we could run all of the functional tests against rax/vexxhost/ovh/citycloud/bluebox | 23:51 |
morgan | and if you don't have a local devstack, it downloads a root-kit, installs it, pivots from windows to linux, and pwns your whole laptop | 23:51 |
mordred | :) | 23:51 |
morgan | you gave it sudo, right?! | 23:52 |
mordred | hahahaha | 23:52 |
morgan | fwiw, my head hurts. | 23:52 |
mordred | morgan: if you keep hitting it against the wall eventually the pain subsides | 23:55 |
morgan | mordred: on the plus side, a lot of the icky work will be done with this because ksc does so much stuff. | 23:55 |
mordred | yup | 23:56 |
morgan | and a lot behind the scenes | 23:56 |
morgan | then we do the "more" "fun" part. | 23:56 |
morgan | drop keystoneclient to the floor and kick it a few times | 23:56 |
mordred | yup | 23:56 |
mordred | that's actualy where it feels really satisfying | 23:56 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!