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

refactor: update service configurations for device plugin and scheduler #799

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

haitwang-cloud
Copy link
Contributor

What type of PR is this?

/kind design

What this PR does / why we need it:

Dynamic Configuration:

  • Ports and Service types are now dynamically configured using values.yaml, making the charts more flexible and easier to customize.
  • Default values are provided for httpPort, monitorPort, and nodePort to ensure backward compatibility.

Removed Hardcoded Values:

  • Hardcoded values for ports and nodePorts have been replaced with dynamic references to values.yaml.

Improved Readability:

  • Added comments to explain the purpose of each section, making the templates easier to understand and maintain.

These changes are necessary to make the Helm charts more adaptable to different deployment scenarios and easier to maintain in the long term
Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
This PR focuses on optimizing the Service configurations in the Helm charts without changing the core functionality.

Does this PR introduce a user-facing change?:

Yes, this PR introduces the following user-facing changes:

Users can now customize Service types (NodePort or ClusterIP) and ports via values.yaml.

The nodePort field is only set when the Service type is NodePort, avoiding unnecessary configurations for ClusterIP Services.

@chaunceyjiang
Copy link
Contributor

/lgtm

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