ServiceExternalIP: Allow multiple services sharing one IP

[meibensteiner]

Describe the problem/challenge you have
Our production environment requires some services to share a single IP, which is currently not possible and is the single reason why we wouldnt be able to remove MetalLB altogether and just use Antrea with the ServiceExternalIP feature flag.

Describe the solution you'd like
A similar solution to MetalLB would be nice which is a simple annotation on the service: metallb.universe.tf/allow-shared-ip

Anything else you would like to add?
Im currently looking into proxying those requests to allow IP sharing, but a native solution in Antrea would be prefered.

[tnqn]

Thanks @meibensteiner for the proposal. I think technically it would work if the services use different ports. We also received another feature request for sharing same IP for Egress and Service LoadBalancerIP(#3334). Actually if we specify same IP for multiple Egresses, it should be already working. Then I'm thinking whether we should just adjust the IP binding strategy a little: instead of making one external IP can only be bound with either one Egress or one Service, we could allow specifying same IP for multiple objects regardless of Egresses and Services, to address both requests. For Egresses and Services that don't specify IPs explictly, they would still be allocated an IP not in use.

This would be how to share IPs for services:

apiVersion: v1
kind: Service
metadata:
  name: my-service1
  annotations:
    service.antrea.io/external-ip-pool: "service-external-ip-pool"
spec:
  selector:
    app: MyApp1
  loadBalancerIP: "10.10.0.2"
  ports:
    - protocol: TCP
      port: 80
      targetPort: 9376
  type: LoadBalancer
---
apiVersion: v1
kind: Service
metadata:
  name: my-service2
  annotations:
    service.antrea.io/external-ip-pool: "service-external-ip-pool"
spec:
  selector:
    app: MyApp2
  loadBalancerIP: "10.10.0.2"
  ports:
    - protocol: TCP
      port: 8080
      targetPort: 9376
  type: LoadBalancer

Does the usage make sense to you? or annotation like "metallb.universe.tf/allow-shared-ip" is expected for some reasons? @meibensteiner
cc @jianjuns @antoninbas @xliuxu for opinions too.

[antoninbas]

The value of the annotation metallb.universe.tf/allow-shared-ip is unclear to me. I thought it was a means to enable IP sharing without having to explicitly provide an IP, but that's not really the case:

If these conditions are satisfied, MetalLB may colocate the two services on the same IP, but does not have to.

(from https://metallb.universe.tf/usage/#ip-address-sharing)

I am fine with Quan's proposal. Do we plan to have some sort of validation if the user mistakenly provides the same IP AND port for 2 Services?

[xliuxu]

+1 for Quan's proposal. Something in my mind:

1. I recall that for Egress we need to assign the external IP to the dummy interface and it conflicts with the implementation of ServiceExternalP. So I think it is not easy to share IP for Egress and Service.
2. What is the expected behavior if Service1 does not have a loadBalancerIP set in the spec, but user sets Service2 with the same IP to the spec.loadBalancerIP which has been allocated to Service1?

[meibensteiner]

@tnqn No. I always considered that metallb annotation to be unnecessary. I thought I would bring it up to illustrate my use case.

Did I read this correctly and multiple services using one IP is already possible? I thought I tried it and was unsuccessful because the IP was already claimed. I will try again and circle back.

[xliuxu]

@meibensteiner Sharing IP for multiple services is not possible for the moment. We need to finalize the API design and implement it in future releases.

[tnqn]

@meibensteiner I meant multiple Egresses using one IP should work today. Egress is another Antrea feature manging egress traffic SNATing, which also consumes the external IP pool.

[tnqn]

I am fine with Quan's proposal. Do we plan to have some sort of validation if the user mistakenly provides the same IP AND port for 2 Services?

Yes, if you don't mean validatingWebhook for Service object, I think we could validate whether the services can share an IP and use ServiceStatus and events assosicated with the Service to report error, for example:
When Service A and Service B are specified same LoadBalancer IP and have same port, if Service A is processed first it will get the LoadBalancer IP, and we do not set Service B's status.loadBalancer.ingress.ip but create an event for it to describe the conflict.

• I recall that for Egress we need to assign the external IP to the dummy interface and it conflicts with the implementation of ServiceExternalP. So I think it is not easy to share IP for Egress and Service.

Now I remember it, then probably we could start with support sharing IP among same kinds of objects first.

• What is the expected behavior if Service1 does not have a loadBalancerIP set in the spec, but user sets Service2 with the same IP to the spec.loadBalancerIP which has been allocated to Service1?

I think we don't have to care whether the specified IP was previously allocated automatically or manually. We could just allow specifying an IP that is already allocated to other Services when the new Service doesn't conflict with the others.

[vrabbi]

A side note, as i just saw it in the example from above.
The spec.loadBalancerIP field is deprecated as of k8s 1.24 and is planned to be removed in a few releases from core k8s. The recommendation from k8s community is to have an annotation for specifying the loadbalancer ip that is provider specific. May be worth adding such an annotation sooner then later into antrea, to give users more time before that field is removed
kubernetes/kubernetes#107235

[meibensteiner]

Another side note, a feature flag was introduced with k8s 1.20 which allows using different protocols (tcp&udp) on one LoadBalancer Service instance. Would be great if that was supported by antrea as well.
kubernetes/enhancements#1435

Not a dealbreaker however, since you could just create two services. One for udp, one for tcp.

[github-actions]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

[antoninbas]

Re-opening this, as it seems like a worthy issue. @tnqn ?

[tnqn]

Re-opening this, as it seems like a worthy issue. @tnqn ?

I agree. Let me add it to milestone 1.12 first and see if we can make it. Assign to me first.

[tnqn]

I have some code to support it but it's risky to include it in 1.12 which is expected to release this Wednesday. Move it to 1.13.

[github-actions]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

[github-actions]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

[github-actions]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

[lezruk]

Good afternoon @luolanzone , by looking at this conversation thread, it is not very clear was this milestone implmented already, dropped, or assigned to future release? Please suggest what is the current state?

[antoninbas]

@lezruk We have not dropped this feature but it has not been implemented it yet and we have not committed to a timeline. To emphasize, we are still planning to support this, which is why the issue has been kept open.

[tnqn]

Reminded the latest comments, I picked up the draft code I had for this support. I should be able to create a PR next week, hopefully we can get it into v2.1.