kakao / network-node-manager

network-node-manager is a kubernetes controller that controls the network configuration of a node to resolve network issues of kubernetes.
Apache License 2.0
108 stars 20 forks source link

Support spec.externalIPs and NodePort services #11

Closed bootc closed 3 years ago

bootc commented 3 years ago

Currently the controller only supports LoadBalancer services, and only handles the IP addresses found in the .status.loadBalancer.ingress field. The controller should also handle the IP addresses in the .spec.externalIPs field, which can also be used to direct traffic into a cluster.

Finally, because externalIPs can also be used with NodePort services, I feel that the controller should accept those too.

ssup2 commented 3 years ago

@bootc Thanks for your comment.

Now. k8s adds IPVS rules for spec.externalIPs to all nodes regardless of externalTrafficPolicy: Local. So network-node-manager doesn't neet to add rules for spec.externalIPs.

bootc commented 3 years ago

The problem I ran into is that on nodes that are not running pods, the service is unreachable. The rules inserted by this controller work around that.

ssup2 commented 3 years ago

@bootc I tested spec.externalIPs with the below service manifest in the v1.18.3 cluster.

apiVersion: v1
kind: Service
metadata:
  name: my-nginx-ipv4
  namespace: default
spec:
  ...
  externalTrafficPolicy: Local
  ipFamily: IPv4
  externalIPs:
    - 192.168.0.54
  ports:
  - port: 80
    protocol: TCP
    targetPort: 80
  selector:
    run: my-nginx
  sessionAffinity: None
  type: LoadBalancer
status:
  loadBalancer:
    ingress:
    - ip: 192.168.0.53

And I check the IPVS rules in the note that doesn't have my-nginx pod. Below is the IPVS rules.

...
TCP  192.168.0.53:80 rr
TCP  192.168.0.54:80 rr
  -> 10.0.1.16:80                 Masq    1      0          0
  -> 10.0.1.75:80                 Masq    1      0          0
...

As you can see, IPVS has a load balancing rule for the 192.168.0.54 address that in spec.externalIPs to the my-nginx pod. So I think that network-node-manager doesn't need to add rules for spec.externalIPs.

And I checked spec.externalIP works on any nodes with flannel CNI.

I think that if your app can't access `spec.externalIP', that's not IPVS issue.

bootc commented 3 years ago

Interesting. For me, on Kubernetes 1.20, the behaviour is the same for externalIP or LoadBalancer IPs. On nodes with active pods, I only have the pod IPs that are on the local node. And on nodes with no pods I have no entries at all.

I believe that in Kubernetes 1.21 this may be resolved by the addition of an internalTrafficPolicy field to services (https://github.com/kubernetes/kubernetes/pull/96600), so actually this can probably be closed, but I definitely don't see the behaviour as you describe it.

ssup2 commented 3 years ago

Please show your service info and result of the below command in the node that doesn't have the pod.

ipvsadm -L -n
bootc commented 3 years ago

I'm not sure why you don't believe me, but here is the service info on a node that isn't running a pod:

# ipvsadm -L -n -t 10.0.255.59:1883
Prot LocalAddress:Port Scheduler Flags
  -> RemoteAddress:Port           Forward Weight ActiveConn InActConn
TCP  10.0.255.59:1883 rr

And the same on a node running a pod:

# ipvsadm -L -n -t 10.0.255.59:1883
Prot LocalAddress:Port Scheduler Flags
  -> RemoteAddress:Port           Forward Weight ActiveConn InActConn
TCP  10.0.255.59:1883 rr
  -> 10.42.18.163:1883            Masq    1      0          0

This behaviour is identical for services using spec.externalIPs or status.loadBalancer.ingress[].ip, and also identical across IPv4 and IPv6. Clearly, this only applies for externalTrafficPolicy: Local services.

ssup2 commented 3 years ago

Sorry if you are offended. It wasn't that I couldn't believe you, but to keep track of the relevant history.

I think it depends on Kubernetes (kube-proxy) version. As your propose, it's ok to add rules for .spec.externalIPs.

Thanks. I will work.

ssup2 commented 3 years ago

@bootc I committed for this issue: https://github.com/kakao/network-node-manager/commit/d1368d3047d8f0b940acfefabd7f69357ac9faa6 And I tagging with your commit. Thanks :)