Tuesday, 2015-08-25

openstackgerritxu-haiwei proposed stackforge/senlin: Handle exceptions in keystone_v3 driver  https://review.openstack.org/21356900:32
*** zhenguo has joined #senlin00:43
*** Qiming has joined #senlin01:04
openstackgerritMerged stackforge/senlin: Use wait_for_delete to wait for nova server deletion  https://review.openstack.org/21444801:10
*** mathspanda has joined #senlin01:12
*** Yanyanhu has joined #senlin01:16
xuhaiweiQiming, morning01:17
xuhaiweihave a question about webhook01:18
openstackgerritMerged stackforge/senlin: Check cluster size constraint before doing node join/leave  https://review.openstack.org/21547501:18
xuhaiweicurrently the webhook url is not stored in the database, is that ok?01:18
xuhaiweifrom webhook-show , we can't refer to the url01:19
Qimingem, that was by design01:20
Qimingit was meant to be shown just once, for security's reason01:20
*** Yanyanhu has quit IRC01:21
Qimingthe user is supposed to save it safely, especially the 'key' part01:21
Qimingmaybe it is a bad design01:21
*** Yanyanhu has joined #senlin01:21
Qimingwe can revisit it if it turns out to be very inconvenient01:21
xuhaiweiand another question, from webhook-show we can't see the webhook's status either01:22
xuhaiweieven webhook is not correctly created, it is still in the webhook list01:23
Qimingwebhook doesn't have a status field?01:23
Qimingwe are not maintaining status for webhook01:23
Qimingwe even don't have an update API for webhook01:24
xuhaiweiI mean use may don't know whether webhook is correctly created01:24
Yanyanhuif error happened during webhook creation, the webhook will not be stored in DB and you can't see it in webhook list01:24
Yanyanhusince webhook's creation is not done in action progress :)01:25
Qimingit is like a profile01:25
Yanyanhujust like profile or policy01:25
xuhaiweino, Yanyanhu, it's in the list01:25
Qimingif it is shown there, it must have been created01:25
Yanyanhulet me have a look01:25
Qimingokay, sounds like a defect01:25
Yanyanhuhmm, if so, it is01:25
xuhaiweiI mean if the webhook url is not created, the webhook is still in the list, without a 'ERROR' status01:26
xuhaiweibecause it's not an action, we can't check it in action-list01:26
openstackgerritMerged stackforge/senlin: Handle exceptions in keystone_v3 driver  https://review.openstack.org/21356901:27
Yanyanhusince the webhook_url generatoin is after webhook.store01:27
xuhaiweiurl is not in database01:27
Yanyanhuhmm, I think maybe we should add a return value check or exceptoin handling in line 129301:27
Yanyanhuto help decide whether webhook_url is generated correctly. If not, we remove the webhook just created from DB01:28
Yanyanhuand return error msg01:28
xuhaiweibecause it is already stored in db in line 1292, add exception handing in line 1293 does not help I think01:29
Qimingcan we postpone the store() call?01:30
Yanyanhuyou can remove it from DB if exception happened in 129301:30
xuhaiweicheck url first and then store in db?01:30
YanyanhuQiming, I remeber there is a dependency here, so the stored is put before webhook.generate_url01:31
xuhaiweithough url is not stored01:31
Yanyanhustore operation01:31
Qimingmy understanding is that you need the UUID of the webhook01:31
Yanyanhuoh, right01:31
Qimingbut you can generate manually before handling it to DB01:31
Yanyanhuself.id is needed01:31
Yanyanhuumm, so we can generate the id at service module and use it to initialize the webhook obj01:32
Yanyanhuif so, we may need to change the logic of Webhook.store method01:33
Yanyanhusince current implementation doesn't include id field when invoke db_api.webhook_create01:34
Yanyanhuthe id will be generated in DB side01:35
YanyanhuI think put store after generate_url is the correct way01:35
Qiminganyway, we don't have webhook update API right?01:35
Yanyanhua easy change I think01:36
xuhaiweibecause there is no status field of webhook, should return an exception to user when the creation failed01:37
YanyanhuI think an error msg is ok01:38
Yanyanhujust put the error info rather than webhook dictionary into the return value01:38
xuhaiweiYanyanhu, I am afraid that doesn't work well01:39
xuhaiweireturn False instead of result?01:40
Yanyanhuho, let me see01:41
Yanyanhuso the resource create API should always return a dictionary in the following format? {'webhook': {...}}01:43
Yanyanhufor cluster, should be {'cluster': {...}}01:43
Yanyanhuif no exception happened during creation?01:43
Yanyanhucluster is not a good example here01:43
Yanyanhusimilar resp for policy, profile01:44
xuhaiweiseem so01:44
Yanyanhuif so, a error msg string in return value is not a correct response based on restful standard?01:45
xuhaiweiyou mean we store the error msg like result['msg'], and return it back?01:46
xuhaiweithat is not as good as an exception I think01:46
Qimingif we unfortunately fails a webhook creation, we will need to raise an exception just like BadRequest01:48
Qimingbut it may have to be an 500 internal server error01:48
Qimingit would be our fault to fail that creation, right?01:49
YanyanhuQiming, I think for exception happened in generate_url, that's true01:49
xuhaiweiyes, i think01:49
QimingYanyanhu, if URL is not correctly generated, the webhook will be useless01:49
Yanyanhuwe already have some exceptoin handling for those cases like target object not found01:49
xuhaiweibut, sometime senlin service is not started will also cause this error01:50
Qimingtarget object not found is a client side error01:50
Qiminguser is naming a non-existent entity for webhook01:50
Yanyanhuhmm, you're right01:50
Qimingbut if generate_url fails, it is server's fault, don't blame the requesters01:50
Yanyanhucurrently, we also treat those exceptions as badrequest01:51
Qimingso, fix it, man!01:51
xuhaiweiok, i will take it01:51
Yanyanhugreat :)01:51
*** jdandrea has quit IRC02:27
*** lkarm has joined #senlin02:50
*** lkarm has quit IRC02:54
openstackgerritxu-haiwei proposed stackforge/senlin: Fail webhook creation if url is not correctly created  https://review.openstack.org/21650803:12
*** jruano has quit IRC03:13
*** xuhaiwei has quit IRC04:06
*** xuhaiwei has joined #senlin05:07
openstackgerritYanyan Hu proposed stackforge/senlin: Remove context usage from driver layer  https://review.openstack.org/21652905:12
openstackgerritxu-haiwei proposed stackforge/python-senlinclient: Delete deleted_time and url option from webhook-show  https://review.openstack.org/21653505:42
*** zhenguo has quit IRC05:53
*** zhenguo has joined #senlin06:09
openstackgerritZhenguo Niu proposed stackforge/senlin-dashboard: Bump pbr version to match global requirement  https://review.openstack.org/21654406:16
*** xuhaiwei has quit IRC06:29
*** xuhaiwei has joined #senlin06:29
openstackgerritxu-haiwei proposed stackforge/python-senlinclient: Add deleted_time to webhook_list  https://review.openstack.org/21655306:32
openstackgerritQiming Teng proposed stackforge/senlin: API layer support for triggers  https://review.openstack.org/21655606:49
openstackgerritQiming Teng proposed stackforge/senlin: API layer support for triggers  https://review.openstack.org/21655606:57
openstackgerritQiming Teng proposed stackforge/senlin: API layer support for triggers  https://review.openstack.org/21655606:58
openstackgerritMerged stackforge/senlin-dashboard: Bump pbr version to match global requirement  https://review.openstack.org/21654406:59
*** Qiming has quit IRC07:01
*** Qiming has joined #senlin07:06
openstackgerritOpenStack Proposal Bot proposed stackforge/senlin: Updated from global requirements  https://review.openstack.org/21657407:12
*** lkarm has joined #senlin07:22
*** lkarm has quit IRC07:27
*** mathspanda has quit IRC07:49
openstackgerritYanyan Hu proposed stackforge/senlin: Remove context usage from driver layer  https://review.openstack.org/21652908:05
Qimingthis change was a complete mess08:05
Qimingsigh ...08:05
Qimingthe adding of service credentials in engine.service is like a virus08:07
Qimingit is polluting the db, and the webhook flow08:07
Yanyanhuyou mean this operation should be done in webhook module?08:07
Yanyanhuif so, maybe we should add something like get_default_credential in webhook.py08:09
Qimingadding this line is producing a lot of garbage in credential08:09
Qimingwe cannot differentiate what was provided by users, what was defaults from the engine08:10
Qimingthe worst thing is that it copied 'project_name' from service_credetials into the dict08:10
Qimingand project_domain_name08:10
Yanyanhubut I think this line will work only when user doesn't provide their own credential?08:10
Qimingit won't08:11
Qimingit won't work under all situation08:11
Qimingbecause the project_name is set to 'service' by this line08:11
Yanyanhuyou mean?08:11
Qimingand we have 'trusts' in the same dict08:12
Yanyanhuah, I see. So actually we should do something like patchset 308:13
Qimingline 1268 is wrong too08:13
Qimingmaybe I'm not understanding it correctly08:14
Yanyanhuhmm, actually I found set trusts to a string of trust_id also worked, this is weird08:14
Yanyanhujust as you said we change it to list in a previous patch08:15
Qimingat least it should be cred.cred['openstack']['trust']08:15
Yanyanhuumm, that's true. But it did passed the pratical test...08:16
Yanyanhuhmm, actually I ran the pratical test using patchset 3...08:17
openstackgerritMerged stackforge/senlin: Updated from global requirements  https://review.openstack.org/21657408:27
Yanyanhuhi, Qiming, so about the error in webhook_create, should we implement the credential generation logic in webhook module and refer it in service module?08:41
Qimingtrying to fix it now08:44
Qimingthe basic idea is save just what user provided in an encrypted form08:44
Yanyanhuthe senlin service related part will be added in trigger progress?08:45
Qimingconstruct a dict for token retrieval in webhook middleware when it is triggered08:45
Yanyanhumuch better08:45
Qimingoverride the dict content with whatever decrypted from the user provided "credential"08:45
Yanyanhuok, understand08:46
Qimingthere is a chicken-egg problem though08:47
Yanyanhuah, I suddenly understand...08:48
YanyanhuI think I misunderstood your comment in patchset3... https://review.openstack.org/#/c/214038/3/senlin/engine/service.py08:48
Yanyanhuin line126508:48
Yanyanhuabout this 'treat the 'credential' field as a customization to the "default connection params"'08:49
Qimingokay, I meant a dict.update() call08:50
openstackgerritYanyan Hu proposed stackforge/senlin: Remove context usage from driver layer  https://review.openstack.org/21652908:54
xuhaiweiwhat shall we do if the user 'credential' contains something useless?08:57
*** Yanyanhu_ has joined #senlin08:58
xuhaiweijust store it in the db?08:59
*** Yanyanhu has quit IRC09:00
xuhaiweisince user 'credential' is not essential, don't know why we need to check whether 'credential' is provided09:04
*** Yanyanhu_ has quit IRC09:04
lawrancejingHI  Qiming, i'm pleasure to contribute to senlin-dashboard, could i join the senlin-dashboard team?09:04
Qimingthank you so much for helping us out there09:04
*** Yanyanhu_ has joined #senlin09:05
Qimingxuhaiwei, credential is not mandatory09:05
xuhaiweiyes, so I really dont understand why we check it is provided, http://git.openstack.org/cgit/stackforge/senlin/tree/senlin/engine/service.py#n126309:06
xuhaiweisince if user provide something useless to 'credential' , this check will be skipped09:07
Qimingif it is not provided, we will use the "default credential"09:07
lawrancejingOK  cheers09:07
Qimingby "credential", it means who will be triggering the webhook in future09:07
Qiminglawrancejing, feel free to dance and sing there, ;)09:08
lawrancejing:😃 sure09:08
lawrancejingand  thanks09:08
zhenguowelcome lawrancejing09:11
xuhaiweiQiming, by the 'default credential', it is the 'trust_id', right? but if user provides their 'credential', what should it be? user_name or project_name?? it seems they are already in the 'kwargs'09:13
Qimingthey can provided their own username and/or password and/or whatever they would like09:14
Qimingby kwargs, you mean the 1280 line?09:14
Qimingthose are the object owner/creator09:15
Qimingit only tells you who created this webhook, to which project the webhook resides in09:16
xuhaiweithat makes sense to me09:16
Qimingif "John" created a webhook, and the webhook can be triggered only by "John", that will be a silly design, right?09:17
Yanyanhu_hi, lawrancejing, welcome to senlin, just missed your message for network disconnection :)09:19
Qimingxuhaiwei, so the design is like this09:19
QimingAlice can create a webhook for objects she owns09:20
lawrancejingYanyanhu_  thanks09:20
QimingAlice can create webhooks for whatever objects, if she is admin09:20
xuhaiweiQiming, got it, I think09:20
xuhaiweilawrancejing, welcome to senlin09:21
QimingWhen webhook is triggered, we will find out "who are you", "are you admin", "do you own the object" ...09:21
lawrancejingthanks  all guys:)09:21
xuhaiweithe webhooks are usually not triggered from the command line, right?09:22
Qimingmaybe the common use case would be an admin creates them and shares them to cluster owners09:25
xuhaiweiwill leave now, see you tonight09:31
Qimingsee you09:31
Yanyanhu_see U09:31
*** lawrance_ has joined #senlin09:53
*** lawrance_ has quit IRC09:56
*** lawrance_ has joined #senlin09:56
*** lawrancejing has left #senlin10:01
*** lawrance_ has quit IRC10:03
*** lawrance_ has joined #senlin10:03
*** lawrance_ has left #senlin10:03
*** lawrance_ has joined #senlin10:07
*** lawrance_ has quit IRC10:22
*** jruano has joined #senlin10:44
*** Yanyanhu_ has quit IRC10:47
*** Qiming has quit IRC10:56
openstackgerritMerged stackforge/senlin: Add fake nova_v2 driver for functional test  https://review.openstack.org/21409011:37
*** zhenguo has quit IRC11:55
*** Qiming has joined #senlin11:57
*** Qiming has quit IRC12:06
*** Qiming has joined #senlin12:06
*** lkarm has joined #senlin12:08
jruanohi qiming... do you have time now to meet with matt and i re: tosca policies?12:10
Qiminghi, yes12:12
Qimingjust back home12:12
jruanono worries. we're on the callin12:13
*** Qiming has quit IRC12:17
*** Qiming has joined #senlin12:17
*** haiwei has joined #senlin12:31
*** haiwei has quit IRC12:42
*** yanyanhu has joined #senlin12:55
Qimingprevious meeting overrun again, sorry13:00
yanyanhuit ended13:01
*** zhenguo has joined #senlin13:16
Qimingadmin__, online?13:21
yanyanhuoh, admin__ is xinhui?13:22
admin__Yes, Yanyanhu13:22
*** haiwei_ has joined #senlin13:49
*** yanyanhu has quit IRC13:49
Qimingseems yanyanhu dropped, haiwei13:50
haiwei_will say to him tomorrow13:51
Qimingtoo late for you13:51
haiwei_yap, but you often stay late13:51
QimingI'm growing old, haha13:52
haiwei_the trip you had last week seems great13:53
Qimingyes, it was cool13:54
Qimingit feels good when people understand what you are talking about, :)13:54
haiwei_you mean the local people?13:55
haiwei_ok, will go to bed, see you13:56
*** jruano has quit IRC13:57
Qimingsee you13:58
openstackgerritMerged stackforge/python-senlinclient: Add deleted_time to webhook_list  https://review.openstack.org/21655313:58
*** haiwei_ has quit IRC14:00
openstackgerritMerged stackforge/senlin: Remove context usage from driver layer  https://review.openstack.org/21652914:11
*** lawrancejing has joined #senlin14:42
*** lawrance_ has joined #senlin14:51
*** lawrancejing has left #senlin14:53
openstackgerritQiming Teng proposed stackforge/senlin: Fix webhook triggering logic  https://review.openstack.org/21675115:02
*** jruano has joined #senlin15:11
openstackgerritQiming Teng proposed stackforge/senlin: API layer support for triggers  https://review.openstack.org/21655615:17
openstackgerritZhenguo Niu proposed stackforge/senlin-dashboard: Add cluster dashboard  https://review.openstack.org/21676215:19
Qimingheading to bed15:21
*** jdandrea has joined #senlin15:23
*** Qiming has quit IRC15:26
*** zhenguo has quit IRC15:36
*** lawrance_ has quit IRC15:39
*** jruano has quit IRC19:02
*** lkarm has quit IRC21:04

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!