Merge lp:~fwereade/juju-core/api-force-destroy-machines into lp:~go-bot/juju-core/trunk

Proposed by William Reade on 2013-11-11
Status: Merged
Approved by: William Reade on 2013-11-12
Approved revision: 2049
Merged at revision: 2050
Proposed branch: lp:~fwereade/juju-core/api-force-destroy-machines
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 1119 lines (+369/-287)
17 files modified
cmd/juju/destroymachine.go (+18/-1)
cmd/juju/destroymachine_test.go (+51/-14)
cmd/juju/destroyunit.go (+1/-1)
cmd/jujud/machine_test.go (+1/-1)
state/api/client.go (+7/-1)
state/api/params/params.go (+1/-0)
state/apiserver/client/client.go (+52/-2)
state/apiserver/client/client_test.go (+136/-31)
state/apiserver/client/perm_test.go (+1/-1)
state/cleanup_test.go (+26/-8)
state/life.go (+8/-1)
state/life_test.go (+5/-15)
state/machine.go (+20/-3)
state/machine_test.go (+13/-44)
state/service_test.go (+3/-3)
state/state.go (+0/-67)
state/unit_test.go (+26/-94)
To merge this branch: bzr merge lp:~fwereade/juju-core/api-force-destroy-machines
Reviewer Review Type Date Requested Status
Juju Engineering 2013-11-11 Pending
Review via email: mp+194764@code.launchpad.net

Commit message

fix lp:1089291

...which involved a lot more churn than I wanted, mainly because those
DestroyWhatevers methods really shouldn't ever have been on state in the
first place, and moving them turned out to be a ridiculous hassle.

Despite the mess, this branch really just involves:

1) moving multi-Destroy methods off state and onto the api
2) fixing/moving/renaming test things
3) hooking up a --force flag to destroy-machine
4) oh, and making the relevant api.Client methods a teeny tiny bit
   consistent.

https://codereview.appspot.com/24790044/

Description of the change

fix lp:1089291

...which involved a lot more churn than I wanted, mainly because those
DestroyWhatevers methods really shouldn't ever have been on state in the
first place, and moving them turned out to be a ridiculous hassle.

Despite the mess, this branch really just involves:

1) moving multi-Destroy methods off state and onto the api
2) fixing/moving/renaming test things
3) hooking up a --force flag to destroy-machine
4) oh, and making the relevant api.Client methods a teeny tiny bit
   consistent.

https://codereview.appspot.com/24790044/

To post a comment you must log in.
William Reade (fwereade) wrote :

Reviewers: mp+194764_code.launchpad.net,

Message:
Please take a look.

Description:
fix lp:1089291

...which involved a lot more churn than I wanted, mainly because those
DestroyWhatevers methods really shouldn't ever have been on state in the
first place, and moving them turned out to be a ridiculous hassle.

Despite the mess, this branch really just involves:

1) moving multi-Destroy methods off state and onto the api
2) fixing/moving/renaming test things
3) hooking up a --force flag to destroy-machine
4) oh, and making the relevant api.Client methods a teeny tiny bit
    consistent.

https://code.launchpad.net/~fwereade/juju-core/api-force-destroy-machines/+merge/194764

(do not edit description out of merge proposal)

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

Affected files (+363, -284 lines):
   A [revision details]
   M cmd/juju/destroymachine.go
   M cmd/juju/destroymachine_test.go
   M cmd/juju/destroyunit.go
   M cmd/jujud/machine_test.go
   M state/api/client.go
   M state/api/params/params.go
   M state/apiserver/client/client.go
   M state/apiserver/client/client_test.go
   M state/apiserver/client/perm_test.go
   M state/cleanup_test.go
   M state/life.go
   M state/life_test.go
   M state/machine.go
   M state/machine_test.go
   M state/service_test.go
   M state/state.go
   M state/unit_test.go

Ian Booth (wallyworld) wrote :
Download full text (3.4 KiB)

Some nice refactoring. It does make me a little sad that we are putting
service methods (ie verbs like Destroy) on domain objects. IMO we want
to be heading towards a SOA and domain objects should only be
responsible for their own internal state, not interacting with services
and jobs etc. So it is great that the methods have come off State but
putting them on Machine is wrong to me.

My other main issue is that forcibly destroying a machine leaves its
life as Alive. That is very confusing to me.

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

https://codereview.appspot.com/24790044/diff/1/cmd/juju/destroymachine.go#newcode34
cmd/juju/destroymachine.go:34: f.BoolVar(&c.Force, "force", false,
"completely remove machine and all dependencies")
Perhaps expand a bit:

"completely remove machine and all dependencies, even if Juju thinks the
machine has running units or containers"

I think we need to be verbose as to what the implications are. I would
even go so far as to say we need a [y/N] prompt like for
destroy-environment as a typo in the machine id could be catastrophic.

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

https://codereview.appspot.com/24790044/diff/1/cmd/juju/destroymachine_test.go#newcode48
cmd/juju/destroymachine_test.go:48: // is warned about.
The above comment needs rewording now the test has been split up

