Refactored tune2fs format option test works

AndrewSirenko · Sep 29, 2023 · e407072 · e407072
1 parent d64a617
commit e407072
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 27 deletions.
diff --git a/tests/e2e/format_options.go b/tests/e2e/format_options.go
@@ -24,24 +24,18 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/test/e2e/framework"
 	admissionapi "k8s.io/pod-security-admission/api"
-	"strconv"
 )
 
 const (
-	initialVolumeSizeGi     = 2
-	volumeSizeIncreaseAmtGi = 2
-
 	blockSizeTestValue      = "1024"
 	inodeSizeTestValue      = "512"
 	bytesPerInodeTestValue  = "8192"
 	numberOfInodesTestValue = "200192"
+
+	expectedBytesPerInodeTestResult = "131072" // TODO having this here is code smell. Hardcode? Test case from original inode PR #1661 https://github.com/kubernetes-sigs/aws-ebs-csi-driver/pull/1661
 )
 
-// TODO is this a clean place for this?
 var (
-	// TODO having this here is code smell. Hardcode? Test case from original inode PR #1661 https://github.com/kubernetes-sigs/aws-ebs-csi-driver/pull/1661
-	expectedBytesPerInodeTestResult = strconv.Itoa(131072 * initialVolumeSizeGi)
-
 	formatOptionTests = []testsuites.FormatOptionTest{
 		{
 			CreateVolumeParameterKey:        ebscsidriver.BlockSizeKey,
@@ -64,7 +58,8 @@ var (
 			CreateVolumeParameterKey:        ebscsidriver.NumberOfINodesKey,
 			CreateVolumeParameterValue:      numberOfInodesTestValue,
 			ExpectedFilesystemInfoParamName: "Inode count",
-			ExpectedFilesystemInfoParamVal:  numberOfInodesTestValue},
+			ExpectedFilesystemInfoParamVal:  numberOfInodesTestValue,
+		},
 	}
 )
 
