From 6653915f95b5212b0c0169d43011cf7beff87d55 Mon Sep 17 00:00:00 2001 From: Ashmeen Kaur Date: Thu, 6 Jun 2024 09:04:23 +0000 Subject: [PATCH 01/10] add normalise function for fstab flags --- cfg/config.go | 7 +++++-- tools/config-gen/config.tpl | 5 +++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cfg/config.go b/cfg/config.go index 996759627c..367879e145 100644 --- a/cfg/config.go +++ b/cfg/config.go @@ -12,11 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -// GENERATED CODE - DO NOT EDIT MANUALLY. - package cfg import ( + "strings" + "github.com/spf13/pflag" "github.com/spf13/viper" ) @@ -57,6 +57,9 @@ type LoggingConfig struct { func BindFlags(flagSet *pflag.FlagSet) error { var err error + flagSet.SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName { + return pflag.NormalizedName(strings.ReplaceAll(name, "-", "_")) + }) flagSet.StringP("app-name", "", "", "The application name of this mount.") diff --git a/tools/config-gen/config.tpl b/tools/config-gen/config.tpl index 242e8b34c7..766082361d 100644 --- a/tools/config-gen/config.tpl +++ b/tools/config-gen/config.tpl @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// GENERATED CODE - DO NOT EDIT MANUALLY. - package cfg import ( @@ -32,6 +30,9 @@ type {{ .TypeName}} struct { func BindFlags(flagSet *pflag.FlagSet) error { var err error + flagSet.SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName { + return pflag.NormalizedName(strings.ReplaceAll(name, "-", "_")) + }) {{range .FlagTemplateData}} flagSet.{{ .Fn}}("{{ .FlagName}}", "{{ .Shorthand}}", {{ .DefaultValue}}, {{ .Usage}}) {{if .IsDeprecated}} From bb457fea83e140901c9441a52a8097ac48ae7a12 Mon Sep 17 00:00:00 2001 From: Ashmeen Kaur Date: Mon, 10 Jun 2024 10:09:14 +0000 Subject: [PATCH 02/10] allow passing gcsfuse hyphen flags in persistent mount --- tools/mount_gcsfuse/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/mount_gcsfuse/main.go b/tools/mount_gcsfuse/main.go index c4bcef8f8f..4bdeba8db3 100644 --- a/tools/mount_gcsfuse/main.go +++ b/tools/mount_gcsfuse/main.go @@ -74,7 +74,7 @@ func makeGcsfuseArgs( opts map[string]string) (args []string, err error) { // Deal with options. for name, value := range opts { - switch name { + switch strings.Replace(name, "-", "_", -1) { // Don't pass through options that are relevant to mount(8) but not to // gcsfuse, and that fusermount chokes on with "Invalid argument" on Linux. case "user", "nouser", "auto", "noauto", "_netdev", "no_netdev": From 4d570b826c38233dba6756bbc8f7fcf35f823b3d Mon Sep 17 00:00:00 2001 From: Ashmeen Kaur Date: Mon, 10 Jun 2024 10:30:19 +0000 Subject: [PATCH 03/10] add missing flags --- tools/mount_gcsfuse/main.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tools/mount_gcsfuse/main.go b/tools/mount_gcsfuse/main.go index 4bdeba8db3..ca1d2275d0 100644 --- a/tools/mount_gcsfuse/main.go +++ b/tools/mount_gcsfuse/main.go @@ -85,7 +85,11 @@ func makeGcsfuseArgs( "experimental_local_file_cache", "reuse_token_from_url", "enable_nonexistent_type_cache", - "experimental_enable_json_read": + "experimental_enable_json_read", + "enable_hns", + "ignore_interrupts", + "anonymous_acess", + "log_rotate_compress": if value == "" { value = "true" } @@ -123,7 +127,16 @@ func makeGcsfuseArgs( "custom_endpoint", "config_file", "experimental_metadata_prefetch_on_mount", - "kernel_list_cache_ttl_secs": + "kernel_list_cache_ttl_secs", + "stat_cache_max_size_mb", + "type_cache_max_size_mb", + "metadata_cache_ttl_secs", + "log_severity", + "log_rotate_max_file_size_mb", + "log_rotate_backup_file-count", + "file_cache_max_size_mb", + "grpc_conn_pool_size": + args = append(args, "--"+strings.Replace(name, "_", "-", -1), value) // Special case: support mount-like formatting for gcsfuse debug flags. From 600c66766856e3dff09a5dba16029db31800f3fb Mon Sep 17 00:00:00 2001 From: Ashmeen Kaur Date: Mon, 10 Jun 2024 10:37:50 +0000 Subject: [PATCH 04/10] revert normalisation changes as it is not required --- cfg/config.go | 5 ----- tools/config-gen/config.tpl | 3 --- 2 files changed, 8 deletions(-) diff --git a/cfg/config.go b/cfg/config.go index 367879e145..c9d7e6be21 100644 --- a/cfg/config.go +++ b/cfg/config.go @@ -15,8 +15,6 @@ package cfg import ( - "strings" - "github.com/spf13/pflag" "github.com/spf13/viper" ) @@ -57,9 +55,6 @@ type LoggingConfig struct { func BindFlags(flagSet *pflag.FlagSet) error { var err error - flagSet.SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName { - return pflag.NormalizedName(strings.ReplaceAll(name, "-", "_")) - }) flagSet.StringP("app-name", "", "", "The application name of this mount.") diff --git a/tools/config-gen/config.tpl b/tools/config-gen/config.tpl index 766082361d..6525a8e21e 100644 --- a/tools/config-gen/config.tpl +++ b/tools/config-gen/config.tpl @@ -30,9 +30,6 @@ type {{ .TypeName}} struct { func BindFlags(flagSet *pflag.FlagSet) error { var err error - flagSet.SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName { - return pflag.NormalizedName(strings.ReplaceAll(name, "-", "_")) - }) {{range .FlagTemplateData}} flagSet.{{ .Fn}}("{{ .FlagName}}", "{{ .Shorthand}}", {{ .DefaultValue}}, {{ .Usage}}) {{if .IsDeprecated}} From d256565fb0b9bf5bc0d65a1822b90b39f0807640 Mon Sep 17 00:00:00 2001 From: Ashmeen Kaur Date: Mon, 10 Jun 2024 13:25:50 +0000 Subject: [PATCH 05/10] fix typo --- tools/mount_gcsfuse/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/mount_gcsfuse/main.go b/tools/mount_gcsfuse/main.go index ca1d2275d0..0b2eed9f6e 100644 --- a/tools/mount_gcsfuse/main.go +++ b/tools/mount_gcsfuse/main.go @@ -88,7 +88,7 @@ func makeGcsfuseArgs( "experimental_enable_json_read", "enable_hns", "ignore_interrupts", - "anonymous_acess", + "anonymous_access", "log_rotate_compress": if value == "" { value = "true" From 3358e71f3dc1bbd5cb9bec7f3c8b3969964e8876 Mon Sep 17 00:00:00 2001 From: Ashmeen Kaur Date: Mon, 10 Jun 2024 13:25:56 +0000 Subject: [PATCH 06/10] add unit test --- tools/mount_gcsfuse/main_test.go | 163 +++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 tools/mount_gcsfuse/main_test.go diff --git a/tools/mount_gcsfuse/main_test.go b/tools/mount_gcsfuse/main_test.go new file mode 100644 index 0000000000..5c41b917f0 --- /dev/null +++ b/tools/mount_gcsfuse/main_test.go @@ -0,0 +1,163 @@ +// Copyright 2024 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMakeGcsfuseArgs(t *testing.T) { + testCases := []struct { + name string + device string + mountPoint string + opts map[string]string + expected []string + expectErr bool + }{ + { + name: "TestMakeGcsfuseArgs with NoOptions", + device: "gcsfuse", + mountPoint: "/mnt/gcs", + opts: map[string]string{}, + expected: []string{"gcsfuse", "/mnt/gcs"}, + }, + + { + name: "TestMakeGcsfuseArgs for BooleanFlags with underscore", + device: "gcsfuse", + mountPoint: "/mnt/gcs", + opts: map[string]string{"implicit_dirs": "", + "foreground": "true", + "experimental_local_file_cache": "", + "reuse_token_from_url": "false", + "enable_nonexistent_type_cache": "", + "experimental_enable_json_read": "true", + "enable_hns": "", + "ignore_interrupts": "", + "anonymous_access": "false", + "log_rotate_compress": "false"}, + expected: []string{"--implicit-dirs=true", + "--foreground=true", + "--experimental-local-file-cache=true", + "--reuse-token-from-url=false", + "--enable-nonexistent-type-cache=true", + "--experimental-enable-json-read=true", + "--enable-hns=true", + "--ignore-interrupts=true", + "--anonymous-access=false", + "--log-rotate-compress=false", + "gcsfuse", "/mnt/gcs"}, + }, + + { + name: "TestMakeGcsfuseArgs for BooleanFlags with hyphens", + device: "gcsfuse", + mountPoint: "/mnt/gcs", + opts: map[string]string{"implicit_dirs": "", + "foreground": "true", + "experimental-local-file-cache": "", + "reuse-token-from-url": "false", + "enable-nonexistent-type-cache": "", + "experimental-enable-json-read": "true", + "enable-hns": "", + "ignore-interrupts": "", + "anonymous-access": "false", + "log_rotate-compress": "false"}, + expected: []string{"--implicit-dirs=true", + "--foreground=true", + "--experimental-local-file-cache=true", + "--reuse-token-from-url=false", + "--enable-nonexistent-type-cache=true", + "--experimental-enable-json-read=true", + "--enable-hns=true", + "--ignore-interrupts=true", + "--anonymous-access=false", + "--log-rotate-compress=false", + "gcsfuse", "/mnt/gcs"}, + }, + + { + name: "TestMakeGcsfuseArgs for StringFlags with underscore", + device: "gcsfuse", + mountPoint: "/mnt/gcs", + opts: map[string]string{ + "dir_mode": "0755", "key_file": "/path/to/key"}, + expected: []string{"--dir-mode", "0755", "--key-file", "/path/to/key", "gcsfuse", "/mnt/gcs"}, + }, + + { + name: "TestMakeGcsfuseArgs for StringFlags with hyphen", + device: "gcsfuse", + mountPoint: "/mnt/gcs", + opts: map[string]string{ + "dir-mode": "0755", "key-file": "/path/to/key"}, + expected: []string{"--dir-mode", "0755", "--key-file", "/path/to/key", "gcsfuse", "/mnt/gcs"}, + }, + + { + name: "TestMakeGcsfuseArgs with DebugFlags", + device: "gcsfuse", + mountPoint: "/mnt/gcs", + opts: map[string]string{"debug_fuse": "", "debug_gcs": ""}, + expected: []string{"--debug_fuse", "--debug_gcs", "gcsfuse", "/mnt/gcs"}, + }, + + // Test ignored options + { + name: "TestMakeGcsfuseArgs with IgnoredOptions", + device: "gcsfuse", + mountPoint: "/mnt/gcs", + opts: map[string]string{"user": "nobody", "_netdev": ""}, + expected: []string{"gcsfuse", "/mnt/gcs"}, + }, + + { + name: "TestMakeGcsfuseArgs with RegularOptions", + device: "gcsfuse", + mountPoint: "/mnt/gcs", + opts: map[string]string{"allow_other": "", "ro": ""}, + expected: []string{"-o", "allow_other", "-o", "ro", "gcsfuse", "/mnt/gcs"}, + }, + + { + name: "TestMakeGcsfuseArgs with MixedOptions", + device: "gcsfuse", + mountPoint: "/mnt/gcs", + opts: map[string]string{ + "implicit_dirs": "", "file_mode": "0644", "debug_fuse": "", "allow_other": "", + }, + expected: []string{"--implicit-dirs=true", "--file-mode", "0644", "--debug_fuse", "-o", "allow_other", "gcsfuse", "/mnt/gcs"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + args, err := makeGcsfuseArgs(tc.device, tc.mountPoint, tc.opts) + if tc.expectErr && err == nil { + t.Errorf("Expected error, but got none") + } else if !tc.expectErr && err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // Assert that all flags are present (no matter the order). + assert.ElementsMatch(t, args[:len(args)-2], tc.expected[:len(tc.expected)-2]) + // Assert that device and mount-point are present at correct position. + assert.Equal(t, args[len(args)-2:], tc.expected[len(tc.expected)-2:]) + }) + } +} From 9f8e657024260a77275f622a1421369925603ee1 Mon Sep 17 00:00:00 2001 From: Ashmeen Kaur Date: Tue, 11 Jun 2024 10:48:45 +0000 Subject: [PATCH 07/10] review comments --- cfg/config.go | 2 ++ tools/config-gen/config.tpl | 2 ++ tools/mount_gcsfuse/main.go | 2 +- tools/mount_gcsfuse/main_test.go | 21 +++++++++++---------- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/cfg/config.go b/cfg/config.go index c9d7e6be21..9997e3c5d4 100644 --- a/cfg/config.go +++ b/cfg/config.go @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +// GENERATED CODE - DO NOT EDIT config.go FILE MANUALLY. + package cfg import ( diff --git a/tools/config-gen/config.tpl b/tools/config-gen/config.tpl index 6525a8e21e..b3d05b3ae4 100644 --- a/tools/config-gen/config.tpl +++ b/tools/config-gen/config.tpl @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +// GENERATED CODE - DO NOT EDIT config.go FILE MANUALLY. + package cfg import ( diff --git a/tools/mount_gcsfuse/main.go b/tools/mount_gcsfuse/main.go index 0b2eed9f6e..b65a70f7a6 100644 --- a/tools/mount_gcsfuse/main.go +++ b/tools/mount_gcsfuse/main.go @@ -133,7 +133,7 @@ func makeGcsfuseArgs( "metadata_cache_ttl_secs", "log_severity", "log_rotate_max_file_size_mb", - "log_rotate_backup_file-count", + "log_rotate_backup_file_count", "file_cache_max_size_mb", "grpc_conn_pool_size": diff --git a/tools/mount_gcsfuse/main_test.go b/tools/mount_gcsfuse/main_test.go index 5c41b917f0..062d3a73fd 100644 --- a/tools/mount_gcsfuse/main_test.go +++ b/tools/mount_gcsfuse/main_test.go @@ -27,7 +27,6 @@ func TestMakeGcsfuseArgs(t *testing.T) { mountPoint string opts map[string]string expected []string - expectErr bool }{ { name: "TestMakeGcsfuseArgs with NoOptions", @@ -96,8 +95,11 @@ func TestMakeGcsfuseArgs(t *testing.T) { device: "gcsfuse", mountPoint: "/mnt/gcs", opts: map[string]string{ - "dir_mode": "0755", "key_file": "/path/to/key"}, - expected: []string{"--dir-mode", "0755", "--key-file", "/path/to/key", "gcsfuse", "/mnt/gcs"}, + "dir_mode": "0755", + "key_file": "/path/to/key", + "log_rotate_backup_file_count": "2", + }, + expected: []string{"--dir-mode", "0755", "--key-file", "/path/to/key", "--log-rotate-backup-file-count", "2", "gcsfuse", "/mnt/gcs"}, }, { @@ -105,8 +107,11 @@ func TestMakeGcsfuseArgs(t *testing.T) { device: "gcsfuse", mountPoint: "/mnt/gcs", opts: map[string]string{ - "dir-mode": "0755", "key-file": "/path/to/key"}, - expected: []string{"--dir-mode", "0755", "--key-file", "/path/to/key", "gcsfuse", "/mnt/gcs"}, + "dir-mode": "0755", + "key-file": "/path/to/key", + "log-rotate-backup-file-count": "2", + }, + expected: []string{"--dir-mode", "0755", "--key-file", "/path/to/key", "--log-rotate-backup-file-count", "2", "gcsfuse", "/mnt/gcs"}, }, { @@ -148,11 +153,7 @@ func TestMakeGcsfuseArgs(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { args, err := makeGcsfuseArgs(tc.device, tc.mountPoint, tc.opts) - if tc.expectErr && err == nil { - t.Errorf("Expected error, but got none") - } else if !tc.expectErr && err != nil { - t.Errorf("Unexpected error: %v", err) - } + assert.Nil(t, err) // Assert that all flags are present (no matter the order). assert.ElementsMatch(t, args[:len(args)-2], tc.expected[:len(tc.expected)-2]) From a926c84503027d5fd0633d14370636c5fd5dce65 Mon Sep 17 00:00:00 2001 From: Ashmeen Kaur Date: Tue, 11 Jun 2024 11:07:01 +0000 Subject: [PATCH 08/10] review comments --- cfg/config.go | 2 +- tools/config-gen/config.tpl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cfg/config.go b/cfg/config.go index 9997e3c5d4..996759627c 100644 --- a/cfg/config.go +++ b/cfg/config.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// GENERATED CODE - DO NOT EDIT config.go FILE MANUALLY. +// GENERATED CODE - DO NOT EDIT MANUALLY. package cfg diff --git a/tools/config-gen/config.tpl b/tools/config-gen/config.tpl index b3d05b3ae4..242e8b34c7 100644 --- a/tools/config-gen/config.tpl +++ b/tools/config-gen/config.tpl @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// GENERATED CODE - DO NOT EDIT config.go FILE MANUALLY. +// GENERATED CODE - DO NOT EDIT MANUALLY. package cfg From c4782cf970c09a9f73ac873ac62b16ec9f1336c3 Mon Sep 17 00:00:00 2001 From: Ashmeen Kaur Date: Thu, 13 Jun 2024 05:14:22 +0000 Subject: [PATCH 09/10] review comments --- tools/mount_gcsfuse/main.go | 6 +-- tools/mount_gcsfuse/main_test.go | 89 ++++++++++++-------------------- 2 files changed, 37 insertions(+), 58 deletions(-) diff --git a/tools/mount_gcsfuse/main.go b/tools/mount_gcsfuse/main.go index b65a70f7a6..9a34c28b93 100644 --- a/tools/mount_gcsfuse/main.go +++ b/tools/mount_gcsfuse/main.go @@ -74,7 +74,7 @@ func makeGcsfuseArgs( opts map[string]string) (args []string, err error) { // Deal with options. for name, value := range opts { - switch strings.Replace(name, "-", "_", -1) { + switch strings.ReplaceAll(name, "-", "_") { // Don't pass through options that are relevant to mount(8) but not to // gcsfuse, and that fusermount chokes on with "Invalid argument" on Linux. case "user", "nouser", "auto", "noauto", "_netdev", "no_netdev": @@ -93,7 +93,7 @@ func makeGcsfuseArgs( if value == "" { value = "true" } - args = append(args, "--"+strings.Replace(name, "_", "-", -1)+"="+value) + args = append(args, "--"+strings.ReplaceAll(name, "_", "-")+"="+value) // Special case: support mount-like formatting for gcsfuse string flags. case "dir_mode", @@ -137,7 +137,7 @@ func makeGcsfuseArgs( "file_cache_max_size_mb", "grpc_conn_pool_size": - args = append(args, "--"+strings.Replace(name, "_", "-", -1), value) + args = append(args, "--"+strings.ReplaceAll(name, "_", "-"), value) // Special case: support mount-like formatting for gcsfuse debug flags. case "debug_fuse", diff --git a/tools/mount_gcsfuse/main_test.go b/tools/mount_gcsfuse/main_test.go index 062d3a73fd..4c360e297f 100644 --- a/tools/mount_gcsfuse/main_test.go +++ b/tools/mount_gcsfuse/main_test.go @@ -22,24 +22,18 @@ import ( func TestMakeGcsfuseArgs(t *testing.T) { testCases := []struct { - name string - device string - mountPoint string - opts map[string]string - expected []string + name string + opts map[string]string + expectedFlags []string }{ { - name: "TestMakeGcsfuseArgs with NoOptions", - device: "gcsfuse", - mountPoint: "/mnt/gcs", - opts: map[string]string{}, - expected: []string{"gcsfuse", "/mnt/gcs"}, + name: "TestMakeGcsfuseArgs with NoOptions", + opts: map[string]string{}, + expectedFlags: []string{}, }, { - name: "TestMakeGcsfuseArgs for BooleanFlags with underscore", - device: "gcsfuse", - mountPoint: "/mnt/gcs", + name: "TestMakeGcsfuseArgs for BooleanFlags with underscore", opts: map[string]string{"implicit_dirs": "", "foreground": "true", "experimental_local_file_cache": "", @@ -50,7 +44,7 @@ func TestMakeGcsfuseArgs(t *testing.T) { "ignore_interrupts": "", "anonymous_access": "false", "log_rotate_compress": "false"}, - expected: []string{"--implicit-dirs=true", + expectedFlags: []string{"--implicit-dirs=true", "--foreground=true", "--experimental-local-file-cache=true", "--reuse-token-from-url=false", @@ -59,14 +53,11 @@ func TestMakeGcsfuseArgs(t *testing.T) { "--enable-hns=true", "--ignore-interrupts=true", "--anonymous-access=false", - "--log-rotate-compress=false", - "gcsfuse", "/mnt/gcs"}, + "--log-rotate-compress=false"}, }, { - name: "TestMakeGcsfuseArgs for BooleanFlags with hyphens", - device: "gcsfuse", - mountPoint: "/mnt/gcs", + name: "TestMakeGcsfuseArgs for BooleanFlags with hyphens", opts: map[string]string{"implicit_dirs": "", "foreground": "true", "experimental-local-file-cache": "", @@ -77,7 +68,7 @@ func TestMakeGcsfuseArgs(t *testing.T) { "ignore-interrupts": "", "anonymous-access": "false", "log_rotate-compress": "false"}, - expected: []string{"--implicit-dirs=true", + expectedFlags: []string{"--implicit-dirs=true", "--foreground=true", "--experimental-local-file-cache=true", "--reuse-token-from-url=false", @@ -86,79 +77,67 @@ func TestMakeGcsfuseArgs(t *testing.T) { "--enable-hns=true", "--ignore-interrupts=true", "--anonymous-access=false", - "--log-rotate-compress=false", - "gcsfuse", "/mnt/gcs"}, + "--log-rotate-compress=false"}, }, { - name: "TestMakeGcsfuseArgs for StringFlags with underscore", - device: "gcsfuse", - mountPoint: "/mnt/gcs", + name: "TestMakeGcsfuseArgs for StringFlags with underscore", opts: map[string]string{ "dir_mode": "0755", "key_file": "/path/to/key", "log_rotate_backup_file_count": "2", }, - expected: []string{"--dir-mode", "0755", "--key-file", "/path/to/key", "--log-rotate-backup-file-count", "2", "gcsfuse", "/mnt/gcs"}, + expectedFlags: []string{"--dir-mode", "0755", "--key-file", "/path/to/key", "--log-rotate-backup-file-count", "2"}, }, { - name: "TestMakeGcsfuseArgs for StringFlags with hyphen", - device: "gcsfuse", - mountPoint: "/mnt/gcs", + name: "TestMakeGcsfuseArgs for StringFlags with hyphen", opts: map[string]string{ "dir-mode": "0755", "key-file": "/path/to/key", "log-rotate-backup-file-count": "2", }, - expected: []string{"--dir-mode", "0755", "--key-file", "/path/to/key", "--log-rotate-backup-file-count", "2", "gcsfuse", "/mnt/gcs"}, + expectedFlags: []string{"--dir-mode", "0755", "--key-file", "/path/to/key", "--log-rotate-backup-file-count", "2"}, }, { - name: "TestMakeGcsfuseArgs with DebugFlags", - device: "gcsfuse", - mountPoint: "/mnt/gcs", - opts: map[string]string{"debug_fuse": "", "debug_gcs": ""}, - expected: []string{"--debug_fuse", "--debug_gcs", "gcsfuse", "/mnt/gcs"}, + name: "TestMakeGcsfuseArgs with DebugFlags", + opts: map[string]string{"debug_fuse": "", "debug_gcs": ""}, + expectedFlags: []string{"--debug_fuse", "--debug_gcs"}, }, // Test ignored options { - name: "TestMakeGcsfuseArgs with IgnoredOptions", - device: "gcsfuse", - mountPoint: "/mnt/gcs", - opts: map[string]string{"user": "nobody", "_netdev": ""}, - expected: []string{"gcsfuse", "/mnt/gcs"}, + name: "TestMakeGcsfuseArgs with IgnoredOptions", + opts: map[string]string{"user": "nobody", "_netdev": ""}, + expectedFlags: []string{}, }, { - name: "TestMakeGcsfuseArgs with RegularOptions", - device: "gcsfuse", - mountPoint: "/mnt/gcs", - opts: map[string]string{"allow_other": "", "ro": ""}, - expected: []string{"-o", "allow_other", "-o", "ro", "gcsfuse", "/mnt/gcs"}, + name: "TestMakeGcsfuseArgs with RegularOptions", + opts: map[string]string{"allow_other": "", "ro": ""}, + expectedFlags: []string{"-o", "allow_other", "-o", "ro"}, }, { - name: "TestMakeGcsfuseArgs with MixedOptions", - device: "gcsfuse", - mountPoint: "/mnt/gcs", + name: "TestMakeGcsfuseArgs with MixedOptions", opts: map[string]string{ "implicit_dirs": "", "file_mode": "0644", "debug_fuse": "", "allow_other": "", }, - expected: []string{"--implicit-dirs=true", "--file-mode", "0644", "--debug_fuse", "-o", "allow_other", "gcsfuse", "/mnt/gcs"}, + expectedFlags: []string{"--implicit-dirs=true", "--file-mode", "0644", "--debug_fuse", "-o", "allow_other"}, }, } + device := "gcsfuse" + mountPoint := "/mnt/gcs" for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - args, err := makeGcsfuseArgs(tc.device, tc.mountPoint, tc.opts) - assert.Nil(t, err) + args, err := makeGcsfuseArgs(device, mountPoint, tc.opts) - // Assert that all flags are present (no matter the order). - assert.ElementsMatch(t, args[:len(args)-2], tc.expected[:len(tc.expected)-2]) - // Assert that device and mount-point are present at correct position. - assert.Equal(t, args[len(args)-2:], tc.expected[len(tc.expected)-2:]) + if assert.Nil(t, err) { + assert.ElementsMatch(t, args[:len(args)-2], tc.expectedFlags) + assert.Equal(t, args[len(args)-2:], []string{device, mountPoint}) + } }) } } From 971da8f26671126edb6038b6896e5c8ed1f96e70 Mon Sep 17 00:00:00 2001 From: Kislay Kishore Date: Thu, 13 Jun 2024 11:50:21 +0530 Subject: [PATCH 10/10] Prefix grpc_conn_pool_size with "experimental_" --- tools/mount_gcsfuse/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/mount_gcsfuse/main.go b/tools/mount_gcsfuse/main.go index 9a34c28b93..b41699a924 100644 --- a/tools/mount_gcsfuse/main.go +++ b/tools/mount_gcsfuse/main.go @@ -135,7 +135,7 @@ func makeGcsfuseArgs( "log_rotate_max_file_size_mb", "log_rotate_backup_file_count", "file_cache_max_size_mb", - "grpc_conn_pool_size": + "experimental_grpc_conn_pool_size": args = append(args, "--"+strings.ReplaceAll(name, "_", "-"), value)