Document and improve defaulting for MakePersistentVolume, dedupe some manual PV creations

test/e2e/framework/pv_util.go

@@ -78,14 +78,29 @@ type PVCMap map[types.NamespacedName]pvcval
 //	 	},
 //	 }
 type PersistentVolumeConfig struct {
-	PVSource         v1.PersistentVolumeSource
+	// [Optional] NamePrefix defaults to "pv-" if unset
-	Prebind          *v1.PersistentVolumeClaim
+	NamePrefix string
+	// [Optional] Labels contains information used to organize and categorize
+	// objects
+	Labels labels.Set
+	// PVSource contains the details of the underlying volume and must be set
+	PVSource v1.PersistentVolumeSource
+	// [Optional] Prebind lets you specify a PVC to bind this PV to before
+	// creation
+	Prebind *v1.PersistentVolumeClaim
+	// [Optiona] ReclaimPolicy defaults to "Reclaim" if unset
 	ReclaimPolicy    v1.PersistentVolumeReclaimPolicy
-	NamePrefix       string
-	Labels           labels.Set
 	StorageClassName string
-	NodeAffinity     *v1.VolumeNodeAffinity
+	// [Optional] NodeAffinity defines constraints that limit what nodes this
-	VolumeMode       *v1.PersistentVolumeMode
+	// volume can be accessed from.
+	NodeAffinity *v1.VolumeNodeAffinity
+	// [Optional] VolumeMode defaults to "Filesystem" if unset
+	VolumeMode *v1.PersistentVolumeMode
+	// [Optional] AccessModes defaults to RWO if unset
+	AccessModes []v1.PersistentVolumeAccessMode
+	// [Optional] Capacity is the storage capacity in Quantity format. Defaults
+	// to "2Gi" if unset
+	Capacity string
 }
 
 // PersistentVolumeClaimConfig is consumed by MakePersistentVolumeClaim() to
@@ -582,17 +597,33 @@ func makePvcKey(ns, name string) types.NamespacedName {
 //   is added later in CreatePVCPV.
 func MakePersistentVolume(pvConfig PersistentVolumeConfig) *v1.PersistentVolume {
 	var claimRef *v1.ObjectReference
-	// If the reclaimPolicy is not provided, assume Retain
+
+	if len(pvConfig.AccessModes) == 0 {
+		pvConfig.AccessModes = append(pvConfig.AccessModes, v1.ReadWriteOnce)
+	}
+
+	if len(pvConfig.NamePrefix) == 0 {
+		pvConfig.NamePrefix = "pv-"
+	}
+
 	if pvConfig.ReclaimPolicy == "" {
-		e2elog.Logf("PV ReclaimPolicy unspecified, default: Retain")
 		pvConfig.ReclaimPolicy = v1.PersistentVolumeReclaimRetain
 	}
+
+	if len(pvConfig.Capacity) == 0 {
+		pvConfig.Capacity = "2Gi"
+	}
+
 	if pvConfig.Prebind != nil {
 		claimRef = &v1.ObjectReference{
-			Name:      pvConfig.Prebind.Name,
+			Kind:       "PersistentVolumeClaim",
-			Namespace: pvConfig.Prebind.Namespace,
+			APIVersion: "v1",
+			Name:       pvConfig.Prebind.Name,
+			Namespace:  pvConfig.Prebind.Namespace,
+			UID:        pvConfig.Prebind.UID,
 		}
 	}
+
 	return &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: pvConfig.NamePrefix,
@@ -604,18 +635,14 @@ func MakePersistentVolume(pvConfig PersistentVolumeConfig) *v1.PersistentVolume
 		Spec: v1.PersistentVolumeSpec{
 			PersistentVolumeReclaimPolicy: pvConfig.ReclaimPolicy,
 			Capacity: v1.ResourceList{
-				v1.ResourceName(v1.ResourceStorage): resource.MustParse("2Gi"),
+				v1.ResourceStorage: resource.MustParse(pvConfig.Capacity),
 			},
 			PersistentVolumeSource: pvConfig.PVSource,
-			AccessModes: []v1.PersistentVolumeAccessMode{
+			AccessModes:            pvConfig.AccessModes,
-				v1.ReadWriteOnce,
+			ClaimRef:               claimRef,
-				v1.ReadOnlyMany,
+			StorageClassName:       pvConfig.StorageClassName,
-				v1.ReadWriteMany,
+			NodeAffinity:           pvConfig.NodeAffinity,
-			},
+			VolumeMode:             pvConfig.VolumeMode,
-			ClaimRef:         claimRef,
-			StorageClassName: pvConfig.StorageClassName,
-			NodeAffinity:     pvConfig.NodeAffinity,
-			VolumeMode:       pvConfig.VolumeMode,
 		},
 	}
 }
@@ -651,7 +678,7 @@ func MakePersistentVolumeClaim(cfg PersistentVolumeClaimConfig, ns string) *v1.P
 			AccessModes: cfg.AccessModes,
 			Resources: v1.ResourceRequirements{
 				Requests: v1.ResourceList{
-					v1.ResourceName(v1.ResourceStorage): resource.MustParse(cfg.ClaimSize),
+					v1.ResourceStorage: resource.MustParse(cfg.ClaimSize),
 				},
 			},
 			StorageClassName: cfg.StorageClassName,

