Add install_ci dependency in Makefile (#5) - Issue #1

* Add install_ci dependency in Makefile

* Fix negation

* Address comments PR#5

* Minor edits

Author: Vasilis

Makefile

@@ -8,7 +8,7 @@ ${builddir}:
 	mkdir -p $(builddir)
 
 .PHONY: bins
-bins:
+bins: install_ci
 	go build -o ${builddir}/arachned github.com/uber/arachne/arachned/
 
 all: bins
@@ -22,7 +22,7 @@ lint:
 	go vet $(PACKAGES)
 
 .PHONY: test
-test: check-license lint
+test: check-license lint install_ci
 	find . -type f -name '*.go' | xargs golint
 	go test $(PACKAGES)
 

arachned/config/test_target_config_darwin.json

@@ -17,6 +17,13 @@
 
   "internal": [
     {"ip": "10.10.39.31"},
-    {"ip": "10.10.15.27"}
+    {"ip": "10.10.15.27"},
+    {"host_name": "compute777.myservers.com"},
+    {"host_name": "hadoop375.myservers.com"}
+  ],
+
+  "external": [
+    {"host_name": "payments.service.org"},
+    {"host_name": "maps.service.com"}
   ]
 }

arachned/config/test_target_config_linux.json

@@ -17,6 +17,13 @@
 
   "internal": [
     {"ip": "10.10.39.31"},
-    {"ip": "10.10.15.27"}
+    {"ip": "10.10.15.27"},
+    {"host_name": "compute777.myservers.com"},
+    {"host_name": "hadoop375.myservers.com"}
+  ],
+
+  "external": [
+    {"host_name": "payments.service.org"},
+    {"host_name": "maps.service.com"}
   ]
 }

bootstrap.go

@@ -49,6 +49,9 @@ func Run(ec *config.Extended, opts ...Option) {
 	apply(&gl, opts...)
 	gl.App, err = config.Get(*gl.CLI.ConfigFile, ec, bootstrapLogger)
 	if err != nil {
+		bootstrapLogger.Error("error reading the configuration file",
+			zap.String("file", *gl.CLI.ConfigFile),
+			zap.Error(err))
 		os.Exit(1)
 	}
 

collector/collector.go

@@ -151,7 +151,7 @@ func (rs resultStore) walkResults(
 	}
 }
 
