address comments

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

@@ -23,15 +23,11 @@ import (
 	glog "k8s.io/klog"
 
 	kvbackup "github.com/pingcap/kvproto/pkg/backup"
+	"github.com/pingcap/tidb-operator/cmd/backup-manager/app/constants"
 	"github.com/pingcap/tidb-operator/cmd/backup-manager/app/util"
 	"github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
 )
 
-const (
-	// MetaFile represents file name
-	MetaFile = "backupmeta"
-)
-
 // Options contains the input arguments to the backup command
 type Options struct {
 	Namespace  string
@@ -75,15 +71,15 @@ func getCommitTs(backup *v1alpha1.Backup) (uint64, error) {
 	}
 	defer s.Close()
 	ctx := context.Background()
-	exist, err := s.Exists(ctx, MetaFile)
+	exist, err := s.Exists(ctx, constants.MetaFile)
 	if err != nil {
 		return commitTs, err
 	}
 	if !exist {
-		return commitTs, fmt.Errorf("%s not exist", MetaFile)
+		return commitTs, fmt.Errorf("%s not exist", constants.MetaFile)
 
 	}
-	metaData, err := s.ReadAll(ctx, MetaFile)
+	metaData, err := s.ReadAll(ctx, constants.MetaFile)
 	if err != nil {
 		return commitTs, err
 	}

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

@@ -53,4 +53,7 @@ const (
 
 	// RcloneConfigArg represents the config argument to rclone cmd
 	RcloneConfigArg = "--config=" + RcloneConfigFile
+
+	// MetaFile is the file name for meta data of backup with BR
+	MetaFile = "backupmeta"
 )

manifests/backup/backup-s3-br.yaml

@@ -0,0 +1,26 @@
+---
+apiVersion: pingcap.com/v1alpha1
+kind: Backup
+metadata:
+  name: demo1-backup-s3
+  namespace: test1
+spec:
+  #backupType: full
+  br:
+    pd: 10.233.40.168:2379
+    # ca: <ca>
+    # cert: <cert>
+    # key: <key>
+    # logLevel: info
+    # statusAddr: <status-addr>
+    # concurrency: 4
+    # rateLimit: 0
+    # timeAgo: <time>
+    # checksum: true
+    # sendCredToTikv: true
+  s3:
+    provider: ceph
+    endpoint: http://10.233.57.220
+    secretName: ceph-secret
+    bucket: backup
+    prefix: test1-demo1

manifests/backup/backup-schedule-s3-br.yaml

@@ -0,0 +1,31 @@
+---
+apiVersion: pingcap.com/v1alpha1
+kind: BackupSchedule
+metadata:
+  name: demo1-backup-schedule-s3
+  namespace: test1
+spec:
+  #maxBackups: 5
+  #pause: true
+  maxReservedTime: "3h"
+  schedule: "*/2 * * * *"
+  backupTemplate:
+    #backupType: full
+    br:
+      pd: 10.233.40.168:2379
+      # ca: <ca>
+      # cert: <cert>
+      # key: <key>
+      # logLevel: info
+      # statusAddr: <status-addr>
+      # concurrency: 4
+      # rateLimit: 0
+      # timeAgo: <time>
+      # checksum: true
+      # sendCredToTikv: true
+    s3:
+      provider: ceph
+      endpoint: http://10.233.57.220
+      secretName: ceph-secret
+      bucket: backup
+      prefix: test1-demo1

manifests/crd.yaml

@@ -1069,16 +1069,6 @@ spec:
                   type: object
                 storageClassName:
                   type: string
-                service:
-                  description: pd service
-                  items:
-                    properties:
-                      name:
-                        type: string
-                      type:
-                        type: string
-                    type: object
-                  type: array
               required:
               - replicas
               type: object
@@ -3039,8 +3029,6 @@ spec:
             storageSize:
               description: StorageSize is the request storage size for backup job
               type: string
-          required:
-          - from
           type: object
       type: object
   version: v1alpha1
@@ -3420,8 +3408,6 @@ spec:
                   description: StorageSize is the request storage size for backup
                     job
                   type: string
-              required:
-              - from
               type: object
             maxBackups:
               description: MaxBackups is to specify how many backups we want to keep

pkg/apis/pingcap/v1alpha1/backup.go

@@ -16,6 +16,7 @@ package v1alpha1
 import (
 	"fmt"
 
+	"github.com/pingcap/tidb-operator/pkg/label"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -40,6 +41,16 @@ 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 {
+		if v, ok := bk.Labels[label.InstanceLabelKey]; ok {
+			return v
+		}
+	}
+	return bk.Name
+}
+
 // GetBackupCondition get the specify type's BackupCondition from the given BackupStatus
 func GetBackupCondition(status *BackupStatus, conditionType BackupConditionType) (int, *BackupCondition) {
 	if status == nil {
@@ -87,6 +98,12 @@ func IsBackupComplete(backup *Backup) bool {
 	return condition != nil && condition.Status == corev1.ConditionTrue
 }
 
+// IsBackupInvalid returns true if a Backup has invalid condition set
+func IsBackupInvalid(backup *Backup) bool {
+	_, condition := GetBackupCondition(&backup.Status, BackupInvalid)
+	return condition != nil && condition.Status == corev1.ConditionTrue
+}
+
 // IsBackupFailed returns true if a Backup has failed
 func IsBackupFailed(backup *Backup) bool {
 	_, condition := GetBackupCondition(&backup.Status, BackupFailed)

pkg/apis/pingcap/v1alpha1/restore.go

@@ -16,6 +16,7 @@ package v1alpha1
 import (
 	"fmt"
 
+	"github.com/pingcap/tidb-operator/pkg/label"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -25,6 +26,16 @@ func (rs *Restore) GetRestoreJobName() string {
 	return fmt.Sprintf("restore-%s", rs.GetName())
 }
 
+// GetInstanceName return the backup instance name
+func (rs *Restore) GetInstanceName() string {
+	if rs.Labels != nil {
+		if v, ok := rs.Labels[label.InstanceLabelKey]; ok {
+			return v
+		}
+	}
+	return rs.Name
+}
+
 // GetTidbEndpointHash return the hash string base on tidb cluster's host and port
 func (rs *Restore) GetTidbEndpointHash() string {
 	return HashContents([]byte(rs.Spec.To.GetTidbEndpoint()))

pkg/apis/pingcap/v1alpha1/types.go

@@ -600,7 +600,7 @@ type TiDBAccessConfig struct {
 // BackupSpec contains the backup specification for a tidb cluster.
 type BackupSpec struct {
 	// From is the tidb cluster that needs to backup.
-	From TiDBAccessConfig `json:"from"`
+	From TiDBAccessConfig `json:"from,omitempty"`
 	// Type is the backup type for tidb cluster.
 	Type BackupType `json:"backupType,omitempty"`
 	// StorageProvider configures where and how backups should be stored.
@@ -659,6 +659,8 @@ const (
 	BackupFailed BackupConditionType = "Failed"
 	// BackupRetryFailed means this failure can be retried
 	BackupRetryFailed BackupConditionType = "RetryFailed"
+	// BackupInvalid means invalid backup CR
+	BackupInvalid BackupConditionType = "Invalid"
 )
 
 // BackupCondition describes the observed state of a Backup at a certain point.

pkg/backup/backup.go

@@ -19,6 +19,8 @@ import "github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
 type BackupManager interface {
 	// Sync	implements the logic for syncing Backup.
 	Sync(backup *v1alpha1.Backup) error
+	// UpdateCondition updates condition for backup CR status
+	UpdateCondition(backup *v1alpha1.Backup, condition *v1alpha1.BackupCondition) error
 }
 
 // RestoreManager implements the logic for manage restore.

pkg/backup/backup/backup_cleaner.go

@@ -123,7 +123,7 @@ func (bc *backupCleaner) makeCleanJob(backup *v1alpha1.Backup) (*batchv1.Job, st
 		fmt.Sprintf("--backupName=%s", name),
 	}
 
-	backupLabel := label.NewBackup().Instance(backup.Spec.From.GetTidbEndpoint()).CleanJob().Backup(name)
+	backupLabel := label.NewBackup().Instance(backup.GetInstanceName()).CleanJob().Backup(name)
 
 	podSpec := &corev1.PodTemplateSpec{
 		ObjectMeta: metav1.ObjectMeta{