Merge pull request kubernetes#76719 from bertinatto/fix_vsphere_size

Fix Vsphere rouding up of volume size

Author: k8s-ci-robot

pkg/volume/vsphere_volume/vsphere_volume_util.go

@@ -94,11 +94,12 @@ func (util *VsphereDiskUtil) CreateVolume(v *vsphereVolumeProvisioner, selectedZ
 	}
 
 	capacity := v.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)]
-	// vSphere works with kilobytes, convert to KiB with rounding up
-	volSizeKiB, err := volumehelpers.RoundUpToKiBInt(capacity)
+	// vSphere works with KiB, but its minimum allocation unit is 1 MiB
+	volSizeMiB, err := volumehelpers.RoundUpToMiBInt(capacity)
 	if err != nil {
 		return nil, err
 	}
+	volSizeKiB := volSizeMiB * 1024
 	name := volumeutil.GenerateVolumeName(v.options.ClusterName, v.options.PVName, 255)
 	volumeOptions := &vclib.VolumeOptions{
 		CapacityKB: volSizeKiB,

test/e2e/storage/vsphere/vsphere_volume_disksize.go

@@ -17,13 +17,12 @@ limitations under the License.
 package vsphere
 
 import (
-	"fmt"
-	"strings"
 	"time"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/test/e2e/framework"
@@ -35,13 +34,12 @@ const (
 )
 
 /*
-	Test to verify disk size specified in PVC is being honored while volume creation.
+	Test to verify disk size specified in PVC is being rounded up correctly.
 
 	Steps
 	1. Create StorageClass.
 	2. Create PVC with invalid disk size which uses the StorageClass created in step 1.
-	3. Expect the PVC to fail.
-	4. Verify the error returned on PVC failure is the correct.
+	3. Verify the provisioned PV size is correct.
 */
 
 var _ = utils.SIGDescribe("Volume Disk Size [Feature:vsphere]", func() {
@@ -61,35 +59,38 @@ var _ = utils.SIGDescribe("Volume Disk Size [Feature:vsphere]", func() {
 		datastore = GetAndExpectStringEnvVar(StorageClassDatastoreName)
 	})
 
-	It("verify dynamically provisioned pv using storageclass with an invalid disk size fails", func() {
-		By("Invoking Test for invalid disk size")
+	It("verify dynamically provisioned pv has size rounded up correctly", func() {
+		By("Invoking Test disk size")
 		scParameters[Datastore] = datastore
 		scParameters[DiskFormat] = ThinDisk
 		diskSize := "1"
-		err := invokeInvalidDiskSizeTestNeg(client, namespace, scParameters, diskSize)
-		Expect(err).To(HaveOccurred())
-		errorMsg := `Failed to provision volume with StorageClass \"` + DiskSizeSCName + `\": A specified parameter was not correct`
-		if !strings.Contains(err.Error(), errorMsg) {
-			framework.ExpectNoError(err, errorMsg)
-		}
-	})
-})
+		expectedDiskSize := "1Mi"
+
+		By("Creating Storage Class")
+		storageclass, err := client.StorageV1().StorageClasses().Create(getVSphereStorageClassSpec(DiskSizeSCName, scParameters, nil))
+		framework.ExpectNoError(err)
+		defer client.StorageV1().StorageClasses().Delete(storageclass.Name, nil)
 
-func invokeInvalidDiskSizeTestNeg(client clientset.Interface, namespace string, scParameters map[string]string, diskSize string) error {
-	By("Creating Storage Class With invalid disk size")
-	storageclass, err := client.StorageV1().StorageClasses().Create(getVSphereStorageClassSpec(DiskSizeSCName, scParameters, nil))
-	framework.ExpectNoError(err, fmt.Sprintf("Failed to create storage class with err: %v", err))
-	defer client.StorageV1().StorageClasses().Delete(storageclass.Name, nil)
+		By("Creating PVC using the Storage Class")
+		pvclaim, err := framework.CreatePVC(client, namespace, getVSphereClaimSpecWithStorageClass(namespace, diskSize, storageclass))
+		framework.ExpectNoError(err)
+		defer framework.DeletePersistentVolumeClaim(client, pvclaim.Name, namespace)
 
-	By("Creating PVC using the Storage Class")
-	pvclaim, err := framework.CreatePVC(client, namespace, getVSphereClaimSpecWithStorageClass(namespace, diskSize, storageclass))
-	framework.ExpectNoError(err)
-	defer framework.DeletePersistentVolumeClaim(client, pvclaim.Name, namespace)
+		By("Waiting for claim to be in bound phase")
+		err = framework.WaitForPersistentVolumeClaimPhase(v1.ClaimBound, client, pvclaim.Namespace, pvclaim.Name, framework.Poll, 2*time.Minute)
+		framework.ExpectNoError(err)
 
-	By("Expect claim to fail provisioning volume")
-	err = framework.WaitForPersistentVolumeClaimPhase(v1.ClaimBound, client, pvclaim.Namespace, pvclaim.Name, framework.Poll, 2*time.Minute)
-	Expect(err).To(HaveOccurred())
+		By("Getting new copy of PVC")
+		pvclaim, err = client.CoreV1().PersistentVolumeClaims(pvclaim.Namespace).Get(pvclaim.Name, metav1.GetOptions{})
+		framework.ExpectNoError(err)
 
-	eventList, err := client.CoreV1().Events(pvclaim.Namespace).List(metav1.ListOptions{})
-	return fmt.Errorf("Failure message: %+q", eventList.Items[0].Message)
-}
+		By("Getting PV created")
+		pv, err := client.CoreV1().PersistentVolumes().Get(pvclaim.Spec.VolumeName, metav1.GetOptions{})
+		framework.ExpectNoError(err)
+
+		By("Verifying if provisioned PV has the correct size")
+		expectedCapacity := resource.MustParse(expectedDiskSize)
+		pvCapacity := pv.Spec.Capacity[v1.ResourceName(v1.ResourceStorage)]
+		Expect(pvCapacity.Value()).To(Equal(expectedCapacity.Value()))
+	})
+})