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

refine export cleaner #2916

Merged
merged 6 commits into from
Jul 14, 2020
Merged

Conversation

lichunzhu
Copy link
Contributor

@lichunzhu lichunzhu commented Jul 13, 2020

What problem does this PR solve?

fix #2914

What is changed and how does it work?

when rclone meet doesn't exist error, make cleaner just ignore it.

Check List

Tests

  • Unit test
  • E2E test
  • Stability test
  • Manual test (add detailed scripts or steps below)
    test delete BR and dumpling's backup file and then delete backup CR.

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

Does this PR introduce a user-facing change?:

Exit without error if we clean not found backup data

@@ -67,7 +68,7 @@ func (bo *Options) cleanRemoteBackupData(bucket string, opts []string) error {
destBucket := util.NormalizeBucketURI(bucket)
args := util.ConstructArgs(constants.RcloneConfigArg, opts, "deletefile", destBucket, "")
output, err := exec.Command("rclone", args...).CombinedOutput()
if err != nil {
if err != nil && !strings.Contains(err.Error(), "doesn't exist") {
Copy link
Contributor

Choose a reason for hiding this comment

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

does rclone document its exit codes?

Copy link
Contributor

Choose a reason for hiding this comment

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

checking the error string is fragile, can we use the exit codes documented here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checking the error string is fragile, can we use the exit codes documented here?

nice advise! I will take a try

@cofyc
Copy link
Contributor

cofyc commented Jul 14, 2020

/test pull-e2e-kind-serial

@lichunzhu
Copy link
Contributor Author

/test pull-e2e-kind-serial

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

@lichunzhu
Copy link
Contributor Author

/test pull-e2e-kind-serial

@lichunzhu lichunzhu merged commit e2fcef5 into pingcap:master Jul 14, 2020
@lichunzhu lichunzhu deleted the refineExportCleaner branch July 14, 2020 09:03
ti-srebot pushed a commit to ti-srebot/tidb-operator that referenced this pull request Jul 14, 2020
@ti-srebot
Copy link
Contributor

cherry pick to release-1.1 in PR #2924

ti-srebot added a commit that referenced this pull request Jul 14, 2020
Signed-off-by: ti-srebot <[email protected]>

Co-authored-by: Chunzhu Li <[email protected]>
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.

operator get blocked when delete backup CR
4 participants