Add unit tests

Author: Sneha-at

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

@@ -15,24 +15,28 @@ import (
 )
 
 const (
-	cacheSuffix              = "csi-fast"
-	mainLvSuffix             = "csi-main"
-	raidedLocalSsdName       = "csi-driver-data-cache"
-	raidMode                 = "0"
-	raidedLssdPrefix         = "/dev/md/"
-	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"
+	raidedLssdPrefix           = "/dev/md/"
+	maxAllowedChunks   int64   = 1000000 // This is the max allowed chunks for LVM
+	GiB                float64 = 1024 * 1024 * 1024
+	KiB                float64 = 1024
 )
 
-var raidedLocalSsdPath = raidedLssdPrefix + raidedLocalSsdName
+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)
+	raidedLocalSsdPath         = raidedLssdPrefix + raidedLocalSsdName
+)
 
 func setupCaching(devicePath string, req *csi.NodeStageVolumeRequest, nodeId string) (string, error) {
 	volumeId := req.GetVolumeId()
 	volumeGroupName := getVolumeGroupName(nodeId)
 	mainDevicePath := "/dev/" + volumeGroupName + "/" + getLvName(mainLvSuffix, volumeId)
 	mainLvName := getLvName(mainLvSuffix, volumeId)
-	klog.V(2).Infof("Volume group available on node %v ", volumeGroupName)
+	klog.V(4).Infof("Volume group available on node %v ", volumeGroupName)
 
 	info, err := common.RunCommand("grep", raidedLocalSsdName, "ls", raidedLssdPrefix)
 	if err != nil {
@@ -72,7 +76,7 @@ func setupCaching(devicePath string, req *csi.NodeStageVolumeRequest, nodeId str
 	infoSlice := strings.Split(strings.TrimSpace(infoString), " ")
 	vgNameForPv := strings.TrimSpace(infoSlice[(len(infoSlice) - 1)])
 	if vgNameForPv == volumeGroupName {
-		klog.V(2).Infof("Physical Volume(PV) already exists in the Volume Group %v", volumeGroupName)
+		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 */, "" /* pipedCmdArg */, "vgchange", []string{"-an", vgNameForPv}...)
 		if err != nil {
@@ -143,10 +147,10 @@ func setupCaching(devicePath string, req *csi.NodeStageVolumeRequest, nodeId str
 	cacheLvName := getLvName(cacheSuffix, volumeId)
 	if isCached {
 		// Validate that cache is setup for required size
-		klog.V(2).Infof("Assuming valid data cache size and mode, resizing cache is not supported")
+		klog.V(4).Infof("Assuming valid data cache size and mode, resizing cache is not supported")
 	} else {
 		cacheSize := req.GetPublishContext()[common.ContexLocalSsdCacheSize]
-		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
@@ -183,7 +187,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",
 		}
@@ -283,7 +287,7 @@ func createVg(volumeGroupName string, devicePath string, raidedLocalSsds string)
 	if err != nil {
 		return fmt.Errorf("Volume group creation failed %w: %s", err, info)
 	}
-	klog.Infof("Volume group creation succeeded for %v", volumeGroupName)
+	klog.V(4).Infof("Volume group creation succeeded for %v", volumeGroupName)
 
 	args = []string{}
 	info, err = common.RunCommand("" /* pipedCmd */, "" /* pipedCmdArg */, "vgscan", args...)
@@ -310,9 +314,9 @@ func reduceVolumeGroup(volumeGroupName string, force bool) {
 func RaidLocalSsds() error {
 	isAlreadyRaided, err := isRaided()
 	if err != nil {
-		klog.V(2).Infof("Errored while scanning for available LocalSSDs err:%v; continuing Raiding", err)
+		klog.V(4).Infof("Errored while scanning for available LocalSSDs err:%v; continuing raiding", err)
 	} else if isAlreadyRaided {
-		klog.V(2).Infof("Local SSDs are already RAIDed, no further action needed here")
+		klog.V(4).Infof("Local SSDs are already RAIDed, no further action needed here")
 		return nil
 	}
 	diskList := []string{}
@@ -356,7 +360,7 @@ func RaidLocalSsds() error {
 	// Validate if Raided successfully
 	isAlreadyRaided, err = isRaided()
 	if err != nil {
-		klog.V(2).Infof("Errored while scanning for available raided LocalSSDs err:%v=", err)
+		klog.V(4).Infof("Errored while scanning for available raided LocalSSDs err:%v=", err)
 	}
 	if !isAlreadyRaided {
 		return fmt.Errorf("failed raiding, raided device not found on scanning")
@@ -397,17 +401,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,50 @@
+package gceGCEDriver
+
+import (
+	"testing"
+
+	"google.golang.org/grpc/codes"
+)
+
+func TestFetchChunkSizeKiB(t *testing.T) {
+	testCases := []struct {
+		name         string
+		cacheSize    string
+		expChunkSize string
+		expErrCode   codes.Code
+	}{
+		{
+			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
+		},
+		// 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 {
+			t.Errorf("Errored %s", err)
+		}
+		if chunkSize != tc.expChunkSize {
+			t.Errorf("Got %s want %s", chunkSize, tc.expChunkSize)
+		}
+
+	}
+
+}