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

backup: fix issue #2028 (#2062) #2073

Merged
merged 5 commits into from
Mar 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/backup-manager/app/backup/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
7 changes: 1 addition & 6 deletions cmd/backup-manager/app/backup/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions cmd/backup-manager/app/cmd/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 2 additions & 0 deletions cmd/backup-manager/app/cmd/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
7 changes: 1 addition & 6 deletions cmd/backup-manager/app/restore/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/backup-manager/app/restore/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
2 changes: 2 additions & 0 deletions cmd/backup-manager/app/util/generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 0 additions & 16 deletions docs/api-references/docs.html
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>
Expand Down
3 changes: 0 additions & 3 deletions manifests/backup/backup-aws-s3-br.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ spec:
br:
cluster: myCluster
# clusterNamespce: <backup-namespace>
# tlsCluster:
# enabled: false
# logLevel: info
# statusAddr: <status-addr>
# concurrency: 4
Expand All @@ -28,7 +26,6 @@ spec:
# port: 4000
# user: root
# tlsClient:
# enabled: false
# tlsSecret: <backup-tls-secretname>
s3:
provider: aws
Expand Down
3 changes: 0 additions & 3 deletions manifests/backup/backup-s3-br.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ spec:
br:
cluster: myCluster
# clusterNamespce: <backup-namespace>
# tlsCluster:
# enabled: false
# logLevel: info
# statusAddr: <status-addr>
# concurrency: 4
Expand All @@ -28,7 +26,6 @@ spec:
# port: 4000
# user: root
# tlsClient:
# enabled: false
# tlsSecret: <backup-tls-secretname>
s3:
provider: ceph
Expand Down
3 changes: 0 additions & 3 deletions manifests/backup/backup-schedule-aws-s3-br.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ spec:
br:
cluster: myCluster
# clusterNamespce: backupNamespace
# tlsCluster:
# enabled: false
# logLevel: info
# statusAddr: <status-addr>
# concurrency: 4
Expand All @@ -33,7 +31,6 @@ spec:
# port: 4000
# user: root
# tlsClient:
# enabled: false
# tlsSecret: <backup-tls-secretname>
s3:
provider: aws
Expand Down
3 changes: 0 additions & 3 deletions manifests/backup/backup-schedule-s3-br.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ spec:
br:
cluster: myCluster
# clusterNamespce: backupNamespace
# tlsCluster:
# enabled: false
# logLevel: info
# statusAddr: <status-addr>
# concurrency: 4
Expand All @@ -33,7 +31,6 @@ spec:
# port: 4000
# user: root
# tlsClient:
# enabled: false
# tlsSecret: <backup-tls-secretname>
s3:
provider: ceph
Expand Down
3 changes: 0 additions & 3 deletions manifests/backup/restore-aws-s3-br.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ spec:
br:
cluster: myCluster
# clusterNamespce: <restore-namespace>
# tlsCluster:
# enabled: false
# db: <db-name>
# table: <table-name>
# logLevel: info
Expand All @@ -30,7 +28,6 @@ spec:
# port: 4000
# user: root
# tlsClient:
# enabled: false
# tlsSecret: <restore-tls-secretname>
s3:
provider: aws
Expand Down
3 changes: 0 additions & 3 deletions manifests/backup/restore-s3-br.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ spec:
br:
cluster: myCluster
# clusterNamespce: <restore-namespace>
# tlsCluster:
# enabled: false
# db: <db-name>
# table: <table-name>
# logLevel: info
Expand All @@ -30,7 +28,6 @@ spec:
# port: 4000
# user: root
# tlsClient:
# enabled: false
# tlsSecret: <restore-tls-secretname>
s3:
provider: ceph
Expand Down
3 changes: 0 additions & 3 deletions manifests/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions pkg/apis/pingcap/v1alpha1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions pkg/apis/pingcap/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/pingcap/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 17 additions & 3 deletions pkg/backup/backup/backup_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -39,6 +40,7 @@ type backupManager struct {
jobLister batchlisters.JobLister
jobControl controller.JobControlInterface
pvcLister corelisters.PersistentVolumeClaimLister
tcLister v1alpha1listers.TidbClusterLister
pvcControl controller.GeneralPVCControlInterface
}

Expand All @@ -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{
Expand All @@ -59,6 +62,7 @@ func NewBackupManager(
jobLister,
jobControl,
pvcLister,
tcLister,
pvcControl,
}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand All @@ -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{
Expand Down
Loading