Skip to content

[feat] : add support for nodebalancers with UDP ports #387

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

Merged
merged 8 commits into from
May 28, 2025
Merged

Conversation

rahulait
Copy link
Collaborator

@rahulait rahulait commented May 16, 2025

General:

This PR adds UDP support for Linode NodeBalancers by updating configuration handling, test cases, and YAML examples.

  • Enabled UDP support by refactoring configuration logic in cloud/linode/loadbalancers.go and loadbalancers_helpers.go
  • Updated YAML examples and end-to-end tests to incorporate UDP ports
  • Adjusted annotations to include default algorithm, stickiness, and UDP check port
  • Have you removed all sensitive information, including but not limited to access keys and passwords?
  • Have you checked to ensure there aren't other open or closed Pull Requests for the same bug/feature/question?

Pull Request Guidelines:

  1. Does your submission pass tests?
  2. Have you added tests?
  3. Are you addressing a single feature in this PR?
  4. Are your commits atomic, addressing one change per commit?
  5. Are you following the conventions of the language?
  6. Have you saved your large formatting changes for a different PR, so we can focus on your work?
  7. Have you explained your rationale for why this feature is needed?
  8. Have you linked your PR to an open issue

Copy link

codecov bot commented May 16, 2025

Codecov Report

Attention: Patch coverage is 91.46341% with 14 lines in your changes missing coverage. Please review.

Project coverage is 74.41%. Comparing base (8cad101) to head (c461a1f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cloud/linode/loadbalancers.go 87.69% 4 Missing and 4 partials ⚠️
cloud/linode/loadbalancers_helpers.go 93.93% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #387      +/-   ##
==========================================
+ Coverage   73.50%   74.41%   +0.90%     
==========================================
  Files          15       16       +1     
  Lines        2744     2865     +121     
==========================================
+ Hits         2017     2132     +115     
- Misses        547      550       +3     
- Partials      180      183       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rahulait rahulait requested a review from Copilot May 16, 2025 04:32
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds UDP port support for nodebalancers, enabling configuration of UDP backends in CCM. Key changes include new YAML examples and tests for UDP configurations, modifications in loadbalancer functions to accept UDP ServicePort objects, and updates to helper functions and annotations to support new UDP-specific settings.

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
examples/udp-nginx.yaml Added a new YAML example for UDP load balancer configuration.
e2e/test/lb-with-udp-ports/*.yaml Added end-to-end tests for deployments and nodebalancer with UDP.
cloud/linode/loadbalancers_test.go Updated tests to use ServicePort objects and validate UDP configs.
cloud/linode/loadbalancers_helpers.go Added helper functions to handle UDP-specific settings.
cloud/linode/loadbalancers.go Refactored NodeBalancer config building to support UDP ports.
cloud/annotations/annotations.go Added new annotations to support algorithm and stickiness defaults.
Files not reviewed (1)
  • go.mod: Language not supported

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds UDP support for Linode NodeBalancers by updating configuration handling, test cases, and YAML examples.

  • Enabled UDP support by refactoring configuration logic in cloud/linode/loadbalancers.go and loadbalancers_helpers.go
  • Updated YAML examples and end-to-end tests to incorporate UDP ports
  • Adjusted annotations to include default algorithm, stickiness, and UDP check port

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
examples/udp-nginx.yaml Added a YAML example for a UDP load balancer and corresponding deployment
e2e/test/lb-with-udp-ports/create-pods-services.yaml Created e2e test for a UDP service configuration
e2e/test/lb-with-udp-ports/chainsaw-test.yaml Added test steps to verify NodeBalancer config and UDP port settings
cloud/linode/loadbalancers_helpers.go Updated helper functions to include UDP-related configuration
cloud/linode/loadbalancers.go Refactored NodeBalancer config building to support UDP protocol changes
cloud/annotations/annotations.go Introduced additional annotations for algorithm, stickiness, and UDP check port
Files not reviewed (1)
  • go.mod: Language not supported
Comments suppressed due to low confidence (1)

e2e/test/lb-with-udp-ports/chainsaw-test.yaml:23

  • [nitpick] Consider adding additional assertions in the test steps to verify UDP-specific configurations (e.g., UDPCheckPort and protocol settings) in the NodeBalancer configuration.
- name: Check that loadbalancer ip is assigned

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for UDP ports on Linode NodeBalancers by extending configuration, examples, and tests.

  • Refactored cloud controller logic to handle UDP protocol, algorithm, stickiness, and health check ports
  • Updated YAML examples and end-to-end tests to exercise UDP LoadBalancer scenarios
  • Expanded documentation and annotations to include UDP-related options

Reviewed Changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
examples/udp-example.yaml New Kubernetes Service and Deployment example for UDP LB
e2e/test/lb-with-udp-ports/create-pods-services.yaml Test resources defining pods and a UDP LoadBalancer svc
e2e/test/lb-with-udp-ports/chainsaw-test.yaml Chainsaw E2E test to validate UDP LB configuration
docs/examples/basic.md Added UDP LoadBalancer example in basic documentation
docs/examples/README.md Listed UDP LoadBalancer and testing note in README
docs/configuration/loadbalancer.md Added udp to supported protocols list
docs/configuration/annotations.md Extended annotations table with algorithm, stickiness, UDP check port
cloud/annotations/annotations.go Introduced constants for default algorithm, stickiness, UDP check port
cloud/linode/loadbalancers_helpers.go New helper methods for protocol, proxy, algorithm, stickiness, UDP checks
cloud/linode/loadbalancers.go Refactored config builders and update/create flows to include UDP
Files not reviewed (2)
  • examples/test.sh: Language not supported
  • go.mod: Language not supported

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces UDP support for Linode NodeBalancers by refactoring configuration logic and updating related examples, tests, and documentation.

  • Updated configuration handling in the cloud package to allow UDP ports.
  • Added YAML examples and end-to-end tests for UDP-based NodeBalancers.
  • Extended annotations and documentation to cover UDP-specific settings.

Reviewed Changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
examples/udp-example.yaml New YAML example demonstrating a UDP LoadBalancer.
e2e/test/lb-with-udp-ports/create-pods-services.yaml Added e2e test resource definitions for UDP ports.
e2e/test/lb-with-udp-ports/chainsaw-test.yaml Introduced test steps for verifying UDP nodebalancer setup.
docs/examples/basic.md Extended documentation to include UDP loadbalancer example.
docs/examples/README.md Updated README to reference UDP test server details.
docs/configuration/loadbalancer.md Documented UDP as a valid protocol.
docs/configuration/annotations.md Added new UDP-specific annotations and defaults.
cloud/linode/loadbalancers_helpers.go Extended helper functions to support UDP configuration.
cloud/linode/loadbalancers.go Refactored loadbalancer creation functions for UDP.
cloud/annotations/annotations.go Added new annotation keys for UDP-related configuration.
Files not reviewed (2)
  • examples/test.sh: Language not supported
  • go.mod: Language not supported
Comments suppressed due to low confidence (1)

e2e/test/lb-with-udp-ports/chainsaw-test.yaml:60

  • The UDP node health check is currently stubbed by hardcoding port_7070_up_nodes to "true". Once UDP health checks are available, please implement an actual check instead of using a fixed value.
port_7070_up_nodes="true"

@rahulait rahulait requested a review from Copilot May 27, 2025 23:06
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for NodeBalancers with UDP ports to the Linode cloud controller manager by extending configuration handling, updating annotations, documentation, and end-to-end tests.

  • Refactored configuration logic in cloud/linode/loadbalancers.go and loadbalancers_helpers.go to accept UDP ports and add UDP-specific options.
  • Updated YAML examples, annotations, and docs to include UDP support.
  • Added end-to-end tests (including chainsaw and service/pod setups) to validate UDP NodeBalancer functionality.

Reviewed Changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
examples/udp-example.yaml Adds YAML example for a UDP LoadBalancer service
e2e/test/lb-with-udp-ports/create-pods-services.yaml Defines test resources for UDP-based NodeBalancer setups
e2e/test/lb-with-udp-ports/chainsaw-test.yaml Implements end-to-end test steps for UDP NodeBalancer checks
docs/examples/basic.md Updates documentation to include a UDP LoadBalancer example
docs/examples/README.md Includes UDP LoadBalancer reference in the examples index
docs/configuration/loadbalancer.md Documents UDP as a valid protocol for NodeBalancers
docs/configuration/annotations.md Updates annotations to include defaults for UDP options
cloud/linode/loadbalancers_helpers.go Extends helper functions to support UDP-specific values
cloud/linode/loadbalancers.go Refactors configuration and node options building for UDP
cloud/annotations/annotations.go Adds new annotation keys for algorithm, stickiness, and UDP
Files not reviewed (2)
  • examples/test.sh: Language not supported
  • go.mod: Language not supported

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for UDP ports for Linode NodeBalancers by extending configuration logic, tests, examples, and documentation.

  • Refactored cloud/linode code to handle UDP-specific settings (protocol, health checks, algorithm, stickiness, check port)
  • Added YAML examples and end-to-end Chainsaw tests for UDP services
  • Updated documentation and annotations tables to include UDP in protocols and new options

Reviewed Changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
examples/udp-example.yaml Added Service and Deployment example using UDP
e2e/test/lb-with-udp-ports/create-pods-services.yaml Created pods and a LoadBalancer Service spec for UDP
e2e/test/lb-with-udp-ports/chainsaw-test.yaml Chainsaw test to verify NodeBalancer config for UDP port
docs/examples/basic.md Added a “UDP LoadBalancer” example section
docs/examples/README.md Included “UDP LoadBalancer” in the examples index
docs/configuration/loadbalancer.md Declared udp as a valid protocol
docs/configuration/annotations.md Expanded annotations table with algorithm, stickiness, udp-check-port
cloud/linode/loadbalancers_helpers.go Extended helper functions to parse and validate UDP settings
cloud/linode/loadbalancers.go Integrated UDP into config build, update, and create flows
cloud/annotations/annotations.go Added constant for the UDP check port annotation
Files not reviewed (2)
  • examples/test.sh: Language not supported
  • go.mod: Language not supported
Comments suppressed due to low confidence (1)

e2e/test/lb-with-udp-ports/chainsaw-test.yaml:62

  • The test asserts health-check and protocol but does not verify that the default algorithm and stickiness for UDP were applied. Add assertions on the NodeBalancer config JSON (e.g., .algorithm == "roundrobin" and .stickiness == "session") to ensure defaults are correct.
if [[ $port_7070_check == "true" && $port_7070_interval == "true" && $port_7070_timeout == "true" && $port_7070_attempts == "true" && $port_7070_protocol == "true" && $port_7070_up_nodes == "true" ]]; then

@rahulait rahulait requested a review from Copilot May 27, 2025 23:16
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds UDP support for Linode NodeBalancers by updating configuration handling, tests, and documentation.

  • Updated YAML examples and end-to-end tests to incorporate UDP ports.
  • Modified NodeBalancer configuration logic in the cloud package to support UDP.
  • Updated documentation and annotations to reflect UDP support.

Reviewed Changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
examples/udp-example.yaml Added a YAML example for a UDP LoadBalancer service.
e2e/test/lb-with-udp-ports/create-pods-services.yaml Introduced a test resource for validating UDP port exposure.
e2e/test/lb-with-udp-ports/chainsaw-test.yaml Added a chainsaw test that verifies nodebalancer UDP configuration.
docs/examples/basic.md Documented a basic UDP LoadBalancer example.
docs/examples/README.md Updated the examples index to include a UDP LoadBalancer entry.
docs/configuration/loadbalancer.md Updated supported protocols to include UDP.
docs/configuration/annotations.md Enhanced annotations documentation with UDP-specific fields.
cloud/linode/loadbalancers_helpers.go Updated helper functions to handle UDP check port configuration.
cloud/linode/loadbalancers.go Modified logic to build NodeBalancer configurations and node rebuild options for UDP ports.
cloud/annotations/annotations.go Added constants for UDP check port, algorithm, and stickiness annotations.
Files not reviewed (2)
  • examples/test.sh: Language not supported
  • go.mod: Language not supported
Comments suppressed due to low confidence (2)

e2e/test/lb-with-udp-ports/chainsaw-test.yaml:60

  • The UDP health check currently uses a hardcoded value ('true') as a placeholder. Consider implementing an actual check for UDP node health or adding a comment to clarify why this placeholder is sufficient for the tests.
port_7070_up_nodes="true"

cloud/linode/loadbalancers.go:960

  • [nitpick] Consider adding a comment explaining why the Mode is intentionally left unset for UDP protocols to improve clarity for future maintainers.
if protocol != linodego.ProtocolUDP { nodeOptions.Mode = "accept" }

@rahulait rahulait merged commit 8d08bde into main May 28, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature for new features in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants