| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2020-07-27 | |||
| 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 | |
| 13:10:55 | mordred | dtantsur: fingers crossed | |
| 13:11:21 | dtantsur | yeah.. it works locally, but the bifrost CI will also test keystone authentication and no-auth | |
| 13:12:42 | openstackgerrit | Sagi Shnaidman proposed openstack/ansible-collections-openstack master: WIP add Ironic modules https://review.opendev.org/743533 | |
| 13:13:34 | openstackgerrit | Sagi Shnaidman proposed openstack/ansible-collections-openstack master: WIP add Ironic modules https://review.opendev.org/743533 | |
| 14:13:13 | nightmare_unreal | can someone look at this patch ( it's getting too big). I want to know if it's even in right direction : https://review.opendev.org/#/c/742210/ | |
| 14:22:47 | openstackgerrit | Merged openstack/keystoneauth master: Fix get_endpoint_data for non-keystone plugins https://review.opendev.org/743227 | |
| 14:38:36 | dtantsur | mordred: so far, one job passed, one made python segfault :) | |
| 14:38:40 | dtantsur | 1:1 I guess? | |
| 14:41:02 | dtantsur | joking aside, my keystoneauth patches seem working so far | |
| 14:41:12 | dtantsur | (I assume I could not cause the segfault with them) | |
| 14:43:31 | mordred | Yay! | |
| 14:44:57 | dtantsur | mordred: do we need to check anything else? I assume openstack itself takes a different code path (with identity)? | |
| 14:46:28 | dtantsur | okay, I'm adding a reno and a meaningful commit message | |
| 14:49:23 | mordred | dtantsur: I think as long as openstacksdk tests pass and bifrost that we should be pretty good | |
| 14:50:17 | openstackgerrit | Dmitry Tantsur proposed openstack/keystoneauth master: Correct endpoint and major version discovery for non-keystone plugins https://review.opendev.org/743282 | |
| 14:50:18 | dtantsur | mordred: okay, please review ^^ | |
| 14:52:43 | mordred | dtantsur: I think that looks great | |
| 14:55:36 | mordred | dtantsur: I pinged knikolla about it in the keystone channel (and apparently here now ;) ) | |
| 14:56:03 | mordred | but maybe lbragstad will save him | |
| 14:56:03 | dtantsur | two pings better than one, right? :) | |
| 14:58:03 | knikolla | why stop there? haha :) | |
| 15:05:52 | openstackgerrit | Dmitry Tantsur proposed openstack/ansible-collections-openstack master: Do not require ironic_url if cloud or auth.endpoint is provided https://review.opendev.org/742932 | |
| 15:05:55 | dtantsur | sshnaidm, TheJulia ^^^ | |
| 15:10:10 | openstackgerrit | Dmitry Tantsur proposed openstack/ansible-collections-openstack master: Do not require ironic_url if cloud or auth.endpoint is provided https://review.opendev.org/742932 | |
| 16:57:00 | openstackgerrit | Sagi Shnaidman proposed openstack/ansible-collections-openstack master: Add Ironic nodes info module https://review.opendev.org/743533 | |
| 17:05:56 | openstackgerrit | Sagi Shnaidman proposed openstack/ansible-collections-openstack master: Add Ironic nodes info module https://review.opendev.org/743533 | |