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

Support pod SecurityContext for PD #3278

Merged
merged 10 commits into from
Sep 24, 2020
Merged

Support pod SecurityContext for PD #3278

merged 10 commits into from
Sep 24, 2020

Conversation

shonge
Copy link
Member

@shonge shonge commented Sep 16, 2020

What problem does this PR solve?

fix #3241

What is changed and how does it work?

Check List

Tests

  • Unit test
  • E2E test

Code changes

  • Has Go code 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?:

NONE

@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2020

Codecov Report

Merging #3278 into master will decrease coverage by 0.14%.
The diff coverage is 6.25%.

@@            Coverage Diff             @@
##           master    #3278      +/-   ##
==========================================
- Coverage   42.31%   42.17%   -0.15%     
==========================================
  Files         159      159              
  Lines       16371    16437      +66     
==========================================
+ Hits         6928     6932       +4     
- Misses       8804     8865      +61     
- Partials      639      640       +1     
Flag Coverage Δ
#unittest 42.17% <6.25%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Contributor

@lonng lonng 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

@july2993 july2993 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot
Copy link
Contributor

@july2993,Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments.See the corresponding SIG page for more information. Related SIG: k8s(slack).

@july2993
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

@july2993 Oops! auto merge is restricted to Committers of the SIG.See the corresponding SIG page for more information. Related SIG: k8s(slack).

@lonng lonng merged commit 36661dd into pingcap:master Sep 24, 2020
@DanielZhangQD
Copy link
Contributor

@shonge Thanks for the contribution!
There is an issue with the current implementation, in some Kubernetes clusters, e.g. GKE, the latest EKS, etc., we're not allowed to customize the arguments of Kubelet, e.g. --allowed-unsafe-sysctls, so it cannot work if we just configure the SecurityContext for PD, please help provide a new fix and you can refer to the implementation for TiDB and TiKV in #1107.
Sorry for the trouble!

cvvz pushed a commit to cvvz/tidb-operator that referenced this pull request Oct 18, 2020
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.

Support pod SecurityContext for PD
6 participants