-
Notifications
You must be signed in to change notification settings - Fork 69
[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
Conversation
Codecov ReportAttention: Patch coverage is
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. 🚀 New features to boost your workflow:
|
There was a problem hiding this 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
There was a problem hiding this 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
There was a problem hiding this 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
There was a problem hiding this 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"
There was a problem hiding this 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
There was a problem hiding this 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
There was a problem hiding this 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" }
General:
This PR adds UDP support for Linode NodeBalancers by updating configuration handling, test cases, and YAML examples.
Pull Request Guidelines: