Skip to content

Commit

Permalink
migrate write config to new config (#2258)
Browse files Browse the repository at this point in the history
  • Loading branch information
ashmeenkaur authored Aug 2, 2024
1 parent d618fa8 commit 82b77ed
Show file tree
Hide file tree
Showing 14 changed files with 98 additions and 22 deletions.
3 changes: 1 addition & 2 deletions cfg/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package cfg
import (
"time"

"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
"github.com/spf13/pflag"
"github.com/spf13/viper"
)
Expand Down Expand Up @@ -571,7 +570,7 @@ func BindFlags(v *viper.Viper, flagSet *pflag.FlagSet) error {
return err
}

flagSet.IntP("max-parallel-downloads", "", config.DefaultMaxParallelDownloads(), "Sets an uber limit of number of concurrent file download requests that are made across all files.")
flagSet.IntP("max-parallel-downloads", "", DefaultMaxParallelDownloads(), "Sets an uber limit of number of concurrent file download requests that are made across all files.")

err = v.BindPFlag("file-cache.max-parallel-downloads", flagSet.Lookup("max-parallel-downloads"))
if err != nil {
Expand Down
6 changes: 6 additions & 0 deletions cfg/config_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,16 @@

package cfg

import "runtime"

// OverrideWithLoggingFlags is for backward compatibility with old flags.
func OverrideWithLoggingFlags(mountConfig *Config, debugFuse bool,
debugGCS bool, debugMutex bool) {
if debugFuse || debugGCS || debugMutex {
mountConfig.Logging.Severity = "TRACE"
}
}

func DefaultMaxParallelDownloads() int {
return max(16, 2*runtime.NumCPU())
}
4 changes: 4 additions & 0 deletions cfg/config_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,7 @@ func TestOverrideWithLoggingFlags(t *testing.T) {
})
}
}

func Test_DefaultMaxParallelDownloads(t *testing.T) {
assert.GreaterOrEqual(t, DefaultMaxParallelDownloads(), 16)
}
33 changes: 33 additions & 0 deletions cmd/config_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,36 @@ func TestValidateCliFlag(t *testing.T) {
})
}
}

func TestValidateConfigFile_WriteConfig(t *testing.T) {
testCases := []struct {
name string
configFile string
expectedConfig *cfg.Config
}{
{
name: "Empty config file [default values].",
configFile: "testdata/empty_file.yaml",
expectedConfig: &cfg.Config{
Write: cfg.WriteConfig{CreateEmptyFile: false},
},
},
{
name: "Valid config file.",
configFile: "testdata/valid_config.yaml",
expectedConfig: &cfg.Config{
Write: cfg.WriteConfig{CreateEmptyFile: 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.Write, gotConfig.Write)
}
})
}
}
2 changes: 1 addition & 1 deletion cmd/legacy_param_mapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func TestPopulateConfigFromLegacyFlags(t *testing.T) {
},
mockCLICtx: &mockCLIContext{isFlagSet: map[string]bool{}},
legacyMountConfig: &config.MountConfig{
WriteConfig: config.WriteConfig{
WriteConfig: cfg.WriteConfig{
CreateEmptyFile: true,
},
LogConfig: config.LogConfig{
Expand Down
42 changes: 42 additions & 0 deletions cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,45 @@ func TestArgsParsing_MountOptions(t *testing.T) {
})
}
}

func TestArgsParsing_CreateEmptyFileFlag(t *testing.T) {
tests := []struct {
name string
args []string
expectedCreateEmptyFile bool
}{
{
name: "Test create-empty-file flag true.",
args: []string{"gcsfuse", "--create-empty-file=true", "abc", "pqr"},
expectedCreateEmptyFile: true,
},
{
name: "Test create-empty-file flag false.",
args: []string{"gcsfuse", "--create-empty-file=false", "abc", "pqr"},
expectedCreateEmptyFile: false,
},
{
name: "Test default create-empty-file flag.",
args: []string{"gcsfuse", "abc", "pqr"},
expectedCreateEmptyFile: false,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var createEmptyFile bool
cmd, err := NewRootCmd(func(cfg *cfg.Config, _ string, _ string) error {
createEmptyFile = cfg.Write.CreateEmptyFile
return nil
})
require.Nil(t, err)
cmd.SetArgs(tc.args)

err = cmd.Execute()

if assert.NoError(t, err) {
assert.Equal(t, tc.expectedCreateEmptyFile, createEmptyFile)
}
})
}
}
2 changes: 2 additions & 0 deletions cmd/testdata/valid_config.yaml
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
app-name: hello
write:
create-empty-file: true
5 changes: 0 additions & 5 deletions internal/config/config_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package config
import (
"fmt"
"math"
"runtime"
"time"
)

