| *** guoshan has joined #senlin | 00:22 | |
| *** catintheroof has joined #senlin | 00:41 | |
| *** zhenguo has joined #senlin | 00:52 | |
| *** guoshan has quit IRC | 01:05 | |
| *** guoshan has joined #senlin | 01:06 | |
| *** guoshan has quit IRC | 01:10 | |
| *** XueFeng has joined #senlin | 01:24 | |
| *** dixiaoli has joined #senlin | 01:25 | |
| *** catintheroof has quit IRC | 01:30 | |
| *** catintheroof has joined #senlin | 01:30 | |
| *** catintheroof has quit IRC | 01:31 | |
| yuanbin | Qiming, https://review.openstack.org/#/c/455211/ this patch , I don't unstand mean, when we remove 'is_profile_only = resource.Body' POST PATCH will miss 'profile-only' | 01:33 |
|---|---|---|
| Qiming | well, I dont understand your question, ;) | 01:34 |
| *** yanyanhu has joined #senlin | 01:34 | |
| *** guoshan has joined #senlin | 01:35 | |
| Qiming | profile_only is made part of the resource body | 01:36 |
| yuanbin | Qiming, I mean, when i use profile-only=True, the cluster profile will be change, but the cluster has exist node don't apply new profile | 01:36 |
| Qiming | yes, that was the goal of this parameter | 01:37 |
| yuanbin | Qiming, but i use profile-only=True, -d '{"cluster": {"name": null, "profile_id": "profile_base", "id": "903d7af7-1552-481d-ab44-145d9d66d801", "timeout": null, "metadata": {}}}' but the para not find profile-only param | 01:37 |
| *** guoshan has quit IRC | 01:37 | |
| *** guoshan_ has joined #senlin | 01:37 | |
| Qiming | when update the cluster, we only update its profile, we don't update nodes | 01:37 |
| yuanbin | Qiming, so, the change will be faild | 01:37 |
| yuanbin | Qiming, yes | 01:38 |
| Qiming | what was your command for cluster update? | 01:38 |
| yuanbin | Qiming, senlin --debug cluster-update --profile profile_base --profile-only=True cluster_001 | 01:39 |
| Qiming | looking | 01:41 |
| openstackgerrit | Qiming Teng proposed openstack/senlin master: Fix setup-service script error https://review.openstack.org/450006 | 01:41 |
| Qiming | what is your version of openstacksdk? | 01:41 |
| Qiming | have you applied that patch? | 01:41 |
| Qiming | or you dont have the is_profile_only = resource.Body('profile_only') line? | 01:42 |
| yuanbin | Qiming, openstacksdk 0.9.13 | 01:43 |
| Qiming | you need the lastest version and you need the patch | 01:43 |
| yuanbin | I have applied patch ,_query_mapping = resource.QueryParameters( | 01:43 |
| yuanbin | 'name', 'status', 'sort', 'global_project', 'profile_only', | 01:43 |
| Qiming | that is wrong | 01:44 |
| Qiming | please check https://review.openstack.org/#/c/455211/ | 01:44 |
| Qiming | the latest patchset | 01:44 |
| yuanbin | Qiming, I use is_profile_only = resource.Body('profile_only') , will be ok | 01:44 |
| Qiming | yes | 01:45 |
| Qiming | that is the correct solution | 01:45 |
| Qiming | query_mapping is meant for cluster_list | 01:45 |
| Qiming | when you list clusters, what are the query parameters you can pass | 01:45 |
| yuanbin | Qiming, https://review.openstack.org/#/c/455211/ this patch fix be me | 01:45 |
| yuanbin | s/be/by me | 01:45 |
| Qiming | 'profile_only' is certainly not part of cluster listing query | 01:46 |
| Qiming | you should not have added 'profile_only' to the _query_mapping field | 01:47 |
| yuanbin | Qiming, ok | 01:47 |
| Qiming | and you should have changed profile_only to is_profile_only when adding it as a cluster 'property' | 01:47 |
| Qiming | in other words, patchset 5 looks good now | 01:47 |
| yuanbin | Qiming, ok, thanks | 01:48 |
| Qiming | yuanbin, when you have time, please help review this: https://review.openstack.org/450006 | 01:49 |
| Qiming | I have reworked your patch | 01:49 |
| yuanbin | Qiming, ok | 01:49 |
| yuanbin | Qiming, https://review.openstack.org/450006 Thank you for let me learn such usage , I think is very good! | 02:06 |
| openstackgerrit | yangyide proposed openstack/senlin master: Improve check_object for health_policy_poll recover https://review.openstack.org/456187 | 02:12 |
| *** zhurong has joined #senlin | 02:32 | |
| *** zhurong has quit IRC | 04:02 | |
| *** guoshan_ has quit IRC | 04:07 | |
| *** dixiaoli has quit IRC | 04:21 | |
| *** zhurong has joined #senlin | 04:30 | |
| *** dixiaoli has joined #senlin | 05:23 | |
| *** zhurong has quit IRC | 06:20 | |
| *** dixiaoli has quit IRC | 06:27 | |
| *** zhurong has joined #senlin | 06:32 | |
| yuanbin | yanyanhu, I have a problem in this https://bugs.launchpad.net/senlin/+bug/1682698 ? | 06:41 |
| openstack | Launchpad bug 1682698 in senlin "senlin validate_for_update faild" [Undecided,New] - Assigned to chenyb4 (chenyb4) | 06:41 |
| yuanbin | yanyanhu, In this check, well be allways return False | 06:42 |
| yanyanhu | hi, yuanbin, let me check it | 06:42 |
| yanyanhu | hi, yuanbin, just as you said in bug report ""BLOCK_DEVICE_MAPPING_V2", "SECURITY_GROUPS" and other is don't configure "updatable=True" parameter, so validate_for_update will always return false" | 06:46 |
| yuanbin | yanyanhu, yes | 06:47 |
| yanyanhu | so any issue with this logic? | 06:47 |
| yuanbin | yanyanhu, when update image, this validate_for_update will be faild, so, the next well be don't execute | 06:49 |
| yanyanhu | return false mean users are trying to update some properties that are not updatable | 06:49 |
| yanyanhu | if the return value is true, that means all properties changed in new profile are updatable and thus the validation passed | 06:50 |
| yanyanhu | umm, updating image shouldn't fail I guess. | 06:51 |
| yanyanhu | let me check nova server profile | 06:51 |
| yanyanhu | http://git.openstack.org/cgit/openstack/senlin/tree/senlin/profiles/os/nova/server.py#n158 | 06:52 |
| yuanbin | yanyanhu, if not self.validate_for_update(new_profile): return False, validate_for_update has return False, so the next don't execute | 06:52 |
| yanyanhu | yuanbin, yes, validate_for_update function returning false means you're trying to update an "non-updatable" property | 06:53 |
| yuanbin | yuanbin, I think logic is validate(newprofile, updatexxx) if newprofile allown updatable return True | 06:53 |
| yanyanhu | so the update operation will finally fail | 06:53 |
| yanyanhu | I guess it is designed like this | 06:54 |
| yanyanhu | yuanbin, yes, what you said is right | 06:54 |
| yanyanhu | and it doesn't conflict to what I said : ) | 06:54 |
| yanyanhu | e.g. you are trying to update image which is an updatable property, then validate_for_update function will return true | 06:55 |
| yanyanhu | and then the following updating operation will get chance to be executed | 06:55 |
| yanyanhu | however, if you try to update the property, lets say, "BLOCK_DEVICE_MAPPING_V2" which is not updatable | 06:56 |
| yuanbin | yanyanhu, but i want to update BLOCK_DEVICE_MAPPING_V2, and i add BLOCK_DEVICE_MAPPING_V2 updateable=True, the error appear "SECURITY_GROUPS" don't update ? but i don't update "SECURITY_GROUPS" | 06:56 |
| yanyanhu | then validate_for_update will return false and then block the following update operation | 06:57 |
| yanyanhu | umm, that is weird | 06:57 |
| yuanbin | yanyanhu, yes, this logic is not probrom | 06:57 |
| yanyanhu | so you didn't change the value of "SECURITY_GROUPS" property? | 06:57 |
| yanyanhu | if so, sounds like a bug | 06:58 |
| yanyanhu | you can see that updatable check will be done only when property is changed | 06:59 |
| yanyanhu | http://git.openstack.org/cgit/openstack/senlin/tree/senlin/profiles/base.py#n485 | 06:59 |
| yuanbin | yanyanhu, yes, please let me try again | 06:59 |
| yanyanhu | maybe you can add some log here to see what happen? | 06:59 |
| yuanbin | yanyanhu, log display "SECURITY_GROUPS" is not updateable | 07:00 |
| Qiming | yanyanhu, maybe be this line is doing things wrong? if self.properties.get(k, None) != v: | 07:01 |
| Qiming | suppose the new_profile doesn't contain security group property | 07:02 |
| Qiming | but the current existing has security group specified | 07:02 |
| yanyanhu | if so http://git.openstack.org/cgit/openstack/senlin/tree/senlin/profiles/base.py#n484 | 07:03 |
| yanyanhu | this check will be false | 07:03 |
| yanyanhu | and line485 won't get executed I think | 07:03 |
| Qiming | line 484 will return ('security_group', None) | 07:03 |
| yanyanhu | I mean security_group key,value is not in new_profile.properties.items() | 07:03 |
| Qiming | sometimes, an unspecified property still appears in the dict | 07:04 |
| Qiming | but its value is None | 07:04 |
| yanyanhu | so line 485 will be skipped for security_group key | 07:04 |
| yanyanhu | hi, Qiming, what you mean by "ometimes, an unspecified property still appears in the dict" | 07:05 |
| Qiming | http://git.openstack.org/cgit/openstack/senlin/tree/senlin/profiles/os/nova/server.py#n590 | 07:05 |
| Qiming | see this line | 07:05 |
| yanyanhu | you mean the return value of "new_profile.properties.items()" is incorrect? | 07:05 |
| Qiming | it means self.SECURITY_GROUP will be in self.properties not matter it is provided or not | 07:06 |
| yanyanhu | Qiming, yes, it is possible | 07:06 |
| Qiming | but that line is not throwing invalid key exception | 07:06 |
| yanyanhu | can't clearly recall how the schema is built | 07:06 |
| Qiming | we had to check whether security group is specified on line 591 | 07:07 |
| Qiming | similarly, this line will return ('security_groups', None): http://git.openstack.org/cgit/openstack/senlin/tree/senlin/profiles/base.py#n484 | 07:07 |
| Qiming | even if the new profile didn't intend to change it | 07:08 |
| yanyanhu | if so, that means new profile is different from old one | 07:08 |
| yanyanhu | I think | 07:08 |
| yanyanhu | since we have security_group specified in old profile, however, this property is removed in new one | 07:08 |
| yanyanhu | so this is a change need to be validated? | 07:09 |
| yanyanhu | and validation failure is expected actually I feel? | 07:09 |
| Qiming | we were using PATCH word for update | 07:09 |
| Qiming | which means the request can contain only the fields it wants to update | 07:09 |
| yanyanhu | ok, I see | 07:10 |
| yanyanhu | if so, how can user to remove the sec_group by profile updating? | 07:10 |
| Qiming | if users want to remove security groups, they should do 'security_groups: []' | 07:10 |
| yanyanhu | hmm, that makes sense | 07:10 |
| Qiming | so a quick fix could be just test if v is None on line 485 | 07:11 |
| yanyanhu | if so, we may need to add some description for this logic | 07:11 |
| yanyanhu | Qiming, yes, that will work | 07:11 |
| Qiming | and only do the comparison if there is a new value specified | 07:11 |
| Qiming | this is a good example how 'v is None' and 'not v' differs | 07:12 |
| yanyanhu | ok, that makes sense. | 07:12 |
| yanyanhu | But I suggest to add some description here to avoid misunderstanding for the usage of "profile update" interface. | 07:15 |
| *** ruijie has quit IRC | 07:15 | |
| yanyanhu | since it works in patch way rather than put | 07:15 |
| *** ruijie has joined #senlin | 07:16 | |
| yanyanhu | and we need to perform similar check when handling all other properties like key_name, network, etc. | 07:19 |
| yanyanhu | since we need to treat new_profile as an increment of old/existing profile rather than a new complete profile | 07:23 |
| *** dixiaoli has joined #senlin | 07:24 | |
| yuanbin | yanyanhu, I try again, the probrom is not appear, I think i configure BLOCK_DEVICE_MAPPING_V2 updateable=true configure error | 07:41 |
| yanyanhu | yuanbin, good to hear that : ) | 07:41 |
| Qiming | yes, agreed, yanyanhu | 07:47 |
| openstackgerrit | XueFeng Liu proposed openstack/senlin master: Supports nova server non-admin integration test https://review.openstack.org/455331 | 08:04 |
| openstackgerrit | Merged openstack/senlin master: Fix setup-service script error https://review.openstack.org/450006 | 08:04 |
| openstackgerrit | Merged openstack/senlin master: Improve check_object for health_policy_poll recover https://review.openstack.org/456187 | 08:04 |
| openstackgerrit | Merged openstack/senlin master: Use 'nosetests -v' replace './run_tempest.sh -N' https://review.openstack.org/456677 | 08:04 |
| openstackgerrit | Merged openstack/senlin master: add health_check() to engine.cluster https://review.openstack.org/456432 | 08:04 |
| openstackgerrit | XueFeng Liu proposed openstack/senlin master: Supports nova server non-admin integration test https://review.openstack.org/455331 | 08:14 |
| *** yuanying has joined #senlin | 08:19 | |
| *** yuanying_ has quit IRC | 08:21 | |
| *** zhurong has quit IRC | 10:03 | |
| *** dixiaoli has quit IRC | 10:04 | |
| *** XueFeng has quit IRC | 10:13 | |
| *** yanyanhu has quit IRC | 10:51 | |
| openstackgerrit | Merged openstack/senlin master: Fix ovo object for requests https://review.openstack.org/455891 | 11:35 |
| *** Guest52586 has quit IRC | 12:50 | |
| *** zigo has joined #senlin | 12:58 | |
| *** ruijie_ has joined #senlin | 14:25 | |
| *** ruijie has quit IRC | 14:28 | |
| *** Qiming has quit IRC | 15:22 | |
| *** Qiming has joined #senlin | 15:30 | |
| *** catintheroof has joined #senlin | 16:45 | |
| *** catintheroof has quit IRC | 16:46 | |
| *** catintheroof has joined #senlin | 16:47 | |
| *** catintheroof has quit IRC | 20:02 | |
| *** catintheroof has joined #senlin | 23:16 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!