| *** Qiming has joined #senlin | 00:25 | |
| *** pratikmallya has joined #senlin | 00:43 | |
| *** pratikmallya has quit IRC | 00:48 | |
| *** Liuqing has joined #senlin | 01:37 | |
| *** pratikmallya has joined #senlin | 01:41 | |
| openstackgerrit | Merged openstack/senlin-dashboard: Use the defined INDEX_URL in clusters forms and views https://review.openstack.org/261995 | 01:50 |
|---|---|---|
| *** elynn has joined #senlin | 01:59 | |
| *** elynn has quit IRC | 02:03 | |
| *** elynn has joined #senlin | 02:04 | |
| *** pratikmallya has quit IRC | 02:06 | |
| elynn | Morning | 02:09 |
| Qiming | o/ | 02:11 |
| xuhaiwei | morning | 02:12 |
| elynn | Just saw my patch for opensdk got an -2 from Brian... | 02:12 |
| openstackgerrit | Cindia-blue proposed openstack/senlin: Add DB API to Help Health Check in Scaling Policy https://review.openstack.org/262709 | 02:12 |
| *** Yanyanhu has joined #senlin | 02:17 | |
| Yanyanhu | hi, Qiming, will check those patches and bugs about policy and lb | 02:19 |
| Qiming | Yanyanhu, pls check the trust id patch | 02:22 |
| Qiming | I'm not quite sure whether trust id should be a list | 02:23 |
| Yanyanhu | yes, me too, I'm checking it now | 02:23 |
| Yanyanhu | hi, junxu, are you around? | 02:39 |
| Yanyanhu | just tried to reproduce this bug you reported https://bugs.launchpad.net/senlin/+bug/1530504 | 02:39 |
| openstack | Launchpad bug 1530504 in senlin "Class Policy's _build_conn_params method return wrong value" [Undecided,New] | 02:39 |
| Yanyanhu | but didn't see error happen | 02:39 |
| Yanyanhu | I think trust_id list is accepetable for openstacksdk | 02:40 |
| junxu | Maybe it has problem for kilo keystone | 02:46 |
| Yanyanhu | another possibility is the error will happen when the length of trust_id list is larger than one | 02:48 |
| Yanyanhu | can you print the params that generated by build_conn_param to check it? | 02:49 |
| junxu | just one trust_id | 02:49 |
| Yanyanhu | hmm, maybe the env issue | 02:50 |
| Yanyanhu | maybe you can update your keystone version and test it again | 02:51 |
| Yanyanhu | anyway, the bahvior of _build_conn_param in profile/base.py and policy/base.py are different, we may need a fix here | 02:53 |
| Yanyanhu | after we find the exact reason of bug1530504 | 02:54 |
| junxu | ok thanks | 02:59 |
| Yanyanhu | junxu, so I think you can keep patch 262941 and use it for this fix after completing the test of trust_id param | 03:02 |
| *** Qiming has quit IRC | 03:21 | |
| *** Qiming has joined #senlin | 03:41 | |
| *** elynn has quit IRC | 04:15 | |
| openstackgerrit | zhangguoqing proposed openstack/senlin: doc: fix cluster https://review.openstack.org/263555 | 04:28 |
| openstackgerrit | xu-haiwei proposed openstack/senlin: Fix KeyError caused by getting 'security_groups' from server_data https://review.openstack.org/263558 | 05:04 |
| *** Liuqing has quit IRC | 05:05 | |
| *** Liuqing has joined #senlin | 05:06 | |
| *** elynn has joined #senlin | 05:12 | |
| openstackgerrit | Merged openstack/senlin: Remove some useless db apis https://review.openstack.org/263163 | 05:44 |
| *** Liuqing has quit IRC | 05:51 | |
| junxu | Hi all, to get all cluster, which request is correct, Get /v1/d3d2f1bc367c493fa5649fd377440c8b/clusters or GET /v1/dclusters? When I uses the latest python-senlinclient which request is like /v1/d3d2f1bc367c493fa5649fd377440c8b/clusters, then return a 404. | 06:07 |
| openstackgerrit | Cindia-blue proposed openstack/senlin: Add DB API to Help Health Check in Scaling Policy https://review.openstack.org/262709 | 06:12 |
| xuhaiwei | junxu, i think the latter one is right | 06:13 |
| junxu | no need project_id ? | 06:15 |
| xuhaiwei | yes | 06:15 |
| openstackgerrit | Cindia-blue proposed openstack/senlin: Add DB API to Help Health Check in Scaling Policy https://review.openstack.org/262709 | 06:21 |
| junxu | Thanks xuhaiwei! | 06:21 |
| xuhaiwei | you are welcome | 06:24 |
| Qiming | junxu, we may release a new version of python-senlinclient soon | 06:38 |
| Qiming | if you are downloading the "latest" version from Pypi, there will be some compatibility problems | 06:38 |
| openstackgerrit | Merged openstack/python-senlinclient: Deprecated tox -downloadcache option removed https://review.openstack.org/256714 | 06:48 |
| openstackgerrit | Merged openstack/senlin: Add DB API to Help Health Check in Scaling Policy https://review.openstack.org/262709 | 06:50 |
| openstackgerrit | Merged openstack/senlin: doc: fix cluster https://review.openstack.org/263555 | 06:50 |
| openstackgerrit | Yanyan Hu proposed openstack/senlin: Add metadata updating support for os.nova.server profile https://review.openstack.org/262658 | 06:55 |
| openstackgerrit | Yanyan Hu proposed openstack/senlin: Remove webhook functional test https://review.openstack.org/263597 | 07:10 |
| openstackgerrit | xu-haiwei proposed openstack/senlin: Add zones property to deletion policy template https://review.openstack.org/263608 | 07:36 |
| openstackgerrit | Merged openstack/senlin: Remove webhook functional test https://review.openstack.org/263597 | 07:37 |
| Qiming | xuhaiwei, there? | 07:43 |
| xuhaiwei | yes | 07:44 |
| Qiming | hi, we need a discussion about the deletion regarding cross-az and regions | 07:44 |
| xuhaiwei | ok | 07:45 |
| Qiming | lixinhui, online? | 07:45 |
| xuhaiwei | I have thought this for a while | 07:45 |
| Qiming | yes, thanks for working on this thread | 07:45 |
| lixinhui | yes | 07:45 |
| Qiming | lixinhui, since you were involved in cross-az/region placement policy | 07:46 |
| Qiming | want to hear your opinions as well | 07:46 |
| Qiming | the problem is that we have placement policy for cross-az and cross-region | 07:46 |
| lixinhui | okay, Qiming and Haiwei. I would like to join this discussion | 07:47 |
| xuhaiwei | by the way I have tested placement policy for zones, it works fine | 07:47 |
| xuhaiwei | currently I have no idea for cross-region | 07:47 |
| Qiming | they can be treated as some builtin policies, users can replace them whenever they want | 07:47 |
| Qiming | however, we still need to make the policies work in an elegant manner | 07:48 |
| xuhaiwei | you mean that will be a new policy? | 07:48 |
| xuhaiwei | or part of deletion policy? | 07:48 |
| Qiming | xuhaiwei, that is something we need to investigate | 07:48 |
| xuhaiwei | ok | 07:49 |
| lixinhui | can we resue the logic in placement | 07:49 |
| xuhaiwei | what about sharing my idea firstly? | 07:49 |
| Qiming | I can see that you are trying to make cross-az factors part of deletion policy | 07:49 |
| xuhaiwei | yes | 07:49 |
| Qiming | we need to think about how placement policy and deletion policy work together | 07:49 |
| xuhaiwei | yes, indeed, I found a problem about them | 07:50 |
| lixinhui | I aggree | 07:50 |
| Qiming | currently, placement policy is only about node creation | 07:50 |
| Qiming | so ... it is incomplete | 07:50 |
| lixinhui | yes, Qiming | 07:50 |
| Qiming | it cannot guarantee nodes are always spread as expected | 07:50 |
| xuhaiwei | I dont understand you | 07:50 |
| lixinhui | We need logic to node deletion | 07:51 |
| Qiming | we need to consider node delete cases as well | 07:51 |
| xuhaiwei | ok | 07:51 |
| lixinhui | I mean to consider ranking nodes to find which one to remove for better balance | 07:51 |
| Qiming | say ... we have a placement policy {'az1': 100, 'az2': 50} | 07:51 |
| Qiming | we are doing/enforcing this when creating nodes | 07:52 |
| xuhaiwei | because we inpurt 'placement' into node data, it seems ok to get information needed to do node deletion | 07:52 |
| Qiming | bu when deleting nodes, we are not considering this distribution | 07:52 |
| xuhaiwei | I am considering this, Qiming | 07:52 |
| Qiming | but if we have a deletion policy which says {'az1': 100, 'az2': 100} | 07:53 |
| xuhaiwei | it seems not difficult to solve this problem | 07:53 |
| lixinhui | :0 | 07:53 |
| lixinhui | :) | 07:53 |
| xuhaiwei | ok, go on | 07:53 |
| lixinhui | :) | 07:53 |
| xuhaiwei | I'd better to listen to you first | 07:53 |
| lixinhui | Qiming, go on | 07:53 |
| Qiming | so ... I'm considering whether we should extend the placement policy to be a policy that is checked both before node creation and node deletion | 07:54 |
| Qiming | no one said that placement can be only checked before node creation | 07:54 |
| Qiming | it is a half-baked solution, though it works fine for node creation (a million thanks for testing this btw) | 07:55 |
| lixinhui | I think it is reasonable to provide recommendation based on placment ranks | 07:56 |
| xuhaiwei | but checking after node deletion is deletion policy's work, isn't it? | 07:56 |
| Qiming | just some gut feeling | 07:56 |
| Qiming | deletion policy will have its opinion too, that is for sure | 07:56 |
| lixinhui | xuhaiwei, I think deletion can not know what is right ranks | 07:56 |
| Qiming | so ... if you only have a placement policy attached to a cluster | 07:57 |
| xuhaiwei | we can set the weight in deletion policy template, lixinhui | 07:57 |
| lixinhui | placement policy is the place for users to define the rank criteria | 07:57 |
| Qiming | you can always guarantee that nodes are spreaded among AZs as expected | 07:57 |
| xuhaiwei | that is not enough? | 07:57 |
| Qiming | if you have deletion policy as well, it can further refine the list of candidates, but from a different perspective | 07:58 |
| Qiming | my primary intent is to first make each policy self-contained | 08:00 |
| Qiming | each policy should achieve its goal without having to consider other policies | 08:00 |
| Qiming | then we handle conflicts between policies, define rules that are easy for users to understand | 08:01 |
| xuhaiwei | about deletion policy for cross-zones, my idea is: we check deletion policy template first to see if 'zones' are provided, it not, do as usual, if it is, then we divide nodes into different zones, and then get a dict like {'az1': ['node1', 'node3'], 'az2': ['node2']}, then we get the weight of each zone and calculate delete how many node from each zones, and finally use other rules like 'old_first' to delete the nodes | 08:02 |
| Qiming | yes, xuhaiwei, that is the logic | 08:03 |
| Qiming | I agree | 08:03 |
| lixinhui | cool | 08:03 |
| xuhaiwei | if we do like this, there seems no conflict with other policies | 08:03 |
| Qiming | my question is whether we should move the generation of dict to placement policy | 08:03 |
| xuhaiwei | hmm | 08:04 |
| xuhaiwei | I think we should do it in deletion policy | 08:04 |
| xuhaiwei | there is a problem that is not every node containing the 'placement' data | 08:05 |
| Qiming | if we put it in deletion policy, there are two drawbacks | 08:05 |
| lixinhui | currently, the target for placmenet to bond is only scale-out | 08:05 |
| xuhaiwei | if the node is not created by placement policy, it does not have that data | 08:05 |
| lixinhui | we can extend it with scale-in | 08:05 |
| Qiming | yes, xuhaiwei, that is why we should consider cross-az distribution in placement policy | 08:06 |
| Qiming | if you don't have a placment policy, you don't have placement data | 08:06 |
| xuhaiwei | it makes sense to me | 08:06 |
| xuhaiwei | but it seems difficult to do it in placement policy | 08:07 |
| xuhaiwei | we can't manage all the nodes while just placing some of them | 08:08 |
| Qiming | if we consider scale-in (and/or resize) in placement policy, we can always ensure how nodes are distributed, no matter we have a deletion policy or not | 08:08 |
| Qiming | xuhaiwei, that is a problem for users to consider | 08:09 |
| Qiming | say you have a cluster of 10 nodes already created | 08:09 |
| Qiming | now you attach a placement policy to it | 08:09 |
| Qiming | we won't move your nodes around | 08:09 |
| Qiming | if you want a full control of your nodes inside a cluster | 08:10 |
| Qiming | create an empty one, attach a placement policy | 08:10 |
| Qiming | then resize/scale it | 08:10 |
| xuhaiwei | I am afraid some people may only use deletion policy without using placement policy | 08:11 |
| Qiming | that is fine | 08:11 |
| Qiming | if you are using deletion policy only | 08:11 |
| Qiming | you are not worrying about node placement | 08:11 |
| Qiming | you cannot guarantee node distribution by only attach a deletion policy | 08:12 |
| lixinhui | i think we would need explaination in doc about these differences | 08:13 |
| Qiming | yep | 08:14 |
| xuhaiwei | placement policy will be used by 'scale-out' and 'resize'? | 08:14 |
| Qiming | we will need a section about using senlin with policies | 08:14 |
| Qiming | we need to guide users through how auto-scaling can be achieved, auto-healing, and cross-az/region distribution etc. | 08:15 |
| Qiming | xuhaiwei, ideally, yes | 08:15 |
| openstackgerrit | Qiming Teng proposed openstack/senlin: Add 'wait_for_delete' call for functional tests https://review.openstack.org/263038 | 08:16 |
| xuhaiwei | so if user use 'node-join' and 'node-leave' or other actions to change the cluster size, placement policy can still handle it? | 08:16 |
| Qiming | I'm expecting that node-join and node-leave won't be used in that scenario | 08:17 |
| Qiming | if you are expecting senlin to do a placement for you, why you are manually add/remove some specific nodes? | 08:17 |
| Qiming | if you do that, no one will stop you, :) it is one of the power you get from senlin | 08:18 |
| Qiming | I mean, you can do whatever you want, :) | 08:18 |
| xuhaiwei | I am just thinking about some cases people will do, in my opinion, we should make senlin used as easy as possiple | 08:19 |
| Qiming | yes | 08:19 |
| Qiming | totally agree | 08:19 |
| xuhaiwei | if we let placement policy do all the jobs, there are so many limits | 08:20 |
| Qiming | then you don't attach placement policy | 08:21 |
| Qiming | say you have 10 nodes in AZ1 and 4 nodes in AZ2, and the distribution you designed is 'AZ1': 100, 'AZ2': 50 | 08:22 |
| Qiming | the current distribution is acceptable | 08:22 |
| Qiming | you can have 9 nodes in AZ1 and 5 in AZ2, it is also acceptable | 08:23 |
| Qiming | now you, as an operator, do a node-join, adding a node from AZ1 | 08:23 |
| Qiming | we cannot reject that | 08:23 |
| Qiming | we don't know what the operator wants | 08:24 |
| Qiming | you are getting a 11:4 distribution after the node-join | 08:24 |
| Qiming | but it is fine | 08:24 |
| Qiming | next time you do a resize or scale, the placement policy still works | 08:24 |
| xuhaiwei | but deletion policy will get wrong information in this case | 08:25 |
| Qiming | okay, got what you mean | 08:25 |
| Qiming | that is a bug we need to fix | 08:25 |
| Qiming | when we add a new node to a cluster, we should assign a 'placement' node data automatically? | 08:26 |
| xuhaiwei | that is one solution | 08:26 |
| xuhaiwei | but if we want to do so much jobs, why not just leave them all to deletion policy? | 08:29 |
| Qiming | deletion policy itself cannot guarantee placement | 08:30 |
| Qiming | just like the current version of placement policy, which is a half-baked solution as well | 08:30 |
| xuhaiwei | it does not need to | 08:30 |
| elynn | Hi Qiming | 08:35 |
| elynn | Cluster_update can not work for now? | 08:35 |
| Qiming | elynn, don't know | 08:35 |
| Qiming | it is a big question | 08:36 |
| Qiming | it depends on what you are updating | 08:36 |
| elynn | In my env, cluster create works well | 08:36 |
| elynn | just update cluster name | 08:36 |
| elynn | And I got a not found error | 08:36 |
| elynn | http://paste.openstack.org/show/483003/ | 08:36 |
| openstackgerrit | Yanyan Hu proposed openstack/senlin: Set cluster to correct status when scaling failed https://review.openstack.org/263631 | 08:37 |
| elynn | Can you update cluster in latest codes? | 08:37 |
| Qiming | it works for me | 08:37 |
| xuhaiwei | you are not using the latest source I think | 08:37 |
| xuhaiwei | the same to me | 08:37 |
| elynn | I think I'm using the latest codes... but openstacksdk is not latest | 08:38 |
| Qiming | how old is your sdk code? | 08:39 |
| Qiming | the recent change to sdk regarding keystoneauth must be there, or else everything breaks | 08:39 |
| elynn | 0.7.1 | 08:40 |
| elynn | Let me upgrade to 0.7.3 and have a try | 08:41 |
| Qiming | 0.7.1 was created on 2015-11-24 | 08:41 |
| elynn | ok, now it works... | 08:42 |
| Qiming | keystoneauth change was merged on 2015-11-20 | 08:42 |
| elynn | Now I got a new error AttributeError: "'Proxy' object has no attribute 'cluster_resize'" ... I might install openstacksdk from codes... | 08:47 |
| xuhaiwei | Qiming, about the topic we discussed, if make placement policy do zone information collection, the data will be stored where? in cluster property? | 08:47 |
| lixinhui | in action.data | 08:51 |
| lixinhui | xuhaiwei | 08:52 |
| lixinhui | the action should be mapped to one kind of cluster_actions | 08:52 |
| Qiming | elynn, please do | 08:52 |
| Qiming | cluster_resize was recently proposed and merged | 08:53 |
| Yanyanhu | elynn, me too, that's a bug I think | 08:54 |
| xuhaiwei | lixihui, if put the data in action.data, how to get it when deleting nodes | 08:54 |
| Yanyanhu | oh, looks I need latest sdk code | 08:54 |
| lixinhui | xuhaiwei, which kind of action will be targeted by deletion policty | 08:55 |
| lixinhui | should be what kinds of actions | 08:55 |
| lixinhui | to target | 08:55 |
| lixinhui | cluster_scale_in? cluster_node_del | 08:56 |
| lixinhui | ? | 08:56 |
| lixinhui | or somethings like this | 08:56 |
| elynn | Hi Qiming, Yanyanhu, Is it ok to seperate cluster profile update from cluster update? | 08:56 |
| elynn | And return action in response. | 08:57 |
| elynn | I see that now we only return cluster id from cluster update. | 08:57 |
| Yanyanhu | elynn, currently action will be returned when doing cluster update operation? | 08:57 |
| Yanyanhu | let me check the code | 08:58 |
| xuhaiwei | I think there is nothing related to action types, when we deleting nodes using cluster-scale-in, we may want to get the nodes' zone infor, so that infor should be in cluster.data I think | 08:58 |
| Qiming | elynn, we'd better make the response consistent | 08:58 |
| Yanyanhu | maybe action is not returned in all cases | 08:58 |
| lixinhui | xuhaiwei | 08:58 |
| lixinhui | why? | 08:58 |
| Qiming | feel free to propose patches to make them consistent | 08:58 |
| openstackgerrit | Qiming Teng proposed openstack/senlin: Remove some example policies https://review.openstack.org/263640 | 08:59 |
| Yanyanhu | yes, that's true, elynn, a dict will be returned | 08:59 |
| elynn | I can't find the action id for cluster profile id. | 08:59 |
| xuhaiwei | lixinhui, because we don't know placement policy is triggered by which action, we just want the result | 08:59 |
| Yanyanhu | you mena for cluster profile update? | 09:00 |
| elynn | for cluster profile update | 09:00 |
| Yanyanhu | s/mena/mean | 09:00 |
| elynn | sorry typo | 09:00 |
| Yanyanhu | line 621 in service.py | 09:00 |
| Yanyanhu | the action is there and you can get the action.id directly I think | 09:00 |
| lixinhui | xuhaiwei, I think that is just a problem brought by your solution | 09:01 |
| elynn | yes, I see it, but where should I return the action.id? | 09:01 |
| lixinhui | of corse, you can do a complete contained the implments in deletion policy :) | 09:02 |
| Yanyanhu | http://git.openstack.org/cgit/openstack/senlin/tree/senlin/engine/service.py#n621 | 09:02 |
| elynn | I directly return cluster.to_dict() in L631 | 09:02 |
| Yanyanhu | line 631 should be changed I think | 09:02 |
| lixinhui | xuhaiwei, what I am thinking is to composite all the fucntions togetehr - different policy focus on differnet logic | 09:02 |
| Yanyanhu | to include both cluster attr dict and action_id fields into the return value? | 09:03 |
| openstackgerrit | Qiming Teng proposed openstack/senlin: Move some policies into WIP directory https://review.openstack.org/263641 | 09:03 |
| lixinhui | xuhaiwei, and no logic dunplication | 09:03 |
| elynn | resp = cluster.to_dict(); resp.update({'action': action.id})? | 09:03 |
| lixinhui | duplication | 09:03 |
| xuhaiwei | lixinhui, yes? | 09:03 |
| elynn | Yanyanhu: That sounds good, and openstacksdk needs to modify too. | 09:03 |
| elynn | Add a action_id for cluster resource in openstacksdk? or directly return dict back? | 09:04 |
| Yanyanhu | elynn, I think we need to check how sdk expects updating API to work | 09:04 |
| elynn | Yanyanhu: It return a cluster class in opensdk. | 09:05 |
| Yanyanhu | whether we have to return the resource attr dict within the resp body | 09:05 |
| elynn | But cluster class won't handle 'action_id' return from senlin. | 09:05 |
| Qiming | elynn, have you read the comments to https://review.openstack.org/261970 ? | 09:06 |
| Qiming | it is not expected by sdk developers | 09:07 |
| lixinhui | xuhaiwei, I am just thinking, either way can realize the goal: placemnet to handle scale out/in and placment for scale-out / deletion for scale-in | 09:07 |
| Qiming | they are still discussing that | 09:07 |
| Yanyanhu | Yes. Just feel we should make Senlin's update API behaves consistently with other services as much as possible? | 09:07 |
| elynn | Qiming: Yes, That's my headcache... | 09:07 |
| Qiming | elynn, we have to wait | 09:07 |
| lixinhui | xuhaiwei but think if one persion want to revise the zone based weight | 09:07 |
| Qiming | we have to try convince them that your patch is not disruptive | 09:07 |
| Qiming | it is the right thing to do | 09:07 |
| lixinhui | xuhaiwei, then the person will need to take care different places | 09:08 |
| Qiming | one thing we can do to straighten things from our side is to make cluster-update always return results in the same way | 09:09 |
| Yanyanhu | Qiming, I'm thinking elynn's suggestion, maybe we can seperate cluster profile update from cluster update? | 09:09 |
| Qiming | currently, we are returning very fast if the request is not about profile change | 09:09 |
| Qiming | so we may be returning results in different format | 09:10 |
| Yanyanhu | it looks like a 'version update'? | 09:10 |
| elynn | Qiming: Even after patch #261970 merged, we still can't get action id. | 09:10 |
| Yanyanhu | Qiming, that is feasible | 09:11 |
| Yanyanhu | in senlin side. But could confuse sdk? | 09:11 |
| Qiming | we should first make ourselves consistent | 09:11 |
| Yanyanhu | yes, agree | 09:12 |
| Qiming | then we try convince sdk guys to accept the header return things | 09:12 |
| elynn | yes, you are right. | 09:12 |
| Yanyanhu | yes, actually cluster create also return a dict | 09:12 |
| Yanyanhu | http://git.openstack.org/cgit/openstack/senlin/tree/senlin/engine/service.py#n544 | 09:13 |
| Qiming | cluster update is about a PATCH request | 09:13 |
| elynn | Let me see if we can find some way both senlin and openstacksdk can accept. | 09:13 |
| Yanyanhu | yea, they are different | 09:13 |
| Qiming | http://git.openstack.org/cgit/openstack/api-wg/tree/guidelines/http.rst#n105 | 09:15 |
| Qiming | for an asynchronous update request, there is no guidelines, the above rules are only about async create | 09:15 |
| Yanyanhu | yes | 09:16 |
| Qiming | anyway, we are referencing the guideline about async create when doing async update (PATCH) | 09:16 |
| Qiming | http://git.openstack.org/cgit/openstack/api-wg/tree/guidelines/http.rst#n204 | 09:16 |
| Qiming | line 204 above says that a PATCH request should have a response body | 09:17 |
| Yanyanhu | maybe returning results in different format is right way to follow | 09:17 |
| Qiming | so we are returning the body (cluster.to_dict()) | 09:17 |
| Qiming | line 105 says we should return a header, okay, we SHOULD return a 'location' header | 09:17 |
| Yanyanhu | at least before api-wg gives more guide | 09:17 |
| Qiming | so ... | 09:18 |
| Qiming | for cluster-update | 09:18 |
| Yanyanhu | so we need a judgement there | 09:18 |
| Yanyanhu | in cluster_update service interface | 09:18 |
| Qiming | we should always return a cluster in dict in body | 09:18 |
| Qiming | because it is a PATCH request | 09:18 |
| Yanyanhu | if action is created | 09:19 |
| Yanyanhu | we return action_id with it | 09:19 |
| Qiming | we are alwayus returning a 202 status_code | 09:19 |
| Qiming | that means we should always return a 'location' header | 09:19 |
| elynn | Yanyanhu: I agree, that's the first step. we include action_id in body and location point to cluster | 09:20 |
| Qiming | so | 09:20 |
| Qiming | what we should do was actually recording there: http://git.openstack.org/cgit/openstack/senlin/tree/senlin/engine/service.py#n626 | 09:20 |
| Qiming | s/recording/recorded | 09:21 |
| Yanyanhu | Qiming, yes, just currently, cluster rather than action 's location ref will be returned... | 09:21 |
| Qiming | we should move all property update into the action body, we need to lock the cluster for such an update | 09:21 |
| Yanyanhu | Qiming, right | 09:22 |
| Qiming | in that way, we will be consistent about cluster-update operation, always return a cluster.to_dict() and always return an action id in 'location' header | 09:22 |
| Yanyanhu | we should avoid this returning http://git.openstack.org/cgit/openstack/senlin/tree/senlin/engine/service.py#n588 | 09:22 |
| Qiming | exactly | 09:22 |
| elynn | I'm a little confused. | 09:23 |
| Yanyanhu | hi, elynn, I think after fixing this issue, we can resolve this problem from senlin side | 09:23 |
| elynn | action id should be in header? | 09:23 |
| Qiming | elynn, let me try restate what we need to do, step by step | 09:24 |
| Qiming | according to http://git.openstack.org/cgit/openstack/api-wg/tree/guidelines/http.rst#n204 | 09:24 |
| Qiming | we should return a cluster dict in response body, okay, we are already doing that | 09:25 |
| elynn | yes | 09:25 |
| Qiming | according to http://git.openstack.org/cgit/openstack/api-wg/tree/guidelines/http.rst#n105 | 09:25 |
| Qiming | we should return a 'location' header when doing asynchronous update, though the guideline didn't explict say that | 09:26 |
| elynn | yes | 09:26 |
| Qiming | for async operations that return a 202 as status code, we should always return a 'location' header, I believe that is the right thing to do | 09:26 |
| Qiming | the problems we have in senlin engine: | 09:27 |
| Qiming | 1. we are doing a SYNC call sometimes, e.g. http://git.openstack.org/cgit/openstack/senlin/tree/senlin/engine/service.py#n588 | 09:27 |
| Qiming | if we are not updating profile, we are returning too fast, without even creating an action | 09:28 |
| Qiming | this has been causing other problems noticed by haiwei before, so we have a comment left on line 625 | 09:28 |
| elynn | yes | 09:28 |
| Qiming | this has to be changed | 09:28 |
| Qiming | we should move all update operations into the action CLUSTER_UPDATE, instead of returning too early | 09:29 |
| Qiming | 2. we are not returning action IDs no matter what fields you want to update | 09:29 |
| Qiming | see line 631 | 09:29 |
| Qiming | this should be changed as well, we should add 'action' into the dict, then rewrite the response at API layer, move the action link to header | 09:30 |
| elynn | So you mean we should create an action for cluster update? | 09:30 |
| Qiming | ALWAYS create an action | 09:30 |
| Qiming | that is the consistency I'm talking about | 09:30 |
| elynn | no matter with or without profile | 09:31 |
| Qiming | yes | 09:31 |
| elynn | Then we return action id in location? | 09:31 |
| Qiming | exactly | 09:31 |
| elynn | Oh, I finally got what you mean T_T | 09:31 |
| Qiming | don't cry | 09:32 |
| elynn | seems we don't need patch #261970 | 09:32 |
| Qiming | why? | 09:32 |
| elynn | Let me thing through it again... | 09:33 |
| elynn | Have you notice what image resource do in openstacksdk? | 09:34 |
| Qiming | I think I know what you want | 09:34 |
| Qiming | you want to skip moving 'action' into header and back | 09:34 |
| elynn | they directly add a location property | 09:34 |
| Qiming | that works | 09:34 |
| Qiming | but that is NOT the right thing to do | 09:35 |
| Qiming | you will pay your debt eventually | 09:35 |
| elynn | ok, I stick with you. | 09:36 |
| Qiming | I believe sdk guys are reasonable people too | 09:37 |
| Qiming | we can persuade them on this | 09:37 |
| elynn | yes, I think you can :) | 09:38 |
| Qiming | we can | 09:38 |
| elynn | I can persuade them in chinese :P | 09:38 |
| Qiming | yep | 09:38 |
| elynn | I can work on update_action first if no one do it. | 09:42 |
| Qiming | okay, it is yours | 09:42 |
| Qiming | \o/ | 09:42 |
| Qiming | one suggestion for this patch | 09:43 |
| elynn | yes? | 09:43 |
| Qiming | you may want to do it in two steps | 09:43 |
| Qiming | first augmenting the existing CLUSTER_UPDATE handling logic, where you will check if any properties other than 'profile_id' is specified | 09:44 |
| Qiming | make the update works there | 09:44 |
| Qiming | then you will remove the current properties update logic from engine service | 09:45 |
| Qiming | or else I can imagine how big a patch it would be | 09:45 |
| Qiming | if you consider the test cases you will need to add | 09:45 |
| elynn | Thanks for the suggestion, I will keep in mind. | 09:46 |
| Qiming | you may even further split it into smaller patches for easier review | 09:47 |
| xuhaiwei | ok, the discussion is over? | 09:48 |
| Qiming | yep | 09:48 |
| elynn | :) | 09:48 |
| lixinhui | :) | 09:48 |
| xuhaiwei | can we go on with the topic just now? | 09:48 |
| Qiming | placement thing? | 09:49 |
| Qiming | or node distribution thing? | 09:49 |
| xuhaiwei | I have discussed with lixinhui privately just now, I found our ideas are totally different | 09:49 |
| xuhaiwei | both | 09:49 |
| xuhaiwei | in your opinion, Qiming, placement should collect the zone datas after placement is done? | 09:50 |
| xuhaiwei | and then store the data into somewhere? | 09:50 |
| xuhaiwei | then when deleting nodes, get that zone data and decide which node to delete | 09:51 |
| *** elynn has quit IRC | 09:52 | |
| Qiming | yes | 09:52 |
| xuhaiwei | store the data into cluster property? | 09:53 |
| Qiming | read the code here: | 09:53 |
| Qiming | http://git.openstack.org/cgit/openstack/senlin/tree/senlin/policies/zone_placement.py#n233 | 09:53 |
| Qiming | we create a placement plan and temporarily store the data into the current action | 09:54 |
| Qiming | then we retrieve that plan here: http://git.openstack.org/cgit/openstack/senlin/tree/senlin/engine/actions/cluster_action.py#n114 | 09:54 |
| Qiming | store the placement data into node data: http://git.openstack.org/cgit/openstack/senlin/tree/senlin/engine/actions/cluster_action.py#n127 | 09:54 |
| xuhaiwei | yes, I got this | 09:55 |
| xuhaiwei | and this is just the zone data of one node | 09:55 |
| Qiming | every node created through _create_nodes() | 09:56 |
| xuhaiwei | what we need is the zone info of all the nodes of one cluster | 09:56 |
| xuhaiwei | so we finally should collect all the info of every node? | 09:56 |
| Qiming | http://git.openstack.org/cgit/openstack/senlin/tree/senlin/policies/zone_placement.py#n143 | 09:57 |
| Qiming | well, it is not done properly I guess | 09:57 |
| Qiming | we are always checking the REAL distribution regarding AZs | 09:58 |
| Qiming | maybe we can optimize that logic | 09:58 |
| *** Yanyanhu has quit IRC | 09:58 | |
| Qiming | for cross region placement, we have this logic: http://git.openstack.org/cgit/openstack/senlin/tree/senlin/policies/region_placement.py#n160 | 09:59 |
| lixinhui | Qiming, in this way, do you mean using placement to handle zone based ranking for scale_in/resize or deletion total handling solution | 10:00 |
| Qiming | yes | 10:00 |
| Qiming | placement policy, in my opinion, should be the sole location to enforce node distribution | 10:01 |
| Qiming | only considering node creation as we do today is not a complete solution | 10:01 |
| lixinhui | yes, I think so. there is only place to handle zone | 10:01 |
| xuhaiwei | so placement policy will do part of deletion job? | 10:02 |
| Qiming | improving it using a deletion policy is doable, but we are possibly introducing a lot duplicated logics, and possibly some conflicts not easy to detect | 10:02 |
| Qiming | placement policy can be enforced when you are deleting nodes | 10:02 |
| Qiming | merely for the purpose of ensuring that a node distribution plan is respected | 10:03 |
| Qiming | need some fresh air | 10:04 |
| xuhaiwei | I can't understand this | 10:05 |
| Qiming | drank 4 grande today | 10:05 |
| Qiming | xuhaiwei, think about this | 10:05 |
| Qiming | if I want to enforce a node distribution plan | 10:05 |
| Qiming | can I use deletion policy alone? | 10:05 |
| xuhaiwei | distribution is not only placement? | 10:06 |
| Qiming | distribution means {'AZ1': 100, 'AZ2': 50}, for example | 10:06 |
| xuhaiwei | ok | 10:06 |
| xuhaiwei | we may need placement and deletion policy | 10:06 |
| Qiming | why we need two policies to achieve a simple goal? | 10:07 |
| Qiming | is it easier to do it in one policy or two? | 10:07 |
| xuhaiwei | first you need to place some nodes | 10:07 |
| xuhaiwei | you mean just create some nodes? | 10:07 |
| xuhaiwei | then just need placement policy | 10:08 |
| Qiming | that is what we have today, the placement policy is only about node creation | 10:08 |
| Qiming | yes, that is what we have today | 10:08 |
| Qiming | we should improve that | 10:08 |
| Qiming | we should make the placement policy capable of handling node deletion events | 10:08 |
| xuhaiwei | to cope with deletion policy? | 10:09 |
| Qiming | come up a plan, i.e. a list of candidates to remove, in order to maintain the node distribution | 10:09 |
| Qiming | when we are talking about node distributions | 10:09 |
| Qiming | you may and may not have a deletion policy attached to a cluster | 10:09 |
| Qiming | a placement policy itself should be capable of doing a good job maintaining node distributions | 10:10 |
| Qiming | just like the LB policy | 10:10 |
| Qiming | when you add nodes to a cluster, you attach them to a LB | 10:10 |
| Qiming | when you remove nodes, you detach them from a LB | 10:10 |
| xuhaiwei | will you add the same rules of deletion policy to placement? like OLD_FIRST | 10:11 |
| Qiming | no | 10:11 |
| Qiming | deletion policy should be able to be used by itself | 10:11 |
| xuhaiwei | but if I want to use it, how to do it | 10:11 |
| Qiming | just as placement policy | 10:11 |
| *** yuanying has quit IRC | 10:11 | |
| *** yuanying has joined #senlin | 10:11 | |
| Qiming | you can still use deletion policy | 10:11 |
| xuhaiwei | what if I want both rules? | 10:12 |
| Qiming | why we have to bring deletion policy into the same picture? | 10:12 |
| xuhaiwei | I want to delete the oldest nodes from az1 | 10:12 |
| Qiming | why ? | 10:14 |
| xuhaiwei | that is a common need i think | 10:14 |
| Qiming | do you still want to maintain node distribution rule? | 10:14 |
| xuhaiwei | first I want to delete some nodes from az1, and which one to choose? Ok, from the oldest | 10:14 |
| Qiming | seems you are thinking of a complicated algorithm | 10:15 |
| Qiming | write it out | 10:16 |
| Qiming | see if it makes sense | 10:16 |
| xuhaiwei | I think the distribution rule may conflict with the use case I said | 10:16 |
| Qiming | yes | 10:16 |
| Qiming | that would be users concern | 10:16 |
| Qiming | if you have a complicated algorithm, you can develop a new policy | 10:17 |
| Qiming | don't use the existing deletion policy | 10:17 |
| Qiming | don't use the placement policy | 10:17 |
| Qiming | when you say oldest node | 10:17 |
| Qiming | there are always questions about: oldest globally? or oldest in a az? or oldest in a region? | 10:18 |
| Qiming | sorry, guys, my eyes are blurring | 10:18 |
| xuhaiwei | ok | 10:19 |
| Qiming | have to get some fresh air | 10:19 |
| xuhaiwei | I will manage this in the blueprint | 10:19 |
| *** Qiming has quit IRC | 10:23 | |
| *** yuanying has quit IRC | 10:28 | |
| *** yuanying has joined #senlin | 10:29 | |
| *** xuhaiwei has quit IRC | 10:48 | |
| *** yuanying has quit IRC | 11:15 | |
| *** Qiming has joined #senlin | 11:31 | |
| *** elynn has joined #senlin | 12:34 | |
| *** yanyanhu has joined #senlin | 12:49 | |
| *** elynn has quit IRC | 12:50 | |
| *** elynn has joined #senlin | 12:58 | |
| *** cschulz has joined #senlin | 13:21 | |
| openstackgerrit | Merged openstack/senlin: Backlog of release notes https://review.openstack.org/263083 | 13:26 |
| *** elynn has quit IRC | 14:10 | |
| *** yanyanhu has quit IRC | 14:11 | |
| *** Qiming has quit IRC | 14:48 | |
| *** bdrich has joined #senlin | 15:38 | |
| *** bdrich has quit IRC | 15:52 | |
| *** bdrich has joined #senlin | 15:56 | |
| *** bdrich_ has joined #senlin | 19:49 | |
| *** bdrich has quit IRC | 19:51 | |
| *** pratikmallya has joined #senlin | 21:57 | |
| *** pratikmallya has quit IRC | 22:14 | |
| *** Qiming has joined #senlin | 22:51 | |
| *** yuanying has joined #senlin | 23:03 | |
| *** xuhaiwei has joined #senlin | 23:31 | |
| openstackgerrit | Merged openstack/senlin: Remove MANIFEST.in https://review.openstack.org/263244 | 23:46 |
| openstackgerrit | Merged openstack/senlin: Remove some example policies https://review.openstack.org/263640 | 23:47 |
| openstackgerrit | Merged openstack/senlin: Move some policies into WIP directory https://review.openstack.org/263641 | 23:47 |
| *** bdrich_ has quit IRC | 23:51 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!