Expand Down Expand Up @@ -96,7 +95,3 @@ func ListCacheTtlSecsToDuration(secs int64) time.Duration {

return time.Duration(secs * int64(time.Second))
}

func DefaultMaxParallelDownloads() int {
return max(16, 2*runtime.NumCPU())
}
4 changes: 0 additions & 4 deletions internal/config/config_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,3 @@ func Test_ListCacheTtlSecsToDuration_InvalidCall(t *testing.T) {
// Calling with invalid argument to trigger panic.
ListCacheTtlSecsToDuration(-3)
}

func Test_DefaultMaxParallelDownloads(t *testing.T) {
assert.GreaterOrEqual(t, DefaultMaxParallelDownloads(), 16)
}
10 changes: 4 additions & 6 deletions internal/config/mount_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ package config

import (
"math"

"github.com/googlecloudplatform/gcsfuse/v2/cfg"
)

const (
Expand Down Expand Up @@ -67,10 +69,6 @@ const (
DefaultMaxRetryAttempts = int64(0)
)

type WriteConfig struct {
CreateEmptyFile bool `yaml:"create-empty-file"`
}

type LogConfig struct {
Severity string `yaml:"severity"`
Format string `yaml:"format"`
Expand Down Expand Up @@ -147,7 +145,7 @@ type GCSRetries struct {
}

type MountConfig struct {
WriteConfig `yaml:"write"`
cfg.WriteConfig `yaml:"write"`
LogConfig `yaml:"logging"`
FileCacheConfig `yaml:"file-cache"`
CacheDir string `yaml:"cache-dir"`
Expand Down Expand Up @@ -196,7 +194,7 @@ func NewMountConfig() *MountConfig {
MaxSizeMB: DefaultFileCacheMaxSizeMB,
EnableParallelDownloads: DefaultEnableParallelDownloads,
ParallelDownloadsPerFile: DefaultParallelDownloadsPerFile,
MaxParallelDownloads: DefaultMaxParallelDownloads(),
MaxParallelDownloads: cfg.DefaultMaxParallelDownloads(),
DownloadChunkSizeMB: DefaultDownloadChunkSizeMB,
EnableCRC: DefaultEnableCRC,
}
Expand Down
3 changes: 2 additions & 1 deletion internal/fs/local_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"strings"
"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"
Expand Down Expand Up @@ -61,7 +62,7 @@ func init() {
func (t *LocalFileTest) SetUpTestSuite() {
t.serverCfg.ImplicitDirectories = true
t.serverCfg.MountConfig = &config.MountConfig{
WriteConfig: config.WriteConfig{
WriteConfig: cfg.WriteConfig{
CreateEmptyFile: false,
}}
t.fsTest.SetUpTestSuite()
Expand Down
2 changes: 1 addition & 1 deletion tools/config-gen/params.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@
config-path: "file-cache.max-parallel-downloads"
type: "int"
usage: "Sets an uber limit of number of concurrent file download requests that are made across all files."
default: "config.DefaultMaxParallelDownloads()"
default: "DefaultMaxParallelDownloads()"

- flag-name: "download-chunk-size-mb"
config-path: "file-cache.download-chunk-size-mb"
Expand Down
1 change: 0 additions & 1 deletion tools/config-gen/templates/config.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package cfg
import (
"time"

"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
"github.com/spf13/pflag"
"github.com/spf13/viper"
)
Expand Down
3 changes: 2 additions & 1 deletion tools/integration_tests/operations/operations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"time"

"cloud.google.com/go/storage"
"github.com/googlecloudplatform/gcsfuse/v2/cfg"
"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
"github.com/googlecloudplatform/gcsfuse/v2/tools/integration_tests/util/client"
"github.com/googlecloudplatform/gcsfuse/v2/tools/integration_tests/util/creds_tests"
Expand Down Expand Up @@ -98,7 +99,7 @@ func createMountConfigsAndEquivalentFlags() (flags [][]string) {

// Set up config file with create-empty-file: true.
mountConfig1 := config.MountConfig{
WriteConfig: config.WriteConfig{
WriteConfig: cfg.WriteConfig{
CreateEmptyFile: true,
},
LogConfig: config.LogConfig{
Expand Down

0 comments on commit 82b77ed

Please sign in to comment.