Monday, 2015-08-17

openstackgerritxu-haiwei proposed stackforge/senlin: Handle exceptions in keystone_v3 driver
*** xuhaiwei has joined #senlin01:03
*** lixinhui_ has joined #senlin01:33
*** lixinhui_ has quit IRC01:40
*** Yanyanhu has joined #senlin01:47
*** lixinhui_ has joined #senlin01:53
openstackgerritxu-haiwei proposed stackforge/senlin: Handle exceptions in keystone_v3 driver
*** lixinhui_ has quit IRC02:03
*** elynn has joined #senlin02:11
*** mathspanda has joined #senlin02:17
*** branw has joined #senlin02:31
*** ChrisSen has joined #senlin02:45
openstackgerritYanyan Hu proposed stackforge/senlin: Use Senlin generic driver to manage ceilometer_v2 driver
*** mathspanda has quit IRC03:05
*** openstack has joined #senlin04:20
*** mathspanda has joined #senlin04:55
*** elynn_ has joined #senlin05:05
*** elynn has quit IRC05:08
*** Zhenqi has joined #senlin05:21
xuhaiweihi Yanyanhu05:59
xuhaiweiI submitted a patch, all the test are ran well in my local env, but the gate failed06:00
xuhaiweican you help check this log06:01
*** yonglihe has joined #senlin06:01
Yanyanhusure, let me have a look06:01
Yanyanhuok, saw you patch, let me make a test locally06:02
xuhaiweiI wonder if this error has something to do with the context fix patch06:03
Yanyanhuhm, it's possible06:04
Yanyanhuso weird, meet another error which is different from yours...06:07
Yanyanhualso happened in lb_policy test case06:07
Yanyanhuthe first run passed, but the second one failed06:08
xuhaiweireally strange, right06:08
xuhaiweican you show me the error message?06:08
xuhaiweiI got an error once too06:08
xuhaiweiThe error is 'AssertionError: Expected 'cred_get' to be called once.' ??06:10
xuhaiweiin test_lb_policy_build_context test case06:11
Yanyanhuxuhaiwei, ok06:12
Yanyanhusorry, I'm just answer another guys question, will be back soon06:13
Yanyanhuhaiwei, this is the error happened in my env06:14
xuhaiweiI got this one too06:14
xuhaiweioccasionally not every time06:15
Yanyanhuhmm, guess some errors happened when access DB use that context06:16
Yanyanhulet me make more investigation06:16
Yanyanhuoh, BTW, I found you have made some revision in this patch. I think my current work also depends on this. May need to some rebase after your work is done :)06:17
xuhaiweiok, probablly has relationships wich the context fix06:17
Yanyanhusome revisoin in keystone_v3 client06:17
Yanyanhuguess so06:17
xuhaiweiok, you can aslo submit your patch too, when one is merged, the other one should be rebased, you dont need to wait for my patch06:18
Yanyanhuxuhaiwei, no problem, but I think I should wait since my patch will move/remove some interfaces in kv3 client, so make it depend on yours is ok :)06:21
*** ChrisSen has quit IRC06:30
Yanyanhuhi, xuhaiwei, I found your patch didn't base on the master code06:32
YanyanhuI think you can rebase it and try again06:32
Yanyanhubut I doubt the problem could still be there06:33
Yanyanhuyes, not the latest code06:33
openstackgerritxu-haiwei proposed stackforge/senlin: Handle exceptions in keystone_v3 driver
Yanyanhuafter rebase, occasionally I met another error
xuhaiweithis error seems like a test case's problem06:39
Yanyanhusince is_admin=None, policy enforce is executed06:39
YanyanhuI guess we really need to remove context from driver layer06:39
Yanyanhuwe actually don't need it06:40
xuhaiweiyes, I really dont understand it well06:41
*** ChrisSen has joined #senlin06:42
YanyanhuI think if the statement of line192/193 in 'test_lb_policy_build_context_trust_not_found' test case can work correctly, this error should not happen06:44
xuhaiwei"this error" means which one06:44
xuhaiwei  this one?06:45
YanyanhuI mean this error
Yanyanhusince the execution should stop here
Yanyanhuand from_dict method should not be invoked06:45
xuhaiweiyes, very interesting06:46
Yanyanhulet me mock the from_dict method to make some tests06:46
xuhaiweibut this error doesn't happen in my env06:47
YanyanhuI guess the problem still happened in this line
*** Zhenqi has quit IRC06:47
Yanyanhuem, weird06:47
xuhaiweiyes, 350 is the root I think06:47
xuhaiweithe error is different in our env, just because our databases are different?06:49
Yanyanhustill not sure about the reason06:50
Yanyanhufeel both the gate and git review website are slow today06:54
xuhaiweiyes, at first, I thought it was the gate's problem06:55
xuhaiweiQiming is having an vocation?06:55
Yanyanhuyes, he is in travel this week.06:56
YanyanhuI think he will have some time to work from wednesday06:56
xuhaiweiand you wont have one too?06:57
Yanyanhuafter mock from_dict, error never happened again06:57
YanyanhuI think I won't have a vacation this summer :)06:57
Yanyanhubut from_dict method was never invoked...06:58
xuhaiweijust curious why in my env, it doesnt happen06:59
Yanyanhume too...06:59
Yanyanhulet's wait for the gate's result to see whether there is any difference07:00
Yanyanhuoh, let me make a test using master branch07:00
Yanyanhuran test 5 times in master branch and error didn't happen...07:08
YanyanhuBTW, I think the logic in line 69 might be incorrect
xuhaiweibecause we caught the InternalError here and dont raise new exception, if we dont return None, 'trust' will get no value, the workflow will go on to 72 line07:16
xuhaiweiyes, I think you are right, this line should be 'trust = None' ?07:18
xuhaiweishould not return None07:18
Yanyanhuxuhaiwei, yes, I guess this is the logic of original implementation07:19
xuhaiweigot it07:19
*** Zhenqi has joined #senlin07:25
openstackgerritYanyan Hu proposed stackforge/senlin: Rework some interfaces in keystone_v3 driver
openstackgerritxu-haiwei proposed stackforge/senlin: Handle exceptions in keystone_v3 driver
openstackgerritYanyan Hu proposed stackforge/senlin: Add a functional test of listing profile_type
xuhaiweigerrit is nearly dead08:03
Yanyanhuseems so...08:04
xuhaiweijust wanted to comment on your patch08:05
Yanyanhuthanks, no hurry I think :)08:05
xuhaiwei   the LOG is not used08:05
Yanyanhuok, let me check the code. Can't open the review web page...08:06
Yanyanhuah, right08:07
Yanyanhushould remove it08:07
xuhaiweiit's forcing me to go home :)08:07
Yanyanhusome guys have complained in openstack-infra channel...08:14
Yanyanhuhope gerrit can recover after this weekend08:15
xuhaiweithis weekend?08:18
xuhaiweifor us maybe tonight08:18
Yanyanhugit review keep failing08:20
openstackgerritYanyan Hu proposed stackforge/senlin: Add functional test for listing policy_types
Yanyanhuwaited for 5 minutes08:21
*** Zhenqi has quit IRC08:37
mathspandahi, please look at this paste:
Yanyanhuhi, mathspanda, I'm looking on it, thanks09:20
Yanyanhuhi, can you paste the error log in engine side?09:21
mathspandano error in engine screen09:22
Yanyanhuok, let me have a check09:22
xuhaiweiI am also investigating it09:24
xuhaiweiit seems like 'count' option is required though it is not assigned a value09:26
*** Tennyson has joined #senlin09:33
Yanyanhufrom the code, seems we don't have to give a count as input parameter09:35
Yanyanhukeep looking on this09:35
xuhaiweiI also saw this line09:35
Yanyanhuguess filter_args functional can't handle the case where some optional paramters are not provided09:36
*** mathspanda_ has joined #senlin09:37
Yanyanhuthis line09:38
*** Tennyson has quit IRC09:39
Yanyanhuso this line enforce us to give value for all parameters in the action function list09:39
*** mathspanda has quit IRC09:39
Yanyanhuhi, mathspanda_, I think we can fix it by changing line 106 like this: filtered_args = dict((d, params.get(d)) for d in accepted_args)09:42
Yanyanhuthen if no value is provided for some optional parameters, None will be used09:42
Yanyanhuas the value09:42
Yanyanhuminute, let me think whether this is correct09:43
mathspanda_ok. i think it is a better way.09:43
Yanyanhuum, but maybe we should use default value rather than None if it is provided in method definition09:44
Yanyanhulet me see whether it is supported09:44
Yanyanhujust a while09:44
xuhaiweiI think we just need to check if params contains the excepted_args09:52
xuhaiwei accepted_args = ([a for a in expected_args and params if a != 'self'])09:52
xuhaiweiwhat do you think09:54
Yanyanhuyou mean just remove line 107?09:54
xuhaiwei-            accepted_args = ([a for a in expected_args if a != 'self']) +            accepted_args = ([a for a in expected_args and params if a != 'self'])09:55
Yanyanhuhm, I think it's ok09:56
Yanyanhuso we just omit those accepted_args who are not provoided09:57
xuhaiweithose accepted_args should have a default value I think09:58
Yanyanhuso, mathspanda_, I think you can fix it using this way :)09:58
mathspanda_ok. thanks.09:59
Yanyanhuthis is much simpler and better than what I'm thinking09:59
Yanyanhuplease also file a bug if possible, thanks a lot :)10:00
xuhaiweiYanyanhu, it seems there is another bug here10:03
Yanyanhuyou mean in client side?10:04
xuhaiweino, after doing this command, one node should be scaled into the cluster, right?10:05
xuhaiweibut in fact, it's not10:05
Yanyanhuyou mean without providing any input for count in cmd line?10:05
xuhaiweiI mean by doing this command, server did nothing10:06
xuhaiweiby cluster-scale-in , one node should be scaled in to the cluster i think10:06
Yanyanhuhmm, I think your right10:07
Yanyanhulet me check the cluster_action code10:07
Yanyanhuyes, you are right, since the value of count field will be None10:08
xuhaiweiif count is None, what should happen?10:09
Yanyanhuthis line will not work as expected I think10:10
Yanyanhuoh, sorry, not this one10:11
Yanyanhulet me check it10:11
*** elynn_ has quit IRC10:11
mathspanda_default is 0 in engine, except attach a scale in policy10:12
Yanyanhubut if count is set to 1 if we get None from input, it will work as haiwei said10:13
mathspanda_then the dafault count of scaleout should also be 1?10:15
Yanyanhuum, I remeber this is the correct logic to use 1 as default count10:15
Yanyanhuwhyen user didn't specify it10:15
xuhaiweiYanyanhu, it should be 1 by  default10:16
xuhaiweior else, this command is meanless10:16
Yanyanhuso maybe we need a patch there :)10:16
xuhaiweiI found another bug in the server side10:16
xuhaiweido you guys want to investigate it today?10:17
xuhaiweiI am a little hungry10:17
Yanyanhume too, actually10:17
xuhaiweiI will report it and leave it tomorrow10:17
xuhaiweiif you are intereted in it, can also see it10:17
Yanyanhusure :)10:18
Yanyanhuhi, xuhaiwei, so about the patch to set default count to 1, I think we just need to change the default value from 0 to 1 in both line 511 and 55210:20
Yanyanhuand also add 'count = 0' before line 54210:20
xuhaiweiwhich file?10:20
Yanyanhuthis one10:20
xuhaiweiwhat about the client side?10:21
xuhaiweichange None to '1' too?10:21
YanyanhuI think we don't need to change clientside since I guess this is decided by the design logic of our scale in/out workflow in engine side10:22
xuhaiweiI am a little confused now10:23
YanyanhuI mean maybe we should let scale in/out create/delete at least one node even no count value is provided in action input10:23
xuhaiweiFor this line 'return self.RES_OK, 'No scaling needed based on policy checking''  it seems not scaling is the right action if count == 010:23
Yanyanhuif count is exact '0'10:24
Yanyanhue.g. scaling policy decide we can't do both creation or deletion operation10:24
Yanyanhuthat is a possible situation10:25
xuhaiweiyou mean someone will do this command ? "senlin cluster-scale-in -c 0 cluster1"??10:25
YanyanhuI think usually this kind of operation is not started from clientside manually10:25
Yanyanhuit is usually triggered automatically by other services like ceilometer10:25
xuhaiweiyes, should not :)10:25
xuhaiweigot it10:26
xuhaiweianyway, leave it tomorrow, it seems 3 or 4 patches are needed10:26
Yanyanhuwill leave, my wife is gonna be angry :p10:27
Yanyanhutalk to you guys later10:27
xuhaiweisee you, good man10:27
Yanyanhubye :)10:28
*** mathspanda_ has quit IRC10:31
*** ChrisSen has quit IRC10:33
*** Yanyanhu has quit IRC10:34
*** Qiming has joined #senlin10:46
*** Qiming has quit IRC10:59
*** Qiming has joined #senlin12:50
*** Qiming has quit IRC12:51
*** lkarm has joined #senlin13:02
*** jroyal has joined #senlin13:11
*** jdandrea has joined #senlin13:36
*** lkarm has quit IRC15:49
*** lkarm has joined #senlin15:57
*** mathspanda_ has joined #senlin16:05
*** mathspanda_ has quit IRC16:06
*** jroyal has quit IRC16:40
*** jroyal has joined #senlin17:40
*** jroyal has quit IRC17:44
*** jroyal has joined #senlin17:57
*** jroyal has quit IRC17:58
*** jroyal_ has joined #senlin18:00
*** jroyal__ has joined #senlin18:01
*** jroyal_ has quit IRC18:04
*** lkarm has quit IRC18:12
*** lkarm has joined #senlin18:41
*** jdandrea has quit IRC18:50
*** lkarm has quit IRC18:57
*** lkarm has joined #senlin18:58
*** lkarm has quit IRC19:02
*** lkarm has joined #senlin19:05
*** jdandrea has joined #senlin19:21
*** jdandrea has quit IRC19:21
*** jdandrea has joined #senlin19:50
*** lkarm has quit IRC19:54
*** jroyal has joined #senlin19:58
*** lkarm has joined #senlin19:58
*** jroyal__ has quit IRC19:58
*** jroyal has quit IRC21:31
*** lkarm has quit IRC21:31
*** jroyal has joined #senlin21:33
*** jroyal_ has joined #senlin21:34
*** jroyal has quit IRC21:37
*** jroyal_ has quit IRC21:38
*** jroyal has joined #senlin21:56
*** jroyal has quit IRC22:00

Generated by 2.14.0 by Marius Gedminas - find it at!