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

Stop the running log backup when deleting CR (#5754) #5765

Merged

Conversation

ti-chi-bot
Copy link
Member

This is an automated cherry-pick of #5754

What problem does this PR solve?

In the previous operator version, delete backup CR will do no clean up on running log backup.
Users may mis-operator and find it hard to running task.

What is changed and how does it work?

  1. Add enum for cleanPolicy, make input standardized.
    Now, only Retain;OnFailure;Delete are accepted as cleanPolicy input, and default is Retain.
    (This is actually no change to user behavior.)

  2. Add finalizer and clean logic check for on-going log backup
    All log backup will have finalizer now. Operator will check if the log backup "on track". If so, a new task to stop log backup will run.
    The "on track" state is BackupScheduled || BackupPrepare || BackupRunning || BackupPaused || BackupStop. We lack the ability to check whether the log backup should be stopped if it failed. If the task is on fail states, we do not provide auto cleaning.

Code changes

  • Has Go code change
  • Has CI related scripts change

Tests

  • Unit test
  • E2E test
  • Manual test
  • No code

Side effects

  • Breaking backward compatibility
  • Other side effects:

Related changes

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

Release Notes

Please refer to Release Notes Language Style Guide before writing the release note.


1. Add enum for cleanPolicy, make input standardized.
   Now, only Retain;OnFailure;Delete are accepted as cleanPolicy input, and Retain is default.
   This is actually no change to user behavior.
2. Add finalizer and clean logic check for on-going log backup
    All log backup will have finalizer now. Operator will check if the log backup "on track". If so, a new task to stop log backup will run.
    The "on track" state is `BackupScheduled || BackupPrepare || BackupRunning || BackupPaused`. 
    We lack the ability to check whether the log backup should be stopped if it failed. If the task is on states, we do not provide auto cleaning.

Co-authored-by: csuzhangxc <csuzhangxc@gmail.com>
@csuzhangxc
Copy link
Member

/run-pull-e2e-kind-br

@codecov-commenter
Copy link

codecov-commenter commented Oct 16, 2024

Codecov Report

Attention: Patch coverage is 63.07692% with 96 lines in your changes missing coverage. Please review.

Please upload report for BASE (release-1.6@2427a92). Learn more about missing BASE report.

Additional details and impacted files
@@              Coverage Diff               @@
##             release-1.6    #5765   +/-   ##
==============================================
  Coverage               ?   20.73%           
==============================================
  Files                  ?      219           
  Lines                  ?    30965           
  Branches               ?        0           
==============================================
  Hits                   ?     6420           
  Misses                 ?    23629           
  Partials               ?      916           
Flag Coverage Δ
e2e 20.73% <63.07%> (?)

Copy link
Contributor

ti-chi-bot bot commented Oct 16, 2024

[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 ask for approval from csuzhangxc, ensuring that each of them provides their approval before proceeding. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

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

@csuzhangxc
Copy link
Member

/run-pull-e2e-kind-br

@csuzhangxc
Copy link
Member

/run-pull-e2e-kind-br

@csuzhangxc csuzhangxc merged commit 718d968 into pingcap:release-1.6 Oct 16, 2024
6 of 7 checks passed
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.

4 participants