From 6a87ccb6cba36c638fe39c21af8283867d0ee3e8 Mon Sep 17 00:00:00 2001 From: Ashmeen Kaur <57195160+ashmeenkaur@users.noreply.github.com> Date: Fri, 16 Aug 2024 15:05:27 +0530 Subject: [PATCH] [CLI Config Parity] Migrate enableHNS and create empty file usage to new config (#2350) * migrate enable hns and create empty file usage * migrate enable hns and create empty file usage to new config --- cmd/config_validation_test.go | 34 +++++++++++++++++++++++++++++++ cmd/root_test.go | 37 ++++++++++++++++++++++++++++++++++ cmd/testdata/valid_config.yaml | 1 + internal/fs/fs.go | 8 ++++---- internal/fs/hns_bucket_test.go | 4 ++-- internal/fs/local_file_test.go | 5 ++--- 6 files changed, 80 insertions(+), 9 deletions(-) diff --git a/cmd/config_validation_test.go b/cmd/config_validation_test.go index f907bf9158..3f51a7b5db 100644 --- a/cmd/config_validation_test.go +++ b/cmd/config_validation_test.go @@ -538,3 +538,37 @@ func TestValidateConfigFile_ListConfigSuccessful(t *testing.T) { }) } } + +func TestValidateConfigFile_EnableHNSConfigSuccessful(t *testing.T) { + testCases := []struct { + name string + configFile string + expectedConfig *cfg.Config + }{ + { + // Test default values. + name: "empty_config_file", + configFile: "testdata/empty_file.yaml", + expectedConfig: &cfg.Config{ + EnableHns: false, + }, + }, + { + name: "valid_config_file", + configFile: "testdata/valid_config.yaml", + expectedConfig: &cfg.Config{ + EnableHns: true, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gotConfig, err := getConfigObjectWithConfigFile(t, tc.configFile) + + if assert.NoError(t, err) { + assert.EqualValues(t, tc.expectedConfig.EnableHns, gotConfig.EnableHns) + } + }) + } +} diff --git a/cmd/root_test.go b/cmd/root_test.go index e6b1f50fe3..9b61a490c9 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -726,3 +726,40 @@ func TestArgsParsing_ListFlags(t *testing.T) { }) } } + +func TestArgsParsing_EnableHNSFlags(t *testing.T) { + tests := []struct { + name string + args []string + expectedEnableHNS bool + }{ + { + name: "normal", + args: []string{"gcsfuse", "--enable-hns", "abc", "pqr"}, + expectedEnableHNS: true, + }, + { + name: "default", + args: []string{"gcsfuse", "abc", "pqr"}, + expectedEnableHNS: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var gotEnableHNS bool + cmd, err := NewRootCmd(func(cfg *cfg.Config, _, _ string) error { + gotEnableHNS = cfg.EnableHns + return nil + }) + require.Nil(t, err) + cmd.SetArgs(tc.args) + + err = cmd.Execute() + + if assert.NoError(t, err) { + assert.Equal(t, tc.expectedEnableHNS, gotEnableHNS) + } + }) + } +} diff --git a/cmd/testdata/valid_config.yaml b/cmd/testdata/valid_config.yaml index cd5ca004da..d97beb3ebf 100644 --- a/cmd/testdata/valid_config.yaml +++ b/cmd/testdata/valid_config.yaml @@ -40,3 +40,4 @@ file-system: temp-dir: ~/temp list: enable-empty-managed-folders: true +enable-hns: true diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 62ae612ee8..04f7edc21b 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -275,7 +275,7 @@ func makeRootForBucket( fs.mtimeClock, fs.cacheClock, fs.mountConfig.MetadataCacheConfig.TypeCacheMaxSizeMB, - fs.mountConfig.EnableHNS, + fs.newConfig.EnableHns, ) } @@ -731,7 +731,7 @@ func (fs *fileSystem) createExplicitDirInode(inodeID fuseops.InodeID, ic inode.C fs.mtimeClock, fs.cacheClock, fs.mountConfig.MetadataCacheConfig.TypeCacheMaxSizeMB, - fs.mountConfig.EnableHNS) + fs.newConfig.EnableHns) return in } @@ -774,7 +774,7 @@ func (fs *fileSystem) mintInode(ic inode.Core) (in inode.Inode) { fs.mtimeClock, fs.cacheClock, fs.mountConfig.MetadataCacheConfig.TypeCacheMaxSizeMB, - fs.mountConfig.EnableHNS, + fs.newConfig.EnableHns, ) case inode.IsSymlink(ic.MinObject): @@ -1709,7 +1709,7 @@ func (fs *fileSystem) CreateFile( } // Create the child. var child inode.Inode - if fs.mountConfig.CreateEmptyFile { + if fs.newConfig.Write.CreateEmptyFile { child, err = fs.createFile(ctx, op.Parent, op.Name, op.Mode) } else { child, err = fs.createLocalFile(op.Parent, op.Name) diff --git a/internal/fs/hns_bucket_test.go b/internal/fs/hns_bucket_test.go index 5582a9440a..4ef5cd999c 100644 --- a/internal/fs/hns_bucket_test.go +++ b/internal/fs/hns_bucket_test.go @@ -19,7 +19,7 @@ import ( "path" "testing" - "github.com/googlecloudplatform/gcsfuse/v2/internal/config" + "github.com/googlecloudplatform/gcsfuse/v2/cfg" "github.com/googlecloudplatform/gcsfuse/v2/internal/storage/gcs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -34,7 +34,7 @@ type HNSBucketTests struct { func TestHNSBucketTests(t *testing.T) { suite.Run(t, new(HNSBucketTests)) } func (t *HNSBucketTests) SetupSuite() { - t.serverCfg.MountConfig = &config.MountConfig{EnableHNS: true} + t.serverCfg.NewConfig = &cfg.Config{EnableHns: true} t.serverCfg.ImplicitDirectories = false bucketType = gcs.Hierarchical t.fsTest.SetUpTestSuite() diff --git a/internal/fs/local_file_test.go b/internal/fs/local_file_test.go index 299aac367a..d5ca49329f 100644 --- a/internal/fs/local_file_test.go +++ b/internal/fs/local_file_test.go @@ -29,7 +29,6 @@ import ( "time" "github.com/googlecloudplatform/gcsfuse/v2/cfg" - "github.com/googlecloudplatform/gcsfuse/v2/internal/config" "github.com/googlecloudplatform/gcsfuse/v2/internal/fs/inode" "github.com/googlecloudplatform/gcsfuse/v2/internal/storage/gcs" "github.com/googlecloudplatform/gcsfuse/v2/internal/storage/storageutil" @@ -61,8 +60,8 @@ func init() { func (t *LocalFileTest) SetUpTestSuite() { t.serverCfg.ImplicitDirectories = true - t.serverCfg.MountConfig = &config.MountConfig{ - WriteConfig: cfg.WriteConfig{ + t.serverCfg.NewConfig = &cfg.Config{ + Write: cfg.WriteConfig{ CreateEmptyFile: false, }} t.fsTest.SetUpTestSuite()