test/e2e/storage/volume_provisioning.go

@@ -33,7 +33,6 @@ import (
 	storagev1 "k8s.io/api/storage/v1"
 	storagev1beta1 "k8s.io/api/storage/v1beta1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
@@ -658,31 +657,25 @@ var _ = utils.SIGDescribe("Dynamic Provisioning", func() {
 			framework.ExpectNoError(err)
 
 			ginkgo.By("creating PV")
-			pv := &v1.PersistentVolume{
+			pv := framework.MakePersistentVolume(framework.PersistentVolumeConfig{
-				ObjectMeta: metav1.ObjectMeta{
+				NamePrefix: "volume-idempotent-delete-",
-					GenerateName: "volume-idempotent-delete-",
+				// Use Retain to keep the PV, the test will change it to Delete
+				// when the time comes.
+				ReclaimPolicy: v1.PersistentVolumeReclaimRetain,
+				AccessModes: []v1.PersistentVolumeAccessMode{
+					v1.ReadWriteOnce,
 				},
-				Spec: v1.PersistentVolumeSpec{
+				Capacity: "1Gi",
-					// Use Retain to keep the PV, the test will change it to Delete
+				// PV is bound to non-existing PVC, so it's reclaim policy is
-					// when the time comes.
+				// executed immediately
-					PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimRetain,
+				Prebind: &v1.PersistentVolumeClaim{
-					AccessModes: []v1.PersistentVolumeAccessMode{
+					ObjectMeta: metav1.ObjectMeta{
-						v1.ReadWriteOnce,
+						Name:      "dummy-claim-name",
-					},
+						Namespace: ns,
-					Capacity: v1.ResourceList{
+						UID:       types.UID("01234567890"),
-						v1.ResourceName(v1.ResourceStorage): resource.MustParse("1Gi"),
-					},
-					// PV is bound to non-existing PVC, so it's reclaim policy is
-					// executed immediately
-					ClaimRef: &v1.ObjectReference{
-						Kind:       "PersistentVolumeClaim",
-						APIVersion: "v1",
-						UID:        types.UID("01234567890"),
-						Namespace:  ns,
-						Name:       "dummy-claim-name",
 					},
 				},
-			}
+			})
 			switch framework.TestContext.Provider {
 			case "aws":
 				pv.Spec.PersistentVolumeSource = v1.PersistentVolumeSource{

test/e2e/storage/vsphere/BUILD

@@ -39,7 +39,6 @@ go_library(
     ],
     importpath = "k8s.io/kubernetes/test/e2e/storage/vsphere",
     deps = [
-        "//pkg/volume/util:go_default_library",
         "//staging/src/k8s.io/api/apps/v1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/api/storage/v1:go_default_library",

test/e2e/storage/vsphere/vsphere_utils.go

@@ -40,7 +40,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientset "k8s.io/client-go/kubernetes"
-	"k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2elog "k8s.io/kubernetes/test/e2e/framework/log"
 	e2essh "k8s.io/kubernetes/test/e2e/framework/ssh"
@@ -156,12 +155,7 @@ func waitForVSphereDiskToDetach(volumePath string, nodeName string) error {
 
 // function to create vsphere volume spec with given VMDK volume path, Reclaim Policy and labels
 func getVSpherePersistentVolumeSpec(volumePath string, persistentVolumeReclaimPolicy v1.PersistentVolumeReclaimPolicy, labels map[string]string) *v1.PersistentVolume {
-	var (
+	return framework.MakePersistentVolume(framework.PersistentVolumeConfig{
-		pvConfig framework.PersistentVolumeConfig
-		pv       *v1.PersistentVolume
-		claimRef *v1.ObjectReference
-	)
-	pvConfig = framework.PersistentVolumeConfig{
 		NamePrefix: "vspherepv-",
 		PVSource: v1.PersistentVolumeSource{
 			VsphereVolume: &v1.VsphereVirtualDiskVolumeSource{
@@ -169,32 +163,13 @@ func getVSpherePersistentVolumeSpec(volumePath string, persistentVolumeReclaimPo
 				FSType:     "ext4",
 			},
 		},
-		Prebind: nil,
+		ReclaimPolicy: persistentVolumeReclaimPolicy,
-	}
+		Capacity:      "2Gi",
-
+		AccessModes: []v1.PersistentVolumeAccessMode{
-	pv = &v1.PersistentVolume{
+			v1.ReadWriteOnce,
-		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: pvConfig.NamePrefix,
-			Annotations: map[string]string{
-				util.VolumeGidAnnotationKey: "777",
-			},
 		},
-		Spec: v1.PersistentVolumeSpec{
+		Labels: labels,
-			PersistentVolumeReclaimPolicy: persistentVolumeReclaimPolicy,
+	})
-			Capacity: v1.ResourceList{
-				v1.ResourceName(v1.ResourceStorage): resource.MustParse("2Gi"),
-			},
-			PersistentVolumeSource: pvConfig.PVSource,
-			AccessModes: []v1.PersistentVolumeAccessMode{
-				v1.ReadWriteOnce,
-			},
-			ClaimRef: claimRef,
-		},
-	}
-	if labels != nil {
-		pv.Labels = labels
-	}
-	return pv
 }
 
 // function to get vsphere persistent volume spec with given selector labels.