Able to omit storageClassName

Signed-off-by: Yecheng Fu <fuyecheng@pingcap.com>

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:

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"`
 }

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)

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
 			}
 		}
 

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

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 == "" {

pkg/controller/backup/backup_controller_test.go

@@ -19,10 +19,9 @@ import (
 	"testing"
 	"time"
 
-	"github.com/pingcap/tidb-operator/pkg/backup/constants"
-
 	. "github.com/onsi/gomega"
 	"github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
+	"github.com/pingcap/tidb-operator/pkg/backup/constants"
 	"github.com/pingcap/tidb-operator/pkg/client/clientset/versioned/fake"
 	informers "github.com/pingcap/tidb-operator/pkg/client/informers/externalversions"
 	corev1 "k8s.io/api/core/v1"
@@ -31,6 +30,7 @@ import (
 	kubeinformers "k8s.io/client-go/informers"
 	kubefake "k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/utils/pointer"
 )
 
 func TestBackupControllerEnqueueBackup(t *testing.T) {
@@ -262,7 +262,7 @@ func newBackup() *v1alpha1.Backup {
 					SecretName: "demo",
 				},
 			},
-			StorageClassName: "local-storage",
+			StorageClassName: pointer.StringPtr("local-storage"),
 			StorageSize:      "1Gi",
 		},
 	}

pkg/manager/member/pd_member_manager.go

@@ -15,7 +15,6 @@ package member
 
 import (
 	"fmt"
-	"github.com/pingcap/tidb-operator/pkg/util"
 	"strconv"
 	"strings"
 
@@ -24,6 +23,7 @@ import (
 	"github.com/pingcap/tidb-operator/pkg/label"
 	"github.com/pingcap/tidb-operator/pkg/manager"
 	"github.com/pingcap/tidb-operator/pkg/pdapi"
+	"github.com/pingcap/tidb-operator/pkg/util"
 	apps "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -588,7 +588,7 @@ func getNewPDSetForTidbCluster(tc *v1alpha1.TidbCluster, cm *corev1.ConfigMap) (
 	podAnnotations := CombineAnnotations(controller.AnnProm(2379), basePDSpec.Annotations())
 	stsAnnotations := getStsAnnotations(tc, label.PDLabelVal)
 	storageClassName := tc.Spec.PD.StorageClassName
-	if storageClassName == nil {
+	if storageClassName == nil && len(controller.DefaultStorageClassName) > 0 {
 		storageClassName = &controller.DefaultStorageClassName
 	}
 	failureReplicas := 0

pkg/manager/member/tikv_member_manager.go

@@ -409,7 +409,7 @@ func getNewTiKVSetForTidbCluster(tc *v1alpha1.TidbCluster, cm *corev1.ConfigMap)
 	capacity := controller.TiKVCapacity(tc.Spec.TiKV.Limits)
 	headlessSvcName := controller.TiKVPeerMemberName(tcName)
 	storageClassName := tc.Spec.TiKV.StorageClassName
-	if storageClassName == nil {
+	if storageClassName == nil && len(controller.DefaultStorageClassName) > 0 {
 		storageClassName = &controller.DefaultStorageClassName
 	}
 

tests/e2e/e2e.go

@@ -71,6 +71,9 @@ func setupSuite() {
 				metav1.NamespaceDefault,
 				metav1.NamespacePublic,
 				v1.NamespaceNodeLease,
+				// kind local path provisioner namespace since 0.7.0
+				// https://github.com/kubernetes-sigs/kind/blob/v0.7.0/pkg/build/node/storage.go#L35
+				"local-path-storage",
 			})
 		if err != nil {
 			e2elog.Failf("Error deleting orphaned namespaces: %v", err)