From 627f3fdd952600a4c08cb8eb6535bb90c2a34504 Mon Sep 17 00:00:00 2001 From: shuijing198799 Date: Fri, 28 Feb 2020 18:26:15 +0800 Subject: [PATCH 1/8] backup: add TLS to backup br --- cmd/backup-manager/app/backup/backup.go | 28 ++++++++++--- cmd/backup-manager/app/constants/constants.go | 6 +++ cmd/backup-manager/app/restore/restore.go | 17 ++++++++ cmd/backup-manager/app/util/util.go | 14 +------ pkg/apis/pingcap/v1alpha1/types.go | 28 +++++++++---- pkg/backup/backup/backup_manager.go | 42 ++++++++++++------- .../backupschedule/backup_schedule_manager.go | 14 ++++++- pkg/backup/constants/constants.go | 6 +++ pkg/backup/restore/restore_manager.go | 42 ++++++++++++------- pkg/backup/util/util.go | 9 ++-- 10 files changed, 147 insertions(+), 59 deletions(-) diff --git a/cmd/backup-manager/app/backup/backup.go b/cmd/backup-manager/app/backup/backup.go index 7d9f0729a5..093e21d1c9 100644 --- a/cmd/backup-manager/app/backup/backup.go +++ b/cmd/backup-manager/app/backup/backup.go @@ -18,6 +18,7 @@ import ( "fmt" "io" "os/exec" + "path" "github.com/gogo/protobuf/proto" glog "k8s.io/klog" @@ -39,10 +40,25 @@ func (bo *Options) String() string { } func (bo *Options) backupData(backup *v1alpha1.Backup) (string, error) { - args, path, err := constructOptions(backup) + var backupNamespace string + args, remotePath, err := constructOptions(backup) if err != nil { return "", err } + if backup.Spec.BackupNamespace == "" { + backupNamespace = bo.Namespace + } else { + backupNamespace = backup.Spec.BackupNamespace + } + if backup.Spec.EnableTLSClient { + args = append(args, fmt.Sprintf("--pd=https://%s-pd.%s", backup.Spec.Cluster, backupNamespace)) + args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) + args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, "cert"))) + args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, "key"))) + } else { + args = append(args, fmt.Sprintf("--pd=http://%s-pd.%s", backup.Spec.Cluster, backupNamespace)) + } + var btype string if backup.Spec.Type == "" { btype = string(v1alpha1.BackupTypeFull) @@ -57,10 +73,10 @@ func (bo *Options) backupData(backup *v1alpha1.Backup) (string, error) { glog.Infof("Running br command with args: %v", fullArgs) output, err := exec.Command("br", fullArgs...).CombinedOutput() if err != nil { - return path, fmt.Errorf("cluster %s, execute br command %v failed, output: %s, err: %v", bo, fullArgs, string(output), err) + return remotePath, fmt.Errorf("cluster %s, execute br command %v failed, output: %s, err: %v", bo, fullArgs, string(output), err) } glog.Infof("Backup data for cluster %s successfully, output: %s", bo, string(output)) - return path, nil + return remotePath, nil } // getCommitTs get backup position from `EndVersion` in BR backup meta @@ -94,9 +110,9 @@ func getCommitTs(backup *v1alpha1.Backup) (uint64, error) { // constructOptions constructs options for BR and also return the remote path func constructOptions(backup *v1alpha1.Backup) ([]string, string, error) { - args, path, err := util.ConstructBRGlobalOptionsForBackup(backup) + args, remotePath, err := util.ConstructBRGlobalOptionsForBackup(backup) if err != nil { - return args, path, err + return args, remotePath, err } config := backup.Spec.BR if config.Concurrency != nil { @@ -111,7 +127,7 @@ func constructOptions(backup *v1alpha1.Backup) ([]string, string, error) { if config.Checksum != nil { args = append(args, fmt.Sprintf("--checksum=%t", *config.Checksum)) } - return args, path, nil + return args, remotePath, nil } // getBackupSize get the backup data size from remote diff --git a/cmd/backup-manager/app/constants/constants.go b/cmd/backup-manager/app/constants/constants.go index 24b5ad7d30..c471181138 100644 --- a/cmd/backup-manager/app/constants/constants.go +++ b/cmd/backup-manager/app/constants/constants.go @@ -56,4 +56,10 @@ const ( // MetaFile is the file name for meta data of backup with BR MetaFile = "backupmeta" + + // BR certificate storage path + BRCertPath = "/var/lib/br-tls" + + // ServiceAccountCAPath is where is CABundle of serviceaccount locates + ServiceAccountCAPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" ) diff --git a/cmd/backup-manager/app/restore/restore.go b/cmd/backup-manager/app/restore/restore.go index 200c013753..53103de9bf 100644 --- a/cmd/backup-manager/app/restore/restore.go +++ b/cmd/backup-manager/app/restore/restore.go @@ -16,9 +16,11 @@ package restore import ( "fmt" "os/exec" + "path" glog "k8s.io/klog" + "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" ) @@ -33,10 +35,25 @@ func (ro *Options) String() string { } func (ro *Options) restoreData(restore *v1alpha1.Restore) error { + var restoreNamespace string args, err := constructBROptions(restore) if err != nil { return err } + if restore.Spec.RestoreNamespace == "" { + restoreNamespace = ro.Namespace + } else { + restoreNamespace = restore.Spec.RestoreNamespace + } + if restore.Spec.EnableTLSClient { + args = append(args, fmt.Sprintf("--pd=https://%s-pd.%s", restore.Spec.Cluster, restoreNamespace)) + args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) + args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, "cert"))) + args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, "key"))) + } else { + args = append(args, fmt.Sprintf("--pd=http://%s-pd.%s", restore.Spec.Cluster, restoreNamespace)) + } + var restoreType string if restore.Spec.Type == "" { restoreType = string(v1alpha1.BackupTypeFull) diff --git a/cmd/backup-manager/app/util/util.go b/cmd/backup-manager/app/util/util.go index e7d436c14c..b71883e731 100644 --- a/cmd/backup-manager/app/util/util.go +++ b/cmd/backup-manager/app/util/util.go @@ -119,7 +119,7 @@ func ConstructBRGlobalOptionsForBackup(backup *v1alpha1.Backup) ([]string, strin return nil, "", fmt.Errorf("no config for br in backup %s/%s", backup.Namespace, backup.Name) } args = append(args, constructBRGlobalOptions(config)...) - storageArgs, path, err := getRemoteStorage(backup.Spec.StorageProvider) + storageArgs, remotePath, err := getRemoteStorage(backup.Spec.StorageProvider) if err != nil { return nil, "", err } @@ -130,7 +130,7 @@ func ConstructBRGlobalOptionsForBackup(backup *v1alpha1.Backup) ([]string, strin if backup.Spec.Type == v1alpha1.BackupTypeTable && config.Table != "" { args = append(args, fmt.Sprintf("--table=%s", config.Table)) } - return args, path, nil + return args, remotePath, nil } // ConstructBRGlobalOptionsForRestore constructs BR global options for restore. @@ -158,16 +158,6 @@ func ConstructBRGlobalOptionsForRestore(restore *v1alpha1.Restore) ([]string, er // constructBRGlobalOptions constructs BR basic global options. func constructBRGlobalOptions(config *v1alpha1.BRConfig) []string { var args []string - args = append(args, fmt.Sprintf("--pd=%s", config.PDAddress)) - if config.CA != "" { - args = append(args, fmt.Sprintf("--ca=%s", config.CA)) - } - if config.Cert != "" { - args = append(args, fmt.Sprintf("--cert=%s", config.Cert)) - } - if config.Key != "" { - args = append(args, fmt.Sprintf("--key=%s", config.Key)) - } if config.LogLevel != "" { args = append(args, fmt.Sprintf("--log-level=%s", config.LogLevel)) } diff --git a/pkg/apis/pingcap/v1alpha1/types.go b/pkg/apis/pingcap/v1alpha1/types.go index 80b26c0f5c..0e217ec5fb 100644 --- a/pkg/apis/pingcap/v1alpha1/types.go +++ b/pkg/apis/pingcap/v1alpha1/types.go @@ -781,23 +781,25 @@ type BackupSpec struct { // Affinity of backup Pods // +optional Affinity *corev1.Affinity `json:"affinity,omitempty"` + // Whether enable TLS in TiDBCluster + // Optional: Defaults to false + // +optional + EnableTLSClient bool `json:"enableTLSClient,omitempty"` + // ClusterName of backup cluster + // Required + Cluster string `json:"cluster,omitempty"` + // Namespace of backup cluster + // Optional: Defaults to namespace of backup job + BackupNamespace string `json:"backupNamespace,omitempty"` } // +k8s:openapi-gen=true // BRConfig contains config for BR type BRConfig struct { - // PDAddress is the PD address of the tidb cluster - PDAddress string `json:"pd"` // DB is the specific DB which will be backed-up or restored DB string `json:"db,omitempty"` // Table is the specific table which will be backed-up or restored Table string `json:"table,omitempty"` - // CA is the CA certificate path for TLS connection - CA string `json:"ca,omitempty"` - // Cert is the certificate path for TLS connection - Cert string `json:"cert,omitempty"` - // Key is the private key path for TLS connection - Key string `json:"key,omitempty"` // LogLevel is the log level LogLevel string `json:"logLevel,omitempty"` // StatusAddr is the HTTP listening address for the status report service. Set to empty string to disable @@ -997,6 +999,16 @@ type RestoreSpec struct { // Affinity of restore Pods // +optional Affinity *corev1.Affinity `json:"affinity,omitempty"` + // Whether enable TLS in TiDBCluster + // Optional: Defaults to false + // +optional + EnableTLSClient bool `json:"enableTLSClient,omitempty"` + // ClusterName of restore cluster + // Required + Cluster string `json:"cluster,omitempty"` + // Namespace of restore cluster + // Optional: Defaults to namespace of restore job + RestoreNamespace string `json:"restoreNamespace,omitempty"` } // RestoreStatus represents the current status of a tidb cluster restore. diff --git a/pkg/backup/backup/backup_manager.go b/pkg/backup/backup/backup_manager.go index 9cbf246041..2870dc0728 100644 --- a/pkg/backup/backup/backup_manager.go +++ b/pkg/backup/backup/backup_manager.go @@ -197,6 +197,31 @@ func (bm *backupManager) makeExportJob(backup *v1alpha1.Backup) (*batchv1.Job, s } backupLabel := label.NewBackup().Instance(backup.GetInstanceName()).BackupJob().Backup(name) + volumeMounts := []corev1.VolumeMount{ + {Name: label.BackupJobLabelVal, MountPath: constants.BackupRootPath}, + } + volumes := []corev1.Volume{ + { + Name: label.BackupJobLabelVal, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: backup.GetBackupPVCName(), + }, + }, + }, + } + if backup.Spec.EnableTLSClient { + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "br-tls", ReadOnly: true, MountPath: constants.BRCertPath, + }) + volumes = append(volumes, corev1.Volume{ + Name: "br-tls", VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(backup.Spec.Cluster)), + }, + }, + }) + } // TODO: need add ResourceRequirement for backup job podSpec := &corev1.PodTemplateSpec{ @@ -211,25 +236,14 @@ func (bm *backupManager) makeExportJob(backup *v1alpha1.Backup) (*batchv1.Job, s Image: controller.TidbBackupManagerImage, Args: args, ImagePullPolicy: corev1.PullAlways, - VolumeMounts: []corev1.VolumeMount{ - {Name: label.BackupJobLabelVal, MountPath: constants.BackupRootPath}, - }, - Env: envVars, + VolumeMounts: volumeMounts, + Env: envVars, }, }, RestartPolicy: corev1.RestartPolicyNever, Affinity: backup.Spec.Affinity, Tolerations: backup.Spec.Tolerations, - Volumes: []corev1.Volume{ - { - Name: label.BackupJobLabelVal, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: backup.GetBackupPVCName(), - }, - }, - }, - }, + Volumes: volumes, }, } diff --git a/pkg/backup/backupschedule/backup_schedule_manager.go b/pkg/backup/backupschedule/backup_schedule_manager.go index 9725ac21e5..2ff95f25e5 100644 --- a/pkg/backup/backupschedule/backup_schedule_manager.go +++ b/pkg/backup/backupschedule/backup_schedule_manager.go @@ -215,9 +215,21 @@ func (bm *backupScheduleManager) createBackup(bs *v1alpha1.BackupSchedule, times } } } else { + var backupNamespace, pdAddress string + if backupSpec.BackupNamespace == "" { + backupNamespace = ns + } else { + backupNamespace = backupSpec.BackupNamespace + } + if backupSpec.EnableTLSClient { + pdAddress = fmt.Sprintf("https://%s-pd.%s", backupSpec.Cluster, backupNamespace) + } else { + pdAddress = fmt.Sprintf("http://%s-pd.%s", backupSpec.Cluster, backupNamespace) + } + if backupSpec.S3 != nil { backupSpec.S3.Prefix = path.Join(backupSpec.S3.Prefix, - strings.ReplaceAll(backupSpec.BR.PDAddress, ":", "-")+"-"+timestamp.UTC().Format(constants.TimeFormat)) + strings.ReplaceAll(pdAddress, ":", "-")+"-"+timestamp.UTC().Format(constants.TimeFormat)) } } diff --git a/pkg/backup/constants/constants.go b/pkg/backup/constants/constants.go index 82ad258a12..ba7d8d706d 100644 --- a/pkg/backup/constants/constants.go +++ b/pkg/backup/constants/constants.go @@ -49,4 +49,10 @@ const ( // BackupManagerEnvVarPrefix represents the environment variable used for tidb-backup-manager must include this prefix BackupManagerEnvVarPrefix = "BACKUP_MANAGER" + + // BR certificate storage path + BRCertPath = "/var/lib/br-tls" + + // ServiceAccountCAPath is where is CABundle of serviceaccount locates + ServiceAccountCAPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" ) diff --git a/pkg/backup/restore/restore_manager.go b/pkg/backup/restore/restore_manager.go index 619d719e0f..74e5357f7f 100644 --- a/pkg/backup/restore/restore_manager.go +++ b/pkg/backup/restore/restore_manager.go @@ -181,6 +181,31 @@ func (rm *restoreManager) makeImportJob(restore *v1alpha1.Restore) (*batchv1.Job } restoreLabel := label.NewBackup().Instance(restore.GetInstanceName()).RestoreJob().Restore(name) + volumeMounts := []corev1.VolumeMount{ + {Name: label.RestoreJobLabelVal, MountPath: constants.BackupRootPath}, + } + volumes := []corev1.Volume{ + { + Name: label.RestoreJobLabelVal, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: restore.GetRestorePVCName(), + }, + }, + }, + } + if restore.Spec.EnableTLSClient { + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "br-tls", ReadOnly: true, MountPath: constants.BRCertPath, + }) + volumes = append(volumes, corev1.Volume{ + Name: "br-tls", VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(restore.Spec.Cluster)), + }, + }, + }) + } // TODO: need add ResourceRequirement for restore job podSpec := &corev1.PodTemplateSpec{ @@ -195,25 +220,14 @@ func (rm *restoreManager) makeImportJob(restore *v1alpha1.Restore) (*batchv1.Job Image: controller.TidbBackupManagerImage, Args: args, ImagePullPolicy: corev1.PullAlways, - VolumeMounts: []corev1.VolumeMount{ - {Name: label.RestoreJobLabelVal, MountPath: constants.BackupRootPath}, - }, - Env: envVars, + VolumeMounts: volumeMounts, + Env: envVars, }, }, RestartPolicy: corev1.RestartPolicyNever, Affinity: restore.Spec.Affinity, Tolerations: restore.Spec.Tolerations, - Volumes: []corev1.Volume{ - { - Name: label.RestoreJobLabelVal, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: restore.GetRestorePVCName(), - }, - }, - }, - }, + Volumes: volumes, }, } diff --git a/pkg/backup/util/util.go b/pkg/backup/util/util.go index b22ebdefad..7a90bb8b45 100644 --- a/pkg/backup/util/util.go +++ b/pkg/backup/util/util.go @@ -298,9 +298,10 @@ func ValidateBackup(backup *v1alpha1.Backup) error { return fmt.Errorf("missing StorageSize config in spec of %s/%s", ns, name) } } else { - if backup.Spec.BR.PDAddress == "" { - return fmt.Errorf("pd address should be configured for BR in spec of %s/%s", ns, name) + if backup.Spec.Cluster == "" { + return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) } + if backup.Spec.Type != "" && backup.Spec.Type != v1alpha1.BackupTypeFull && backup.Spec.Type != v1alpha1.BackupTypeDB && @@ -349,8 +350,8 @@ func ValidateRestore(restore *v1alpha1.Restore) error { return fmt.Errorf("missing StorageSize config in spec of %s/%s", ns, name) } } else { - if restore.Spec.BR.PDAddress == "" { - return fmt.Errorf("pd address should be configured for BR in spec of %s/%s", ns, name) + if restore.Spec.Cluster == "" { + return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) } if restore.Spec.Type != "" && restore.Spec.Type != v1alpha1.BackupTypeFull && From 58d05da82975e8c0a98a3ecbee40d82358d0e46c Mon Sep 17 00:00:00 2001 From: shuijing198799 Date: Mon, 2 Mar 2020 10:25:34 +0800 Subject: [PATCH 2/8] backup: support tls for BR fix errors --- cmd/backup-manager/app/backup/backup.go | 4 +- cmd/backup-manager/app/restore/restore.go | 5 +- pkg/backup/backup/backup_manager.go | 99 +++++++++++-------- .../backupschedule/backup_schedule_manager.go | 6 +- 4 files changed, 60 insertions(+), 54 deletions(-) diff --git a/cmd/backup-manager/app/backup/backup.go b/cmd/backup-manager/app/backup/backup.go index 093e21d1c9..c19e5e854c 100644 --- a/cmd/backup-manager/app/backup/backup.go +++ b/cmd/backup-manager/app/backup/backup.go @@ -50,13 +50,11 @@ func (bo *Options) backupData(backup *v1alpha1.Backup) (string, error) { } else { backupNamespace = backup.Spec.BackupNamespace } + args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", backup.Spec.Cluster, backupNamespace)) if backup.Spec.EnableTLSClient { - args = append(args, fmt.Sprintf("--pd=https://%s-pd.%s", backup.Spec.Cluster, backupNamespace)) args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, "cert"))) args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, "key"))) - } else { - args = append(args, fmt.Sprintf("--pd=http://%s-pd.%s", backup.Spec.Cluster, backupNamespace)) } var btype string diff --git a/cmd/backup-manager/app/restore/restore.go b/cmd/backup-manager/app/restore/restore.go index 53103de9bf..db0c055a6c 100644 --- a/cmd/backup-manager/app/restore/restore.go +++ b/cmd/backup-manager/app/restore/restore.go @@ -45,13 +45,12 @@ func (ro *Options) restoreData(restore *v1alpha1.Restore) error { } else { restoreNamespace = restore.Spec.RestoreNamespace } + + args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", restore.Spec.Cluster, restoreNamespace)) if restore.Spec.EnableTLSClient { - args = append(args, fmt.Sprintf("--pd=https://%s-pd.%s", restore.Spec.Cluster, restoreNamespace)) args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, "cert"))) args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, "key"))) - } else { - args = append(args, fmt.Sprintf("--pd=http://%s-pd.%s", restore.Spec.Cluster, restoreNamespace)) } var restoreType string diff --git a/pkg/backup/backup/backup_manager.go b/pkg/backup/backup/backup_manager.go index 2870dc0728..8302d8b00a 100644 --- a/pkg/backup/backup/backup_manager.go +++ b/pkg/backup/backup/backup_manager.go @@ -29,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" batchlisters "k8s.io/client-go/listers/batch/v1" corelisters "k8s.io/client-go/listers/core/v1" + glog "k8s.io/klog" ) type backupManager struct { @@ -116,18 +117,6 @@ func (bm *backupManager) syncBackupJob(backup *v1alpha1.Backup) error { }) return err } - - reason, err = bm.ensureBackupPVCExist(backup) - if err != nil { - bm.statusUpdater.Update(backup, &v1alpha1.BackupCondition{ - Type: v1alpha1.BackupRetryFailed, - Status: corev1.ConditionTrue, - Reason: reason, - Message: err.Error(), - }) - return err - } - } else { // not found backup job, so we need to create it job, reason, err = bm.makeBackupJob(backup) @@ -142,6 +131,17 @@ func (bm *backupManager) syncBackupJob(backup *v1alpha1.Backup) error { } } + reason, err = bm.ensureBackupPVCExist(backup) + if err != nil { + bm.statusUpdater.Update(backup, &v1alpha1.BackupCondition{ + Type: v1alpha1.BackupRetryFailed, + Status: corev1.ConditionTrue, + Reason: reason, + Message: err.Error(), + }) + return err + } + if err := bm.jobControl.CreateJob(backup, job); err != nil { errMsg := fmt.Errorf("create backup %s/%s job %s failed, err: %v", ns, name, backupJobName, err) bm.statusUpdater.Update(backup, &v1alpha1.BackupCondition{ @@ -197,32 +197,6 @@ func (bm *backupManager) makeExportJob(backup *v1alpha1.Backup) (*batchv1.Job, s } backupLabel := label.NewBackup().Instance(backup.GetInstanceName()).BackupJob().Backup(name) - volumeMounts := []corev1.VolumeMount{ - {Name: label.BackupJobLabelVal, MountPath: constants.BackupRootPath}, - } - volumes := []corev1.Volume{ - { - Name: label.BackupJobLabelVal, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: backup.GetBackupPVCName(), - }, - }, - }, - } - if backup.Spec.EnableTLSClient { - volumeMounts = append(volumeMounts, corev1.VolumeMount{ - Name: "br-tls", ReadOnly: true, MountPath: constants.BRCertPath, - }) - volumes = append(volumes, corev1.Volume{ - Name: "br-tls", VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(backup.Spec.Cluster)), - }, - }, - }) - } - // TODO: need add ResourceRequirement for backup job podSpec := &corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ @@ -235,15 +209,26 @@ func (bm *backupManager) makeExportJob(backup *v1alpha1.Backup) (*batchv1.Job, s Name: label.BackupJobLabelVal, Image: controller.TidbBackupManagerImage, Args: args, - ImagePullPolicy: corev1.PullAlways, - VolumeMounts: volumeMounts, - Env: envVars, + ImagePullPolicy: corev1.PullIfNotPresent, + VolumeMounts: []corev1.VolumeMount{ + {Name: label.BackupJobLabelVal, MountPath: constants.BackupRootPath}, + }, + Env: envVars, }, }, RestartPolicy: corev1.RestartPolicyNever, Affinity: backup.Spec.Affinity, Tolerations: backup.Spec.Tolerations, - Volumes: volumes, + Volumes: []corev1.Volume{ + { + Name: label.BackupJobLabelVal, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: backup.GetBackupPVCName(), + }, + }, + }, + }, }, } @@ -265,6 +250,7 @@ func (bm *backupManager) makeExportJob(backup *v1alpha1.Backup) (*batchv1.Job, s return job, "", nil } func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, string, error) { + glog.Info("by jony %#v", backup) ns := backup.GetNamespace() name := backup.GetName() @@ -280,6 +266,31 @@ func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, s } backupLabel := label.NewBackup().Instance(backup.GetInstanceName()).BackupJob().Backup(name) + volumeMounts := []corev1.VolumeMount{ + {Name: label.BackupJobLabelVal, MountPath: constants.BackupRootPath}, + } + volumes := []corev1.Volume{ + { + Name: label.BackupJobLabelVal, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: backup.GetBackupPVCName(), + }, + }, + }, + } + if backup.Spec.EnableTLSClient { + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "br-tls", ReadOnly: true, MountPath: constants.BRCertPath, + }) + volumes = append(volumes, corev1.Volume{ + Name: "br-tls", VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(backup.Spec.Cluster)), + }, + }, + }) + } podSpec := &corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ @@ -292,13 +303,15 @@ func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, s Name: label.BackupJobLabelVal, Image: controller.TidbBackupManagerImage, Args: args, - ImagePullPolicy: corev1.PullAlways, + ImagePullPolicy: corev1.PullIfNotPresent, + VolumeMounts: volumeMounts, Env: envVars, }, }, RestartPolicy: corev1.RestartPolicyNever, Affinity: backup.Spec.Affinity, Tolerations: backup.Spec.Tolerations, + Volumes: volumes, }, } diff --git a/pkg/backup/backupschedule/backup_schedule_manager.go b/pkg/backup/backupschedule/backup_schedule_manager.go index 2ff95f25e5..2a3222bf6c 100644 --- a/pkg/backup/backupschedule/backup_schedule_manager.go +++ b/pkg/backup/backupschedule/backup_schedule_manager.go @@ -221,11 +221,7 @@ func (bm *backupScheduleManager) createBackup(bs *v1alpha1.BackupSchedule, times } else { backupNamespace = backupSpec.BackupNamespace } - if backupSpec.EnableTLSClient { - pdAddress = fmt.Sprintf("https://%s-pd.%s", backupSpec.Cluster, backupNamespace) - } else { - pdAddress = fmt.Sprintf("http://%s-pd.%s", backupSpec.Cluster, backupNamespace) - } + pdAddress = fmt.Sprintf("%s-pd.%s:2379", backupSpec.Cluster, backupNamespace) if backupSpec.S3 != nil { backupSpec.S3.Prefix = path.Join(backupSpec.S3.Prefix, From af91935a883152ab892c8c1322316a984f668868 Mon Sep 17 00:00:00 2001 From: shuijing198799 Date: Mon, 2 Mar 2020 11:02:36 +0800 Subject: [PATCH 3/8] backup: fix restore --- cmd/backup-manager/app/backup/backup.go | 2 +- pkg/backup/backup/backup_manager.go | 17 +------ pkg/backup/restore/restore_manager.go | 59 +++++++++++++------------ 3 files changed, 33 insertions(+), 45 deletions(-) diff --git a/cmd/backup-manager/app/backup/backup.go b/cmd/backup-manager/app/backup/backup.go index 93b560d92b..d8c753cff5 100644 --- a/cmd/backup-manager/app/backup/backup.go +++ b/cmd/backup-manager/app/backup/backup.go @@ -73,7 +73,7 @@ func (bo *Options) backupData(backup *v1alpha1.Backup) (string, error) { if err != nil { return remotePath, fmt.Errorf("cluster %s, execute br command %v failed, output: %s, err: %v", bo, fullArgs, string(output), err) } - glog.Infof("Backup data for cluster %s successfully, output: %s", bo, string(output)) + klog.Infof("Backup data for cluster %s successfully, output: %s", bo, string(output)) return remotePath, nil } diff --git a/pkg/backup/backup/backup_manager.go b/pkg/backup/backup/backup_manager.go index 8302d8b00a..52d6c3595c 100644 --- a/pkg/backup/backup/backup_manager.go +++ b/pkg/backup/backup/backup_manager.go @@ -29,7 +29,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" batchlisters "k8s.io/client-go/listers/batch/v1" corelisters "k8s.io/client-go/listers/core/v1" - glog "k8s.io/klog" ) type backupManager struct { @@ -250,7 +249,6 @@ func (bm *backupManager) makeExportJob(backup *v1alpha1.Backup) (*batchv1.Job, s return job, "", nil } func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, string, error) { - glog.Info("by jony %#v", backup) ns := backup.GetNamespace() name := backup.GetName() @@ -266,19 +264,8 @@ func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, s } backupLabel := label.NewBackup().Instance(backup.GetInstanceName()).BackupJob().Backup(name) - volumeMounts := []corev1.VolumeMount{ - {Name: label.BackupJobLabelVal, MountPath: constants.BackupRootPath}, - } - volumes := []corev1.Volume{ - { - Name: label.BackupJobLabelVal, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: backup.GetBackupPVCName(), - }, - }, - }, - } + volumeMounts := []corev1.VolumeMount{} + volumes := []corev1.Volume{} if backup.Spec.EnableTLSClient { volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: "br-tls", ReadOnly: true, MountPath: constants.BRCertPath, diff --git a/pkg/backup/restore/restore_manager.go b/pkg/backup/restore/restore_manager.go index 74e5357f7f..e043fe1c81 100644 --- a/pkg/backup/restore/restore_manager.go +++ b/pkg/backup/restore/restore_manager.go @@ -181,31 +181,6 @@ func (rm *restoreManager) makeImportJob(restore *v1alpha1.Restore) (*batchv1.Job } restoreLabel := label.NewBackup().Instance(restore.GetInstanceName()).RestoreJob().Restore(name) - volumeMounts := []corev1.VolumeMount{ - {Name: label.RestoreJobLabelVal, MountPath: constants.BackupRootPath}, - } - volumes := []corev1.Volume{ - { - Name: label.RestoreJobLabelVal, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: restore.GetRestorePVCName(), - }, - }, - }, - } - if restore.Spec.EnableTLSClient { - volumeMounts = append(volumeMounts, corev1.VolumeMount{ - Name: "br-tls", ReadOnly: true, MountPath: constants.BRCertPath, - }) - volumes = append(volumes, corev1.Volume{ - Name: "br-tls", VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(restore.Spec.Cluster)), - }, - }, - }) - } // TODO: need add ResourceRequirement for restore job podSpec := &corev1.PodTemplateSpec{ @@ -220,14 +195,25 @@ func (rm *restoreManager) makeImportJob(restore *v1alpha1.Restore) (*batchv1.Job Image: controller.TidbBackupManagerImage, Args: args, ImagePullPolicy: corev1.PullAlways, - VolumeMounts: volumeMounts, - Env: envVars, + VolumeMounts: []corev1.VolumeMount{ + {Name: label.RestoreJobLabelVal, MountPath: constants.BackupRootPath}, + }, + Env: envVars, }, }, RestartPolicy: corev1.RestartPolicyNever, Affinity: restore.Spec.Affinity, Tolerations: restore.Spec.Tolerations, - Volumes: volumes, + Volumes: []corev1.Volume{ + { + Name: label.RestoreJobLabelVal, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: restore.GetRestorePVCName(), + }, + }, + }, + }, }, } @@ -264,7 +250,20 @@ func (rm *restoreManager) makeRestoreJob(restore *v1alpha1.Restore) (*batchv1.Jo } restoreLabel := label.NewBackup().Instance(restore.GetInstanceName()).RestoreJob().Restore(name) - + volumeMounts := []corev1.VolumeMount{} + volumes := []corev1.Volume{} + if restore.Spec.EnableTLSClient { + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "br-tls", ReadOnly: true, MountPath: constants.BRCertPath, + }) + volumes = append(volumes, corev1.Volume{ + Name: "br-tls", VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(restore.Spec.Cluster)), + }, + }, + }) + } // TODO: need add ResourceRequirement for restore job podSpec := &corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ @@ -278,9 +277,11 @@ func (rm *restoreManager) makeRestoreJob(restore *v1alpha1.Restore) (*batchv1.Jo Image: controller.TidbBackupManagerImage, Args: args, ImagePullPolicy: corev1.PullAlways, + VolumeMounts: volumeMounts, Env: envVars, }, }, + Volumes: volumes, RestartPolicy: corev1.RestartPolicyNever, }, } From 2e457ac8f4c26771f1e20ff3563a811c2efa1218 Mon Sep 17 00:00:00 2001 From: shuijing198799 Date: Mon, 2 Mar 2020 12:00:44 +0800 Subject: [PATCH 4/8] backup: br generator crd and openapi --- cmd/backup-manager/app/backup/backup.go | 10 +-- cmd/backup-manager/app/restore/restore.go | 11 +-- manifests/crd.yaml | 69 ++++++++----------- .../pingcap/v1alpha1/openapi_generated.go | 30 +++----- pkg/apis/pingcap/v1alpha1/types.go | 26 ++----- pkg/backup/backup/backup_manager.go | 8 +-- .../backupschedule/backup_schedule_manager.go | 9 +-- pkg/backup/restore/restore_manager.go | 4 +- pkg/backup/util/util.go | 18 +++-- 9 files changed, 70 insertions(+), 115 deletions(-) diff --git a/cmd/backup-manager/app/backup/backup.go b/cmd/backup-manager/app/backup/backup.go index d8c753cff5..d21099ed14 100644 --- a/cmd/backup-manager/app/backup/backup.go +++ b/cmd/backup-manager/app/backup/backup.go @@ -40,18 +40,12 @@ func (bo *Options) String() string { } func (bo *Options) backupData(backup *v1alpha1.Backup) (string, error) { - var backupNamespace string args, remotePath, err := constructOptions(backup) if err != nil { return "", err } - if backup.Spec.BackupNamespace == "" { - backupNamespace = bo.Namespace - } else { - backupNamespace = backup.Spec.BackupNamespace - } - args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", backup.Spec.Cluster, backupNamespace)) - if backup.Spec.EnableTLSClient { + args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", backup.Spec.BR.Cluster, backup.Spec.BR.ClusterNamespace)) + if backup.Spec.BR.EnableTLSClient { args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, "cert"))) args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, "key"))) diff --git a/cmd/backup-manager/app/restore/restore.go b/cmd/backup-manager/app/restore/restore.go index d569ef19d3..0923a5d3f6 100644 --- a/cmd/backup-manager/app/restore/restore.go +++ b/cmd/backup-manager/app/restore/restore.go @@ -35,19 +35,12 @@ func (ro *Options) String() string { } func (ro *Options) restoreData(restore *v1alpha1.Restore) error { - var restoreNamespace string args, err := constructBROptions(restore) if err != nil { return err } - if restore.Spec.RestoreNamespace == "" { - restoreNamespace = ro.Namespace - } else { - restoreNamespace = restore.Spec.RestoreNamespace - } - - args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", restore.Spec.Cluster, restoreNamespace)) - if restore.Spec.EnableTLSClient { + args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", restore.Spec.BR.Cluster, restore.Spec.BR.ClusterNamespace)) + if restore.Spec.BR.EnableTLSClient { args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, "cert"))) args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, "key"))) diff --git a/manifests/crd.yaml b/manifests/crd.yaml index 8f1351d5ce..44ca4f3037 100644 --- a/manifests/crd.yaml +++ b/manifests/crd.yaml @@ -6742,15 +6742,15 @@ spec: br: description: BRConfig contains config for BR properties: - ca: - description: CA is the CA certificate path for TLS connection - type: string - cert: - description: Cert is the certificate path for TLS connection - type: string checksum: description: Checksum specifies whether to run checksum after backup type: boolean + cluster: + description: ClusterName of backup cluster + type: string + clusterNamespace: + description: Namespace of backup/restore cluster + type: string concurrency: description: Concurrency is the size of thread pool on each node that execute the backup task @@ -6759,18 +6759,15 @@ spec: db: description: DB is the specific DB which will be backed-up or restored type: string - key: - description: Key is the private key path for TLS connection - type: string + enableTLSClient: + description: Whether enable TLS in TiDBCluster + type: boolean logLevel: description: LogLevel is the log level type: string onLine: description: OnLine specifies whether online during restore type: boolean - pd: - description: PDAddress is the PD address of the tidb cluster - type: string rateLimit: description: RateLimit is the rate limit of the backup task, MB/s per node @@ -6792,8 +6789,6 @@ spec: description: TimeAgo is the history version of the backup task, e.g. 1m, 1h type: string - required: - - pd type: object from: description: TiDBAccessConfig defines the configuration for access tidb @@ -7578,15 +7573,15 @@ spec: br: description: BRConfig contains config for BR properties: - ca: - description: CA is the CA certificate path for TLS connection - type: string - cert: - description: Cert is the certificate path for TLS connection - type: string checksum: description: Checksum specifies whether to run checksum after backup type: boolean + cluster: + description: ClusterName of backup cluster + type: string + clusterNamespace: + description: Namespace of backup/restore cluster + type: string concurrency: description: Concurrency is the size of thread pool on each node that execute the backup task @@ -7595,18 +7590,15 @@ spec: db: description: DB is the specific DB which will be backed-up or restored type: string - key: - description: Key is the private key path for TLS connection - type: string + enableTLSClient: + description: Whether enable TLS in TiDBCluster + type: boolean logLevel: description: LogLevel is the log level type: string onLine: description: OnLine specifies whether online during restore type: boolean - pd: - description: PDAddress is the PD address of the tidb cluster - type: string rateLimit: description: RateLimit is the rate limit of the backup task, MB/s per node @@ -7628,8 +7620,6 @@ spec: description: TimeAgo is the history version of the backup task, e.g. 1m, 1h type: string - required: - - pd type: object gcs: description: GcsStorageProvider represents the google cloud storage @@ -8455,16 +8445,16 @@ spec: br: description: BRConfig contains config for BR properties: - ca: - description: CA is the CA certificate path for TLS connection - type: string - cert: - description: Cert is the certificate path for TLS connection - type: string checksum: description: Checksum specifies whether to run checksum after backup type: boolean + cluster: + description: ClusterName of backup cluster + type: string + clusterNamespace: + description: Namespace of backup/restore cluster + type: string concurrency: description: Concurrency is the size of thread pool on each node that execute the backup task @@ -8474,18 +8464,15 @@ spec: description: DB is the specific DB which will be backed-up or restored type: string - key: - description: Key is the private key path for TLS connection - type: string + enableTLSClient: + description: Whether enable TLS in TiDBCluster + type: boolean logLevel: description: LogLevel is the log level type: string onLine: description: OnLine specifies whether online during restore type: boolean - pd: - description: PDAddress is the PD address of the tidb cluster - type: string rateLimit: description: RateLimit is the rate limit of the backup task, MB/s per node @@ -8507,8 +8494,6 @@ spec: description: TimeAgo is the history version of the backup task, e.g. 1m, 1h type: string - required: - - pd type: object from: description: TiDBAccessConfig defines the configuration for access diff --git a/pkg/apis/pingcap/v1alpha1/openapi_generated.go b/pkg/apis/pingcap/v1alpha1/openapi_generated.go index f76834fed6..0fe8bbff84 100644 --- a/pkg/apis/pingcap/v1alpha1/openapi_generated.go +++ b/pkg/apis/pingcap/v1alpha1/openapi_generated.go @@ -375,44 +375,37 @@ func schema_pkg_apis_pingcap_v1alpha1_BRConfig(ref common.ReferenceCallback) com Description: "BRConfig contains config for BR", Type: []string{"object"}, Properties: map[string]spec.Schema{ - "pd": { + "enableTLSClient": { SchemaProps: spec.SchemaProps{ - Description: "PDAddress is the PD address of the tidb cluster", - Type: []string{"string"}, - Format: "", - }, - }, - "db": { - SchemaProps: spec.SchemaProps{ - Description: "DB is the specific DB which will be backed-up or restored", - Type: []string{"string"}, + Description: "Whether enable TLS in TiDBCluster", + Type: []string{"boolean"}, Format: "", }, }, - "table": { + "cluster": { SchemaProps: spec.SchemaProps{ - Description: "Table is the specific table which will be backed-up or restored", + Description: "ClusterName of backup cluster", Type: []string{"string"}, Format: "", }, }, - "ca": { + "clusterNamespace": { SchemaProps: spec.SchemaProps{ - Description: "CA is the CA certificate path for TLS connection", + Description: "Namespace of backup/restore cluster", Type: []string{"string"}, Format: "", }, }, - "cert": { + "db": { SchemaProps: spec.SchemaProps{ - Description: "Cert is the certificate path for TLS connection", + Description: "DB is the specific DB which will be backed-up or restored", Type: []string{"string"}, Format: "", }, }, - "key": { + "table": { SchemaProps: spec.SchemaProps{ - Description: "Key is the private key path for TLS connection", + Description: "Table is the specific table which will be backed-up or restored", Type: []string{"string"}, Format: "", }, @@ -474,7 +467,6 @@ func schema_pkg_apis_pingcap_v1alpha1_BRConfig(ref common.ReferenceCallback) com }, }, }, - Required: []string{"pd"}, }, }, } diff --git a/pkg/apis/pingcap/v1alpha1/types.go b/pkg/apis/pingcap/v1alpha1/types.go index 0e217ec5fb..60bd847285 100644 --- a/pkg/apis/pingcap/v1alpha1/types.go +++ b/pkg/apis/pingcap/v1alpha1/types.go @@ -781,21 +781,17 @@ type BackupSpec struct { // Affinity of backup Pods // +optional Affinity *corev1.Affinity `json:"affinity,omitempty"` - // Whether enable TLS in TiDBCluster - // Optional: Defaults to false - // +optional - EnableTLSClient bool `json:"enableTLSClient,omitempty"` - // ClusterName of backup cluster - // Required - Cluster string `json:"cluster,omitempty"` - // Namespace of backup cluster - // Optional: Defaults to namespace of backup job - BackupNamespace string `json:"backupNamespace,omitempty"` } // +k8s:openapi-gen=true // BRConfig contains config for BR type BRConfig struct { + // Whether enable TLS in TiDBCluster + EnableTLSClient bool `json:"enableTLSClient,omitempty"` + // ClusterName of backup cluster + Cluster string `json:"cluster,omitempty"` + // Namespace of backup/restore cluster + ClusterNamespace string `json:"clusterNamespace,omitempty"` // DB is the specific DB which will be backed-up or restored DB string `json:"db,omitempty"` // Table is the specific table which will be backed-up or restored @@ -999,16 +995,6 @@ type RestoreSpec struct { // Affinity of restore Pods // +optional Affinity *corev1.Affinity `json:"affinity,omitempty"` - // Whether enable TLS in TiDBCluster - // Optional: Defaults to false - // +optional - EnableTLSClient bool `json:"enableTLSClient,omitempty"` - // ClusterName of restore cluster - // Required - Cluster string `json:"cluster,omitempty"` - // Namespace of restore cluster - // Optional: Defaults to namespace of restore job - RestoreNamespace string `json:"restoreNamespace,omitempty"` } // RestoreStatus represents the current status of a tidb cluster restore. diff --git a/pkg/backup/backup/backup_manager.go b/pkg/backup/backup/backup_manager.go index 52d6c3595c..a0819b3ddd 100644 --- a/pkg/backup/backup/backup_manager.go +++ b/pkg/backup/backup/backup_manager.go @@ -208,7 +208,7 @@ func (bm *backupManager) makeExportJob(backup *v1alpha1.Backup) (*batchv1.Job, s Name: label.BackupJobLabelVal, Image: controller.TidbBackupManagerImage, Args: args, - ImagePullPolicy: corev1.PullIfNotPresent, + ImagePullPolicy: corev1.PullAlways, VolumeMounts: []corev1.VolumeMount{ {Name: label.BackupJobLabelVal, MountPath: constants.BackupRootPath}, }, @@ -266,14 +266,14 @@ 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.EnableTLSClient { + if backup.Spec.BR.EnableTLSClient { volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: "br-tls", ReadOnly: true, MountPath: constants.BRCertPath, }) volumes = append(volumes, corev1.Volume{ Name: "br-tls", VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ - SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(backup.Spec.Cluster)), + SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(backup.Spec.BR.Cluster)), }, }, }) @@ -290,7 +290,7 @@ func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, s Name: label.BackupJobLabelVal, Image: controller.TidbBackupManagerImage, Args: args, - ImagePullPolicy: corev1.PullIfNotPresent, + ImagePullPolicy: corev1.PullAlways, VolumeMounts: volumeMounts, Env: envVars, }, diff --git a/pkg/backup/backupschedule/backup_schedule_manager.go b/pkg/backup/backupschedule/backup_schedule_manager.go index 05ff3541be..99db077d46 100644 --- a/pkg/backup/backupschedule/backup_schedule_manager.go +++ b/pkg/backup/backupschedule/backup_schedule_manager.go @@ -215,13 +215,8 @@ func (bm *backupScheduleManager) createBackup(bs *v1alpha1.BackupSchedule, times } } } else { - var backupNamespace, pdAddress string - if backupSpec.BackupNamespace == "" { - backupNamespace = ns - } else { - backupNamespace = backupSpec.BackupNamespace - } - pdAddress = fmt.Sprintf("%s-pd.%s:2379", backupSpec.Cluster, backupNamespace) + var pdAddress string + pdAddress = fmt.Sprintf("%s-pd.%s:2379", backupSpec.BR.Cluster, backupSpec.BR.ClusterNamespace) if backupSpec.S3 != nil { backupSpec.S3.Prefix = path.Join(backupSpec.S3.Prefix, diff --git a/pkg/backup/restore/restore_manager.go b/pkg/backup/restore/restore_manager.go index e043fe1c81..a71a7a36e1 100644 --- a/pkg/backup/restore/restore_manager.go +++ b/pkg/backup/restore/restore_manager.go @@ -252,14 +252,14 @@ func (rm *restoreManager) makeRestoreJob(restore *v1alpha1.Restore) (*batchv1.Jo restoreLabel := label.NewBackup().Instance(restore.GetInstanceName()).RestoreJob().Restore(name) volumeMounts := []corev1.VolumeMount{} volumes := []corev1.Volume{} - if restore.Spec.EnableTLSClient { + if restore.Spec.BR.EnableTLSClient { volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: "br-tls", ReadOnly: true, MountPath: constants.BRCertPath, }) volumes = append(volumes, corev1.Volume{ Name: "br-tls", VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ - SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(restore.Spec.Cluster)), + SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(restore.Spec.BR.Cluster)), }, }, }) diff --git a/pkg/backup/util/util.go b/pkg/backup/util/util.go index 7a90bb8b45..48b84a52cb 100644 --- a/pkg/backup/util/util.go +++ b/pkg/backup/util/util.go @@ -298,8 +298,13 @@ func ValidateBackup(backup *v1alpha1.Backup) error { return fmt.Errorf("missing StorageSize config in spec of %s/%s", ns, name) } } else { - if backup.Spec.Cluster == "" { - return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) + if backup.Spec.BR != nil { + if backup.Spec.BR.Cluster == "" { + return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) + } + if backup.Spec.BR.ClusterNamespace == "" { + backup.Spec.BR.ClusterNamespace = ns + } } if backup.Spec.Type != "" && @@ -350,8 +355,13 @@ func ValidateRestore(restore *v1alpha1.Restore) error { return fmt.Errorf("missing StorageSize config in spec of %s/%s", ns, name) } } else { - if restore.Spec.Cluster == "" { - return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) + if restore.Spec.BR != nil { + if restore.Spec.BR.Cluster == "" { + return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) + } + if restore.Spec.BR.ClusterNamespace == "" { + restore.Spec.BR.ClusterNamespace = ns + } } if restore.Spec.Type != "" && restore.Spec.Type != v1alpha1.BackupTypeFull && From 7e10f628185ab8a674ad43f198700a1fbc0ef217 Mon Sep 17 00:00:00 2001 From: shuijing198799 Date: Mon, 2 Mar 2020 17:37:58 +0800 Subject: [PATCH 5/8] backup: fix tiny --- cmd/backup-manager/app/backup/backup.go | 6 +++++- cmd/backup-manager/app/restore/restore.go | 6 +++++- .../backupschedule/backup_schedule_manager.go | 8 ++++++-- pkg/backup/util/util.go | 19 ++++--------------- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/cmd/backup-manager/app/backup/backup.go b/cmd/backup-manager/app/backup/backup.go index d21099ed14..641c02bff7 100644 --- a/cmd/backup-manager/app/backup/backup.go +++ b/cmd/backup-manager/app/backup/backup.go @@ -40,11 +40,15 @@ func (bo *Options) String() string { } func (bo *Options) backupData(backup *v1alpha1.Backup) (string, error) { + clusterNamespace := backup.Spec.BR.ClusterNamespace + if backup.Spec.BR.ClusterNamespace == "" { + clusterNamespace = backup.Namespace + } args, remotePath, err := constructOptions(backup) if err != nil { return "", err } - args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", backup.Spec.BR.Cluster, backup.Spec.BR.ClusterNamespace)) + args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", backup.Spec.BR.Cluster, clusterNamespace)) if backup.Spec.BR.EnableTLSClient { args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, "cert"))) diff --git a/cmd/backup-manager/app/restore/restore.go b/cmd/backup-manager/app/restore/restore.go index 0923a5d3f6..c7cef96b86 100644 --- a/cmd/backup-manager/app/restore/restore.go +++ b/cmd/backup-manager/app/restore/restore.go @@ -35,11 +35,15 @@ func (ro *Options) String() string { } func (ro *Options) restoreData(restore *v1alpha1.Restore) error { + clusterNamespace := restore.Spec.BR.ClusterNamespace + if restore.Spec.BR.ClusterNamespace == "" { + clusterNamespace = restore.Namespace + } args, err := constructBROptions(restore) if err != nil { return err } - args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", restore.Spec.BR.Cluster, restore.Spec.BR.ClusterNamespace)) + args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", restore.Spec.BR.Cluster, clusterNamespace)) if restore.Spec.BR.EnableTLSClient { args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, "cert"))) diff --git a/pkg/backup/backupschedule/backup_schedule_manager.go b/pkg/backup/backupschedule/backup_schedule_manager.go index 99db077d46..b27ae240c6 100644 --- a/pkg/backup/backupschedule/backup_schedule_manager.go +++ b/pkg/backup/backupschedule/backup_schedule_manager.go @@ -215,8 +215,12 @@ func (bm *backupScheduleManager) createBackup(bs *v1alpha1.BackupSchedule, times } } } else { - var pdAddress string - pdAddress = fmt.Sprintf("%s-pd.%s:2379", backupSpec.BR.Cluster, backupSpec.BR.ClusterNamespace) + var pdAddress, clusterNamespace string + clusterNamespace = backupSpec.BR.ClusterNamespace + if backupSpec.BR.ClusterNamespace == "" { + clusterNamespace = ns + } + pdAddress = fmt.Sprintf("%s-pd.%s:2379", backupSpec.BR.Cluster, clusterNamespace) if backupSpec.S3 != nil { backupSpec.S3.Prefix = path.Join(backupSpec.S3.Prefix, diff --git a/pkg/backup/util/util.go b/pkg/backup/util/util.go index 48b84a52cb..5c33aec1f1 100644 --- a/pkg/backup/util/util.go +++ b/pkg/backup/util/util.go @@ -298,15 +298,9 @@ func ValidateBackup(backup *v1alpha1.Backup) error { return fmt.Errorf("missing StorageSize config in spec of %s/%s", ns, name) } } else { - if backup.Spec.BR != nil { - if backup.Spec.BR.Cluster == "" { - return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) - } - if backup.Spec.BR.ClusterNamespace == "" { - backup.Spec.BR.ClusterNamespace = ns - } + if backup.Spec.BR.Cluster == "" { + return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) } - if backup.Spec.Type != "" && backup.Spec.Type != v1alpha1.BackupTypeFull && backup.Spec.Type != v1alpha1.BackupTypeDB && @@ -355,13 +349,8 @@ func ValidateRestore(restore *v1alpha1.Restore) error { return fmt.Errorf("missing StorageSize config in spec of %s/%s", ns, name) } } else { - if restore.Spec.BR != nil { - if restore.Spec.BR.Cluster == "" { - return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) - } - if restore.Spec.BR.ClusterNamespace == "" { - restore.Spec.BR.ClusterNamespace = ns - } + if restore.Spec.BR.Cluster == "" { + return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) } if restore.Spec.Type != "" && restore.Spec.Type != v1alpha1.BackupTypeFull && From ea1cf301781b622060565d2a78536bf7c17a5ffa Mon Sep 17 00:00:00 2001 From: shuijing198799 Date: Mon, 2 Mar 2020 21:18:06 +0800 Subject: [PATCH 6/8] backup: fix tiny --- cmd/backup-manager/app/backup/backup.go | 5 +++-- cmd/backup-manager/app/restore/restore.go | 5 +++-- pkg/backup/backup/backup_manager.go | 23 ++++++++++++----------- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/cmd/backup-manager/app/backup/backup.go b/cmd/backup-manager/app/backup/backup.go index 641c02bff7..afeb6df709 100644 --- a/cmd/backup-manager/app/backup/backup.go +++ b/cmd/backup-manager/app/backup/backup.go @@ -27,6 +27,7 @@ import ( "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" + corev1 "k8s.io/api/core/v1" ) // Options contains the input arguments to the backup command @@ -51,8 +52,8 @@ func (bo *Options) backupData(backup *v1alpha1.Backup) (string, error) { args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", backup.Spec.BR.Cluster, clusterNamespace)) if backup.Spec.BR.EnableTLSClient { args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) - args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, "cert"))) - args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, "key"))) + args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, corev1.TLSCertKey))) + args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, corev1.TLSPrivateKeyKey))) } var btype string diff --git a/cmd/backup-manager/app/restore/restore.go b/cmd/backup-manager/app/restore/restore.go index c7cef96b86..4c6fe1fbd0 100644 --- a/cmd/backup-manager/app/restore/restore.go +++ b/cmd/backup-manager/app/restore/restore.go @@ -23,6 +23,7 @@ import ( "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" + corev1 "k8s.io/api/core/v1" ) type Options struct { @@ -46,8 +47,8 @@ func (ro *Options) restoreData(restore *v1alpha1.Restore) error { args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", restore.Spec.BR.Cluster, clusterNamespace)) if restore.Spec.BR.EnableTLSClient { args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) - args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, "cert"))) - args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, "key"))) + args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, corev1.TLSCertKey))) + args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, corev1.TLSPrivateKeyKey))) } var restoreType string diff --git a/pkg/backup/backup/backup_manager.go b/pkg/backup/backup/backup_manager.go index a0819b3ddd..0fd7a8a489 100644 --- a/pkg/backup/backup/backup_manager.go +++ b/pkg/backup/backup/backup_manager.go @@ -116,6 +116,18 @@ func (bm *backupManager) syncBackupJob(backup *v1alpha1.Backup) error { }) return err } + + reason, err = bm.ensureBackupPVCExist(backup) + if err != nil { + bm.statusUpdater.Update(backup, &v1alpha1.BackupCondition{ + Type: v1alpha1.BackupRetryFailed, + Status: corev1.ConditionTrue, + Reason: reason, + Message: err.Error(), + }) + return err + } + } else { // not found backup job, so we need to create it job, reason, err = bm.makeBackupJob(backup) @@ -130,17 +142,6 @@ func (bm *backupManager) syncBackupJob(backup *v1alpha1.Backup) error { } } - reason, err = bm.ensureBackupPVCExist(backup) - if err != nil { - bm.statusUpdater.Update(backup, &v1alpha1.BackupCondition{ - Type: v1alpha1.BackupRetryFailed, - Status: corev1.ConditionTrue, - Reason: reason, - Message: err.Error(), - }) - return err - } - if err := bm.jobControl.CreateJob(backup, job); err != nil { errMsg := fmt.Errorf("create backup %s/%s job %s failed, err: %v", ns, name, backupJobName, err) bm.statusUpdater.Update(backup, &v1alpha1.BackupCondition{ From a0446c47ff4bbd8c74d4a4c17cdb684ab839520c Mon Sep 17 00:00:00 2001 From: shuijing198799 Date: Tue, 3 Mar 2020 23:15:11 +0800 Subject: [PATCH 7/8] address comment --- manifests/crd.yaml | 6 +++--- pkg/apis/pingcap/v1alpha1/openapi_generated.go | 2 +- pkg/apis/pingcap/v1alpha1/types.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/manifests/crd.yaml b/manifests/crd.yaml index a431809223..afe6d1d134 100644 --- a/manifests/crd.yaml +++ b/manifests/crd.yaml @@ -6749,7 +6749,7 @@ spec: description: Checksum specifies whether to run checksum after backup type: boolean cluster: - description: ClusterName of backup cluster + description: ClusterName of backup/restore cluster type: string clusterNamespace: description: Namespace of backup/restore cluster @@ -7580,7 +7580,7 @@ spec: description: Checksum specifies whether to run checksum after backup type: boolean cluster: - description: ClusterName of backup cluster + description: ClusterName of backup/restore cluster type: string clusterNamespace: description: Namespace of backup/restore cluster @@ -8453,7 +8453,7 @@ spec: backup type: boolean cluster: - description: ClusterName of backup cluster + description: ClusterName of backup/restore cluster type: string clusterNamespace: description: Namespace of backup/restore cluster diff --git a/pkg/apis/pingcap/v1alpha1/openapi_generated.go b/pkg/apis/pingcap/v1alpha1/openapi_generated.go index 7085fd8616..8495569ef2 100644 --- a/pkg/apis/pingcap/v1alpha1/openapi_generated.go +++ b/pkg/apis/pingcap/v1alpha1/openapi_generated.go @@ -384,7 +384,7 @@ func schema_pkg_apis_pingcap_v1alpha1_BRConfig(ref common.ReferenceCallback) com }, "cluster": { SchemaProps: spec.SchemaProps{ - Description: "ClusterName of backup cluster", + Description: "ClusterName of backup/restore cluster", Type: []string{"string"}, Format: "", }, diff --git a/pkg/apis/pingcap/v1alpha1/types.go b/pkg/apis/pingcap/v1alpha1/types.go index 60bd847285..a65ba1bb66 100644 --- a/pkg/apis/pingcap/v1alpha1/types.go +++ b/pkg/apis/pingcap/v1alpha1/types.go @@ -788,7 +788,7 @@ type BackupSpec struct { type BRConfig struct { // Whether enable TLS in TiDBCluster EnableTLSClient bool `json:"enableTLSClient,omitempty"` - // ClusterName of backup cluster + // ClusterName of backup/restore cluster Cluster string `json:"cluster,omitempty"` // Namespace of backup/restore cluster ClusterNamespace string `json:"clusterNamespace,omitempty"` From 6af6cc60fee9fde97054ed0ef8af06610094f7a4 Mon Sep 17 00:00:00 2001 From: shuijing198799 Date: Wed, 4 Mar 2020 10:37:00 +0800 Subject: [PATCH 8/8] address comment --- manifests/crd.yaml | 6 ++++++ pkg/apis/pingcap/v1alpha1/openapi_generated.go | 1 + pkg/apis/pingcap/v1alpha1/types.go | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/manifests/crd.yaml b/manifests/crd.yaml index afe6d1d134..86dbfae2a0 100644 --- a/manifests/crd.yaml +++ b/manifests/crd.yaml @@ -6792,6 +6792,8 @@ spec: description: TimeAgo is the history version of the backup task, e.g. 1m, 1h type: string + required: + - cluster type: object from: description: TiDBAccessConfig defines the configuration for access tidb @@ -7623,6 +7625,8 @@ spec: description: TimeAgo is the history version of the backup task, e.g. 1m, 1h type: string + required: + - cluster type: object gcs: description: GcsStorageProvider represents the google cloud storage @@ -8497,6 +8501,8 @@ spec: description: TimeAgo is the history version of the backup task, e.g. 1m, 1h type: string + required: + - cluster type: object from: description: TiDBAccessConfig defines the configuration for access diff --git a/pkg/apis/pingcap/v1alpha1/openapi_generated.go b/pkg/apis/pingcap/v1alpha1/openapi_generated.go index 8495569ef2..6de096069d 100644 --- a/pkg/apis/pingcap/v1alpha1/openapi_generated.go +++ b/pkg/apis/pingcap/v1alpha1/openapi_generated.go @@ -467,6 +467,7 @@ func schema_pkg_apis_pingcap_v1alpha1_BRConfig(ref common.ReferenceCallback) com }, }, }, + Required: []string{"cluster"}, }, }, } diff --git a/pkg/apis/pingcap/v1alpha1/types.go b/pkg/apis/pingcap/v1alpha1/types.go index a65ba1bb66..45c6e68a17 100644 --- a/pkg/apis/pingcap/v1alpha1/types.go +++ b/pkg/apis/pingcap/v1alpha1/types.go @@ -789,7 +789,7 @@ type BRConfig struct { // Whether enable TLS in TiDBCluster EnableTLSClient bool `json:"enableTLSClient,omitempty"` // ClusterName of backup/restore cluster - Cluster string `json:"cluster,omitempty"` + Cluster string `json:"cluster"` // Namespace of backup/restore cluster ClusterNamespace string `json:"clusterNamespace,omitempty"` // DB is the specific DB which will be backed-up or restored