[openstacksdk][magnum] Adding get_coe_cluster_by_id() method to openstack.cloud._coe
Hi everyone, here's something I recently ran into while playing with openstacksdk against Magnum; I could use some help/feedback there if anyone's inclined. :) It looks as though openstacksdk, which presently doesn't include get_coe_cluster_by_id() in cloud._coe[1], will effectively only ever list clusters and then filter client-side, as opposed to looking up individual clusters directly. That means that it only ever hits the Magnum API's /v1/clusters URL[2], which provides only limited properties on each cluster it returns. Were it to implement get_coe_cluster_by_id(), then any Connection object that has use_direct_get set to True could hit /v1/clusters/<uuid> instead[3], which provides a much richer set of properties. I have submitted a patch for this[4], but I have a couple of open questions: (1) I'm not quite sure what's the proper way to test that get_coe_cluster() actually invokes get_coe_cluster_by_id() when passed a UUID. Normally I'd do that with unittest.mock's assert_called() method, but openstacksdk heavily wraps various mock.patch() calls so I'm sure there's a better, preferred way to do it for the openstacksdk codebase. (2) Even if implementing get_coe_cluster_by_id() is the correct approach (which I am entirely not sure of), it still strikes me as a bit unintuitive that there are additional details that could be retrieved only if use_direct_get is set on the Connection object. I think having an explicit "get me more detail" toggle on the call would be preferable, which would hit /v1/clusters first (to retrieve the cluster UUID from the name), and then hit /v1/clusters/<uuid> to retrieve the rich set of properties. Now, the docstring for search_coe_clusters() mentions a "detail" parameter[5], but that appears to be a bit of a red herring as that method doesn't actually use that param. So I'm a bit stuck and unsure what to do there. :) If anyone has thoughts on this they'd like to share, I'd be most grateful! Cheers, Florian References: [1] https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/cloud... [2] https://docs.openstack.org/api-ref/container-infrastructure-management/?expa... [3] https://docs.openstack.org/api-ref/container-infrastructure-management/?expa... [4] https://review.opendev.org/c/openstack/openstacksdk/+/828791 [5] https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/cloud...
Hello again, just taking the liberty to re-up on this one. :) I've rebased my patch on current master, but I'm still looking for input on whether it's actually on the right track. Artem, I hope you don't mind the CC. Cheers, Florian On 14/02/2022 13:41, Florian Haas wrote:
Hi everyone,
here's something I recently ran into while playing with openstacksdk against Magnum; I could use some help/feedback there if anyone's inclined. :)
It looks as though openstacksdk, which presently doesn't include get_coe_cluster_by_id() in cloud._coe[1], will effectively only ever list clusters and then filter client-side, as opposed to looking up individual clusters directly. That means that it only ever hits the Magnum API's /v1/clusters URL[2], which provides only limited properties on each cluster it returns.
Were it to implement get_coe_cluster_by_id(), then any Connection object that has use_direct_get set to True could hit /v1/clusters/<uuid> instead[3], which provides a much richer set of properties.
I have submitted a patch for this[4], but I have a couple of open questions:
(1) I'm not quite sure what's the proper way to test that get_coe_cluster() actually invokes get_coe_cluster_by_id() when passed a UUID. Normally I'd do that with unittest.mock's assert_called() method, but openstacksdk heavily wraps various mock.patch() calls so I'm sure there's a better, preferred way to do it for the openstacksdk codebase.
(2) Even if implementing get_coe_cluster_by_id() is the correct approach (which I am entirely not sure of), it still strikes me as a bit unintuitive that there are additional details that could be retrieved only if use_direct_get is set on the Connection object. I think having an explicit "get me more detail" toggle on the call would be preferable, which would hit /v1/clusters first (to retrieve the cluster UUID from the name), and then hit /v1/clusters/<uuid> to retrieve the rich set of properties. Now, the docstring for search_coe_clusters() mentions a "detail" parameter[5], but that appears to be a bit of a red herring as that method doesn't actually use that param. So I'm a bit stuck and unsure what to do there. :)
If anyone has thoughts on this they'd like to share, I'd be most grateful!
Cheers, Florian
References: [1] https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/cloud... [2] https://docs.openstack.org/api-ref/container-infrastructure-management/?expa... [3] https://docs.openstack.org/api-ref/container-infrastructure-management/?expa... [4] https://review.opendev.org/c/openstack/openstacksdk/+/828791 [5] https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/cloud...
participants (1)
-
Florian Haas