juju-core

Merge lp:~jameinel/juju-core/ensure-availability-default-3-1311083 into lp:~go-bot/juju-core/trunk

  1. ensure-availability-default-3-1311083
  2. Merge into trunk
Proposed by John A Meinel
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: 2670
Proposed branch: lp:~jameinel/juju-core/ensure-availability-default-3-1311083
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 474 lines (+232/-65)
6 files modified
cmd/juju/ensureavailability.go (+12/-13)
cmd/juju/ensureavailability_test.go (+56/-27)
state/addmachine.go (+13/-6)
state/apiserver/client/client.go (+17/-5)
state/apiserver/client/client_test.go (+78/-12)
state/state_test.go (+56/-2)
To merge this branch: bzr merge lp:~jameinel/juju-core/ensure-availability-default-3-1311083
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+216706@code.launchpad.net

Commit message

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.

The new default is not strictly '3' but to use 3 if you only have 1, and otherwise preserve whatever number of servers you already have. (which will start new servers to bring you back up to your requested availability.)

https://codereview.appspot.com/90160044/

Description of the change

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://codereview.appspot.com/90160044/

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :
Download full text (3.2 KiB)

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(...

Read more...

Revision history for this message
Andrew Wilkins (axwalk) wrote :

On 2014/04/22 12:32:29, jameinel wrote:
> Please take a look.

I originally just defaulted it to 1, but rog asked me to take out the
default so that we can, in the future, change the command to maintain
the current availability. That would enable a simple cron job to
periodically "juju ensure-availability", replacing unavailable state
servers and reinstating available ones.

https://codereview.appspot.com/90160044/

Revision history for this message
John A Meinel (jameinel) wrote :

On 2014/04/22 12:42:23, axw wrote:
> On 2014/04/22 12:32:29, jameinel wrote:
> > Please take a look.

> I originally just defaulted it to 1, but rog asked me to take out the
default so
> that we can, in the future, change the command to maintain the current
> availability. That would enable a simple cron job to periodically
"juju
> ensure-availability", replacing unavailable state servers and
reinstating
> available ones.

ensure-availability 1 can't ever do anything.

Because if 1 machine is dead, there is nobody to contact to set up
availability.
And if you pass 1 to a system that has 3, you get a message about "you
cannot reduce the number", because we would be trying to decrease from 3
to 1.

There is room to say that "ensure-availability" with nothing specified
is actually -1, which means "3 if you are currently running with only 1,
and N if you are currently running with N".

I would be happy to make that change, so that we allow -1 or 0 to be
passed up, and then interpret it according to the above rules.

But *quite honestly* I don't expect anyone to ever run with anything but
1 or 3, so making it overly flexible doesn't actually gain us much. (It
is possible that in a year or more we'll have found ways to scale beyond
3 productively, I don't think we're anywhere close to that yet).

https://codereview.appspot.com/90160044/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

On 2014/04/22 14:11:08, jameinel wrote:
> On 2014/04/22 12:42:23, axw wrote:
> > On 2014/04/22 12:32:29, jameinel wrote:
> > > Please take a look.
> >
> > I originally just defaulted it to 1, but rog asked me to take out
 the default
> so
> > that we can, in the future, change the command to maintain the
 current
> > availability. That would enable a simple cron job to periodically
 "juju
> > ensure-availability", replacing unavailable state servers and
 reinstating
> > available ones.

> ensure-availability 1 can't ever do anything.

> Because if 1 machine is dead, there is nobody to contact to set up
availability.
> And if you pass 1 to a system that has 3, you get a message about "you
cannot
> reduce the number", because we would be trying to decrease from 3 to
1.

That should be fixed eventually - we should allow people to reduce their
state server count.

> There is room to say that "ensure-availability" with nothing specified
is
> actually -1, which means "3 if you are currently running with only 1,
and N if
> you are currently running with N".

That was the plan.

> I would be happy to make that change, so that we allow -1 or 0 to be
passed up,
> and then interpret it according to the above rules.

SGTM

https://codereview.appspot.com/90160044/

Revision history for this message
Ian Booth (wallyworld) wrote :

LGTM

https://codereview.appspot.com/90160044/diff/1/cmd/juju/ensureavailability.go
File cmd/juju/ensureavailability.go (right):

https://codereview.appspot.com/90160044/diff/1/cmd/juju/ensureavailability.go#newcode36
cmd/juju/ensureavailability.go:36: juju ensure-availability
nitpick - add another example showing use of n?, eg
juju ensure-availability -n 5
Ensure that 5 state servers ....

https://codereview.appspot.com/90160044/

Revision history for this message
John A Meinel (jameinel) wrote :

Please take a look.

https://codereview.appspot.com/90160044/

Revision history for this message
Andrew Wilkins (axwalk) wrote :

On 2014/04/23 06:55:47, jameinel wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/90160044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cmd/juju/ensureavailability.go'
--- cmd/juju/ensureavailability.go 2014-04-01 04:53:43 +0000
+++ cmd/juju/ensureavailability.go 2014-04-23 06:53:26 +0000
@@ -33,18 +33,17 @@
33An odd number of state servers is required.33An odd number of state servers is required.
3434
35Examples:35Examples:
36 juju ensure-availability -n 336 juju ensure-availability
37 Ensure that 3 state servers are available,37 Ensure that the system is still in highly available mode. If
38 with newly created state server machines38 there is only 1 state server running, this will ensure there
39 having the default series and constraints.39 are 3 running. If you have previously requested more than 3,
40 then that number will be ensured.
40 juju ensure-availability -n 5 --series=trusty41 juju ensure-availability -n 5 --series=trusty
41 Ensure that 5 state servers are available,42 Ensure that 5 state servers are available, with newly created
42 with newly created state server machines43 state server machines having the "trusty" series.
43 having the "trusty" series.
44 juju ensure-availability -n 7 --constraints mem=8G44 juju ensure-availability -n 7 --constraints mem=8G
45 Ensure that 7 state servers are available,45 Ensure that 7 state servers are available, with newly created
46 with newly created state server machines46 state server machines having the default series, and at least
47 having the default series, and at least
48 8GB RAM.47 8GB RAM.
49`48`
5049
@@ -58,7 +57,7 @@
5857
59func (c *EnsureAvailabilityCommand) SetFlags(f *gnuflag.FlagSet) {58func (c *EnsureAvailabilityCommand) SetFlags(f *gnuflag.FlagSet) {
60 c.EnvCommandBase.SetFlags(f)59 c.EnvCommandBase.SetFlags(f)
61 f.IntVar(&c.NumStateServers, "n", -1, "number of state servers to make available")60 f.IntVar(&c.NumStateServers, "n", 0, "number of state servers to make available")
62 f.StringVar(&c.Series, "series", "", "the charm series")61 f.StringVar(&c.Series, "series", "", "the charm series")
63 f.Var(constraints.ConstraintsValue{&c.Constraints}, "constraints", "additional machine constraints")62 f.Var(constraints.ConstraintsValue{&c.Constraints}, "constraints", "additional machine constraints")
64}63}
@@ -68,8 +67,8 @@
68 if err != nil {67 if err != nil {
69 return err68 return err
70 }69 }
71 if c.NumStateServers%2 != 1 || c.NumStateServers <= 0 {70 if c.NumStateServers < 0 || (c.NumStateServers%2 != 1 && c.NumStateServers != 0) {
72 return fmt.Errorf("must specify a number of state servers odd and greater than zero")71 return fmt.Errorf("must specify a number of state servers odd and non-negative")
73 }72 }
74 return cmd.CheckEmpty(args)73 return cmd.CheckEmpty(args)
75}74}
7675
=== 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-23 06:53:26 +0000
@@ -12,17 +12,44 @@
12 "launchpad.net/juju-core/constraints"12 "launchpad.net/juju-core/constraints"
13 jujutesting "launchpad.net/juju-core/juju/testing"13 jujutesting "launchpad.net/juju-core/juju/testing"
14 "launchpad.net/juju-core/state"14 "launchpad.net/juju-core/state"
15 "launchpad.net/juju-core/testing"15 "launchpad.net/juju-core/state/presence"
16 coretesting "launchpad.net/juju-core/testing"
16)17)
1718
18type EnsureAvailabilitySuite struct {19type EnsureAvailabilitySuite struct {
19 jujutesting.RepoSuite20 jujutesting.RepoSuite
21 machine0Pinger *presence.Pinger
20}22}
2123
22var _ = gc.Suite(&EnsureAvailabilitySuite{})24var _ = gc.Suite(&EnsureAvailabilitySuite{})
2325
26func (s *EnsureAvailabilitySuite) SetUpTest(c *gc.C) {
27 s.RepoSuite.SetUpTest(c)
28 // Add a state server to the environment, and ensure that it is
29 // considered 'alive' so that calls don't spawn new instances
30 _, err := s.State.AddMachine("precise", state.JobManageEnviron)
31 c.Assert(err, gc.IsNil)
32 m, err := s.BackingState.Machine("0")
33 c.Assert(err, gc.IsNil)
34 s.machine0Pinger, err = m.SetAgentAlive()
35 c.Assert(err, gc.IsNil)
36 s.BackingState.StartSync()
37 err = m.WaitAgentAlive(coretesting.LongWait)
38 c.Assert(err, gc.IsNil)
39}
40
41func (s *EnsureAvailabilitySuite) TearDownTest(c *gc.C) {
42 // We have to Kill the Pinger before TearDownTest, otherwise the State
43 // connection is already closed.
44 if s.machine0Pinger != nil {
45 s.machine0Pinger.Kill()
46 s.machine0Pinger = nil
47 }
48 s.RepoSuite.TearDownTest(c)
49}
50
24func runEnsureAvailability(c *gc.C, args ...string) error {51func runEnsureAvailability(c *gc.C, args ...string) error {
25 _, err := testing.RunCommand(c, &EnsureAvailabilityCommand{}, args)52 _, err := coretesting.RunCommand(c, &EnsureAvailabilityCommand{}, args)
26 return err53 return err
27}54}
2855
@@ -39,17 +66,17 @@
39}66}
4067
41func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityWithSeries(c *gc.C) {68func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityWithSeries(c *gc.C) {
42 err := runEnsureAvailability(c, "--series", "series", "-n", "1")69 err := runEnsureAvailability(c, "--series", "series", "-n", "3")
43 c.Assert(err, gc.IsNil)70 c.Assert(err, gc.IsNil)
44 m, err := s.State.Machine("0")71 m, err := s.State.Machine("1")
45 c.Assert(err, gc.IsNil)72 c.Assert(err, gc.IsNil)
46 c.Assert(m.Series(), gc.DeepEquals, "series")73 c.Assert(m.Series(), gc.DeepEquals, "series")
47}74}
4875
49func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityWithConstraints(c *gc.C) {76func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityWithConstraints(c *gc.C) {
50 err := runEnsureAvailability(c, "--constraints", "mem=4G", "-n", "1")77 err := runEnsureAvailability(c, "--constraints", "mem=4G", "-n", "3")
51 c.Assert(err, gc.IsNil)78 c.Assert(err, gc.IsNil)
52 m, err := s.State.Machine("0")79 m, err := s.State.Machine("1")
53 c.Assert(err, gc.IsNil)80 c.Assert(err, gc.IsNil)
54 mcons, err := m.Constraints()81 mcons, err := m.Constraints()
55 c.Assert(err, gc.IsNil)82 c.Assert(err, gc.IsNil)
@@ -62,6 +89,9 @@
62 err := runEnsureAvailability(c, "-n", "1")89 err := runEnsureAvailability(c, "-n", "1")
63 c.Assert(err, gc.IsNil)90 c.Assert(err, gc.IsNil)
64 }91 }
92 machines, err := s.State.AllMachines()
93 c.Assert(err, gc.IsNil)
94 c.Assert(machines, gc.HasLen, 1)
65 m, err := s.State.Machine("0")95 m, err := s.State.Machine("0")
66 c.Assert(err, gc.IsNil)96 c.Assert(err, gc.IsNil)
67 mcons, err := m.Constraints()97 mcons, err := m.Constraints()
@@ -81,22 +111,7 @@
81}111}
82112
83func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityMultiple(c *gc.C) {113func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityMultiple(c *gc.C) {
84 err := runEnsureAvailability(c, "-n", "1")114 err := runEnsureAvailability(c, "-n", "3", "--constraints", "mem=4G")
85 c.Assert(err, gc.IsNil)
86
87 // make sure machine-0 remains alive for the second call to
88 // EnsureAvailability, or machine-0 will get bumped down to
89 // non-voting.
90 m0, err := s.BackingState.Machine("0")
91 c.Assert(err, gc.IsNil)
92 pinger, err := m0.SetAgentAlive()
93 c.Assert(err, gc.IsNil)
94 defer pinger.Kill()
95 s.BackingState.StartSync()
96 err = m0.WaitAgentAlive(testing.LongWait)
97 c.Assert(err, gc.IsNil)
98
99 err = runEnsureAvailability(c, "-n", "3", "--constraints", "mem=4G")
100 c.Assert(err, gc.IsNil)115 c.Assert(err, gc.IsNil)
101116
102 machines, err := s.State.AllMachines()117 machines, err := s.State.AllMachines()
@@ -114,14 +129,28 @@
114}129}
115130
116func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityErrors(c *gc.C) {131func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityErrors(c *gc.C) {
117 err := runEnsureAvailability(c)132 for _, n := range []int{-1, 2} {
118 c.Assert(err, gc.ErrorMatches, "must specify a number of state servers odd and greater than zero")
119 for _, n := range []int{-1, 0, 2} {
120 err := runEnsureAvailability(c, "-n", fmt.Sprint(n))133 err := runEnsureAvailability(c, "-n", fmt.Sprint(n))
121 c.Assert(err, gc.ErrorMatches, "must specify a number of state servers odd and greater than zero")134 c.Assert(err, gc.ErrorMatches, "must specify a number of state servers odd and non-negative")
122 }135 }
123 err = runEnsureAvailability(c, "-n", "3")136 err := runEnsureAvailability(c, "-n", "3")
124 c.Assert(err, gc.IsNil)137 c.Assert(err, gc.IsNil)
125 err = runEnsureAvailability(c, "-n", "1")138 err = runEnsureAvailability(c, "-n", "1")
126 c.Assert(err, gc.ErrorMatches, "cannot reduce state server count")139 c.Assert(err, gc.ErrorMatches, "cannot reduce state server count")
127}140}
141
142func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityAllows0(c *gc.C) {
143 err := runEnsureAvailability(c, "-n", "0")
144 c.Assert(err, gc.IsNil)
145 machines, err := s.State.AllMachines()
146 c.Assert(err, gc.IsNil)
147 c.Assert(machines, gc.HasLen, 3)
148}
149
150func (s *EnsureAvailabilitySuite) TestEnsureAvailabilityDefaultsTo3(c *gc.C) {
151 err := runEnsureAvailability(c)
152 c.Assert(err, gc.IsNil)
153 machines, err := s.State.AllMachines()
154 c.Assert(err, gc.IsNil)
155 c.Assert(machines, gc.HasLen, 3)
156}
128157
=== modified file 'state/addmachine.go'
--- state/addmachine.go 2014-04-21 23:10:05 +0000
+++ state/addmachine.go 2014-04-23 06:53:26 +0000
@@ -497,8 +497,8 @@
497// the number of live state servers equal to numStateServers. The given497// the number of live state servers equal to numStateServers. The given
498// constraints and series will be attached to any new machines.498// constraints and series will be attached to any new machines.
499func (st *State) EnsureAvailability(numStateServers int, cons constraints.Value, series string) error {499func (st *State) EnsureAvailability(numStateServers int, cons constraints.Value, series string) error {
500 if numStateServers%2 != 1 || numStateServers <= 0 {500 if numStateServers < 0 || (numStateServers != 0 && numStateServers%2 != 1) {
501 return fmt.Errorf("number of state servers must be odd and greater than zero")501 return fmt.Errorf("number of state servers must be odd and non-negative")
502 }502 }
503 if numStateServers > replicaset.MaxPeers {503 if numStateServers > replicaset.MaxPeers {
504 return fmt.Errorf("state server count is too large (allowed %d)", replicaset.MaxPeers)504 return fmt.Errorf("state server count is too large (allowed %d)", replicaset.MaxPeers)
@@ -508,7 +508,14 @@
508 if err != nil {508 if err != nil {
509 return err509 return err
510 }510 }
511 if len(currentInfo.VotingMachineIds) > numStateServers {511 desiredStateServerCount := numStateServers
512 if desiredStateServerCount == 0 {
513 desiredStateServerCount = len(currentInfo.VotingMachineIds)
514 if desiredStateServerCount <= 1 {
515 desiredStateServerCount = 3
516 }
517 }
518 if len(currentInfo.VotingMachineIds) > desiredStateServerCount {
512 return fmt.Errorf("cannot reduce state server count")519 return fmt.Errorf("cannot reduce state server count")
513 }520 }
514521
@@ -522,15 +529,15 @@
522 voteCount++529 voteCount++
523 }530 }
524 }531 }
525 if voteCount == numStateServers && len(intent.remove) == 0 {532 if voteCount == desiredStateServerCount && len(intent.remove) == 0 {
526 return nil533 return nil
527 }534 }
528 // Promote as many machines as we can to fulfil the shortfall.535 // Promote as many machines as we can to fulfil the shortfall.
529 if n := numStateServers - voteCount; n < len(intent.promote) {536 if n := desiredStateServerCount - voteCount; n < len(intent.promote) {
530 intent.promote = intent.promote[:n]537 intent.promote = intent.promote[:n]
531 }538 }
532 voteCount += len(intent.promote)539 voteCount += len(intent.promote)
533 intent.newCount = numStateServers - voteCount540 intent.newCount = desiredStateServerCount - voteCount
534 logger.Infof("%d new machines; promoting %v", intent.newCount, intent.promote)541 logger.Infof("%d new machines; promoting %v", intent.newCount, intent.promote)
535 ops, err := st.ensureAvailabilityIntentionOps(intent, currentInfo, cons, series)542 ops, err := st.ensureAvailabilityIntentionOps(intent, currentInfo, cons, series)
536 if err != nil {543 if err != nil {
537544
=== modified file 'state/apiserver/client/client.go'
--- state/apiserver/client/client.go 2014-04-17 12:53:23 +0000
+++ state/apiserver/client/client.go 2014-04-23 06:53:26 +0000
@@ -1044,11 +1044,23 @@
1044func (c *Client) EnsureAvailability(args params.EnsureAvailability) error {1044func (c *Client) EnsureAvailability(args params.EnsureAvailability) error {
1045 series := args.Series1045 series := args.Series
1046 if series == "" {1046 if series == "" {
1047 cfg, err := c.api.state.EnvironConfig()1047 ssi, err := c.api.state.StateServerInfo()
1048 if err != nil {1048 if err != nil {
1049 return err1049 return err
1050 }1050 }
1051 series = config.PreferredSeries(cfg)1051 // We should always have at least one voting machine
1052 // If we *really* wanted we could just pick whatever series is
1053 // in the majority, but really, if we always copy the value of
1054 // the first one, then they'll stay in sync.
1055 if len(ssi.VotingMachineIds) == 0 {
1056 // Better than a panic()?
1057 return fmt.Errorf("internal error, failed to find any voting machines")
1058 }
1059 templateMachine, err := c.api.state.Machine(ssi.VotingMachineIds[0])
1060 if err != nil {
1061 return err
1062 }
1063 series = templateMachine.Series()
1052 }1064 }
1053 return c.api.state.EnsureAvailability(args.NumStateServers, args.Constraints, series)1065 return c.api.state.EnsureAvailability(args.NumStateServers, args.Constraints, series)
1054}1066}
10551067
=== modified file 'state/apiserver/client/client_test.go'
--- state/apiserver/client/client_test.go 2014-04-17 12:53:23 +0000
+++ state/apiserver/client/client_test.go 2014-04-23 06:53:26 +0000
@@ -2143,27 +2143,51 @@
2143 return pinger2143 return pinger
2144}2144}
21452145
2146var (
2147 emptyCons = constraints.Value{}
2148 defaultSeries = ""
2149)
2150
2146func (s *clientSuite) TestClientEnsureAvailabilitySeries(c *gc.C) {2151func (s *clientSuite) TestClientEnsureAvailabilitySeries(c *gc.C) {
2147 err := s.APIState.Client().EnsureAvailability(1, constraints.Value{}, "")2152 _, err := s.State.AddMachine("quantal", state.JobManageEnviron)
2148 c.Assert(err, gc.IsNil)2153 c.Assert(err, gc.IsNil)
2154 // We have to ensure the agents are alive, or EnsureAvailability will
2155 // create more to replace them.
2149 pinger := s.setAgentAlive(c, "0")2156 pinger := s.setAgentAlive(c, "0")
2150 defer pinger.Kill()2157 defer pinger.Kill()
2151 err = s.APIState.Client().EnsureAvailability(3, constraints.Value{}, "non-default")
2152 c.Assert(err, gc.IsNil)
2153 machines, err := s.State.AllMachines()2158 machines, err := s.State.AllMachines()
2154 c.Assert(err, gc.IsNil)2159 c.Assert(err, gc.IsNil)
2160 c.Assert(machines, gc.HasLen, 1)
2161 c.Assert(machines[0].Series(), gc.Equals, "quantal")
2162 err = s.APIState.Client().EnsureAvailability(3, emptyCons, defaultSeries)
2163 c.Assert(err, gc.IsNil)
2164 machines, err = s.State.AllMachines()
2165 c.Assert(err, gc.IsNil)
2155 c.Assert(machines, gc.HasLen, 3)2166 c.Assert(machines, gc.HasLen, 3)
2156 c.Assert(machines[0].Series(), gc.Equals, "precise")2167 c.Assert(machines[0].Series(), gc.Equals, "quantal")
2157 c.Assert(machines[1].Series(), gc.Equals, "non-default")2168 c.Assert(machines[1].Series(), gc.Equals, "quantal")
2158 c.Assert(machines[2].Series(), gc.Equals, "non-default")2169 c.Assert(machines[2].Series(), gc.Equals, "quantal")
2170 defer s.setAgentAlive(c, "1").Kill()
2171 defer s.setAgentAlive(c, "2").Kill()
2172 err = s.APIState.Client().EnsureAvailability(5, emptyCons, "non-default")
2173 c.Assert(err, gc.IsNil)
2174 machines, err = s.State.AllMachines()
2175 c.Assert(err, gc.IsNil)
2176 c.Assert(machines, gc.HasLen, 5)
2177 c.Assert(machines[0].Series(), gc.Equals, "quantal")
2178 c.Assert(machines[1].Series(), gc.Equals, "quantal")
2179 c.Assert(machines[2].Series(), gc.Equals, "quantal")
2180 c.Assert(machines[3].Series(), gc.Equals, "non-default")
2181 c.Assert(machines[4].Series(), gc.Equals, "non-default")
2159}2182}
21602183
2161func (s *clientSuite) TestClientEnsureAvailabilityConstraints(c *gc.C) {2184func (s *clientSuite) TestClientEnsureAvailabilityConstraints(c *gc.C) {
2162 err := s.APIState.Client().EnsureAvailability(1, constraints.Value{}, "")2185 _, err := s.State.AddMachine("quantal", state.JobManageEnviron)
2163 c.Assert(err, gc.IsNil)2186 c.Assert(err, gc.IsNil)
2164 pinger := s.setAgentAlive(c, "0")2187 pinger := s.setAgentAlive(c, "0")
2165 defer pinger.Kill()2188 defer pinger.Kill()
2166 err = s.APIState.Client().EnsureAvailability(3, constraints.MustParse("mem=4G"), "")2189 err = s.APIState.Client().EnsureAvailability(
2190 3, constraints.MustParse("mem=4G"), defaultSeries)
2167 c.Assert(err, gc.IsNil)2191 c.Assert(err, gc.IsNil)
2168 machines, err := s.State.AllMachines()2192 machines, err := s.State.AllMachines()
2169 c.Assert(err, gc.IsNil)2193 c.Assert(err, gc.IsNil)
@@ -2180,11 +2204,53 @@
2180 }2204 }
2181}2205}
21822206
2207func (s *clientSuite) TestClientEnsureAvailability0Preserves(c *gc.C) {
2208 _, err := s.State.AddMachine("quantal", state.JobManageEnviron)
2209 c.Assert(err, gc.IsNil)
2210 pinger := s.setAgentAlive(c, "0")
2211 defer pinger.Kill()
2212 // A value of 0 says either "if I'm not HA, make me HA" or "preserve my
2213 // current HA settings".
2214 err = s.APIState.Client().EnsureAvailability(0, emptyCons, defaultSeries)
2215 c.Assert(err, gc.IsNil)
2216 machines, err := s.State.AllMachines()
2217 c.Assert(machines, gc.HasLen, 3)
2218 defer s.setAgentAlive(c, "1").Kill()
2219 // Now, we keep agent 1 alive, but not agent 2, calling
2220 // EnsureAvailability(0) again will cause us to start another machine
2221 err = s.APIState.Client().EnsureAvailability(0, emptyCons, defaultSeries)
2222 c.Assert(err, gc.IsNil)
2223 machines, err = s.State.AllMachines()
2224 c.Assert(machines, gc.HasLen, 4)
2225}
2226
2227func (s *clientSuite) TestClientEnsureAvailability0Preserves5(c *gc.C) {
2228 _, err := s.State.AddMachine("quantal", state.JobManageEnviron)
2229 c.Assert(err, gc.IsNil)
2230 pinger := s.setAgentAlive(c, "0")
2231 defer pinger.Kill()
2232 // Start off with 5 servers
2233 err = s.APIState.Client().EnsureAvailability(5, emptyCons, defaultSeries)
2234 c.Assert(err, gc.IsNil)
2235 machines, err := s.State.AllMachines()
2236 c.Assert(machines, gc.HasLen, 5)
2237 defer s.setAgentAlive(c, "1").Kill()
2238 defer s.setAgentAlive(c, "2").Kill()
2239 defer s.setAgentAlive(c, "3").Kill()
2240 // Keeping all alive but one, will bring up 1 more server to preserve 5
2241 err = s.APIState.Client().EnsureAvailability(0, emptyCons, defaultSeries)
2242 c.Assert(err, gc.IsNil)
2243 machines, err = s.State.AllMachines()
2244 c.Assert(machines, gc.HasLen, 6)
2245}
2246
2183func (s *clientSuite) TestClientEnsureAvailabilityErrors(c *gc.C) {2247func (s *clientSuite) TestClientEnsureAvailabilityErrors(c *gc.C) {
2184 var emptyCons constraints.Value2248 _, err := s.State.AddMachine("quantal", state.JobManageEnviron)
2185 defaultSeries := ""2249 c.Assert(err, gc.IsNil)
2186 err := s.APIState.Client().EnsureAvailability(0, emptyCons, defaultSeries)2250 pinger := s.setAgentAlive(c, "0")
2187 c.Assert(err, gc.ErrorMatches, "number of state servers must be odd and greater than zero")2251 defer pinger.Kill()
2252 err = s.APIState.Client().EnsureAvailability(-1, emptyCons, defaultSeries)
2253 c.Assert(err, gc.ErrorMatches, "number of state servers must be odd and non-negative")
2188 err = s.APIState.Client().EnsureAvailability(3, emptyCons, defaultSeries)2254 err = s.APIState.Client().EnsureAvailability(3, emptyCons, defaultSeries)
2189 c.Assert(err, gc.IsNil)2255 c.Assert(err, gc.IsNil)
2190 err = s.APIState.Client().EnsureAvailability(1, emptyCons, defaultSeries)2256 err = s.APIState.Client().EnsureAvailability(1, emptyCons, defaultSeries)
21912257
=== modified file 'state/state_test.go'
--- state/state_test.go 2014-04-21 23:10:05 +0000
+++ state/state_test.go 2014-04-23 06:53:26 +0000
@@ -2926,9 +2926,9 @@
2926}2926}
29272927
2928func (s *StateSuite) TestEnsureAvailabilityFailsWithBadCount(c *gc.C) {2928func (s *StateSuite) TestEnsureAvailabilityFailsWithBadCount(c *gc.C) {
2929 for _, n := range []int{-1, 0, 2, 6} {2929 for _, n := range []int{-1, 2, 6} {
2930 err := s.State.EnsureAvailability(n, constraints.Value{}, "")2930 err := s.State.EnsureAvailability(n, constraints.Value{}, "")
2931 c.Assert(err, gc.ErrorMatches, "number of state servers must be odd and greater than zero")2931 c.Assert(err, gc.ErrorMatches, "number of state servers must be odd and non-negative")
2932 }2932 }
2933 err := s.State.EnsureAvailability(replicaset.MaxPeers+2, constraints.Value{}, "")2933 err := s.State.EnsureAvailability(replicaset.MaxPeers+2, constraints.Value{}, "")
2934 c.Assert(err, gc.ErrorMatches, `state server count is too large \(allowed \d+\)`)2934 c.Assert(err, gc.ErrorMatches, `state server count is too large \(allowed \d+\)`)
@@ -3069,6 +3069,60 @@
3069 c.Assert(m0.IsManager(), jc.IsFalse)3069 c.Assert(m0.IsManager(), jc.IsFalse)
3070}3070}
30713071
3072func (s *StateSuite) TestEnsureAvailabilityMaintainsVoteList(c *gc.C) {
3073 err := s.State.EnsureAvailability(5, constraints.Value{}, "quantal")
3074 c.Assert(err, gc.IsNil)
3075 s.assertStateServerInfo(c,
3076 []string{"0", "1", "2", "3", "4"},
3077 []string{"0", "1", "2", "3", "4"})
3078 // Mark machine-0 as dead, so we'll want to create another one again
3079 s.PatchValue(state.StateServerAvailable, func(m *state.Machine) (bool, error) {
3080 return m.Id() != "0", nil
3081 })
3082 err = s.State.EnsureAvailability(0, constraints.Value{}, "quantal")
3083 c.Assert(err, gc.IsNil)
3084
3085 // New state server machine "5" is created; "0" still exists in MachineIds,
3086 // but no longer in VotingMachineIds.
3087 s.assertStateServerInfo(c,
3088 []string{"0", "1", "2", "3", "4", "5"},
3089 []string{"1", "2", "3", "4", "5"})
3090 m0, err := s.State.Machine("0")
3091 c.Assert(err, gc.IsNil)
3092 c.Assert(m0.WantsVote(), jc.IsFalse)
3093 c.Assert(m0.IsManager(), jc.IsTrue) // job still intact for now
3094 m3, err := s.State.Machine("5")
3095 c.Assert(err, gc.IsNil)
3096 c.Assert(m3.WantsVote(), jc.IsTrue)
3097 c.Assert(m3.IsManager(), jc.IsTrue)
3098}
3099
3100func (s *StateSuite) TestEnsureAvailabilityDefaultsTo3(c *gc.C) {
3101 err := s.State.EnsureAvailability(0, constraints.Value{}, "quantal")
3102 c.Assert(err, gc.IsNil)
3103 s.assertStateServerInfo(c, []string{"0", "1", "2"}, []string{"0", "1", "2"})
3104 // Mark machine-0 as dead, so we'll want to create it again
3105 s.PatchValue(state.StateServerAvailable, func(m *state.Machine) (bool, error) {
3106 return m.Id() != "0", nil
3107 })
3108 err = s.State.EnsureAvailability(0, constraints.Value{}, "quantal")
3109 c.Assert(err, gc.IsNil)
3110
3111 // New state server machine "3" is created; "0" still exists in MachineIds,
3112 // but no longer in VotingMachineIds.
3113 s.assertStateServerInfo(c,
3114 []string{"0", "1", "2", "3"},
3115 []string{"1", "2", "3"})
3116 m0, err := s.State.Machine("0")
3117 c.Assert(err, gc.IsNil)
3118 c.Assert(m0.WantsVote(), jc.IsFalse)
3119 c.Assert(m0.IsManager(), jc.IsTrue) // job still intact for now
3120 m3, err := s.State.Machine("3")
3121 c.Assert(err, gc.IsNil)
3122 c.Assert(m3.WantsVote(), jc.IsTrue)
3123 c.Assert(m3.IsManager(), jc.IsTrue)
3124}
3125
3072func (s *StateSuite) TestEnsureAvailabilityConcurrentSame(c *gc.C) {3126func (s *StateSuite) TestEnsureAvailabilityConcurrentSame(c *gc.C) {
3073 s.PatchValue(state.StateServerAvailable, func(m *state.Machine) (bool, error) {3127 s.PatchValue(state.StateServerAvailable, func(m *state.Machine) (bool, error) {
3074 return true, nil3128 return true, nil

Subscribers

People subscribed via source and target branches

to status/vote changes: