br: a more straight forward operator interface #5710

RidRisR · 2024-08-15T06:38:30Z

What problem does this PR solve?

Use a more straight forward operator interface to replace the old one.
support pause and resume pitr task

What is changed and how does it work?

Now, the crd to control log backup would loops like

  backupMode: log
  logSubcommand: log-start/log-stop/log-pause
  logTruncateUntil: "xxxxxxxxx"

(If no subcommand is provided, the default value is log-start)

If the task is not created before:
It could transfer to running state using log-start

If the task is running:
It could transfer to paused state using log-pause
It could transfer to stop state using log-stop

If the task is paused:
It could transfer to running state using log-start
It could transfer to stop state using log-stop

If the task is stopped:
This is the end state and can't transfer state again

As long as the task is created, user could set logTruncateUntil and truncate would be sync after subcommand is done

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. Use a more straight forward operator interface to replace the old one.
Now, the crd to control log backup would loops like
`spec:
  backupMode: log
  logSubcommand: `log-start`/`log-stop`/`log-pause`
  logTruncateUntil: "xxxxxxxxx"
`
(If no subcommand is provided, the default value is `log-start`)

2. Support pause and resume pitr task
Now, users could pause and resume pitr task by using `log-pause` and `log-start`

sre-bot · 2024-08-15T06:38:37Z

All committers have signed the CLA.

codecov-commenter · 2024-08-27T05:27:36Z

Codecov Report

Attention: Patch coverage is 6.00000% with 47 lines in your changes missing coverage. Please review.

Project coverage is 61.42%. Comparing base (9ef26f8) to head (7ee8579).
Report is 22 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5710      +/-   ##
==========================================
- Coverage   61.47%   61.42%   -0.05%     
==========================================
  Files         235      235              
  Lines       30653    30854     +201     
==========================================
+ Hits        18843    18952     +109     
- Misses       9920     9991      +71     
- Partials     1890     1911      +21

Flag	Coverage Δ
unittest	`61.42% <6.00%> (-0.05%)`	⬇️

pkg/apis/pingcap/v1alpha1/backup.go

pkg/apis/pingcap/v1alpha1/types.go

pkg/controller/backup/backup_controller.go

cmd/backup-manager/app/backup/backup.go

pkg/apis/pingcap/v1alpha1/backup.go

pkg/controller/backup/backup_controller.go

pkg/apis/pingcap/v1alpha1/backup.go

BornChanger · 2024-09-17T17:58:57Z

For the state machine figure, please also add the scenarios where the state change due to gc-ttl. @RidRisR

RidRisR · 2024-09-25T05:17:20Z

For the state machine figure, please also add the scenarios where the state change due to gc-ttl. @RidRisR

we can't modify advancer time out setting now. therefore, the gc-ttl will only time out after 24 hours, it seems too long for a test.

cmd/backup-manager/app/backup/manager.go

pkg/apis/pingcap/v1alpha1/types.go

csuzhangxc · 2024-09-25T09:39:50Z

pkg/apis/pingcap/v1alpha1/backup.go

