Add force option to "pd-ctl store delete" command

[shunki-fujita]

What problem does this PR solve?

Issue Number: Close #8878

What is changed and how does it work?

Add --force option to pd-ctl store delete command.
Set status to Tombstone directly.
https://download.pingcap.com/pd-api-doc.html#store__storeid__delete

$ ./bin/pd-ctl store delete --help
delete the store

Usage:
  pd-ctl store delete <store_id> [flags]
  pd-ctl store delete [command]

Available Commands:
  addr        delete store by its address

Flags:
  -f, --force   Set status to Tombstone directly
  -h, --help    help for delete

Global Flags:
      --cacert string   path of file that contains list of trusted SSL CAs
      --cert string     path of file that contains X509 certificate in PEM format
      --key string      path of file that contains X509 key in PEM format
  -u, --pd string       address of PD (default "http://127.0.0.1:2379")

Use "pd-ctl store delete [command] --help" for more information about a command.
$

Check List

Tests

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

Code changes

• Has the configuration change
• Has HTTP API interfaces changed (Don't forget to add the declarative for the new API)
• Has persistent data change

Side effects

• Possible performance regression
• Increased code complexity
• Breaking backward compatibility

Related changes

• PR to update pingcap/docs/pingcap/docs-cn:
• PR to update pingcap/tiup:
• Need to cherry-pick to the release branch

Release note

None.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign rleungx for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

Welcome @shunki-fujita!

It looks like this is your first PR to tikv/pd 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!



Thank you, and welcome to tikv/pd. 😃

[ti-chi-bot]

Hi @shunki-fujita. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[codecov]

Codecov Report

Attention: Patch coverage is 40.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 76.13%. Comparing base (29dfaf9) to head (68740fd).
Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8879      +/-   ##
==========================================
- Coverage   76.14%   76.13%   -0.01%     
==========================================
  Files         458      459       +1     
  Lines       70208    70247      +39     
==========================================
+ Hits        53462    53486      +24     
- Misses      13389    13396       +7     
- Partials     3357     3365       +8     

Flag	Coverage Δ
unittests	76.13% <40.00%> (-0.01%)	⬇️

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

[rleungx]

@shunki-fujita Thanks for your PR. But we have encountered many issues that misuse this API, so we prevent doing that anymore, see #4039.

[shunki-fujita]

@rleungx
Thank you for your reply!

The --force option sets physical_destroyed=true rather than directly setting store state to Tombstone.
#3388

Even so, is it better not to implement this option?

(By the way, since it is not directly set to Tombstone, the help message might not be correct.)
https://download.pingcap.com/pd-api-doc.html#store__storeid__delete

[rleungx]

Thank you for your reply!
The --force option sets physical_destroyed=true rather than directly setting store state to Tombstone.
#3388
Even so, is it better not to implement this option?
(By the way, since it is not directly set to Tombstone, the help message might not be correct.)
https://download.pingcap.com/pd-api-doc.html#store__storeid__delete>

The reason why we use physical_destroyed is that we might need to add the store with the same address back in some scenarios, e.g., such as k8s env and we are sure that the previous store will never come back. physical_destroyed will let us no need to wait the damaged store finish offline before adding it's replacement. As for API docs, I believe that it's outdated. Let me fix it.

[shunki-fujita]

@rleungx

The reason why we use physical_destroyed is that we might need to add the store with the same address back in some scenarios, e.g., such as k8s env and we are sure that the previous store will never come back. physical_destroyed will let us no need to wait the damaged store finish offline before adding it's replacement.

I encountered exactly that scenario, so I created this PR.
If implementing it in pd-ctl is considered risky, I will execute it via the API instead.

[rleungx]

@shunki-fujita
Both pd-ctl command and API are dangerous. If you use the force option, it will set the store state to offline, and it won't block you from adding a replacement node.

Here is an example:
if the damaged store id is 5 which never comes back including the data. When calling the API with a force option equal to true, the store 5 will be in an offline state. At this moment, you can add a replacement node, and assume that the new store id is 6.
We might have two stores with the same address but different id in the cluster. After the store 5 finishes offline, it will become a tombstone state.

If store 5 cannot be tombstoned, it might have some other issues, which we need to fix instead of directly setting it to tombstoned.

[shunki-fujita]

@rleungx
Thank you for the advice.

If the store 5 cannot be tombstone, it might have some other issues which we need to fix it instead of directly setting it to tombstone state.

I see,
I will conduct a verification separately.