Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Publish service does not account for separate TCP and UDP LBs #2544

Closed
3 tasks
rainest opened this issue Jun 6, 2022 · 3 comments · Fixed by #3325
Closed
3 tasks

Publish service does not account for separate TCP and UDP LBs #2544

rainest opened this issue Jun 6, 2022 · 3 comments · Fixed by #3325
Assignees
Labels
area/feature New feature or request
Milestone

Comments

@rainest
Copy link
Contributor

rainest commented Jun 6, 2022

Kubernetes does not have GA mixed protocol LoadBalancers. As of 1.24, this is available as a gated beta.

We support resources that use TCP or UDP at L4. In the chart, we provide a separate section to configure the UDP LB.

In the controller, we use a single publish service to determine the proper IP for all resource statuses regardless of transport. This address will only be correct for one of TCP or UDP, not both.

We should add a new argument for the UDP publish service and reworking status updaters (including the Gateway status updater/Kong config parser) to use the appropriate address by protocol. This likely will not change when mixed protocol is GA, since support is vendor-dependent and some LB providers will continue supporting only single-protocol LBs.

Acceptance:

  • KIC provides a CLI argument to specify the UDP publish service.
  • KIC provides a CLI argument to specify the UDP publish status addresses. When present, this takes precedence over the service argument.
  • If there are no UDP service/address arguments set, KIC uses the regular publish service for UDP routes. The CLI documentation indicates this.
@czeslavo
Copy link
Contributor

As it's a blocker for the #3237, I'm adding this to the 2.9.0 milestone.

@mflendrich
Copy link
Contributor

Moving to "Awaiting AC & Prioritization" to scope this out.

@rainest
Copy link
Contributor Author

rainest commented Jan 4, 2023

Added AC; prioritization is kinda "whatever" since it's pretty simple to write--may as well just do it now.

I actually don't know if we want to fall back to the default service (which is effectively what we are currently doing--we use it for UDPIngress even though we have no idea if it's correct). This effectively assumes a dual-transport LB, which is probably fairly uncommon because of how new support for those is.

Do we want to instead require explicitly setting UDP config if you want status info for them? If you do have dual-transport, you'd need to provide the same value twice in the default and UDP arguments. If you omit UDP configuration, we'd just not set UDPIngress status. Not sure how we'd handle Gateway binding if you omit it--we could either bind unconditionally like we are now, or disable binding if you omit status configuration, which feels clunky.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants