| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-01-22 | |||
| 00:20:03 | brinzhang | gmann: I know you are an expert in tempest, can you help to check? | |
| 00:20:30 | brinzhang | gmann: the tempest patch https://review.opendev.org/c/openstack/tempest/+/769120 | |
| 00:22:01 | gmann | brinzhang: sure, you want to move to openstack-qa ? | |
| 00:24:07 | brinzhang | gmann: of course | |
| 00:50:48 | gmann | brinzhang_: replied on gerrit for your query about os-simple-tenant-usage action https://review.opendev.org/c/openstack/nova/+/768852 | |
| 00:51:19 | gmann | brinzhang_: we still nee old API route and controller/sample/test etc as this is still valid for old microversion | |
| 00:53:06 | gmann | we do not have microversion knowledge in routes so we can add new one with old one still there and in API method in controller we can return 404 for old API for >2.89 | |
| 00:53:44 | brinzhang_ | gmann: greate, will check | |
| 00:54:02 | gmann | which is nothing but this @wsgi.Controller.api_version("2.1", "2.89") | |
| 00:55:09 | gmann | and new API method can be decorated with @wsgi.Controller.api_version("2.90") | |
| 00:55:28 | gmann | so any user requesting new API will get 404 with <2.90 | |
| 00:57:14 | gmann | basically keep the old API things (test/sample etc) as it is but with capping it with microversion 2.89. | |
| 00:58:46 | brinzhang_ | you mean, I should add @wsgi.Controller.api_version("2.90") in the index and show api? | |
| 00:59:02 | brinzhang_ | s/in/on/ | |
| 00:59:26 | gmann | for new API yes | |
| 00:59:50 | brinzhang_ | https://review.opendev.org/c/openstack/nova/+/768509/10/nova/api/openstack/compute/simple_tenant_usage.py | |
| 01:00:29 | brinzhang_ | gmann: I think I should add two api for 2.90, and add the max version limit the old version | |
| 01:00:56 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for resource providers https://review.opendev.org/c/openstack/placement/+/760240 | |
| 01:00:58 | gmann | like ServerTopology API added in 2.78 https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/server_topology.py#L26 | |
| 01:01:03 | brinzhang_ | gmann: I am not understand waht you say about the router change | |
| 01:01:38 | gmann | brinzhang_: in routes, you just need to add the new controller and url path and leave old one as it is | |
| 01:02:09 | brinzhang_ | ack | |
| 01:02:31 | gmann | because old API still valid for older microversion. | |
| 01:02:41 | brinzhang_ | gmann: do we need to rename the tenant file? | |
| 01:02:50 | gmann | which one? | |
| 01:03:24 | gmann | this one? https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/simple_tenant_usage.py | |
| 01:03:29 | brinzhang_ | this patch I add the 2.90 support https://review.opendev.org/c/openstack/nova/+/768509/10/nova/api/openstack/compute/simple_tenant_usage.py | |
| 01:04:56 | brinzhang_ | and https://review.opendev.org/c/openstack/nova/+/768852 renamed the tenant* to project* | |
| 01:06:06 | gmann | brinzhang_: there are two part 1. changing the 'tenant' in response etc which is 768509 this is good as you are doing, I had a quick glance | |
| 01:06:45 | brinzhang_ | https://review.opendev.org/c/openstack/nova/+/768509/10/nova/api/openstack/compute/simple_tenant_usage.py#295 | |
| 01:07:10 | gmann | and 2nd is about adding new API method which is 768852 for this we can add new file or new method using old method in existing file | |
| 01:07:42 | brinzhang_ | this patch I should add the @wsgi.Controller.api_version("2.1", "2.89") in the old index and show api, and add @wsgi.Controller.api_version("2.90") to as the new api for index and show, right? | |
| 01:07:58 | gmann | right | |
| 01:08:36 | brinzhang_ | but this change in the 768852 | |
| 01:08:48 | brinzhang_ | as your suggestion | |
| 01:08:58 | gmann | have two option here 1. if rename file then add new controller class which can reuse the old APi method 2. have complete new file with new method which can also use existing method | |
| 01:09:40 | gmann | may be 1st option is better. you can rename file and keep old controller class in that file with all API method as @wsgi.Controller.api_version("2.1", "2.89") | |
| 01:10:01 | gmann | and new controller class with new method with @wsgi.Controller.api_version("2.90") and add these in routes.py | |
| 01:12:25 | gmann | and same way in tests also we can do the same to reuse the code | |
| 01:13:13 | brinzhang_ | gmann: yes, in 768852, we renamed the file, and reused the old test | |
| 01:14:31 | brinzhang_ | I need to keep the old action in the router and add the new action controller | |
| 01:14:50 | gmann | brinzhang_: but you are changing old API route and controller which means old API is completely gone - https://review.opendev.org/c/openstack/nova/+/768852/7/nova/api/openstack/compute/simple_project_usage.py | |
| 01:15:48 | brinzhang_ | is it need to restort the nova/api/openstack/compute/simple_tenant_usage.py | |
| 01:15:49 | brinzhang_ | file? | |
| 01:15:52 | gmann | for example: GET /os-simple-tenant-usage wil return 404 even for older microversion | |
| 01:15:59 | brinzhang_ | and add the nova/api/openstack/compute/simple_project_usage.py? | |
| 01:16:15 | gmann | that is one option | |
| 01:16:18 | gmann | or other option is | |
| 01:17:22 | gmann | rename and add SimpleProjectUsageController(SimpleTenantUsageController) as new class there | |
| 01:18:25 | gmann | and in routes.py you can add new path like ('/os-simple-project-usage', { | |
| 01:18:25 | gmann | 'GET': [simple_project_usage_controller, 'index'] | |
| 01:19:04 | gmann | def index() in new controller will just call the old SimpleTenantUsageController.index() and have @wsgi.Controller.api_version("2.90") | |
| 01:19:49 | brinzhang_ | gmann: ok, we will try to use the two option | |
| 01:20:05 | brinzhang_ | a question | |
| 01:21:00 | brinzhang_ | @wsgi.Controller.api_version("2.1", "2.89") and @wsgi.Controller.api_version("2.90") for the api change, is it need to change with the router change? | |
| 01:21:15 | brinzhang_ | I mean keep this change in 768852 patch | |
| 01:21:57 | brinzhang_ | gmann: what do you think? | |
| 01:25:29 | gmann | brinzhang_: in routes.py if you rename the file then 1. for old API route, change import (from nova.api.openstack.compute import simple_tenant_usage -> from nova.api.openstack.compute import simple_project_usage ) | |
| 01:25:47 | gmann | and for new API add new route as separate | |
| 01:29:07 | gmann | brinzhang_: like this http://paste.openstack.org/show/801854/ | |
| 01:30:26 | brinzhang_ | gmann: yeah, get it | |
| 01:31:28 | brinzhang_ | gmann: above question is which patch I shuold change the max or min version changing of the index and show usages apis? | |
| 01:32:06 | gmann | but other file like request/response sample file can exist in separate if it become very lengthy and unreadable | |
| 01:32:53 | gmann | brinzhang_: ah that is good question but tricky also. | |
| 01:33:10 | brinzhang_ | ok, I can seperate the route change from 768852 | |
| 01:35:26 | gmann | brinzhang_: it has to be route change in 768852 and 768509 changing the response etc. so 768509 as first will not change anything for older microversion | |
| 01:35:45 | gmann | so you current order of patches is good | |
| 01:36:14 | gmann | this way in 768852 we expose the new API url with new response | |
| 01:36:45 | gmann | and restrict old API with 2.89 and return old response | |
| 01:37:06 | brinzhang_ | ok, just need to do these changes in 768852 ^ | |
| 01:37:24 | brinzhang_ | and keep the 768509 not change | |
| 01:37:41 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for aggregates https://review.opendev.org/c/openstack/placement/+/760235 | |
| 01:37:42 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for allocations https://review.opendev.org/c/openstack/placement/+/760236 | |
| 01:37:43 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for allocation candidates https://review.opendev.org/c/openstack/placement/+/760237 | |
| 01:37:44 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for inventories https://review.opendev.org/c/openstack/placement/+/760238 | |
| 01:37:45 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for resource classes https://review.opendev.org/c/openstack/placement/+/760239 | |
| 01:37:46 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for traits https://review.opendev.org/c/openstack/placement/+/760241 | |
| 01:37:47 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for usage https://review.opendev.org/c/openstack/placement/+/760242 | |
| 01:37:57 | lbragstad | gmann ^ those should have all the bits for testing the deprecated policies | |
| 01:39:28 | gmann | lbragstad: thanks. I will review tomorrow with results. basically almost good to go just need to have new test green | |
| 01:40:03 | gmann | brinzhang_: yes. for this route things. I will review the complete series tomorrow if any thing else missing | |
| 01:40:28 | brinzhang_ | gmann: thanks | |
| 01:40:29 | lbragstad | gmann sounds good - they all pass for me locally, i'll follow up tomorrow | |
| 01:40:34 | gmann | brinzhang_: key thing is to pass the existing tests and Tempest without any change means you are not breaking anything :) | |
| 01:40:42 | gmann | lbragstad: great. | |
| 01:41:25 | gmann | brinzhang_: if you have to change anything then we are introducing some backward incompatible change | |
| 01:41:35 | brinzhang_ | gmann: yeah, the tempest change will change as your suggestion, and it you can review tomorrow | |
| 01:42:08 | gmann | brinzhang_: +1 | |
| 01:42:19 | brinzhang_ | yes, we cannt break the old test in tempest | |
| 01:42:42 | gmann | I can help on Tempest change tomorrow if anything you miss of need help with. | |
| 01:42:50 | gmann | *or need | |
| 01:47:06 | gmann | brinzhang_: going away. will check the series tomorrow. happy Friday to you :) | |
| 01:47:54 | brinzhang_ | gmann: thanks, good night^^ | |
| 03:04:15 | brinzhang_ | gmann: hi are you around? | |
| 03:05:20 | brinzhang_ | nova/tests/functional/api_sample_tests/api_samples/os-simple-project-usage/simple-tenant-usage-get.json.tpl like this file, the sample file do need keep ing the "os-simple-tenant-usage" dir? | |
| 03:05:30 | brinzhang_ | s/ing/under | |
| 03:44:56 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for resource providers https://review.opendev.org/c/openstack/placement/+/760240 | |
| 03:44:57 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for aggregates https://review.opendev.org/c/openstack/placement/+/760235 | |
| 03:44:58 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for allocations https://review.opendev.org/c/openstack/placement/+/760236 | |
| 03:44:59 | openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for allocation candidates https://review.opendev.org/c/openstack/placement/+/760237 | |