Add unit tests

Author: Sneha-at

pkg/gce-pd-csi-driver/cache.go

@@ -17,13 +17,18 @@ import (
 )
 
 const (
-	cacheSuffix              = "csi-fast"
-	mainLvSuffix             = "csi-main"
-	raidedLocalSsdName       = "csi-driver-data-cache"
-	raidMode                 = "0"
-	maxAllowedChunks   int64 = 1000000 // This is the max allowed chunks for LVM
-	GiB                int64 = 1024 * 1024 * 1024
-	KiB                int64 = 1024
+	cacheSuffix                = "csi-fast"
+	mainLvSuffix               = "csi-main"
+	raidedLocalSsdName         = "csi-driver-data-cache"
+	raidMode                   = "0"
+	maxAllowedChunks   int64   = 1000000 // This is the max allowed chunks for LVM
+	GiB                float64 = 1024 * 1024 * 1024
+	KiB                float64 = 1024
+)
+
+var (
+	maxChunkSize float64 = 1 * GiB   // Max allowed chunk size as per LVM documentation
+	minChunkSize float64 = 160 * KiB // This is randomly selected, we need a multiple of 32KiB, the default size would be too small for caching https://man7.org/linux/man-pages/man8/lvcreate.8.html (--chunksize)
 )
 
 func fetchRAIDedLocalSsdPath() (string, error) {
@@ -88,7 +93,7 @@ func setupCaching(devicePath string, req *csi.NodeStageVolumeRequest, nodeId str
 	vgNameForPv := strings.TrimSpace(infoSlice[(len(infoSlice) - 1)])
 	klog.V(4).Infof("Physical volume is part of Volume group: %v", vgNameForPv)
 	if vgNameForPv == volumeGroupName {
-		klog.V(4).Infof("Physical Volume(PV) already exists in the Volume Group")
+		klog.V(4).Infof("Physical Volume(PV) already exists in the Volume Group %v", volumeGroupName)
 	} else if vgNameForPv != "VG" && vgNameForPv != "" {
 
 		info, err = common.RunCommand("" /* pipedCmd */, nil /* pipedCmdArg */, "vgchange", []string{"-an", vgNameForPv}...)
@@ -164,7 +169,7 @@ func setupCaching(devicePath string, req *csi.NodeStageVolumeRequest, nodeId str
 		klog.V(4).Infof("Assuming valid data cache size and mode, resizing cache is not supported")
 	} else {
 		cacheSize := req.GetPublishContext()[common.ContextDataCacheSize]
-		chunkSize, err := fetchChunkSize(cacheSize)
+		chunkSize, err := fetchChunkSizeKiB(cacheSize)
 		if err != nil {
 			klog.Errorf("Errored to fetch cache size, verify the data-cache-size is valid: got %v, error: %q", cacheSize, err)
 			return mainDevicePath, err
@@ -201,7 +206,7 @@ func setupCaching(devicePath string, req *csi.NodeStageVolumeRequest, nodeId str
 			req.GetPublishContext()[common.ContextDataCacheMode],
 			volumeGroupName + "/" + mainLvName,
 			"--chunksize",
-			chunkSize,
+			chunkSize, // default unit is KiB
 			"--force",
 			"-y",
 		}
@@ -528,17 +533,17 @@ func isCachingSetup(mainLvName string) (error, bool) {
 	return nil, false
 }
 
-func fetchChunkSize(cacheSize string) (string, error) {
+func fetchChunkSizeKiB(cacheSize string) (string, error) {
 	var chunkSize float64
-	var maxChunkSize int64 = 1 * GiB   // Max allowed chunk size as per LVM documentation
-	var minChunkSize int64 = 320 * KiB // This is randomly selected, we need a multiple of 32KiB, the default size would be too small for caching https://man7.org/linux/man-pages/man8/lvcreate.8.html (--chunksize)
+
 	cacheSizeInt, err := common.ConvertGiStringToInt64(cacheSize)
 	if err != nil {
 		return "0", err
 	}
 	// Chunksize should be divisible by 32Kib so we need (chunksize/32*1024)*32*1024
-	chunkSize = float64(cacheSizeInt) / float64(maxAllowedChunks)
-	chunkSize = math.Ceil(chunkSize/float64(32*KiB)) * float64(32*KiB)
-	chunkSize = math.Min(math.Max(chunkSize, float64(minChunkSize)), float64(maxChunkSize))
-	return strconv.FormatInt(int64(chunkSize), 10), nil
+	chunkSize = (float64(cacheSizeInt) * GiB) / float64(maxAllowedChunks)
+	chunkSize = math.Round(chunkSize/(32*KiB)) * (32 * KiB)
+	chunkSize = math.Min(math.Max(chunkSize, minChunkSize), maxChunkSize) / KiB
+	// default chunk size unit KiB
+	return strconv.FormatInt(int64(chunkSize), 10) + "KiB", nil
 }

pkg/gce-pd-csi-driver/cache_test.go

@@ -0,0 +1,57 @@
+package gceGCEDriver
+
+import (
+	"testing"
+)
+
+func TestFetchChunkSizeKiB(t *testing.T) {
+	testCases := []struct {
+		name         string
+		cacheSize    string
+		expChunkSize string
+		expErr       bool
+	}{
+		{
+			name:         "chunk size is in the allowed range",
+			cacheSize:    "500Gi",
+			expChunkSize: "512KiB", //range defined in fetchChunkSizeKiB
+		},
+		{
+			name:         "chunk size is set to the range ceil",
+			cacheSize:    "30000000Gi",
+			expChunkSize: "1048576KiB", //range defined in fetchChunkSizeKiB - max 1GiB
+		},
+		{
+			name:         "chunk size is set to the allowed range floor",
+			cacheSize:    "10Gi",
+			expChunkSize: "160KiB", //range defined in fetchChunkSizeKiB - min 160 KiB
+		},
+		{
+			name:         "cacheSize set to KiB also sets the chunk size to range floor",
+			cacheSize:    "100Ki",
+			expChunkSize: "160KiB", //range defined in fetchChunkSizeKiB - min 160 KiB
+		},
+		{
+			name:         "invalid cacheSize",
+			cacheSize:    "fdfsdKi",
+			expChunkSize: "160KiB", //range defined in fetchChunkSizeKiB - min 160 KiB
+			expErr:       true,
+		},
+		// cacheSize is validated in storage class parameter so assuming invalid cacheSize (like negative, 0) would not be passed to the function
+	}
+
+	for _, tc := range testCases {
+		chunkSize, err := fetchChunkSizeKiB(tc.cacheSize)
+		if err != nil {
+			if !tc.expErr {
+				t.Errorf("Errored %s", err)
+			}
+			continue
+		}
+		if chunkSize != tc.expChunkSize {
+			t.Errorf("Got %s want %s", chunkSize, tc.expChunkSize)
+		}
+
+	}
+
+}