Fixes #979: Adds code to check the CLI and config file port definitio…

…ns (#1014)

* Fixes #979: Adds code to check the CLI and config file and ensure the port is defined consistently

* Fixes #1016: Modifies code to not use default values to resolve this issue

control/flags.go

@@ -19,19 +19,21 @@ limitations under the License.
 
 package control
 
-import "github.com/codegangsta/cli"
+import (
+	"fmt"
+
+	"github.com/codegangsta/cli"
+)
 
 var (
-	flNumberOfPLs = cli.IntFlag{
+	flNumberOfPLs = cli.StringFlag{
 		Name:   "max-running-plugins, m",
-		Value:  defaultMaxRunningPlugins,
-		Usage:  "The maximum number of instances of a loaded plugin to run",
+		Usage:  fmt.Sprintf("The maximum number of instances of a loaded plugin to run (default: %v)", defaultMaxRunningPlugins),
 		EnvVar: "SNAP_MAX_PLUGINS",
 	}
-	flPluginTrust = cli.IntFlag{
+	flPluginTrust = cli.StringFlag{
 		Name:   "plugin-trust, t",
-		Usage:  "0-2 (Disabled, Enabled, Warning)",
-		Value:  defaultPluginTrust,
+		Usage:  fmt.Sprintf("0-2 (Disabled, Enabled, Warning; default: %v)", defaultPluginTrust),
 		EnvVar: "SNAP_TRUST_LEVEL",
 	}
 
@@ -45,16 +47,15 @@ var (
 		Usage:  "Keyring paths for signing verification separated by colons",
 		EnvVar: "SNAP_KEYRING_PATHS",
 	}
-	flCache = cli.DurationFlag{
+	flCache = cli.StringFlag{
 		Name:   "cache-expiration",
-		Usage:  "The time limit for which a metric cache entry is valid",
-		Value:  defaultCacheExpiration,
+		Usage:  fmt.Sprintf("The time limit for which a metric cache entry is valid (default: %v)", defaultCacheExpiration),
 		EnvVar: "SNAP_CACHE_EXPIRATION",
 	}
 
-	flControlRpcPort = cli.IntFlag{
+	flControlRpcPort = cli.StringFlag{
 		Name:   "control-listen-port",
-		Usage:  "Listen port for control RPC server",
+		Usage:  fmt.Sprintf("Listen port for control RPC server (default: %v)", defaultListenPort),
 		EnvVar: "SNAP_CONTROL_LISTEN_PORT",
 	}
 

mgmt/rest/client/client_func_test.go

@@ -93,7 +93,7 @@ func startAPI() string {
 		}
 		log.Fatal(err)
 	}(r.Err())
-	r.SetAddress("127.0.0.1", 0)
+	r.SetAddress("127.0.0.1:0")
 	r.Start()
 	time.Sleep(100 * time.Millisecond)
 	return fmt.Sprintf("http://localhost:%d", r.Port())

mgmt/rest/client/client_tribe_func_test.go

@@ -202,7 +202,7 @@ func startTribes(count int) []int {
 		r.BindMetricManager(c)
 		r.BindTaskManager(s)
 		r.BindTribeManager(t)
-		r.SetAddress("", mgtPort)
+		r.SetAddress(fmt.Sprintf("127.0.0.1:%d", mgtPort))
 		r.Start()
 		wg.Add(1)
 		timer := time.After(10 * time.Second)

mgmt/rest/flags.go

@@ -39,10 +39,9 @@ var (
 		Usage:  "API Address[:port] to bind to/listen on. Default: empty string => listen on all interfaces",
 		EnvVar: "SNAP_ADDR",
 	}
-	flAPIPort = cli.IntFlag{
+	flAPIPort = cli.StringFlag{
 		Name:   "api-port, p",
-		Usage:  fmt.Sprintf("API port (Default: %d)", defaultPort),
-		Value:  defaultPort,
+		Usage:  fmt.Sprintf("API port (default: %v)", defaultPort),
 		EnvVar: "SNAP_PORT",
 	}
 	flRestHTTPS = cli.BoolFlag{

mgmt/rest/rest_func_test.go

@@ -490,7 +490,7 @@ func startAPI(cfg *mockConfig) *restAPIInstance {
 		}
 		log.Fatal(err)
 	}(r.Err())
-	r.SetAddress("127.0.0.1", 0)
+	r.SetAddress("127.0.0.1:0")
 	r.Start()
 	time.Sleep(time.Millisecond * 100)
 	return &restAPIInstance{

mgmt/rest/server.go

@@ -56,6 +56,7 @@ const (
 	defaultRestKey         string = ""
 	defaultAuth            bool   = false
 	defaultAuthPassword    string = ""
+	defaultPortSetByConfig bool   = false
 )
 
 var (
@@ -78,6 +79,7 @@ type Config struct {
 	RestKey          string `json:"rest_key"yaml:"rest_key"`
 	RestAuth         bool   `json:"rest_auth"yaml:"rest_auth"`
 	RestAuthPassword string `json:"rest_auth_password"yaml:"rest_auth_password"`
+	portSetByConfig  bool   ``
 }
 
 const (
@@ -219,9 +221,16 @@ func GetDefaultConfig() *Config {
 		RestKey:          defaultRestKey,
 		RestAuth:         defaultAuth,
 		RestAuthPassword: defaultAuthPassword,
+		portSetByConfig:  defaultPortSetByConfig,
 	}
 }
 
+// define a method that can be used to determine if the port the RESTful
+// API is listening on was set in the configuration file
+func (c *Config) PortSetByConfigFile() bool {
+	return c.portSetByConfig
+}
+
 // UnmarshalJSON unmarshals valid json into a Config.  An example Config can be found
 // at github.com/intelsdi-x/snap/blob/master/examples/configs/snap-config-sample.json
 func (c *Config) UnmarshalJSON(data []byte) error {
@@ -244,6 +253,7 @@ func (c *Config) UnmarshalJSON(data []byte) error {
 			if err := json.Unmarshal(v, &(c.Port)); err != nil {
 				return fmt.Errorf("%v (while parsing 'restapi::port')", err)
 			}
+			c.portSetByConfig = true
 		case "addr":
 			if err := json.Unmarshal(v, &(c.Address)); err != nil {
 				return fmt.Errorf("%v (while parsing 'restapi::addr')", err)
@@ -303,23 +313,13 @@ func (s *Server) authMiddleware(rw http.ResponseWriter, r *http.Request, next ht
 	}
 }
 
-func (s *Server) Name() string {
-	return "REST"
+func (s *Server) SetAddress(addrString string) {
+	s.addrString = addrString
+	restLogger.Info(fmt.Sprintf("Address used for binding: [%v]", s.addrString))
 }
 
-func (s *Server) SetAddress(addrString string, dfltPort int) {
-	restLogger.Info(fmt.Sprintf("Setting address to: [%v] Default port: %v", addrString, dfltPort))
-	// In the future, we could extend this to support multiple comma separated IP[:port] values
-	if strings.Index(addrString, ",") != -1 {
-		restLogger.Fatal("Invalid address")
-	}
-	// If no port is specified, use default port
-	if strings.Index(addrString, ":") != -1 {
-		s.addrString = addrString
-	} else {
-		s.addrString = fmt.Sprintf("%s:%d", addrString, dfltPort)
-	}
-	restLogger.Info(fmt.Sprintf("Address used for binding: [%v]", s.addrString))
+func (s *Server) Name() string {
+	return "REST"
 }
 
 func (s *Server) Start() error {

mgmt/rest/tribe_test.go

@@ -702,7 +702,7 @@ func startTribes(count int, seed string) ([]int, int) {
 		r.BindMetricManager(c)
 		r.BindTaskManager(s)
 		r.BindTribeManager(t)
-		r.SetAddress("", mgtPort)
+		r.SetAddress(fmt.Sprintf("127.0.0.1:%d", mgtPort))
 		r.Start()
 		wg.Add(1)
 		timer := time.After(10 * time.Second)

mgmt/tribe/flags.go

@@ -19,7 +19,11 @@ limitations under the License.
 
 package tribe
 
-import "github.com/codegangsta/cli"
+import (
+	"fmt"
+
+	"github.com/codegangsta/cli"
+)
 
 var (
 	flTribeNodeName = cli.StringFlag{
@@ -40,10 +44,9 @@ var (
 		EnvVar: "SNAP_TRIBE_SEED",
 	}
 
-	flTribeAdvertisePort = cli.IntFlag{
+	flTribeAdvertisePort = cli.StringFlag{
 		Name:   "tribe-port",
-		Usage:  "Port tribe gossips over to maintain membership",
-		Value:  defaultBindPort,
+		Usage:  fmt.Sprintf("Port tribe gossips over to maintain membership (default: %v)", defaultBindPort),
 		EnvVar: "SNAP_TRIBE_PORT",
 	}
 

scheduler/flags.go

@@ -26,17 +26,15 @@ import (
 )
 
 var (
-	flSchedulerQueueSize = cli.IntFlag{
+	flSchedulerQueueSize = cli.StringFlag{
 		Name:   "work-manager-queue-size",
-		Usage:  fmt.Sprintf("Size of the work manager queue (default: %d)", defaultWorkManagerQueueSize),
-		Value:  int(defaultWorkManagerQueueSize),
+		Usage:  fmt.Sprintf("Size of the work manager queue (default: %v)", defaultWorkManagerQueueSize),
 		EnvVar: "WORK_MANAGER_QUEUE_SIZE",
 	}
 
-	flSchedulerPoolSize = cli.IntFlag{
+	flSchedulerPoolSize = cli.StringFlag{
 		Name:   "work-manager-pool-size",
-		Usage:  fmt.Sprintf("Size of the work manager pool (default %d)", defaultWorkManagerPoolSize),
-		Value:  int(defaultWorkManagerPoolSize),
+		Usage:  fmt.Sprintf("Size of the work manager pool (default: %v)", defaultWorkManagerPoolSize),
 		EnvVar: "WORK_MANAGER_POOL_SIZE",
 	}