Skip to content

fix(port-allocator): add startup args to charts#249

Merged
Syspretor merged 1 commit intosgl-project:mainfrom
NoobDream2568:port-allocator-chart
Mar 27, 2026
Merged

fix(port-allocator): add startup args to charts#249
Syspretor merged 1 commit intosgl-project:mainfrom
NoobDream2568:port-allocator-chart

Conversation

@NoobDream2568
Copy link
Copy Markdown
Contributor

@NoobDream2568 NoobDream2568 commented Mar 27, 2026

Ⅰ. Motivation

Add Helm configuration support for port allocator parameters to allow users customize port allocation strategy and range without modifying templates directly.

Ⅱ. Modifications

  • manager.yaml: Updated to read port allocator config from .Values.portAllocator with defaults
  • values.yaml: Added portAllocator section with strategy, startPort, and portRange options

Default values (backward compatible):

  • strategy: random
  • startPort: 30000
  • portRange: 5000

Ⅲ. Does this pull request fix one issue?

NONE

Ⅳ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅴ. Describe how to verify it

VI. Special notes for reviews

Checklist

  • Format your code make fmt.
  • Add unit tests or integration tests.
  • Update the documentation related to the change.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces port allocation configuration to the manager deployment by adding command-line arguments for strategy, start port, and port range in the Helm templates, along with corresponding default values. A review comment suggests enhancing the template's robustness by using a conditional block to handle scenarios where the portAllocator configuration might be missing from the user-provided values.

Comment thread deploy/helm/rbgs/templates/manager.yaml
Copy link
Copy Markdown
Collaborator

@Syspretor Syspretor left a comment

Choose a reason for hiding this comment

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

/lgtm

@Syspretor Syspretor merged commit fa5c8d5 into sgl-project:main Mar 27, 2026
10 of 11 checks passed
@NoobDream2568 NoobDream2568 deleted the port-allocator-chart branch March 30, 2026 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants