Merge pull request #23703 from tiborvass/carry-23694

Carry 23694: Default port for listen addr

api/client/swarm/init.go

@@ -22,7 +22,7 @@ type initOptions struct {
 func newInitCommand(dockerCli *client.DockerCli) *cobra.Command {
 	var flags *pflag.FlagSet
 	opts := initOptions{
-		listenAddr: NewNodeAddrOption(),
+		listenAddr: NewListenAddrOption(),
 		autoAccept: NewAutoAcceptOption(),
 	}
 
@@ -36,9 +36,9 @@ func newInitCommand(dockerCli *client.DockerCli) *cobra.Command {
 	}
 
 	flags = cmd.Flags()
-	flags.Var(&opts.listenAddr, "listen-addr", "Listen address")
-	flags.Var(&opts.autoAccept, "auto-accept", "Auto acceptance policy (worker, manager, or none)")
-	flags.StringVar(&opts.secret, "secret", "", "Set secret value needed to accept nodes into cluster")
+	flags.Var(&opts.listenAddr, flagListenAddr, "Listen address")
+	flags.Var(&opts.autoAccept, flagAutoAccept, "Auto acceptance policy (worker, manager, or none)")
+	flags.StringVar(&opts.secret, flagSecret, "", "Set secret value needed to accept nodes into cluster")
 	flags.BoolVar(&opts.forceNewCluster, "force-new-cluster", false, "Force create a new cluster from current state.")
 	return cmd
 }

api/client/swarm/join.go

@@ -20,7 +20,7 @@ type joinOptions struct {
 
 func newJoinCommand(dockerCli *client.DockerCli) *cobra.Command {
 	opts := joinOptions{
-		listenAddr: NodeAddrOption{addr: defaultListenAddr},
+		listenAddr: NewListenAddrOption(),
 	}
 
 	cmd := &cobra.Command{
@@ -34,7 +34,7 @@ func newJoinCommand(dockerCli *client.DockerCli) *cobra.Command {
 	}
 
 	flags := cmd.Flags()
-	flags.Var(&opts.listenAddr, "listen-addr", "Listen address")
+	flags.Var(&opts.listenAddr, flagListenAddr, "Listen address")
 	flags.BoolVar(&opts.manager, "manager", false, "Try joining as a manager.")
 	flags.StringVar(&opts.secret, "secret", "", "Secret for node acceptance")
 	flags.StringVar(&opts.CACertHash, "ca-hash", "", "Hash of the Root Certificate Authority certificate used for trusted join")

api/client/swarm/opts.go

@@ -2,17 +2,27 @@ package swarm
 
 import (
 	"fmt"
+	"net"
+	"strconv"
 	"strings"
 
 	"github.com/docker/engine-api/types/swarm"
 )
 
 const (
-	defaultListenAddr = "0.0.0.0:2377"
+	defaultListenAddr        = "0.0.0.0"
+	defaultListenPort uint16 = 2377
 	// WORKER constant for worker name
 	WORKER = "WORKER"
 	// MANAGER constant for manager name
 	MANAGER = "MANAGER"
+
+	flagAutoAccept          = "auto-accept"
+	flagCertExpiry          = "cert-expiry"
+	flagDispatcherHeartbeat = "dispatcher-heartbeat"
+	flagListenAddr          = "listen-addr"
+	flagSecret              = "secret"
+	flagTaskHistoryLimit    = "task-history-limit"
 )
 
 var (
@@ -25,25 +35,35 @@ var (
 // NodeAddrOption is a pflag.Value for listen and remote addresses
 type NodeAddrOption struct {
 	addr string
+	port uint16
 }
 
 // String prints the representation of this flag
 func (a *NodeAddrOption) String() string {
-	return a.addr
+	return a.Value()
 }
 
 // Set the value for this flag
 func (a *NodeAddrOption) Set(value string) error {
 	if !strings.Contains(value, ":") {
-		return fmt.Errorf("Invalid url, a host and port are required")
+		a.addr = value
+		return nil
 	}
 
-	parts := strings.Split(value, ":")
-	if len(parts) != 2 {
-		return fmt.Errorf("Invalid url, too many colons")
+	host, port, err := net.SplitHostPort(value)
+	if err != nil {
+		return fmt.Errorf("Invalid url, %v", err)
 	}
 
-	a.addr = value
+	portInt, err := strconv.ParseUint(port, 10, 16)
+	if err != nil {
+		return fmt.Errorf("invalid url, %v", err)
+	}
+	a.port = uint16(portInt)
+
+	if host != "" {
+		a.addr = host
+	}
 	return nil
 }
 
@@ -52,9 +72,19 @@ func (a *NodeAddrOption) Type() string {
 	return "node-addr"
 }
 
+// Value returns the value of this option as addr:port
+func (a *NodeAddrOption) Value() string {
+	return net.JoinHostPort(a.addr, strconv.Itoa(int(a.port)))
+}
+
 // NewNodeAddrOption returns a new node address option
-func NewNodeAddrOption() NodeAddrOption {
-	return NodeAddrOption{addr: defaultListenAddr}
+func NewNodeAddrOption(host string, port uint16) NodeAddrOption {
+	return NodeAddrOption{addr: host, port: port}
+}
+
+// NewListenAddrOption returns a NodeAddrOption with default values
+func NewListenAddrOption() NodeAddrOption {
+	return NewNodeAddrOption(defaultListenAddr, defaultListenPort)
 }
 
 // AutoAcceptOption is a value type for auto-accept policy
@@ -65,10 +95,10 @@ type AutoAcceptOption struct {
 // String prints a string representation of this option
 func (o *AutoAcceptOption) String() string {
 	keys := []string{}
-	for key := range o.values {
-		keys = append(keys, key)
+	for key, value := range o.values {
+		keys = append(keys, fmt.Sprintf("%s=%v", strings.ToLower(key), value))
 	}
-	return strings.Join(keys, " ")
+	return strings.Join(keys, ", ")
 }
 
 // Set sets a new value on this option

api/client/swarm/opts_test.go

@@ -0,0 +1,118 @@
+package swarm
+
+import (
+	"testing"
+
+	"github.com/docker/docker/pkg/testutil/assert"
+	"github.com/docker/engine-api/types/swarm"
+)
+
+func TestNodeAddrOptionSetHostAndPort(t *testing.T) {
+	opt := NewNodeAddrOption("old", 123)
+	addr := "newhost:5555"
+	assert.NilError(t, opt.Set(addr))
+	assert.Equal(t, opt.addr, "newhost")
+	assert.Equal(t, opt.port, uint16(5555))
+	assert.Equal(t, opt.Value(), addr)
+}
+
+func TestNodeAddrOptionSetHostOnly(t *testing.T) {
+	opt := NewListenAddrOption()
+	assert.NilError(t, opt.Set("newhost"))
+	assert.Equal(t, opt.addr, "newhost")
+	assert.Equal(t, opt.port, defaultListenPort)
+}
+
+func TestNodeAddrOptionSetPortOnly(t *testing.T) {
+	opt := NewListenAddrOption()
+	assert.NilError(t, opt.Set(":4545"))
+	assert.Equal(t, opt.addr, defaultListenAddr)
+	assert.Equal(t, opt.port, uint16(4545))
+}
+
+func TestNodeAddrOptionSetInvalidFormat(t *testing.T) {
+	opt := NewListenAddrOption()
+	assert.Error(t, opt.Set("http://localhost:4545"), "Invalid url")
+}
+
+func TestAutoAcceptOptionSetWorker(t *testing.T) {
+	opt := NewAutoAcceptOption()
+	assert.NilError(t, opt.Set("worker"))
+	assert.Equal(t, opt.values[WORKER], true)
+}
+
+func TestAutoAcceptOptionSetManager(t *testing.T) {
+	opt := NewAutoAcceptOption()
+	assert.NilError(t, opt.Set("manager"))
+	assert.Equal(t, opt.values[MANAGER], true)
+}
+
+func TestAutoAcceptOptionSetInvalid(t *testing.T) {
+	opt := NewAutoAcceptOption()
+	assert.Error(t, opt.Set("bogus"), "must be one of")
+}
+
+func TestAutoAcceptOptionSetNone(t *testing.T) {
+	opt := NewAutoAcceptOption()
+	assert.NilError(t, opt.Set("none"))
+	assert.Equal(t, opt.values[MANAGER], false)
+	assert.Equal(t, opt.values[WORKER], false)
+}
+
+func TestAutoAcceptOptionSetConflict(t *testing.T) {
+	opt := NewAutoAcceptOption()
+	assert.NilError(t, opt.Set("manager"))
+	assert.Error(t, opt.Set("none"), "value NONE is incompatible with MANAGER")
+
+	opt = NewAutoAcceptOption()
+	assert.NilError(t, opt.Set("none"))
+	assert.Error(t, opt.Set("worker"), "value NONE is incompatible with WORKER")
+}
+
+func TestAutoAcceptOptionPoliciesDefault(t *testing.T) {
+	opt := NewAutoAcceptOption()
+	secret := "thesecret"
+
+	policies := opt.Policies(&secret)
+	assert.Equal(t, len(policies), 2)
+	assert.Equal(t, policies[0], swarm.Policy{
+		Role:       WORKER,
+		Autoaccept: true,
+		Secret:     &secret,
+	})
+	assert.Equal(t, policies[1], swarm.Policy{
+		Role:       MANAGER,
+		Autoaccept: false,
+		Secret:     &secret,
+	})
+}
+
+func TestAutoAcceptOptionPoliciesWithManager(t *testing.T) {
+	opt := NewAutoAcceptOption()
+	secret := "thesecret"
+
+	assert.NilError(t, opt.Set("manager"))
+
+	policies := opt.Policies(&secret)
+	assert.Equal(t, len(policies), 2)
+	assert.Equal(t, policies[0], swarm.Policy{
+		Role:       WORKER,
+		Autoaccept: false,
+		Secret:     &secret,
+	})
+	assert.Equal(t, policies[1], swarm.Policy{
+		Role:       MANAGER,
+		Autoaccept: true,
+		Secret:     &secret,
+	})
+}
+
+func TestAutoAcceptOptionString(t *testing.T) {
+	opt := NewAutoAcceptOption()
+	assert.NilError(t, opt.Set("manager"))
+	assert.NilError(t, opt.Set("worker"))
+
+	repr := opt.String()
+	assert.Contains(t, repr, "worker=true")
+	assert.Contains(t, repr, "manager=true")
+}