Config refactor (#3)

* Updating logging to reflect changes made to zap.Tee * refactoring how configuration files are set
uber · Jan 17, 2017 · 4090cc7 · 4090cc7
1 parent bcedfa8
commit 4090cc7
Show file tree

Hide file tree

Showing 11 changed files with 47 additions and 130 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -3,9 +3,6 @@ sudo: required
 go:
   - 1.7
   - tip
-env:
-  global:
-    - ARACHNE_CONFIG_DIR=./arachned/config
 install: make install_ci
 script: make test
 cache:

diff --git a/README.md b/README.md
@@ -48,19 +48,18 @@ ____________________________________________________________/\\\________________
         __\////////\//__\///___________\////////\//_____\////////__\///____\///__\///____\///____\//////////__
 
 
-Usage: arachne [--foreground] [-c=<config_file_path>] [--receiver_only] [--sender_only] [--orchestrator]
+Usage: arachne [--foreground] [-c=<config_file>] [--receiver_only] [--sender_only]
 
 Arachne is a packet loss detection system and an underperforming path detection
 system for Data Center and Cloud infrastructures.
 
 Options:
   -v, --version                                     Show the version and exit
   --foreground=false                                Force foreground mode
-  -c, --config="/etc/arachne/target_config.json"   Configuration file path
-  (by default in /etc/arachne/)
+  -c, --config_file="/etc/arachne/arachne.yaml"     Configuration file path
+  (default: /etc/arachne/arachne.yaml)
   --receiver_only=false                             Force TCP receiver-only mode
   --sender_only=false                               Force TCP sender-only mode
-  --orchestrator=false                              Force orchestrator mode
 ```
 
 
@@ -78,4 +77,4 @@ Released under the [MIT License](LICENSE).
 [doc]: https://godoc.org/github.com/uber/arachne
 [ci-img]: https://travis-ci.org/uber/arachne.svg?branch=master
 [ci]: https://travis-ci.org/uber/arachne
-[capabilities]: http://linux.die.net/man/7/capabilities
+[capabilities]: http://linux.die.net/man/7/capabilities
diff --git a/arachned/config/arachne.yaml b/arachned/config/arachne.yaml
@@ -1,8 +1,10 @@
 arachne:
   pidPath: /var/run/arachne/arachne.pid
   orchestrator:
+    enabled: false
     addrport: 127.0.0.1:12345
     restVersion: api/v1
+  standaloneTargetConfig: /etc/arachne/target_config.json
 
 logging:
   stdout: true

diff --git a/arachned/main.go b/arachned/main.go
@@ -37,6 +37,5 @@ func main() {
 	arachne.Run(
 		ec,
 		arachne.ReceiverOnlyMode(false),
-		arachne.OrchestratorMode(false),
 	)
 }
diff --git a/bootstrap.go b/bootstrap.go
@@ -47,7 +47,7 @@ func Run(ec *config.Extended, opts ...Option) {
 
 	gl.CLI = config.ParseCliArgs(bootstrapLogger, d.ArachneService, d.ArachneVersion)
 	apply(&gl, opts...)
-	gl.App, err = config.Get(ec, bootstrapLogger)
+	gl.App, err = config.Get(*gl.CLI.ConfigFile, ec, bootstrapLogger)
 	if err != nil {
 		os.Exit(1)
 	}

diff --git a/config/config.go b/config/config.go
@@ -27,17 +27,13 @@ import (
 	"net"
 	"net/http"
 	"os"
-	"path"
 	"runtime"
 	"strings"
 	"sync"
 	"time"
 
-	"github.com/fsnotify/fsnotify"
 	"github.com/jawher/mow.cli"
-	"github.com/spf13/viper"
 	"github.com/uber-go/zap"
-	"github.com/uber/arachne/defines"
 	"github.com/uber/arachne/internal/log"
 	"github.com/uber/arachne/internal/network"
 	"github.com/uber/arachne/internal/tcp"
@@ -46,20 +42,18 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
-const (
-	configDirKey  = "ARACHNE_CONFIG_DIR"
-	configDir     = "/etc/arachne"
-	defTargetFile = "target_config.json"
-)
+const defaultConfigFile = "/etc/arachne/arachne.yaml"
 
 // ArachneConfiguration contains specific configuration to Arachne.
 type ArachneConfiguration struct {
-	PIDPath      string             `yaml:"pidPath"`
-	Orchestrator OrchestratorConfig `yaml:"orchestrator"`
+	PIDPath                string             `yaml:"pidPath"`
+	Orchestrator           OrchestratorConfig `yaml:"orchestrator"`
+	StandaloneTargetConfig string             `yaml:"standaloneTargetConfig"`
 }
 
 // OrchestratorConfig contains configuration for the Arachne Orchestrator.
 type OrchestratorConfig struct {
+	Enabled     bool   `yaml:"enabled"`
 	AddrPort    string `yaml:"addrport"`
 	RESTVersion string `yaml:"restVersion"`
 }
@@ -116,20 +110,20 @@ type RemoteFileConfig struct {
 
 // AppConfig holds the info parsed from the local YAML config file.
 type AppConfig struct {
-	Logging      log.Config
-	Verbose      bool
-	PIDPath      string
-	Orchestrator OrchestratorConfig
-	Metrics      metrics.Config
+	Logging                log.Config
+	Verbose                bool
+	PIDPath                string
+	Orchestrator           OrchestratorConfig
+	StandaloneTargetConfig string
+	Metrics                metrics.Config
 }
 
 // CLIConfig holds the info parsed from CLI.
 type CLIConfig struct {
+	ConfigFile       *string
 	Foreground       *bool
-	TargetsLocalPath *string
 	ReceiverOnlyMode *bool
 	SenderOnlyMode   *bool
-	OrchestratorMode *bool
 }
 
 // RemoteConfig holds the info parsed from the JSON config file.
@@ -162,15 +156,6 @@ type Global struct {
 	Remotes      RemoteStore
 }
 
-func getConfigDir() string {
-	realConfigDir := configDir
-	if configRoot := os.Getenv(configDirKey); configRoot != "" {
-		realConfigDir = configRoot
-	}
-
-	return realConfigDir
-}
-
 func localFileReadable(path string) error {
 	if _, err := ioutil.ReadFile(path); err != nil {
 		return err
@@ -185,16 +170,13 @@ func ParseCliArgs(logger zap.Logger, service string, version string) *CLIConfig
 	app := cli.App(service, "Utility to echo the DC and Cloud Infrastructure")
 
 	app.Version("v version", "Arachne "+version)
-	app.Spec = "[--foreground] [-c=<config_file_path>] [--receiver_only] [--sender_only] [--orchestrator]"
-
-	defTargetLocalPath := path.Join(getConfigDir(), defTargetFile)
+	app.Spec = "[--foreground] [-c=<config_file>] [--receiver_only] [--sender_only]"
 
 	args.Foreground = app.BoolOpt("foreground", false, "Force foreground mode")
-	args.TargetsLocalPath = app.StringOpt("c config", defTargetLocalPath,
-		fmt.Sprintf("Local target list file path (by default: %s)", defTargetLocalPath))
+	args.ConfigFile = app.StringOpt("c config_file", defaultConfigFile,
+		fmt.Sprintf("Agent's primary yaml configuration file (by default: %s)", defaultConfigFile))
 	args.ReceiverOnlyMode = app.BoolOpt("receiver_only", false, "Force TCP receiver-only mode")
 	args.SenderOnlyMode = app.BoolOpt("sender_only", false, "Force TCP sender-only mode")
-	args.OrchestratorMode = app.BoolOpt("orchestrator", false, "Force orchestrator mode")
 
 	app.Action = func() {
 		logger.Debug("Command line arguments parsed")
@@ -205,42 +187,27 @@ func ParseCliArgs(logger zap.Logger, service string, version string) *CLIConfig
 }
 
 // Get fetches the configuration file from local path.
-func Get(ec *Extended, logger zap.Logger) (*AppConfig, error) {
-
-	fname := path.Join(getConfigDir(), fmt.Sprintf("%s.yaml", defines.ArachneService))
+func Get(cf string, ec *Extended, logger zap.Logger) (*AppConfig, error) {
 
-	viper.SetConfigName(defines.ArachneService)
-	viper.AddConfigPath(getConfigDir())
-
-	if err := viper.ReadInConfig(); err != nil {
-		logger.Error("error initializing configuration", zap.Error(err))
-		return nil, err
-	}
-
-	viper.WatchConfig()
-	viper.OnConfigChange(func(e fsnotify.Event) {
-		// TODO Add action on change
-		logger.Warn("Config file changed", zap.String("name", e.Name), zap.String("path", fname))
-	})
-
-	data, err := ioutil.ReadFile(fname)
+	data, err := ioutil.ReadFile(cf)
 	if err != nil {
-		logger.Error("error reading the configuration file", zap.String("file", fname), zap.Error(err))
+		logger.Error("error reading the configuration file", zap.String("file", cf), zap.Error(err))
 		return nil, err
 	}
 
-	b, err := unmarshalBasicConfig(data, fname, logger)
-	mc, err := ec.Metrics.UnmarshalConfig(data, fname, logger)
+	b, err := unmarshalBasicConfig(data, cf, logger)
+	mc, err := ec.Metrics.UnmarshalConfig(data, cf, logger)
 	if err != nil {
 		return nil, err
 	}
 
 	cfg := AppConfig{
-		Logging:      b.Logging,
-		Verbose:      b.Verbose,
-		PIDPath:      b.Arachne.PIDPath,
-		Orchestrator: b.Arachne.Orchestrator,
-		Metrics:      mc,
+		Logging:                b.Logging,
+		Verbose:                b.Verbose,
+		PIDPath:                b.Arachne.PIDPath,
+		Orchestrator:           b.Arachne.Orchestrator,
+		StandaloneTargetConfig: b.Arachne.StandaloneTargetConfig,
+		Metrics:                mc,
 	}
 
 	return &cfg, nil
@@ -282,23 +249,25 @@ func FetchRemoteList(
 	remotes := make(RemoteStore, maxNumRemoteTargets)
 
 	// Standalone (non-Orchestrator) mode
-	if !*(gl.CLI.OrchestratorMode) {
-		logger.Debug("Orchestrator mode disabled")
-		if err := localFileReadable(*gl.CLI.TargetsLocalPath); err != nil {
+	if !gl.App.Orchestrator.Enabled {
+		logger.Debug("Orchestrator mode disabled, using static target config file.",
+			zap.String("path", gl.App.StandaloneTargetConfig))
+
+		if err := localFileReadable(gl.App.StandaloneTargetConfig); err != nil {
 			logger.Fatal("unable to retrieve local configuration file",
-				zap.String("file", *gl.CLI.TargetsLocalPath),
+				zap.String("file", gl.App.StandaloneTargetConfig),
 				zap.Error(err))
 		}
-		logger.Info("Configuration file", zap.String("file", *gl.CLI.TargetsLocalPath))
+		logger.Info("Configuration file", zap.String("file", gl.App.StandaloneTargetConfig))
 
-		raw, err := ioutil.ReadFile(*gl.CLI.TargetsLocalPath)
+		raw, err := ioutil.ReadFile(gl.App.StandaloneTargetConfig)
 		if err != nil {
 			return fmt.Errorf("file error: %v", err)
 		}
 		if err := readRemoteList(raw, gl.RemoteConfig, remotes, maxNumSrcTCPPorts, minBatchInterval,
 			logger); err != nil {
 			logger.Fatal("error parsing default target list file",
-				zap.String("file", *gl.CLI.TargetsLocalPath),
+				zap.String("file", gl.App.StandaloneTargetConfig),
 				zap.Error(err))
 		}
 		gl.Remotes = remotes

diff --git a/example_run_arachne_test.go b/example_run_arachne_test.go
@@ -37,7 +37,6 @@ func Example_run_arachne() {
 	arachne.Run(
 		ec,
 		arachne.ReceiverOnlyMode(false),
-		arachne.OrchestratorMode(false),
 	)
 
 }
diff --git a/glide.lock b/glide.lock
diff --git a/glide.yaml b/glide.yaml
@@ -4,13 +4,12 @@ import:
   subpackages:
   - statsd
 - package: github.com/fatih/color
-- package: github.com/fsnotify/fsnotify
 - package: github.com/jawher/mow.cli
 - package: github.com/miekg/dns
 - package: github.com/spacemonkeygo/monotime
-- package: github.com/spf13/viper
 - package: github.com/uber/uber-licence
 - package: github.com/uber-go/zap
+  version: master
 - package: gopkg.in/validator.v2
 - package: gopkg.in/yaml.v2
 testImport:

diff --git a/internal/log/log.go b/internal/log/log.go
@@ -75,7 +75,7 @@ func CreateLogger(
 		}
 		bootstrapLogger.Info("Log file path provided", zap.String("path", c.LogSink))
 		if c.StdOut || foreground {
-			output = zap.Output(zap.Tee(os.Stdout, sink))
+			output = zap.Output(zap.MultiWriteSyncer(os.Stdout, sink))
 		} else {
 			output = zap.Output(sink)
 		}

diff --git a/options.go b/options.go
@@ -56,12 +56,3 @@ func SenderOnlyMode(b bool) Option {
 		return SenderOnlyMode(previous)
 	}
 }
-
-// OrchestratorMode sets orchestrator mode to `b`.
-func OrchestratorMode(b bool) Option {
-	return func(gl *config.Global) Option {
-		previous := *gl.CLI.OrchestratorMode
-		*gl.CLI.OrchestratorMode = b
-		return OrchestratorMode(previous)
-	}
-}
-Original file line number
+Diff line change
@@ Expand Up / @@ -3,9 +3,6 @@ sudo: required @@
     go:
       - 1.7
       - tip
-    env:
-      global:
-        - ARACHNE_CONFIG_DIR=./arachned/config
     install: make install_ci
     script: make test
     cache:
@@ Expand Down @@