| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2019-08-15 | |||
| 18:11:09 | mordred | Shrews: thanks! ^^ merged the block-storage patch into the catalog fixture patch | |
| 18:11:23 | mordred | (which is still a net-reduction in LOC) | |
| 19:14:54 | efried | mordred: Still that first ironic-sdk-in-nova patch succeeds but the second one fails. Do you have a minute to help me debug? | |
| 19:24:35 | openstackgerrit | Merged openstack/openstacksdk master: Avoid unnecessary object meta prefix in proxy https://review.opendev.org/676726 | |
| 19:24:36 | openstackgerrit | Merged openstack/openstacksdk master: Add header to auto-delete image upload objects https://review.opendev.org/676714 | |
| 19:36:59 | openstackgerrit | Merged openstack/openstacksdk master: Validate that connect_as connects as the project https://review.opendev.org/675178 | |
| 20:55:50 | mordred | efried: poop - was AFK - can you point me at the failng patch? | |
| 20:56:13 | efried | https://zuul.opendev.org/t/openstack/build/fcf71dbb7bd743249b69c83c8f2ba72a/log/controller/logs/screen-n-cpu.txt.gz?severity=3 | |
| 20:56:50 | efried | mordred: From what I can tell, we're (correctly) getting a conn error while the service is still coming up | |
| 20:56:50 | efried | BUT | |
| 20:56:50 | efried | because allow_version_hack=True we're actually returning an adapter rather than raising an exception. | |
| 20:56:50 | mordred | efried: cool thanks | |
| 20:56:59 | mordred | oh | |
| 20:57:02 | mordred | that's ... | |
| 20:57:42 | efried | I noticed there's a few places we explicitly set allow_version_hack=True, but afaict that's the default in ksa anyway. | |
| 20:57:46 | mordred | this is really a behavior design issue that I think we're going to need to fix - and is an actual valid usecase for wanting the other behavior | |
| 20:58:10 | mordred | sdk is assuming that an end-user is wanting to talk to a cloud, and that the cloud exists, so it does what it can to get them _something_ | |
| 20:58:28 | mordred | but for nova - you want the inverse - you *want* an error when there is an issue with the remote servie | |
| 20:58:44 | mordred | beause you're more assured that you know what you're doing so if you get an error it's because there is an error | |
| 20:58:49 | efried | I don't think this is one of the sdk paths where we explicitly set allow_version_hack=True. I think we're not setting that, but getting True because that's the default. | |
| 20:59:11 | efried | that being the case, a possible solution is to default allow_version_hack=False *except* in those cases where we set it explicitly. | |
| 20:59:14 | efried | Three of them, I believe. | |
| 20:59:34 | efried | efried@efried-ThinkPad-W520:~/openstack/openstacksdk$ git grep -n allow_version_hack | |
| 20:59:34 | efried | openstack/service_description.py:109: allow_version_hack=True, | |
| 20:59:34 | efried | openstack/service_description.py:205: allow_version_hack=True, | |
| 20:59:34 | efried | openstack/service_description.py:229: allow_version_hack=True, | |
| 20:59:51 | mordred | yeah - but regardless of that setting, sdk is going to, as it currently stands, try REALLY hard to not throw an exception for you | |
| 21:00:13 | efried | mm. Then yeah, I need a way to pass an option that says "throw an exception unless what you give me is really working" | |
| 21:00:17 | mordred | I kind of think we need a connection flag "raise_on_missing_service" or something named better than that | |
| 21:00:19 | mordred | yeah | |
| 21:01:19 | mordred | because contrary to an end-user who may be trying to use a weird cloud, or one that's configured weirdly and may not know all the ins and outs - it is *expected* that an admin will have told nova about anyhting weird in their cloud | |
| 21:01:35 | mordred | and it is much better to get a hard logged error so the admin can investigate the misconfig | |
| 21:01:45 | efried | for sure. | |
| 21:01:56 | mordred | efried: I have a thought ... let me see if I can make a patch | |
| 21:02:15 | efried | fwiw, the same sequence with ironicclient looks like this: https://zuul.opendev.org/t/openstack/build/402a73a9238643c2b893d53b37a6ce27/log/controller/logs/screen-n-cpu.txt.gz?severity=3 | |
| 21:02:50 | efried | wait, sorry, that's not right. | |
| 21:02:58 | mordred | efried: you get "virtdriver not ready" when the service pointed to by the catalog is not there, yes? | |
| 21:04:24 | efried | VirtDriverNotReady is raised by the Ironic driver itself when it encounters an error trying to get the client (in whatever form) | |
| 21:04:58 | efried | This is from the previous patch, which still uses sdk, but doesn't *rely* on it really early on, for node_list: | |
| 21:04:58 | efried | https://zuul.opendev.org/t/openstack/build/402a73a9238643c2b893d53b37a6ce27/log/controller/logs/screen-n-cpu.txt.gz?severity=3 | |
| 21:04:58 | efried | You can see it's getting the same error, but eventually it succeeds. | |
| 21:06:09 | efried | actually, I think we're seeing that error for baremetal while we're building conn for a different service | |
| 21:06:32 | efried | Here's a "normal" one, from before attempting to use sdk for ironic: https://zuul.opendev.org/t/openstack/build/cbe668cd674747168ac05759e9e147f3/log/controller/logs/screen-n-cpu.txt.gz?severity=3 | |
| 21:07:01 | efried | That StrictVersion exception, which I think comes from ironicclient when the service isn't ready, is the one we're translating to VirtDriverNotReady. | |
| 21:08:26 | mordred | yeah - makes sense - you're asking what version the service is and you're getting nothing because there is no service | |
| 21:09:42 | efried | Okay, so that gels - that's raising an exception and nova is set up to deal with that and retry; but the sdk path isn't raising, so we thing things are fine and actually (probably) bounce trying to access the real thing. | |
| 21:10:36 | mordred | yeah- and what's worse, we're never going to re-try version discovery beacuse teh proxy will already be constructed | |
| 21:11:14 | mordred | I think I've got a good handle on what we want the behavior to be | |
| 21:11:44 | mordred | now if I can just translate that into python words | |
| 21:31:05 | efried | mordred: FWIW, I think I'm going to want to use this option universally from nova. | |
| 21:31:14 | efried | I just did some local twiddling with a placement service | |
| 21:31:25 | efried | and (as expected, I think) I get a Proxy both times. | |
| 21:32:13 | efried | here's the diff in the logs (< is service down) http://paste.openstack.org/raw/757499/ | |
| 21:36:12 | efried | I tried setting allow_version_hack=False but it still returns the proxy | |
| 21:36:45 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: WIP Add a strict_proxies option for services https://review.opendev.org/676829 | |
| 21:36:57 | mordred | efried: ^^ how about something liek this | |
| 21:37:22 | efried | mordred: does it need to be on top of those other patches? | |
| 21:39:12 | mordred | efried: no - that's just my local working state - that's a very unclean patch | |
| 21:39:22 | efried | mordred: Okay. | |
| 21:39:40 | efried | So right off the bat: I just tried allow_version_hack=False locally and it still returns the proxy. | |
| 21:39:44 | mordred | efried: (wanted to get the first stab down so I didn't lose thinking context during dinner) | |
| 21:40:04 | efried | so I think that path should still have a check and raise, maybe on that get_endpoint() | |
| 21:40:05 | mordred | efried: yeah - I think there are some other code paths in there that will return you something | |
| 21:40:40 | efried | no, this was for placement, which doesn't have a shim, so it should be hitting that code path. | |
| 21:40:55 | efried | which is actually confirmed by the logging - I don't see that "Fallback" message, which is only emitted when allow_version_hack=True. | |
| 21:41:26 | mordred | ah - gotit. so maybe check that get_endpoint() actually returns something? | |
| 21:42:14 | efried | yeah, exactly | |
| 21:42:21 | mordred | what if ... | |
| 21:42:52 | mordred | if self._strict_proxies - we do temp_client.get_endpoint_data() instead of get_endpoint() - and check to make sure endpoint_data has a version? | |
| 21:43:10 | efried | do all services have a version? | |
| 21:45:21 | mordred | everything except swift I think | |
| 21:45:46 | mordred | but I'm not 100% sure if that's the right thing to test | |
| 21:46:22 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: WIP Add a strict_proxies option for services https://review.opendev.org/676829 | |
| 21:46:33 | mordred | efried: ^^ shotgun to see if we can get one of the things to trigger :) | |
| 21:46:56 | mordred | and then figure out which one is the RIGHT one to trigger on when the brain has more available brainjuice | |
| 21:46:56 | efried | mordred: What's weird is, I'm making sure that get_endpoint[_data] should fail, but the proxy is still returning. | |
| 21:47:10 | efried | is there a try/except somewhere up above? | |
| 21:47:16 | mordred | there shouldn't be | |
| 21:47:27 | mordred | and that is weird | |
| 21:47:41 | efried | I'm not getting an actual exception until I try to use the thing. | |
| 21:47:46 | mordred | you don't have that patch that added a placement service_description in do you? | |
| 21:48:34 | mordred | efried: (I'm probably going to want to construct a bunch of weird tests for this one so we can be sure we've got this covered) | |
| 21:49:20 | efried | mordred: Well, yes, I have the commit that added placement service_description, but that only happens in a test fixture, when you setUp the fixture, which I'm not doing locally. | |
| 21:50:46 | mordred | weird | |
| 21:51:46 | mordred | efried: I unfortunately have to adjourn to the dinner - if you find anything, let me know - otherwise I'll pick it up first thing in the morning and see if I can't construct some catalogs/request_mock tests we can use | |
| 21:52:11 | efried | mordred: Thank you sir. I'll noodle with it for a few more minutes before I, too, have to run (and choke some people) | |
| 21:59:51 | efried | mordred: a couple of typos notwithstanding, strict_proxies is a success. | |
| 22:00:56 | mordred | efried: yes? SWEET | |
| 22:01:09 | mordred | efried: then in the morning I'll work on tests and cleaning it up | |
| 22:01:28 | efried | mordred: Sweet, then we'll need 0.35.0 pretty quick. | |
| 22:01:48 | efried | I'll mark up the typos (at least the ones I saw), least I can do :P | |
| 22:03:04 | efried | mordred: It works for placement... I'm not sure it'll work for ironic. I need to dig more. | |
| 22:54:54 | adriant | mordred: how/when do we want to do the MFA work? | |
| 23:01:36 | openstackgerrit | Eric Fried proposed openstack/openstacksdk master: WIP: Connection(strict_proxies) https://review.opendev.org/676837 | |
| 23:01:56 | efried | mordred: there's my spin on it ^ | |
| 23:02:02 | adriant | I don't know your timetable, but wanna organise a meeting time and I can stay up late so we can discuss and plan? | |
| 23:02:05 | adriant | mordred: ^ | |
| 23:09:59 | openstackgerrit | Eric Fried proposed openstack/openstacksdk master: WIP: Connection(strict_proxies) https://review.opendev.org/676837 | |
| 23:16:36 | openstackgerrit | Eric Fried proposed openstack/openstacksdk master: WIP: Connection(strict_proxies) https://review.opendev.org/676837 | |
| #openstack-sdks - 2019-08-16 | |||
| 02:45:24 | openstackgerrit | Brin Zhang proposed openstack/python-openstackclient master: Microversion 2.77: Support Specifying AZ to unshelve https://review.opendev.org/665336 | |