Skip to content

[Enhancement] Define some default values uniformly #425

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

yandongxiao
Copy link
Collaborator

Description

In the individual spec definitions of the sub-components, there are indeed many fields with the same name, such as image, imagePullSecrets, nodeSelector, schedulerName. Generally, the contents of these fields are consistent in the individual spec definitions of the sub-components.

Fixes: #424

Checklist

For helm chart, please complete the following checklist:

  • make sure you have updated the values.yaml
    file of starrocks chart.
  • In scripts directory, run bash create-parent-chart-values.sh to update the values.yaml file of the parent
    chart( kube-starrocks chart).

@yandongxiao yandongxiao marked this pull request as ready for review January 29, 2024 06:43
@yandongxiao yandongxiao requested review from dengliu and kevincai and removed request for dengliu January 29, 2024 06:44
@yandongxiao yandongxiao force-pushed the enhancement/define-default-values-uniformly branch from a5ff654 to 3d15606 Compare January 29, 2024 06:49
@@ -61,7 +106,7 @@ starrocksFESpec:
image:
# image sliced by "repository:tag"
repository: starrocks/fe-ubuntu
tag: 3.1-latest
tag: ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

if we set it to empty string. will it also take precedence over the values in componentValues?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No. Only when it has a non-empty value.

Copy link
Collaborator

@dengliu dengliu left a comment

Choose a reason for hiding this comment

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

Copy link
Collaborator

@dengliu dengliu left a comment

Choose a reason for hiding this comment

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

Thanks for the change. This is super useful!

# - key: metadata.name
# operator: In
# values:
# - target-host-name
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add podLabels as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

commited

@yandongxiao yandongxiao force-pushed the enhancement/define-default-values-uniformly branch from 3d15606 to 05deb8f Compare January 30, 2024 03:10
@yandongxiao yandongxiao force-pushed the enhancement/define-default-values-uniformly branch from 05deb8f to f5c450d Compare January 30, 2024 06:15
Get the value of the nodeSelector field in the starrocksFESpec
*/}}
{{- define "starrockscluster.fe.nodeSelector" -}}
{{- if .Values.starrocksFESpec.nodeSelector -}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a complete replace behavior, not the merge together behavior, need explicit this in the values.yaml comments.

Otherwise, the user can define some of the common criteria in compontValues and add additional specific criteria in each XXSpec and expect the final result is the merge-replace output.

The same applies to hostAlias/Labels/affinity ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

committed

@yandongxiao yandongxiao merged commit 03fe897 into StarRocks:main Jan 30, 2024
@yandongxiao yandongxiao added v1.9.2 enhancement New feature or request labels Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v1.9.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a global image:tag field in SR Helm chart
4 participants