Able to omit storageClassName

Signed-off-by: Yecheng Fu <fuyecheng@pingcap.com>
pingcap · Jan 18, 2020 · 7232a2f · 7232a2f
1 parent 97d07a1
commit 7232a2f
Show file tree

Hide file tree

Showing 13 changed files with 101 additions and 63 deletions.
diff --git a/charts/tidb-operator/values.yaml b/charts/tidb-operator/values.yaml
@@ -15,11 +15,28 @@ timezone: UTC
 operatorImage: pingcap/tidb-operator:v1.1.0-beta.1
 imagePullPolicy: IfNotPresent
 
-defaultStorageClassName: local-storage
+#
+# Specify storage class for pd/tikv componnents. 
+#
+# By default, "standard" is used. If you want to use default storage class in
+# the Kubernetes cluster, set its value to "".
+#
+# https://kubernetes.io/docs/tasks/administer-cluster/change-default-storage-class/
+# 
+# defaultStorageClassName: standard 
 
 # tidbBackupManagerImage is tidb backup manager image
 # tidbBackupManagerImage: pingcap/tidb-backup-manager:latest
-# defaultBackupStorageClassName: local-storage
+
+#
+# Specify storage class for backup and restore. 
+#
+# By default, "standard" is used. If you want to use default storage class in
+# the Kubernetes cluster, set its value to "".
+#
+# https://kubernetes.io/docs/tasks/administer-cluster/change-default-storage-class/
+# 
+# defaultBackupStorageClassName: standard
 
 #
 # Enable or disable tidb-operator features:

diff --git a/manifests/crd.yaml b/manifests/crd.yaml
@@ -1854,9 +1854,8 @@ spec:
                   type: object
                 storageClassName:
                   description: 'The storageClassName of the persistent volume for
-                    PD data storage, empty string means not explicitly set and use
-                    the cluster default set by admission controller. Optionals: Defaults
-                    to the default-storage-class-name set in the tidb-operator'
+                    PD data storage. Optionals: Defaults to the default-storage-class-name
+                    set in the tidb-operator or Kubernetes default storage classs.'
                   type: string
                 tolerations:
                   description: 'Tolerations of the component. Override the cluster-level
@@ -2694,9 +2693,8 @@ spec:
                   type: string
                 storageClassName:
                   description: 'The storageClassName of the persistent volume for
-                    Pump data storage, empty string means not explicitly set and use
-                    the cluster default set by admission controller. Optionals: Defaults
-                    to the default-storage-class-name set in the tidb-operator'
+                    Pump data storage. Optionals: Defaults to the default-storage-class-name
+                    set in the tidb-operator or Kubernetes default storage classs.'
                   type: string
                 tolerations:
                   description: 'Tolerations of the component. Override the cluster-level
@@ -6002,9 +6000,8 @@ spec:
                   type: string
                 storageClassName:
                   description: 'The storageClassName of the persistent volume for
-                    TiKV data storage, empty string means not explicitly set and use
-                    the cluster default set by admission controller. Optionals: Defaults
-                    to the default-storage-class-name set in the tidb-operator'
+                    TiKV data storage. Optionals: Defaults to the default-storage-class-name
+                    set in the tidb-operator or Kubernetes default storage classs.'
                   type: string
                 tolerations:
                   description: 'Tolerations of the component. Override the cluster-level
@@ -6316,8 +6313,9 @@ spec:
               - secretName
               type: object
             storageClassName:
-              description: StorageClassName is the storage class for backup job's
-                PV.
+              description: 'The storageClassName of the persistent volume for Backup
+                data storage. Optionals: Defaults to the default-backup-storage-class-name
+                set in the tidb-operator or Kubernetes default storage classs.'
               type: string
             storageSize:
               description: StorageSize is the request storage size for backup job
@@ -6448,8 +6446,9 @@ spec:
               - secretName
               type: object
             storageClassName:
-              description: StorageClassName is the storage class for backup job's
-                PV.
+              description: 'The storageClassName of the persistent volume for Restore
+                data storage. Optionals: Defaults to the default-backup-storage-class-name
+                set in the tidb-operator or Kubernetes default storage classs.'
               type: string
             storageSize:
               description: StorageSize is the request storage size for backup job
@@ -6479,7 +6478,6 @@ spec:
               type: object
           required:
           - to
-          - storageClassName
           - storageSize
           type: object
       type: object
@@ -6694,8 +6692,9 @@ spec:
                   - secretName
                   type: object
                 storageClassName:
-                  description: StorageClassName is the storage class for backup job's
-                    PV.
+                  description: 'The storageClassName of the persistent volume for
+                    Backup data storage. Optionals: Defaults to the default-backup-storage-class-name
+                    set in the tidb-operator or Kubernetes default storage classs.'
                   type: string
                 storageSize:
                   description: StorageSize is the request storage size for backup
