opt(operator-v2): Resolve BR todos (#6086)

Author: ideascf

.golangci.yml

@@ -126,3 +126,6 @@ issues:
 
 run:
   timeout: 15m
+  skip-files:
+    - ".*/br/.*/_test.go"
+    - ".*/br/.*/testutils/.*"

api/br/v1alpha1/backup.go

@@ -68,19 +68,6 @@ func (bk *Backup) GetAllLogBackupJobName() []string {
 	}
 }
 
-// TODO(ideascf): do we need these functions?
-// // GetTidbEndpointHash return the hash string base on tidb cluster's host and port
-//
-//	func (bk *Backup) GetTidbEndpointHash() string {
-//		return HashContents([]byte(bk.Spec.From.GetTidbEndpoint()))
-//	}
-//
-// // GetBackupPVCName return the backup pvc name
-//
-//	func (bk *Backup) GetBackupPVCName() string {
-//		return fmt.Sprintf("backup-pvc-%s", bk.GetTidbEndpointHash())
-//	}
-//
 // GetInstanceName return the backup instance name
 func (bk *Backup) GetInstanceName() string {
 	if bk.Labels != nil {
@@ -121,7 +108,6 @@ var (
 	RestoreControllerKind        = SchemeGroupVersion.WithKind("Restore")
 )
 
-// TODO(ideascf): copy UT
 // GetBackupOwnerRef returns Backup's OwnerReference
 func GetBackupOwnerRef(backup *Backup) metav1.OwnerReference {
 	controller := true

api/br/v1alpha1/backup_test.go

@@ -0,0 +1,111 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package v1alpha1
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+
+	metav1alpha1 "github.com/pingcap/tidb-operator/api/v2/meta/v1alpha1"
+)
+
+func TestGetBackupOwnerRef(t *testing.T) {
+	g := NewGomegaWithT(t)
+
+	b := newBackup()
+	b.UID = types.UID("demo-uid")
+	ref := GetBackupOwnerRef(b)
+	g.Expect(ref.APIVersion).To(Equal(BackupControllerKind.GroupVersion().String()))
+	g.Expect(ref.Kind).To(Equal(BackupControllerKind.Kind))
+	g.Expect(ref.Name).To(Equal(b.GetName()))
+	g.Expect(ref.UID).To(Equal(types.UID("demo-uid")))
+	g.Expect(*ref.Controller).To(BeTrue())
+	g.Expect(*ref.BlockOwnerDeletion).To(BeTrue())
+}
+
+func TestGetRestoreOwnerRef(t *testing.T) {
+	g := NewGomegaWithT(t)
+
+	r := newRestore()
+	r.UID = types.UID("demo-uid")
+	ref := GetRestoreOwnerRef(r)
+	g.Expect(ref.APIVersion).To(Equal(RestoreControllerKind.GroupVersion().String()))
+	g.Expect(ref.Kind).To(Equal(RestoreControllerKind.Kind))
+	g.Expect(ref.Name).To(Equal(r.GetName()))
+	g.Expect(ref.UID).To(Equal(types.UID("demo-uid")))
+	g.Expect(*ref.Controller).To(BeTrue())
+	g.Expect(*ref.BlockOwnerDeletion).To(BeTrue())
+}
+
+func TestGetBackupScheduleOwnerRef(t *testing.T) {
+	g := NewGomegaWithT(t)
+
+	b := newBackupSchedule()
+	b.UID = types.UID("demo-uid")
+	ref := GetBackupScheduleOwnerRef(b)
+	g.Expect(ref.APIVersion).To(Equal(backupScheduleControllerKind.GroupVersion().String()))
+	g.Expect(ref.Kind).To(Equal(backupScheduleControllerKind.Kind))
+	g.Expect(ref.Name).To(Equal(b.GetName()))
+	g.Expect(ref.UID).To(Equal(types.UID("demo-uid")))
+	g.Expect(*ref.Controller).To(BeTrue())
+	g.Expect(*ref.BlockOwnerDeletion).To(BeTrue())
+}
+
+func newBackup() *Backup {
+	backup := &Backup{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "demo-backup",
+			Namespace: metav1.NamespaceDefault,
+			Labels: map[string]string{
+				metav1alpha1.NameLabelKey: metav1alpha1.BackupJobLabelVal,
+			},
+		},
+		Spec: BackupSpec{},
+	}
+	return backup
+}
+
+func newRestore() *Restore {
+	restore := &Restore{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "demo-backup",
+			Namespace: metav1.NamespaceDefault,
+			Labels: map[string]string{
+				metav1alpha1.NameLabelKey: metav1alpha1.RestoreJobLabelVal,
+			},
+		},
+		Spec: RestoreSpec{},
+	}
+	return restore
+}
+
+func newBackupSchedule() *BackupSchedule {
+	backup := &BackupSchedule{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "demo-backup",
+			Namespace: metav1.NamespaceDefault,
+			Labels: map[string]string{
+				metav1alpha1.NameLabelKey: metav1alpha1.BackupScheduleJobLabelVal,
+			},
+		},
+		Spec: BackupScheduleSpec{
+			BackupTemplate: BackupSpec{},
+		},
+	}
+	return backup
+}

