18:07:51 #startmeeting networking_policy 18:07:52 Meeting started Thu Dec 17 18:07:51 2015 UTC and is due to finish in 60 minutes. The chair is SumitNaiksatam. Information about MeetBot at http://wiki.debian.org/MeetBot. 18:07:53 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 18:07:55 The meeting name has been set to 'networking_policy' 18:08:04 david-lyle: worked :-) 18:08:44 #info agenda https://wiki.openstack.org/wiki/Meetings/GroupBasedPolicy#Dec_17th_2015 18:08:55 #topic bugs 18:08:59 is magesh around? 18:09:35 wanted to check if any new high priority bugs beyond the ones we discussed last week and are pending 18:10:04 SumitNaiksatam: I don't see him online 18:10:07 vks_: anything you saw recently that needs immediate attention? 18:10:09 songole: okay 18:10:30 specifically asking vks_ and magesh since they are doing a lot of testing 18:10:51 SumitNaiksatam, there is a limit of 4096 byte for ServicechainNode data 18:11:01 hi - sorry I’m late 18:11:20 ivar-lazzaro: did you get any more feedback on #link https://bugs.launchpad.net/group-based-policy/+bug/1521545 ? 18:11:20 Launchpad bug 1521545 in Group Based Policy "If service chain delete fails then the PTG can never be cleaned up" [Undecided,Incomplete] - Assigned to Ivar Lazzaro (mmaleckk) 18:11:28 rkukura: just in time :-) 18:11:31 SumitNaiksatam: nope, was waiting for the logs 18:11:42 ivar-lazzaro: okay, will try to ping magesh 18:11:56 vks_: do you know if there is any more info available on #link https://bugs.launchpad.net/group-based-policy/+bug/1521545 ? 18:12:41 it can be easily simulated 18:12:57 just restart neutron server in middle of delete 18:13:02 vks_: okay, can you add some info to the bug in that case? 18:13:38 vks_: and perhaps it might also help to give ivar-lazzaro quick access to your system when you are able to reproduce this 18:13:41 SumitNaiksatam, yeah but see the relevant log is there already, apart from that don't see any oyher logs 18:14:13 vks_: we need to check little bit prior to that particular log 18:14:18 vks_: we also discussed #link https://bugs.launchpad.net/group-based-policy/+bug/1523733 offline 18:14:19 Launchpad bug 1523733 in Group Based Policy "GBP PT delete allows to delete PT , even though the PT is bound to VM" [Undecided,New] 18:14:53 vks_: i was not able to reproduce the PT getting deleted when the port is deleted, so i need to check that on your setup 18:15:05 SumitNaiksatam, ok 18:15:07 any other burning issues? 18:15:34 okay lets move to the desing related discussions 18:15:43 #topic Service chain node config attr size 18:15:49 vks_: you brought this up 18:15:56 SumitNaiksatam, yeah 18:16:21 so currently we model the congfig column is the DB as a string of length 4096 18:16:42 vks_: however you are running into configs much larger than that 18:16:56 SumitNaiksatam, yes 18:17:15 so the question is how large do we go, or do we need to look at it in a different way 18:17:34 SumitNaiksatam, Essentially in case of firewall especially , where than can be n number of rules 18:17:40 vks_: right 18:17:43 we will have problem 18:17:55 and n is difficult to predict how large it can be 18:18:07 SumitNaiksatam, thats true 18:18:12 rkukura: ivar-lazzaro songole: any thoughts on this? 18:19:15 SumitNaiksatam: what is the proposed solution? 18:19:40 ivar-lazzaro: currently there is only a requirement that the size needs to be increased 18:19:49 however no clear proposal 18:19:56 we can increase to the max limit 18:19:59 Is this config attr really meant to be used in place of a proper REST service API? 18:20:04 but that might not be enough 18:20:12 rkukura: yes 18:20:26 rkukura: because this is service-specific config which GBP does not care about 18:21:15 SumitNaiksatam: this looks like something that would require a separate config repository 18:21:42 ivar-lazzaro: something like that would be ideal where we just provide a reference 18:21:48 SumitNaiksatam: that can't be the GBP Controller itself, since we also need to distribute that config around for high availability 18:21:51 ivar-lazzaro: rather than the entire config itself 18:22:03 SumitNaiksatam: which means that we should use something like Swift 18:22:21 SumitNaiksatam: to store these configurations 18:22:40 ivar-lazzaro: we can be agnostic to where its stored, just enough information in the config attribute to be able to look it up 18:22:43 I believe you would just use S3 if you were in amazon 18:22:59 I think sqlalchemy has a Text type that is unbounded 18:23:02 SumitNaiksatam: right, I was thinking about the reference implementation though :) 18:23:32 ivar-lazzaro: yeah that can contract can be between the user and the driver which knows where to pull it from 18:23:37 rkukura: do we really want to use the database for this kind of things? You can't even really write a proper query in that field 18:23:46 rkukura: okay 18:24:11 Maybe the config attr isn’t that useful to query 18:24:11 ivar-lazzaro: i dont think we perform any DB queries on that particular column 18:24:17 rkukura: right 18:24:36 No it isn't, that's just a blob 18:24:50 that's why I believe it should be stored in the proper service 18:25:02 it's not useful from a relational point of view 18:25:14 For example 18:25:34 if you query the DB and retrieve 20 MB worth of config files (just saying)( 18:25:44 you keep the lock for too long potentially 18:25:50 ivar-lazzaro: that is correct 18:25:56 so how about we do this - 18:26:31 we explore changing this column to a text field with the recommendation that you actually dont store the config itself in that field 18:26:54 after that if someone wants to shoot themselves in the foot then its their call 18:27:11 SumitNaiksatam: someone who? The Node Driver? 18:27:16 Or the user? 18:27:26 what is text field? You can store anything of any size? 18:27:32 we shouldn't give the user too much ability to shoot himself in the foot :) 18:27:59 ivar-lazzaro: the user is going to be go by the recommendation of a particular node driver in this case 18:28:13 ivar-lazzaro: so its the node-driver that i am referring to 18:28:23 If we want to go this route 18:28:34 songole: the “text” field is as referenced by rkukura 18:28:43 I believe we should have a text field, and a reference implementation that uses it properly (without recomandations) 18:29:01 http://docs.sqlalchemy.org/en/latest/core/type_basics.html#sqlalchemy.types.Text 18:29:02 then if a vendor implementation wants people to store config there, so be it 18:29:32 rkukura: thanks 18:29:32 but the reference should always enforce the best practice 18:29:48 ivar-lazzaro: i dont think we have a “reference” implementation 18:30:05 ivar-lazzaro: we have had this discussion before on the reviews as well 18:30:11 SumitNaiksatam: then we should just enforce the best practice :D 18:31:01 This is probably a dumb question that I should know the answer to, but what would need to happen to allow use of the neutron *aaS REST APIs rather than this config attr? 18:31:29 rkukura: I guess that would go through a heat template 18:31:49 rkukura: we dont assume that its always a *aas service 18:32:27 I understand we don’t assume that, but if it is, or is something close enough, is an API useful in the context for GBP service chaining? 18:33:16 Or is the fact that the API’s are per-instance the reason a heat template is more appropriate? 18:33:18 rkukura: how will the API look like? The idea of the config file is to guarantee vendor differentiation 18:33:28 rkukura: which can't happen if we normalize the operations 18:33:34 ivar is right 18:33:52 rkukura: we would need to make the service chain layer service aware, which we dont want to 18:34:36 rkukura: in theory, the specific node driver should be able to read the configuration in a service specific manner 18:34:48 SumitNaiksatam, but there shud be a generic way to pass a template from any service vendor 18:35:08 rkukura: so for instance, if you create a chain with a config blob that the node driver doesn't understand, it will reject the node creation and try on the next driver 18:35:46 A prerequisite for a Node Driver to accept to be scheduled for a node, is that it understands the configuration format 18:35:55 okay, so vks_ do you want to explore using this #link: http://docs.sqlalchemy.org/en/latest/core/type_basics.html#sqlalchemy.types.Text 18:36:27 ivar-lazzaro: btw, ivar-lazzaro i think you pretty much addressed your own point about “enforcement” in those two last comments ;-) 18:36:32 SumitNaiksatam, yeah sure 18:36:36 OK, I now understand the current GBP SC approach does not cater to service’s with APIs. 18:36:37 vks_: okay 18:36:58 rkukura: its agnostic of that 18:37:15 rkukura: i dont think “does not cater to” would be the right characterization 18:37:33 ok we have a couple of more topics to discuss at least 18:37:46 Would it make sense for this config attribute to not be queries and returned by default? 18:38:39 just came across http://arxiv.org/ftp/cs/papers/0701/0701168.pdf 18:38:44 rkukura: we need to be able to display config as stored 18:38:46 should give a look at it as well 18:39:27 although it's a pretty old paper 18:39:37 ivar-lazzaro: the debate around blob is not new 18:39:54 ivar-lazzaro: its good to use in certain places and its not in certain others 18:40:45 From the abstract of ivar-lazzaro’s link, it seems <=256KB belongs in the DB 18:41:29 we only allow 4k right now 18:41:37 rkukura: but different DB implementations wil tune this in a different way 18:41:37 but I'm not sure there's actually a limit 18:41:59 rkukura: its proabably they size of the records they can read in a single shot 18:42:12 versus multiple reads 18:42:59 okay, so vks_ and perhaps magesh can take the action item to explore using the text field as we previously discussed 18:43:05 moving on 18:43:11 SumitNaiksatam, OK 18:43:22 #topic Port extra attribute extension 18:43:32 #link https://review.openstack.org/#/c/256880/ 18:44:17 ivar-lazzaro: i believe bob had a comment to at least cross refernce a bug for this 18:44:23 bob -> rkukura 18:44:47 SumitNaiksatam: mmm I created a bp 18:44:58 apparently didn't set it in the commit message though 18:45:32 ivar-lazzaro: okay, but in that case was a spec posted? 18:45:47 I wasn’t too thrilled about adding a special write-only attribute to the REST API, but I think ivar-lazzaro has addressed that, right? 18:45:53 SumitNaiksatam: nope 18:46:03 SumitNaiksatam: the change is for internal API only 18:46:19 SumitNaiksatam: and no persistence 18:46:32 ivar-lazzaro: i would recommend treating this as a bug in that case, because having a BP without a spec has no meaning 18:46:34 I can write a spec about this, but I feel it would take like 5 lines total or something 18:46:52 I bug seems sufficient to me 18:46:53 Even if the issue is explained in the BP itself? 18:47:06 s/I/A/ 18:47:28 rkukura: yeah, the API is only internal now 18:47:38 ok I'll move the BP as a bug 18:47:54 ivar-lazzaro: Is the policy.json change still needed? 18:48:02 ivar-lazzaro: yes, launchpad BP is only for tracking purposes, specs are in gerrit as patches 18:48:07 ivar-lazzaro: who is the intended user of this API? 18:48:18 songole: node drivers 18:48:20 songole: its not an API change as of now 18:48:37 ivar-lazzaro: ok 18:48:39 rkukura: no, but we missed that entry anyways 18:48:52 rkukura: I can remove that change 18:49:11 rkukura: did you get a chance to review the current version of the patch? 18:49:16 is it possible to convert a BP into a bug? 18:49:18 https://blueprints.launchpad.net/group-based-policy/+spec/port-extra-attributes 18:50:20 SumitNaiksatam: I have not, but will 18:50:22 ivar-lazzaro: i will create the bug for you ;-) 18:50:46 SumitNaiksatam: I can do it, just didn't want to leave the BP around 18:51:02 So this is a short-term fix that we need to backport to juno, etc? 18:51:03 ivar-lazzaro: i think we can obsolete that 18:51:09 rkukura: yes 18:51:10 SumitNaiksatam: ok! 18:51:53 ivar-lazzaro: thanks for the update, so seems like we are good to go with this once the bug is cross-referenced 18:51:57 moving on 18:53:02 #topic Propagating post-commit exceptions 18:53:13 this is a discussion ivar-lazzaro and were having offline 18:53:24 and i believe at some point we might discussed with rkukura as well 18:53:39 so we have this block of code: 18:53:41 #link https://github.com/openstack/group-based-policy/blob/master/gbpservice/neutron/services/grouppolicy/policy_driver_manager.py#L122-L127 18:53:56 which masks the exceptions thrown by the drivers 18:54:02 and for good reason 18:54:35 so as to abstract the driver technology specific details from the user 18:55:34 however, since we are mostly doing orchestration in our drivers as well, we are increasingly feeling the need to propagate these exceptions to the users 18:56:04 it seems like there is a lot more benefit to the end user seeing the real exceptions than from abstracting them 18:56:23 not to mention that it greatly helps in debugging 18:56:23 Would it make sense to propagate a certain portion of the exception hierarchy, but mask the rest? 18:56:45 rkukura: i agree with that, if there is a deterministic way to do that 18:58:07 Maybe anything derived from GroupPolicyDriverError should get propogated, so drivers can subclass this if they want to return details to the user. 18:58:25 rkukura: okay 18:58:46 I haven’t thought that through, but just wanted to give an example of how it might work. 18:58:54 rkukura: right 18:59:19 okay so there doesnt seem to be outright objection to doing this, which is good 18:59:23 so we can explore 18:59:32 #topic Open Discussion 18:59:47 just a quick reminder we wont be meeting for the next couple of weeks 18:59:55 anything else that we might have missed? 19:00:45 okay then lets wrap up 19:00:49 not that I can think of 19:00:54 rkukura: oka 19:00:56 y 19:01:03 best wishes for the holidays and the new year to all! 19:01:17 bye! 19:01:21 thanks SumitNaiksatam! 19:01:28 bye 19:01:31 bye 19:01:37 #endmeeting