| *** Qiming has quit IRC | 00:16 | |
| *** sridhar_ram1 has joined #senlin | 00:31 | |
| *** sridhar_ram has quit IRC | 00:33 | |
| *** sridhar_ram1 has quit IRC | 01:06 | |
| *** Qiming has joined #senlin | 01:14 | |
| *** xuhaiwei has joined #senlin | 01:20 | |
| *** Yanyanhu has joined #senlin | 01:37 | |
| *** elynn has joined #senlin | 01:56 | |
| elynn | Morning :) | 02:03 |
|---|---|---|
| *** elynn_ has joined #senlin | 02:05 | |
| *** elynn has quit IRC | 02:09 | |
| openstackgerrit | Merged openstack/senlin: Set default isolation_level to READ_COMMITTED https://review.openstack.org/281093 | 02:15 |
| *** elynn__ has joined #senlin | 02:15 | |
| *** elynn_ has quit IRC | 02:17 | |
| *** elynn_ has joined #senlin | 02:21 | |
| openstackgerrit | Merged openstack/senlin: Fix race condition in service delete https://review.openstack.org/281069 | 02:23 |
| *** haiwei has joined #senlin | 02:24 | |
| *** elynn__ has quit IRC | 02:24 | |
| *** lixinhui_ has joined #senlin | 02:33 | |
| openstackgerrit | Di XiaoLi proposed openstack/senlin: Raise InvalidParameter exception when get resource with invalid sort key https://review.openstack.org/281609 | 02:37 |
| openstackgerrit | Merged openstack/senlin: Add cluster check and recover into API https://review.openstack.org/280677 | 02:41 |
| *** haiwei_ has joined #senlin | 02:47 | |
| *** haiwei has quit IRC | 02:48 | |
| *** sridhar_ram has joined #senlin | 03:19 | |
| Qiming | dixiaoli there? | 03:21 |
| dixiaoli | yes | 03:21 |
| *** yuanying_ has quit IRC | 03:21 | |
| Qiming | commented on your patch | 03:21 |
| Qiming | it is a good starting point | 03:21 |
| Qiming | if you need more clarification, please speak up | 03:21 |
| *** sridhar_ram has quit IRC | 03:22 | |
| openstackgerrit | Cindia-blue proposed openstack/senlin: Revise node check and recover parameters https://review.openstack.org/281619 | 03:24 |
| openstackgerrit | Merged openstack/senlin: Enforce multi-tenancy for cluster find https://review.openstack.org/281264 | 03:32 |
| openstackgerrit | Merged openstack/senlin: Enforce multi-tenancy for node find https://review.openstack.org/281295 | 03:32 |
| openstackgerrit | Merged openstack/senlin: Revise node check and recover parameters https://review.openstack.org/281619 | 03:39 |
| *** yuanying has joined #senlin | 03:39 | |
| dixiaoli | Qiming, I saw your comments. I partly agree with your comments. I think we just need step1. | 03:40 |
| dixiaoli | So my thought is:Add whitelist list check in method parse_sort_param in senlin.common.utils is ok. | 03:40 |
| dixiaoli | Since the parse_sort_param method have already raised InvalidParameter exception twice, there is no need to eliminate the DB level parsing which you said in step2 | 03:42 |
| dixiaoli | How do u think about my thoughts ? | 03:44 |
| *** yuanying has quit IRC | 03:55 | |
| *** yuanying has joined #senlin | 04:05 | |
| *** yuanying_ has joined #senlin | 04:07 | |
| *** yuanying has quit IRC | 04:09 | |
| *** elynn_ has quit IRC | 04:40 | |
| openstackgerrit | Di XiaoLi proposed openstack/senlin: Raise InvalidParameter exception when get resource with invalid sort key https://review.openstack.org/281609 | 05:01 |
| *** elynn_ has joined #senlin | 05:08 | |
| *** elynn_ has quit IRC | 05:12 | |
| *** elynn_ has joined #senlin | 05:13 | |
| *** zzxwill has joined #senlin | 05:14 | |
| *** lixinhui_ has quit IRC | 05:18 | |
| *** zzxwill has quit IRC | 05:25 | |
| openstackgerrit | Qiming Teng proposed openstack/senlin: Enforce multi-tenancy for event find https://review.openstack.org/281640 | 05:41 |
| openstackgerrit | Di XiaoLi proposed openstack/senlin: Raise InvalidParameter exception when get resource with invalid sort key https://review.openstack.org/281609 | 05:51 |
| openstackgerrit | Merged openstack/senlin: Enforce multi-tenancy for policy-find https://review.openstack.org/280710 | 05:52 |
| openstackgerrit | Merged openstack/senlin: Remove some useless methods in rpc client https://review.openstack.org/281021 | 05:52 |
| openstackgerrit | Merged openstack/senlin: Avoid using '__class__.__name__' when possible https://review.openstack.org/281033 | 05:53 |
| openstackgerrit | Merged openstack/senlin: Enforce multi-tenancy for actions https://review.openstack.org/281330 | 05:55 |
| *** sridhar_ram has joined #senlin | 06:05 | |
| *** lixinhui_ has joined #senlin | 06:06 | |
| *** sridhar_ram1 has joined #senlin | 06:12 | |
| *** sridhar_ram has quit IRC | 06:13 | |
| openstackgerrit | Qiming Teng proposed openstack/senlin: Rename SenlinBadRequest to BadRequest https://review.openstack.org/281656 | 06:17 |
| *** PennyLiu has joined #senlin | 06:20 | |
| *** sridhar_ram1 has quit IRC | 06:28 | |
| Yanyanhu | hi, Qiming, the action scheduling is broken for project_safe is enabled by default now. I have proposed a bug report here. | 06:29 |
| Yanyanhu | working on a fix for it | 06:29 |
| Qiming | Yanyanhu, local functional testing didn't complain a thing | 06:29 |
| Qiming | could it be your database is old? | 06:30 |
| Yanyanhu | oh, I see | 06:30 |
| Yanyanhu | my fault | 06:30 |
| Yanyanhu | I need to upgrade my db | 06:30 |
| Yanyanhu | thanks :) | 06:30 |
| Qiming | do a 'senlin-manage db_sync', that's all | 06:30 |
| Yanyanhu | yes | 06:30 |
| Qiming | though that could be the reason why the latest code doesn't work for you ... | 06:31 |
| Qiming | I did noticed some mysql-level error reports the first time I ran functional tests locally | 06:31 |
| Qiming | I tried to reproduce the problem but I wasn't able to do that | 06:32 |
| Qiming | it was something like 'operation out of order' ... | 06:32 |
| Yanyanhu | hmm, let me test it again | 06:32 |
| Qiming | great | 06:32 |
| Yanyanhu | that error still happened... | 06:34 |
| Yanyanhu | https://bugs.launchpad.net/senlin/+bug/1546881 | 06:34 |
| openstack | Launchpad bug 1546881 in senlin "Error happened when scheduling an action" [Critical,New] - Assigned to Yanyan Hu (yanyanhu) | 06:34 |
| Yanyanhu | let me check my db table | 06:34 |
| Yanyanhu | I didn't see any output when performing db update using senlin-manage db_sync | 06:35 |
| Qiming | so your db schema is up to date | 06:36 |
| Yanyanhu | action table has been the latest version | 06:36 |
| Yanyanhu | hmm | 06:36 |
| Qiming | yep, can see that | 06:36 |
| Qiming | so the problem is ... in ActionProc, the conext is different | 06:37 |
| Yanyanhu | yes | 06:37 |
| Yanyanhu | it is | 06:37 |
| Yanyanhu | it's admin context actually | 06:37 |
| Qiming | and the project could be empty | 06:37 |
| Yanyanhu | em, it's different from the project of requester | 06:37 |
| Qiming | this is actually a global bug | 06:37 |
| Qiming | if not context.is_admin and project_safe and context.project != objec.context: | 06:38 |
| Qiming | return None | 06:38 |
| Yanyanhu | yes, seems some bugs here | 06:38 |
| Yanyanhu | let me check it | 06:38 |
| Qiming | okay | 06:39 |
| *** branw has joined #senlin | 06:44 | |
| Yanyanhu | hi, Qiming, currently, is_admin flag will not be checked when performing ***_get() DB operations | 06:44 |
| Qiming | yes, that is the 'global' bug I was referring to | 06:44 |
| Yanyanhu | oh | 06:45 |
| Qiming | would you jump onto it? | 06:45 |
| Yanyanhu | so how should we fix it? | 06:45 |
| Yanyanhu | ok | 06:45 |
| Yanyanhu | so you mean we add it | 06:45 |
| Qiming | maybe a refactor of the sqlalchemy.api | 06:45 |
| Yanyanhu | yes, I think so | 06:46 |
| Qiming | where ever we are checking project_safe, we should make an exception for context.is_admin | 06:46 |
| Yanyanhu | right | 06:46 |
| Qiming | okay, it's yours | 06:46 |
| Yanyanhu | ok | 06:46 |
| Yanyanhu | will fix it asap | 06:47 |
| Yanyanhu | it blocks all actions execution... | 06:47 |
| Qiming | weird thing is it worked fine for me ... | 06:47 |
| Yanyanhu | yea... | 06:48 |
| Qiming | dixiaoli, xuhaiwei online? | 06:48 |
| dixiaoli | Qiming, yes | 06:48 |
| Qiming | regarding https://review.openstack.org/#/c/281609/ | 06:48 |
| Qiming | seems we need a quick sync | 06:48 |
| Qiming | I'm okay getting it in | 06:49 |
| haiwei_ | yes | 06:49 |
| Qiming | haiwei's comment on code optimization makes sense | 06:49 |
| Qiming | what I want to bring up is about the next step | 06:49 |
| haiwei_ | check the paras in service layer? | 06:50 |
| Qiming | it is suboptimal to call common.utils.parse_sort_param twice | 06:50 |
| Qiming | if we want to enforce a validation, it should be done earlier along the call path | 06:50 |
| haiwei_ | agree | 06:51 |
| Qiming | I mean, in the db layer, we should have a validated 'sort' param, what it has to do is to ensure that 'id' is in the sort_keys before passing to oslo_db layer | 06:51 |
| haiwei_ | currently this is checking is done in db/api | 06:53 |
| Qiming | there are cases where we are adding default sorting keys .. so the db layer '_parse_sort_param' should be improved | 06:53 |
| Qiming | so ... next step | 06:53 |
| Qiming | we need two util functions, one focusing on validation, invoked from engine service layer | 06:54 |
| Qiming | the other remain in the db layer, focusing on some further augmentation of the param | 06:54 |
| dixiaoli | for " ensure that 'id' is in the sort_keys" , I think we can do it in service layer too. | 06:55 |
| Qiming | dixiaoli, that is an option, agree | 06:55 |
| haiwei_ | yes, I agree | 06:55 |
| Qiming | however, db layer apis are called not just from service layer | 06:56 |
| haiwei_ | what do you mean by 'further augmentation of the param'? | 06:56 |
| Qiming | by further augmentation, I mean: 1) make sure id is in sort_keys, so that no matter user specified sort or not, the resulted list is always predictable; 2) add default_sorting_key | 06:57 |
| dixiaoli | _get_sort_params() method in db.sqlalchemy.api | 06:59 |
| dixiaoli | haiwei_, _get_sort_params() method in db.sqlalchemy.api did the 'further augmentation of the param' | 07:00 |
| haiwei_ | so about service layer validation, you will add a new method? | 07:01 |
| Qiming | yes, mostly a revision of the existing function | 07:02 |
| Qiming | let's get 281609 in | 07:02 |
| Qiming | I'll explain the idea via code | 07:03 |
| haiwei_ | ok | 07:03 |
| openstackgerrit | Yanyan Hu proposed openstack/senlin: Enable is_admin check in DB APIs https://review.openstack.org/281667 | 07:07 |
| Yanyanhu | hi, Qiming, patch is proposed here. I plan to add unit tests for this change in another patch | 07:08 |
| Yanyanhu | since this issue breaks senlin action workflow, hope fix it asap | 07:08 |
| Qiming | yes | 07:12 |
| Qiming | reviewed | 07:12 |
| Qiming | mostly minor nits | 07:12 |
| Qiming | understand the urgency of getting this fixed | 07:12 |
| openstackgerrit | Merged openstack/senlin: Raise InvalidParameter exception when get resource with invalid sort key https://review.openstack.org/281609 | 07:12 |
| openstackgerrit | Di XiaoLi proposed openstack/python-senlinclient: Add OpenstackClient plugin for cluster profile list https://review.openstack.org/281055 | 07:12 |
| Yanyanhu | got it | 07:23 |
| openstackgerrit | Yanyan Hu proposed openstack/senlin: Enable is_admin check in DB APIs https://review.openstack.org/281667 | 07:26 |
| Qiming | Yanyanhu, +2'ed | 07:29 |
| Yanyanhu | thanks :) | 07:30 |
| openstackgerrit | Merged openstack/senlin: Enable is_admin check in DB APIs https://review.openstack.org/281667 | 07:39 |
| openstackgerrit | Merged openstack/python-senlinclient: Add OpenstackClient plugin for cluster profile list https://review.openstack.org/281055 | 07:54 |
| haiwei_ | hi, Yanyanhu, Qiming, I used a demo user to create a node, why is_admin is True? | 08:12 |
| Yanyanhu | the demo user has admin role? | 08:13 |
| Qiming | smells like a configuration thing | 08:17 |
| haiwei_ | how to check a user's role | 08:18 |
| Qiming | openstack | 08:18 |
| Qiming | you got to be admin to list roles and role assignments | 08:19 |
| Qiming | it is a restriction of keystone | 08:19 |
| haiwei_ | got it, my demo user only has 'Member' and 'anotherrole' role | 08:19 |
| Qiming | well, it depends | 08:20 |
| Qiming | maybe that is not true | 08:20 |
| Qiming | you got to switch to keystone v3 to find it out | 08:20 |
| haiwei_ | you can get user role by 'openstack user role list USERNAME' | 08:20 |
| haiwei_ | $ openstack user role list demo +----------------------------------+-------------+---------+------+ | ID | Name | Project | User | +----------------------------------+-------------+---------+------+ | 59c481869c634f8cba5f68d818174b01 | anotherrole | demo | demo | | 6f6693e1b8ba4218bb398a85ecc2bb16 | Member | demo | demo | +----------------------------------+-------------+---------+---- | 08:21 |
| openstackgerrit | Qiming Teng proposed openstack/senlin: Refactor sorting param parsing at db layer https://review.openstack.org/281689 | 08:22 |
| haiwei_ | my demo user does not have 'admin' role, but is_admin is still True | 08:22 |
| Yanyanhu | hi, haiwei_ where the is_admin check return true? | 08:23 |
| Qiming | haiwei, you are not using keystone v3 | 08:23 |
| haiwei_ | in db/sqlachelmy/api.py | 08:23 |
| Qiming | the output from your command is misleading | 08:23 |
| Qiming | to switch to use keystone v3 when using openstackclient | 08:24 |
| haiwei_ | ok | 08:24 |
| Qiming | you have to append a --os-identity-api-version=3 | 08:24 |
| Qiming | to you openstack command | 08:24 |
| Qiming | after switching to v3 | 08:24 |
| Qiming | you won't have 'user role list' command | 08:24 |
| Qiming | it becomes 'role assignment list' | 08:25 |
| Qiming | it is confusing sometimes, so I really hate keystone v2 is still being used | 08:26 |
| Yanyanhu | haiwei_, which table you are accessing. The context using to access db could be different from the context of requester in many cases. | 08:26 |
| Yanyanhu | e.g. in ActionProc, senlin engine uses its own admin context(db session) to access action table | 08:28 |
| haiwei_ | action table, Yanyanhu | 08:28 |
| haiwei_ | in action_get method | 08:28 |
| Yanyanhu | if this is a get show request from user | 08:29 |
| Yanyanhu | is_admin flag should be false if this user doesn't have admin role | 08:29 |
| Qiming | haiwei_, dixiaoli, #281689 is a refactor at the DB layer, please help review when u r not so busy | 08:29 |
| Yanyanhu | sorry, a action show request | 08:29 |
| dixiaoli | Qiming, ok | 08:30 |
| Yanyanhu | since senlin engine service interfaces will parse the context transmitted from rpc | 08:30 |
| haiwei_ | I confirmed in keystone v3, demo user doesn't have admin role | 08:31 |
| Yanyanhu | and extract some useful info like, user,project,domain and use them as parameters to create action | 08:32 |
| Yanyanhu | let me find an example code for you | 08:32 |
| Yanyanhu | http://git.openstack.org/cgit/openstack/senlin/tree/senlin/engine/service.py#n715 | 08:33 |
| Yanyanhu | then these information will be handle here when initilizing an action object | 08:35 |
| Yanyanhu | http://git.openstack.org/cgit/openstack/senlin/tree/senlin/engine/actions/base.py#n98 | 08:35 |
| Yanyanhu | so the action.context is actually not the context of requester | 08:36 |
| Yanyanhu | This is because we planned to remove context from action | 08:37 |
| Yanyanhu | we believe only db_session is needed by action to finish its job | 08:37 |
| Yanyanhu | so we don't need to transmit the entire context through rpc | 08:37 |
| Yanyanhu | only transmitting user informatoin and rebuilding a db session object will be enough | 08:38 |
| Yanyanhu | although this work has not been done completely :) | 08:38 |
| Yanyanhu | that's why you can see some TODO about db session in action base module | 08:39 |
| haiwei_ | you mean the 'project' of context should be None? | 08:39 |
| Yanyanhu | no, it won't | 08:39 |
| haiwei_ | while action's project is not None | 08:39 |
| haiwei_ | from my test, context.project turned out to be None | 08:40 |
| Yanyanhu | unless the project property of original context is None | 08:40 |
| Yanyanhu | where did you put the test code | 08:41 |
| haiwei_ | wait a minite | 08:41 |
| Yanyanhu | ok | 08:42 |
| xuhaiwei | http://paste.openstack.org/show/487371/ | 08:42 |
| Yanyanhu | context.project is None? | 08:44 |
| Yanyanhu | but not action.project | 08:45 |
| Yanyanhu | http://git.openstack.org/cgit/openstack/senlin/tree/senlin/engine/scheduler.py#n43 | 08:45 |
| Yanyanhu | and here: http://git.openstack.org/cgit/openstack/senlin/tree/senlin/engine/actions/base.py#n500 | 08:46 |
| haiwei_ | yes, context.project is None | 08:46 |
| Yanyanhu | this is an admin_context which used by engine to schedule actions | 08:46 |
| Yanyanhu | so it doesn't belong to any user | 08:46 |
| Yanyanhu | and project as well, so only is_admin Flag is set | 08:47 |
| haiwei_ | yes | 08:47 |
| Yanyanhu | so in line500 in action/base.py, engine accesses db to query action table with this context | 08:48 |
| Yanyanhu | that's why you see context.project is None | 08:48 |
| Yanyanhu | and also is_admin is True :) | 08:49 |
| haiwei_ | ok, makes sense to me | 08:49 |
| haiwei_ | I will test some more | 08:49 |
| Yanyanhu | ok :) | 08:50 |
| openstackgerrit | Qiming Teng proposed openstack/senlin: Util function 'validate_sort_param' https://review.openstack.org/281696 | 08:52 |
| openstackgerrit | Di XiaoLi proposed openstack/python-senlinclient: Add OpenstackClient plugin for cluster profile delete https://review.openstack.org/281107 | 08:54 |
| openstackgerrit | Liuqing Jing proposed openstack/senlin: Update devstack comment https://review.openstack.org/281703 | 09:03 |
| *** Qiming has quit IRC | 09:06 | |
| openstackgerrit | Yanyan Hu proposed openstack/senlin: Add unit test for is_admin check in DB interfaces https://review.openstack.org/281714 | 09:26 |
| *** lixinhui_ has quit IRC | 09:38 | |
| *** Yanyanhu has quit IRC | 09:52 | |
| *** elynn_ has quit IRC | 09:52 | |
| openstackgerrit | Di XiaoLi proposed openstack/python-senlinclient: Add OpenstackClient plugin for cluster profile create https://review.openstack.org/281229 | 09:55 |
| *** openstackgerrit has quit IRC | 10:02 | |
| *** openstackgerrit has joined #senlin | 10:03 | |
| *** PennyLiu has quit IRC | 10:41 | |
| openstackgerrit | Di XiaoLi proposed openstack/python-senlinclient: Add OpenstackClient plugin for cluster profile update https://review.openstack.org/281758 | 10:50 |
| *** Qiming has joined #senlin | 11:56 | |
| openstackgerrit | Di XiaoLi proposed openstack/python-senlinclient: Add OpenstackClient plugin for cluster profile type list https://review.openstack.org/281809 | 12:45 |
| *** Qiming has quit IRC | 13:30 | |
| *** Qiming has joined #senlin | 13:32 | |
| *** Qiming has quit IRC | 13:32 | |
| *** Qiming has joined #senlin | 13:34 | |
| *** Liuqing has joined #senlin | 13:38 | |
| Liuqing | HI https://github.com/openstack/senlin/blob/master/devstack/plugin.sh#L17 should the cmd cleanup_senlin behind install_senlin?, | 13:39 |
| *** PennyLiu has joined #senlin | 13:46 | |
| *** Liuqing has quit IRC | 14:02 | |
| openstackgerrit | Merged openstack/senlin: Refactor sorting param parsing at db layer https://review.openstack.org/281689 | 14:23 |
| openstackgerrit | Qiming Teng proposed openstack/senlin: Util function 'validate_sort_param' https://review.openstack.org/281696 | 14:46 |
| *** Qiming has quit IRC | 15:49 | |
| *** zigo has quit IRC | 16:03 | |
| *** zigo has joined #senlin | 16:05 | |
| *** PennyLiu has quit IRC | 16:28 | |
| *** sridhar_ram has joined #senlin | 17:13 | |
| openstackgerrit | Ayush Garg proposed openstack/python-senlinclient: Add filters option to profile-list command https://review.openstack.org/282001 | 18:39 |
| *** sridhar_ram1 has joined #senlin | 19:02 | |
| *** sridhar_ram has quit IRC | 19:04 | |
| *** sridhar_ram1 has quit IRC | 21:03 | |
| *** sridhar_ram has joined #senlin | 21:04 | |
| *** sridhar_ram has quit IRC | 21:41 | |
| *** sridhar_ram has joined #senlin | 21:44 | |
| *** sridhar_ram has quit IRC | 22:15 | |
| *** sridhar_ram has joined #senlin | 22:18 | |
| *** Qiming has joined #senlin | 23:33 | |
| *** xuhaiwei has quit IRC | 23:39 | |
| *** openstackgerrit has quit IRC | 23:47 | |
| *** openstackgerrit_ is now known as openstackgerrit | 23:47 | |
| *** openstackgerrit_ has joined #senlin | 23:48 | |
| *** openstackgerrit_ is now known as openstackgerrit | 23:48 | |
| *** openstackgerrit_ has joined #senlin | 23:49 | |
| *** openstackgerrit_ has quit IRC | 23:55 | |
| *** openstackgerrit_ has joined #senlin | 23:57 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!