Monday, 2017-02-13

*** jamielennox is now known as jamielennox|away00:24
*** jamielennox|away is now known as jamielennox00:50
*** gouthamr has quit IRC03:03
*** openstackgerrit has quit IRC04:32
*** yfried has joined #openstack-shade05:34
*** yfried has quit IRC05:39
*** yfried has joined #openstack-shade07:06
*** yfried has quit IRC07:32
*** ioggstream has joined #openstack-shade07:41
mordredmorgan: let's circle back on that when it's not 2:30am - any filters passed should be happening after the list_servers cache in search_servers (We do not pass filters down to nova atm - except for all_projects, which admittedly could use some follow up work related to caching before we release again)08:31
*** abregman has joined #openstack-shade08:34
*** abregman has quit IRC08:56
*** cdent has joined #openstack-shade09:39
morganmordred: it's how caching it setup09:40
morganmordred: invalidation only occurs with exact key invalidation... we can circle up on it09:40
morganmordred: also i have a massive rework of register_uri that is needed, chasing down all the cases now...09:41
morganmordred: already 3hrs into the edge cases.09:41
morganmordred: but in short, caching has to be region wide if we have various arguments for a given thing, basically list_servers() and list_servers(filter=<blah>) are two different things, and cloud.list_servers.invalidate(self.cloud) only invalidates the cache for the former.09:42
morganthe latter would be stale then09:42
*** jamielennox is now known as jamielennox|away09:48
*** jamielennox|away is now known as jamielennox10:24
*** cdent has quit IRC10:49
*** ioggstream has quit IRC12:17
*** gouthamr has joined #openstack-shade12:59
*** ioggstream has joined #openstack-shade13:10
*** iogg has joined #openstack-shade13:28
*** ioggstream has quit IRC13:32
*** iogg has quit IRC14:03
*** jamielennox is now known as jamielennox|away14:39
*** abregman has joined #openstack-shade14:41
*** yfried has joined #openstack-shade14:58
*** yfried has quit IRC15:05
mordredmorgan: well - at some point hooking servers up to dogpile is a tdl (https://review.openstack.org/#/c/366143/) but for now list_servers has neither an invalidate method nor does it take filters15:05
mordredmorgan: I think we need two things before we can get that patch working again - one being fixing invalidation like you're talking about- the other being we need a generalized way to say "get_{resource}(wait_for_new)" that we can use internally to shade15:11
mordredbecause we don't have that at the moment, which means there are just places in the code which know to just sleep for SERVER_AGE before doing a get - but that quickly becomes hard to understand15:12
*** iogg has joined #openstack-shade15:42
*** yfried has joined #openstack-shade15:44
-openstackstatus- NOTICE: We are currently investigating an issue with our AFS mirrors which is causing some projects jobs to fail. We are working to correct the issue.15:50
*** abregman has quit IRC16:00
*** yfried has quit IRC16:03
*** abregman has joined #openstack-shade16:13
*** yfried has joined #openstack-shade16:24
*** yfried has quit IRC16:29
*** iogg has quit IRC16:29
*** abregman is now known as abregman|afk16:38
*** iogg has joined #openstack-shade17:08
*** abregman|afk has quit IRC17:13
-openstackstatus- NOTICE: AFS replication issue has been addressed. Mirrors are currently re-syncing and coming back online.17:20
morganmordred: it does have invalidate, doglike adds it.18:12
morganI was looking at the current implementation.18:12
morgandogpile18:12
morgan*18:12
mordredmorgan: we do not use dogpile for servers18:15
mordredmorgan: servers, ports and floating_ips have a manually implemented caching/batching system18:16
mordredmorgan: the keystone objects use dogpile though - so it might just be that we're crossing streams on what we're talking about :)18:17
mordredmorgan: in order to get list_servers to be able to use dogpile, I believe we'll need to get async_creation_runner working OR we may need to do some different magic18:20
*** gouthamr has quit IRC18:26
*** gouthamr has joined #openstack-shade18:27
morganmordred: ugh... i meant to say list_projects18:31
morganmordred: sorry.....18:31
morganmordred: stupid brain crossing the streams18:31
morgan*gorp*18:31
morgan@_utils.cache_on_arguments()18:31
morgan    def list_projects(self, domain_id=None, name_or_id=None, filters=None):18:31
morganif someone passes domain_id, or... filter, etc the invalidate of .list_projects.invalidate(cloud) wont invalidate that cache18:32
morganbasically... we need to do what keystone did, and we need to create a cache region explicitly for list and do a global invalidation of that region instead of a targeted18:32
morganit's, historically (and amusingly) the reason keystone doesn't cache list ops18:33
morgantoo many ways to call it and we didn't have a distributed way to invalidate the cache until recently(ish)18:33
mordredmorgan: yes! I completely agree with you18:33
mordredmorgan: I fixated on the word "servers" :)18:34
morganlol yeah18:35
morganalso like i said, doing a *massive* rework of register_uri18:35
morganyou made some assumptions that... well don't work18:35
morganbut it wont break until you need to re-register the same URI with different values18:35
morganbeing returned18:35
mordredmorgan: I'm pretty sure I have tests that re-register urls multiple time with the same url18:45
mordredit should be collecting those into a list18:46
mordredand submitting to requests_mock using the list form18:46
mordredis it not?18:46
morganit's requests_mock18:46
morganthat doesn't work right18:46
morganit adds a matcher18:46
morganyou need to create a response_list18:46
morganand pass it to requestS_mock, then it will return the responses in order as you request18:46
morgantest_object sortof works because you're changing enough, but if i'm doing something like same exact request with different responses, it doesn't18:47
morganbecause the original matcher catches it.18:47
morgantyring to convert test_cache shows this to be an issue18:48
*** cdent has joined #openstack-shade18:50
mordredawesome18:52
morganif i can figure out why test_object_segment_retries is hanging the tests, i can get this pushed18:52
morganhttps://www.irccloud.com/pastebin/enPXCHnu/18:53
morgan^ and it hangs18:53
morganhmm18:55
morgan2017-02-13 10:50:34,772 keystoneauth.session             RESP: [501] content-type: application/json18:55
mordredmorgan: it may be retrying until a timeout?18:57
morganmaybe18:57
morgani'm going to go fix the other failing test first18:57
mordredcool18:57
morganbecause test_create_image_task needs a lot of rework18:58
morganjust how URIs are registered18:58
mordredthat makes me sad18:58
morganugh. oh this is going to be rough because of how assert_calls works.18:59
mordredyah18:59
morganfor complex setups like this assert_calls might be too simplistic.18:59
morganhm. oh no this should be fine-ish19:00
mordredI mean - how can test_create_image_task be broken? it registers the same url twice but with different content, and the test wouldn't work if the content didn't get returned differently the two different times19:00
morganmordred: i think it's luck it works19:00
morganmordred: right now, the matcher order seems to be different than when i do the same thing with test_list_project19:00
morganin test_cache19:00
morganunless it's a dogpile issue... *looks* but it doesn't seem like it would be.19:01
mordredcan we tell requests_mock to clear its matchers?19:01
morganhmm. sec19:01
mordredlike, when we make a second register_uri call we have the context to konw that we have what should be registered as a list of calls - we could just blow away whatever is in r_m and re-register19:02
* mordred shuts up :)19:02
morganmordred: yeah if that is the case, we should be abvle to do that19:02
morgani think i was just in a coffee shop with too much background noise to see the dogpile issue19:02
morganbut... hmm.19:03
mordredmorgan: I know that problem well :)19:03
morganthis is just weird.19:03
mordredmorgan: we DO have an issue in the test case if you have any cache settings in a clouds.yaml that I keep meaning to fix19:03
morganyeah that isn't the case here.19:03
morganit just is exibiting behavior that doesn't make sense when we stop mocking keystneclient19:04
mordred\o/19:05
morganand when i use a response_list, it passes19:05
morgan*GAH*19:05
morganand now it works.19:10
morganw.t.f.19:10
morganmordred: whatever, just going to roll with it working now19:11
morganmordred: i can't break it in the same wya19:11
morganmordred: *shrug*19:11
mordredmorgan: those are my least favorite things ever19:12
*** openstackgerrit has joined #openstack-shade19:15
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: Remove mock of keystoneclient for test_caching  https://review.openstack.org/43323619:15
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: Remove mock of keystoneclient for test_caching for projects  https://review.openstack.org/43323619:15
*** yfried has joined #openstack-shade19:16
morganmordred: yep, hitting the same weird issue again19:39
morganmordred: i think requests_mock matchers are at fault here.19:39
*** iogg has quit IRC19:41
morganmordred: right now there is just no good way to make matchers get cleared in requests_mock.19:42
morganmordred: i think i can re-write assert_calls to be smart enough to handle the reponse_list, since it should be able to know that the responses are supposed to come in a specific order, i'll need to re-write self.calls though19:42
morganmordred: unless you can tell me where the bug in the logic here is:19:43
morganhttps://www.irccloud.com/pastebin/Btix4cVS/19:43
morganit failes at line #51 in that paste19:44
mordredmorgan: looking19:48
morganmordred: the list operation is still returning [] not the new updated list19:49
mordredmorgan: two questions:19:49
morgansure.19:50
mordredmorgan: is it making an additional call there? (like, do you see the call returning the wrong thing in the logs?) ... could be a bug in create doing invalidation19:50
morgannot sure. but it worked just fine when keystoneclient was mocked19:50
morganand i didn't add extra mocks19:51
mordredsure - but maybe there was a bug that was hidden somehow19:51
mordredthat you've uncovered19:51
morganmaybe19:51
morganbut list_users doesn't take filter args and looking at create19:51
morganit seems sane19:51
morganinvalidate is done last step before return19:52
morganif cloud.create_user is called, list_users should always be invalidated19:52
mordredmorgan: is that in the patch you pushed above?19:54
mordredmorgan: or are those local changes?19:54
morganlocal19:54
mordredmorgan: can you push up a wip patch? will be easier for me to help look that way19:54
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: Remove keystoneclient mocks in test_caching for users  https://review.openstack.org/43324419:55
morganmordred: ^19:55
morganmordred: i also need to make the self._mock_for_user bit19:56
morganbut eh.19:56
mordredwell - shade.tests.unit.test_caching.TestMemoryCache.test_modify_user_invalidates_cache failed, so that's good19:56
morganhehe19:57
mordredmorgan: yes - the request ran and returned the empty list19:57
morganyep19:57
morganthough it shouldn't19:57
morganas you can see i re-mock the URL right above the list request19:58
morganoh i wonder.....19:58
morgannope. was just making sure create didn't do a magic get19:58
morganor some such19:58
morganmordred: just added/tested with assert_calls, it actually says we made the correct mocks/requests until that point20:00
*** yfried has quit IRC20:01
mordredmorgan: this is super weird20:05
mordredmorgan: because we literally do this same thing20:06
morgani need to harass jamielennox|away so we can make requests_mock object less opaque for debugging20:06
morganso, print debugging20:08
morgani'm seeing that the request response has only one response to it20:08
morgannot the list like expected?20:08
morganactually strike that20:08
morgan{'status_code': 200, 'json': {'users': [{'description': None, 'name': 'username-2', 'enabled': True, 'email': 'test@example.com', 'id': '61ee3250c2b7483c9133626cce705eb6'}]}, 'headers': {'content-type': 'application/json'}}20:08
morganthat is the response object20:09
morganthat looks *100%* correct20:09
mordredawesome20:09
mordredso maybe the request_mock mocking _is_ working but something else is bong?20:09
morganself.adapter._adapter._matchers[-1]._responses[0]._params20:09
morganif that matcher isn't matching the request20:09
morganis the only thing I can think of20:09
mordredare params somehow different?20:10
morganhm.20:10
morganwell hold on20:10
morganmordred: well, afaict, this should be matching and working20:14
morganmordred: method is GET, URL is https://identity.example.com/v2.0/users20:14
mordredmorgan: I agree with you20:14
morganparams for the response is ^ above.20:14
morganunless *somehow* dogpile is broken, we are in some edgecase of requests_mock20:14
mordredmorgan: for giggles, I just deleted the list_users cache decorator and also calls to list_users.invalidate and still same issue20:16
morganmordred: yeah i did that early on20:16
morganmordred: ftr, when i hit this with project, i re-wrote the request_uri to allow me to pass a response_list20:16
morganand it "worked" after20:17
mordredyah - I mean, the part that bothers me is that we have examples of the same pattern working elsewhere20:17
morganhttps://www.irccloud.com/pastebin/9RbQkiPN/20:17
mordredso without understanding why it's broken here and not there, it's like - wtf?20:17
morganexactly i just *dont* get what is going on here20:17
morgan... huh20:19
morganoh FFS20:20
morganmordred: found it20:20
morgan[{'headers': {'content-type': 'application/json'}, 'status_code': 200, 'json': {'users': []}}, {'headers': {'content-type': 'application/json'}, 'status_code': 200, 'json': {'users': [{'name': 'username-2', 'description': None, 'id': '3858078e45514139a089ad7ae1a294f6', 'enabled': True, 'email': 'test@example.com'}]}}]20:20
morgannote the difference?20:21
morganwhen we re-register the URI, it resets the entire call stack20:21
morganbecause we do register_uri(method, uri, self._registered_uris[key])20:21
morganso if we did self.cloud.list_users() and then self.cloud.list_users() again it would be correct20:22
morganwatch20:22
mordredmorgan: oh - so ...20:22
morganwe are implicitly creating a response_list20:22
morganand we re-set the entire response-list when we re-register a uri20:23
mordredmorgan: like in the other tests, we have all of the registration before we call any methods20:23
morganyep, need to move that up20:23
mordredk. wow.20:23
morganin reality we should make that a lot more straight forward where we only accept a single .register_uri (actually .register_uris) per test20:23
morganand pass a list of things to register20:24
morganthen raise a massive exception if we try and register uris again20:24
morganthe reason the project_list one "fixed" is i moved the registration of the URIs above the action in question20:24
mordredyah.20:24
*** gouthamr has quit IRC20:24
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: WIP: Remove keystoneclient mocks in test_caching for users  https://review.openstack.org/43324420:25
morgantest with that version ^20:25
morganit should fail20:25
mordredwell, we can't pass in a list of thigns to register in one set - because we can't express expected call sequence that way when they're interleaved20:25
morganbut it shows EXACTLY how it works20:25
mordredcool20:25
morganmordred: we can, we pass each uri as a separate part20:25
*** gouthamr has joined #openstack-shade20:25
morganaka [{method=get, uri=<blah>, response=<blah>}, {...}]20:25
mordredmorgan: oh - I get you - like a single ... yeah20:25
morganyes20:25
mordredtotally20:25
mordredI think that will be much less easy to break20:26
mordredand be really clear what it is20:26
morganand we can even structure it as response_lists being passed to requests_mock instead of many many many matchers20:26
morganbasically, one mock per URI/Method20:26
morganinstead of overriding matchers20:26
morganessentially we populate .registered_uris, then iterate it to register to the adapter since we know we'll only register once20:27
*** kong has joined #openstack-shade20:27
* morgan fixes this test then will restructure .register_uri -> register_uris and fix all the other tests20:27
morgansince i'd like this bit to be done first.20:27
mordred++20:27
morganalso... *SO NOT INTUATIVE*20:28
mordredmorgan: I think the single list is going to wind up being _way_ more readable20:31
morganyes20:31
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: WIP: Remove keystoneclient mocks in test_caching for users  https://review.openstack.org/43324420:58
morganmordred: ^ brain melting incoming20:58
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: Remove keystoneclient mocks in test_caching for users  https://review.openstack.org/43324420:59
mordredmorgan: yay for brain melting!20:59
morganmordred: it doesn't help that dear-god keystoneclient v2 does terrible things21:00
morganlike... get list to get id, then get resource by id, for every operation21:00
morganbecause... not like the data in list was all there... oh wait it was21:01
*** jamielennox|away is now known as jamielennox21:02
*** cdent has quit IRC21:15
*** cdent has joined #openstack-shade21:16
*** cdent has quit IRC21:30
*** gouthamr has quit IRC21:59
dhellmannmordred : does shade support setting the UUID of new resources when creating them? (assuming I have the privileges needed for that on the server)22:07
mordreddhellmann: for the resources that support that (that I know about) yes22:14
mordreddhellmann: I think22:14
mordreddhellmann: nope. nevermind - I may be lying.22:14
dhellmannaw, bummer22:14
mordred(it honestly depends on a resource by resource basis)22:14
dhellmannyou had me all psyched22:14
mordredwell- tell me what you want to accomplish and I'll happy try to help!22:15
dhellmannok, so it's not that you've decided not to allow it22:15
dhellmannexport all of a tenant's things from one cloud and create them in another cloud with the same uuids22:15
dhellmannin case they have config settings that use the uuids instead of names22:15
dhellmannfor their app or whatever22:15
mordredah - gotcha. yup, haven't disabled it explicitly - but haven't enabled it either22:16
dhellmannok, that's fair22:16
dhellmannwe'll see how far this goes and whether I end up sending patches to support that where it's missing22:16
dhellmannthe next step would be exposing all of that through ansible, which would be more work22:16
openstackgerritMorgan Fainberg proposed openstack-infra/shade master: Remove keystoneclient mocks in test_caching for users  https://review.openstack.org/43324422:21
morganmordred: ^ that should be a TON easier to read22:21
morgandhellmann: in the case of keystone, setting uuids of resources is explicitly not allowed22:22
morgandhellmann: not sure about the other services22:22
dhellmannmorgan : yeah, I think we're expecting keystone data to be migrated in another way22:22
morgandhellmann: basically keystone will override the values passed (such as user_id, and project_id) and do a uuid.uuid4().hex thing22:23
* morgan nods22:23
dhellmannwe want the passwords anyway, and we know we can't get those out22:23
morganyep22:23
morganand with good reason, but yeah, makes needing to migrate keystone data a bit mroe not-via-shade for sure22:23
morganmordred: ok going to convert everything else over to using register_uris now and make register_uri only used for discovery/auth cases that we handle internally22:24
morganmordred: so we can assert that a given test case only registers uri's once22:24
mordred++22:26
morganmordred: and wow, is register_uris way way easier to read22:26
*** gouthamr has joined #openstack-shade22:37
*** dhellmann has quit IRC23:05
*** dhellmann has joined #openstack-shade23:06
*** jamielennox is now known as jamielennox|away23:15
*** jamielennox|away is now known as jamielennox23:20

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