00:01:45 #startmeeting congressteammeeting 00:01:46 Meeting started Thu Sep 28 00:01:45 2017 UTC and is due to finish in 60 minutes. The chair is ekcs. Information about MeetBot at http://wiki.debian.org/MeetBot. 00:01:47 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 00:01:49 The meeting name has been set to 'congressteammeeting' 00:02:08 hi all. welcome back! topics are here as usual: #link https://etherpad.openstack.org/p/congress-meeting-topics 00:02:51 ekcs: hi 00:02:58 hi ramineni_ ! 00:04:28 let’s get started then =) 00:04:53 let’s start with the qos driver 00:05:09 #topic QoS patch 00:05:14 #link https://review.openstack.org/#/c/488992/ 00:05:15 patch 488992 - congress - Add Qos translator in neutron datasource drive. 00:05:59 maybe we can settle on a course of action for the duplicated code between neutron driver and neutron qos driver 00:06:42 ramineni_ said: Actually, my initial thought was if we inherit from Neutronv2 driver, would make sense, but you pointed out that both are independent, may be we can make this class method in neutronv2 and use it here or if we can reuse this for every datasource, adding in common file like utils.py makes sense.? 00:07:21 I saw your comment .. if it's too difficult to common out we can leave it as it is .. And fix that later 00:10:01 I like the thought about whether it’s a general thing many drivers can use. 00:10:36 the concept is certainly general, but I think it’s used only in neutron because of the way neutron’s update method works. 00:11:00 ekcs: right 00:12:39 a class method is complicated because it calls instance methods. 00:12:53 I do think it’s a good idea to minimize duplication. 00:13:09 So I wonder if it’s worth introducing an abstract parent class for both neutron and neutron qos 00:14:01 ekcs: I think sounds good 00:14:23 We can add all the common functions to base class 00:15:29 i’m on the fence because avoiding duplication is good, but may also be overkill in this case. 00:15:48 You know another thought is maybe we can remove that method altogether from qos 00:16:48 I think we can just do that actually. 00:16:51 I’ll look into that. 00:16:58 Is it not required 00:17:03 ? 00:17:47 For action execution we can directly call neutron: update.. 00:17:48 it’s a special action to make it more convenient to invoke neutron update action. 00:18:04 but I don’t think we necessarily need to in the qos driver 00:18:48 unless it’s needed for qos specific action, which I don’t think it is. 00:18:49 Ok got it .then we can remove it 00:19:02 alright great! easiest solution of all haha. 00:19:21 It uses neutron client .. So it should be supported for all actions 00:19:38 Both uses neutron client 00:20:10 So ..ya method is not required in QoS I suppose 00:20:25 right. 00:20:43 ok moving on then 00:20:56 #topic other patches 00:21:02 any other patch we should discuss? 00:21:16 #link https://review.openstack.org/#/q/project:openstack/congress+status:open 00:21:55 maybe not. 00:22:23 On this patch: https://review.openstack.org/#/c/492791/ 00:22:23 patch 492791 - congress - Resolve replica test instability and re-enable 00:22:40 Your first patch seems good ..but I want to check logs once .. It's giving file not found .. So issued recheck 00:23:10 Then Jenkins seems not passing 00:23:14 :( 00:24:08 ekcs: yes, please go ahead .. I didn't understand that patch fix 00:24:10 got it yea that shouldn’t be hard to fix. i’ll push it through later. 00:24:33 492791 00:24:50 Yes for that one, I don’t totally understand either. 00:25:03 I’m with you that it seems initialize to called before services are launched 00:25:18 so by the time congress responds to curl the file should be there. 00:25:36 but it’s only even remotely plausible possibility I could come up with why sometimes 00:25:45 the permission setting didn’t go through. 00:26:27 Since it’s just testing code that doesn’t affect users, I’d like to just merge it and see what happens. 00:26:48 if the same problem remains, then we can investigate further. 00:26:54 but if the problem goes away then we’re good. 00:27:20 Hmm 00:27:52 But can we check permission is there at that point in test 00:28:05 Did u try to print that one 00:29:00 Ok .. Let me go through patch again 00:29:51 Replica instability came after the encryption patch merged? 00:30:41 I know it’s because of permission because I see it in the logs. first they won’t work at all. then I added this permission fix to devstack to make it work. but somehow once in a while I still saw the same permission issue. 00:30:55 yes it’s because of encryption patch. 00:31:01 how about we add an bash echo statement here: https://review.openstack.org/#/c/492791/1/devstack/plugin.sh@238 00:31:02 patch 492791 - congress - Resolve replica test instability and re-enable 00:31:31 and merge it. that way we’ll be able to see whether it ever happens the file doesn’t exist at this point i ndevstack. 00:33:21 ekcs: ok .. hoping we won't be looping forever if file creation failed with some reason 00:34:07 well then at least the problem should be obvious =) 00:34:09 ok then. 00:34:40 moving on then. 00:35:15 #topic proposed neutron action to attach/detach security group to port 00:35:45 When a Congress policy modifies the security groups associated with a neutron port, it is often helpful to have an action to add or remove a single security group. Here is an example: https://etherpad.openstack.org/p/congress-microseg (find string: neutronv2:attach_security_group_to_port) 00:36:13 But the standard neutron API requires updating the entire list of security groups associated with a port (https://developer.openstack.org/api-ref/network/v2/index.html#update-port) 00:36:26 which is hard to do in congress policy. 00:37:03 So I’m thinking I’ll add a driver action for attach and an action for detach (a single group to port) 00:37:43 Ok .. And u call update port only in that right 00:38:18 the code should be fairly simple. read the SGs attached to port. add a new one. then call update with the new list. 00:38:37 Ok 00:38:46 the only problem is that it can give an undesirable result sometimes because it’s not transactional. 00:39:13 if between the retrieval and the update, another API call made a change, that change can get lost. 00:39:19 actually overwritten. 00:39:39 Ohk .. Right 00:40:18 We can't use update_ resource_attrs 00:40:36 Even with that we have same problem right 00:40:40 we can. but update means replace old list with new list. not add/remove individual element. 00:40:42 right. 00:41:14 Ok .. I think sounds good .. I'll take a look at patch when u submit 00:42:12 ok. yea unfortunately we’re fundamentally limited by the neutron api here. i’ll submit a patch and include these discussions. 00:42:41 Ok 00:43:42 #topic enable all/most drivers in conf by default 00:44:14 we have already discussed and agreed between you, me, and masahito. was hoping to bring it up when thinrichs is here 00:45:00 Oohk .. then may be next time then 00:45:02 but I think we should just go ahead and do it. pretty straightforward thing. no change for existing configs. can be overridden by new config. 00:46:05 do you see any possible problems to discuss? 00:47:10 ekcs: my main idea is not to keep it configurable .. and just in Conf if we have to disable ..just use driver names like nova , etc instead of class names 00:48:12 ekcs: so not my ideal solution .. but solves part of issue I suppose :p 00:48:23 Main issue 00:48:41 so you’re proposing we change the conf option so that we disable rather than enable drivers? 00:49:20 what case do you see that’s better addressed by changing the conf option rather than just enabling by default? 00:49:31 Yes , if we have to ..by default all will be enabled .. That's my idea 00:50:03 ok I’m a little confused. 00:50:22 from our discussions we have a possible approach: 00:51:04 I don't see it's ideal to enable via class names in Conf .. it's not user changeable ..who does nt know the code 00:51:05 A. all/most drivers enabled by default. most users would not need to specify the drivers conf option at all in their conf file. 00:52:04 So you’re saying A is good, but you would like to do something in addition to A. 00:52:43 that is if a user does find the need to specify the drivers conf option, that they do it by driver name rather than class path 00:52:46 is that correct? 00:53:14 Using stevedore ..not enable by conf .. 00:53:44 Enable by class names in Conf is not my option and expose to user 00:54:05 ok i’m confused again. 00:54:38 if we make it not configurable, then it doesn’t matter whether it’s by class path or driver name right? 00:54:44 But I think we can go ahead as discussed before .. It's just I'm not fully on-board somehow .. 00:55:38 ok can you explain a situation where a user benefits from the stevedore approach but not this default conf approach? 00:55:50 we can continue the disucssion later too. 00:55:52 From user / operator point of view .. if we expose in Conf ..it's confurable right 00:56:02 yes. 00:56:09 You can't make it read-only option for them 00:56:19 no 00:57:10 I think time up again ..we can continue later 00:57:25 Any other topics 00:57:35 i don’t have other topics. 00:57:40 Queen priority 00:57:45 ? 00:57:57 I assigned 2 features 00:58:21 great! 00:58:22 Tempest and policy lib ..do they sound good 00:58:38 Or any other priority I can take it up? 00:59:01 yea that sounds great! 00:59:16 Great 01:00:23 ok then see you next time then! have a great week/end! 01:00:34 Bye 01:00:43 bye! 01:00:46 #endmeeting