https://codereview.appspot.com/24790044/diff/1/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/24790044/diff/1/state/api/client.go#newcode154
state/api/client.go:154: func (c *Client) ForceDestroyMachines(machines
...string) error {
It makes me sad that we need to add a whole new endpoint because Go
doesn't support default parameter values :-(

https://codereview.appspot.com/24790044/diff/1/state/apiserver/client/client_test.go
File state/apiserver/client/client_test.go (right):

https://codereview.appspot.com/24790044/diff/1/state/apiserver/client/client_test.go#newcode549
state/apiserver/client/client_test.go:549: assertLife(c, m0,
state.Alive)
Why doesn't force destroy cause the life to be set to Dying? I find it
particularly unintuitive that a status would show the machine as alive
when in fact it is on death row and waiting for the clean up job to
remove it.

https://codereview.appspot.com/24790044/diff/1/state/cleanup_test.go
File state/cleanup_test.go (right):

https://codereview.appspot.com/24790044/diff/1/state/cleanup_test.go#newcode93
state/cleanup_test.go:93: func (s *CleanupSuite)
TestForceDestroyMachineErrors(c *gc.C) {
The code in this test can definitely be extracted out into a helper

func (s *CleanupSuite) assertForceDestroyMachineError(c *gc.C, job
state.MachineJob) {
     m, err := s.State.AddMachine("quantal", job)
     c.Assert(err, gc.IsNil)
     err = environManager.ForceDestroy()
     c.Assert(err, gc.ErrorMatches, "machine [0-9] is required by the
environment")
     s.assertDoesNotNeedCleanup(c)
     assertLife(c, m, state.Alive)
}

https://codereview.appspot.com/24790044/diff/1/state/machine.go
File state/machin...

Read more...

John A Meinel (jameinel) wrote :

just some of my thoughts, not a full review.

https://codereview.appspot.com/24790044/diff/1/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/24790044/diff/1/state/api/client.go#newcode154
state/api/client.go:154: func (c *Client) ForceDestroyMachines(machines
...string) error {
On 2013/11/12 00:43:42, wallyworld wrote:
> It makes me sad that we need to add a whole new endpoint because Go
doesn't
> support default parameter values :-(

It isn't really an endpoint, but it is a new function.

I only make the distinction because I was about to say the next change:
-func (c *Client) DestroyServiceUnits(unitNames []string) error {
+func (c *Client) DestroyServiceUnits(unitNames ...string) error {

was an API break, but this is all just within the client process.

so we *could* spell it:

DestroyMachines(force bool, machines ...string)

I'm guessing it was a bit ackward and William wanted to avoid bool in
APIs especially for a Force sort of function.

https://codereview.appspot.com/24790044/diff/1/state/apiserver/client/client_test.go
File state/apiserver/client/client_test.go (right):

https://codereview.appspot.com/24790044/diff/1/state/apiserver/client/client_test.go#newcode549
state/apiserver/client/client_test.go:549: assertLife(c, m0,
state.Alive)
On 2013/11/12 00:43:42, wallyworld wrote:
> Why doesn't force destroy cause the life to be set to Dying? I find it
> particularly unintuitive that a status would show the machine as alive
when in
> fact it is on death row and waiting for the clean up job to remove it.

In earlier discussions, this is a limitation of the Machine lifecycle
model. A Machine can't be set to dying until all Units are no longer on
the machine.

https://codereview.appspot.com/24790044/

William Reade (fwereade) wrote :
Download full text (4.9 KiB)

Machine lifecycle is out of scope; SOA concepts demand more discussion
in a separate forum; I think I've addressed the rest.

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

https://codereview.appspot.com/24790044/diff/1/cmd/juju/destroymachine.go#newcode34
cmd/juju/destroymachine.go:34: f.BoolVar(&c.Force, "force", false,
"completely remove machine and all dependencies")
On 2013/11/12 00:43:42, wallyworld wrote:
> Perhaps expand a bit:

> "completely remove machine and all dependencies, even if Juju thinks
the machine
> has running units or containers"

> I think we need to be verbose as to what the implications are. I would
even go
> so far as to say we need a [y/N] prompt like for destroy-environment
as a typo
> in the machine id could be catastrophic.

Expanded the doc string -- I'm kinda -1 on an additional gate beyond
specifying "--force" though.

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

https://codereview.appspot.com/24790044/diff/1/cmd/juju/destroymachine_test.go#newcode48
cmd/juju/destroymachine_test.go:48: // is warned about.
On 2013/11/12 00:43:42, wallyworld wrote:
> The above comment needs rewording now the test has been split up

Whoops, thanks.

https://codereview.appspot.com/24790044/diff/1/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/24790044/diff/1/state/api/client.go#newcode154
state/api/client.go:154: func (c *Client) ForceDestroyMachines(machines
...string) error {
On 2013/11/12 02:58:37, jameinel wrote:
> On 2013/11/12 00:43:42, wallyworld wrote:
> > It makes me sad that we need to add a whole new endpoint because Go
doesn't
> > support default parameter values :-(

> It isn't really an endpoint, but it is a new function.

> I only make the distinction because I was about to say the next
change:
> -func (c *Client) DestroyServiceUnits(unitNames []string) error {
> +func (c *Client) DestroyServiceUnits(unitNames ...string) error {

> was an API break, but this is all just within the client process.

> so we *could* spell it:

> DestroyMachines(force bool, machines ...string)

> I'm guessing it was a bit ackward and William wanted to avoid bool in
APIs
> especially for a Force sort of function.

Yeah, it's just a new function, and the bool option felt horrible. Re
DestroyServiceUnits I'm willing to impose that minimal pain on anyone
who's using api.Client in an external library, because I think the
global pain of inconsistency is worse.

https://codereview.appspot.com/24790044/diff/1/state/apiserver/client/client_test.go
File state/apiserver/client/client_test.go (right):

https://codereview.appspot.com/24790044/diff/1/state/apiserver/client/client_test.go#newcode549
state/apiserver/client/client_test.go:549: assertLife(c, m0,
state.Alive)
On 2013/11/12 02:58:37, jameinel wrote:
> On 2013/11/12 00:43:42, wallyworld wrote:
> > Why doesn't force destroy cause the life to be set to Dying? I find
it
> > particularly unintuitive that a status would show the machine as
alive when in
> > fact it is on death row and waiting for the clean ...

Read more...

2049. By William Reade on 2013-11-12

address review

William Reade (fwereade) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmd/juju/destroymachine.go'
2--- cmd/juju/destroymachine.go 2013-11-01 06:53:54 +0000
3+++ cmd/juju/destroymachine.go 2013-11-12 10:22:12 +0000
4@@ -6,6 +6,8 @@
5 import (
6 "fmt"
7
8+ "launchpad.net/gnuflag"
9+
10 "launchpad.net/juju-core/cmd"
11 "launchpad.net/juju-core/juju"
12 "launchpad.net/juju-core/names"
13@@ -15,18 +17,30 @@
14 type DestroyMachineCommand struct {
15 cmd.EnvCommandBase
16 MachineIds []string
17+ Force bool
18 }
19
20+const destroyMachineDoc = `
21+Machines that are responsible for the environment cannot be destroyed. Machines
22+running units or containers can only be destroyed with the --force flag; doing
23+so will also destroy all those units and containers without giving them any
24+opportunity to shut down cleanly.
25+`
26+
27 func (c *DestroyMachineCommand) Info() *cmd.Info {
28 return &cmd.Info{
29 Name: "destroy-machine",
30 Args: "<machine> ...",
31 Purpose: "destroy machines",
32- Doc: "Machines that have assigned units, or are responsible for the environment, cannot be destroyed.",
33+ Doc: "",
34 Aliases: []string{"terminate-machine"},
35 }
36 }
37
38+func (c *DestroyMachineCommand) SetFlags(f *gnuflag.FlagSet) {
39+ f.BoolVar(&c.Force, "force", false, "completely remove machine and all dependencies")
40+}
41+
42 func (c *DestroyMachineCommand) Init(args []string) error {
43 if len(args) == 0 {
44 return fmt.Errorf("no machines specified")
45@@ -46,5 +60,8 @@
46 return err
47 }
48 defer apiclient.Close()
49+ if c.Force {
50+ return apiclient.ForceDestroyMachines(c.MachineIds...)
51+ }
52 return apiclient.DestroyMachines(c.MachineIds...)
53 }
54
55=== modified file 'cmd/juju/destroymachine_test.go'
56--- cmd/juju/destroymachine_test.go 2013-09-27 07:05:45 +0000
57+++ cmd/juju/destroymachine_test.go 2013-11-12 10:22:12 +0000
58@@ -6,9 +6,11 @@
59 import (
60 gc "launchpad.net/gocheck"
61
62+ "launchpad.net/juju-core/errors"
63 jujutesting "launchpad.net/juju-core/juju/testing"
64 "launchpad.net/juju-core/state"
65 "launchpad.net/juju-core/testing"
66+ jc "launchpad.net/juju-core/testing/checkers"
67 )
68
69 type DestroyMachineSuite struct {
70@@ -22,7 +24,7 @@
71 return err
72 }
73
74-func (s *DestroyMachineSuite) TestDestroyMachine(c *gc.C) {
75+func (s *DestroyMachineSuite) TestDestroyMachineWithUnit(c *gc.C) {
76 // Create a machine running a unit.
77 testing.Charms.BundlePath(s.SeriesPath, "riak")
78 err := runDeploy(c, "local:riak", "riak")
79@@ -38,19 +40,16 @@
80 // Try to destroy the machine and fail.
81 err = runDestroyMachine(c, "0")
82 c.Assert(err, gc.ErrorMatches, `no machines were destroyed: machine 0 has unit "riak/0" assigned`)
83+}
84
85- // Remove the unit, and try to destroy the machine along with another that
86- // doesn't exist; check that the machine is destroyed, but the missing one
87- // is warned about.
88- err = u.Destroy()
89- c.Assert(err, gc.IsNil)
90- err = u.EnsureDead()
91- c.Assert(err, gc.IsNil)
92- err = u.Remove()
93+func (s *DestroyMachineSuite) TestDestroyEmptyMachine(c *gc.C) {
94+ // Destroy an empty machine alongside a state server; only the empty machine
95+ // gets destroyed.
96+ m0, err := s.State.AddMachine("quantal", state.JobHostUnits)
97 c.Assert(err, gc.IsNil)
98 err = runDestroyMachine(c, "0", "1")
99 c.Assert(err, gc.ErrorMatches, `some machines were not destroyed: machine 1 does not exist`)
100- m0, err := s.State.Machine("0")
101+ err = m0.Refresh()
102 c.Assert(err, gc.IsNil)
103 c.Assert(m0.Life(), gc.Equals, state.Dying)
104
105@@ -60,9 +59,13 @@
106 err = m0.Refresh()
107 c.Assert(err, gc.IsNil)
108 c.Assert(m0.Life(), gc.Equals, state.Dying)
109+}
110
111- // As is destroying a Dead machine; destroying it alongside a JobManageEnviron
112- // machine complains only about the JMA machine.
113+func (s *DestroyMachineSuite) TestDestroyDeadMachine(c *gc.C) {
114+ // Destroying a Dead machine is a no-op; destroying it alongside a JobManageEnviron
115+ // machine complains only about the JME machine.
116+ m0, err := s.State.AddMachine("quantal", state.JobHostUnits)
117+ c.Assert(err, gc.IsNil)
118 err = m0.EnsureDead()
119 c.Assert(err, gc.IsNil)
120 m1, err := s.State.AddMachine("quantal", state.JobManageEnviron)
121@@ -75,9 +78,43 @@
122 err = m1.Refresh()
123 c.Assert(err, gc.IsNil)
124 c.Assert(m1.Life(), gc.Equals, state.Alive)
125-
126+}
127+
128+func (s *DestroyMachineSuite) TestForce(c *gc.C) {
129+ // Create a machine running a unit.
130+ testing.Charms.BundlePath(s.SeriesPath, "riak")
131+ err := runDeploy(c, "local:riak", "riak")
132+ c.Assert(err, gc.IsNil)
133+
134+ // Get the state entities to allow sane testing.
135+ u, err := s.State.Unit("riak/0")
136+ c.Assert(err, gc.IsNil)
137+ m0, err := s.State.Machine("0")
138+ c.Assert(err, gc.IsNil)
139+
140+ // Create a manager machine.
141+ m1, err := s.State.AddMachine("quantal", state.JobManageEnviron)
142+ c.Assert(err, gc.IsNil)
143+
144+ // Try to force-destroy the machines.
145+ err = runDestroyMachine(c, "0", "1", "--force")
146+ c.Assert(err, gc.ErrorMatches, `some machines were not destroyed: machine 1 is required by the environment`)
147+
148+ // Clean up, check state.
149+ err = s.State.Cleanup()
150+ c.Assert(err, gc.IsNil)
151+ err = m0.Refresh()
152+ c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
153+ err = u.Refresh()
154+ c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
155+ err = m1.Refresh()
156+ c.Assert(err, gc.IsNil)
157+ c.Assert(m1.Life(), gc.Equals, state.Alive)
158+}
159+
160+func (s *DestroyMachineSuite) TestBadArgs(c *gc.C) {
161 // Check invalid args.
162- err = runDestroyMachine(c)
163+ err := runDestroyMachine(c)
164 c.Assert(err, gc.ErrorMatches, `no machines specified`)
165 err = runDestroyMachine(c, "1", "2", "nonsense", "rubbish")
166 c.Assert(err, gc.ErrorMatches, `invalid machine id "nonsense"`)
167
168=== modified file 'cmd/juju/destroyunit.go'
169--- cmd/juju/destroyunit.go 2013-11-05 01:22:35 +0000
170+++ cmd/juju/destroyunit.go 2013-11-12 10:22:12 +0000
171@@ -48,5 +48,5 @@
172 return err
173 }
174 defer client.Close()
175- return client.DestroyServiceUnits(c.UnitNames)
176+ return client.DestroyServiceUnits(c.UnitNames...)
177 }
178
179=== modified file 'cmd/jujud/machine_test.go'
180--- cmd/jujud/machine_test.go 2013-11-11 10:57:06 +0000
181+++ cmd/jujud/machine_test.go 2013-11-12 10:22:12 +0000
182@@ -78,7 +78,7 @@
183 err = m.SetPassword(initialMachinePassword)
184 c.Assert(err, gc.IsNil)
185 tag := names.MachineTag(m.Id())
186- if m.IsStateServer() {
187+ if m.IsManager() {
188 err = m.SetMongoPassword(initialMachinePassword)
189 c.Assert(err, gc.IsNil)
190 config, tools = s.agentSuite.primeStateAgent(c, tag, initialMachinePassword)
191
192=== modified file 'state/api/client.go'
193--- state/api/client.go 2013-11-08 06:36:55 +0000
194+++ state/api/client.go 2013-11-12 10:22:12 +0000
195@@ -150,6 +150,12 @@
196 return c.st.Call("Client", "", "DestroyMachines", params, nil)
197 }
198
199+// ForceDestroyMachines removes a given set of machines and all associated units.
200+func (c *Client) ForceDestroyMachines(machines ...string) error {
201+ params := params.DestroyMachines{Force: true, MachineNames: machines}
202+ return c.st.Call("Client", "", "DestroyMachines", params, nil)
203+}
204+
205 // ServiceExpose changes the juju-managed firewall to expose any ports that
206 // were also explicitly marked by units as open.
207 func (c *Client) ServiceExpose(service string) error {
208@@ -207,7 +213,7 @@
209 }
210
211 // DestroyServiceUnits decreases the number of units dedicated to a service.
212-func (c *Client) DestroyServiceUnits(unitNames []string) error {
213+func (c *Client) DestroyServiceUnits(unitNames ...string) error {
214 params := params.DestroyServiceUnits{unitNames}
215 return c.st.Call("Client", "", "DestroyServiceUnits", params, nil)
216 }
217
218=== modified file 'state/api/params/params.go'
219--- state/api/params/params.go 2013-11-08 06:36:55 +0000
220+++ state/api/params/params.go 2013-11-12 10:22:12 +0000
221@@ -93,6 +93,7 @@
222 // DestroyMachines holds parameters for the DestroyMachines call.
223 type DestroyMachines struct {
224 MachineNames []string
225+ Force bool
226 }
227
228 // ServiceDeploy holds the parameters for making the ServiceDeploy call.
229
230=== modified file 'state/apiserver/client/client.go'
231--- state/apiserver/client/client.go 2013-11-08 06:36:55 +0000
232+++ state/apiserver/client/client.go 2013-11-12 10:22:12 +0000
233@@ -6,11 +6,13 @@
234 import (
235 "errors"
236 "fmt"
237+ "strings"
238
239 "launchpad.net/loggo"
240
241 "launchpad.net/juju-core/charm"
242 "launchpad.net/juju-core/environs"
243+ coreerrors "launchpad.net/juju-core/errors"
244 "launchpad.net/juju-core/instance"
245 "launchpad.net/juju-core/juju"
246 "launchpad.net/juju-core/names"
247@@ -413,7 +415,25 @@
248
249 // DestroyServiceUnits removes a given set of service units.
250 func (c *Client) DestroyServiceUnits(args params.DestroyServiceUnits) error {
251- return c.api.state.DestroyUnits(args.UnitNames...)
252+ var errs []string
253+ for _, name := range args.UnitNames {
254+ unit, err := c.api.state.Unit(name)
255+ switch {
256+ case coreerrors.IsNotFoundError(err):
257+ err = fmt.Errorf("unit %q does not exist", name)
258+ case err != nil:
259+ case unit.Life() != state.Alive:
260+ continue
261+ case unit.IsPrincipal():
262+ err = unit.Destroy()
263+ default:
264+ err = fmt.Errorf("unit %q is a subordinate", name)
265+ }
266+ if err != nil {
267+ errs = append(errs, err.Error())
268+ }
269+ }
270+ return destroyErr("units", args.UnitNames, errs)
271 }
272
273 // ServiceDestroy destroys a given service.
274@@ -625,7 +645,25 @@
275
276 // DestroyMachines removes a given set of machines.
277 func (c *Client) DestroyMachines(args params.DestroyMachines) error {
278- return c.api.state.DestroyMachines(args.MachineNames...)
279+ var errs []string
280+ for _, id := range args.MachineNames {
281+ machine, err := c.api.state.Machine(id)
282+ switch {
283+ case coreerrors.IsNotFoundError(err):
284+ err = fmt.Errorf("machine %s does not exist", id)
285+ case err != nil:
286+ case args.Force:
287+ err = machine.ForceDestroy()
288+ case machine.Life() != state.Alive:
289+ continue
290+ default:
291+ err = machine.Destroy()
292+ }
293+ if err != nil {
294+ errs = append(errs, err.Error())
295+ }
296+ }
297+ return destroyErr("machines", args.MachineNames, errs)
298 }
299
300 // CharmInfo returns information about the requested charm.
301@@ -786,3 +824,15 @@
302 // Now try to apply the new validated config.
303 return c.api.state.SetEnvironConfig(newProviderConfig)
304 }
305+
306+func destroyErr(desc string, ids, errs []string) error {
307+ if len(errs) == 0 {
308+ return nil
309+ }
310+ msg := "some %s were not destroyed"
311+ if len(errs) == len(ids) {
312+ msg = "no %s were destroyed"
313+ }
314+ msg = fmt.Sprintf(msg, desc)
315+ return fmt.Errorf("%s: %s", msg, strings.Join(errs, "; "))
316+}
317
318=== modified file 'state/apiserver/client/client_test.go'
319--- state/apiserver/client/client_test.go 2013-11-08 06:36:55 +0000
320+++ state/apiserver/client/client_test.go 2013-11-12 10:22:12 +0000
321@@ -493,38 +493,143 @@
322 c.Assert(service.Life(), gc.Not(gc.Equals), state.Alive)
323 }
324
325-func (s *clientSuite) TestClientDestroyMachines(c *gc.C) {
326- s.setUpScenario(c)
327- err := s.APIState.Client().DestroyMachines("1")
328- c.Assert(err, gc.ErrorMatches, `no machines were destroyed: machine 1 has unit "wordpress/0" assigned`)
329- m, err := s.State.AddMachine("trusty", state.JobHostUnits)
330- c.Assert(err, gc.IsNil)
331- err = s.APIState.Client().DestroyMachines(m.Id())
332- c.Assert(err, gc.IsNil)
333- err = m.Refresh()
334- c.Assert(err, gc.IsNil)
335- c.Assert(m.Life(), gc.Not(gc.Equals), state.Alive)
336-}
337-
338-func (s *clientSuite) TestClientDestroyUnits(c *gc.C) {
339- // Setup:
340- s.setUpScenario(c)
341- service, err := s.State.Service("wordpress")
342- c.Assert(err, gc.IsNil)
343- _, err = service.AddUnit()
344- c.Assert(err, gc.IsNil)
345- // Destroy some units and check the result.
346- err = s.APIState.Client().DestroyServiceUnits([]string{"wordpress/1", "wordpress/2"})
347- c.Assert(err, gc.IsNil)
348- units, err := service.AllUnits()
349- c.Assert(err, gc.IsNil)
350- for i, u := range units {
351- if i == 0 {
352- c.Assert(u.Life(), gc.Equals, state.Alive)
353- } else {
354- c.Assert(u.Life(), gc.Equals, state.Dying)
355- }
356+func assertLife(c *gc.C, entity state.Living, life state.Life) {
357+ err := entity.Refresh()
358+ c.Assert(err, gc.IsNil)
359+ c.Assert(entity.Life(), gc.Equals, life)
360+}
361+
362+func assertRemoved(c *gc.C, entity state.Living) {
363+ err := entity.Refresh()
364+ c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
365+}
366+
367+func (s *clientSuite) setupDestroyMachinesTest(c *gc.C) (*state.Machine, *state.Machine, *state.Machine, *state.Unit) {
368+ m0, err := s.State.AddMachine("quantal", state.JobHostUnits)
369+ c.Assert(err, gc.IsNil)
370+ m1, err := s.State.AddMachine("quantal", state.JobManageEnviron)
371+ c.Assert(err, gc.IsNil)
372+ m2, err := s.State.AddMachine("quantal", state.JobHostUnits)
373+ c.Assert(err, gc.IsNil)
374+
375+ sch := s.AddTestingCharm(c, "wordpress")
376+ wordpress, err := s.State.AddService("wordpress", sch)
377+ c.Assert(err, gc.IsNil)
378+ u, err := wordpress.AddUnit()
379+ c.Assert(err, gc.IsNil)
380+ err = u.AssignToMachine(m0)
381+ c.Assert(err, gc.IsNil)
382+
383+ return m0, m1, m2, u
384+}
385+
386+func (s *clientSuite) TestDestroyMachines(c *gc.C) {
387+ m0, m1, m2, u := s.setupDestroyMachinesTest(c)
388+
389+ err := s.APIState.Client().DestroyMachines("0", "1", "2")
390+ c.Assert(err, gc.ErrorMatches, `some machines were not destroyed: machine 0 has unit "wordpress/0" assigned; machine 1 is required by the environment`)
391+ assertLife(c, m0, state.Alive)
392+ assertLife(c, m1, state.Alive)
393+ assertLife(c, m2, state.Dying)
394+
395+ err = u.UnassignFromMachine()
396+ c.Assert(err, gc.IsNil)
397+ err = s.APIState.Client().DestroyMachines("0", "1", "2")
398+ c.Assert(err, gc.ErrorMatches, `some machines were not destroyed: machine 1 is required by the environment`)
399+ assertLife(c, m0, state.Dying)
400+ assertLife(c, m1, state.Alive)
401+ assertLife(c, m2, state.Dying)
402+}
403+
404+func (s *clientSuite) TestForceDestroyMachines(c *gc.C) {
405+ m0, m1, m2, u := s.setupDestroyMachinesTest(c)
406+
407+ err := s.APIState.Client().ForceDestroyMachines("0", "1", "2")
408+ c.Assert(err, gc.ErrorMatches, `some machines were not destroyed: machine 1 is required by the environment`)
409+ assertLife(c, m0, state.Alive)
410+ assertLife(c, m1, state.Alive)
411+ assertLife(c, m2, state.Alive)
412+ assertLife(c, u, state.Alive)
413+
414+ err = s.State.Cleanup()
415+ c.Assert(err, gc.IsNil)
416+ assertRemoved(c, m0)
417+ assertLife(c, m1, state.Alive)
418+ assertRemoved(c, m2)
419+ assertRemoved(c, u)
420+}
421+
422+func (s *clientSuite) TestDestroyPrincipalUnits(c *gc.C) {
423+ wordpress, err := s.State.AddService("wordpress", s.AddTestingCharm(c, "wordpress"))
424+ c.Assert(err, gc.IsNil)
425+ units := make([]*state.Unit, 5)
426+ for i := range units {
427+ unit, err := wordpress.AddUnit()
428+ c.Assert(err, gc.IsNil)
429+ err = unit.SetStatus(params.StatusStarted, "", nil)
430+ c.Assert(err, gc.IsNil)
431+ units[i] = unit
432 }
433+
434+ // Destroy 2 of them; check they become Dying.
435+ err = s.APIState.Client().DestroyServiceUnits("wordpress/0", "wordpress/1")
436+ c.Assert(err, gc.IsNil)
437+ assertLife(c, units[0], state.Dying)
438+ assertLife(c, units[1], state.Dying)
439+
440+ // Try to destroy an Alive one and a Dying one; check
441+ // it destroys the Alive one and ignores the Dying one.
442+ err = s.APIState.Client().DestroyServiceUnits("wordpress/2", "wordpress/0")
443+ c.Assert(err, gc.IsNil)
444+ assertLife(c, units[2], state.Dying)
445+
446+ // Try to destroy an Alive one along with a nonexistent one; check that
447+ // the valid instruction is followed but the invalid one is warned about.
448+ err = s.APIState.Client().DestroyServiceUnits("boojum/123", "wordpress/3")
449+ c.Assert(err, gc.ErrorMatches, `some units were not destroyed: unit "boojum/123" does not exist`)
450+ assertLife(c, units[3], state.Dying)
451+
452+ // Make one Dead, and destroy an Alive one alongside it; check no errors.
453+ wp0, err := s.State.Unit("wordpress/0")
454+ c.Assert(err, gc.IsNil)
455+ err = wp0.EnsureDead()
456+ c.Assert(err, gc.IsNil)
457+ err = s.APIState.Client().DestroyServiceUnits("wordpress/0", "wordpress/4")
458+ c.Assert(err, gc.IsNil)
459+ assertLife(c, units[0], state.Dead)
460+ assertLife(c, units[4], state.Dying)
461+}
462+
463+func (s *clientSuite) TestDestroySubordinateUnits(c *gc.C) {
464+ wordpress, err := s.State.AddService("wordpress", s.AddTestingCharm(c, "wordpress"))
465+ c.Assert(err, gc.IsNil)
466+ wordpress0, err := wordpress.AddUnit()
467+ c.Assert(err, gc.IsNil)
468+ _, err = s.State.AddService("logging", s.AddTestingCharm(c, "logging"))
469+ c.Assert(err, gc.IsNil)
470+ eps, err := s.State.InferEndpoints([]string{"logging", "wordpress"})
471+ c.Assert(err, gc.IsNil)
472+ rel, err := s.State.AddRelation(eps...)
473+ c.Assert(err, gc.IsNil)
474+ ru, err := rel.Unit(wordpress0)
475+ c.Assert(err, gc.IsNil)
476+ err = ru.EnterScope(nil)
477+ c.Assert(err, gc.IsNil)
478+ logging0, err := s.State.Unit("logging/0")
479+ c.Assert(err, gc.IsNil)
480+
481+ // Try to destroy the subordinate alone; check it fails.
482+ err = s.APIState.Client().DestroyServiceUnits("logging/0")
483+ c.Assert(err, gc.ErrorMatches, `no units were destroyed: unit "logging/0" is a subordinate`)
484+ assertLife(c, logging0, state.Alive)
485+
486+ // Try to destroy the principal and the subordinate together; check it warns
487+ // about the subordinate, but destroys the one it can. (The principal unit
488+ // agent will be resposible for destroying the subordinate.)
489+ err = s.APIState.Client().DestroyServiceUnits("wordpress/0", "logging/0")
490+ c.Assert(err, gc.ErrorMatches, `some units were not destroyed: unit "logging/0" is a subordinate`)
491+ assertLife(c, wordpress0, state.Dying)
492+ assertLife(c, logging0, state.Alive)
493 }
494
495 func (s *clientSuite) testClientUnitResolved(c *gc.C, retry bool, expectedResolvedMode state.ResolvedMode) {
496
497=== modified file 'state/apiserver/client/perm_test.go'
498--- state/apiserver/client/perm_test.go 2013-11-06 10:51:34 +0000
499+++ state/apiserver/client/perm_test.go 2013-11-12 10:22:12 +0000
500@@ -348,7 +348,7 @@
501 }
502
503 func opClientDestroyServiceUnits(c *gc.C, st *api.State, mst *state.State) (func(), error) {
504- err := st.Client().DestroyServiceUnits([]string{"wordpress/99"})
505+ err := st.Client().DestroyServiceUnits("wordpress/99")
506 if err != nil && strings.HasPrefix(err.Error(), "no units were destroyed") {
507 err = nil
508 }
509
510=== modified file 'state/cleanup_test.go'
511--- state/cleanup_test.go 2013-11-11 15:45:06 +0000
512+++ state/cleanup_test.go 2013-11-12 10:22:12 +0000
513@@ -1,6 +1,8 @@
514 package state_test
515
516 import (
517+ "fmt"
518+
519 gc "launchpad.net/gocheck"
520
521 "launchpad.net/juju-core/charm"
522@@ -90,6 +92,22 @@
523 c.Assert(err, gc.ErrorMatches, `cannot read settings for unit "riak/0" in relation "riak:ring": settings not found`)
524 }
525
526+func (s *CleanupSuite) testForceDestroyManagerError(c *gc.C, job state.MachineJob) {
527+ manager, err := s.State.AddMachine("quantal", job)
528+ c.Assert(err, gc.IsNil)
529+ s.assertDoesNotNeedCleanup(c)
530+ err = manager.ForceDestroy()
531+ expect := fmt.Sprintf("machine %s is required by the environment", manager.Id())
532+ c.Assert(err, gc.ErrorMatches, expect)
533+ s.assertDoesNotNeedCleanup(c)
534+ assertLife(c, manager, state.Alive)
535+}
536+
537+func (s *CleanupSuite) TestForceDestroyMachineErrors(c *gc.C) {
538+ s.testForceDestroyManagerError(c, state.JobManageState)
539+ s.testForceDestroyManagerError(c, state.JobManageEnviron)
540+}
541+
542 func (s *CleanupSuite) TestCleanupForceDestroyedMachineUnit(c *gc.C) {
543 s.assertDoesNotNeedCleanup(c)
544
545@@ -106,7 +124,7 @@
546 s.assertDoesNotNeedCleanup(c)
547
548 // Force machine destruction, check cleanup queued.
549- err = s.State.ForceDestroyMachines(machine.Id())
550+ err = machine.ForceDestroy()
551 c.Assert(err, gc.IsNil)
552 s.assertNeedsCleanup(c)
553
554@@ -117,7 +135,7 @@
555 c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
556
557 // ...and so has the unit...
558- assertUnitRemoved(c, pr.u0)
559+ assertRemoved(c, pr.u0)
560
561 // ...and the unit has departed relation scope.
562 assertNotInScope(c, pr.ru0)
563@@ -156,13 +174,13 @@
564 s.assertDoesNotNeedCleanup(c)
565
566 // Force removal of the top-level machine.
567- err = s.State.ForceDestroyMachines(machine.Id())
568+ err = machine.ForceDestroy()
569 c.Assert(err, gc.IsNil)
570 s.assertNeedsCleanup(c)
571
572 // And do it again, just to check that the second cleanup doc for the same
573 // machine doesn't cause problems down the line.
574- err = s.State.ForceDestroyMachines(machine.Id())
575+ err = machine.ForceDestroy()
576 c.Assert(err, gc.IsNil)
577 s.assertNeedsCleanup(c)
578
579@@ -175,10 +193,10 @@
580 c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
581
582 // ...and so have all the units...
583- assertUnitRemoved(c, prr.pu0)
584- assertUnitRemoved(c, prr.pu1)
585- assertUnitRemoved(c, prr.ru0)
586- assertUnitRemoved(c, prr.ru1)
587+ assertRemoved(c, prr.pu0)
588+ assertRemoved(c, prr.pu1)
589+ assertRemoved(c, prr.ru0)
590+ assertRemoved(c, prr.ru1)
591
592 // ...and none of the units have left relation scopes occupied.
593 assertNotInScope(c, prr.pru0)
594
595=== modified file 'state/life.go'
596--- state/life.go 2013-07-09 10:32:23 +0000
597+++ state/life.go 2013-11-12 10:22:12 +0000
598@@ -38,8 +38,15 @@
599 type Living interface {
600 Life() Life
601 Destroy() error
602+ Refresh() error
603+}
604+
605+// AgentLiving describes state entities with a lifecycle and an agent that
606+// manages it.
607+type AgentLiving interface {
608+ Living
609 EnsureDead() error
610- Refresh() error
611+ Remove() error
612 }
613
614 func isAlive(coll *mgo.Collection, id interface{}) (bool, error) {
615
616=== modified file 'state/life_test.go'
617--- state/life_test.go 2013-09-27 07:05:45 +0000
618+++ state/life_test.go 2013-11-12 10:22:12 +0000
619@@ -81,8 +81,7 @@
620
621 type lifeFixture interface {
622 id() (coll string, id interface{})
623- setup(s *LifeSuite, c *gc.C) state.Living
624- teardown(s *LifeSuite, c *gc.C)
625+ setup(s *LifeSuite, c *gc.C) state.AgentLiving
626 }
627
628 type unitLife struct {
629@@ -93,7 +92,7 @@
630 return "units", l.unit.Name()
631 }
632
633-func (l *unitLife) setup(s *LifeSuite, c *gc.C) state.Living {
634+func (l *unitLife) setup(s *LifeSuite, c *gc.C) state.AgentLiving {
635 unit, err := s.svc.AddUnit()
636 c.Assert(err, gc.IsNil)
637 preventUnitDestroyRemove(c, unit)
638@@ -101,11 +100,6 @@
639 return l.unit
640 }
641
642-func (l *unitLife) teardown(s *LifeSuite, c *gc.C) {
643- err := l.unit.Remove()
644- c.Assert(err, gc.IsNil)
645-}
646-
647 type machineLife struct {
648 machine *state.Machine
649 }
650@@ -114,18 +108,13 @@
651 return "machines", l.machine.Id()
652 }
653
654-func (l *machineLife) setup(s *LifeSuite, c *gc.C) state.Living {
655+func (l *machineLife) setup(s *LifeSuite, c *gc.C) state.AgentLiving {
656 var err error
657 l.machine, err = s.State.AddMachine("quantal", state.JobHostUnits)
658 c.Assert(err, gc.IsNil)
659 return l.machine
660 }
661
662-func (l *machineLife) teardown(s *LifeSuite, c *gc.C) {
663- err := l.machine.Remove()
664- c.Assert(err, gc.IsNil)
665-}
666-
667 func (s *LifeSuite) prepareFixture(living state.Living, lfix lifeFixture, cached, dbinitial state.Life, c *gc.C) {
668 collName, id := lfix.id()
669 coll := s.MgoSuite.Session.DB("juju").C(collName)
670@@ -165,7 +154,8 @@
671 c.Assert(living.Life(), gc.Equals, v.dbfinal)
672 err = living.EnsureDead()
673 c.Assert(err, gc.IsNil)
674- lfix.teardown(s, c)
675+ err = living.Remove()
676+ c.Assert(err, gc.IsNil)
677 }
678 }
679 }
680
681=== modified file 'state/machine.go'
682--- state/machine.go 2013-11-11 23:42:49 +0000
683+++ state/machine.go 2013-11-12 10:22:12 +0000
684@@ -195,10 +195,11 @@
685 return m.doc.Jobs
686 }
687
688-// IsStateServer returns true if the machine has a JobManageState job.
689-func (m *Machine) IsStateServer() bool {
690+// IsManager returns true if the machine has JobManageState or JobManageEnviron.
691+func (m *Machine) IsManager() bool {
692 for _, job := range m.doc.Jobs {
693- if job == JobManageState {
694+ switch job {
695+ case JobManageEnviron, JobManageState:
696 return true
697 }
698 }
699@@ -314,6 +315,22 @@
700 return m.advanceLifecycle(Dying)
701 }
702
703+// ForceDestroy queues the machine for complete removal, including the
704+// destruction of all units and containers on the machine.
705+func (m *Machine) ForceDestroy() error {
706+ if !m.IsManager() {
707+ ops := []txn.Op{{
708+ C: m.st.machines.Name,
709+ Id: m.doc.Id,
710+ Assert: D{{"jobs", D{{"$nin", []MachineJob{JobManageState}}}}},
711+ }, m.st.newCleanupOp("machine", m.doc.Id)}
712+ if err := m.st.runTransaction(ops); err != txn.ErrAborted {
713+ return err
714+ }
715+ }
716+ return fmt.Errorf("machine %s is required by the environment", m.doc.Id)
717+}
718+
719 // EnsureDead sets the machine lifecycle to Dead if it is Alive or Dying.
720 // It does nothing otherwise. EnsureDead will fail if the machine has
721 // principal units assigned, or if the machine has JobManageEnviron.
722
723=== modified file 'state/machine_test.go'
724--- state/machine_test.go 2013-11-12 01:59:52 +0000
725+++ state/machine_test.go 2013-11-12 10:22:12 +0000
726@@ -67,15 +67,17 @@
727 c.Assert(ok, gc.Equals, true)
728 }
729
730-func (s *MachineSuite) TestMachineIsStateServer(c *gc.C) {
731+func (s *MachineSuite) TestMachineIsManager(c *gc.C) {
732 tests := []struct {
733 isStateServer bool
734 jobs []state.MachineJob
735 }{
736 {false, []state.MachineJob{state.JobHostUnits}},
737- {false, []state.MachineJob{state.JobHostUnits, state.JobManageEnviron}},
738+ {true, []state.MachineJob{state.JobManageState}},
739+ {true, []state.MachineJob{state.JobManageEnviron}},
740+ {true, []state.MachineJob{state.JobHostUnits, state.JobManageState}},
741+ {true, []state.MachineJob{state.JobHostUnits, state.JobManageEnviron}},
742 {true, []state.MachineJob{state.JobHostUnits, state.JobManageState, state.JobManageEnviron}},
743- {true, []state.MachineJob{state.JobManageState}},
744 }
745 for _, test := range tests {
746 params := state.AddMachineParams{
747@@ -84,7 +86,7 @@
748 }
749 m, err := s.State.AddMachineWithConstraints(&params)
750 c.Assert(err, gc.IsNil)
751- c.Assert(m.IsStateServer(), gc.Equals, test.isStateServer)
752+ c.Assert(m.IsManager(), gc.Equals, test.isStateServer)
753 }
754 }
755
756@@ -94,6 +96,8 @@
757 c.Assert(err, gc.IsNil)
758 err = m.Destroy()
759 c.Assert(err, gc.ErrorMatches, "machine 1 is required by the environment")
760+ err = m.ForceDestroy()
761+ c.Assert(err, gc.ErrorMatches, "machine 1 is required by the environment")
762 err = m.EnsureDead()
763 c.Assert(err, gc.ErrorMatches, "machine 1 is required by the environment")
764 }
765@@ -218,41 +222,6 @@
766 c.Assert(err, gc.IsNil)
767 }
768
769-func (s *MachineSuite) TestDestroyMachines(c *gc.C) {
770- m0 := s.machine
771- m1, err := s.State.AddMachine("quantal", state.JobManageEnviron)
772- c.Assert(err, gc.IsNil)
773- m2, err := s.State.AddMachine("quantal", state.JobHostUnits)
774- c.Assert(err, gc.IsNil)
775-
776- sch := s.AddTestingCharm(c, "wordpress")
777- wordpress, err := s.State.AddService("wordpress", sch)
778- c.Assert(err, gc.IsNil)
779- u, err := wordpress.AddUnit()
780- c.Assert(err, gc.IsNil)
781- err = u.AssignToMachine(m0)
782- c.Assert(err, gc.IsNil)
783-
784- err = s.State.DestroyMachines("0", "1", "2")
785- c.Assert(err, gc.ErrorMatches, `some machines were not destroyed: machine 0 has unit "wordpress/0" assigned; machine 1 is required by the environment`)
786- assertLife := func(m *state.Machine, life state.Life) {
787- err := m.Refresh()
788- c.Assert(err, gc.IsNil)
789- c.Assert(m.Life(), gc.Equals, life)
790- }
791- assertLife(m0, state.Alive)
792- assertLife(m1, state.Alive)
793- assertLife(m2, state.Dying)
794-
795- err = u.UnassignFromMachine()
796- c.Assert(err, gc.IsNil)
797- err = s.State.DestroyMachines("0", "1", "2")
798- c.Assert(err, gc.ErrorMatches, `some machines were not destroyed: machine 1 is required by the environment`)
799- assertLife(m0, state.Dying)
800- assertLife(m1, state.Alive)
801- assertLife(m2, state.Dying)
802-}
803-
804 func (s *MachineSuite) TestMachineSetAgentAlive(c *gc.C) {
805 alive, err := s.machine.AgentAlive()
806 c.Assert(err, gc.IsNil)
807@@ -627,11 +596,11 @@
808 func (s *MachineSuite) TestWatchDiesOnStateClose(c *gc.C) {
809 // This test is testing logic in watcher.entityWatcher, which
810 // is also used by:
811- // Machine.WatchHardwareCharacteristics
812- // Service.Watch
813- // Unit.Watch
814- // State.WatchForEnvironConfigChanges
815- // Unit.WatchConfigSettings
816+ // Machine.WatchHardwareCharacteristics
817+ // Service.Watch
818+ // Unit.Watch
819+ // State.WatchForEnvironConfigChanges
820+ // Unit.WatchConfigSettings
821 testWatcherDiesWhenStateCloses(c, func(c *gc.C, st *state.State) waiter {
822 m, err := st.Machine(s.machine.Id())
823 c.Assert(err, gc.IsNil)
824
825=== modified file 'state/service_test.go'
826--- state/service_test.go 2013-09-27 19:20:20 +0000
827+++ state/service_test.go 2013-11-12 10:22:12 +0000
828@@ -1090,7 +1090,7 @@
829 err = s.mysql.Destroy()
830 c.Assert(err, gc.IsNil)
831 for _, unit := range units {
832- assertUnitLife(c, unit, state.Alive)
833+ assertLife(c, unit, state.Alive)
834 }
835
836 // Check a cleanup doc was added.
837@@ -1103,9 +1103,9 @@
838 c.Assert(err, gc.IsNil)
839 for i, unit := range units {
840 if i%2 != 0 {
841- assertUnitLife(c, unit, state.Dying)
842+ assertLife(c, unit, state.Dying)
843 } else {
844- assertUnitRemoved(c, unit)
845+ assertRemoved(c, unit)
846 }
847 }
848
849
850=== modified file 'state/state.go'
851--- state/state.go 2013-11-08 16:15:10 +0000
852+++ state/state.go 2013-11-12 10:22:12 +0000
853@@ -1051,73 +1051,6 @@
854 return newUnit(st, &doc), nil
855 }
856
857-// DestroyUnits destroys the units with the specified names.
858-func (st *State) DestroyUnits(names ...string) (err error) {
859- // TODO(rog) make this a transaction?
860- var errs []string
861- for _, name := range names {
862- unit, err := st.Unit(name)
863- switch {
864- case errors.IsNotFoundError(err):
865- err = fmt.Errorf("unit %q does not exist", name)
866- case err != nil:
867- case unit.Life() != Alive:
868- continue
869- case unit.IsPrincipal():
870- err = unit.Destroy()
871- default:
872- err = fmt.Errorf("unit %q is a subordinate", name)
873- }
874- if err != nil {
875- errs = append(errs, err.Error())
876- }
877- }
878- return destroyErr("units", names, errs)
879-}
880-
881-// DestroyMachines destroys the machines with the specified ids.
882-func (st *State) DestroyMachines(ids ...string) (err error) {
883- var errs []string
884- for _, id := range ids {
885- machine, err := st.Machine(id)
886- switch {
887- case errors.IsNotFoundError(err):
888- err = fmt.Errorf("machine %s does not exist", id)
889- case err != nil:
890- case machine.Life() != Alive:
891- continue
892- default:
893- err = machine.Destroy()
894- }
895- if err != nil {
896- errs = append(errs, err.Error())
897- }
898- }
899- return destroyErr("machines", ids, errs)
900-}
901-
902-func destroyErr(desc string, ids, errs []string) error {
903- if len(errs) == 0 {
904- return nil
905- }
906- msg := "some %s were not destroyed"
907- if len(errs) == len(ids) {
908- msg = "no %s were destroyed"
909- }
910- msg = fmt.Sprintf(msg, desc)
911- return fmt.Errorf("%s: %s", msg, strings.Join(errs, "; "))
912-}
913-
914-// ForceDestroyMachines queues the machines with the specified ids for
915-// complete removal, including the destruction of all units on the machine.
916-func (st *State) ForceDestroyMachines(ids ...string) error {
917- var ops []txn.Op
918- for _, id := range ids {
919- ops = append(ops, st.newCleanupOp("machine", id))
920- }
921- return st.runTransaction(ops)
922-}
923-
924 // AssignUnit places the unit on a machine. Depending on the policy, and the
925 // state of the environment, this may lead to new instances being launched
926 // within the environment.
927
928=== modified file 'state/unit_test.go'
929--- state/unit_test.go 2013-11-07 17:30:38 +0000
930+++ state/unit_test.go 2013-11-12 10:22:12 +0000
931@@ -435,49 +435,12 @@
932 c.Assert(err, gc.ErrorMatches, `unit "wordpress/0" is dead`)
933 }
934
935-func (s *UnitSuite) TestDestroyPrincipalUnits(c *gc.C) {
936- preventUnitDestroyRemove(c, s.unit)
937- for i := 0; i < 4; i++ {
938- unit, err := s.service.AddUnit()
939- c.Assert(err, gc.IsNil)
940- preventUnitDestroyRemove(c, unit)
941- }
942-
943- // Destroy 2 of them; check they become Dying.
944- err := s.State.DestroyUnits("wordpress/0", "wordpress/1")
945- c.Assert(err, gc.IsNil)
946- s.assertUnitLife(c, "wordpress/0", state.Dying)
947- s.assertUnitLife(c, "wordpress/1", state.Dying)
948-
949- // Try to destroy an Alive one and a Dying one; check
950- // it destroys the Alive one and ignores the Dying one.
951- err = s.State.DestroyUnits("wordpress/2", "wordpress/0")
952- c.Assert(err, gc.IsNil)
953- s.assertUnitLife(c, "wordpress/2", state.Dying)
954-
955- // Try to destroy an Alive one along with a nonexistent one; check that
956- // the valid instruction is followed but the invalid one is warned about.
957- err = s.State.DestroyUnits("boojum/123", "wordpress/3")
958- c.Assert(err, gc.ErrorMatches, `some units were not destroyed: unit "boojum/123" does not exist`)
959- s.assertUnitLife(c, "wordpress/3", state.Dying)
960-
961- // Make one Dead, and destroy an Alive one alongside it; check no errors.
962- wp0, err := s.State.Unit("wordpress/0")
963- c.Assert(err, gc.IsNil)
964- err = wp0.EnsureDead()
965- c.Assert(err, gc.IsNil)
966- err = s.State.DestroyUnits("wordpress/0", "wordpress/4")
967- c.Assert(err, gc.IsNil)
968- s.assertUnitLife(c, "wordpress/0", state.Dead)
969- s.assertUnitLife(c, "wordpress/4", state.Dying)
970-}
971-
972 func (s *UnitSuite) TestDestroySetStatusRetry(c *gc.C) {
973 defer state.SetRetryHooks(c, s.State, func() {
974 err := s.unit.SetStatus(params.StatusStarted, "", nil)
975 c.Assert(err, gc.IsNil)
976 }, func() {
977- assertUnitLife(c, s.unit, state.Dying)
978+ assertLife(c, s.unit, state.Dying)
979 }).Check()
980 err := s.unit.Destroy()
981 c.Assert(err, gc.IsNil)
982@@ -488,7 +451,7 @@
983 err := s.unit.SetCharmURL(s.charm.URL())
984 c.Assert(err, gc.IsNil)
985 }, func() {
986- assertUnitRemoved(c, s.unit)
987+ assertRemoved(c, s.unit)
988 }).Check()
989 err := s.unit.Destroy()
990 c.Assert(err, gc.IsNil)
991@@ -505,7 +468,7 @@
992 err := s.unit.SetCharmURL(newCharm.URL())
993 c.Assert(err, gc.IsNil)
994 }, func() {
995- assertUnitRemoved(c, s.unit)
996+ assertRemoved(c, s.unit)
997 }).Check()
998 err = s.unit.Destroy()
999 c.Assert(err, gc.IsNil)
1000@@ -519,7 +482,7 @@
1001 err := s.unit.AssignToMachine(machine)
1002 c.Assert(err, gc.IsNil)
1003 }, func() {
1004- assertUnitRemoved(c, s.unit)
1005+ assertRemoved(c, s.unit)
1006 // Also check the unit ref was properly removed from the machine doc --
1007 // if it weren't, we wouldn't be able to make the machine Dead.
1008 err := machine.EnsureDead()
1009@@ -539,7 +502,7 @@
1010 err := s.unit.UnassignFromMachine()
1011 c.Assert(err, gc.IsNil)
1012 }, func() {
1013- assertUnitRemoved(c, s.unit)
1014+ assertRemoved(c, s.unit)
1015 }).Check()
1016 err = s.unit.Destroy()
1017 c.Assert(err, gc.IsNil)
1018@@ -550,7 +513,7 @@
1019 err := s.unit.Destroy()
1020 c.Assert(err, gc.IsNil)
1021 c.Assert(s.unit.Life(), gc.Equals, state.Dying)
1022- assertUnitRemoved(c, s.unit)
1023+ assertRemoved(c, s.unit)
1024 }
1025
1026 func (s *UnitSuite) TestCannotShortCircuitDestroyWithSubordinates(c *gc.C) {
1027@@ -568,7 +531,7 @@
1028 err = s.unit.Destroy()
1029 c.Assert(err, gc.IsNil)
1030 c.Assert(s.unit.Life(), gc.Equals, state.Dying)
1031- assertUnitLife(c, s.unit, state.Dying)
1032+ assertLife(c, s.unit, state.Dying)
1033 }
1034
1035 func (s *UnitSuite) TestCannotShortCircuitDestroyWithStatus(c *gc.C) {
1036@@ -592,7 +555,7 @@
1037 err = unit.Destroy()
1038 c.Assert(err, gc.IsNil)
1039 c.Assert(unit.Life(), gc.Equals, state.Dying)
1040- assertUnitLife(c, unit, state.Dying)
1041+ assertLife(c, unit, state.Dying)
1042 }
1043 }
1044
1045@@ -610,56 +573,25 @@
1046 err = s.unit.Destroy()
1047 c.Assert(err, gc.IsNil)
1048 c.Assert(s.unit.Life(), gc.Equals, state.Dying)
1049- assertUnitRemoved(c, s.unit)
1050-}
1051-
1052-func (s *UnitSuite) TestDestroySubordinateUnits(c *gc.C) {
1053- lgsch := s.AddTestingCharm(c, "logging")
1054- _, err := s.State.AddService("logging", lgsch)
1055- c.Assert(err, gc.IsNil)
1056- eps, err := s.State.InferEndpoints([]string{"logging", "wordpress"})
1057- c.Assert(err, gc.IsNil)
1058- rel, err := s.State.AddRelation(eps...)
1059- c.Assert(err, gc.IsNil)
1060- ru, err := rel.Unit(s.unit)
1061- c.Assert(err, gc.IsNil)
1062- err = ru.EnterScope(nil)
1063- c.Assert(err, gc.IsNil)
1064-
1065- // Try to destroy the subordinate alone; check it fails.
1066- err = s.State.DestroyUnits("logging/0")
1067- c.Assert(err, gc.ErrorMatches, `no units were destroyed: unit "logging/0" is a subordinate`)
1068- s.assertUnitLife(c, "logging/0", state.Alive)
1069-
1070- // Try to destroy the principal and the subordinate together; check it warns
1071- // about the subordinate, but destroys the one it can. (The principal unit
1072- // agent will be resposible for destroying the subordinate.)
1073- err = s.State.DestroyUnits("wordpress/0", "logging/0")
1074- c.Assert(err, gc.ErrorMatches, `some units were not destroyed: unit "logging/0" is a subordinate`)
1075- s.assertUnitLife(c, "wordpress/0", state.Dying)
1076- s.assertUnitLife(c, "logging/0", state.Alive)
1077-}
1078-
1079-func (s *UnitSuite) assertUnitLife(c *gc.C, name string, life state.Life) {
1080- unit, err := s.State.Unit(name)
1081- c.Assert(err, gc.IsNil)
1082- assertUnitLife(c, unit, life)
1083-}
1084-
1085-func assertUnitLife(c *gc.C, unit *state.Unit, life state.Life) {
1086- c.Assert(unit.Refresh(), gc.IsNil)
1087- c.Assert(unit.Life(), gc.Equals, life)
1088-}
1089-
1090-func assertUnitRemoved(c *gc.C, unit *state.Unit) {
1091- err := unit.Refresh()
1092+ assertRemoved(c, s.unit)
1093+}
1094+
1095+func assertLife(c *gc.C, entity state.Living, life state.Life) {
1096+ c.Assert(entity.Refresh(), gc.IsNil)
1097+ c.Assert(entity.Life(), gc.Equals, life)
1098+}
1099+
1100+func assertRemoved(c *gc.C, entity state.Living) {
1101+ err := entity.Refresh()
1102 c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
1103- err = unit.Destroy()
1104- c.Assert(err, gc.IsNil)
1105- err = unit.EnsureDead()
1106- c.Assert(err, gc.IsNil)
1107- err = unit.Remove()
1108- c.Assert(err, gc.IsNil)
1109+ err = entity.Destroy()
1110+ c.Assert(err, gc.IsNil)
1111+ if entity, ok := entity.(state.AgentLiving); ok {
1112+ err = entity.EnsureDead()
1113+ c.Assert(err, gc.IsNil)
1114+ err = entity.Remove()
1115+ c.Assert(err, gc.IsNil)
1116+ }
1117 }
1118
1119 func (s *UnitSuite) TestTag(c *gc.C) {

Subscribers

People subscribed via source and target branches

to status/vote changes: