Support mixed protocol LBs

[kiall]

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR adds support for configuring a service of type LoadBalancer with more than 1 protocol. For example, this allows configuring a LoadBalancer with both TCP and UDP port 53 (for a DNS server), or TCP and UDP 443 for HTTPS + QUIC (HTTP 3.0).

Which issue(s) this PR fixes:

Fixes #23880

Special notes for your reviewer:

Mixed protocol LBs are supported by Azure and MetalLB. Other providers MAY support this, however, I'm unable to verify the implementations and have opted to reject mixed protocol LBs on these CPIs where necessary (this turned out to only need rejection in the GCP provider)

An example service to enable and use this feature:

apiVersion: v1
kind: Service
metadata:
  name: mixed-protocol
spec:
  type: LoadBalancer
  ports:
    - name: dns-udp
      port: 53
      protocol: UDP
    - name: dns-tcp
      port: 53
      protocol: TCP
  selector:
    app: my-dns-server

If the feature gate is enabled, while a CPI without support is enabled, the following event will be recorded:

 Type     Reason                      Age                   From                Message
 ----     ------                      ----                  ----                -------
 Warning  CreatingLoadBalancerFailed  112s (x71 over 32m)   service-controller  Error creating load balancer (will retry): failed to ensure load balancer for service default/mixed-protocol: Cannot create an GCP load balancer with mixed protocols

Does this PR introduce a user-facing change?:

Added support for mixed protocol services of type=LoadBalancer, enabled through the use of a new `ServiceLoadBalancerMixedProtocol` feature gate.

[k8s-ci-robot]

Hi @kiall. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[grahamhayes]

/ok-to-test

[itsc0lin]

would welcome support for this!

[mattkeeler]

It would be great to have support for mixed protocol LBs.

[andyzhangx]

@kiall any progress on this PR?

[alvaroaleman]

Can you expand a little on the reasoning for an annotation? A check if a Service contains ports for more than one protocol shouldn't be too hard to write and could be used as a basis for having providers that do not support that emit an event. Or are there more reasons for having it?

[kiall]

Really, I think the annotation should be little more than a stopgap measure to handle compatibility and allow people (i.e. CPI vendors) time to update - I gave some detail in the PR message above.

[feiskyer]

I decided to make this feature opt-in via the annotation, however I believe that is the wrong approach long term.

For new features, it's preferable to add a feature gate instead of annotations. For the first release, it may be alpha so that users can try it. And in later beta releases, enable it by default.

[alvaroaleman]

@kiall Do you currently have the capacity to continue with this? Else I'd like to go ahead and write a KEP for the validation changes to make sure we manage to get this in within the 1.17 cycle

[Hyzhou]

Any update? Some game developer deploy their application used mixed protocols. when use the k8s to deploy it, mixed protocols was not supported in service.
If not to merge this PR. If we have another way to support this feature.

[adampl]

Does it really need a KEP and a feature gate? Looks more like a bugfix to me.

[dcbw]

Tim says it was originally disabled because there were possibilities for "being charged twice" depending on your cloud provider. Also Tim wants it to be opt-in.

We need a KEP for this first to explore the issues and proposed solutions.

[k8s-ci-robot]

@kiall: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[adampl]

But "being charged twice" is possible only if someone enabled this in their config, so what's the problem? And who cares that some cloud provider can't do their job right?

[DreamRivulet]

Any update on this? without this, we have to create several service for each port, which is inconvenient.

[xiaoxubeii]

Any updates? We also really need mixed protocal.

[jrx-sjg]

Oh my god, can anyone solve the conflicts? We really need mixed protocols for our SIP infrastructure.

[janosi]

The KEP pull request is on its way to kubernetes/enhancements. Please check and comment.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[prestonvanloon]

/remove-lifecycle rotten

[alvaroaleman]

@prestonvanloon I don't think there is much point in keeping this PR open, there is a KEP for this, discussion should continue there: kubernetes/enhancements#1438 or the issue at #23880

[prestonvanloon]

OK thanks. I hadn't seen those links yet.

…

[cmluciano]

Let's continue discussion in #23880 since there is now a merged KEP added

/close

[k8s-ci-robot]

@cmluciano: Closed this PR.

Details

In response to this:

Let's continue discussion in #23880 since there is now a merged KEP added

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.