14:00:27 <slaweq> #startmeeting neutron_drivers
14:00:28 <openstack> Meeting started Fri Sep 27 14:00:27 2019 UTC and is due to finish in 60 minutes.  The chair is slaweq. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:29 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:31 <openstack> The meeting name has been set to 'neutron_drivers'
14:00:31 <slaweq> hi
14:00:35 <njohnston> o/
14:00:39 <ralonsoh> hi
14:01:05 <mlavalle> o/
14:01:11 <slaweq> let's wait for the quorum
14:01:45 <yamamoto> hi
14:01:54 <haleyb> hi
14:02:10 <slaweq> ok, so we have quorum now
14:02:14 <slaweq> and we can start
14:02:27 <slaweq> #topic RFEs
14:02:39 <slaweq> I prepared couple of RFEs for today
14:03:00 <slaweq> first is: https://bugs.launchpad.net/neutron/+bug/1843211
14:03:00 <openstack> Launchpad bug 1843211 in neutron "network-ip-availabilities' result is not correct when the subnet has no allocation-pool" [Wishlist,In progress] - Assigned to yangjianfeng (yangjianfeng)
14:04:31 <ralonsoh> I dont know if yangjianfeng is here now
14:04:44 <ralonsoh> just to comment about it
14:04:50 <ralonsoh> I think the idea is good
14:04:53 <slaweq> ralonsoh: yes, but we can still talk about it :)
14:04:58 <ralonsoh> but instead of changing the current parameter
14:05:17 <ralonsoh> IMO, we can add another one with the expected value
14:05:34 <ralonsoh> --> total amount of IP availables in the pools
14:05:35 <slaweq> ralonsoh: it's exactly what was proposed in comment #8 there
14:05:43 <slaweq> to add "available_ips" value
14:05:47 <slaweq> as new one
14:06:01 <amotoki> sorry for late
14:06:13 <ralonsoh> slaweq, yes, I know
14:06:25 <ralonsoh> that's what, IMO, this should be
14:06:27 <slaweq> hi amotoki
14:06:46 <slaweq> let's see what others think
14:07:40 <njohnston> I think what ralonsoh is saying makes sense, although I do not feel strongly about that
14:08:08 <amotoki> I think we can add some fields to cover allocation pools is different from defaulta as we discussed before.
14:08:46 <amotoki> alloc pool size, available IPs in the pool
14:09:07 <ralonsoh> exactly
14:11:26 <slaweq> ok, so we all agree that we should left "total_ips" value as it is now, to represent all IPs in subnet, and add new value which will reflect IPs available in defined allocation pools, is that correct summary?
14:11:59 <ralonsoh> I think so
14:12:00 <amotoki> I think the size of the allocation pool(s) is also needed
14:12:38 <amotoki> as used_ips counts IPs from both inside and outside of the allocation pools.
14:12:50 <ralonsoh> the amount of total IPs in the pools, yes, that's easy to be calculated and useful
14:13:11 <slaweq> amotoki: so something like "total_allocation_pool_ips" and "used_allocation_pool_ips"?
14:13:23 <amotoki> slaweq: exactly
14:13:31 <slaweq> amotoki: makes sense for me
14:13:39 <amotoki> it will allow us to know the IP allocation status clearly
14:14:36 <amotoki> it means the combos of (used, total) x (all under subnet, allocation pools)
14:15:10 <slaweq> yamamoto: mlavalle haleyb: what do You think about it?
14:15:29 <mlavalle> mhhhh
14:15:43 <mlavalle> are we proposing a new extension?
14:16:13 <slaweq> I think it will require new extension to make this api change discoverable
14:16:28 <mlavalle> and leaving the current extension unchanged?
14:18:32 <slaweq> IMO it's the way how we are introducing changes in API, so yes
14:19:25 <mlavalle> the reason I'm asking these questions is that reading through the discussion, I see that we have oscillated from "invalid", to this is a bug to a new extension
14:19:32 <njohnston> At the denver PTG we talked about adding versions for extensions specifically because adding an extension just to add an attribute is excessive.  Did we drop that idea?
14:20:00 <amotoki> let me ask one question: is the current "total_ips" the size of allocation pools or the subnet CIDR size?
14:20:37 <amotoki> my understanding is that total_ips is the size of allocation pools
14:20:42 <slaweq> amotoki: I didn't check that by self but from bug report it seems that it's size of CIDR
14:20:50 <slaweq> amotoki: let me check that
14:21:32 <ralonsoh> the size of the CIDR is trivial, isn't it?
14:22:22 <amotoki> After stack.sh, I see total_ips='253' for /24 subnet, so I believe it means the size of allocation pools
14:22:26 <slaweq> amotoki: it's size of allocation pools
14:22:53 <amotoki> one issue we need to fix in the current API is total_ips with an empty allocation_pools.
14:23:00 <amotoki> this is apparently a bug.
14:23:09 <mlavalle> here's the official doc: https://docs.openstack.org/api-ref/network/v2/index.html?expanded=list-network-ip-availability-detail#network-ip-availability-and-usage-stats
14:23:54 <mlavalle> total_ips == The total number of IP addresses in a network
14:24:13 <mlavalle> this is what our public commitment is about this API
14:24:41 <amotoki> mlavalle: yes, but it is different from what we have now....
14:25:11 <slaweq> hmm, so it seems for me like a bug
14:25:30 <slaweq> when I created subnet with allocation pool specified, I had in total_ips number of ips from this allocation pool
14:25:32 <amotoki> the question is "do we change the behavior of total_ips to follow the API reference?"
14:25:48 <amotoki> or "do we update the API reference to sync the current behavior?"
14:25:50 <slaweq> but when I deleted allocation pool, I got total_ips=254 (size of cidr)
14:25:55 <mlavalle> exactly, we are getting to the thorny part of this
14:26:25 <mlavalle> that is why I want to untangle the bug side of this from the need / use case of the submitter
14:27:26 <mlavalle> amotok'is comment suggests that the API is not actually behaving the way the doc states
14:27:29 <mlavalle> right?
14:27:45 <amotoki> mlavalle: yes
14:27:59 <amotoki> I think total_ips shows the size of allocation pools.
14:28:26 <mlavalle> but on the other hand, this API is not new
14:28:35 <mlavalle> it's been in the wild for a long time
14:28:46 <liuyulong_> I may mention a ninteresting issue, if user's subnet CIDR is /27, then the total IPs will be 2, and the config dhcp_agents_per_network = 3 or more, bad things may happen.
14:29:22 <amotoki> liuyulong_: yeah, that's the confusing point of this API.
14:29:25 <amotoki> according to a patch the bug author submitted, it introduces a new field, so I guess the bug author would like to have more consistent information in the API response.
14:29:32 <mlavalle> so either nobody is using it and we can adjust the bahavior to the doc or we potentially are going to break deployers that have tooling based on the actual bahavior
14:29:36 <slaweq> amotoki: I'm not exactly sure about that - according to my test now it seems that it should number of IPs inside allocation pools when pools are defined and total number of IPs from cidr(s) when allocaton pools are not defined
14:30:04 <amotoki> slaweq: okay, my memory might be wrong....
14:30:38 <liuyulong_> sorry, it should be /31
14:31:13 <mlavalle> slaweq: but that behavior you are seeing might be factored in in deployers tooling, right?
14:31:36 <slaweq> mlavalle: it might be, that's true
14:32:00 <slaweq> so the best here would be to have microversioning and fix it in new version :P
14:32:21 <slaweq> (or fix it and add new extension :P)
14:32:58 <mlavalle> yes add a new extension, that might behave in contradictory ways
14:33:09 <mlavalle> which might be the only way out
14:33:20 <amotoki> what I propose now are (1) to update the API reference to match the current API and (2) to improve the missing point of the current API  if any (perhaps we have).
14:33:25 <mlavalle> I just want us to have clarity of the situation
14:33:52 <mlavalle> so we can communicate it properly to the community
14:35:03 <mlavalle> even if this means "hey community, we meesed with the first API but we are not going to change the behavior becuase you might rely on it. Here's a new extension with a differente (and potentially contradictory) behavior"
14:35:54 <slaweq> mlavalle: yes, we will probably need something like that to deal with this issue
14:36:47 <slaweq> ok, so here is what I propose for now:
14:36:54 <mlavalle> amotoki: and yes, let's adjust the doc to reflect, in extreme precision, what the actual bahavior is
14:37:17 <slaweq> next week I will check exactly current, real behaviour of this API and will describe it in comment to this bug
14:37:34 <mlavalle> great, I was going to propose the same thing
14:37:44 <slaweq> I will also try to highligh how it is different (if it is) from current api-ref
14:37:50 <mlavalle> I am also going to play with the extension and leave a note in the RFE
14:38:05 <slaweq> and next week can check this and decide what to do
14:38:09 <amotoki> sounds perfect
14:38:15 <slaweq> ok for You?
14:38:20 <mlavalle> +1
14:38:23 <yamamoto> +1
14:38:37 <amotoki> +1
14:38:45 <ralonsoh> +1
14:38:51 <njohnston> +1
14:39:03 <yamamoto> is it common to use a pool size so different from cidr size?
14:39:18 <mlavalle> we don't know
14:39:37 <amotoki> me neither :p
14:39:49 <haleyb> +1
14:39:55 <mlavalle> I mean, we don't have a factual foundation to answer that accross the community
14:40:09 <mlavalle> in fact, one step we can add to this is
14:40:16 <njohnston> ideally we would be size agnostic
14:40:19 <mlavalle> why don't we ask in the ML?
14:41:00 <amotoki> njohnston: what do you mean by "size agnostic"? could you clarify?
14:41:27 <slaweq> mlavalle: do You mean to ask about how common it is to use allocation pools smaller than cidr or what?
14:42:03 <mlavalle> that can be one question. the other question is how is this API doc being interpreted and used
14:42:37 <slaweq> mlavalle: can You send such email to ML than?
14:42:43 <mlavalle> sure
14:42:55 <slaweq> mlavalle: thx a lot
14:43:35 <mlavalle> I will probably do some testing before barfing a stupid question on the ML
14:43:57 <slaweq> sure :)
14:44:02 <amotoki> as my hat of a user, I usually read the API reference and then check the real behavior, so I think it is enough to clarify the behavior in the API refernce.
14:44:22 <amotoki> I am not sure what we are asking in a ML thread.
14:45:06 <yamamoto> did the bug submitter explain his use case?
14:45:37 <mlavalle> yamamoto: maybe we haven't clarifed enough yet....
14:45:49 <yamamoto> to me it looks like he's trying something interesting with the extension
14:46:14 <mlavalle> that is why I want to untangle it from the potential bug / shortcoming in docs side of it
14:47:05 <yamamoto> or maybe he is just testing corner cases without use cases
14:47:29 <mlavalle> that may very well be the case
14:48:30 <slaweq> ok, I think we have some action plan for this one
14:48:37 <slaweq> so we can continue
14:48:40 <slaweq> correct?
14:48:44 <mlavalle> amotoki: and what really raises a concern in my mind is the fact that even though we might have a mismatch between doc and actual behavior, that has been like that for along time
14:49:01 <mlavalle> and nobody has complained about it
14:49:10 <mlavalle> other that this submitter
14:49:39 <slaweq> tbh I didn't even know about this extension before this bug ;)
14:50:19 <amotoki> mlavalle: that's true. An operator I was involved write a custom tool as the API is not sufficient, so at least I have one usecase. It was not just reported :(
14:51:00 <mlavalle> I'm done with this RFE for the time being
14:51:16 <slaweq> ok, lets go quickly to look at next one
14:51:21 <slaweq> https://bugs.launchpad.net/neutron/+bug/1821208
14:51:21 <openstack> Launchpad bug 1821208 in neutron "[RFE] Only enforce policy when selected option does not match default" [Wishlist,Confirmed]
14:51:48 <slaweq> I wanted to get back to this as njohnston answered to some questions there already
14:53:19 <njohnston> ah yes, this one
14:53:58 <slaweq> njohnston: quite old one, right? :)
14:54:21 <mlavalle> what if the deployer is not using the default set of policies?
14:55:26 <mlavalle> how is that going to play with "A data structure would need to be created from the policy-processing code that matches policy names with their respective default values."?
14:55:39 <njohnston> It should probably be phrased "Only enforce policy when selected option does not match (default | supplied policy config)"
14:56:29 <amotoki> njohnston: I am okay if the new behavior is limited to attributes visible to users.
14:56:44 <njohnston> amotoki: that makes sense
14:57:35 <amotoki> I need to remember the full context. let me check and reply in the bug.
14:58:08 <slaweq> amotoki: sure, so lets revive this spec during this week, and reply in comments there
14:58:14 <njohnston> amotoki: Thanks.  And it looks like the horizon behavior that spawned this conversation is still unfixed: https://bugs.launchpad.net/horizon/+bug/1841050
14:58:14 <openstack> Launchpad bug 1841050 in OpenStack Dashboard (Horizon) "Horizon network port create panel shows "port security" checkbox that breaks port creation for non-admin users " [Undecided,Confirmed] - Assigned to Radomir Dopieralski (deshipu)
14:58:21 <slaweq> we can get back to it on one of next meetings
14:58:51 <njohnston> sounds good slaweq, the urgency is low
14:58:58 <amotoki> sounds good
14:58:58 <slaweq> njohnston: I though that :)
14:59:09 <slaweq> ok, so it seems we have a plan
14:59:13 <slaweq> thx for attending
14:59:23 <slaweq> and have a great weekend
14:59:27 <amotoki> have a good weekend!
14:59:27 <mlavalle> o/
14:59:29 <slaweq> o/
14:59:30 <ralonsoh> bye
14:59:30 <yamamoto> good night
14:59:36 <slaweq> #endmeeting