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

keep scheduler name consistent as defined in values.yml #188

Merged

Conversation

onlymellb
Copy link
Contributor

The scheduler can't use scheduler policy configmap correctly when you change the schedulerName in values.yaml. This PR fix it.

@weekface
Copy link
Contributor

/run-e2e-tests

Copy link
Contributor

@weekface weekface left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -26,7 +26,7 @@ spec:
serviceAccount: {{ .Values.scheduler.serviceAccount }}
{{- end }}
containers:
- name: tidb-scheduler
- name: {{ .Values.scheduler.schedulerName }}
Copy link
Member

Choose a reason for hiding this comment

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

Container name is used internally, using tidb-scheduler is handy for kubectl exec and kubectl logs. So it's unnecessary to change this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better to keep the name consistent, and it's not necessary to fix the container name to executing kubectl exec and kubectl logs

Copy link
Member

Choose a reason for hiding this comment

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

If using tidb-scheduler as container name, the two container names are always tidb-scheduler and kube-scheduler. When debugging we can always use the same name without thinking what it is. Besides container name should reflect the actual image it uses other than using the installer specified name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you want to use tidb-scheduler as container name, you can keep the schedulerName as default. But if you want to change the default value then keeping the container name consistent with it makes sense.

@tennix tennix merged commit f0cf8a4 into pingcap:master Nov 22, 2018
queenliuxx pushed a commit to queenliuxx/tidb-operator that referenced this pull request Dec 19, 2018
yahonda pushed a commit that referenced this pull request Dec 27, 2021
* Add markdownlint in CI

Signed-off-by: Aylei <rayingecho@gmail.com>

* Update .markdownlint.yaml

Co-Authored-By: Keke Yi <40977455+yikeke@users.noreply.github.com>

Co-authored-by: Lilian Lee <lilin@pingcap.com>
Co-authored-by: Keke Yi <40977455+yikeke@users.noreply.github.com>
fgksgf pushed a commit to fgksgf/tidb-operator that referenced this pull request Dec 23, 2024
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.

4 participants