Code review comment for lp:~jameinel/juju-core/ensure-availability-default-3-1311083

Reviewers: mp+216706_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/juju/ensureavailability: default to 3 servers

I filed bug #1311083 because we currently have a default of '-1' and we
immediately fail when running the command. I don't think this is very
good UI for a user. I think we can make an opinionated statement that
n=3 is going to be the best for someone who wants to run this command.
And that is what this patch does.

https://code.launchpad.net/~jameinel/juju-core/ensure-availability-default-3-1311083/+merge/216706

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/90160044/

Affected files (+13, -5 lines):
   A [revision details]
   M cmd/juju/ensureavailability.go
   M cmd/juju/ensureavailability_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140422051657-1svpzqvmysbn4tqm
+New revision: <email address hidden>

Index: cmd/juju/ensureavailability.go
=== modified file 'cmd/juju/ensureavailability.go'
--- cmd/juju/ensureavailability.go 2014-04-01 04:53:43 +0000
+++ cmd/juju/ensureavailability.go 2014-04-22 12:28:07 +0000
@@ -33,7 +33,7 @@
  An odd number of state servers is required.

  Examples:
- juju ensure-availability -n 3
+ juju ensure-availability
       Ensure that 3 state servers are available,
       with newly created state server machines
       having the default series and constraints.
@@ -58,7 +58,7 @@

  func (c *EnsureAvailabilityCommand) SetFlags(f *gnuflag.FlagSet) {
   c.EnvCommandBase.SetFlags(f)
- f.IntVar(&c.NumStateServers, "n", -1, "number of state servers to make
available")
+ f.IntVar(&c.NumStateServers, "n", 3, "number of state servers to make
available")
   f.StringVar(&c.Series, "series", "", "the charm series")

f.Var(constraints.ConstraintsValue{&c.Constraints}, "constraints", "additional
machine constraints")
  }

Index: cmd/juju/ensureavailability_test.go
=== modified file 'cmd/juju/ensureavailability_test.go'
--- cmd/juju/ensureavailability_test.go 2014-04-10 13:00:21 +0000
+++ cmd/juju/ensureavailability_test.go 2014-04-22 12:28:07 +0000
@@ -114,14 +114,20 @@
  }

  func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityErrors(c *gc.C) {
- err := runEnsureAvailability(c)
- c.Assert(err, gc.ErrorMatches, "must specify a number of state servers
odd and greater than zero")
   for _, n := range []int{-1, 0, 2} {
    err := runEnsureAvailability(c, "-n", fmt.Sprint(n))
    c.Assert(err, gc.ErrorMatches, "must specify a number of state servers
odd and greater than zero")
   }
- err = runEnsureAvailability(c, "-n", "3")
+ err := runEnsureAvailability(c, "-n", "3")
   c.Assert(err, gc.IsNil)
   err = runEnsureAvailability(c, "-n", "1")
   c.Assert(err, gc.ErrorMatches, "cannot reduce state server count")
  }
+
+func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityDefaultsTo3(c
*gc.C) {
+ err := runEnsureAvailability(c)
+ c.Assert(err, gc.IsNil)
+ machines, err := s.State.AllMachines()
+ c.Assert(err, gc.IsNil)
+ c.Assert(machines, gc.HasLen, 3)
+}