*** jamielennox is now known as jamielennox|away | 00:24 | |
*** jamielennox|away is now known as jamielennox | 00:50 | |
*** gouthamr has quit IRC | 03:03 | |
*** openstackgerrit has quit IRC | 04:32 | |
*** yfried has joined #openstack-shade | 05:34 | |
*** yfried has quit IRC | 05:39 | |
*** yfried has joined #openstack-shade | 07:06 | |
*** yfried has quit IRC | 07:32 | |
*** ioggstream has joined #openstack-shade | 07:41 | |
mordred | morgan: 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-shade | 08:34 | |
*** abregman has quit IRC | 08:56 | |
*** cdent has joined #openstack-shade | 09:39 | |
morgan | mordred: it's how caching it setup | 09:40 |
morgan | mordred: invalidation only occurs with exact key invalidation... we can circle up on it | 09:40 |
morgan | mordred: also i have a massive rework of register_uri that is needed, chasing down all the cases now... | 09:41 |
morgan | mordred: already 3hrs into the edge cases. | 09:41 |
morgan | mordred: 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 |
morgan | the latter would be stale then | 09:42 |
*** jamielennox is now known as jamielennox|away | 09:48 | |
*** jamielennox|away is now known as jamielennox | 10:24 | |
*** cdent has quit IRC | 10:49 | |
*** ioggstream has quit IRC | 12:17 | |
*** gouthamr has joined #openstack-shade | 12:59 | |
*** ioggstream has joined #openstack-shade | 13:10 | |
*** iogg has joined #openstack-shade | 13:28 | |
*** ioggstream has quit IRC | 13:32 | |
*** iogg has quit IRC | 14:03 | |
*** jamielennox is now known as jamielennox|away | 14:39 | |
*** abregman has joined #openstack-shade | 14:41 | |
*** yfried has joined #openstack-shade | 14:58 | |
*** yfried has quit IRC | 15:05 | |
mordred | morgan: 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 filters | 15:05 |
mordred | morgan: 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 shade | 15:11 |
mordred | because 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 understand | 15:12 |
*** iogg has joined #openstack-shade | 15:42 | |
*** yfried has joined #openstack-shade | 15: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 IRC | 16:00 | |
*** yfried has quit IRC | 16:03 | |
*** abregman has joined #openstack-shade | 16:13 | |
*** yfried has joined #openstack-shade | 16:24 | |
*** yfried has quit IRC | 16:29 | |
*** iogg has quit IRC | 16:29 | |
*** abregman is now known as abregman|afk | 16:38 | |
*** iogg has joined #openstack-shade | 17:08 | |
*** abregman|afk has quit IRC | 17:13 | |
-openstackstatus- NOTICE: AFS replication issue has been addressed. Mirrors are currently re-syncing and coming back online. | 17:20 | |
morgan | mordred: it does have invalidate, doglike adds it. | 18:12 |
morgan | I was looking at the current implementation. | 18:12 |
morgan | dogpile | 18:12 |
morgan | * | 18:12 |
mordred | morgan: we do not use dogpile for servers | 18:15 |
mordred | morgan: servers, ports and floating_ips have a manually implemented caching/batching system | 18:16 |
mordred | morgan: the keystone objects use dogpile though - so it might just be that we're crossing streams on what we're talking about :) | 18:17 |
mordred | morgan: 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 magic | 18:20 |
*** gouthamr has quit IRC | 18:26 | |
*** gouthamr has joined #openstack-shade | 18:27 | |
morgan | mordred: ugh... i meant to say list_projects | 18:31 |
morgan | mordred: sorry..... | 18:31 |
morgan | mordred: stupid brain crossing the streams | 18: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 |
morgan | if someone passes domain_id, or... filter, etc the invalidate of .list_projects.invalidate(cloud) wont invalidate that cache | 18:32 |
morgan | basically... 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 targeted | 18:32 |
morgan | it's, historically (and amusingly) the reason keystone doesn't cache list ops | 18:33 |
morgan | too many ways to call it and we didn't have a distributed way to invalidate the cache until recently(ish) | 18:33 |
mordred | morgan: yes! I completely agree with you | 18:33 |
mordred | morgan: I fixated on the word "servers" :) | 18:34 |
morgan | lol yeah | 18:35 |
morgan | also like i said, doing a *massive* rework of register_uri | 18:35 |
morgan | you made some assumptions that... well don't work | 18:35 |
morgan | but it wont break until you need to re-register the same URI with different values | 18:35 |
morgan | being returned | 18:35 |
mordred | morgan: I'm pretty sure I have tests that re-register urls multiple time with the same url | 18:45 |
mordred | it should be collecting those into a list | 18:46 |
mordred | and submitting to requests_mock using the list form | 18:46 |
mordred | is it not? | 18:46 |
morgan | it's requests_mock | 18:46 |
morgan | that doesn't work right | 18:46 |
morgan | it adds a matcher | 18:46 |
morgan | you need to create a response_list | 18:46 |
morgan | and pass it to requestS_mock, then it will return the responses in order as you request | 18:46 |
morgan | test_object sortof works because you're changing enough, but if i'm doing something like same exact request with different responses, it doesn't | 18:47 |
morgan | because the original matcher catches it. | 18:47 |
morgan | tyring to convert test_cache shows this to be an issue | 18:48 |
*** cdent has joined #openstack-shade | 18:50 | |
mordred | awesome | 18:52 |
morgan | if i can figure out why test_object_segment_retries is hanging the tests, i can get this pushed | 18:52 |
morgan | https://www.irccloud.com/pastebin/enPXCHnu/ | 18:53 |
morgan | ^ and it hangs | 18:53 |
morgan | hmm | 18:55 |
morgan | 2017-02-13 10:50:34,772 keystoneauth.session RESP: [501] content-type: application/json | 18:55 |
mordred | morgan: it may be retrying until a timeout? | 18:57 |
morgan | maybe | 18:57 |
morgan | i'm going to go fix the other failing test first | 18:57 |
mordred | cool | 18:57 |
morgan | because test_create_image_task needs a lot of rework | 18:58 |
morgan | just how URIs are registered | 18:58 |
mordred | that makes me sad | 18:58 |
morgan | ugh. oh this is going to be rough because of how assert_calls works. | 18:59 |
mordred | yah | 18:59 |
morgan | for complex setups like this assert_calls might be too simplistic. | 18:59 |
morgan | hm. oh no this should be fine-ish | 19:00 |
mordred | I 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 times | 19:00 |
morgan | mordred: i think it's luck it works | 19:00 |
morgan | mordred: right now, the matcher order seems to be different than when i do the same thing with test_list_project | 19:00 |
morgan | in test_cache | 19:00 |
morgan | unless it's a dogpile issue... *looks* but it doesn't seem like it would be. | 19:01 |
mordred | can we tell requests_mock to clear its matchers? | 19:01 |
morgan | hmm. sec | 19:01 |
mordred | like, 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-register | 19:02 |
* mordred shuts up :) | 19:02 | |
morgan | mordred: yeah if that is the case, we should be abvle to do that | 19:02 |
morgan | i think i was just in a coffee shop with too much background noise to see the dogpile issue | 19:02 |
morgan | but... hmm. | 19:03 |
mordred | morgan: I know that problem well :) | 19:03 |
morgan | this is just weird. | 19:03 |
mordred | morgan: we DO have an issue in the test case if you have any cache settings in a clouds.yaml that I keep meaning to fix | 19:03 |
morgan | yeah that isn't the case here. | 19:03 |
morgan | it just is exibiting behavior that doesn't make sense when we stop mocking keystneclient | 19:04 |
mordred | \o/ | 19:05 |
morgan | and when i use a response_list, it passes | 19:05 |
morgan | *GAH* | 19:05 |
morgan | and now it works. | 19:10 |
morgan | w.t.f. | 19:10 |
morgan | mordred: whatever, just going to roll with it working now | 19:11 |
morgan | mordred: i can't break it in the same wya | 19:11 |
morgan | mordred: *shrug* | 19:11 |
mordred | morgan: those are my least favorite things ever | 19:12 |
*** openstackgerrit has joined #openstack-shade | 19:15 | |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: Remove mock of keystoneclient for test_caching https://review.openstack.org/433236 | 19:15 |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: Remove mock of keystoneclient for test_caching for projects https://review.openstack.org/433236 | 19:15 |
*** yfried has joined #openstack-shade | 19:16 | |
morgan | mordred: yep, hitting the same weird issue again | 19:39 |
morgan | mordred: i think requests_mock matchers are at fault here. | 19:39 |
*** iogg has quit IRC | 19:41 | |
morgan | mordred: right now there is just no good way to make matchers get cleared in requests_mock. | 19:42 |
morgan | mordred: 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 though | 19:42 |
morgan | mordred: unless you can tell me where the bug in the logic here is: | 19:43 |
morgan | https://www.irccloud.com/pastebin/Btix4cVS/ | 19:43 |
morgan | it failes at line #51 in that paste | 19:44 |
mordred | morgan: looking | 19:48 |
morgan | mordred: the list operation is still returning [] not the new updated list | 19:49 |
mordred | morgan: two questions: | 19:49 |
morgan | sure. | 19:50 |
mordred | morgan: 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 invalidation | 19:50 |
morgan | not sure. but it worked just fine when keystoneclient was mocked | 19:50 |
morgan | and i didn't add extra mocks | 19:51 |
mordred | sure - but maybe there was a bug that was hidden somehow | 19:51 |
mordred | that you've uncovered | 19:51 |
morgan | maybe | 19:51 |
morgan | but list_users doesn't take filter args and looking at create | 19:51 |
morgan | it seems sane | 19:51 |
morgan | invalidate is done last step before return | 19:52 |
morgan | if cloud.create_user is called, list_users should always be invalidated | 19:52 |
mordred | morgan: is that in the patch you pushed above? | 19:54 |
mordred | morgan: or are those local changes? | 19:54 |
morgan | local | 19:54 |
mordred | morgan: can you push up a wip patch? will be easier for me to help look that way | 19:54 |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: Remove keystoneclient mocks in test_caching for users https://review.openstack.org/433244 | 19:55 |
morgan | mordred: ^ | 19:55 |
morgan | mordred: i also need to make the self._mock_for_user bit | 19:56 |
morgan | but eh. | 19:56 |
mordred | well - shade.tests.unit.test_caching.TestMemoryCache.test_modify_user_invalidates_cache failed, so that's good | 19:56 |
morgan | hehe | 19:57 |
mordred | morgan: yes - the request ran and returned the empty list | 19:57 |
morgan | yep | 19:57 |
morgan | though it shouldn't | 19:57 |
morgan | as you can see i re-mock the URL right above the list request | 19:58 |
morgan | oh i wonder..... | 19:58 |
morgan | nope. was just making sure create didn't do a magic get | 19:58 |
morgan | or some such | 19:58 |
morgan | mordred: just added/tested with assert_calls, it actually says we made the correct mocks/requests until that point | 20:00 |
*** yfried has quit IRC | 20:01 | |
mordred | morgan: this is super weird | 20:05 |
mordred | morgan: because we literally do this same thing | 20:06 |
morgan | i need to harass jamielennox|away so we can make requests_mock object less opaque for debugging | 20:06 |
morgan | so, print debugging | 20:08 |
morgan | i'm seeing that the request response has only one response to it | 20:08 |
morgan | not the list like expected? | 20:08 |
morgan | actually strike that | 20: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 |
morgan | that is the response object | 20:09 |
morgan | that looks *100%* correct | 20:09 |
mordred | awesome | 20:09 |
mordred | so maybe the request_mock mocking _is_ working but something else is bong? | 20:09 |
morgan | self.adapter._adapter._matchers[-1]._responses[0]._params | 20:09 |
morgan | if that matcher isn't matching the request | 20:09 |
morgan | is the only thing I can think of | 20:09 |
mordred | are params somehow different? | 20:10 |
morgan | hm. | 20:10 |
morgan | well hold on | 20:10 |
morgan | mordred: well, afaict, this should be matching and working | 20:14 |
morgan | mordred: method is GET, URL is https://identity.example.com/v2.0/users | 20:14 |
mordred | morgan: I agree with you | 20:14 |
morgan | params for the response is ^ above. | 20:14 |
morgan | unless *somehow* dogpile is broken, we are in some edgecase of requests_mock | 20:14 |
mordred | morgan: for giggles, I just deleted the list_users cache decorator and also calls to list_users.invalidate and still same issue | 20:16 |
morgan | mordred: yeah i did that early on | 20:16 |
morgan | mordred: ftr, when i hit this with project, i re-wrote the request_uri to allow me to pass a response_list | 20:16 |
morgan | and it "worked" after | 20:17 |
mordred | yah - I mean, the part that bothers me is that we have examples of the same pattern working elsewhere | 20:17 |
morgan | https://www.irccloud.com/pastebin/9RbQkiPN/ | 20:17 |
mordred | so without understanding why it's broken here and not there, it's like - wtf? | 20:17 |
morgan | exactly i just *dont* get what is going on here | 20:17 |
morgan | ... huh | 20:19 |
morgan | oh FFS | 20:20 |
morgan | mordred: found it | 20: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 |
morgan | note the difference? | 20:21 |
morgan | when we re-register the URI, it resets the entire call stack | 20:21 |
morgan | because we do register_uri(method, uri, self._registered_uris[key]) | 20:21 |
morgan | so if we did self.cloud.list_users() and then self.cloud.list_users() again it would be correct | 20:22 |
morgan | watch | 20:22 |
mordred | morgan: oh - so ... | 20:22 |
morgan | we are implicitly creating a response_list | 20:22 |
morgan | and we re-set the entire response-list when we re-register a uri | 20:23 |
mordred | morgan: like in the other tests, we have all of the registration before we call any methods | 20:23 |
morgan | yep, need to move that up | 20:23 |
mordred | k. wow. | 20:23 |
morgan | in reality we should make that a lot more straight forward where we only accept a single .register_uri (actually .register_uris) per test | 20:23 |
morgan | and pass a list of things to register | 20:24 |
morgan | then raise a massive exception if we try and register uris again | 20:24 |
morgan | the reason the project_list one "fixed" is i moved the registration of the URIs above the action in question | 20:24 |
mordred | yah. | 20:24 |
*** gouthamr has quit IRC | 20:24 | |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: WIP: Remove keystoneclient mocks in test_caching for users https://review.openstack.org/433244 | 20:25 |
morgan | test with that version ^ | 20:25 |
morgan | it should fail | 20:25 |
mordred | well, 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 interleaved | 20:25 |
morgan | but it shows EXACTLY how it works | 20:25 |
mordred | cool | 20:25 |
morgan | mordred: we can, we pass each uri as a separate part | 20:25 |
*** gouthamr has joined #openstack-shade | 20:25 | |
morgan | aka [{method=get, uri=<blah>, response=<blah>}, {...}] | 20:25 |
mordred | morgan: oh - I get you - like a single ... yeah | 20:25 |
morgan | yes | 20:25 |
mordred | totally | 20:25 |
mordred | I think that will be much less easy to break | 20:26 |
mordred | and be really clear what it is | 20:26 |
morgan | and we can even structure it as response_lists being passed to requests_mock instead of many many many matchers | 20:26 |
morgan | basically, one mock per URI/Method | 20:26 |
morgan | instead of overriding matchers | 20:26 |
morgan | essentially we populate .registered_uris, then iterate it to register to the adapter since we know we'll only register once | 20:27 |
*** kong has joined #openstack-shade | 20:27 | |
* morgan fixes this test then will restructure .register_uri -> register_uris and fix all the other tests | 20:27 | |
morgan | since i'd like this bit to be done first. | 20:27 |
mordred | ++ | 20:27 |
morgan | also... *SO NOT INTUATIVE* | 20:28 |
mordred | morgan: I think the single list is going to wind up being _way_ more readable | 20:31 |
morgan | yes | 20:31 |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: WIP: Remove keystoneclient mocks in test_caching for users https://review.openstack.org/433244 | 20:58 |
morgan | mordred: ^ brain melting incoming | 20:58 |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: Remove keystoneclient mocks in test_caching for users https://review.openstack.org/433244 | 20:59 |
mordred | morgan: yay for brain melting! | 20:59 |
morgan | mordred: it doesn't help that dear-god keystoneclient v2 does terrible things | 21:00 |
morgan | like... get list to get id, then get resource by id, for every operation | 21:00 |
morgan | because... not like the data in list was all there... oh wait it was | 21:01 |
*** jamielennox|away is now known as jamielennox | 21:02 | |
*** cdent has quit IRC | 21:15 | |
*** cdent has joined #openstack-shade | 21:16 | |
*** cdent has quit IRC | 21:30 | |
*** gouthamr has quit IRC | 21:59 | |
dhellmann | mordred : 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 |
mordred | dhellmann: for the resources that support that (that I know about) yes | 22:14 |
mordred | dhellmann: I think | 22:14 |
mordred | dhellmann: nope. nevermind - I may be lying. | 22:14 |
dhellmann | aw, bummer | 22:14 |
mordred | (it honestly depends on a resource by resource basis) | 22:14 |
dhellmann | you had me all psyched | 22:14 |
mordred | well- tell me what you want to accomplish and I'll happy try to help! | 22:15 |
dhellmann | ok, so it's not that you've decided not to allow it | 22:15 |
dhellmann | export all of a tenant's things from one cloud and create them in another cloud with the same uuids | 22:15 |
dhellmann | in case they have config settings that use the uuids instead of names | 22:15 |
dhellmann | for their app or whatever | 22:15 |
mordred | ah - gotcha. yup, haven't disabled it explicitly - but haven't enabled it either | 22:16 |
dhellmann | ok, that's fair | 22:16 |
dhellmann | we'll see how far this goes and whether I end up sending patches to support that where it's missing | 22:16 |
dhellmann | the next step would be exposing all of that through ansible, which would be more work | 22:16 |
openstackgerrit | Morgan Fainberg proposed openstack-infra/shade master: Remove keystoneclient mocks in test_caching for users https://review.openstack.org/433244 | 22:21 |
morgan | mordred: ^ that should be a TON easier to read | 22:21 |
morgan | dhellmann: in the case of keystone, setting uuids of resources is explicitly not allowed | 22:22 |
morgan | dhellmann: not sure about the other services | 22:22 |
dhellmann | morgan : yeah, I think we're expecting keystone data to be migrated in another way | 22:22 |
morgan | dhellmann: basically keystone will override the values passed (such as user_id, and project_id) and do a uuid.uuid4().hex thing | 22:23 |
* morgan nods | 22:23 | |
dhellmann | we want the passwords anyway, and we know we can't get those out | 22:23 |
morgan | yep | 22:23 |
morgan | and with good reason, but yeah, makes needing to migrate keystone data a bit mroe not-via-shade for sure | 22:23 |
morgan | mordred: 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 internally | 22:24 |
morgan | mordred: so we can assert that a given test case only registers uri's once | 22:24 |
mordred | ++ | 22:26 |
morgan | mordred: and wow, is register_uris way way easier to read | 22:26 |
*** gouthamr has joined #openstack-shade | 22:37 | |
*** dhellmann has quit IRC | 23:05 | |
*** dhellmann has joined #openstack-shade | 23:06 | |
*** jamielennox is now known as jamielennox|away | 23:15 | |
*** jamielennox|away is now known as jamielennox | 23:20 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!