backup: fix issue #2028 (#2062)

pingcap · Mar 30, 2020 · 046eee1 · 046eee1
1 parent 2aa227e
commit 046eee1
Show file tree

Hide file tree

Showing 22 changed files with 48 additions and 75 deletions.
diff --git a/cmd/backup-manager/app/backup/backup.go b/cmd/backup-manager/app/backup/backup.go
@@ -45,7 +45,7 @@ func (bo *Options) backupData(backup *v1alpha1.Backup) (string, error) {
 		return "", err
 	}
 	args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", backup.Spec.BR.Cluster, clusterNamespace))
-	if backup.Spec.BR.TLSCluster != nil && backup.Spec.BR.TLSCluster.Enabled {
+	if bo.TLSCluster {
 		args = append(args, fmt.Sprintf("--ca=%s", path.Join(util.ClusterClientTLSPath, corev1.ServiceAccountRootCAKey)))
 		args = append(args, fmt.Sprintf("--cert=%s", path.Join(util.ClusterClientTLSPath, corev1.TLSCertKey)))
 		args = append(args, fmt.Sprintf("--key=%s", path.Join(util.ClusterClientTLSPath, corev1.TLSPrivateKeyKey)))

diff --git a/cmd/backup-manager/app/backup/manager.go b/cmd/backup-manager/app/backup/manager.go
@@ -80,11 +80,6 @@ func (bm *Manager) ProcessBackup() error {
 		})
 	}
 
-	enableTLSClient := false
-	if backup.Spec.From.TLSClient != nil && backup.Spec.From.TLSClient.Enabled {
-		enableTLSClient = true
-	}
-
 	if backup.Spec.BR == nil {
 		return fmt.Errorf("no br config in %s", bm)
 	}