@@ -88,19 +83,15 @@ var _ = Describe("[ebs-csi-e2e] [single-az] [format-options] Formatting a volume
 
 	for _, fsType := range testedFsTypes {
 		Context(fmt.Sprintf("with an %s filesystem", fsType), func() {
-			// TODO: is formatOptionTestCase clear? Or should it be 'formatOptionTestCaseValues'
 			for _, formatOptionTestCase := range formatOptionTests {
+				formatOptionTestCase := formatOptionTestCase // Go trap
 				if fsTypeDoesNotSupportFormatOptionParameter(fsType, formatOptionTestCase.CreateVolumeParameterKey) {
 					continue
 				}
 
 				Context(fmt.Sprintf("with a custom %s parameter", formatOptionTestCase.CreateVolumeParameterKey), func() {
 					It("successfully mounts and is resizable", func() {
-						test := testsuites.FormatOptionTest{
-							CSIDriver:                   ebsDriver,
-							testsuites.FormatOptionTest: formatOptionTestCase,
-						}
-						test.Run(cs, ns)
+						formatOptionTestCase.Run(cs, ns, ebsDriver, fsType)
 					})
 				})
 			}

diff --git a/tests/e2e/testsuites/format_options_tester.go b/tests/e2e/testsuites/format_options_tester.go
@@ -17,6 +17,7 @@ package testsuites
 import (
 	"fmt"
 	awscloud "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud"
+	"github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/driver"
 	"github.com/onsi/gomega/format"
 	v1 "k8s.io/api/core/v1"
 	clientset "k8s.io/client-go/kubernetes"
@@ -39,9 +40,8 @@ type FormatOptionTest struct {
 }
 
 const (
-	volumeSizeIncreaseAmtGi = 2
-
-	volumeMountPath = "/mnt/test-format-option" // TODO should I keep this as mnt/test-1, and refactor to be `DefaultMountPath` globally in testsuites?
+	volumeSizeIncreaseAmtGi = 1
+	volumeMountPath         = "/mnt/test-format-option" // TODO should I keep this as mnt/test-1, and refactor to be `DefaultMountPath` globally in testsuites?
 )
 
 var (
@@ -50,7 +50,7 @@ var (
 
 )
 
-func (t *FormatOptionTest) Run(client clientset.Interface, namespace *v1.Namespace, fsType string) {
+func (t *FormatOptionTest) Run(client clientset.Interface, namespace *v1.Namespace, ebsDriver driver.PVTestDriver, fsType string) {
 	By("setting up pvc")
 	volumeDetails := createFormatOptionVolumeDetails(fsType, volumeMountPath, t)
 	testPvc, _ := volumeDetails.SetupDynamicPersistentVolumeClaim(client, namespace, ebsDriver)
@@ -79,14 +79,14 @@ func (t *FormatOptionTest) Run(client clientset.Interface, namespace *v1.Namespa
 }
 
 // TODO should we improve this across e2e tests via builder design pattern? Or is that not go-like?
-func createFormatOptionVolumeDetails(fsType string, volumeMountPath string, t FormatOptionTest) *VolumeDetails {
+func createFormatOptionVolumeDetails(fsType string, volumeMountPath string, t *FormatOptionTest) *VolumeDetails {
 	allowVolumeExpansion := true
 
 	volume := VolumeDetails{
 		VolumeType:   awscloud.VolumeTypeGP2,
 		FSType:       fsType,
 		MountOptions: []string{"rw"},
-		ClaimSize:    fmt.Sprintf("%vGi", initialVolumeSizeGi),
+		ClaimSize:    driver.MinimumSizeForVolumeType(awscloud.VolumeTypeGP2),
 		VolumeMount: VolumeMountDetails{
 			NameGenerate:      "test-volume-format-option",
 			MountPathGenerate: volumeMountPath,
@@ -109,16 +109,17 @@ func createPodWithVolume(client clientset.Interface, namespace *v1.Namespace, cm
 	return testPod
 }
 
-// TODO: Maybe should use something other than Find(), but *shrug*
-// TODO should I move this to go ?
-// TODO Should I instead use RunHostCmd or LookForString from https://github.com/kubernetes/kubernetes/blob/master/test/e2e/framework/pod/output/output.go ?
+// TODO should I move this to testsuites.go ?
 
 // FindRegexpInPodLogs searches given testPod's logs for a given regular expression. Returns `true` if found.
 func FindRegexpInPodLogs(regexpPattern string, testPod *TestPod) bool {
+	By(fmt.Sprintf("Searching for matching regexp '%s' in logs of pod", regexpPattern))
 	podLogs, err := testPod.Logs()
-	framework.ExpectNoError(err, "Tried getting logs for pod %s", format.Object(testPod, 2))
+	framework.ExpectNoError(err, "tried getting logs for pod %s", format.Object(testPod, 2))
 
 	var expectedLine = regexp.MustCompile(regexpPattern)
+
 	res := expectedLine.Find(podLogs)
+	framework.Logf("result of regexp search through pod logs: '%s'", string(res))
 	return res != nil
 }
diff --git a/tests/e2e/testsuites/specs.go b/tests/e2e/testsuites/specs.go
@@ -107,9 +107,9 @@ func (pod *PodDetails) SetupWithPreProvisionedVolumes(client clientset.Interface
 		cleanupFuncs = append(cleanupFuncs, funcs...)
 
 		if v.VolumeMode == Block {
-			tpod.SetupRawBlockVolume(tpvc.PersistentVolumeClaim, fmt.Sprintf("%s%d", v.VolumeDevice.NameGenerate, n+1), v.VolumeDevice.DevicePath)
+			tpod.SetupRawBlockVolume(tpvc.persistentVolumeClaim, fmt.Sprintf("%s%d", v.VolumeDevice.NameGenerate, n+1), v.VolumeDevice.DevicePath)
 		} else {
-			tpod.SetupVolume(tpvc.PersistentVolumeClaim, fmt.Sprintf("%s%d", v.VolumeMount.NameGenerate, n+1), fmt.Sprintf("%s%d", v.VolumeMount.MountPathGenerate, n+1), v.VolumeMount.ReadOnly)
+			tpod.SetupVolume(tpvc.persistentVolumeClaim, fmt.Sprintf("%s%d", v.VolumeMount.NameGenerate, n+1), fmt.Sprintf("%s%d", v.VolumeMount.MountPathGenerate, n+1), v.VolumeMount.ReadOnly)
 		}
 	}
 	return tpod, cleanupFuncs