refactor: refactor damon config code

1. move daemon config code from main.go to daemon/config/config.go. 2. supports to resolve config file contains nested object. 3. skip check conflict in slice type flags and merge them. Signed-off-by: Ace-Tang <[email protected]>
eahydra · Apr 18, 2018 · 7713b77 · 7713b77
1 parent 5caea36
commit 7713b77
Show file tree

Hide file tree

Showing 6 changed files with 207 additions and 89 deletions.
diff --git a/daemon/config/config.go b/daemon/config/config.go
@@ -1,7 +1,11 @@
 package config
 
 import (
+	"bytes"
+	"encoding/json"
 	"fmt"
+	"io/ioutil"
+	"os"
 	"strings"
 	"sync"
 
@@ -10,6 +14,8 @@ import (
 	"github.com/alibaba/pouch/network"
 	"github.com/alibaba/pouch/pkg/utils"
 	"github.com/alibaba/pouch/storage/volume"
+
+	"github.com/spf13/pflag"
 )
 
 // Config refers to daemon's whole configurations.
@@ -110,3 +116,101 @@ func (cfg *Config) Validate() error {
 
 	return nil
 }
+
+//MergeConfigurations merges flagSet flags and config file flags into Config.
+func (cfg *Config) MergeConfigurations(config *Config, flagSet *pflag.FlagSet) error {
+	contents, err := ioutil.ReadFile(cfg.ConfigFile)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return fmt.Errorf("failed to read contents from config file %s: %s", cfg.ConfigFile, err)
+	}
+
+	var origin map[string]interface{}
+	if err = json.NewDecoder(bytes.NewReader(contents)).Decode(&origin); err != nil {
+		return fmt.Errorf("failed to decode json: %s", err)
+	}
+
+	if len(origin) == 0 {
+		return nil
+	}
+
+	fileFlags := make(map[string]interface{}, 0)
+	iterateConfig(origin, fileFlags)
+
+	// check if invalid or unknown flag exist in config file
+	if err = getUnknownFlags(flagSet, fileFlags); err != nil {
+		return err
+	}
+
+	// check conflict in command line flags and config file
+	if err = getConflictConfigurations(flagSet, fileFlags); err != nil {
+		return err
+	}
+
+	fileConfig := &Config{}
+	if err = json.NewDecoder(bytes.NewReader(contents)).Decode(fileConfig); err != nil {
+		return fmt.Errorf("failed to decode json: %s", err)
+	}
+
+	// merge configurations from command line flags and config file
+	err = mergeConfigurations(fileConfig, cfg)
+	return err
+
+}
+
+// iterateConfig resolves key-value from config file iteratly.
+func iterateConfig(origin map[string]interface{}, config map[string]interface{}) {
+	for k, v := range origin {
+		if c, ok := v.(map[string]interface{}); ok {
+			iterateConfig(c, config)
+		} else {
+			config[k] = v
+		}
+	}
+}
+
+// find unknown flag in config file
+func getUnknownFlags(flagSet *pflag.FlagSet, fileFlags map[string]interface{}) error {
+	var unknownFlags []string
+
+	for k := range fileFlags {
+		f := flagSet.Lookup(k)
+		if f == nil {
+			unknownFlags = append(unknownFlags, k)
+		}
+	}
+
+	if len(unknownFlags) > 0 {
+		return fmt.Errorf("unknown flags: %s", strings.Join(unknownFlags, ", "))
+	}
+
+	return nil
+}
+
+// find conflict in command line flags and config file, note that if flag value
+// is slice type, we will skip it and merge it from flags and config file later.
+func getConflictConfigurations(flagSet *pflag.FlagSet, fileFlags map[string]interface{}) error {
+	var conflictFlags []string
+	flagSet.Visit(func(f *pflag.Flag) {
+		flagType := f.Value.Type()
+		if strings.Contains(flagType, "Slice") {
+			return
+		}
+		if v, exist := fileFlags[f.Name]; exist {
+			conflictFlags = append(conflictFlags, fmt.Sprintf("from flag: %s and from config file: %s", f.Value.String(), v))
+		}
+	})
+
+	if len(conflictFlags) > 0 {
+		return fmt.Errorf("found conflict flags in command line and config file: %v", strings.Join(conflictFlags, ", "))
+	}
+
+	return nil
+}
+
+// merge flagSet and config file into cfg
+func mergeConfigurations(src *Config, dest *Config) error {
+	return utils.Merge(src, dest)
+}
diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go
@@ -0,0 +1,45 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIterateConfig(t *testing.T) {
+	assert := assert.New(t)
+	origin := map[string]interface{}{
+		"a": "a",
+		"b": "b",
+		"c": "c",
+		"iter1": map[string]interface{}{
+			"i1": "i1",
+			"i2": "i2",
+		},
+		"iter11": map[string]interface{}{
+			"ii1": map[string]interface{}{
+				"iii1": "iii1",
+				"iii2": "iii2",
+			},
+		},
+	}
+
+	expect := map[string]interface{}{
+		"a":    "a",
+		"b":    "b",
+		"c":    "c",
+		"i1":   "i1",
+		"i2":   "i2",
+		"iii1": "iii1",
+		"iii2": "iii2",
+	}
+
+	config := make(map[string]interface{})
+	iterateConfig(origin, config)
+	assert.Equal(config, expect)
+
+	// test nil map will not cause panic
+	config = make(map[string]interface{})
+	iterateConfig(nil, config)
+	assert.Equal(config, map[string]interface{}{})
+}
diff --git a/main.go b/main.go
@@ -1,10 +1,7 @@
 package main
 
 import (
-	"bytes"
-	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"os"
 	osexec "os/exec"
 	"os/signal"
@@ -269,92 +266,9 @@ func checkLxcfsCfg() error {
 
 // load daemon config file
 func loadDaemonFile(cfg *config.Config, flagSet *pflag.FlagSet) error {
-	configFile := cfg.ConfigFile
-	if configFile == "" {
+	if cfg.ConfigFile == "" {
 		return nil
 	}
 
-	contents, err := ioutil.ReadFile(configFile)
-	if err != nil {
-		if os.IsNotExist(err) {
-			return nil
-		}
-		return fmt.Errorf("failed to read contents from config file %s: %s", configFile, err)
-	}
-
-	var fileFlags map[string]interface{}
-	if err = json.NewDecoder(bytes.NewReader(contents)).Decode(&fileFlags); err != nil {
-		return fmt.Errorf("failed to decode json: %s", err)
-	}
-
-	if len(fileFlags) == 0 {
-		return nil
-	}
-
-	// check if invalid or unknown flag exist in config file
-	if err = getUnknownFlags(flagSet, fileFlags); err != nil {
-		return err
-	}
-
-	// check conflict in command line flags and config file
-	if err = getConflictConfigurations(flagSet, fileFlags); err != nil {
-		return err
-	}
-
-	fileConfig := &config.Config{}
-	if err = json.NewDecoder(bytes.NewReader(contents)).Decode(fileConfig); err != nil {
-		return fmt.Errorf("failed to decode json: %s", err)
-	}
-
-	// merge configurations from command line flags and config file
-	err = mergeConfigurations(fileConfig, cfg)
-	return err
-}
-
-// find unknown flag in config file
-func getUnknownFlags(flagSet *pflag.FlagSet, fileFlags map[string]interface{}) error {
-	var unknownFlags []string
-
-	for k, v := range fileFlags {
-		if m, ok := v.(map[string]interface{}); ok {
-			for k = range m {
-				f := flagSet.Lookup(k)
-				if f == nil {
-					unknownFlags = append(unknownFlags, k)
-				}
-			}
-			continue
-		}
-		f := flagSet.Lookup(k)
-		if f == nil {
-			unknownFlags = append(unknownFlags, k)
-		}
-	}
-
-	if len(unknownFlags) > 0 {
-		return fmt.Errorf("unknown flags: %s", strings.Join(unknownFlags, ", "))
-	}
-
-	return nil
-}
-
-// find conflict in command line flags and config file
-func getConflictConfigurations(flagSet *pflag.FlagSet, fileFlags map[string]interface{}) error {
-	var conflictFlags []string
-	flagSet.Visit(func(f *pflag.Flag) {
-		if v, exist := fileFlags[f.Name]; exist {
-			conflictFlags = append(conflictFlags, fmt.Sprintf("from flag: %s and from config file: %s", f.Value.String(), v))
-		}
-	})
-
-	if len(conflictFlags) > 0 {
-		return fmt.Errorf("found conflict flags in command line and config file: %v", strings.Join(conflictFlags, ", "))
-	}
-
-	return nil
-}
-
-// merge flagSet and config file into cfg
-func mergeConfigurations(src *config.Config, dest *config.Config) error {
-	return utils.Merge(src, dest)
+	return cfg.MergeConfigurations(cfg, flagSet)
 }
diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
@@ -122,7 +122,8 @@ func Merge(src, dest interface{}) error {
 	return doMerge(srcVal, destVal)
 }
 
-// doMerge, begin merge action
+// doMerge, begin merge action, note that we will merge slice type,
+// but we do not validate if slice has duplicate values.
 func doMerge(src, dest reflect.Value) error {
 	if !src.IsValid() || !dest.CanSet() || isEmptyValue(src) {
 		return nil
@@ -143,6 +144,9 @@ func doMerge(src, dest reflect.Value) error {
 			}
 		}
 
+	case reflect.Slice:
+		dest.Set(reflect.AppendSlice(dest, src))
+
 	default:
 		dest.Set(src)
 	}

diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go
@@ -138,6 +138,7 @@ func TestMerge(t *testing.T) {
 		Sc bool
 		Sd map[string]string
 		Se nestS
+		Sf []string
 	}
 
 	getIntAddr := func(i int) *int {
@@ -184,6 +185,11 @@ func TestMerge(t *testing.T) {
 			dest:     &simple{Sa: 1, Sb: "hello", Sc: true, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 22}},
 			expected: &simple{Sa: 1, Sb: "hello", Sc: true, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 22}},
 			ok:       true,
+		}, {
+			src:      &simple{Sa: 1, Sc: true, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 11}, Sf: []string{"foo"}},
+			dest:     &simple{Sa: 2, Sb: "world", Sc: false, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 22}, Sf: []string{"foo"}},
+			expected: &simple{Sa: 1, Sb: "world", Sc: true, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 11}, Sf: []string{"foo", "foo"}},
+			ok:       true,
 		},
 	} {
 		err := Merge(tm.src, tm.dest)

diff --git a/test/z_cli_daemon_test.go b/test/z_cli_daemon_test.go
@@ -144,6 +144,51 @@ func (suite *PouchDaemonSuite) TestDaemonConfigFileConfilct(c *check.C) {
 	c.Assert(err, check.NotNil)
 }
 
+// TestDaemonNestObjectConfilct tests start daemon with configure file contains nest objects confilicts with parameter.
+func (suite *PouchDaemonSuite) TestDaemonNestObjectConfilct(c *check.C) {
+	path := "/tmp/pouch_nest.json"
+	type TLSConfig struct {
+		CA               string `json:"tlscacert,omitempty"`
+		Cert             string `json:"tlscert,omitempty"`
+		Key              string `json:"tlskey,omitempty"`
+		VerifyRemote     bool   `json:"tlsverify"`
+		ManagerWhiteList string `json:"manager-whitelist"`
+	}
+	cfg := struct {
+		TLS TLSConfig
+	}{
+		TLS: TLSConfig{
+			CA:   "ca",
+			Cert: "cert",
+			Key:  "key",
+		},
+	}
+	err := CreateConfigFile(path, cfg)
+	c.Assert(err, check.IsNil)
+	defer os.Remove(path)
+
+	dcfg, err := StartDefaultDaemon("--tlscacert", "ca", "--config-file="+path)
+	dcfg.KillDaemon()
+	c.Assert(err, check.NotNil)
+}
+
+// TestDaemonSliceFlagNotConflict tests start daemon with configure file contains slice flag will not oconfilicts with parameter.
+func (suite *PouchDaemonSuite) TestDaemonSliceFlagNotConflict(c *check.C) {
+	path := "/tmp/pouch_slice.json"
+	cfg := struct {
+		Labels []string `json:"label"`
+	}{
+		Labels: []string{"a=a", "b=b"},
+	}
+	err := CreateConfigFile(path, cfg)
+	c.Assert(err, check.IsNil)
+	defer os.Remove(path)
+
+	dcfg, err := StartDefaultDaemon("--label", "c=d", "--config-file="+path)
+	dcfg.KillDaemon()
+	c.Assert(err, check.IsNil)
+}
+
 // TestDaemonConfigFileUnknownFlag tests start daemon with unknown flags in configure file.
 func (suite *PouchDaemonSuite) TestDaemonConfigFileUnknownFlag(c *check.C) {
 	path := "/tmp/pouch.json"