| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-06 | |||
| 16:10:07 | sean-k-mooney | i think we can all agree that if it alllow us to finally resovle thos bugs in nova either approch is better then what we have today. so if that the direction we head in im fine with say lets define that in the placement api spec | |
| 16:13:11 | gibi | I agree | |
| 16:14:01 | dansmith | yup | |
| 17:06:15 | melwitt | sean-k-mooney: not sure what you'll think of this but I uploaded a patch in regard to the oslo.limit endpoint discovery https://review.opendev.org/c/openstack/nova/+/943507 fyi | |
| 17:17:52 | sean-k-mooney | so we remove the end point check since oslo limists will do that for use internally | |
| 17:18:44 | sean-k-mooney | and we repalce the end point fuction with one that returns the limits for this service/region | |
| 17:19:26 | sean-k-mooney | these are the global limits right not the project specific ones | |
| 17:21:25 | sean-k-mooney | melwitt: without properly loadign context and looking at oslo limits its seams reasonable at first glance | |
| 17:23:50 | melwitt | sean-k-mooney: yeah the thing I think is controversial is the private attributes access | |
| 17:24:08 | sean-k-mooney | that not required with the latest version of oslo.limits right | |
| 17:24:09 | melwitt | maybe in parallel propose some public accesses in oslo.limit and update it when available? dunno | |
| 17:25:19 | sean-k-mooney | https://review.opendev.org/c/openstack/oslo.limit/+/914783/3/oslo_limit/limit.py | |
| 17:26:46 | melwitt | what is there currently will work for both old and new versions | |
| 17:27:09 | melwitt | and underneath oslo.limit will do what it needs to | |
| 17:27:10 | sean-k-mooney | right in the new version we can just call get_registered_limits | |
| 17:27:13 | sean-k-mooney | on the enforcer | |
| 17:27:48 | sean-k-mooney | https://github.com/openstack/oslo.limit/blob/8ada297e6a82f7e50bf746453b8dd21716743798/oslo_limit/limit.py#L183 | |
| 17:29:43 | sean-k-mooney | melwitt: do you need to use the connection today | |
| 17:30:50 | sean-k-mooney | oh that funciton takes a set of resouces to check | |
| 17:33:25 | melwitt | sean-k-mooney: yeah I do that because I want to pull all the resourcs limits in one API call. oslo.limiit does them one API call per resource | |
| 17:34:07 | sean-k-mooney | it sound like we shoudl add a funciton to oslo limits to do that | |
| 17:34:17 | sean-k-mooney | but i think we can work around it in nova for now | |
| 17:34:33 | melwitt | maybe. there's already one so not sure if we change the internal impl or make another alongside | |
| 17:35:45 | sean-k-mooney | you could make None a seential value for all or use the sentenial object pattern | |
| 17:36:18 | melwitt | yeah | |
| 17:38:14 | sean-k-mooney | so gibi's main concern seams to be those are private filed and there for could break which is valid. if oslo were ok providing public access in some way i would be ok with this as a backportable approch as long as we update to using the public appropch in the future | |
| 17:39:31 | melwitt | sean-k-mooney: right. it's mine too, which is why I mentioned it in the commit message. yeah I think that sounds reasonable | |
| 17:39:33 | sean-k-mooney | i dont hate it but if we could just do ENFORCE.get_registered_limists(None) | |
| 17:39:39 | sean-k-mooney | i would be happier | |
| 17:39:56 | melwitt | sure | |
| 17:39:57 | sean-k-mooney | the other option for now | |
| 17:40:12 | melwitt | will that work currently? /me looks | |
| 17:40:15 | sean-k-mooney | is kep doing it the old way and instead fo raising try and do it this way | |
| 17:40:28 | sean-k-mooney | melwitt: i was trying ot figure that out | |
| 17:40:36 | sean-k-mooney | you can pass it i just dont know what happens if you do | |
| 17:41:09 | sean-k-mooney | its this right https://docs.openstack.org/api-ref/identity/v3/#list-registered-limits | |
| 17:41:13 | melwitt | it looks like it would do nothing | |
| 17:41:20 | melwitt | https://github.com/openstack/oslo.limit/blob/master/oslo_limit/limit.py#L355 | |
| 17:41:30 | sean-k-mooney | it say resouce name is optional in the api | |
| 17:41:53 | sean-k-mooney | you could try [""] | |
| 17:42:16 | melwitt | yes that's why I'm using the connection directly | |
| 17:42:24 | sean-k-mooney | i.e. ENFORCE.get_registered_limists([""]) | |
| 17:42:44 | sean-k-mooney | ya well modle.connection is public it seams | |
| 17:42:58 | sean-k-mooney | its just the endpoint id and service id that not right | |
| 17:42:59 | melwitt | yes and I'm using it in that patch :) | |
| 17:43:38 | melwitt | Enforcer.get_registered_limits() does "for resource_name in resource_names:" and will do nothing if you don't pass it a list of names and if you pass it a list of names it will pull each limit in a separate API call | |
| 17:44:03 | melwitt | sean-k-mooney: correct it's the endpoint id and service id that are not public | |
| 17:45:31 | melwitt | I could alternatively pull those separately the same way that oslo.limit does today | |
| 17:45:39 | sean-k-mooney | ya im not seeing a better way to do it with the current api | |
| 17:45:41 | melwitt | I considered that might be more well liked | |
| 17:46:43 | melwitt | so maybe I should push a new PS that pulls the services and then gets the endpoint ID from it, that way no private attrs need to be used | |
| 17:47:31 | sean-k-mooney | thats an incremental imporment. i fell lilke they didnt fully fix the usablity problem | |
| 17:48:18 | sean-k-mooney | as in they didnt really fix the usablity problem reported in https://launchpad.net/bugs/1931875 | |
| 17:48:35 | sean-k-mooney | if we have to port that code to nova | |
| 17:48:59 | melwitt | well that works for 99% of users who have no need to query for the list of limits | |
| 17:49:22 | sean-k-mooney | ya but athat a pretty fundemental thing | |
| 17:49:26 | melwitt | strictly for limit enforcement what they did fixes it all | |
| 17:50:18 | sean-k-mooney | i guess oslo.limits is not inteded as a generic keyston clinet | |
| 17:51:09 | sean-k-mooney | you could argue that we shoudl not be using ot get the registered limits and shoudl use the sdk instead | |
| 17:51:18 | melwitt | yeah.. it's not. that was what I felt conflicted about. like making it a passthrough to the underlying keystone client? when it already has a public connection attribute? I dunno. just seemed kind of weird | |
| 17:52:29 | melwitt | yeah. I think maybe they did that because the keystone API does not have the ability to filter a limit list by a set of resource names. but you could just filter it yourself after getting it. you'll get all of them in one API call | |
| 17:53:41 | sean-k-mooney | ubuntu@sean-devstack-watcher-1:~/repos/nova$ curl -s -H "X-Auth-Token: $TOKEN" -H "OpenStack-API-Version: compute 2.99" http://192.168.50.140/identity/v3/registered_limits | jq | nc termbin.com 9999 | |
| 17:53:43 | sean-k-mooney | https://termbin.com/kqfj | |
| 17:54:07 | melwitt | so maybe the right thing to do is "fix" the internal impl of get_registered_limits() to just filter in python instead of doing N API calls | |
| 17:54:46 | sean-k-mooney | without a regioun or service id you get ll registered limits if you call keystoen | |
| 17:54:56 | melwitt | I know lol | |
| 17:55:04 | sean-k-mooney | the region we know right | |
| 17:55:07 | melwitt | that's literally what it's doing already | |
| 17:55:22 | melwitt | yes we know the region | |
| 17:55:24 | sean-k-mooney | well yes but is it just the service id we ned | |
| 17:55:28 | melwitt | we don't know the endpoint id | |
| 17:55:30 | melwitt | yes | |
| 17:57:30 | melwitt | er sorry yeah it's service id that the call actually needs. I keep mixing it up with the conf options | |
| 17:57:54 | sean-k-mooney | so the sdk supprot the service | |
| 17:57:59 | sean-k-mooney | https://github.com/openstack/openstacksdk/blob/master/openstack/identity/v3/service.py | |
| 17:58:17 | sean-k-mooney | and you can query by type i.e.. nova | |
| 17:58:21 | sean-k-mooney | *compute | |
| 17:58:22 | melwitt | yes, that's what oslo.limit does for the discovery | |
| 17:58:37 | melwitt | https://github.com/openstack/oslo.limit/blob/master/oslo_limit/limit.py#L294 | |
| 17:58:45 | sean-k-mooney | ya so you were suggeting we coudl prot that to nova | |
| 17:58:51 | melwitt | so we can do that too obviously | |
| 17:59:43 | melwitt | yeah, if I do that then no more private access. I thought about it but figured I'd just throw the "most efficient" thing up to get discussion | |
| 17:59:58 | opendevreview | Dan Smith proposed openstack/nova-specs master: Add one-time-use-devices spec https://review.opendev.org/c/openstack/nova-specs/+/943486 | |
| 18:00:24 | melwitt | that would also get us endpoint discovery as a backport | |
| 18:00:37 | sean-k-mooney | ya | |
| 18:01:21 | melwitt | probably a better move. I'll push an update with that and see what yall think | |
| 18:04:59 | sean-k-mooney | before you do | |
| 18:05:19 | sean-k-mooney | why are we even using oslo.limist for this when the openstack sdk has a function ot get registered limits | |
| 18:05:21 | sean-k-mooney | https://github.com/openstack/openstacksdk/blob/master/openstack/identity/v3/_proxy.py#L1625-L1634 | |
| 18:05:50 | melwitt | sean-k-mooney: to make it use the [oslo.limit] config section. or do you suppose there's another way to do that? | |
| 18:06:34 | sean-k-mooney | well we coudl read those config varible oursleves | |
| 18:06:44 | sean-k-mooney | i.e. for region | |
| 18:07:03 | sean-k-mooney | lookup the service id our selves and then just call keystone directly | |
| 18:07:24 | melwitt | yeah. will need all the other auth stuff too and I was thinking it should match what nova is using for enforcing | |
| 18:07:25 | sean-k-mooney | or even use our keystoneauth section | |
| 18:08:13 | sean-k-mooney | i would agree if oslo.limit provided the apis we needed :) | |
| 18:08:22 | sean-k-mooney | ok for now lets not boil the ocean | |
| 18:08:35 | sean-k-mooney | and focus on the smallest change to make it work | |
| 18:08:48 | melwitt | eh.. I was wary of that because the idea of using one avenue to get the limits and a different one to enforce. I'm not sure if there could be consequence for some part of the config being different | |
| 18:08:52 | melwitt | I was thinking it's likely less error prone to use the same exact thing oslo.limit is using | |