Remove route agent "submariner.io/cniIfaceIp" node annotation #3010
Conversation
...as it's used by components outside of the route agent Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
tpantelis
requested review from
Oats87,
skitt,
sridhargaddam and
vthapar
as code owners
|
🤖 Created branch: z_pr3010/tpantelis/cni_iface_ip |
Re: submariner-io/submariner#3010 Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
Re: submariner-io/submariner#3010 Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
| @@ -52,21 +52,34 @@ func NewNodeController(config *syncer.ResourceSyncerConfig, pool *ipam.IPPool, n | |||
| nodeName: nodeName, | |||
| } | |||
|
|
|||
| clusterCIDR := "" | |||
| if len(clusterCIDRs) > 0 { | |||
There was a problem hiding this comment.
Don't we want to report an error if len(clusterCIDRs) isn't bigger than 0?
There was a problem hiding this comment.
yeah we could although the operator would never pass empty cluster CIDRs anyway. I have a follow-up PR to check all clusterCIDRs so this check will go away anyway.
| @@ -61,7 +63,7 @@ const ( | |||
| globalIngressIPName = "nginx-ingress-ip" | |||
| kubeProxyIPTableChainName = "KUBE-SVC-Y7DIXXI5PNAUV7FB" | |||
| serviceName = "nginx" | |||
| cniInterfaceIP = "10.20.30.40" | |||
| cniInterfaceIP = "169.254.1.50" | |||
There was a problem hiding this comment.
Why do we need to change 'cniInterfaceIP' value ?
cniInterfaceIP shouldn't be from globalIP cidr range
There was a problem hiding this comment.
I changed it to be in range of localCIDR b/c originally I was checking for that in the fake cni.DiscoverFunc like the real one does but I later changed cni.DiscoverFunc to just verify that the configured local CIDR is passed in. localCIDR is also used as the global CIDR which actually doesn't really matter for the tests but logically should be defined separately.
| @@ -86,56 +81,3 @@ func discover(clusterCIDR string) (*Interface, error) { | |||
|
|
|||
| return nil, errors.Errorf("unable to find CNI Interface on the host which has IP from %q", clusterCIDR) | |||
| } | |||
|
|
|||
| func AnnotateNodeWithCNIInterfaceIP(nodeName string, clientSet kubernetes.Interface, clusterCidr []string) error { | |||
There was a problem hiding this comment.
I assume that after upgrade we'll have orphaned/useless cni IP address annotation that won't be deleted ,
do we want to handle it ?
There was a problem hiding this comment.
We can't remove it b/c then we'd need update permission for nodes which we're removing. I don't see any way we can handle it other than to add a release note. It's just an annotation so doesn't hurt anything.
There was a problem hiding this comment.
Yep, adding release note sounds reasonable.
...rather than retrieving the CNI IP from the route agent Node annotation. Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
...as globalnet no longer uses it. Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
It checks if the health check IP is set on Endpoint and, if not, it doesn't verify the latency info but that could also be due to an issue in the gateway and/or globalnet with determining the IP. Instead parse the env vars in the gateway pod to determine if the health check var is present and enabled. If so then expect the health check IP to be set. Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
|
🤖 Closed branches: [z_pr3010/tpantelis/cni_iface_ip] |
Re: submariner-io/submariner#3010 Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
Re: submariner-io/submariner#3010 Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
Re: submariner-io/submariner#3010 Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
Release notes for submariner-io/submariner#3010 Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
Release notes for submariner-io/submariner#3010 Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
Release notes for submariner-io/submariner#3010 Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
Release notes for submariner-io/submariner#3010 Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
The route agent discovers the CNI IP and adds the annotation to the node for use by Globalnet. However granting RBAC permission to update nodes poses a potential vulnerability exploitation by attackers. We can eliminate this annotation and the RBAC permission by having Globalnet discover the CNI IP itself.
See commits for details.