Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default enable viper #2109

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from
4 changes: 4 additions & 0 deletions cfg/config_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,7 @@ func OverrideWithLoggingFlags(mountConfig *Config, debugFuse bool,
mountConfig.Logging.Severity = "TRACE"
}
}

func IsFileCacheEnabled(mountConfig *Config) bool {
return mountConfig.FileCache.MaxSizeMb != 0 && string(mountConfig.CacheDir) != ""
}
5 changes: 4 additions & 1 deletion cfg/decode_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ func stringToURLHookFunc() mapstructure.DecodeHookFuncType {
return data, nil
}
s := data.(string)
if s == "" {
return nil, nil
}
u, err := url.Parse(s)
if err != nil {
return nil, err
Expand All @@ -46,8 +49,8 @@ func stringToURLHookFunc() mapstructure.DecodeHookFuncType {
func DecodeHook() mapstructure.DecodeHookFunc {
return mapstructure.ComposeDecodeHookFunc(
mapstructure.TextUnmarshallerHookFunc(),
stringToURLHookFunc(),
mapstructure.StringToTimeDurationHookFunc(), // default hook
mapstructure.StringToSliceHookFunc(","), // default hook
stringToURLHookFunc(),
)
}
7 changes: 7 additions & 0 deletions cfg/decode_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ func TestParsingSuccess(t *testing.T) {
assert.Equal(t, *u, *c.URLParam)
},
},
{
name: "URL2",
args: []string{""},
testFn: func(t *testing.T, c TestConfig) {
assert.Nil(t, c.URLParam)
},
},
{
name: "Bool1",
args: []string{"--boolParam"},
Expand Down
72 changes: 72 additions & 0 deletions cfg/validate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// 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 cfg

import (
"fmt"
"math"

"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
"github.com/googlecloudplatform/gcsfuse/v2/internal/mount"
"github.com/googlecloudplatform/gcsfuse/v2/internal/util"
"github.com/spf13/viper"
)

func Validate(c *Config) error {
if c.MetadataCache.DeprecatedStatCacheCapacity < 0 {
return fmt.Errorf("invalid value of stat-cache-capacity (%v), can't be less than 0", c.MetadataCache.DeprecatedStatCacheCapacity)
}
return nil
}

func VetConfig(v *viper.Viper, c *Config) {
// The EnableEmptyManagedFolders flag must be set to true to enforce folder prefixes for Hierarchical buckets.
if c.EnableHns {
c.List.EnableEmptyManagedFolders = true
}
// Handle metadatacache ttl
resolveMetadataCacheTTL(v, c)
resolveMetadataCacheCapacity(v, c)
}

// RresolveMetadataCacheTTL returns the ttl to be used for stat/type cache based on the user flags/configs.
func resolveMetadataCacheTTL(v *viper.Viper, config *Config) {
// If metadata-cache:ttl-secs has been set in config-file, then
// it overrides both stat-cache-ttl and type-cache-tll.
if v.IsSet("metadata-cache.ttl-secs") {
// if ttl-secs is set to -1, set StatOrTypeCacheTTL to the max possible duration.
if config.MetadataCache.TtlSecs == -1 {
config.MetadataCache.TtlSecs = math.MaxInt64
}
return
}
config.MetadataCache.TtlSecs = int64(math.Ceil(math.Min(config.MetadataCache.DeprecatedStatCacheTtl.Seconds(),
config.MetadataCache.DeprecatedTypeCacheTtl.Seconds())))
}

func resolveMetadataCacheCapacity(v *viper.Viper, c *Config) {
if v.IsSet("metadata-cache.stat-cache-max-size-mb") {
if c.MetadataCache.StatCacheMaxSizeMb != -1 {
return
}
c.MetadataCache.StatCacheMaxSizeMb = int64(config.MaxSupportedStatCacheMaxSizeMB)
return
}
if v.IsSet("metadata-cache.deprecated-stat-cache-capacity") {
avgTotalStatCacheEntrySize := mount.AverageSizeOfPositiveStatCacheEntry + mount.AverageSizeOfNegativeStatCacheEntry
c.MetadataCache.StatCacheMaxSizeMb = int64(util.BytesToHigherMiBs(
uint64(c.MetadataCache.DeprecatedStatCacheCapacity) * avgTotalStatCacheEntrySize))
}
}
222 changes: 222 additions & 0 deletions cmd/config_vetting_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
// 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 cmd

import (
"fmt"
"math"
"testing"

"github.com/googlecloudplatform/gcsfuse/v2/cfg"
"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func getConfigObject(t *testing.T, args []string) (*cfg.Config, error) {
t.Helper()
var c cfg.Config
cmd, err := NewRootCmd(func(config cfg.Config, _, _ string) error {
c = config
return nil
})
require.Nil(t, err)
cmdArgs := append([]string{"gcsfuse"}, args...)
cmdArgs = append(cmdArgs, "a")
cmd.SetArgs(cmdArgs)
if err = cmd.Execute(); err != nil {
return nil, err
}

return &c, nil
}

func TestMetadataCacheTTLResolution(t *testing.T) {
testcases := []struct {
name string
args []string
expectedTTLSecs int64
}{
{
name: "Most common scenario, when user doesn't set any of the TTL config parameters.",
args: []string{},
expectedTTLSecs: 60,
},
{
name: "user sets only metadata-cache:ttl-secs and sets it to -1",
args: []string{"--metadata-cache-ttl=-1"},
expectedTTLSecs: math.MaxInt64,
},
{
name: "user sets only metadata-cache:ttl-secs and sets it to 0.",
args: []string{"--metadata-cache-ttl=0"},
expectedTTLSecs: 0,
},
{
name: "user sets only metadata-cache:ttl-secs and sets it to a positive value.",
args: []string{"--metadata-cache-ttl=30"},
expectedTTLSecs: 30,
},
{
name: "user sets only metadata-cache:ttl-secs and sets it to its highest supported value.",
args: []string{fmt.Sprintf("--metadata-cache-ttl=%d", config.MaxSupportedTtlInSeconds)},
expectedTTLSecs: config.MaxSupportedTtlInSeconds,
},
{
name: "user sets both the old flags and the metadata-cache:ttl-secs. Here ttl-secs overrides both flags.",
args: []string{"--stat-cache-ttl=5m", "--type-cache-ttl=1h", "--metadata-cache-ttl=10800"},
expectedTTLSecs: 10800,
},
{
name: "user sets only stat/type-cache-ttl flag(s), and not metadata-cache:ttl-secs.",
args: []string{"--stat-cache-ttl=0s", "--type-cache-ttl=0s"},
expectedTTLSecs: 0,
},
{
name: "stat-cache enabled, but not type-cache.",
args: []string{"--stat-cache-ttl=1h", "--type-cache-ttl=0s"},
expectedTTLSecs: 0,
},
{
name: "type-cache enabled, but not stat-cache.",
args: []string{"--type-cache-ttl=1h", "--stat-cache-ttl=0s"},
expectedTTLSecs: 0,
},
{
name: "both type-cache and stat-cache enabled.",
args: []string{"--type-cache-ttl=1h", "--stat-cache-ttl=30s"},
expectedTTLSecs: 30,
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
c, err := getConfigObject(t, tc.args)

if assert.Nil(t, err) {
c.MetadataCache.TtlSecs = tc.expectedTTLSecs
}
})
}
}

func TestEnableEmptyManagedFoldersResolution(t *testing.T) {
testcases := []struct {
name string
args []string
expected bool
}{
{
name: "enable-hns set to true",
args: []string{"--enable-hns"},
expected: true,
},
{
name: "enable-hns set to true but enable-empty-managed-folders set to false",
args: []string{"--enable-hns", "--enable-empty-managed-folders=false"},
expected: true,
},
{
name: "enable-hns not true but enable-empty-managed-folders set to true",
args: []string{"--enable-hns=false", "--enable-empty-managed-folders=true"},
expected: true,
},
{
name: "both enable-hns and enable-empty-managed-folders not true",
args: []string{"--enable-hns=false", "--enable-empty-managed-folders=false"},
expected: false,
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
c, err := getConfigObject(t, tc.args)

if assert.Nil(t, err) {
c.List.EnableEmptyManagedFolders = tc.expected
}
})
}
}

func TestStatCacheCapacityResolution(t *testing.T) {
testcases := []struct {
name string
args []string
expected int64
}{
{
name: "Default behavior",
args: []string{},
expected: 32,
},
{
name: "stat-cache-max-size-mb specified as -1",
args: []string{"--stat-cache-max-size-mb=-1"},
expected: int64(config.MaxSupportedStatCacheMaxSizeMB),
},
{
name: "stat-cache-max-size-mb set to 0",
args: []string{"--stat-cache-max-size-mb=0"},
expected: 0,
},
{
name: "--stat-cache-max-size-mb set to a positive value",
args: []string{"--stat-cache-max-size-mb=100"},
expected: 100,
},
{
name: "highest possible value",
args: []string{fmt.Sprintf("--stat-cache-max-size-mb=%d", config.MaxSupportedStatCacheMaxSizeMB)},
expected: int64(config.MaxSupportedStatCacheMaxSizeMB),
},
{
name: "highest possible value of stat-cache-max-size-mb",
args: []string{fmt.Sprintf("--stat-cache-max-size-mb=%d", config.MaxSupportedStatCacheMaxSizeMB)},
expected: int64(config.MaxSupportedStatCacheMaxSizeMB),
},
{
name: "both stat-cache-max-size-mb and stat-cache-capacity set",
args: []string{"--stat-cache-max-size-mb=100", "--stat-cache-capacity=10000"},
expected: 100,
},
{
name: "both stat-cache-max-size-mb and stat-cache-capacity set, stat-cache-capacity set to 0",
args: []string{"--stat-cache-max-size-mb=100", "--stat-cache-capacity=0"},
expected: 100,
},
{
name: "both stat-cache-max-size-mb and stat-cache-capacity set, stat-cache-max-size-mb set to 0",
args: []string{"--stat-cache-max-size-mb=0", "--stat-cache-capacity=10000"},
expected: 0,
},
{
name: "stat-cache-capacity set to 0",
args: []string{"--stat-cache-capacity=0"},
expected: 0,
},
{
name: "stat-cache-capacity set",
args: []string{"--stat-cache-capacity=10000"},
expected: 16,
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
c, err := getConfigObject(t, tc.args)
if assert.Nil(t, err) {
assert.Equal(t, tc.expected, c.MetadataCache.StatCacheMaxSizeMb)
}
})
}
}
11 changes: 3 additions & 8 deletions cmd/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,8 @@ type flagStorage struct {
ConfigFile string

// File system
MountOptions map[string]string
// Deprecated: Use the param from cfg/config.go
MountOptions []string

// Deprecated: Use the param from cfg/config.go
DirMode os.FileMode
Expand Down Expand Up @@ -592,7 +593,7 @@ func populateFlags(c *cli.Context) (flags *flagStorage, err error) {
ConfigFile: c.String("config-file"),

// File system
MountOptions: make(map[string]string),
MountOptions: c.StringSlice("o"),
DirMode: os.FileMode(*c.Generic("dir-mode").(*OctalInt)),
FileMode: os.FileMode(*c.Generic("file-mode").(*OctalInt)),
Uid: int64(c.Int("uid")),
Expand Down Expand Up @@ -643,12 +644,6 @@ func populateFlags(c *cli.Context) (flags *flagStorage, err error) {
// Post-mount actions
ExperimentalMetadataPrefetchOnMount: c.String(ExperimentalMetadataPrefetchOnMountFlag),
}

// Handle the repeated "-o" flag.
for _, o := range c.StringSlice("o") {
mountpkg.ParseOptions(flags.MountOptions, o)
}

err = validateFlags(flags)

return
Expand Down
14 changes: 9 additions & 5 deletions cmd/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,18 +244,22 @@ func (t *FlagsTest) Maps() {
f := parseArgs(t, args)

var keys sort.StringSlice
for k := range f.MountOptions {
parsedOptions := make(map[string]string)
for _, o := range f.MountOptions {
mount.ParseOptions(parsedOptions, o)
}
for k := range parsedOptions {
keys = append(keys, k)
}

sort.Sort(keys)
assert.ElementsMatch(t.T(),
keys, []string{"noauto", "nodev", "rw", "user"})

assert.Equal(t.T(), "", f.MountOptions["noauto"])
assert.Equal(t.T(), "", f.MountOptions["nodev"])
assert.Equal(t.T(), "", f.MountOptions["rw"])
assert.Equal(t.T(), "jacobsa", f.MountOptions["user"])
assert.Equal(t.T(), "", parsedOptions["noauto"])
assert.Equal(t.T(), "", parsedOptions["nodev"])
assert.Equal(t.T(), "", parsedOptions["rw"])
assert.Equal(t.T(), "jacobsa", parsedOptions["user"])
}

func (t *FlagsTest) TestResolvePathForTheFlagInContext() {
Expand Down
Loading
Loading