From 1124764cfd34631df0ea7bb2a19a4d9f84223e17 Mon Sep 17 00:00:00 2001 From: Yuvraj Date: Thu, 12 Aug 2021 09:44:06 +0530 Subject: [PATCH 1/3] Generated pflag for stow config Signed-off-by: Yuvraj --- storage/config.go | 4 +- storage/config_flags.go | 13 +++- storage/config_flags_test.go | 124 +++++++---------------------------- 3 files changed, 38 insertions(+), 103 deletions(-) diff --git a/storage/config.go b/storage/config.go index a44c150a..e1ee2c98 100644 --- a/storage/config.go +++ b/storage/config.go @@ -79,8 +79,8 @@ type ConnectionConfig struct { } type StowConfig struct { - Kind string `json:"kind,omitempty" pflag:"-,Kind of Stow backend to use. Refer to github/graymeta/stow"` - Config map[string]string `json:"config,omitempty" pflag:"-,Configuration for stow backend. Refer to github/graymeta/stow"` + Kind string `json:"kind,omitempty" pflag:",Kind of Stow backend to use. Refer to github/graymeta/stow"` + Config map[string]string `json:"config,omitempty" pflag:",Configuration for stow backend. Refer to github/graymeta/stow"` } type CachingConfig struct { diff --git a/storage/config_flags.go b/storage/config_flags.go index 42cdb7bc..c9c2eb9a 100755 --- a/storage/config_flags.go +++ b/storage/config_flags.go @@ -28,6 +28,15 @@ func (Config) elemValueOrNil(v interface{}) interface{} { return v } +func (Config) mustJsonMarshal(v interface{}) string { + raw, err := json.Marshal(v) + if err != nil { + panic(err) + } + + return string(raw) +} + func (Config) mustMarshalJSON(v json.Marshaler) string { raw, err := v.MarshalJSON() if err != nil { @@ -48,7 +57,9 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "connection.secret-key"), defaultConfig.Connection.SecretKey, "Secret to use when accesskey is set.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "connection.region"), defaultConfig.Connection.Region, "Region to connect to.") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "connection.disable-ssl"), defaultConfig.Connection.DisableSSL, "Disables SSL connection. Should only be used for development.") - cmdFlags.String(fmt.Sprintf("%v%v", prefix, "container"), defaultConfig.InitContainer, "Initial container to create -if it doesn't exist-.'") + cmdFlags.String(fmt.Sprintf("%v%v", prefix, "stow.kind"), defaultConfig.Stow.Kind, "Kind of Stow backend to use. Refer to github/graymeta/stow") + cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "stow.config"), defaultConfig.Stow.Config, "Configuration for stow backend. Refer to github/graymeta/stow") + cmdFlags.String(fmt.Sprintf("%v%v", prefix, "container"), defaultConfig.InitContainer, "Initial container (in s3 a bucket) to create -if it doesn't exist-.'") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "enable-multicontainer"), defaultConfig.MultiContainerEnabled, "If this is true, then the container argument is overlooked and redundant. This config will automatically open new connections to new containers/buckets as they are encountered") cmdFlags.Int(fmt.Sprintf("%v%v", prefix, "cache.max_size_mbs"), defaultConfig.Cache.MaxSizeMegabytes, "Maximum size of the cache where the Blob store data is cached in-memory. If not specified or set to 0, cache is not used") cmdFlags.Int(fmt.Sprintf("%v%v", prefix, "cache.target_gc_percent"), defaultConfig.Cache.TargetGCPercent, "Sets the garbage collection target percentage.") diff --git a/storage/config_flags_test.go b/storage/config_flags_test.go index 61a7bbdb..c71a3e8f 100755 --- a/storage/config_flags_test.go +++ b/storage/config_flags_test.go @@ -84,7 +84,7 @@ func testDecodeJson_Config(t *testing.T, val, result interface{}) { assert.NoError(t, decode_Config(val, result)) } -func testDecodeSlice_Config(t *testing.T, vStringSlice, result interface{}) { +func testDecodeRaw_Config(t *testing.T, vStringSlice, result interface{}) { assert.NoError(t, decode_Config(vStringSlice, result)) } @@ -100,14 +100,6 @@ func TestConfig_SetFlags(t *testing.T) { assert.True(t, cmdFlags.HasFlags()) t.Run("Test_type", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vString, err := cmdFlags.GetString("type"); err == nil { - assert.Equal(t, string(defaultConfig.Type), vString) - } else { - assert.FailNow(t, err.Error()) - } - }) t.Run("Override", func(t *testing.T) { testValue := "1" @@ -122,14 +114,6 @@ func TestConfig_SetFlags(t *testing.T) { }) }) t.Run("Test_connection.endpoint", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vString, err := cmdFlags.GetString("connection.endpoint"); err == nil { - assert.Equal(t, string(defaultConfig.Connection.Endpoint.String()), vString) - } else { - assert.FailNow(t, err.Error()) - } - }) t.Run("Override", func(t *testing.T) { testValue := defaultConfig.Connection.Endpoint.String() @@ -144,14 +128,6 @@ func TestConfig_SetFlags(t *testing.T) { }) }) t.Run("Test_connection.auth-type", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vString, err := cmdFlags.GetString("connection.auth-type"); err == nil { - assert.Equal(t, string(defaultConfig.Connection.AuthType), vString) - } else { - assert.FailNow(t, err.Error()) - } - }) t.Run("Override", func(t *testing.T) { testValue := "1" @@ -166,14 +142,6 @@ func TestConfig_SetFlags(t *testing.T) { }) }) t.Run("Test_connection.access-key", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vString, err := cmdFlags.GetString("connection.access-key"); err == nil { - assert.Equal(t, string(defaultConfig.Connection.AccessKey), vString) - } else { - assert.FailNow(t, err.Error()) - } - }) t.Run("Override", func(t *testing.T) { testValue := "1" @@ -188,14 +156,6 @@ func TestConfig_SetFlags(t *testing.T) { }) }) t.Run("Test_connection.secret-key", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vString, err := cmdFlags.GetString("connection.secret-key"); err == nil { - assert.Equal(t, string(defaultConfig.Connection.SecretKey), vString) - } else { - assert.FailNow(t, err.Error()) - } - }) t.Run("Override", func(t *testing.T) { testValue := "1" @@ -210,14 +170,6 @@ func TestConfig_SetFlags(t *testing.T) { }) }) t.Run("Test_connection.region", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vString, err := cmdFlags.GetString("connection.region"); err == nil { - assert.Equal(t, string(defaultConfig.Connection.Region), vString) - } else { - assert.FailNow(t, err.Error()) - } - }) t.Run("Override", func(t *testing.T) { testValue := "1" @@ -232,36 +184,48 @@ func TestConfig_SetFlags(t *testing.T) { }) }) t.Run("Test_connection.disable-ssl", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly + + t.Run("Override", func(t *testing.T) { + testValue := "1" + + cmdFlags.Set("connection.disable-ssl", testValue) if vBool, err := cmdFlags.GetBool("connection.disable-ssl"); err == nil { - assert.Equal(t, bool(defaultConfig.Connection.DisableSSL), vBool) + testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.Connection.DisableSSL) + } else { assert.FailNow(t, err.Error()) } }) + }) + t.Run("Test_stow.kind", func(t *testing.T) { t.Run("Override", func(t *testing.T) { testValue := "1" - cmdFlags.Set("connection.disable-ssl", testValue) - if vBool, err := cmdFlags.GetBool("connection.disable-ssl"); err == nil { - testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.Connection.DisableSSL) + cmdFlags.Set("stow.kind", testValue) + if vString, err := cmdFlags.GetString("stow.kind"); err == nil { + testDecodeJson_Config(t, fmt.Sprintf("%v", vString), &actual.Stow.Kind) } else { assert.FailNow(t, err.Error()) } }) }) - t.Run("Test_container", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vString, err := cmdFlags.GetString("container"); err == nil { - assert.Equal(t, string(defaultConfig.InitContainer), vString) + t.Run("Test_stow.config", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "a=1,b=2" + + cmdFlags.Set("stow.config", testValue) + if vStringToString, err := cmdFlags.GetStringToString("stow.config"); err == nil { + testDecodeRaw_Config(t, vStringToString, &actual.Stow.Config) + } else { assert.FailNow(t, err.Error()) } }) + }) + t.Run("Test_container", func(t *testing.T) { t.Run("Override", func(t *testing.T) { testValue := "1" @@ -276,14 +240,6 @@ func TestConfig_SetFlags(t *testing.T) { }) }) t.Run("Test_enable-multicontainer", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vBool, err := cmdFlags.GetBool("enable-multicontainer"); err == nil { - assert.Equal(t, bool(defaultConfig.MultiContainerEnabled), vBool) - } else { - assert.FailNow(t, err.Error()) - } - }) t.Run("Override", func(t *testing.T) { testValue := "1" @@ -298,14 +254,6 @@ func TestConfig_SetFlags(t *testing.T) { }) }) t.Run("Test_cache.max_size_mbs", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vInt, err := cmdFlags.GetInt("cache.max_size_mbs"); err == nil { - assert.Equal(t, int(defaultConfig.Cache.MaxSizeMegabytes), vInt) - } else { - assert.FailNow(t, err.Error()) - } - }) t.Run("Override", func(t *testing.T) { testValue := "1" @@ -320,14 +268,6 @@ func TestConfig_SetFlags(t *testing.T) { }) }) t.Run("Test_cache.target_gc_percent", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vInt, err := cmdFlags.GetInt("cache.target_gc_percent"); err == nil { - assert.Equal(t, int(defaultConfig.Cache.TargetGCPercent), vInt) - } else { - assert.FailNow(t, err.Error()) - } - }) t.Run("Override", func(t *testing.T) { testValue := "1" @@ -342,14 +282,6 @@ func TestConfig_SetFlags(t *testing.T) { }) }) t.Run("Test_limits.maxDownloadMBs", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vInt64, err := cmdFlags.GetInt64("limits.maxDownloadMBs"); err == nil { - assert.Equal(t, int64(defaultConfig.Limits.GetLimitMegabytes), vInt64) - } else { - assert.FailNow(t, err.Error()) - } - }) t.Run("Override", func(t *testing.T) { testValue := "1" @@ -364,14 +296,6 @@ func TestConfig_SetFlags(t *testing.T) { }) }) t.Run("Test_defaultHttpClient.timeout", func(t *testing.T) { - t.Run("DefaultValue", func(t *testing.T) { - // Test that default value is set properly - if vString, err := cmdFlags.GetString("defaultHttpClient.timeout"); err == nil { - assert.Equal(t, string(defaultConfig.DefaultHTTPClient.Timeout.String()), vString) - } else { - assert.FailNow(t, err.Error()) - } - }) t.Run("Override", func(t *testing.T) { testValue := defaultConfig.DefaultHTTPClient.Timeout.String() From d005197d5121409776914d88bd6f421102584983 Mon Sep 17 00:00:00 2001 From: Yuvraj Date: Thu, 12 Aug 2021 11:04:17 +0530 Subject: [PATCH 2/3] more changes Signed-off-by: Yuvraj --- storage/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/config.go b/storage/config.go index e1ee2c98..1e62c171 100644 --- a/storage/config.go +++ b/storage/config.go @@ -47,7 +47,7 @@ var ( type Config struct { Type Type `json:"type" pflag:",Sets the type of storage to configure [s3/minio/local/mem/stow]."` Connection ConnectionConfig `json:"connection"` - Stow *StowConfig `json:"stow,omitempty"` + Stow *StowConfig `json:"stow,omitempty" pflag:",Storage config for stow backend."` // Container here is misleading, it refers to a Bucket (AWS S3) like blobstore entity. In some terms it could be a table InitContainer string `json:"container" pflag:",Initial container (in s3 a bucket) to create -if it doesn't exist-.'"` // By default if this is not enabled, multiple containers are not supported by the storage layer. Only the configured `container` InitContainer will be allowed to requests data from. But, if enabled then data will be loaded to written to any From 3ac2955551a360b85eea601902caff5cda0ae0e4 Mon Sep 17 00:00:00 2001 From: Yuvraj Date: Fri, 13 Aug 2021 08:23:48 +0530 Subject: [PATCH 3/3] more changes Signed-off-by: Yuvraj --- cli/pflags/api/testdata/testtype.go | 2 ++ cli/pflags/api/testdata/testtype_test.go | 28 ++++++++++++++++++++++++ storage/config.go | 2 +- storage/stow_store.go | 3 ++- storage/stow_store_test.go | 10 ++++----- 5 files changed, 38 insertions(+), 7 deletions(-) diff --git a/cli/pflags/api/testdata/testtype.go b/cli/pflags/api/testdata/testtype.go index 045a9c2c..ca352843 100755 --- a/cli/pflags/api/testdata/testtype.go +++ b/cli/pflags/api/testdata/testtype.go @@ -64,6 +64,8 @@ func (cfg TestType) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "storage.connection.secret-key"), DefaultTestType.StorageConfig.Connection.SecretKey, "Secret to use when accesskey is set.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "storage.connection.region"), DefaultTestType.StorageConfig.Connection.Region, "Region to connect to.") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "storage.connection.disable-ssl"), DefaultTestType.StorageConfig.Connection.DisableSSL, "Disables SSL connection. Should only be used for development.") + cmdFlags.String(fmt.Sprintf("%v%v", prefix, "storage.stow.kind"), DefaultTestType.StorageConfig.Stow.Kind, "Kind of Stow backend to use. Refer to github/graymeta/stow") + cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "storage.stow.config"), DefaultTestType.StorageConfig.Stow.Config, "Configuration for stow backend. Refer to github/graymeta/stow") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "storage.container"), DefaultTestType.StorageConfig.InitContainer, "Initial container (in s3 a bucket) to create -if it doesn't exist-.'") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "storage.enable-multicontainer"), DefaultTestType.StorageConfig.MultiContainerEnabled, "If this is true, then the container argument is overlooked and redundant. This config will automatically open new connections to new containers/buckets as they are encountered") cmdFlags.Int(fmt.Sprintf("%v%v", prefix, "storage.cache.max_size_mbs"), DefaultTestType.StorageConfig.Cache.MaxSizeMegabytes, "Maximum size of the cache where the Blob store data is cached in-memory. If not specified or set to 0, cache is not used") diff --git a/cli/pflags/api/testdata/testtype_test.go b/cli/pflags/api/testdata/testtype_test.go index cb35842e..37a97618 100755 --- a/cli/pflags/api/testdata/testtype_test.go +++ b/cli/pflags/api/testdata/testtype_test.go @@ -295,6 +295,34 @@ func TestTestType_SetFlags(t *testing.T) { } }) }) + t.Run("Test_storage.stow.kind", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "1" + + cmdFlags.Set("storage.stow.kind", testValue) + if vString, err := cmdFlags.GetString("storage.stow.kind"); err == nil { + testDecodeJson_TestType(t, fmt.Sprintf("%v", vString), &actual.StorageConfig.Stow.Kind) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) + t.Run("Test_storage.stow.config", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "a=1,b=2" + + cmdFlags.Set("storage.stow.config", testValue) + if vStringToString, err := cmdFlags.GetStringToString("storage.stow.config"); err == nil { + testDecodeRaw_TestType(t, vStringToString, &actual.StorageConfig.Stow.Config) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) t.Run("Test_storage.container", func(t *testing.T) { t.Run("Override", func(t *testing.T) { diff --git a/storage/config.go b/storage/config.go index 1e62c171..bbafd36d 100644 --- a/storage/config.go +++ b/storage/config.go @@ -47,7 +47,7 @@ var ( type Config struct { Type Type `json:"type" pflag:",Sets the type of storage to configure [s3/minio/local/mem/stow]."` Connection ConnectionConfig `json:"connection"` - Stow *StowConfig `json:"stow,omitempty" pflag:",Storage config for stow backend."` + Stow StowConfig `json:"stow,omitempty" pflag:",Storage config for stow backend."` // Container here is misleading, it refers to a Bucket (AWS S3) like blobstore entity. In some terms it could be a table InitContainer string `json:"container" pflag:",Initial container (in s3 a bucket) to create -if it doesn't exist-.'"` // By default if this is not enabled, multiple containers are not supported by the storage layer. Only the configured `container` InitContainer will be allowed to requests data from. But, if enabled then data will be loaded to written to any diff --git a/storage/stow_store.go b/storage/stow_store.go index 18f7fb7c..4e3ad2d1 100644 --- a/storage/stow_store.go +++ b/storage/stow_store.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io" + "reflect" "sync" "time" @@ -319,7 +320,7 @@ func newStowRawStore(cfg *Config, metricsScope promutils.Scope) (RawStore, error var cfgMap stow.ConfigMap var kind string - if cfg.Stow != nil { + if !reflect.DeepEqual(cfg.Stow, StowConfig{}) { kind = cfg.Stow.Kind cfgMap = cfg.Stow.Config } else { diff --git a/storage/stow_store_test.go b/storage/stow_store_test.go index 87025ed7..16fff35c 100644 --- a/storage/stow_store_test.go +++ b/storage/stow_store_test.go @@ -303,7 +303,7 @@ func TestNewLocalStore(t *testing.T) { t.Run("Valid config", func(t *testing.T) { testScope := promutils.NewTestScope() store, err := newStowRawStore(&Config{ - Stow: &StowConfig{ + Stow: StowConfig{ Kind: local.Kind, Config: map[string]string{ local.ConfigKeyPath: "./", @@ -339,7 +339,7 @@ func TestNewLocalStore(t *testing.T) { assert.NotNil(t, stats) store, err := newStowRawStore(&Config{ - Stow: &StowConfig{ + Stow: StowConfig{ Kind: local.Kind, Config: map[string]string{ local.ConfigKeyPath: tmpDir, @@ -368,7 +368,7 @@ func TestNewLocalStore(t *testing.T) { assert.NotNil(t, stats) store, err := newStowRawStore(&Config{ - Stow: &StowConfig{ + Stow: StowConfig{ Kind: local.Kind, Config: map[string]string{ local.ConfigKeyPath: tmpDir, @@ -390,7 +390,7 @@ func TestNewLocalStore(t *testing.T) { assert.NotNil(t, stats) store, err := newStowRawStore(&Config{ - Stow: &StowConfig{ + Stow: StowConfig{ Kind: local.Kind, Config: map[string]string{ local.ConfigKeyPath: tmpDir, @@ -424,7 +424,7 @@ func Test_newStowRawStore(t *testing.T) { {"fail", args{&Config{}, promutils.NewTestScope()}, true}, {"google", args{&Config{ InitContainer: "flyte", - Stow: &StowConfig{ + Stow: StowConfig{ Kind: google.Kind, Config: map[string]string{ google.ConfigProjectId: "x",