22:01:21 <kevinbenton> #startmeeting neutron_drivers
22:01:21 <openstack> Meeting started Thu Jun  8 22:01:21 2017 UTC and is due to finish in 60 minutes.  The chair is kevinbenton. Information about MeetBot at http://wiki.debian.org/MeetBot.
22:01:22 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
22:01:25 <openstack> The meeting name has been set to 'neutron_drivers'
22:01:32 <ihrachys> o/
22:01:44 <kevinbenton> armax, amotoki_away: ping
22:01:54 <kevinbenton> presumably amotoki is indeed away :)
22:02:03 <mlavalle> indeed.... LOL
22:02:25 <kevinbenton> armax told me he might not be able to make it today
22:02:40 <ihrachys> heh
22:02:43 <ihrachys> 3/5?
22:03:02 <kevinbenton> yeah, i suppose we can go through a few things
22:03:11 <kevinbenton> one i want to talk about is the l2 extension manager
22:03:55 <ihrachys> what's there? people showed up and drive the work?
22:04:07 <kevinbenton> so it sounds like Thomas is willing to be approver
22:04:08 <ihrachys> I lost the context since all fire drills that never end
22:04:20 <kevinbenton> and David is willing to write the code
22:04:42 <kevinbenton> i'm looking for spec now
22:04:48 <mlavalle> davidsha?
22:05:03 <kevinbenton> https://review.openstack.org/#/c/320439/
22:05:07 <ihrachys> ok then let them? I believe the initial implementation doesn't imply that everything should switch to the new way in one go, so we can consider it experimental and play with it.
22:05:08 <kevinbenton> mlavalle: yep
22:05:59 <kevinbenton> mlavalle:  can you take a look through that spec so you're familiar with it?
22:06:09 <kevinbenton> mlavalle: and give +2 if it looks good
22:06:13 <kevinbenton> i'
22:06:17 <kevinbenton> will go through it as well
22:06:25 <kevinbenton> and that will give us enough to approve
22:06:30 <kevinbenton> and let them go on their way
22:06:30 <mlavalle> I was about to propose to review it
22:06:42 <mlavalle> yes, I will look at it tomorrow
22:06:54 <kevinbenton> ok
22:06:56 <mlavalle> is that ok?
22:07:20 <kevinbenton> yep
22:07:29 <mlavalle> added to my pile
22:08:15 <kevinbenton> ok
22:08:33 <kevinbenton> shall we take a look at a few RFE's?
22:08:49 <mlavalle> yeah
22:09:00 <kevinbenton> #link https://bugs.launchpad.net/neutron/+bugs?field.status%3Alist=Triaged&field.tag=rfe
22:09:06 <kevinbenton> #link https://bugs.launchpad.net/neutron/+bug/1649417
22:09:07 <openstack> Launchpad bug 1649417 in neutron "RFE: Security group rule using address set" [Wishlist,Triaged] - Assigned to Dongcan Ye (hellochosen)
22:09:48 <mlavalle> I thought it is a sensible use case
22:09:57 <ihrachys> I *think* it was once proposed in the past
22:10:02 <ihrachys> I think ~Barcelona
22:10:20 <kevinbenton> i was thinking we might not even need to change the SG API itself
22:10:21 <ihrachys> there was a guy (I don't remember the name) who pitched the idea
22:10:30 <kevinbenton> we can overload remote_group_id
22:10:39 <kevinbenton> to reference either a new set thingyt
22:10:43 <kevinbenton> or security group
22:11:07 <mlavalle> so it becomes only adapting the pipes
22:11:22 <kevinbenton> yeah
22:11:36 <kevinbenton> and a separate api to create AddressSets
22:11:40 <ihrachys> so remote group will point to a new compound resource and the latter will point to set of SGs?
22:12:14 <ihrachys> I mean, set of addresses
22:12:40 <kevinbenton> so remote_group will either be an AddressSet or a SecurityGroup
22:12:44 <kevinbenton> is what i was thinking
22:13:06 <ihrachys> so consider this: a user not knowing about overloading fetches SG, looks at remote group, fetches it via SG API and gets 404
22:13:10 <kevinbenton> yeah
22:13:13 <kevinbenton> that's the downside
22:13:41 <ihrachys> that's a blocker I would say no?
22:13:52 <kevinbenton> Another option may be to allow address sets to associate with a security group
22:14:08 <kevinbenton> then for remote_group_id they still reference a security group
22:14:17 <kevinbenton> and that includes all of the sets associated with that group
22:15:12 <kevinbenton> I want to avoid adding a new thing to the rules themselves
22:15:34 <ihrachys> that may ofc raise some eyebrows like - I have my SG rules blocking all but X and Y, but I can access Z too
22:15:42 <ihrachys> but that's a downside of any new api
22:15:56 <kevinbenton> there is something else here
22:16:03 <kevinbenton> this already sort of works with allowed address pairs
22:16:18 <kevinbenton> a little unknown feature is that security groups allows all addresses in members allowed address pairs
22:16:52 <kevinbenton> that may actually solve this use case
22:16:59 <kevinbenton> create a dummy port for your external stuff
22:17:07 <kevinbenton> add each IP into the allowed_address_pairs entries
22:18:19 <mlavalle> let's propose that to the submitter in the RFE, let's see his feedback
22:19:14 <ihrachys> +
22:19:14 <kevinbenton> i will do that right now
22:19:32 <mlavalle> kevinbenton: do you mind going out of order for one RFE?
22:20:57 <kevinbenton> mlavalle: sure
22:21:00 <kevinbenton> mlavalle: link it now
22:21:04 <mlavalle> https://bugs.launchpad.net/neutron/+bug/1682775
22:21:05 <openstack> Launchpad bug 1682775 in neutron "[RFE] Tag mechanism supports all resouces with standard attribute." [Wishlist,Triaged] - Assigned to Hirofumi Ichihara (ichihara-hirofumi)
22:21:18 <mlavalle> hichihara already has a patchset in gerrit
22:21:29 <mlavalle> which I reviewed earlier today
22:21:44 <mlavalle> so I thought we migh as well look at the RFE
22:21:47 <kevinbenton> yeah
22:21:54 <kevinbenton> ihrachys: did you have a concern here?
22:22:17 <ihrachys> looking
22:22:28 <kevinbenton> IIUC after briefly looking at the patches this is just extending to include all standard attr resources
22:22:37 <mlavalle> correct?
22:22:47 <ihrachys> my concern was about how we define 'all' resources
22:22:59 <kevinbenton> mlavalle: do you have a patch link handy
22:23:12 <kevinbenton> maybe i'll just update the title to all standard attribute resources if that's the case
22:23:15 <ihrachys> we don't have this concept of overarching api extensions covering 'everything'
22:23:24 <mlavalle> https://review.openstack.org/#/c/460391/
22:23:42 <kevinbenton> yeah, this is just standard attr resources
22:23:47 <kevinbenton> so i'll update rfe title to reflect that
22:23:52 <kevinbenton> ihrachys: will that address your concerns?
22:23:56 <ihrachys> it's not like we have a stable list of those resources either
22:24:07 <ihrachys> at least not discoverable
22:24:20 <kevinbenton> ihrachys: if those resources change then there will be a separate extension that indicates that
22:24:27 <mlavalle> ihrachys: he tries to address that, hang on
22:25:07 <mlavalle> https://review.openstack.org/#/c/460391/5/neutron/extensions/tagging.py@42
22:25:07 <ihrachys> so what the RFE proposes is to add tags to all resources that atm support stdattrs, and to NOT cover for any resources that may add support for stdattr in the future?
22:25:24 <kevinbenton> ihrachys: they would automatically get them in the future
22:25:25 <ihrachys> then I am good, no controvercy from api discoverability pov
22:25:26 <mlavalle> yeah, that's what it is doing now
22:25:32 <kevinbenton> ihrachys: similar to revision_number and timestamp
22:26:03 <ihrachys> kevinbenton, and how does api user determine if it's going to be present on resourceA?
22:26:26 <ihrachys> it would somehow need to know the server version to relate, but we don't have versions.
22:26:30 <kevinbenton> ihrachys: resourceA is a new extension, right?
22:26:45 <kevinbenton> i mean a new resource introduced by an extension
22:27:30 <ihrachys> let's say ports have stdattrs, but qos don't. you add 'all-tags' extension that indicates that ports have tags; then in next release, you add support for qos. will you add a new extension called 'qos-tags'?
22:27:41 <kevinbenton> no
22:27:56 <ihrachys> then it's not discoverable
22:28:02 <kevinbenton> i don't understand your question
22:28:08 <kevinbenton> this patch introduces a new extension
22:28:16 <kevinbenton> that covers all standard attr stuff that exists now
22:28:28 <kevinbenton> so the only two options left are completely new standard attr resources
22:28:36 <kevinbenton> and resources that weren't standard attr before
22:28:43 <kevinbenton> both of those will have a new extension
22:28:49 <kevinbenton> which is the case before this patch as well
22:29:13 <ihrachys> ok, then the proposal doesn't cover 'all stdattr enabled resources' but 'all stdattr resources from Pike'
22:29:38 <kevinbenton> all standard attr resources that didn't have tags yet
22:30:04 <ihrachys> that makes sense. it's not open end, which is not controvercial.
22:30:26 <kevinbenton> well say we add AddressSets and they are standard attr
22:30:31 <kevinbenton> they will automatically get tags
22:30:54 <ihrachys> new resources are easy
22:30:59 <kevinbenton> but the address-set extension or whatever we would call it would automatically include tags
22:31:02 <kevinbenton> ok
22:31:21 <kevinbenton> so your concern is things that aren't standard attr now that already exist?
22:31:33 <ihrachys> kevinbenton, so to understand, it won't be possible to implement address-sets without tags?
22:32:00 <kevinbenton> ihrachys: if they are standard attr, they automatically get tags
22:32:04 <mlavalle> if it has standard attributes, no
22:32:07 <ihrachys> kevinbenton, yes, that's my concern, resourceX that is not there yet.
22:32:52 <kevinbenton> ihrachys: so converting resourceX to standard attr should have an extension regardless of this because of revision numbers, timestamps, and descriptions
22:33:03 <ihrachys> kevinbenton, that's implementation. I am talking more about api semantics. from neutron api model, you don't get anything automatically, you get it by virtue of attr map loaded into neutron-server.
22:33:30 <ihrachys> kevinbenton, I think we can discuss those details in review. RFE is fine. maybe wording is not ideal for me, but that's ok
22:33:45 <kevinbenton> ihrachys: attr map is derived from standard attr inheritors
22:33:52 <kevinbenton> ihrachys: automatically
22:34:13 <ihrachys> attr map is supposed to be static, it doesn't depend on implementation
22:34:17 <kevinbenton> ihrachys: https://review.openstack.org/#/c/460391/5/neutron/extensions/tagging.py@44
22:34:24 <ihrachys> you commit it to neutron-lib and don't touch it
22:34:58 <ihrachys> kevinbenton, I know the code; remember I was eagerly against it when you pushed it into tree late Newton?
22:35:11 <kevinbenton> ihrachys: so why are you suggesting that's not how it works? :)
22:35:27 <ihrachys> I am saying that's not how it *should* work
22:35:43 <ihrachys> api doesn't depend on server code, it's contract.
22:35:50 <ihrachys> *should not* depend
22:36:14 <kevinbenton> that ship sailed long ago with extensions defined out of tree in code
22:36:18 <kevinbenton> but i get your point
22:36:26 <kevinbenton> you can suggest that he statically enumerate them
22:36:41 <kevinbenton> let's approve RFE and you can provide that feedback in the review
22:37:00 <ihrachys> kevinbenton, enumerate is what I support; hence my comments on difference between open ended 'all' and closed list. anyhoo, I think we can move.
22:37:05 <ihrachys> ++
22:38:55 <kevinbenton> ok, left a comment
22:40:02 <ihrachys> nicely worded, thanks
22:40:43 <kevinbenton> mlavalle: so patch is good in current approach, basically the only line that needs to change is that one you linked
22:40:54 <mlavalle> cool
22:40:56 <kevinbenton> mlavalle: instead of making that function call for getting all standard attrs
22:41:02 <kevinbenton> ihrachys: just manually list each resource
22:41:10 <ihrachys> +
22:41:25 <kevinbenton> #link https://bugs.launchpad.net/neutron/+bug/1652071
22:41:26 <openstack> Launchpad bug 1652071 in neutron "[RFE] Implement migration from iptables-based security groups to ovsfw" [Wishlist,Triaged]
22:41:49 <ihrachys> I looked at the abandoned patch, I don't think it's the way to go
22:41:57 <ihrachys> I suggest we don't explore further in-place switch
22:42:03 <ihrachys> instead expand on live migration
22:42:16 <ihrachys> which ofc requires multiple port bindings and nova work
22:42:38 <ihrachys> but it's a complete solution for all backends, not a hack
22:42:52 <kevinbenton> yep
22:43:01 <kevinbenton> the only downside is that i'm not sure we can switch default
22:43:14 <kevinbenton> or suggest a default switch
22:43:34 <kevinbenton> because it's a non-standard upgrade that requires rolling migration
22:43:46 <ihrachys> I don't think it matters if live migration works. let the market (users) choose where they want to migrate to. maybe it's ovs to linuxbridge ;)
22:43:47 <kevinbenton> (we have no default, but deployment tools do)
22:44:08 <kevinbenton> well i was hoping we would have a path to actually dump the hybrid iptables driver
22:44:14 <kevinbenton> at some point
22:44:36 <ihrachys> you can always do it. follow deprecation process and remove/push code out of tree
22:44:46 <ihrachys> I don't think anything stops us if we document the process
22:45:01 <ihrachys> a release note will ofc be needed
22:45:04 <kevinbenton> would be the first time we had a breaking upgrade for a node, no?
22:45:24 <ihrachys> why no
22:45:34 <ihrachys> let's say a config option changes name
22:45:40 <ihrachys> we are going to break in 2 cycles
22:45:49 <ihrachys> 1st cycle we warn, 2nd we remove
22:45:50 <kevinbenton> they just update the config in response to warnings
22:46:01 <kevinbenton> there is nothing they can do here to avoid dataplane disruption
22:46:02 <ihrachys> mitigation will be more involving ofc
22:46:17 <ihrachys> in theory live migration is not disruptive
22:46:22 <ihrachys> from instance pov
22:47:04 <kevinbenton> do all deployments support live migration? Is that something other project upgrades depend on?
22:47:35 <ihrachys> well from product pov, RH-OSP upgrade implies live migration of all instances from compute to another node before upgrade.
22:47:39 <ihrachys> so it should work :)
22:48:06 <kevinbenton> ok, so you don't even upgrade components with VMs running on them?
22:48:07 <ihrachys> not to say I want to drag RH into the formula, just saying it works
22:48:17 <ihrachys> kevinbenton, not for compute
22:48:35 <ihrachys> for agents procedures should actually be similar but I am not sure we document it that way
22:48:51 <kevinbenton> well yeah it works, but I'm wondering if other projects force live migration on upgrade
22:49:00 <kevinbenton> e.g. cinder
22:49:04 <ihrachys> the reason why we do it this way is because we usually need a reboot for kernel and stuff
22:49:14 <kevinbenton> makes sense
22:49:40 <kevinbenton> so the proposed patch i was definitely against
22:49:44 <ihrachys> well I don't say neutron *forces* it. we can recommend.
22:49:55 <kevinbenton> well we will force if we deprecate
22:49:58 <ihrachys> worst case is they just stick to the old driver indefinitely
22:49:59 <kevinbenton> which i was hoping to do
22:50:07 <kevinbenton> but for now that doesn't seem feasible
22:50:35 <kevinbenton> the other option for migration is to leave iptables bridge behind and clear all of it's iptables rules
22:51:21 <ihrachys> ok, let's say it this way - mainstream path is live migration, but we can explore in-place if we have time and will after we close the first path.
22:51:32 <kevinbenton> yeah, i'm for marking this deferred
22:51:38 <kevinbenton> then revisit next cycle as people start to move
22:51:41 <ihrachys> in place should be a thing you pick knowingly, can live out of stadium
22:52:11 <kevinbenton> ihrachys: well it could be the new hybriddriver eventually
22:52:15 <ihrachys> not to mention there are still major issues with the firewall, so it's not time to discuss how we will move everyone to greener pastures ;p
22:52:29 <kevinbenton> true
22:52:30 <kevinbenton> :)
22:53:03 <ihrachys> + for defer/postpone/whatever it's called
22:53:11 <mlavalle> +
22:54:02 <kevinbenton> let's try one more
22:54:04 <kevinbenton> #link https://bugs.launchpad.net/neutron/+bug/1653932
22:54:05 <openstack> Launchpad bug 1653932 in neutron "[rfe] network router:external field not exported" [Wishlist,Triaged] - Assigned to Kevin Benton (kevinbenton)
22:54:58 <kevinbenton> basically the issue here is that subnets aren't visible on external networks
22:55:10 <kevinbenton> beyond their IDs in the network body
22:56:00 <ihrachys> well that's as designed. unless we introduce shared field for subnets, or a new rbac action for network. is it what's proposed?
22:56:24 <mlavalle> I think the latter
22:56:30 <kevinbenton> so subnets has a shared field that is derived from the network's shared field
22:57:01 <kevinbenton> so we could do the same thing
22:57:25 <kevinbenton> have a 'router:external' field that derives from parent network
22:57:25 <ihrachys> inherit access_as_external from network?
22:57:58 <kevinbenton> let's revisit this
22:58:06 <kevinbenton> policy engine can do parent lookups
22:58:16 <kevinbenton> so i need to see if it's possible without adding more fields
22:58:47 <ihrachys> like policy.json rule change?
22:58:50 <kevinbenton> https://github.com/openstack/neutron/blob/d26b96b7a27c8fcd5e99a6494de406436a48339e/etc/policy.json#L6
22:59:02 <kevinbenton> so that triggers policy engine to lookup parent network
22:59:31 <kevinbenton> if we can just make this possible for this guy to write a rule that says (network:router_external=True) or something on get_subnet
22:59:35 <kevinbenton> then we should be good
23:00:08 <ihrachys> +
23:00:09 <ihrachys> time
23:00:27 <kevinbenton> thanks
23:00:30 <mlavalle> o/
23:00:32 <bzhao_> bye
23:00:35 <kevinbenton> #endmeeting