api/br/v1alpha1/backup_types.go

@@ -259,7 +259,7 @@ type Progress struct {
 	// Step is the step name of progress
 	Step string `json:"step,omitempty"`
 	// Progress is the backup progress value
-	Progress int `json:"progress,omitempty"` // TODO(ideascf): type changed from float64 to int
+	Progress int `json:"progress,omitempty"`
 	// LastTransitionTime is the update time
 	// +nullable
 	LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
@@ -295,40 +295,41 @@ type BackupSpec struct {
 	// Mode is the backup mode, such as snapshot backup or log backup.
 	// +kubebuilder:default=snapshot
 	Mode BackupMode `json:"backupMode,omitempty"`
-	// TikvGCLifeTime is to specify the safe gc life time for backup.
-	// The time limit during which data is retained for each GC, in the format of Go Duration.
-	// When a GC happens, the current time minus this value is the safe point.
-	TikvGCLifeTime *string `json:"tikvGCLifeTime,omitempty"`
 	// StorageProvider configures where and how backups should be stored.
 	// *** Note: This field should generally not be left empty, unless you are certain the storage provider
 	// *** can be obtained from another source, such as a schedule CR.
 	StorageProvider `json:",inline"`
+	// BRConfig is the configs for BR
+	// *** Note: This field should generally not be left empty, unless you are certain the BR config
+	// *** can be obtained from another source, such as a schedule CR.
+	BR *BRConfig `json:"br,omitempty"`
+
 	// The storageClassName of the persistent volume for Backup data storage.
 	// Defaults to Kubernetes default storage class.
 	// +optional
 	StorageClassName *string `json:"storageClassName,omitempty"`
 	// StorageSize is the request storage size for backup job
 	StorageSize string `json:"storageSize,omitempty"`
-	// BRConfig is the configs for BR
-	// *** Note: This field should generally not be left empty, unless you are certain the BR config
-	// *** can be obtained from another source, such as a schedule CR.
-	BR *BRConfig `json:"br,omitempty"`
+
 	// CommitTs is the commit ts of the backup, snapshot ts for full backup or start ts for log backup.
 	// Format supports TSO or datetime, e.g. '400036290571534337', '2018-05-11 01:42:23'.
 	// Default is current timestamp.
 	// +optional
 	CommitTs string `json:"commitTs,omitempty"`
 	// Subcommand is the subcommand for BR, such as start, stop, pause etc.
 	// +optional
-	// +kubebuilder:validation:Enum:="log-start";"log-stop";"log-pause"
+	// +kubebuilder:validation:Enum:="log-start";"log-stop";"log-pause";"log-resume";"log-truncate"
 	LogSubcommand LogSubCommandType `json:"logSubcommand,omitempty"`
 	// LogTruncateUntil is log backup truncate until timestamp.
 	// Format supports TSO or datetime, e.g. '400036290571534337', '2018-05-11 01:42:23'.
+	// It's required when LogSubcommand is "log-truncate".
 	// +optional
 	LogTruncateUntil string `json:"logTruncateUntil,omitempty"`
+	// Deprecated: use LogSubcommand instead. it will be removed later.
 	// LogStop indicates that will stop the log backup.
 	// +optional
 	LogStop bool `json:"logStop,omitempty"`
+
 	// Base tolerations of backup Pods, components may add more tolerations upon this respectively
 	// +optional
 	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
@@ -349,6 +350,7 @@ type BackupSpec struct {
 	UseKMS bool `json:"useKMS,omitempty"`
 	// Specify service account of backup
 	ServiceAccount string `json:"serviceAccount,omitempty"`
+
 	// CleanPolicy denotes whether to clean backup data when the object is deleted from the cluster, if not set, the backup data will be retained
 	// +kubebuilder:validation:Enum:=Retain;OnFailure;Delete
 	// +kubebuilder:default=Retain

api/br/v1alpha1/restore.go

@@ -121,15 +121,3 @@ func IsRestoreFailed(restore *Restore) bool {
 	_, condition := GetRestoreCondition(&restore.Status, RestoreFailed)
 	return condition != nil && condition.Status == metav1.ConditionTrue
 }
-
-// IsRestoreTiKVComplete returns true if all TiKVs run successfully during volume restore
-func IsRestoreTiKVComplete(restore *Restore) bool {
-	_, condition := GetRestoreCondition(&restore.Status, RestoreTiKVComplete)
-	return condition != nil && condition.Status == metav1.ConditionTrue
-}
-
-// IsRestoreDataComplete returns true if a Restore for data consistency has successfully completed
-func IsRestoreDataComplete(restore *Restore) bool {
-	_, condition := GetRestoreCondition(&restore.Status, RestoreDataComplete)
-	return condition != nil && condition.Status == metav1.ConditionTrue
-}

api/br/v1alpha1/restore_types.go

@@ -76,11 +76,6 @@ const (
 	RestoreScheduled RestoreConditionType = "Scheduled"
 	// RestoreRunning means the Restore is currently being executed.
 	RestoreRunning RestoreConditionType = "Running"
-	// RestoreDataComplete means the Restore has successfully executed part-2 and the
-	// data in restore volumes has been deal with consistency based on min_resolved_ts
-	RestoreDataComplete RestoreConditionType = "DataComplete"
-	// RestoreTiKVComplete means in volume restore, all TiKV instances are started and up
-	RestoreTiKVComplete RestoreConditionType = "TikvComplete"
 	// RestoreComplete means the Restore has successfully executed and the
 	// backup data has been loaded into tidb cluster.
 	RestoreComplete RestoreConditionType = "Complete"
@@ -131,28 +126,27 @@ type RestoreSpec struct {
 	// Mode is the restore mode. such as snapshot or pitr.
 	// +kubebuilder:default=snapshot
 	Mode RestoreMode `json:"restoreMode,omitempty"`
+	// StorageProvider configures where and how backups should be stored.
+	StorageProvider `json:",inline"`
+	// BR is the configs for BR.
+	BR *BRConfig `json:"br,omitempty"`
+
 	// PitrRestoredTs is the pitr restored ts.
 	PitrRestoredTs string `json:"pitrRestoredTs,omitempty"`
 	// LogRestoreStartTs is the start timestamp which log restore from.
 	// +optional
 	LogRestoreStartTs string `json:"logRestoreStartTs,omitempty"`
-	// TikvGCLifeTime is to specify the safe gc life time for restore.
-	// The time limit during which data is retained for each GC, in the format of Go Duration.
-	// When a GC happens, the current time minus this value is the safe point.
-	TikvGCLifeTime *string `json:"tikvGCLifeTime,omitempty"`
-	// StorageProvider configures where and how backups should be stored.
-	StorageProvider `json:",inline"`
 	// PitrFullBackupStorageProvider configures where and how pitr dependent full backup should be stored.
 	// +optional
 	PitrFullBackupStorageProvider StorageProvider `json:"pitrFullBackupStorageProvider,omitempty"`
+
 	// The storageClassName of the persistent volume for Restore data storage.
 	// Defaults to Kubernetes default storage class.
 	// +optional
 	StorageClassName *string `json:"storageClassName,omitempty"`
 	// StorageSize is the request storage size for backup job
 	StorageSize string `json:"storageSize,omitempty"`
-	// BR is the configs for BR.
-	BR *BRConfig `json:"br,omitempty"`
+
 	// Base tolerations of restore Pods, components may add more tolerations upon this respectively
 	// +optional
 	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

api/br/v1alpha1/zz_generated.deepcopy.go

@@ -90,7 +90,6 @@ const (
 	DirPathClusterTLSPD      = "/var/lib/pd-tls"
 	DirPathClusterTLSTiKV    = "/var/lib/tikv-tls"
 	DirPathClusterTLSTiDB    = "/var/lib/tidb-tls"
-	DirPathTiDBClientTLS     = "/var/lib/tidb-client-tls" // FIXME(ideascf): do we need this?
 	DirPathClusterTLSTiFlash = "/var/lib/tiflash-tls"
 	DirPathClusterClientTLS  = "/var/lib/cluster-client-tls"
 	DirPathClusterTLSTiCDC   = "/var/lib/ticdc-tls"

api/core/v1alpha1/names.go

@@ -5,6 +5,7 @@ go 1.22.0
 toolchain go1.22.3
 
 require (
+	github.com/onsi/gomega v1.36.2
 	k8s.io/api v0.31.0
 	k8s.io/apimachinery v0.31.0
 )
@@ -13,15 +14,17 @@ require (
 	github.com/fxamacker/cbor/v2 v2.7.0 // indirect
 	github.com/go-logr/logr v1.4.2 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
+	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/x448/float16 v0.8.4 // indirect
-	golang.org/x/net v0.28.0 // indirect
-	golang.org/x/text v0.17.0 // indirect
+	golang.org/x/net v0.33.0 // indirect
+	golang.org/x/text v0.21.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/klog/v2 v2.130.1 // indirect
 	k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect