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

Remain PVC when TidbMonitor get deleted. #2374

Merged
merged 2 commits into from
May 7, 2020

Conversation

Yisaer
Copy link
Contributor

@Yisaer Yisaer commented May 6, 2020

What problem does this PR solve?

close #2329

What is changed and how does it work?

Currently we would delete pvc when tidbmonitor is deleted. In this request, we would remain PVC if TidbMonitor is deleted.

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Related changes

  • Need to cherry-pick to the release branch

Does this PR introduce a user-facing change?:

Don't set owner references to keep `PVC` for `TidbMonitor` when it gets deleted

@Yisaer Yisaer requested review from aylei, DanielZhangQD, weekface and cofyc and removed request for aylei May 6, 2020 05:46
@Yisaer Yisaer added the area/monitor monitoring label May 6, 2020
@cofyc
Copy link
Contributor

cofyc commented May 6, 2020

why not set reclaimPolicy to Retain?

@Yisaer
Copy link
Contributor Author

Yisaer commented May 6, 2020

why not set reclaimPolicy to Retain?

We would keep the same strategy in PV/PVC between TidbMonitor and TidbCluster:

  1. Remain PVC when the object get deleted.
  2. expose ReclaimPlicy for user.

@Yisaer Yisaer added the enhancement New feature or request label May 6, 2020
@cofyc
Copy link
Contributor

cofyc commented May 6, 2020

Support to remain `PVC` for `TidbMonitor` when it get deleted.

->

Don't set owner references to keep `PVC` for `TidbMonitor` when it gets deleted

describe what's changed.

Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

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

LGTM

@Yisaer Yisaer merged commit a072387 into pingcap:master May 7, 2020
sre-bot pushed a commit to sre-bot/tidb-operator that referenced this pull request May 7, 2020
@sre-bot
Copy link
Contributor

sre-bot commented May 7, 2020

cherry pick to release-1.1 in PR #2386

cofyc added a commit to cofyc/tidb-operator that referenced this pull request May 15, 2020
cofyc added a commit to cofyc/tidb-operator that referenced this pull request May 15, 2020
cofyc added a commit that referenced this pull request May 15, 2020
* release v1.1.0-rc.4

* Apply suggestions from code review

Co-authored-by: Ran <huangran@pingcap.com>

* update release notes for #2374

* update release notes for #2374

Co-authored-by: Ran <huangran@pingcap.com>
sre-bot pushed a commit to sre-bot/tidb-operator that referenced this pull request May 15, 2020
* release v1.1.0-rc.4

* Apply suggestions from code review

Co-authored-by: Ran <huangran@pingcap.com>

* update release notes for pingcap#2374

* update release notes for pingcap#2374

Co-authored-by: Ran <huangran@pingcap.com>
cofyc added a commit that referenced this pull request May 15, 2020
* release v1.1.0-rc.4

* Apply suggestions from code review

Co-authored-by: Ran <huangran@pingcap.com>

* update release notes for #2374

* update release notes for #2374

Co-authored-by: Ran <huangran@pingcap.com>

Co-authored-by: Yecheng Fu <fuyecheng@pingcap.com>
Co-authored-by: Ran <huangran@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

monitor PVC will be deleted if TidbMonitor CR is deleted
4 participants