@@ -94,7 +89,7 @@ func (bm *Manager) ProcessBackup() error {
 	var db *sql.DB
 	var dsn string
 	err = wait.PollImmediate(constants.PollInterval, constants.CheckTimeout, func() (done bool, err error) {
-		dsn, err = bm.GetDSN(enableTLSClient)
+		dsn, err = bm.GetDSN(bm.TLSClient)
 		if err != nil {
 			klog.Errorf("can't get dsn of tidb cluster %s, err: %s", bm, err)
 			return false, err

diff --git a/cmd/backup-manager/app/cmd/backup.go b/cmd/backup-manager/app/cmd/backup.go
@@ -42,6 +42,8 @@ func NewBackupCommand() *cobra.Command {
 
 	cmd.Flags().StringVar(&bo.Namespace, "namespace", "", "Backup CR's namespace")
 	cmd.Flags().StringVar(&bo.ResourceName, "backupName", "", "Backup CRD object name")
+	cmd.Flags().BoolVar(&bo.TLSClient, "client-tls", false, "Whether client tls is enabled")
+	cmd.Flags().BoolVar(&bo.TLSCluster, "cluster-tls", false, "Whether cluster tls is enabled")
 	return cmd
 }
 

diff --git a/cmd/backup-manager/app/cmd/restore.go b/cmd/backup-manager/app/cmd/restore.go
@@ -44,6 +44,8 @@ func NewRestoreCommand() *cobra.Command {
 
 	cmd.Flags().StringVar(&ro.Namespace, "namespace", "", "Restore CR's namespace")
 	cmd.Flags().StringVar(&ro.ResourceName, "restoreName", "", "Restore CRD object name")
+	cmd.Flags().BoolVar(&ro.TLSClient, "client-tls", false, "Whether client tls is enabled")
+	cmd.Flags().BoolVar(&ro.TLSCluster, "cluster-tls", false, "Whether cluster tls is enabled")
 	return cmd
 }
 

diff --git a/cmd/backup-manager/app/restore/manager.go b/cmd/backup-manager/app/restore/manager.go
@@ -82,17 +82,12 @@ func (rm *Manager) ProcessRestore() error {
 		return fmt.Errorf("no br config in %s", rm)
 	}
 
-	enableTLSClient := false
-	if restore.Spec.To.TLSClient != nil && restore.Spec.To.TLSClient.Enabled {
-		enableTLSClient = true
-	}
-
 	rm.setOptions(restore)
 
 	var db *sql.DB
 	var dsn string
 	err = wait.PollImmediate(constants.PollInterval, constants.CheckTimeout, func() (done bool, err error) {
-		dsn, err = rm.GetDSN(enableTLSClient)
+		dsn, err = rm.GetDSN(rm.TLSClient)
 		if err != nil {
 			klog.Errorf("can't get dsn of tidb cluster %s, err: %s", rm, err)
 			return false, err

diff --git a/cmd/backup-manager/app/restore/restore.go b/cmd/backup-manager/app/restore/restore.go
@@ -39,7 +39,7 @@ func (ro *Options) restoreData(restore *v1alpha1.Restore) error {
 		return err
 	}
 	args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", restore.Spec.BR.Cluster, clusterNamespace))
-	if restore.Spec.BR.TLSCluster != nil && restore.Spec.BR.TLSCluster.Enabled {
+	if ro.TLSCluster {
 		args = append(args, fmt.Sprintf("--ca=%s", path.Join(util.ClusterClientTLSPath, corev1.ServiceAccountRootCAKey)))
 		args = append(args, fmt.Sprintf("--cert=%s", path.Join(util.ClusterClientTLSPath, corev1.TLSCertKey)))
 		args = append(args, fmt.Sprintf("--key=%s", path.Join(util.ClusterClientTLSPath, corev1.TLSPrivateKeyKey)))

diff --git a/cmd/backup-manager/app/util/generic.go b/cmd/backup-manager/app/util/generic.go
@@ -33,6 +33,8 @@ type GenericOptions struct {
 	Namespace string
 	// ResourceName can be the name of a backup or restore resource
 	ResourceName string
+	TLSClient    bool
+	TLSCluster   bool
 	Host         string
 	Port         int32
 	Password     string

diff --git a/docs/api-references/docs.html b/docs/api-references/docs.html
@@ -1559,21 +1559,6 @@ <h3 id="pingcap.com/v1alpha1.BRConfig">BRConfig
 <tbody>
 <tr>
 <td>
-<code>tlsCluster</code></br>
-<em>
-<a href="#pingcap.com/v1alpha1.TLSCluster">
-TLSCluster
-</a>
-</em>
-</td>
-<td>
-<em>(Optional)</em>
-<p>Whether enable the TLS connection between TiDB server components
-Optional: Defaults to nil</p>
-</td>
-</tr>
-<tr>
-<td>
 <code>cluster</code></br>
 <em>
 string
@@ -6995,7 +6980,6 @@ <h3 id="pingcap.com/v1alpha1.TLSCluster">TLSCluster
 </h3>
 <p>
 (<em>Appears on:</em>
-<a href="#pingcap.com/v1alpha1.BRConfig">BRConfig</a>, 
 <a href="#pingcap.com/v1alpha1.TidbClusterSpec">TidbClusterSpec</a>)
 </p>
 <p>

diff --git a/manifests/backup/backup-aws-s3-br.yaml b/manifests/backup/backup-aws-s3-br.yaml
@@ -13,8 +13,6 @@ spec:
   br:
     cluster: myCluster
     # clusterNamespce: <backup-namespace>
-    # tlsCluster:
-    #   enabled: false
     # logLevel: info
     # statusAddr: <status-addr>
     # concurrency: 4
@@ -28,7 +26,6 @@ spec:
     # port: 4000
     # user: root
     # tlsClient:
-    #   enabled: false
     #   tlsSecret: <backup-tls-secretname>
   s3:
     provider: aws

diff --git a/manifests/backup/backup-s3-br.yaml b/manifests/backup/backup-s3-br.yaml
@@ -13,8 +13,6 @@ spec:
   br:
     cluster: myCluster
     # clusterNamespce: <backup-namespace>
-    # tlsCluster:
-    #   enabled: false
     # logLevel: info
     # statusAddr: <status-addr>
     # concurrency: 4
@@ -28,7 +26,6 @@ spec:
     # port: 4000
     # user: root
     # tlsClient:
-    #   enabled: false
     #   tlsSecret: <backup-tls-secretname>
   s3:
     provider: ceph

diff --git a/manifests/backup/backup-schedule-aws-s3-br.yaml b/manifests/backup/backup-schedule-aws-s3-br.yaml
@@ -18,8 +18,6 @@ spec:
     br:
       cluster: myCluster
       # clusterNamespce: backupNamespace
-      # tlsCluster:
-      #   enabled: false
       # logLevel: info
       # statusAddr: <status-addr>
       # concurrency: 4
@@ -33,7 +31,6 @@ spec:
       # port:         4000
       # user:         root
       # tlsClient:
-      #   enabled: false
       #   tlsSecret: <backup-tls-secretname>
     s3:
       provider: aws

diff --git a/manifests/backup/backup-schedule-s3-br.yaml b/manifests/backup/backup-schedule-s3-br.yaml
@@ -18,8 +18,6 @@ spec:
     br:
       cluster: myCluster
       # clusterNamespce: backupNamespace
-      # tlsCluster:
-      #   enabled: false
       # logLevel: info
       # statusAddr: <status-addr>
       # concurrency: 4
@@ -33,7 +31,6 @@ spec:
       # port:         4000
       # user:         root
       # tlsClient:
-      #   enabled: false
       #   tlsSecret: <backup-tls-secretname>
     s3:
       provider: ceph

diff --git a/manifests/backup/restore-aws-s3-br.yaml b/manifests/backup/restore-aws-s3-br.yaml
@@ -13,8 +13,6 @@ spec:
   br:
     cluster: myCluster
     # clusterNamespce: <restore-namespace>
-    # tlsCluster:
-    #   enabled: false
     # db: <db-name>
     # table: <table-name>
     # logLevel: info
@@ -30,7 +28,6 @@ spec:
     # port: 4000
     # user: root
     # tlsClient:
-    #   enabled: false
     #   tlsSecret: <restore-tls-secretname>
   s3:
     provider: aws

diff --git a/manifests/backup/restore-s3-br.yaml b/manifests/backup/restore-s3-br.yaml
@@ -13,8 +13,6 @@ spec:
   br:
     cluster: myCluster
     # clusterNamespce: <restore-namespace>
-    # tlsCluster:
-    #   enabled: false
     # db: <db-name>
     # table: <table-name>
     # logLevel: info
@@ -30,7 +28,6 @@ spec:
     # port: 4000
     # user: root
     # tlsClient:
-    #   enabled: false
     #   tlsSecret: <restore-tls-secretname>
   s3:
     provider: ceph

diff --git a/manifests/crd.yaml b/manifests/crd.yaml
@@ -7286,7 +7286,6 @@ spec:
                   description: TimeAgo is the history version of the backup task,
                     e.g. 1m, 1h
                   type: string
-                tlsCluster: {}
               required:
               - cluster
               type: object
@@ -8129,7 +8128,6 @@ spec:
                   description: TimeAgo is the history version of the backup task,
                     e.g. 1m, 1h
                   type: string
-                tlsCluster: {}
               required:
               - cluster
               type: object
@@ -9015,7 +9013,6 @@ spec:
                       description: TimeAgo is the history version of the backup task,
                         e.g. 1m, 1h
                       type: string
-                    tlsCluster: {}
                   required:
                   - cluster
                   type: object

diff --git a/pkg/apis/pingcap/v1alpha1/openapi_generated.go b/pkg/apis/pingcap/v1alpha1/openapi_generated.go
diff --git a/pkg/apis/pingcap/v1alpha1/types.go b/pkg/apis/pingcap/v1alpha1/types.go
@@ -855,10 +855,6 @@ type BackupSpec struct {
 // +k8s:openapi-gen=true
 // BRConfig contains config for BR
 type BRConfig struct {
-	// Whether enable the TLS connection between TiDB server components
-	// Optional: Defaults to nil
-	// +optional
-	TLSCluster *TLSCluster `json:"tlsCluster,omitempty"`
 	// ClusterName of backup/restore cluster
 	Cluster string `json:"cluster"`
 	// Namespace of backup/restore cluster

diff --git a/pkg/apis/pingcap/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pingcap/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/backup/backup/backup_manager.go b/pkg/backup/backup/backup_manager.go
@@ -20,6 +20,7 @@ import (
 	"github.com/pingcap/tidb-operator/pkg/backup"
 	"github.com/pingcap/tidb-operator/pkg/backup/constants"
 	backuputil "github.com/pingcap/tidb-operator/pkg/backup/util"
+	v1alpha1listers "github.com/pingcap/tidb-operator/pkg/client/listers/pingcap/v1alpha1"
 	"github.com/pingcap/tidb-operator/pkg/controller"
 	"github.com/pingcap/tidb-operator/pkg/label"
 	"github.com/pingcap/tidb-operator/pkg/util"
@@ -39,6 +40,7 @@ type backupManager struct {
 	jobLister     batchlisters.JobLister
 	jobControl    controller.JobControlInterface
 	pvcLister     corelisters.PersistentVolumeClaimLister
+	tcLister      v1alpha1listers.TidbClusterLister
 	pvcControl    controller.GeneralPVCControlInterface
 }
 
@@ -50,6 +52,7 @@ func NewBackupManager(
 	jobLister batchlisters.JobLister,
 	jobControl controller.JobControlInterface,
 	pvcLister corelisters.PersistentVolumeClaimLister,
+	tcLister v1alpha1listers.TidbClusterLister,
 	pvcControl controller.GeneralPVCControlInterface,
 ) backup.BackupManager {
 	return &backupManager{
@@ -59,6 +62,7 @@ func NewBackupManager(
 		jobLister,
 		jobControl,
 		pvcLister,
+		tcLister,
 		pvcControl,
 	}
 }
@@ -255,6 +259,14 @@ func (bm *backupManager) makeExportJob(backup *v1alpha1.Backup) (*batchv1.Job, s
 func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, string, error) {
 	ns := backup.GetNamespace()
 	name := backup.GetName()
+	backupNamespace := ns
+	if backup.Spec.BR.ClusterNamespace != "" {
+		backupNamespace = backup.Spec.BR.ClusterNamespace
+	}
+	tc, err := bm.tcLister.TidbClusters(backupNamespace).Get(backup.Spec.BR.Cluster)
+	if err != nil {
+		return nil, fmt.Sprintf("failed to fetch tidbcluster %s/%s", backupNamespace, backup.Spec.BR.Cluster), err
+	}
 
 	envVars, reason, err := backuputil.GenerateTidbPasswordEnv(ns, name, backup.Spec.From.SecretName, backup.Spec.UseKMS, bm.secretLister)
 	if err != nil {
@@ -277,7 +289,8 @@ func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, s
 	backupLabel := label.NewBackup().Instance(backup.GetInstanceName()).BackupJob().Backup(name)
 	volumeMounts := []corev1.VolumeMount{}
 	volumes := []corev1.Volume{}
-	if backup.Spec.BR.TLSCluster != nil && backup.Spec.BR.TLSCluster.Enabled {
+	if tc.Spec.TLSCluster != nil && tc.Spec.TLSCluster.Enabled {
+		args = append(args, "--cluster-tls=true")
 		volumeMounts = append(volumeMounts, corev1.VolumeMount{
 			Name:      "cluster-client-tls",
 			ReadOnly:  true,
@@ -292,9 +305,10 @@ func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, s
 			},
 		})
 	}
-	if backup.Spec.From.TLSClient != nil && backup.Spec.From.TLSClient.Enabled {
+	if tc.Spec.TiDB.TLSClient != nil && tc.Spec.TiDB.TLSClient.Enabled {
+		args = append(args, "--client-tls=true")
 		clientSecretName := util.TiDBClientTLSSecretName(backup.Spec.BR.Cluster)
-		if backup.Spec.From.TLSClient.TLSSecret != "" {
+		if backup.Spec.From.TLSClient != nil && backup.Spec.From.TLSClient.TLSSecret != "" {
 			clientSecretName = backup.Spec.From.TLSClient.TLSSecret
 		}
 		volumeMounts = append(volumeMounts, corev1.VolumeMount{