| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-02-09 | |||
| 08:09:01 | sean-k-mooney | :) | |
| 08:09:44 | sean-k-mooney | im +1 on the first two patches and the last 1 and have a question in the 3rd one but nothing minor | |
| 08:10:00 | sean-k-mooney | so hopefully we can get that feature done this week | |
| 08:11:12 | gibi | I hope too, it was a long drag | |
| 08:23:47 | bauzas | gibi: sean-k-mooney: hola | |
| 08:24:04 | sean-k-mooney | bauzas: o/ | |
| 08:24:13 | bauzas | yeah, having IPs from different segments for the same port looked to me weird | |
| 08:24:22 | bauzas | but, alas. | |
| 08:24:55 | bauzas | I've been teached that there are no limits to the creativity of a fooled brain, if this brain works on networking | |
| 08:25:03 | gibi | :) | |
| 08:25:23 | bauzas | anyway, there is a FIXME, I think we're good with this | |
| 08:26:01 | sean-k-mooney | yep | |
| 08:26:14 | bauzas | sean-k-mooney: you said you have a question in the patch, looking | |
| 08:26:23 | sean-k-mooney | multiple ips is fine but multiple ips from different segments not so much | |
| 08:26:30 | sean-k-mooney | patch 3 | |
| 08:26:54 | sean-k-mooney | the get segments by subnet function if i rememebr correctly | |
| 08:27:16 | bauzas | sean-k-mooney: multiple IPs is totally fine | |
| 08:27:21 | bauzas | think of v4 and v6 | |
| 08:27:44 | bauzas | but, indeed, from two different segments, really ? | |
| 08:27:47 | sean-k-mooney | there are cases where it is not. e.g. if they are form different segments on the same port and its not a trunk | |
| 08:28:13 | bauzas | for those cases, please refer to my first comment of the morning here | |
| 08:28:24 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/773976/3/nova/network/neutron.py#3543 | |
| 08:28:30 | bauzas | I'm totally OK with excluding those usecases at first | |
| 08:28:37 | sean-k-mooney | yep | |
| 08:28:52 | bauzas | as I don't pretend to be able to solve all the world problems, including starvation and climate change | |
| 08:29:24 | bauzas | even if I appreciate people think I could act this way | |
| 08:30:07 | sean-k-mooney | basically my question on the patch is are we sure its valid to assume that if the segment extention is enabled all networks have segments | |
| 08:30:31 | sean-k-mooney | im not sure that is correct so im not sure raisign an exception when we dont find them is correct | |
| 08:30:34 | bauzas | if not, this would be an operational issue, nope ? | |
| 08:30:38 | sean-k-mooney | unless you are catching that | |
| 08:30:54 | sean-k-mooney | not nessisarly | |
| 08:30:55 | bauzas | https://docs.openstack.org/neutron/latest/admin/config-routed-networks.html | |
| 08:31:05 | bauzas | the docs says you have to segment | |
| 08:31:17 | sean-k-mooney | it would depend on if you had a mix of tenant networks and routed provider networks in teh same cloud | |
| 08:31:29 | bauzas | I need coffee | |
| 08:31:42 | sean-k-mooney | in any one netwok you have to either have all subnets connect to segments or none | |
| 08:31:43 | bauzas | because I think I'll suffer headache very soon | |
| 08:32:03 | bauzas | sean-k-mooney: back up a littler | |
| 08:32:13 | bauzas | sean-k-mooney: if you configure routed networks | |
| 08:32:13 | sean-k-mooney | if you have 2 networks 1 can be routed and the other not | |
| 08:32:17 | bauzas | right | |
| 08:32:36 | bauzas | technically, you can have net1 with segments and net2 with not | |
| 08:32:43 | sean-k-mooney | yep | |
| 08:32:52 | bauzas | hmmm | |
| 08:33:07 | sean-k-mooney | so i dont think we should be raising excpeiton if we dont find segments or networks | |
| 08:33:13 | bauzas | sean-k-mooney: indeed | |
| 08:33:19 | sean-k-mooney | unless you catch it | |
| 08:33:22 | bauzas | that's a good call | |
| 08:33:35 | bauzas | that said | |
| 08:33:51 | bauzas | we only call this method on the port or network that was provided by the user | |
| 08:34:11 | bauzas | but, as this is a user, they have no context whether the net is routed or not | |
| 08:34:25 | sean-k-mooney | mostly correct | |
| 08:34:30 | sean-k-mooney | they dont | |
| 08:34:38 | sean-k-mooney | but routed networks can only be created by admins | |
| 08:34:43 | bauzas | I know | |
| 08:34:49 | sean-k-mooney | so the end user just knows its a shared network | |
| 08:34:53 | bauzas | right | |
| 08:34:55 | sean-k-mooney | or one the admin made for them | |
| 08:35:15 | bauzas | so, you can end up with indeed asking for net2 which isn't configured | |
| 08:35:17 | sean-k-mooney | if they create there own networks in addtion to that they will not be routed | |
| 08:35:25 | bauzas | and in this case, we don't wanna fail | |
| 08:35:30 | sean-k-mooney | yep | |
| 08:35:44 | bauzas | we just won't require aggregates | |
| 08:35:48 | sean-k-mooney | we want to say oke we found no segments so this is not routed so all hosts are vlaid | |
| 08:35:59 | sean-k-mooney | so you could catch the exception at the top of the filter | |
| 08:36:10 | sean-k-mooney | and return true with no update to the placment query | |
| 08:36:18 | bauzas | yes and no, I have the choice | |
| 08:36:28 | sean-k-mooney | yes you do | |
| 08:36:51 | bauzas | either we consider this utils method should raise an exception as this is purely for routed networks | |
| 08:37:04 | bauzas | (and then, agreed, we have to catch the exception in the filter) | |
| 08:37:13 | sean-k-mooney | or it returns []/None | |
| 08:37:32 | sean-k-mooney | as if the extention was disabled | |
| 08:37:40 | bauzas | or, we consider this method as general, and yeah None or an empty list sounds reasonable | |
| 08:37:54 | bauzas | yeah, that's why I'm on the latter side | |
| 08:37:56 | sean-k-mooney | yep i personaly prefer avoiding excpetions | |
| 08:38:18 | sean-k-mooney | since this is not exceptional behavior | |
| 08:38:26 | sean-k-mooney | its a perfectly vaild outcome | |
| 08:38:52 | sean-k-mooney | for what its worth this is the only really question i had left on the seriese the rest looks good to me | |
| 08:38:54 | bauzas | last call, singular/plural on the output | |
| 08:39:24 | sean-k-mooney | well for get a segment by subnet its singualar as i noted in the comment | |
| 08:39:30 | sean-k-mooney | networks can be plural | |
| 08:39:38 | sean-k-mooney | you can retun both as list as you do now | |
| 08:39:47 | sean-k-mooney | or you can simply the other path as i suggested | |
| 08:39:58 | bauzas | well, ok | |
| 08:40:37 | bauzas | gibi: I won't provide a FUP, please bear with me with a second review loop when I'm done with sean-k-mooney's comments | |
| 08:41:59 | gibi | bauzas: ack | |
| 08:42:02 | sean-k-mooney | gibi: do you have any preferecne for ^ i dont really so ill leave it to bauzas to desice between the two choiced excption/return []/None and list or no list for get segment by subnet | |
| 08:42:04 | gibi | no problemo | |
| 08:42:23 | bauzas | no exception, I decided :) | |
| 08:42:24 | gibi | sean-k-mooney: I have to run, but I will read back before lunch | |
| 08:42:33 | sean-k-mooney | bauzas: works for me | |
| 08:42:34 | gibi | bauzas: as you see fit :) | |
| 08:43:26 | sean-k-mooney | bauzas: if you are removing the excpetion you might want to make the LOG.error a LOG.debug | |
| 08:44:23 | sean-k-mooney | anyway cool im going to get coffee | |
| 08:55:45 | bauzas | sean-k-mooney: quick call, I'm touching the get_network too but what gives you /segments?network_id=net1 if net1 isn't configured ? | |
| 08:55:55 | bauzas | I suppose an empty list ? | |
| 08:56:11 | bauzas | context https://review.opendev.org/c/openstack/nova/+/773976/3/nova/network/neutron.py#3523 | |
| 08:57:19 | bauzas | I don't have a routed networks env at hand | |
| 09:03:04 | sean-k-mooney | sure we can have a call just need to get my headset | |
| 09:03:57 | sean-k-mooney | oh your asking what is the result | |