Skip to content

WIP: Support for mixed protocol LB services #880

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

WIP: Support for mixed protocol LB services #880

wants to merge 10 commits into from
+553 −269

Conversation

justinsb
Copy link
Member

  • Add doc for MixedProtocolLBService support
  • fixup! Add doc for MixedProtocolLBService support
  • controller support for mixed protocol LB services

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 25, 2025
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If the repository mantainers determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 25, 2025
@k8s-ci-robot k8s-ci-robot requested review from cici37 and seans3 July 25, 2025 00:51
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: justinsb
Once this PR has been reviewed and has the lgtm label, please assign cheftako for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@justinsb justinsb force-pushed the mixedprotocol_lbservice branch 2 times, most recently from 8d02ff5 to 8e75fbe Compare July 25, 2025 02:10
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 25, 2025
@justinsb justinsb force-pushed the mixedprotocol_lbservice branch 2 times, most recently from f5a5bf6 to 6394dd8 Compare July 25, 2025 02:28
@justinsb justinsb force-pushed the mixedprotocol_lbservice branch from 6394dd8 to 3cda00c Compare July 25, 2025 02:38
@justinsb justinsb force-pushed the mixedprotocol_lbservice branch from 8bac30d to ec865ef Compare July 25, 2025 03:16
aojea
aojea reviewed Jul 25, 2025
View reviewed changes
docs/design-docs/multi-protocol-cidr.md
@@ -66,6 +66,14 @@ The necessary changes would be localized within the GCP-specific implementation
* gce.EnsureLoadBalancerDeleted: This function would need to be updated to iterate through all potential forwarding rules based on the new naming scheme and delete them.
* **Resource Naming Functions:** Helper functions that generate names for GCP resources would need to be adapted to produce the protocol-specific forwarding rule names.

### 5. Production Readiness

For ease of introduction, we will implement a feature flag for the support. If the feature flag is not set,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is not really an error , is it? the status gets updated to indicate one protocol is not supported

aojea
aojea reviewed Jul 25, 2025
View reviewed changes
providers/gce/gce_loadbalancer_external.go Outdated
if err != nil {
return nil, err
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic is super complex that it is better to use unit tests to avoid issues

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely - I'm planning on breaking this down into manageable steps, and I think most of those will be about making the code simpler.

@justinsb justinsb force-pushed the mixedprotocol_lbservice branch from ec865ef to 476dead Compare July 25, 2025 12:24
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 25, 2025
@justinsb justinsb force-pushed the mixedprotocol_lbservice branch 2 times, most recently from 13c400d to 47a2c6f Compare July 25, 2025 12:49
@justinsb justinsb force-pushed the mixedprotocol_lbservice branch from 47a2c6f to 796dc40 Compare July 25, 2025 12:51
@k8s-ci-robot
Copy link
Contributor

@justinsb: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
cloud-provider-gcp-verify-up-to-date 796dc40 link true /test cloud-provider-gcp-verify-up-to-date
cloud-provider-gcp-tests 796dc40 link true /test cloud-provider-gcp-tests
cloud-provider-gcp-verify-all 796dc40 link true /test cloud-provider-gcp-verify-all
pull-cloud-provider-gcp-scenario-kops-simple 796dc40 link false /test pull-cloud-provider-gcp-scenario-kops-simple
cloud-provider-gcp-e2e-full 796dc40 link true /test cloud-provider-gcp-e2e-full

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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-sigs/prow repository. I understand the commands that are listed here.

@justinsb justinsb mentioned this pull request Jul 25, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aojea aojea aojea left review comments

@cici37 cici37 Awaiting requested review from cici37

@seans3 seans3 Awaiting requested review from seans3

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@justinsb @k8s-ci-robot @aojea