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

binding clusterrole system:kube-scheduler to sa tidb-scheduler #1355

Merged
merged 13 commits into from
Dec 23, 2019

Conversation

shonge
Copy link
Member

@shonge shonge commented Dec 17, 2019

What problem does this PR solve?

#1252

What is changed and how does it work?

Service account tidb-scheduler is bound three roles/clusterroles :

  1. tidb-scheduler (role or cluster-role)
  2. system:kube-scheduler (cluster-role)
  3. system:volume-scheduler (cluster-role)

Check List

Tests

  • Unit test
  • E2E test
  • Manual test (add detailed scripts or steps below)
  • No code

Code changes

  • Has Helm charts change

Side effects

  • Breaking backward compatibility

    Related changes

  • Need to cherry-pick to the release branch

  • Need to update the documentation

Does this PR introduce a user-facing change?:

Use cluster-role system:kube-scheduler to binding service account tidb-scheduler.

@cofyc
Copy link
Contributor

cofyc commented Dec 18, 2019

/run-e2e-test

@cofyc
Copy link
Contributor

cofyc commented Dec 18, 2019

xref: #1356

@cofyc
Copy link
Contributor

cofyc commented Dec 18, 2019

hi, tidb-scheduler pod has two components:

  • tidb-scheduler/kube-scheduler needs permission of clusterrole system:kube-scheduler
  • tidb-scheduler/tidb-scheduler needs some extra permissions (our CRDs, etc).

So, in addition to bind the the service account of tidb-scheduler to clusterrole system:kube-scheduler, we need to create a clusterrole for tidb-scheduler/tidb-scheduler.

@shonge
Copy link
Member Author

shonge commented Dec 19, 2019

@cofyc PTAL

@cofyc
Copy link
Contributor

cofyc commented Dec 20, 2019

permissions for kube-scheduler can be removed from {{ .Release.Name }}:{{ .Values.scheduler.schedulerName }} because the service account has been bound to system:kube-scheduler, you can go through tidb-scheduler code to check what permission it needs.

is it ok to remove non-clusterScoped support? cc @tennix @weekface @aylei

@cofyc
Copy link
Contributor

cofyc commented Dec 20, 2019

/run-e2e-test

@cofyc
Copy link
Contributor

cofyc commented Dec 23, 2019

/run-e2e-test

@cofyc cofyc requested review from weekface and aylei December 23, 2019 04:13
Copy link
Contributor

@aylei aylei left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@cofyc cofyc left a comment

Choose a reason for hiding this comment

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

LGTM Thanks for you contributions!

@sre-bot
Copy link
Contributor

sre-bot commented Dec 23, 2019

cherry pick to release-1.0 failed

@cofyc
Copy link
Contributor

cofyc commented Dec 23, 2019

@shonge can you help cherry-pick this PR into release-1.0? /hack/cherry_pick_pull.sh can help you to create a cherry-pick PR if you have hub installed locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants