Fix leading comma issue for mount option when enable only-dir

Author: yaozile123

test/e2e/specs/specs.go

@@ -1106,21 +1106,16 @@ func deployGCSFuseVersionFetcherPod(ctx context.Context, clientset clientset.Int
 	configMaps, err := clientset.CoreV1().ConfigMaps("").List(ctx, metav1.ListOptions{
 		FieldSelector: "metadata.name=gcsfusecsi-image-config",
 	})
-	if err != nil {
-		klog.Errorf("Expected no error when list configmaps, but found: %v", err)
-		return ""
-	}
+	framework.ExpectNoError(err)
 	if len(configMaps.Items) != 1 {
-		klog.Errorf("Expected 1 gcsfusecsi-image-config ConfigMap, found %d", len(configMaps.Items))
-		return ""
+		framework.Failf("expected one config map `gcsfusecsi-image-config` but found %d", len(configMaps.Items))
 	}
 
 	sidecarImageConfig := configMaps.Items[0]
 
 	image := sidecarImageConfig.Data["sidecar-image"]
 	if image == "" {
-		klog.Errorf("sidecar-image data is empty in configmap")
-		return ""
+		framework.Failf("expected data for key `sidecar-image` in the config map `gcsfusecsi-image-config`")
 	}
 
 	pod := &corev1.Pod{

test/e2e/testsuites/failed_mount.go

@@ -196,7 +196,7 @@ func (t *gcsFuseCSIFailedMountTestSuite) DefineTests(driver storageframework.Tes
 		ginkgo.By("Checking that the pod has failed mount error")
 
 		if GCSFuseVersionStr == "" {
-			GCSFuseVersionStr = specs.GetGCSFuseVersion(ctx, f)
+			GCSFuseVersionStr = specs.GetGCSFuseVersion(ctx)
 		}
 		v, err := version.ParseSemantic(GCSFuseVersionStr)
 		if configPrefix == specs.SkipCSIBucketAccessCheckAndInvalidVolumePrefix && (err != nil || v.AtLeast(version.MustParseSemantic("v2.9.0-gke.0"))) {

test/e2e/testsuites/gcsfuse_integration.go

@@ -246,7 +246,7 @@ func (t *gcsFuseCSIGCSFuseIntegrationTestSuite) DefineTests(driver storageframew
 		// Check if the GCSFuseVersion is already set, if not, we should set it.
 		// This is for the case that we run this test suite individually.
 		if GCSFuseVersionStr == "" {
-			GCSFuseVersionStr = specs.GetGCSFuseVersion(ctx, f)
+			GCSFuseVersionStr = specs.GetGCSFuseVersion(ctx)
 		}
 		gcsfuseVersion := version.MustParseSemantic(GCSFuseVersionStr)
 

test/e2e/testsuites/gcsfuse_integration_file_cache.go

@@ -113,7 +113,7 @@ func (t *gcsFuseCSIGCSFuseIntegrationFileCacheTestSuite) DefineTests(driver stor
 	gcsfuseIntegrationFileCacheTest := func(testName string, readOnly bool, fileCacheCapacity, fileCacheForRangeRead, metadataCacheTTLSeconds string, mountOptions ...string) {
 		ginkgo.By("Checking GCSFuse version and skip test if needed")
 		if GCSFuseVersionStr == "" {
-			GCSFuseVersionStr = specs.GetGCSFuseVersion(ctx, f)
+			GCSFuseVersionStr = specs.GetGCSFuseVersion(ctx)
 		}
 		ginkgo.By(fmt.Sprintf("Running integration test %v with GCSFuse version %v", testName, GCSFuseVersionStr))
 		gcsfuseTestBranch := skipTestOrProceedWithBranch(GCSFuseVersionStr, testName)

test/e2e/testsuites/gcsfuse_integration_file_cache_parallel_downloads.go

@@ -219,8 +219,7 @@ func (t *gcsFuseCSIGCSFuseIntegrationFileCacheParallelDownloadsTestSuite) Define
 
 		// Filter out duplicate logging:severity:info from testdriver set up
 		mo := l.volumeResource.VolSource.CSI.VolumeAttributes["mountOptions"]
-		mo = strings.ReplaceAll(mo, "logging:severity:info", "")
-		mo = strings.TrimPrefix(mo, ",") // Remove leading comma because of ,onlyDir
+		mo = strings.ReplaceAll(mo, "logging:severity:info", fmt.Sprintf("logging:severity:%v", config.LogSeverity))
 		l.volumeResource.VolSource.CSI.VolumeAttributes["mountOptions"] = mo
 
 		tPod.SetupVolume(l.volumeResource, volumeName, mountPath, config.ReadOnly, config.MountOptions...)

test/e2e/utils/utils.go

@@ -39,11 +39,16 @@ const (
 	MinGCSFuseKernelParamsVersion = "v3.7.0-gke.0"
 	MinGCSFuseTestConfigVersion   = "v3.5.0-gke.0"
 
-	testConfigUrlFormat   = "https://raw.githubusercontent.com/GoogleCloudPlatform/gcsfuse/%s/tools/integration_tests/test_config.yaml"
-	flagFileCacheCapacity = "file-cache-max-size-mb="
-	flagReadOnly          = "o=ro"
-	flagLogFile           = "log-file="
-	flagCacheDir          = "cache-dir="
+	testConfigUrlFormat                  = "https://raw.githubusercontent.com/GoogleCloudPlatform/gcsfuse/%s/tools/integration_tests/test_config.yaml"
+	flagFileCacheCapacity                = "file-cache-max-size-mb="
+	flagReadOnly                         = "o=ro"
+	flagLogFile                          = "log-file="
+	flagCacheDir                         = "cache-dir="
+	flagLogSeverity                      = "log-severity="
+	flagLogFormat                        = "log-format="
+	flagFileCacheEnableParallelDownloads = "file-cache-enable-parallel-downloads"
+	flagFileCacheEnableODirect           = "file-cache-enable-o-direct"
+	flagEnableKernelReader               = "enable-kernel-reader"
 )
 
 var (
@@ -192,6 +197,7 @@ type ParsedConfig struct {
 	FileCacheCapacity string
 	ReadOnly          bool
 	LogFilePath       string
+	LogSeverity       string
 	CacheDir          string
 	MountOptions      []string
 }
@@ -203,6 +209,7 @@ func ParseConfigFlags(flagStr string) ParsedConfig {
 		FileCacheCapacity: "50Mi",
 		ReadOnly:          false,
 		LogFilePath:       "",
+		LogSeverity:       "info",
 		CacheDir:          "",
 		MountOptions:      []string{},
 	}
@@ -218,56 +225,63 @@ func ParseConfigFlags(flagStr string) ParsedConfig {
 		// They cannot be passed via mountOptions and must be parsed to configure the test environment manually.
 		// See: https://github.com/GoogleCloudPlatform/gcs-fuse-csi-driver/blob/main/pkg/sidecar_mounter/sidecar_mounter_config.go#L83
 
+		// Group 1: Flags that are parsed manually and should not be passed to mountOptions.
+
 		// file-cache:max-size-mb is used by the CSI driver to enable the file cache feature and configure the cache volume.
 		// If not provided or set to 0, the cache directory will not be created.
 		// See: https://github.com/GoogleCloudPlatform/gcs-fuse-csi-driver/blob/main/pkg/sidecar_mounter/sidecar_mounter_config.go#L237-L241
 		if strings.HasPrefix(f, flagFileCacheCapacity) {
 			parsed.FileCacheCapacity = strings.TrimPrefix(f, flagFileCacheCapacity) + "Mi"
+			continue
 		}
 
 		// "o" is disallowed and will be used in SetupVolume to set the readOnly flag for test pod.
 		if f == flagReadOnly {
 			parsed.ReadOnly = true
-		}
-
-		// "log-file" is disallowed. The gcsfuse e2e tests hardcoded the log file in their test config.s
-		// We parse this to configure the test pod to align with the gcsfuse test config.
-		if strings.HasPrefix(f, flagLogFile) {
-			parsed.LogFilePath = strings.TrimPrefix(f, flagLogFile)
-			f = "logging:file-path:" + parsed.LogFilePath
+			continue
 		}
 
 		// "cache-dir" is disallowed to prevent storage exhaustion. The gcsfuse e2e tests hardcoded the cache dir in their test config.
 		// We parse this to manually set up the cache volume in the test pod to align with the gcsfuse test config.
 		if strings.HasPrefix(f, flagCacheDir) {
 			parsed.CacheDir = strings.TrimPrefix(f, flagCacheDir)
+			continue
 		}
 
-		// Translate legacy CLI flags to config file representation
-		if f == "file-cache-enable-parallel-downloads" || strings.HasPrefix(f, "file-cache-enable-parallel-downloads=") {
+		// "log-severity" is disallowed so we parse it into the test pod configuration.
+		if strings.HasPrefix(f, flagLogSeverity) {
+			parsed.LogSeverity = strings.TrimPrefix(f, flagLogSeverity)
+			continue
+		}
+
+		// Group 2: The following flags are translated to config file representation and passed to mountOptions.
+
+		// "log-file" is disallowed. The gcsfuse e2e tests hardcoded the log file in their test config.s
+		// We parse this to configure the test pod to align with the gcsfuse test config.
+		if strings.HasPrefix(f, flagLogFile) {
+			parsed.LogFilePath = strings.TrimPrefix(f, flagLogFile)
+			f = "logging:file-path:" + parsed.LogFilePath
+		} else if f == flagFileCacheEnableParallelDownloads || strings.HasPrefix(f, flagFileCacheEnableParallelDownloads+"=") {
 			val := "true"
 			if strings.Contains(f, "=") {
 				val = strings.SplitN(f, "=", 2)[1]
 			}
 			f = "file-cache:enable-parallel-downloads:" + val
-		} else if f == "file-cache-enable-o-direct" || strings.HasPrefix(f, "file-cache-enable-o-direct=") {
+		} else if f == flagFileCacheEnableODirect || strings.HasPrefix(f, flagFileCacheEnableODirect+"=") {
 			val := "true"
 			if strings.Contains(f, "=") {
 				val = strings.SplitN(f, "=", 2)[1]
 			}
 			f = "file-cache:enable-o-direct:" + val
-		} else if f == "enable-kernel-reader" || strings.HasPrefix(f, "enable-kernel-reader=") {
+		} else if f == flagEnableKernelReader || strings.HasPrefix(f, flagEnableKernelReader+"=") {
 			val := "true"
 			if strings.Contains(f, "=") {
 				val = strings.SplitN(f, "=", 2)[1]
 			}
 			f = "file-system:enable-kernel-reader:" + val
-		} else if strings.HasPrefix(f, "log-severity=") {
-			// "log-severity" is disallowed so we need to format it to config file format
-			f = "logging:severity:" + strings.TrimPrefix(f, "log-severity=")
-		} else if strings.HasPrefix(f, "log-format=") {
+		} else if strings.HasPrefix(f, flagLogFormat) {
 			// "log-format" is disallowed so we need to format it to config file format
-			f = "logging:format:" + strings.TrimPrefix(f, "log-format=")
+			f = "logging:format:" + strings.TrimPrefix(f, flagLogFormat)
 		}
 
 		parsed.MountOptions = append(parsed.MountOptions, f)