+	switch backup.Spec.LogSubcommand {
+	case "":
+		fallthrough
+	case LogStartCommand:
+		if IsLogBackupAlreadyPaused(backup) {
+			subCommand = LogResumeCommand
+		} else {
+			subCommand = LogStartCommand
+		}
+	case LogStopCommand:
+		subCommand = LogStopCommand
+	case LogPauseCommand:
+		subCommand = LogPauseCommand
+	default:
+		return LogUnknownCommand


if set LogSubcommand as "", what the subcommand will be used, log-unkown or log-start.

Let me make it more clear:

Actually we don't need the "" as a subcommand. The only reason we keep it is for compatible with the old version interface.
The old usage is demonstrated here: br: add log backup, include start, truncate, stop #4682
TL;DR
In the old version, we use tthe following CRD to start a new task. Therefore, LogSubcommand == "" means LogStartCommand:

Spec: # empty field

I add LogUnknownCommand for handling unexpected input from users, but now we use +kubebuilder:validation:Enum instead. This field is useless now. I keep it just for a double insurance.

Now I refactored the code in the new commit, I suppose it could be more clear now.

pkg/apis/pingcap/v1alpha1/backup.go

ti-chi-bot · 2024-09-29T03:04:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: WangLe1321

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [WangLe1321]

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 · 2024-09-29T03:04:43Z

[LGTM Timeline notifier]

Timeline:

2024-09-29 03:04:06.077631345 +0000 UTC m=+151201.497844358: ☑️ agreed by WangLe1321.
2024-09-29 03:04:42.955361523 +0000 UTC m=+151238.375574539: ✖️🔁 reset by ti-chi-bot[bot].

ti-chi-bot · 2024-09-29T03:04:44Z

New changes are detected. LGTM label has been removed.

ti-chi-bot · 2024-09-29T03:04:44Z

@RidRisR: Your PR was out of date, I have automatically updated it for you.

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 ti-community-infra/tichi repository.

csuzhangxc · 2024-09-29T06:58:10Z

/run-pull-e2e-kind-br

csuzhangxc · 2024-09-29T08:23:49Z

/run-pull-e2e-kind-br

csuzhangxc · 2024-09-29T09:54:12Z

/run-pull-e2e-kind-br

ti-chi-bot bot added first-time-contributor do-not-merge/work-in-progress labels Aug 15, 2024

ti-chi-bot bot requested a review from howardlau1999 August 15, 2024 06:38

ti-chi-bot bot added the size/XXL label Aug 15, 2024

ti-chi-bot bot added the needs-rebase label Aug 28, 2024

RidRisR marked this pull request as ready for review August 29, 2024 09:35

ti-chi-bot bot requested a review from shonge August 29, 2024 09:35

ti-chi-bot bot removed the do-not-merge/work-in-progress label Aug 29, 2024

RidRisR force-pushed the subcommand branch from 3b00e5e to 52532e5 Compare August 30, 2024 03:13

ti-chi-bot bot removed the needs-rebase label Aug 30, 2024

RidRisR changed the title ~~Subcommand~~ br: a more straight forward operator interface Aug 30, 2024

BornChanger requested review from csuzhangxc and WangLe1321 and removed request for howardlau1999 and shonge August 30, 2024 10:24

csuzhangxc reviewed Sep 2, 2024

View reviewed changes

pkg/apis/pingcap/v1alpha1/backup.go Outdated Show resolved Hide resolved

RidRisR requested a review from csuzhangxc September 3, 2024 09:59

WangLe1321 reviewed Sep 14, 2024

View reviewed changes

RidRisR force-pushed the subcommand branch from 5910ac2 to b1b481b Compare September 23, 2024 10:05

RidRisR requested a review from WangLe1321 September 25, 2024 05:18

RidRisR force-pushed the subcommand branch from 7ee8579 to 3000264 Compare September 25, 2024 07:05

RidRisR added 5 commits September 25, 2024 15:53

add command

ac93521

add more

b698c61

add truncate task

5bc9a60

add pause(p1)

d4d0477

task didn't apply

66f0c91

RidRisR added 2 commits September 25, 2024 15:53

add truncate check

2797bf3

handle empty subcommand

90cf8f2

RidRisR force-pushed the subcommand branch from 1a913c6 to 90cf8f2 Compare September 25, 2024 07:53

csuzhangxc reviewed Sep 25, 2024

View reviewed changes

cmd/backup-manager/app/backup/manager.go Show resolved Hide resolved

pkg/apis/pingcap/v1alpha1/types.go Outdated Show resolved Hide resolved

pkg/apis/pingcap/v1alpha1/types.go Show resolved Hide resolved

csuzhangxc added the needs-update-docs label Sep 25, 2024

more restricted subCommand check

a31daf1

RidRisR requested a review from csuzhangxc September 25, 2024 09:27

csuzhangxc reviewed Sep 25, 2024

View reviewed changes

RidRisR added 2 commits September 25, 2024 12:13

user should be able to truncate after stop

04721de

lint

aebc1d5

RidRisR requested a review from csuzhangxc September 25, 2024 10:27

lint

4fab351

WangLe1321 reviewed Sep 27, 2024

View reviewed changes

pkg/apis/pingcap/v1alpha1/backup.go Outdated Show resolved Hide resolved

compatible with old version

638ca5f

RidRisR requested a review from WangLe1321 September 27, 2024 09:21

BornChanger added the tools/br label Sep 28, 2024

WangLe1321 approved these changes Sep 29, 2024

View reviewed changes

ti-chi-bot bot added the lgtm label Sep 29, 2024

ti-chi-bot bot added the approved label Sep 29, 2024

Merge branch 'master' into subcommand

d74f1e3

ti-chi-bot bot removed the lgtm label Sep 29, 2024

Update backup.go

726e5b3

RidRisR requested a review from WangLe1321 September 29, 2024 03:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

br: a more straight forward operator interface #5710

br: a more straight forward operator interface #5710

RidRisR commented Aug 15, 2024 •

edited by csuzhangxc

Loading

sre-bot commented Aug 15, 2024 •

edited

Loading

codecov-commenter commented Aug 27, 2024 •

edited

Loading

BornChanger commented Sep 17, 2024 •

edited

Loading

RidRisR commented Sep 25, 2024

csuzhangxc Sep 25, 2024

RidRisR Sep 25, 2024 •

edited

Loading

ti-chi-bot bot commented Sep 29, 2024

ti-chi-bot bot commented Sep 29, 2024

ti-chi-bot bot commented Sep 29, 2024

ti-chi-bot bot commented Sep 29, 2024

csuzhangxc commented Sep 29, 2024

csuzhangxc commented Sep 29, 2024

csuzhangxc commented Sep 29, 2024

br: a more straight forward operator interface #5710

Are you sure you want to change the base?

br: a more straight forward operator interface #5710

Conversation

RidRisR commented Aug 15, 2024 • edited by csuzhangxc Loading

What problem does this PR solve?

What is changed and how does it work?

Code changes

Tests

Side effects

Related changes

Release Notes

sre-bot commented Aug 15, 2024 • edited Loading

codecov-commenter commented Aug 27, 2024 • edited Loading

Codecov Report

BornChanger commented Sep 17, 2024 • edited Loading

RidRisR commented Sep 25, 2024

csuzhangxc Sep 25, 2024

Choose a reason for hiding this comment

RidRisR Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

ti-chi-bot bot commented Sep 29, 2024

ti-chi-bot bot commented Sep 29, 2024

[LGTM Timeline notifier]

ti-chi-bot bot commented Sep 29, 2024

ti-chi-bot bot commented Sep 29, 2024

csuzhangxc commented Sep 29, 2024

csuzhangxc commented Sep 29, 2024

csuzhangxc commented Sep 29, 2024

RidRisR commented Aug 15, 2024 •

edited by csuzhangxc

Loading

sre-bot commented Aug 15, 2024 •

edited

Loading

codecov-commenter commented Aug 27, 2024 •

edited

Loading

BornChanger commented Sep 17, 2024 •

edited

Loading

RidRisR Sep 25, 2024 •

edited

Loading