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

Remove the PodRestarter controller and tidb.pingcap.com/pod-defer-deleting annotation #3296

Merged
merged 3 commits into from
Sep 22, 2020
Merged

Remove the PodRestarter controller and tidb.pingcap.com/pod-defer-deleting annotation #3296

merged 3 commits into from
Sep 22, 2020

Conversation

lonng
Copy link
Contributor

@lonng lonng commented Sep 22, 2020

What problem does this PR solve?

This PodRestarter is used to restart a pod by annotating it with tidb.pingcap.com/pod-defer-deleting. However, there is no user to use this feature because we recommandate users to restart pods gracefully via instrucations https://docs.pingcap.com/tidb-in-kubernetes/dev/restart-a-tidb-cluster#performing-a-graceful-rolling-restart-to-all-pods-in-a-component. The PodRestarter control will retrieve all pods belongs to the current TiDBCluster instance when every sync iteration and delete some of it, I think there is unnecessary cost.

What is changed and how does it work?

Remove the PodRestarter

Check List

Tests

  • Unit test
  • E2E test

Code changes

  • Has Go code change

Does this PR introduce a user-facing change?:

Remove the PodRestarter controller and `tidb.pingcap.com/pod-defer-deleting` annotation

@lonng lonng added enhancement New feature or request needs-cherry-pick-1.0 labels Sep 22, 2020
Signed-off-by: Lonng <[email protected]>
@lonng lonng added this to the v1.1.5 milestone Sep 22, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #3296 into master will increase coverage by 2.57%.
The diff coverage is 30.88%.

@@            Coverage Diff             @@
##           master    #3296      +/-   ##
==========================================
+ Coverage   39.76%   42.33%   +2.57%     
==========================================
  Files         172      159      -13     
  Lines       17558    16371    -1187     
==========================================
- Hits         6982     6931      -51     
+ Misses       9925     8802    -1123     
+ Partials      651      638      -13     
Flag Coverage Δ
#unittest 42.33% <30.88%> (?)

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

Copy link
Contributor

@Yisaer Yisaer left a comment

Choose a reason for hiding this comment

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

LGTM

@lonng lonng requested review from lichunzhu and DanielZhangQD and removed request for DanielZhangQD September 22, 2020 04:21
Copy link
Contributor

@lichunzhu lichunzhu left a comment

Choose a reason for hiding this comment

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

LGTM

@lonng lonng merged commit 18703b9 into pingcap:master Sep 22, 2020
@lonng lonng deleted the remove-pod-restart branch September 22, 2020 12:37
ti-srebot pushed a commit to ti-srebot/tidb-operator that referenced this pull request Sep 22, 2020
@ti-srebot
Copy link
Contributor

cherry pick to release-1.0 in PR #3300

@ti-srebot
Copy link
Contributor

cherry pick to release-1.1 in PR #3301

lonng added a commit to ti-srebot/tidb-operator that referenced this pull request Sep 22, 2020
Yisaer pushed a commit that referenced this pull request Sep 23, 2020
cvvz pushed a commit to cvvz/tidb-operator that referenced this pull request Oct 18, 2020
@sangheee sangheee mentioned this pull request May 12, 2022
10 tasks
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.

5 participants