-// processResults calculates metrics, uploads stats and stores in results[] for stdout, if needed
+// processResults calculates metrics, uploads stats and stores in results[] for stdout, if needed.
 func (rs resultStore) processResults(
 	gl *config.Global,
 	remotes config.RemoteStore,
@@ -390,7 +390,7 @@ func statsUpload(
 
 }
 
-// zeroOutResults fills latencies for targets not existing in resultStore with zeros
+// zeroOutResults fills latencies for targets not existing in resultStore with zeros.
 func zeroOutResults(
 	glr *config.RemoteConfig,
 	ms messageStore,

config/config.go

@@ -22,6 +22,7 @@ package config
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -61,7 +62,6 @@ type OrchestratorConfig struct {
 // BasicConfig holds the basic parameter configurations for the application.
 type BasicConfig struct {
 	Logging log.Config           `yaml:"logging"`
-	Verbose bool                 `yaml:"verbose"`
 	Arachne ArachneConfiguration `yaml:"arachne"`
 }
 
@@ -70,7 +70,7 @@ type Extended struct {
 	Metrics metrics.Opt
 }
 
-// RemoteStore holds all Remotes
+// RemoteStore holds all Remotes.
 type RemoteStore map[string]Remote
 
 // Remote holds the info for every target to be echoed.
@@ -191,48 +191,50 @@ func Get(cf string, ec *Extended, logger zap.Logger) (*AppConfig, error) {
 
 	data, err := ioutil.ReadFile(cf)
 	if err != nil {
-		logger.Error("error reading the configuration file", zap.String("file", cf), zap.Error(err))
 		return nil, err
 	}
 
 	b, err := unmarshalBasicConfig(data, cf, logger)
+	if err != nil {
+		return nil, err
+	}
+
 	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,
 		StandaloneTargetConfig: b.Arachne.StandaloneTargetConfig,
 		Metrics:                mc,
 	}
 
+	if !cfg.Orchestrator.Enabled && cfg.StandaloneTargetConfig == "" {
+		return nil, errors.New("the standalone-mode target configuration file has not been specified")
+	}
+
 	return &cfg, nil
 }
 
-// unmarshalBasicConfig fetches the configuration file from local path
+// unmarshalBasicConfig fetches the configuration file from local path.
 func unmarshalBasicConfig(data []byte, fname string, logger zap.Logger) (*BasicConfig, error) {
 
 	cfg := new(BasicConfig)
 	if err := yaml.Unmarshal(data, cfg); err != nil {
-		logger.Error("error unmarshaling the configuration file",
-			zap.String("file", fname),
-			zap.Error(err))
-		return nil, err
+		return nil, fmt.Errorf("error unmarshaling the configuration file %s: %v", fname, err)
 	}
 	// Validate on the merged config at the end
 	if err := validator.Validate(cfg); err != nil {
-		logger.Error("invalid info in configuration file", zap.String("file", fname), zap.Error(err))
-		return nil, err
+		return nil, fmt.Errorf("invalid info in configuration file %s: %v", fname, err)
 	}
 
 	return cfg, nil
 }
 
-// FetchRemoteList fetches the configuration file from local path or, remotely, from Arachne Orchestrator
+// FetchRemoteList fetches the configuration file from local path or, remotely, from Arachne Orchestrator.
 func FetchRemoteList(
 	gl *Global,
 	maxNumRemoteTargets int,
@@ -254,7 +256,7 @@ func FetchRemoteList(
 			zap.String("path", gl.App.StandaloneTargetConfig))
 
 		if err := localFileReadable(gl.App.StandaloneTargetConfig); err != nil {
-			logger.Fatal("unable to retrieve local configuration file",
+			logger.Fatal("unable to retrieve local target configuration file",
 				zap.String("file", gl.App.StandaloneTargetConfig),
 				zap.Error(err))
 		}
@@ -294,7 +296,7 @@ func FetchRemoteList(
 	return err
 }
 
-// createHTTPClient returns an HTTP client to connect to remote server
+// createHTTPClient returns an HTTP client to connect to remote server.
 func createHTTPClient(timeout time.Duration, disableKeepAlives bool) *http.Client {
 	client := &http.Client{
 		Transport: &http.Transport{
@@ -309,7 +311,7 @@ func createHTTPClient(timeout time.Duration, disableKeepAlives bool) *http.Clien
 	return client
 }
 
-// GetHostname returns the hostname
+// GetHostname returns the hostname.
 func GetHostname(logger zap.Logger) (string, error) {
 	host, err := os.Hostname()
 	if err != nil {
@@ -319,7 +321,7 @@ func GetHostname(logger zap.Logger) (string, error) {
 	return host, nil
 }
 
-// refreshRemoteList checks with Arachne Orchestrator if new a config file should be fetched
+// refreshRemoteList checks with Arachne Orchestrator if new a config file should be fetched.
 func refreshRemoteList(
 	gl *Global,
 	remotes RemoteStore,
@@ -335,8 +337,6 @@ func refreshRemoteList(
 	retryTime := gl.RemoteConfig.PollOrchestratorInterval.Failure
 
 	for {
-		var err error
-
 		hostname, _ := GetHostname(logger)
 		RESTReq := fmt.Sprintf("http://%s/%s/%s?hostname=%s",
 			gl.App.Orchestrator.AddrPort,
@@ -395,7 +395,7 @@ func fetchRemoteListFromOrchestrator(
 	// Build the request
 	req, err := http.NewRequest("GET", url, nil)
 	if err != nil {
-		logger.Debug("NewRequest", zap.Error(err))
+		logger.Warn("NewRequest", zap.Error(err))
 		return 0, nil, err
 	}
 	resp, err := client.Do(req)
@@ -411,19 +411,14 @@ func fetchRemoteListFromOrchestrator(
 	switch resp.StatusCode {
 	case http.StatusOK:
 		logger.Debug("HTTP response status code from Orchestrator")
-
-		if bResp, err = ioutil.ReadAll(resp.Body); err != nil {
-			break
-		}
-
+		bResp, err = ioutil.ReadAll(resp.Body)
 	case http.StatusNotFound:
-		logger.Error("HTTP response from Orchestrator: 'Idle mode'")
+		err = fmt.Errorf("HTTP response from Orchestrator: 'Idle mode'")
 	case http.StatusBadRequest:
-		logger.Error("HTTP response from Orchestrator: 'Please specify hostname or DC!'")
+		err = fmt.Errorf("HTTP response from Orchestrator: 'Please specify hostname or DC!'")
 	case http.StatusInternalServerError:
 		logger.Warn("HTTP response from Orchestrator: Error opening requested configuration file")
 	default:
-		logger.Error("unhandled HTTP response from Orchestrator")
 		err = fmt.Errorf("unhandled HTTP response from Orchestrator")
 	}
 
@@ -448,8 +443,7 @@ func readRemoteList(
 	c := new(RemoteFileConfig)
 
 	if err := json.Unmarshal(raw, c); err != nil {
-		logger.Error("configuration file parse error", zap.Error(err))
-		return err
+		return fmt.Errorf("configuration file parse error (%v)", err)
 	}
 
 	// Populate global variables
@@ -475,15 +469,15 @@ func readRemoteList(
 		srcIP, err = network.GetSourceAddr("ip6", strings.ToLower(c.Local.SrcAddress),
 			glRC.HostName, glRC.InterfaceName, logger)
 		if err != nil {
-			return fmt.Errorf("could not retrieve an IPv4 or IPv6 source address")
+			return fmt.Errorf("could not retrieve an IPv4 or IPv6 source address (%v)", err)
 		}
 	}
 	glRC.SrcAddress = *srcIP
 	logger.Debug("Arachne agent's source IP address", zap.Object("address", glRC.SrcAddress))
 
 	glRC.TargetTCPPort = c.Local.TargetTCPPort
 	if glRC.Timeout, err = time.ParseDuration(c.Local.Timeout); err != nil {
-		return fmt.Errorf("failed to parse the timeout")
+		return fmt.Errorf("failed to parse the timeout (%v)", err)
 	}
 	glRC.SrcTCPPortRange[0] = c.Local.BaseSrcTCPPort
 	if c.Local.NumSrcTCPPorts > maxNumSrcTCPPorts {
@@ -498,18 +492,18 @@ func readRemoteList(
 			"source ports [%d-%d]", glRC.SrcTCPPortRange[0], glRC.SrcTCPPortRange[1])
 	}
 	if glRC.BatchInterval, err = time.ParseDuration(c.Local.BatchInterval); err != nil {
-		return fmt.Errorf("failed to parse the batch interval")
+		return fmt.Errorf("failed to parse the batch interval (%v)", err)
 	}
 	if glRC.BatchInterval < minBatchInterval {
 		return fmt.Errorf("the batch cycle interval cannot be shorter than %v", minBatchInterval)
 	}
 	if glRC.PollOrchestratorInterval.Success, err =
 		time.ParseDuration(c.Local.PollOrchestratorIntervalSuccess); err != nil {
-		return fmt.Errorf("failed to parse the Orchestrator poll interval for success")
+		return fmt.Errorf("failed to parse the Orchestrator poll interval for success (%v)", err)
 	}
 	if glRC.PollOrchestratorInterval.Failure, err =
 		time.ParseDuration(c.Local.PollOrchestratorIntervalFailure); err != nil {
-		return fmt.Errorf("failed to parse the Orchestrator poll interval for failure")
+		return fmt.Errorf("failed to parse the Orchestrator poll interval for failure (%v)", err)
 	}
 
 	glRC.QoSEnabled = isTrue(c.Local.QoSEnabled)
@@ -537,7 +531,7 @@ func readRemoteList(
 	return nil
 }
 
-// Validate and create map of ipv4 and ipv6 addresses with string as their key
+// Validate and create map of ipv4 and ipv6 addresses with string as their key.
 func walkTargets(grc *RemoteConfig, ts []target, ext bool, remotes RemoteStore, logger zap.Logger) {
 
 	for _, t := range ts {
@@ -563,7 +557,8 @@ func walkTargets(grc *RemoteConfig, ts []target, ext bool, remotes RemoteStore,
 				zap.String("target", currIP.String()))
 			continue
 		}
-		remotes[currIP.String()] = Remote{currIP, network.Family(&currIP), t.HostName, ext}
+		remotes[currIP.String()] = Remote{currIP, network.Family(&currIP),
+			t.HostName, ext}
 	}
 }
 

internal/log/log.go

@@ -44,19 +44,19 @@ type Config struct {
 	LogSink string    `yaml:"logSink"`
 }
 
-// Fatal extends the zap Fatal to also remove the PID file
+// Fatal extends the zap Fatal to also remove the PID file.
 func (log PIDRemoverLogger) Fatal(msg string, fields ...zap.Field) {
 	log.RemovePID(log.PIDPath, log.Logger)
 	log.Logger.Fatal(msg, fields...)
 }
 
-// Panic extends the zap Panic to also remove the PID file
+// Panic extends the zap Panic to also remove the PID file.
 func (log PIDRemoverLogger) Panic(msg string, fields ...zap.Field) {
 	log.RemovePID(log.PIDPath, log.Logger)
 	log.Logger.Fatal(msg, fields...)
 }
 
-// CreateLogger creates a zap logger
+// CreateLogger creates a zap logger.
 func CreateLogger(
 	c *Config,
 	service string,
@@ -101,7 +101,7 @@ func CreateLogger(
 }
 
 // ResetLogFile keeps the last 'LogFileSizeKeepKB' KB of the log file if the size of the log file
-// has exceeded 'LogFileSizeMaxMB' MB within the last 'PollOrchestratorIntervalSuccess' hours
+// has exceeded 'LogFileSizeMaxMB' MB within the last 'PollOrchestratorIntervalSuccess' hours.
 func ResetLogFile(logFilePath string, fileSizeMaxMB int, fileSizeKeepKB int, logger zap.Logger) error {
 	file, err := os.Open(logFilePath)
 	if err != nil {

internal/network/network.go

@@ -63,7 +63,7 @@ func GetSourceAddr(
 	return anyInterfaceAddress(af)
 }
 
-// Resolve given domain hostname/address in the given address family
+// Resolve given domain hostname/address in the given address family.
 //TODO replace with net.LookupHost?
 func resolveHost(af string, hostname string, logger zap.Logger) (*net.IP, error) {
 	addr, err := net.ResolveIPAddr(af, hostname)