@@ -6718,8 +6717,9 @@ spec:
               description: Schedule specifies the cron string used for backup scheduling.
               type: string
             storageClassName:
-              description: StorageClassName is the storage class for backup job's
-                PV.
+              description: 'The storageClassName of the persistent volume for Backup
+                data storage. Optionals: Defaults to the default-backup-storage-class-name
+                set in the tidb-operator or Kubernetes default storage classs.'
               type: string
             storageSize:
               description: StorageSize is the request storage size for backup job

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
@@ -222,9 +222,8 @@ type PDSpec struct {
 	// +optional
 	Service *ServiceSpec `json:"service,omitempty"`
 
-	// The storageClassName of the persistent volume for PD data storage, empty string means not explicitly set
-	// and use the cluster default set by admission controller.
-	// Optionals: Defaults to the default-storage-class-name set in the tidb-operator
+	// The storageClassName of the persistent volume for PD data storage.
+	// Optionals: Defaults to the default-storage-class-name set in the tidb-operator or Kubernetes default storage classs.
 	// +optional
 	StorageClassName *string `json:"storageClassName,omitempty"`
 
@@ -261,9 +260,8 @@ type TiKVSpec struct {
 	// +optional
 	MaxFailoverCount *int32 `json:"maxFailoverCount,omitempty"`
 
-	// The storageClassName of the persistent volume for TiKV data storage, empty string means not explicitly set
-	// and use the cluster default set by admission controller.
-	// Optionals: Defaults to the default-storage-class-name set in the tidb-operator
+	// The storageClassName of the persistent volume for TiKV data storage.
+	// Optionals: Defaults to the default-storage-class-name set in the tidb-operator or Kubernetes default storage classs.
 	// +optional
 	StorageClassName *string `json:"storageClassName,omitempty"`
 
@@ -343,9 +341,8 @@ type PumpSpec struct {
 	// +optional
 	BaseImage string `json:"baseImage"`
 
-	// The storageClassName of the persistent volume for Pump data storage, empty string means not explicitly set
-	// and use the cluster default set by admission controller.
-	// Optionals: Defaults to the default-storage-class-name set in the tidb-operator
+	// The storageClassName of the persistent volume for Pump data storage.
+	// Optionals: Defaults to the default-storage-class-name set in the tidb-operator or Kubernetes default storage classs.
 	// +optional
 	StorageClassName *string `json:"storageClassName,omitempty"`
 
@@ -744,8 +741,10 @@ type BackupSpec struct {
 	Type BackupType `json:"backupType,omitempty"`
 	// StorageProvider configures where and how backups should be stored.
 	StorageProvider `json:",inline"`
-	// StorageClassName is the storage class for backup job's PV.
-	StorageClassName string `json:"storageClassName,omitempty"`
+	// The storageClassName of the persistent volume for Backup data storage.
+	// Optionals: Defaults to the default-backup-storage-class-name set in the tidb-operator or Kubernetes default storage classs.
+	// +optional
+	StorageClassName *string `json:"storageClassName,omitempty"`
 	// StorageSize is the request storage size for backup job
 	StorageSize string `json:"storageSize,omitempty"`
 	// BRConfig is the configs for BR
@@ -866,8 +865,10 @@ type BackupScheduleSpec struct {
 	MaxReservedTime *string `json:"maxReservedTime,omitempty"`
 	// BackupTemplate is the specification of the backup structure to get scheduled.
 	BackupTemplate BackupSpec `json:"backupTemplate"`
-	// StorageClassName is the storage class for backup job's PV.
-	StorageClassName string `json:"storageClassName,omitempty"`
+	// The storageClassName of the persistent volume for Backup data storage.
+	// Optionals: Defaults to the default-backup-storage-class-name set in the tidb-operator or Kubernetes default storage classs.
+	// +optional
+	StorageClassName *string `json:"storageClassName,omitempty"`
 	// StorageSize is the request storage size for backup job
 	StorageSize string `json:"storageSize,omitempty"`
 }
@@ -944,8 +945,10 @@ type RestoreSpec struct {
 	Type BackupType `json:"backupType,omitempty"`
 	// StorageProvider configures where and how backups should be stored.
 	StorageProvider `json:",inline"`
-	// StorageClassName is the storage class for backup job's PV.
-	StorageClassName string `json:"storageClassName"`
+	// The storageClassName of the persistent volume for Restore data storage.
+	// Optionals: Defaults to the default-backup-storage-class-name set in the tidb-operator or Kubernetes default storage classs.
+	// +optional
+	StorageClassName *string `json:"storageClassName,omitempty"`
 	// StorageSize is the request storage size for backup job
 	StorageSize string `json:"storageSize"`
 }

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
@@ -327,18 +327,13 @@ func (bm *backupManager) ensureBackupPVCExist(backup *v1alpha1.Backup) (string,
 	}
 
 	// not found PVC, so we need to create PVC for backup job
-	storageClassName := controller.DefaultBackupStorageClassName
-	if backup.Spec.StorageClassName != "" {
-		storageClassName = backup.Spec.StorageClassName
-	}
 	pvc := &corev1.PersistentVolumeClaim{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      backupPVCName,
 			Namespace: ns,
 			Labels:    label.NewBackup().Instance(backup.GetInstanceName()),
 		},
 		Spec: corev1.PersistentVolumeClaimSpec{
-			StorageClassName: &storageClassName,
 			AccessModes: []corev1.PersistentVolumeAccessMode{
 				corev1.ReadWriteOnce,
 			},
@@ -349,6 +344,11 @@ func (bm *backupManager) ensureBackupPVCExist(backup *v1alpha1.Backup) (string,
 			},
 		},
 	}
+	if backup.Spec.StorageClassName != nil {
+		pvc.Spec.StorageClassName = backup.Spec.StorageClassName
+	} else if len(controller.DefaultBackupStorageClassName) > 0 {
+		pvc.Spec.StorageClassName = &controller.DefaultBackupStorageClassName
+	}
 
 	if err := bm.pvcControl.CreatePVC(backup, pvc); err != nil {
 		errMsg := fmt.Errorf("backup %s/%s create backup pvc %s failed, err: %v", ns, name, pvc.GetName(), err)

diff --git a/pkg/backup/backupschedule/backup_schedule_manager.go b/pkg/backup/backupschedule/backup_schedule_manager.go
@@ -203,11 +203,11 @@ func (bm *backupScheduleManager) createBackup(bs *v1alpha1.BackupSchedule, times
 
 	backupSpec := *bs.Spec.BackupTemplate.DeepCopy()
 	if backupSpec.BR == nil {
-		if backupSpec.StorageClassName == "" {
-			if bs.Spec.StorageClassName != "" {
+		if backupSpec.StorageClassName == nil || *backupSpec.StorageClassName == "" {
+			if bs.Spec.StorageClassName != nil && *bs.Spec.StorageClassName != "" {
 				backupSpec.StorageClassName = bs.Spec.StorageClassName
-			} else {
-				backupSpec.StorageClassName = controller.DefaultBackupStorageClassName
+			} else if len(controller.DefaultBackupStorageClassName) > 0 {
+				backupSpec.StorageClassName = &controller.DefaultBackupStorageClassName
 			}
 		}
 

diff --git a/pkg/backup/restore/restore_manager.go b/pkg/backup/restore/restore_manager.go
@@ -222,18 +222,13 @@ func (rm *restoreManager) ensureRestorePVCExist(restore *v1alpha1.Restore) (stri
 	if err != nil {
 		// get the object from the local cache, the error can only be IsNotFound,
 		// so we need to create PVC for restore job
-		storageClassName := controller.DefaultBackupStorageClassName
-		if restore.Spec.StorageClassName != "" {
-			storageClassName = restore.Spec.StorageClassName
-		}
 		pvc := &corev1.PersistentVolumeClaim{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      restorePVCName,
 				Namespace: ns,
 				Labels:    label.NewRestore().Instance(restore.GetInstanceName()),
 			},
 			Spec: corev1.PersistentVolumeClaimSpec{
-				StorageClassName: &storageClassName,
 				AccessModes: []corev1.PersistentVolumeAccessMode{
 					corev1.ReadWriteOnce,
 				},
@@ -244,6 +239,11 @@ func (rm *restoreManager) ensureRestorePVCExist(restore *v1alpha1.Restore) (stri
 				},
 			},
 		}
+		if restore.Spec.StorageClassName != nil {
+			pvc.Spec.StorageClassName = restore.Spec.StorageClassName
+		} else if len(controller.DefaultBackupStorageClassName) > 0 {
+			pvc.Spec.StorageClassName = &controller.DefaultBackupStorageClassName
+		}
 		if err := rm.pvcControl.CreatePVC(restore, pvc); err != nil {
 			errMsg := fmt.Errorf(" %s/%s create restore pvc %s failed, err: %v", ns, name, pvc.GetName(), err)
 			return "CreatePVCFailed", errMsg

diff --git a/pkg/backup/util/util.go b/pkg/backup/util/util.go
@@ -294,7 +294,7 @@ func ValidateBackup(backup *v1alpha1.Backup) error {
 		if backup.Spec.From.SecretName == "" {
 			return fmt.Errorf("missing tidbSecretName config in spec of %s/%s", ns, name)
 		}
-		if backup.Spec.StorageClassName == "" {
+		if backup.Spec.StorageClassName == nil || *backup.Spec.StorageClassName == "" {
 			return fmt.Errorf("missing storageClassName config in spec of %s/%s", ns, name)
 		}
 		if backup.Spec.StorageSize == "" {