| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2020-07-27 | |||
| 13:21:17 | dtantsur | +++ | |
| 13:22:01 | mordred | dtantsur: oh - when you get back ... | |
| 13:22:10 | mordred | no, nevermind | |
| 13:26:50 | openstackgerrit | Monty Taylor proposed openstack/keystoneauth master: Add unit test for httpbasicauth and discovery https://review.opendev.org/743227 | |
| 13:27:13 | mordred | dtantsur: remote: https://review.opendev.org/743227 Add unit test for httpbasicauth and discovery | |
| 13:27:13 | mordred | dtantsur: when you get back, can you point me to an example anywhere of this not working? unit test for the auth plugin calls get_api_major_version just fine on an http basic auth plugin | |
| 13:35:49 | openstackgerrit | jayaditya gupta proposed openstack/python-openstackclient master: openstack CLI migration list support https://review.opendev.org/742210 | |
| 13:36:39 | nightmare_unreal | hello can someone review this patch and also leave a response to my comment regarding it. https://review.opendev.org/#/c/742210/ | |
| 15:07:37 | dtantsur | mordred: if you have a spare VM, I can show you how to reproduce the bifrost failure | |
| 15:07:48 | dtantsur | I don't have a simple reproducer (which is another reason why I'm so frustrated) | |
| 15:12:25 | dtantsur | mordred: may I take over https://review.opendev.org/#/c/743227 and add a fix things there that might be a partial fix for my situation? | |
| 15:21:07 | openstackgerrit | Dmitry Tantsur proposed openstack/keystoneauth master: Fix get_endpoint_data for non-keystone plugins https://review.opendev.org/743227 | |
| 15:21:20 | dtantsur | mordred: ^^^ | |
| 15:26:49 | mordred | dtantsur: neat | |
| 15:27:21 | mordred | dtantsur: oh - that looks good | |
| 15:27:30 | mordred | dtantsur: still no clue why it works for one and not the other :) | |
| 15:28:03 | mordred | dtantsur: I had a thought - is it possible that ironic is http-auth protecting the discovery endpoit and ksa is just not sending auth when it talks to it? | |
| 15:28:33 | mordred | so ksa is trying to do discovery on the endpoint and failing? | |
| 15:28:49 | mordred | (which would explain why noauth works) | |
| 15:28:57 | dtantsur | mordred: checked that already | |
| 15:29:04 | mordred | nod | |
| 15:29:05 | dtantsur | curl works correctly | |
| 15:29:23 | dtantsur | (we had this problem with ironic-inspector, but we seem to be past it ) | |
| 16:04:37 | dtantsur | AttributeError: 'EntryPoint' object has no attribute 'module_name' | |
| 16:04:46 | dtantsur | hasn't it been fixed? I see it in keystoneauth on `tox -edocs` | |
| 16:06:43 | mordred | I thougth so? | |
| 16:07:00 | mordred | I thought it was a version something something | |
| 16:07:58 | mordred | dtantsur: I agree - I see that same issue | |
| 16:08:10 | dtantsur | yep :( | |
| 16:08:17 | mordred | dtantsur: maybe we fixed it in sdk but not ksa | |
| 16:12:14 | mordred | dtantsur: fix coming | |
| 16:12:45 | dtantsur | nice! if you also guess why this bloody discovery does not work, you'll be my saviour today :) | |
| 16:13:18 | openstackgerrit | Monty Taylor proposed openstack/keystoneauth master: Fix docs builds for modern stevedors https://review.opendev.org/743274 | |
| 16:13:23 | mordred | dtantsur: ^^ that | |
| 16:13:34 | mordred | dtantsur: I like being saviors :) | |
| 16:17:46 | dtantsur | but I think that get_endpoint_data patch is a part of the puzzle | |
| 16:21:25 | dtantsur | mordred: mmm, look. the adapter calls get_endpoint_data with service_type:baremetal, version:1 and so on, but it all goes to /dev/null | |
| 16:21:49 | dtantsur | because **kwargs are ignored in BaseAuthPlugin.get_endpoint_data | |
| 16:21:57 | dtantsur | is it intended? | |
| 16:22:17 | dtantsur | okay, I guess ignoring service_type is fine, but ignoring version seems to inhibit the discovery? | |
| 16:22:20 | mordred | not to my knowlege - probably just oversight. does passing the **kwargs on help? | |
| 16:22:23 | mordred | yeah | |
| 16:22:39 | mordred | (still confused why noauth works) | |
| 16:22:40 | dtantsur | which brings me back to the question why we inhibit discovery when a version is not requested.. | |
| 16:22:46 | dtantsur | yep. me too | |
| 16:26:45 | dtantsur | quite a few tests break if I remove the no_version handling from discover.py | |
| 16:26:55 | dtantsur | maybe I should go the other way and set max_version=min_version=kwargs.get('version')? | |
| 16:27:56 | dtantsur | wait, it's much easier! | |
| 16:28:01 | mordred | dtantsur: I think we inhibit discovery when a version isn't provided because we just assume that whatever is the default in the catalog is fine | |
| 16:28:02 | mordred | oh? | |
| 16:28:04 | mordred | cool! | |
| 16:28:35 | dtantsur | mordred: do you have a clue why inside get_api_major_version we call get_endpoint_data with discover_version=False? | |
| 16:28:41 | dtantsur | it seems like a version is what we need to discover | |
| 16:28:50 | dtantsur | if I change that to true, things start to work for me | |
| 16:29:20 | mordred | hrm. I think it's because we're trying to avoid a full discovery call if we don't need one | |
| 16:29:33 | mordred | but honestly - I kind of feel like perhaps that's effort we should stop caring about | |
| 16:29:59 | dtantsur | yeah, but then we short-circuit discovery if no versions are requested | |
| 16:30:09 | dtantsur | resulting in api_version being None | |
| 16:30:26 | mordred | does is break things to set it to true? :) | |
| 16:30:32 | dtantsur | lemme try! | |
| 16:30:56 | mordred | is there any chance that noauth vs httpauth we're using versioned vs unversioned endpoints as input? | |
| 16:31:13 | mordred | like - because we'll infer major api version from the /v1 in the endpoint if its there | |
| 16:31:24 | mordred | so maybe we've just accidentally worked with noauth based on input data? | |
| 16:31:40 | dtantsur | mordred: in case of bifrost the endpoints should be the same (unversioned) | |
| 16:31:49 | mordred | nod | |
| 16:31:55 | mordred | so yeah - no clue here | |
| 16:32:22 | dtantsur | if I change it, the no-auth/http_basic tests fail with: No mock address: GET http://keystone.example.com:5000/v3 | |
| 16:32:25 | dtantsur | which is pretty expected | |
| 16:32:47 | dtantsur | I guess what we need is "please do discover the major version but don't discovery microversions" | |
| 16:33:30 | mordred | or just add in mocks for mv discovery to those tests | |
| 16:35:22 | dtantsur | yeah, I'm still trying to avoid discovering microversions.. but it seems non-trivial with the code we have | |
| 16:37:26 | dtantsur | mordred: and this is why it works for keystone ;) https://opendev.org/openstack/keystoneauth/src/branch/master/keystoneauth1/identity/base.py#L501-L506 | |
| 16:38:14 | mordred | dtantsur: hahahahah | |
| 16:38:56 | dtantsur | okay, I'll update the same patch and hope I don't break anything else :D | |
| 16:39:44 | mordred | ++ | |
| 16:41:31 | dtantsur | and I think I see why the unit tests don't catch: we're using a versioned endpoint there | |
| 16:48:11 | dtantsur | and if I change it, everything breaks | |
| 16:58:15 | dtantsur | mordred: actually, discover_versions is also responsible for populating EndpointData.url | |
| 16:59:32 | openstackgerrit | Dmitry Tantsur proposed openstack/keystoneauth master: [WIP] Correct endpoint discovery for non-keystone plugins https://review.opendev.org/743282 | |
| 16:59:33 | dtantsur | this is a rabbit hole ^^^ | |
| 16:59:38 | dtantsur | I'll continue tomorrow | |
| 18:34:55 | openstackgerrit | Merged openstack/ansible-collections-openstack master: Add non-voting bifrost jobs https://review.opendev.org/743176 | |
| 21:23:04 | manasareddybethi | Looking for +2 reviews please: https://review.opendev.org/#/c/741320/ Delete the testcase that is not needed anymore | |
| 22:14:32 | openstackgerrit | melanie witt proposed openstack/python-openstackclient master: WIP Show words indicating booted from volume for server image https://review.opendev.org/743429 | |
| 22:20:16 | openstackgerrit | Merged openstack/keystoneauth master: Fix docs builds for modern stevedors https://review.opendev.org/743274 | |
| 23:11:30 | openstackgerrit | Merged openstack/python-openstackclient master: Delete the testcases that arent needed anymore https://review.opendev.org/741320 | |
| #openstack-sdks - 2020-07-28 | |||
| 02:59:24 | openstackgerrit | melanie witt proposed openstack/python-openstackclient master: WIP Show words indicating booted from volume for server image https://review.opendev.org/743429 | |
| 10:49:00 | openstackgerrit | jayaditya gupta proposed openstack/python-openstackclient master: openstack CLI migration list support https://review.opendev.org/742210 | |
| 12:04:37 | openstackgerrit | Dmitry Tantsur proposed openstack/openstacksdk master: baremetal-introspection: allow fetching unprocessed data https://review.opendev.org/743517 | |
| 12:38:42 | dtantsur | mordred: morning! so, wdyt https://review.opendev.org/#/c/743227/ as the first step towards the happiness? | |
| 12:39:28 | mordred | dtantsur: ++ | |
| 12:54:16 | openstackgerrit | jayaditya gupta proposed openstack/python-openstackclient master: openstack CLI migration list support https://review.opendev.org/742210 | |
| 12:55:14 | dtantsur | mordred: a question: what do we expect auth.get_endpoint() (and thus session.get_endpoint) to return: a versioned or unversioned endpoint? | |
| 13:00:42 | openstackgerrit | Dmitry Tantsur proposed openstack/keystoneauth master: [WIP] Correct endpoint discovery for non-keystone plugins https://review.opendev.org/743282 | |
| 13:03:29 | mordred | dtantsur: I think versioned? I think we expect get_endpoint() to return a usable endpoint that could then be fed to a session.get call | |
| 13:03:35 | mordred | dtantsur: I *think* | |
| 13:03:50 | dtantsur | mordred: my unit tests show otherwise :( see the WIP patch above | |
| 13:04:12 | mordred | dtantsur: ah - then I think maybe we expect otherwise | |
| 13:06:53 | dtantsur | :) | |
| 13:07:12 | dtantsur | https://review.opendev.org/#/c/743530/ will hopefully give these patches some testing | |