openstackgerrit | xu-haiwei proposed stackforge/senlin: Handle exceptions in keystone_v3 driver https://review.openstack.org/213569 | 01:02 |
---|---|---|
*** xuhaiwei has joined #senlin | 01:03 | |
*** lixinhui_ has joined #senlin | 01:33 | |
*** lixinhui_ has quit IRC | 01:40 | |
*** Yanyanhu has joined #senlin | 01:47 | |
*** lixinhui_ has joined #senlin | 01:53 | |
openstackgerrit | xu-haiwei proposed stackforge/senlin: Handle exceptions in keystone_v3 driver https://review.openstack.org/213569 | 01:56 |
*** lixinhui_ has quit IRC | 02:03 | |
*** elynn has joined #senlin | 02:11 | |
*** mathspanda has joined #senlin | 02:17 | |
*** branw has joined #senlin | 02:31 | |
*** ChrisSen has joined #senlin | 02:45 | |
openstackgerrit | Yanyan Hu proposed stackforge/senlin: Use Senlin generic driver to manage ceilometer_v2 driver https://review.openstack.org/213593 | 02:55 |
*** mathspanda has quit IRC | 03:05 | |
*** openstack has joined #senlin | 04:20 | |
*** mathspanda has joined #senlin | 04:55 | |
*** elynn_ has joined #senlin | 05:05 | |
*** elynn has quit IRC | 05:08 | |
*** Zhenqi has joined #senlin | 05:21 | |
xuhaiwei | Yanyanyu | 05:58 |
xuhaiwei | hi Yanyanhu | 05:59 |
Yanyanhu | hello | 06:00 |
xuhaiwei | I submitted a patch, all the test are ran well in my local env, but the gate failed | 06:00 |
xuhaiwei | http://logs.openstack.org/69/213569/2/check/gate-senlin-python27/636fc10/testr_results.html.gz | 06:00 |
xuhaiwei | can you help check this log | 06:01 |
*** yonglihe has joined #senlin | 06:01 | |
Yanyanhu | sure, let me have a look | 06:01 |
Yanyanhu | ok, saw you patch, let me make a test locally | 06:02 |
xuhaiwei | I wonder if this error has something to do with the context fix patch | 06:03 |
Yanyanhu | hm, it's possible | 06:04 |
Yanyanhu | so weird, meet another error which is different from yours... | 06:07 |
Yanyanhu | also happened in lb_policy test case | 06:07 |
Yanyanhu | the first run passed, but the second one failed | 06:08 |
xuhaiwei | really strange, right | 06:08 |
xuhaiwei | can you show me the error message? | 06:08 |
xuhaiwei | I got an error once too | 06:08 |
xuhaiwei | The error is 'AssertionError: Expected 'cred_get' to be called once.' ?? | 06:10 |
xuhaiwei | in test_lb_policy_build_context test case | 06:11 |
Yanyanhu | xuhaiwei, ok | 06:12 |
Yanyanhu | sorry, I'm just answer another guys question, will be back soon | 06:13 |
xuhaiwei | ok | 06:13 |
Yanyanhu | haiwei, this is the error happened in my env | 06:14 |
Yanyanhu | http://paste.openstack.org/show/418797/ | 06:14 |
xuhaiwei | I got this one too | 06:14 |
xuhaiwei | occasionally not every time | 06:15 |
Yanyanhu | hmm, guess some errors happened when access DB use that context | 06:16 |
Yanyanhu | let me make more investigation | 06:16 |
Yanyanhu | oh, 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 |
xuhaiwei | ok, probablly has relationships wich the context fix | 06:17 |
Yanyanhu | some revisoin in keystone_v3 client | 06:17 |
Yanyanhu | guess so | 06:17 |
xuhaiwei | ok, you can aslo submit your patch too, when one is merged, the other one should be rebased, you dont need to wait for my patch | 06:18 |
Yanyanhu | xuhaiwei, 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 |
xuhaiwei | ok | 06:21 |
*** ChrisSen has quit IRC | 06:30 | |
Yanyanhu | hi, xuhaiwei, I found your patch didn't base on the master code | 06:32 |
Yanyanhu | I think you can rebase it and try again | 06:32 |
xuhaiwei | really? | 06:32 |
Yanyanhu | but I doubt the problem could still be there | 06:33 |
Yanyanhu | yes, not the latest code | 06:33 |
openstackgerrit | xu-haiwei proposed stackforge/senlin: Handle exceptions in keystone_v3 driver https://review.openstack.org/213569 | 06:34 |
Yanyanhu | after rebase, occasionally I met another error http://paste.openstack.org/show/418806/ | 06:35 |
xuhaiwei | this error seems like a test case's problem | 06:39 |
Yanyanhu | since is_admin=None, policy enforce is executed | 06:39 |
Yanyanhu | I guess we really need to remove context from driver layer | 06:39 |
Yanyanhu | we actually don't need it | 06:40 |
xuhaiwei | yes, I really dont understand it well | 06:41 |
*** ChrisSen has joined #senlin | 06:42 | |
Yanyanhu | I 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 happen | 06:44 |
xuhaiwei | "this error" means which one | 06:44 |
xuhaiwei | http://paste.openstack.org/show/418806/ this one? | 06:45 |
Yanyanhu | I mean this error http://paste.openstack.org/show/418806/ | 06:45 |
Yanyanhu | yes | 06:45 |
Yanyanhu | since the execution should stop here http://git.openstack.org/cgit/stackforge/senlin/tree/senlin/policies/lb_policy.py#n353 | 06:45 |
Yanyanhu | and from_dict method should not be invoked | 06:45 |
xuhaiwei | yes, very interesting | 06:46 |
Yanyanhu | let me mock the from_dict method to make some tests | 06:46 |
xuhaiwei | but this error doesn't happen in my env | 06:47 |
Yanyanhu | I guess the problem still happened in this line http://git.openstack.org/cgit/stackforge/senlin/tree/senlin/policies/lb_policy.py#n350 | 06:47 |
*** Zhenqi has quit IRC | 06:47 | |
Yanyanhu | em, weird | 06:47 |
xuhaiwei | yes, 350 is the root I think | 06:47 |
xuhaiwei | the error is different in our env, just because our databases are different? | 06:49 |
Yanyanhu | maybe... | 06:50 |
Yanyanhu | still not sure about the reason | 06:50 |
Yanyanhu | feel both the gate and git review website are slow today | 06:54 |
xuhaiwei | yes, at first, I thought it was the gate's problem | 06:55 |
xuhaiwei | Qiming is having an vocation? | 06:55 |
Yanyanhu | yes, he is in travel this week. | 06:56 |
Yanyanhu | I think he will have some time to work from wednesday | 06:56 |
xuhaiwei | and you wont have one too? | 06:57 |
Yanyanhu | after mock from_dict, error never happened again | 06:57 |
Yanyanhu | I think I won't have a vacation this summer :) | 06:57 |
Yanyanhu | but from_dict method was never invoked... | 06:58 |
Yanyanhu | ft | 06:58 |
xuhaiwei | just curious why in my env, it doesnt happen | 06:59 |
Yanyanhu | me too... | 06:59 |
Yanyanhu | let's wait for the gate's result to see whether there is any difference | 07:00 |
Yanyanhu | oh, let me make a test using master branch | 07:00 |
xuhaiwei | ok | 07:01 |
Yanyanhu | ran test 5 times in master branch and error didn't happen... | 07:08 |
Yanyanhu | BTW, I think the logic in line 69 might be incorrect https://review.openstack.org/#/c/213569/2/senlin/api/middleware/trust.py | 07:09 |
xuhaiwei | because 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 line | 07:16 |
xuhaiwei | yes, I think you are right, this line should be 'trust = None' ? | 07:18 |
xuhaiwei | should not return None | 07:18 |
Yanyanhu | xuhaiwei, yes, I guess this is the logic of original implementation | 07:19 |
xuhaiwei | got it | 07:19 |
*** Zhenqi has joined #senlin | 07:25 | |
openstackgerrit | Yanyan Hu proposed stackforge/senlin: Rework some interfaces in keystone_v3 driver https://review.openstack.org/213617 | 07:26 |
openstackgerrit | xu-haiwei proposed stackforge/senlin: Handle exceptions in keystone_v3 driver https://review.openstack.org/213569 | 07:47 |
openstackgerrit | Yanyan Hu proposed stackforge/senlin: Add a functional test of listing profile_type https://review.openstack.org/213040 | 07:52 |
xuhaiwei | gerrit is nearly dead | 08:03 |
Yanyanhu | seems so... | 08:04 |
xuhaiwei | just wanted to comment on your patch | 08:05 |
Yanyanhu | thanks, no hurry I think :) | 08:05 |
xuhaiwei | https://review.openstack.org/#/c/213040/2 the LOG is not used | 08:05 |
Yanyanhu | ok, let me check the code. Can't open the review web page... | 08:06 |
Yanyanhu | ah, right | 08:07 |
Yanyanhu | should remove it | 08:07 |
xuhaiwei | it's forcing me to go home :) | 08:07 |
Yanyanhu | :) | 08:07 |
Yanyanhu | some guys have complained in openstack-infra channel... | 08:14 |
Yanyanhu | hope gerrit can recover after this weekend | 08:15 |
xuhaiwei | this weekend? | 08:18 |
xuhaiwei | for us maybe tonight | 08:18 |
Yanyanhu | yes | 08:18 |
Yanyanhu | git review keep failing | 08:20 |
openstackgerrit | Yanyan Hu proposed stackforge/senlin: Add functional test for listing policy_types https://review.openstack.org/213626 | 08:21 |
Yanyanhu | ... | 08:21 |
Yanyanhu | waited for 5 minutes | 08:21 |
*** Zhenqi has quit IRC | 08:37 | |
mathspanda | hi, please look at this paste: http://paste.openstack.org/show/418941/ | 08:55 |
Yanyanhu | hi, mathspanda, I'm looking on it, thanks | 09:20 |
Yanyanhu | hi, can you paste the error log in engine side? | 09:21 |
mathspanda | no error in engine screen | 09:22 |
Yanyanhu | ok, let me have a check | 09:22 |
mathspanda | ok | 09:23 |
xuhaiwei | I am also investigating it | 09:24 |
xuhaiwei | it seems like 'count' option is required though it is not assigned a value | 09:26 |
mathspanda | yes. | 09:27 |
*** Tennyson has joined #senlin | 09:33 | |
Yanyanhu | from the code, seems we don't have to give a count as input parameter | 09:35 |
Yanyanhu | http://git.openstack.org/cgit/stackforge/python-senlinclient/tree/senlinclient/v1/models.py#n326 | 09:35 |
Yanyanhu | keep looking on this | 09:35 |
xuhaiwei | I also saw this line | 09:35 |
Yanyanhu | guess filter_args functional can't handle the case where some optional paramters are not provided | 09:36 |
*** mathspanda_ has joined #senlin | 09:37 | |
Yanyanhu | http://git.openstack.org/cgit/stackforge/python-senlinclient/tree/senlinclient/v1/client.py#n106 | 09:38 |
Yanyanhu | this line | 09:38 |
*** Tennyson has quit IRC | 09:39 | |
Yanyanhu | so this line enforce us to give value for all parameters in the action function list | 09:39 |
*** mathspanda has quit IRC | 09:39 | |
Yanyanhu | hi, 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 |
Yanyanhu | then if no value is provided for some optional parameters, None will be used | 09:42 |
Yanyanhu | as the value | 09:42 |
Yanyanhu | minute, let me think whether this is correct | 09:43 |
mathspanda_ | ok. i think it is a better way. | 09:43 |
Yanyanhu | um, but maybe we should use default value rather than None if it is provided in method definition | 09:44 |
Yanyanhu | let me see whether it is supported | 09:44 |
Yanyanhu | just a while | 09:44 |
mathspanda_ | ok. | 09:45 |
xuhaiwei | Yanyanhu, | 09:51 |
Yanyanhu | hi | 09:51 |
xuhaiwei | I think we just need to check if params contains the excepted_args | 09:52 |
xuhaiwei | accepted_args = ([a for a in expected_args and params if a != 'self']) | 09:52 |
xuhaiwei | what do you think | 09:54 |
Yanyanhu | you mean just remove line 107? | 09:54 |
xuhaiwei | no | 09:55 |
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 |
Yanyanhu | hm, I think it's ok | 09:56 |
Yanyanhu | so we just omit those accepted_args who are not provoided | 09:57 |
xuhaiwei | yes | 09:57 |
xuhaiwei | those accepted_args should have a default value I think | 09:58 |
Yanyanhu | yes | 09:58 |
Yanyanhu | so, mathspanda_, I think you can fix it using this way :) | 09:58 |
mathspanda_ | ok. thanks. | 09:59 |
Yanyanhu | this is much simpler and better than what I'm thinking | 09:59 |
Yanyanhu | please also file a bug if possible, thanks a lot :) | 10:00 |
mathspanda_ | ok. | 10:00 |
xuhaiwei | Yanyanhu, it seems there is another bug here | 10:03 |
Yanyanhu | ok | 10:04 |
Yanyanhu | you mean in client side? | 10:04 |
xuhaiwei | no, after doing this command, one node should be scaled into the cluster, right? | 10:05 |
xuhaiwei | but in fact, it's not | 10:05 |
Yanyanhu | you mean without providing any input for count in cmd line? | 10:05 |
xuhaiwei | no | 10:06 |
xuhaiwei | I mean by doing this command, server did nothing | 10:06 |
xuhaiwei | by cluster-scale-in , one node should be scaled in to the cluster i think | 10:06 |
Yanyanhu | hmm, I think your right | 10:07 |
Yanyanhu | let me check the cluster_action code | 10:07 |
Yanyanhu | yes, you are right, since the value of count field will be None | 10:08 |
xuhaiwei | if count is None, what should happen? | 10:09 |
Yanyanhu | this line will not work as expected I think | 10:10 |
Yanyanhu | http://git.openstack.org/cgit/stackforge/senlin/tree/senlin/engine/actions/cluster_action.py#n552 | 10:10 |
Yanyanhu | oh, sorry, not this one | 10:11 |
Yanyanhu | let me check it | 10:11 |
*** elynn_ has quit IRC | 10:11 | |
xuhaiwei | yes | 10:11 |
mathspanda_ | default is 0 in engine, except attach a scale in policy | 10:12 |
Yanyanhu | yes | 10:12 |
xuhaiwei | https://github.com/stackforge/senlin/blob/master/senlin/engine/service.py#L817 | 10:13 |
Yanyanhu | but if count is set to 1 if we get None from input, it will work as haiwei said | 10:13 |
mathspanda_ | yes. | 10:14 |
mathspanda_ | then the dafault count of scaleout should also be 1? | 10:15 |
Yanyanhu | um, I remeber this is the correct logic to use 1 as default count | 10:15 |
Yanyanhu | whyen user didn't specify it | 10:15 |
Yanyanhu | when | 10:15 |
xuhaiwei | Yanyanhu, it should be 1 by default | 10:16 |
xuhaiwei | or else, this command is meanless | 10:16 |
Yanyanhu | yes | 10:16 |
Yanyanhu | so maybe we need a patch there :) | 10:16 |
xuhaiwei | I found another bug in the server side | 10:16 |
xuhaiwei | do you guys want to investigate it today? | 10:17 |
xuhaiwei | I am a little hungry | 10:17 |
Yanyanhu | me too, actually | 10:17 |
xuhaiwei | I will report it and leave it tomorrow | 10:17 |
Yanyanhu | ok | 10:17 |
xuhaiwei | if you are intereted in it, can also see it | 10:17 |
Yanyanhu | sure :) | 10:18 |
Yanyanhu | hi, 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 552 | 10:20 |
Yanyanhu | and also add 'count = 0' before line 542 | 10:20 |
xuhaiwei | which file? | 10:20 |
Yanyanhu | http://git.openstack.org/cgit/stackforge/senlin/tree/senlin/engine/actions/cluster_action.py#n552 | 10:20 |
Yanyanhu | this one | 10:20 |
xuhaiwei | what about the client side? | 10:21 |
xuhaiwei | change None to '1' too? | 10:21 |
Yanyanhu | I 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 side | 10:22 |
xuhaiwei | I am a little confused now | 10:23 |
Yanyanhu | I mean maybe we should let scale in/out create/delete at least one node even no count value is provided in action input | 10:23 |
xuhaiwei | For this line 'return self.RES_OK, 'No scaling needed based on policy checking'' it seems not scaling is the right action if count == 0 | 10:23 |
Yanyanhu | yes | 10:24 |
Yanyanhu | if count is exact '0' | 10:24 |
Yanyanhu | e.g. scaling policy decide we can't do both creation or deletion operation | 10:24 |
Yanyanhu | that is a possible situation | 10:25 |
xuhaiwei | you mean someone will do this command ? "senlin cluster-scale-in -c 0 cluster1"?? | 10:25 |
Yanyanhu | I think usually this kind of operation is not started from clientside manually | 10:25 |
Yanyanhu | it is usually triggered automatically by other services like ceilometer | 10:25 |
xuhaiwei | yes, should not :) | 10:25 |
xuhaiwei | got it | 10:26 |
Yanyanhu | :) | 10:26 |
xuhaiwei | anyway, leave it tomorrow, it seems 3 or 4 patches are needed | 10:26 |
Yanyanhu | sure | 10:26 |
Yanyanhu | will leave, my wife is gonna be angry :p | 10:27 |
Yanyanhu | talk to you guys later | 10:27 |
xuhaiwei | see you, good man | 10:27 |
Yanyanhu | bye :) | 10:28 |
*** mathspanda_ has quit IRC | 10:31 | |
*** ChrisSen has quit IRC | 10:33 | |
*** Yanyanhu has quit IRC | 10:34 | |
*** Qiming has joined #senlin | 10:46 | |
*** Qiming has quit IRC | 10:59 | |
*** Qiming has joined #senlin | 12:50 | |
*** Qiming has quit IRC | 12:51 | |
*** lkarm has joined #senlin | 13:02 | |
*** jroyal has joined #senlin | 13:11 | |
*** jdandrea has joined #senlin | 13:36 | |
*** lkarm has quit IRC | 15:49 | |
*** lkarm has joined #senlin | 15:57 | |
*** mathspanda_ has joined #senlin | 16:05 | |
*** mathspanda_ has quit IRC | 16:06 | |
*** jroyal has quit IRC | 16:40 | |
*** jroyal has joined #senlin | 17:40 | |
*** jroyal has quit IRC | 17:44 | |
*** jroyal has joined #senlin | 17:57 | |
*** jroyal has quit IRC | 17:58 | |
*** jroyal_ has joined #senlin | 18:00 | |
*** jroyal__ has joined #senlin | 18:01 | |
*** jroyal_ has quit IRC | 18:04 | |
*** lkarm has quit IRC | 18:12 | |
*** lkarm has joined #senlin | 18:41 | |
*** jdandrea has quit IRC | 18:50 | |
*** lkarm has quit IRC | 18:57 | |
*** lkarm has joined #senlin | 18:58 | |
*** lkarm has quit IRC | 19:02 | |
*** lkarm has joined #senlin | 19:05 | |
*** jdandrea has joined #senlin | 19:21 | |
*** jdandrea has quit IRC | 19:21 | |
*** jdandrea has joined #senlin | 19:50 | |
*** lkarm has quit IRC | 19:54 | |
*** jroyal has joined #senlin | 19:58 | |
*** lkarm has joined #senlin | 19:58 | |
*** jroyal__ has quit IRC | 19:58 | |
*** jroyal has quit IRC | 21:31 | |
*** lkarm has quit IRC | 21:31 | |
*** jroyal has joined #senlin | 21:33 | |
*** jroyal_ has joined #senlin | 21:34 | |
*** jroyal has quit IRC | 21:37 | |
*** jroyal_ has quit IRC | 21:38 | |
*** jroyal has joined #senlin | 21:56 | |
*** jroyal has quit IRC | 22:00 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!