Merge pull request #24146 from johnharris85/fix-swarm-update-auto-accept

Add comma-separated --auto-accept support.

api/client/swarm/opts.go

@@ -14,10 +14,10 @@ import (
 
 const (
 	defaultListenAddr = "0.0.0.0:2377"
-	// WORKER constant for worker name
-	WORKER = "WORKER"
-	// MANAGER constant for manager name
-	MANAGER = "MANAGER"
+
+	worker  = "WORKER"
+	manager = "MANAGER"
+	none    = "NONE"
 
 	flagAutoAccept          = "auto-accept"
 	flagCertExpiry          = "cert-expiry"
@@ -30,8 +30,8 @@ const (
 
 var (
 	defaultPolicies = []swarm.Policy{
-		{Role: WORKER, Autoaccept: true},
-		{Role: MANAGER, Autoaccept: false},
+		{Role: worker, Autoaccept: true},
+		{Role: manager, Autoaccept: false},
 	}
 )
 
@@ -86,40 +86,33 @@ func NewListenAddrOption() NodeAddrOption {
 
 // AutoAcceptOption is a value type for auto-accept policy
 type AutoAcceptOption struct {
-	values map[string]bool
+	values map[string]struct{}
 }
 
 // String prints a string representation of this option
 func (o *AutoAcceptOption) String() string {
 	keys := []string{}
-	for key, value := range o.values {
-		keys = append(keys, fmt.Sprintf("%s=%v", strings.ToLower(key), value))
+	for key := range o.values {
+		keys = append(keys, fmt.Sprintf("%s=true", strings.ToLower(key)))
 	}
 	return strings.Join(keys, ", ")
 }
 
 // Set sets a new value on this option
-func (o *AutoAcceptOption) Set(value string) error {
-	value = strings.ToUpper(value)
-	switch value {
-	case "", "NONE":
-		if accept, ok := o.values[WORKER]; ok && accept {
-			return fmt.Errorf("value NONE is incompatible with %s", WORKER)
-		}
-		if accept, ok := o.values[MANAGER]; ok && accept {
-			return fmt.Errorf("value NONE is incompatible with %s", MANAGER)
-		}
-		o.values[WORKER] = false
-		o.values[MANAGER] = false
-	case WORKER, MANAGER:
-		if accept, ok := o.values[value]; ok && !accept {
-			return fmt.Errorf("value NONE is incompatible with %s", value)
+func (o *AutoAcceptOption) Set(acceptValues string) error {
+	for _, value := range strings.Split(acceptValues, ",") {
+		value = strings.ToUpper(value)
+		switch value {
+		case none, worker, manager:
+			o.values[value] = struct{}{}
+		default:
+			return fmt.Errorf("must be one / combination of %s, %s; or NONE", worker, manager)
 		}
-		o.values[value] = true
-	default:
-		return fmt.Errorf("must be one of %s, %s, NONE", WORKER, MANAGER)
 	}
-
+	// NONE must stand alone, so if any non-NONE setting exist with it, error with conflict
+	if o.isPresent(none) && len(o.values) > 1 {
+		return fmt.Errorf("value NONE cannot be specified alongside other node types")
+	}
 	return nil
 }
 
@@ -133,7 +126,11 @@ func (o *AutoAcceptOption) Policies(secret *string) []swarm.Policy {
 	policies := []swarm.Policy{}
 	for _, p := range defaultPolicies {
 		if len(o.values) != 0 {
-			p.Autoaccept = o.values[string(p.Role)]
+			if _, ok := o.values[string(p.Role)]; ok {
+				p.Autoaccept = true
+			} else {
+				p.Autoaccept = false
+			}
 		}
 		p.Secret = secret
 		policies = append(policies, p)
@@ -141,9 +138,15 @@ func (o *AutoAcceptOption) Policies(secret *string) []swarm.Policy {
 	return policies
 }
 
+// isPresent returns whether the key exists in the set or not
+func (o *AutoAcceptOption) isPresent(key string) bool {
+	_, c := o.values[key]
+	return c
+}
+
 // NewAutoAcceptOption returns a new auto-accept option
 func NewAutoAcceptOption() AutoAcceptOption {
-	return AutoAcceptOption{values: make(map[string]bool)}
+	return AutoAcceptOption{values: make(map[string]struct{})}
 }
 
 // ExternalCAOption is a Value type for parsing external CA specifications.

api/client/swarm/opts_test.go

@@ -40,35 +40,51 @@ func TestNodeAddrOptionSetInvalidFormat(t *testing.T) {
 func TestAutoAcceptOptionSetWorker(t *testing.T) {
 	opt := NewAutoAcceptOption()
 	assert.NilError(t, opt.Set("worker"))
-	assert.Equal(t, opt.values[WORKER], true)
+	assert.Equal(t, opt.isPresent(worker), true)
 }
 
 func TestAutoAcceptOptionSetManager(t *testing.T) {
 	opt := NewAutoAcceptOption()
 	assert.NilError(t, opt.Set("manager"))
-	assert.Equal(t, opt.values[MANAGER], true)
+	assert.Equal(t, opt.isPresent(manager), true)
 }
 
 func TestAutoAcceptOptionSetInvalid(t *testing.T) {
 	opt := NewAutoAcceptOption()
-	assert.Error(t, opt.Set("bogus"), "must be one of")
+	assert.Error(t, opt.Set("bogus"), "must be one / combination")
+}
+
+func TestAutoAcceptOptionSetEmpty(t *testing.T) {
+	opt := NewAutoAcceptOption()
+	assert.Error(t, opt.Set(""), "must be one / combination")
 }
 
 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)
+	assert.Equal(t, opt.isPresent(manager), false)
+	assert.Equal(t, opt.isPresent(worker), false)
+}
+
+func TestAutoAcceptOptionSetTwo(t *testing.T) {
+	opt := NewAutoAcceptOption()
+	assert.NilError(t, opt.Set("worker,manager"))
+	assert.Equal(t, opt.isPresent(manager), true)
+	assert.Equal(t, opt.isPresent(worker), true)
 }
 
 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")
+	assert.Error(t, opt.Set("none,manager"), "value NONE cannot be specified alongside other node types")
 
 	opt = NewAutoAcceptOption()
-	assert.NilError(t, opt.Set("none"))
-	assert.Error(t, opt.Set("worker"), "value NONE is incompatible with WORKER")
+	assert.Error(t, opt.Set("none,worker"), "value NONE cannot be specified alongside other node types")
+
+	opt = NewAutoAcceptOption()
+	assert.Error(t, opt.Set("worker,none,manager"), "value NONE cannot be specified alongside other node types")
+
+	opt = NewAutoAcceptOption()
+	assert.Error(t, opt.Set("worker,manager,none"), "value NONE cannot be specified alongside other node types")
 }
 
 func TestAutoAcceptOptionPoliciesDefault(t *testing.T) {
@@ -78,12 +94,12 @@ func TestAutoAcceptOptionPoliciesDefault(t *testing.T) {
 	policies := opt.Policies(&secret)
 	assert.Equal(t, len(policies), 2)
 	assert.Equal(t, policies[0], swarm.Policy{
-		Role:       WORKER,
+		Role:       worker,
 		Autoaccept: true,
 		Secret:     &secret,
 	})
 	assert.Equal(t, policies[1], swarm.Policy{
-		Role:       MANAGER,
+		Role:       manager,
 		Autoaccept: false,
 		Secret:     &secret,
 	})
@@ -98,12 +114,12 @@ func TestAutoAcceptOptionPoliciesWithManager(t *testing.T) {
 	policies := opt.Policies(&secret)
 	assert.Equal(t, len(policies), 2)
 	assert.Equal(t, policies[0], swarm.Policy{
-		Role:       WORKER,
+		Role:       worker,
 		Autoaccept: false,
 		Secret:     &secret,
 	})
 	assert.Equal(t, policies[1], swarm.Policy{
-		Role:       MANAGER,
+		Role:       manager,
 		Autoaccept: true,
 		Secret:     &secret,
 	})

docs/reference/commandline/swarm_init.md

@@ -17,7 +17,7 @@ Usage:  docker swarm init [OPTIONS]
 Initialize a Swarm
 
 Options:
-      --auto-accept value               Auto acceptance policy (worker, manager or none)
+      --auto-accept value               Auto acceptance policy (default worker)
       --cert-expiry duration            Validity period for node certificates (default 2160h0m0s)
       --dispatcher-heartbeat duration   Dispatcher heartbeat period (default 5s)
       --external-ca value               Specifications of one or more certificate signing endpoints
@@ -66,6 +66,13 @@ For example, the following initializes a cluster with auto-acceptance of workers
 $ docker swarm init --listen-addr 192.168.99.121:2377 --auto-accept worker
 ```
 
+It is possible to pass a comma-separated list of node types. The following initializes a cluster
+with auto-acceptance of both `worker` and `manager` nodes
+
+```bash
+$ docker swarm init --listen-addr 192.168.99.121:2377 --auto-accept worker,manager
+```
+
 ### `--cert-expiry`
 
 This flag sets the validity period for node certificates.