Merge pull request #76719 from bertinatto/fix_vsphere_size

Fix Vsphere rouding up of volume size

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
+	// vSphere works with KiB, but its minimum allocation unit is 1 MiB
-	volSizeKiB, err := volumehelpers.RoundUpToKiBInt(capacity)
+	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.
+	3. Verify the provisioned PV size is correct.
-	4. Verify the error returned on PVC failure is the 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() {
+	It("verify dynamically provisioned pv has size rounded up correctly", func() {
-		By("Invoking Test for invalid disk size")
+		By("Invoking Test disk size")
 		scParameters[Datastore] = datastore
 		scParameters[DiskFormat] = ThinDisk
 		diskSize := "1"
-		err := invokeInvalidDiskSizeTestNeg(client, namespace, scParameters, diskSize)
+		expectedDiskSize := "1Mi"
-		Expect(err).To(HaveOccurred())
+
-		errorMsg := `Failed to provision volume with StorageClass \"` + DiskSizeSCName + `\": A specified parameter was not correct`
+		By("Creating Storage Class")
-		if !strings.Contains(err.Error(), errorMsg) {
+		storageclass, err := client.StorageV1().StorageClasses().Create(getVSphereStorageClassSpec(DiskSizeSCName, scParameters, nil))
-			framework.ExpectNoError(err, errorMsg)
+		framework.ExpectNoError(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("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("Getting new copy of PVC")
+		pvclaim, err = client.CoreV1().PersistentVolumeClaims(pvclaim.Namespace).Get(pvclaim.Name, metav1.GetOptions{})
+		framework.ExpectNoError(err)
+
+		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()))
 	})
 })
-
-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("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())
-
-	eventList, err := client.CoreV1().Events(pvclaim.Namespace).List(metav1.ListOptions{})
-	return fmt.Errorf("Failure message: %+q", eventList.Items[0].Message)
-}