From b4061bdbdad2eb0c5cf72095b78c40d6f0a11969 Mon Sep 17 00:00:00 2001 From: jerry Date: Thu, 14 Feb 2019 15:00:43 +0800 Subject: [PATCH 1/2] optimize configuration feature and remove some un-used code --- cmd/osdsapiserver/osdsapiserver.go | 20 +++-- cmd/osdsdock/osdsdock.go | 22 +++--- cmd/osdslet/osdslet.go | 20 +++-- pkg/utils/config/config.go | 30 +++++--- pkg/utils/config/config_define.go | 1 - pkg/utils/config/config_test.go | 2 +- pkg/utils/config/flag.go | 116 ----------------------------- pkg/utils/config/flag_test.go | 97 ------------------------ pkg/utils/daemon/daemon.go | 6 -- pkg/utils/daemon/daemon_test.go | 4 +- script/devsds/lib/opensds.sh | 3 - 11 files changed, 52 insertions(+), 269 deletions(-) delete mode 100755 pkg/utils/config/flag.go delete mode 100755 pkg/utils/config/flag_test.go diff --git a/cmd/osdsapiserver/osdsapiserver.go b/cmd/osdsapiserver/osdsapiserver.go index 2dcc3eb5b..aa8d1dfbc 100644 --- a/cmd/osdsapiserver/osdsapiserver.go +++ b/cmd/osdsapiserver/osdsapiserver.go @@ -20,26 +20,24 @@ This module implements a entry into the OpenSDS REST service. package main import ( + "flag" + "github.com/opensds/opensds/pkg/api" "github.com/opensds/opensds/pkg/db" . "github.com/opensds/opensds/pkg/utils/config" - "github.com/opensds/opensds/pkg/utils/constants" "github.com/opensds/opensds/pkg/utils/daemon" "github.com/opensds/opensds/pkg/utils/logs" ) func init() { - // Get the default global configuration. - def := GetDefaultConfig() - - // Parse some configuration fields from command line. - flag := &CONF.Flag - flag.StringVar(&CONF.OsdsApiServer.ApiEndpoint, "api-endpoint", def.OsdsApiServer.ApiEndpoint, "Listen endpoint of api-server service") - flag.DurationVar(&CONF.OsdsApiServer.LogFlushFrequency, "log-flush-frequency", def.OsdsApiServer.LogFlushFrequency, "Maximum number of seconds between log flushes") - flag.BoolVar(&CONF.OsdsApiServer.Daemon, "daemon", def.OsdsApiServer.Daemon, "Run app as a daemon with -daemon=true") - // Load global configuration from specified config file. - CONF.Load(constants.OpensdsConfigPath) + CONF.Load() + + // Parse some configuration fields from command line. and it will override the value which is got from config file. + flag.StringVar(&CONF.OsdsApiServer.ApiEndpoint, "api-endpoint", CONF.OsdsApiServer.ApiEndpoint, "Listen endpoint of api-server service") + flag.DurationVar(&CONF.OsdsApiServer.LogFlushFrequency, "log-flush-frequency", CONF.OsdsApiServer.LogFlushFrequency, "Maximum number of seconds between log flushes") + flag.BoolVar(&CONF.OsdsApiServer.Daemon, "daemon", CONF.OsdsApiServer.Daemon, "Run app as a daemon with -daemon=true") + flag.Parse() daemon.CheckAndRunDaemon(CONF.OsdsApiServer.Daemon) } diff --git a/cmd/osdsdock/osdsdock.go b/cmd/osdsdock/osdsdock.go index d6a968458..502a3e319 100755 --- a/cmd/osdsdock/osdsdock.go +++ b/cmd/osdsdock/osdsdock.go @@ -20,27 +20,25 @@ This module implements a entry into the OpenSDS REST service. package main import ( + "flag" + "github.com/opensds/opensds/pkg/db" "github.com/opensds/opensds/pkg/dock" . "github.com/opensds/opensds/pkg/utils/config" - "github.com/opensds/opensds/pkg/utils/constants" "github.com/opensds/opensds/pkg/utils/daemon" "github.com/opensds/opensds/pkg/utils/logs" ) func init() { - // Get the default global configuration. - def := GetDefaultConfig() - - // Parse some configuration fields from command line. - flag := &CONF.Flag - flag.StringVar(&CONF.OsdsDock.ApiEndpoint, "api-endpoint", def.OsdsDock.ApiEndpoint, "Listen endpoint of dock service") - flag.StringVar(&CONF.OsdsDock.DockType, "dock-type", def.OsdsDock.DockType, "Type of dock service") - flag.BoolVar(&CONF.OsdsDock.Daemon, "daemon", def.OsdsDock.Daemon, "Run app as a daemon with -daemon=true") - flag.DurationVar(&CONF.OsdsDock.LogFlushFrequency, "log-flush-frequency", def.OsdsLet.LogFlushFrequency, "Maximum number of seconds between log flushes") - // Load global configuration from specified config file. - CONF.Load(constants.OpensdsConfigPath) + CONF.Load() + + // Parse some configuration fields from command line. and it will override the value which is got from config file. + flag.StringVar(&CONF.OsdsDock.ApiEndpoint, "api-endpoint", CONF.OsdsDock.ApiEndpoint, "Listen endpoint of dock service") + flag.StringVar(&CONF.OsdsDock.DockType, "dock-type", CONF.OsdsDock.DockType, "Type of dock service") + flag.BoolVar(&CONF.OsdsDock.Daemon, "daemon", CONF.OsdsDock.Daemon, "Run app as a daemon with -daemon=true") + flag.DurationVar(&CONF.OsdsDock.LogFlushFrequency, "log-flush-frequency", CONF.OsdsDock.LogFlushFrequency, "Maximum number of seconds between log flushes") + flag.Parse() daemon.CheckAndRunDaemon(CONF.OsdsDock.Daemon) } diff --git a/cmd/osdslet/osdslet.go b/cmd/osdslet/osdslet.go index 701b61584..920f3a3d4 100755 --- a/cmd/osdslet/osdslet.go +++ b/cmd/osdslet/osdslet.go @@ -20,26 +20,24 @@ This module implements a entry into the OpenSDS REST service. package main import ( + "flag" + c "github.com/opensds/opensds/pkg/controller" "github.com/opensds/opensds/pkg/db" . "github.com/opensds/opensds/pkg/utils/config" - "github.com/opensds/opensds/pkg/utils/constants" "github.com/opensds/opensds/pkg/utils/daemon" "github.com/opensds/opensds/pkg/utils/logs" ) func init() { - // Get the default global configuration. - def := GetDefaultConfig() - - // Parse some configuration fields from command line. - flag := &CONF.Flag - flag.StringVar(&CONF.OsdsLet.ApiEndpoint, "api-endpoint", def.OsdsLet.ApiEndpoint, "Listen endpoint of controller service") - flag.BoolVar(&CONF.OsdsLet.Daemon, "daemon", def.OsdsLet.Daemon, "Run app as a daemon with -daemon=true") - flag.DurationVar(&CONF.OsdsLet.LogFlushFrequency, "log-flush-frequency", def.OsdsLet.LogFlushFrequency, "Maximum number of seconds between log flushes") - // Load global configuration from specified config file. - CONF.Load(constants.OpensdsConfigPath) + CONF.Load() + + // Parse some configuration fields from command line. and it will override the value which is got from config file. + flag.StringVar(&CONF.OsdsLet.ApiEndpoint, "api-endpoint", CONF.OsdsLet.ApiEndpoint, "Listen endpoint of controller service") + flag.BoolVar(&CONF.OsdsLet.Daemon, "daemon", CONF.OsdsLet.Daemon, "Run app as a daemon with -daemon=true") + flag.DurationVar(&CONF.OsdsLet.LogFlushFrequency, "log-flush-frequency", CONF.OsdsLet.LogFlushFrequency, "Maximum number of seconds between log flushes") + flag.Parse() daemon.CheckAndRunDaemon(CONF.OsdsLet.Daemon) } diff --git a/pkg/utils/config/config.go b/pkg/utils/config/config.go index 79909bd92..267661a63 100755 --- a/pkg/utils/config/config.go +++ b/pkg/utils/config/config.go @@ -15,15 +15,18 @@ package config import ( - gflag "flag" + "flag" "fmt" "log" + "os" "reflect" + "regexp" "strconv" "strings" "time" "github.com/go-ini/ini" + "github.com/opensds/opensds/pkg/utils/constants" ) const ( @@ -225,9 +228,6 @@ func parseSections(cfg *ini.File, t reflect.Type, v reflect.Value) error { for i := 0; i < t.NumField(); i++ { field := v.Field(i) section := t.Field(i).Tag.Get("conf") - if "FlagSet" == field.Type().Name() { - continue - } if "" == section { if err := parseSections(cfg, field.Type(), field); err != nil { return err @@ -262,11 +262,23 @@ func GetDefaultConfig() *Config { return conf } -func (c *Config) Load(confFile string) { - gflag.StringVar(&confFile, "config-file", confFile, "The configuration file of OpenSDS") - c.Flag.Parse() - initConf(confFile, CONF) - c.Flag.AssignValue() +func GetConfigPath() string { + path := constants.OpensdsConfigPath + for i := 1; i < len(os.Args)-1; i++ { + if m, _ := regexp.MatchString(`^-{1,2}config-file$`, os.Args[i]); m { + if !strings.HasSuffix(os.Args[i+1], "-") { + path = os.Args[i+1] + } + } + } + return path +} + +func (c *Config) Load() { + var dummyConfigPath string + // Flag 'config-file' is set here for usage show and parameter check, the config path will be parsed by GetConfigPath + flag.StringVar(&dummyConfigPath, "config-file", constants.OpensdsConfigPath, "OpenSDS config file path") + initConf(GetConfigPath(), CONF) } func GetBackendsMap() map[string]BackendProperties { diff --git a/pkg/utils/config/config_define.go b/pkg/utils/config/config_define.go index 4644685eb..d36d8e9f6 100755 --- a/pkg/utils/config/config_define.go +++ b/pkg/utils/config/config_define.go @@ -94,5 +94,4 @@ type Config struct { OsdsDock `conf:"osdsdock"` Database `conf:"database"` KeystoneAuthToken `conf:"keystone_authtoken"` - Flag FlagSet } diff --git a/pkg/utils/config/config_test.go b/pkg/utils/config/config_test.go index 02f25b046..9c3c1254f 100755 --- a/pkg/utils/config/config_test.go +++ b/pkg/utils/config/config_test.go @@ -261,7 +261,7 @@ func TestFunctionDefaultValue(t *testing.T) { } func TestOpensdsConfig(t *testing.T) { - CONF.Load("testdata/opensds.conf") + initConf("testdata/opensds.conf", CONF) if CONF.OsdsApiServer.ApiEndpoint != "localhost:50040" { t.Error("Test OsdsApiServer.ApiEndpoint error") diff --git a/pkg/utils/config/flag.go b/pkg/utils/config/flag.go deleted file mode 100755 index c4ac24c5e..000000000 --- a/pkg/utils/config/flag.go +++ /dev/null @@ -1,116 +0,0 @@ -// Copyright (c) 2017 Huawei Technologies Co., Ltd. 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 config - -import ( - gflag "flag" - "reflect" - "time" -) - -type Flag struct { - InValue interface{} - Value interface{} -} - -type FlagSet struct { - flagMap map[string]*Flag -} - -type Value interface { - Set(string) error -} - -func (f *FlagSet) BoolVar(p *bool, name string, defValue bool, usage string) { - inVal := new(bool) - flag := &Flag{Value: p, InValue: inVal} - gflag.BoolVar(inVal, name, defValue, usage) - f.Add(name, flag) -} - -func (f *FlagSet) IntVar(p *int, name string, defValue int, usage string) { - inVal := new(int) - flag := &Flag{Value: p, InValue: inVal} - gflag.IntVar(inVal, name, defValue, usage) - f.Add(name, flag) -} - -func (f *FlagSet) Int64Var(p *int64, name string, defValue int64, usage string) { - inVal := new(int64) - flag := &Flag{Value: p, InValue: inVal} - gflag.Int64Var(inVal, name, defValue, usage) - f.Add(name, flag) -} - -func (f *FlagSet) UintVar(p *uint, name string, defValue uint, usage string) { - inVal := new(uint) - flag := &Flag{Value: p, InValue: inVal} - gflag.UintVar(inVal, name, defValue, usage) - f.Add(name, flag) -} - -func (f *FlagSet) Uint64Var(p *uint64, name string, defValue uint64, usage string) { - inVal := new(uint64) - flag := &Flag{Value: p, InValue: inVal} - gflag.Uint64Var(inVal, name, defValue, usage) - f.Add(name, flag) -} - -func (f *FlagSet) Float64Var(p *float64, name string, defValue float64, usage string) { - inVal := new(float64) - flag := &Flag{Value: p, InValue: inVal} - gflag.Float64Var(inVal, name, defValue, usage) - f.Add(name, flag) -} - -func (f *FlagSet) StringVar(p *string, name string, defValue string, usage string) { - inVal := new(string) - flag := &Flag{Value: p, InValue: inVal} - gflag.StringVar(inVal, name, defValue, usage) - f.Add(name, flag) -} - -func (f *FlagSet) DurationVar(p *time.Duration, name string, defValue time.Duration, usage string) { - inVal := new(time.Duration) - flag := &Flag{Value: p, InValue: inVal} - gflag.DurationVar(inVal, name, defValue, usage) - f.Add(name, flag) -} - -func (f *FlagSet) Add(name string, flag *Flag) { - if f.flagMap == nil { - f.flagMap = make(map[string]*Flag) - } - f.flagMap[name] = flag -} - -func (f *FlagSet) Parse() { - gflag.Parse() -} - -func (f *FlagSet) AssignValue() { - var actual []string - gflag.CommandLine.Visit(func(flag *gflag.Flag) { - actual = append(actual, flag.Name) - }) - for _, name := range actual { - if _, ok := f.flagMap[name]; !ok { - continue - } - iv := reflect.ValueOf(f.flagMap[name].InValue).Elem() - v := reflect.ValueOf(f.flagMap[name].Value).Elem() - v.Set(iv) - } -} diff --git a/pkg/utils/config/flag_test.go b/pkg/utils/config/flag_test.go deleted file mode 100755 index 72904c337..000000000 --- a/pkg/utils/config/flag_test.go +++ /dev/null @@ -1,97 +0,0 @@ -// Copyright (c) 2017 Huawei Technologies Co., Ltd. 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 config - -import ( - gflag "flag" - "fmt" - "os" - "testing" -) - -func ResetForTesting(usage func()) { - gflag.CommandLine = gflag.NewFlagSet(os.Args[0], gflag.ContinueOnError) - gflag.CommandLine.Usage = func() {} - gflag.Usage = usage -} - -func TestFlag(t *testing.T) { - f := FlagSet{} - var stringval string - var boolval bool - var intval int - var int64val int64 - var uintval uint - var uint64val uint64 - var flat64val float64 - f.StringVar(&stringval, "StringVar", "DefualtString", "test") - f.BoolVar(&boolval, "BoolVar", false, "test") - f.IntVar(&intval, "IntVar", -321, "test") - f.Int64Var(&int64val, "Int64Var", -321, "test") - f.UintVar(&uintval, "UintVar", 321, "test") - f.Uint64Var(&uint64val, "Uint64Var", 321, "test") - f.Float64Var(&flat64val, "Float64Var", 0.321, "test") - cmd := []string{ - "-StringVar", "HelloWorld", - "-BoolVar", - "-IntVar", "-123", - "-Int64Var", "-123", - "-UintVar", "123", - "-Uint64Var", "123", - "-Float64Var", "0.123", - } - err := gflag.CommandLine.Parse(cmd) - if err != nil { - t.Error(err) - } - f.AssignValue() - if stringval != "HelloWorld" { - t.Error("Test StringVar Failed!") - } - if boolval != true { - t.Error("Test BoolVar Failed!") - } - if intval != -123 { - t.Error("Test IntVar Failed!") - } - if int64val != -123 { - t.Error("Test Int64Var Failed!") - } - if uintval != 123 { - t.Error("Test UintVar Failed!") - } - if uint64val != 123 { - t.Error("Test Uint64Var Failed!") - } - if flat64val != 0.123 { - t.Error("Test Float64Var Failed!") - } -} - -func TestDefaultValue(t *testing.T) { - ResetForTesting(func() { t.Fatal("bad parse") }) - f := FlagSet{} - var stringval string - f.StringVar(&stringval, "StringVar", "DefualtString", "test") - err := gflag.CommandLine.Parse([]string{}) - if err != nil { - t.Error(err) - } - f.AssignValue() - fmt.Println(stringval) - if stringval != "" { - t.Error("Test StringVar Failed!") - } -} diff --git a/pkg/utils/daemon/daemon.go b/pkg/utils/daemon/daemon.go index e6b2cdbc5..79cbfbb04 100644 --- a/pkg/utils/daemon/daemon.go +++ b/pkg/utils/daemon/daemon.go @@ -20,16 +20,10 @@ import ( "os/exec" "regexp" "runtime" - - "github.com/opensds/opensds/pkg/utils/config" ) var execCmdHandler = execCmd -func SetDaemonFlag(isDaemon *bool, defaultVal bool) { - config.CONF.Flag.BoolVar(isDaemon, "daemon", defaultVal, "run app as a daemon with -daemon=true") -} - func execCmd(name string, args ...string) { cmd := exec.Command(name, args...) if err := cmd.Start(); err != nil { diff --git a/pkg/utils/daemon/daemon_test.go b/pkg/utils/daemon/daemon_test.go index d01735e27..563532eb3 100644 --- a/pkg/utils/daemon/daemon_test.go +++ b/pkg/utils/daemon/daemon_test.go @@ -15,6 +15,7 @@ package daemon import ( + "flag" "fmt" "io/ioutil" "os" @@ -31,8 +32,7 @@ const testFile = "test.txt" func TestDaemon(t *testing.T) { var godaemon bool - - SetDaemonFlag(&godaemon, false) + flag.BoolVar(&godaemon, "daemon", false, "Run app as a daemon with -daemon=true") execCmdHandler = mockCmdHandler diff --git a/script/devsds/lib/opensds.sh b/script/devsds/lib/opensds.sh index 191b7ae4d..961f7ae9a 100644 --- a/script/devsds/lib/opensds.sh +++ b/script/devsds/lib/opensds.sh @@ -25,7 +25,6 @@ osds:opensds:configuration(){ cat >> $OPENSDS_CONFIG_DIR/opensds.conf << OPENSDS_GLOBAL_CONFIG_DOC [osdsapiserver] api_endpoint = 0.0.0.0:50040 -log_file = /var/log/opensds/osdsapiserver.log auth_strategy = $OPENSDS_AUTH_STRATEGY # If https is enabled, the default value of cert file # is /opt/opensds-security/opensds/opensds-cert.pem, @@ -36,11 +35,9 @@ beego_https_key_file = [osdslet] api_endpoint = $HOST_IP:50049 -log_file = /var/log/opensds/osdslet.log [osdsdock] api_endpoint = $HOST_IP:50050 -log_file = /var/log/opensds/osdsdock.log # Specify which backends should be enabled, sample,ceph,cinder,lvm and so on. enabled_backends = $OPENSDS_BACKEND_LIST From 0cf6eef9feefd72a78ed32638ae62bcc26d3902a Mon Sep 17 00:00:00 2001 From: jerry Date: Thu, 14 Feb 2019 15:26:02 +0800 Subject: [PATCH 2/2] remove un-used configuration item --- examples/opensds.conf | 3 --- pkg/utils/config/testdata/opensds.conf | 3 --- test/integration/prepare.sh | 3 --- 3 files changed, 9 deletions(-) diff --git a/examples/opensds.conf b/examples/opensds.conf index b884b2ba1..145ec43c9 100755 --- a/examples/opensds.conf +++ b/examples/opensds.conf @@ -14,7 +14,6 @@ [osdsapiserver] api_endpoint = 0.0.0.0:50040 -log_file = /var/log/opensds/osdsapiserver.log auth_strategy = keystone # If https is enabled, the default value of cert file # is /opt/opensds-security/opensds/opensds-cert.pem, @@ -27,11 +26,9 @@ password_decrypt_tool = aes [osdslet] api_endpoint = 0.0.0.0:50049 -log_file = /var/log/opensds/osdslet.log [osdsdock] api_endpoint = 0.0.0.0:50050 -log_file = /var/log/opensds/osdsdock.log # Choose the type of dock resource, only support 'provisioner' and 'attacher'. dock_type = provisioner # Specify which backends should be enabled, sample,ceph,cinder,lvm and so on. diff --git a/pkg/utils/config/testdata/opensds.conf b/pkg/utils/config/testdata/opensds.conf index 4b27eb21d..b9cd698b0 100755 --- a/pkg/utils/config/testdata/opensds.conf +++ b/pkg/utils/config/testdata/opensds.conf @@ -1,6 +1,5 @@ [osdsapiserver] api_endpoint = localhost:50040 -log_file = /var/log/opensds/osdsapiserver.log log_flush_frequency = 2s auth_strategy = keystone # If https is enabled, the default value of cert file @@ -14,12 +13,10 @@ password_decrypt_tool = aes [osdslet] api_endpoint = localhost:50049 -log_file = /var/log/opensds/osdslet.log log_flush_frequency = 3s [osdsdock] api_endpoint = localhost:50050 -log_file = /var/log/opensds/osdsdock.log # Choose the type of dock resource, only support 'provisioner' and 'attacher'. dock_type = provisioner # Specify which backends should be enabled, sample,ceph,cinder,lvm and so on. diff --git a/test/integration/prepare.sh b/test/integration/prepare.sh index 705c0cbec..5d3da75e0 100755 --- a/test/integration/prepare.sh +++ b/test/integration/prepare.sh @@ -26,15 +26,12 @@ mkdir -p /etc/opensds cat > ${OPENSDS_CONF} << OPENSDS_GLOBAL_CONFIG_DOC [osdsapiserver] api_endpoint = 0.0.0.0:50040 -log_file = /var/log/opensds/osdsapiserver.log [osdslet] api_endpoint = 0.0.0.0:50049 -log_file = /var/log/opensds/osdslet.log [osdsdock] api_endpoint = 0.0.0.0:50050 -log_file = /var/log/opensds/osdsdock.log # Choose the type of dock resource, only support 'provisioner' and 'attacher'. dock_type = provisioner # Specify which backends should be enabled, sample,ceph,cinder,lvm and so on.