Merge lp:~axwalk/juju-core/lp1260171-stopinstances-ids into lp:~go-bot/juju-core/trunk
- lp1260171-stopinstances-ids
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Andrew Wilkins |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2728 |
Proposed branch: | lp:~axwalk/juju-core/lp1260171-stopinstances-ids |
Merge into: | lp:~go-bot/juju-core/trunk |
Diff against target: |
1305 lines (+232/-257) 38 files modified
container/interface.go (+7/-3) container/kvm/kvm.go (+2/-2) container/kvm/kvm_test.go (+1/-1) container/kvm/live_test.go (+1/-1) container/lxc/lxc.go (+2/-2) container/lxc/lxc_test.go (+5/-5) dependencies.tsv (+1/-1) environs/broker.go (+3/-2) environs/jujutest/livetests.go (+6/-6) environs/jujutest/tests.go (+1/-1) provider/azure/environ.go (+17/-16) provider/azure/environ_test.go (+11/-12) provider/common/bootstrap.go (+1/-1) provider/common/bootstrap_test.go (+7/-7) provider/common/destroy.go (+6/-1) provider/common/destroy_test.go (+11/-11) provider/common/mock_test.go (+3/-3) provider/dummy/environs.go (+7/-7) provider/ec2/ec2.go (+1/-5) provider/ec2/live_test.go (+5/-5) provider/joyent/environ_instance.go (+39/-6) provider/joyent/instance.go (+0/-41) provider/joyent/local_test.go (+3/-3) provider/local/environ.go (+5/-5) provider/maas/environ.go (+11/-25) provider/maas/environ_whitebox_test.go (+17/-13) provider/maas/util.go (+3/-3) provider/maas/util_test.go (+1/-1) provider/manual/environ.go (+1/-1) provider/openstack/local_test.go (+8/-8) provider/openstack/provider.go (+23/-19) state/apiserver/client/destroy.go (+1/-22) worker/provisioner/kvm-broker.go (+4/-4) worker/provisioner/kvm-broker_test.go (+3/-3) worker/provisioner/lxc-broker.go (+4/-4) worker/provisioner/lxc-broker_test.go (+3/-3) worker/provisioner/provisioner_task.go (+6/-2) worker/provisioner/provisioner_test.go (+2/-2) |
To merge this branch: | bzr merge lp:~axwalk/juju-core/lp1260171-stopinstances-ids |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email: mp+219347@code.launchpad.net |
Commit message
Change StopInstances to take []instance.Id
StopInstances previously took []instance.
which is unnecessary for most providers, and imposes
an unnecessary cost for callers that only have IDs
initially. We change StopInstances to
The openstack provider's StopInstances is now slightly
more expensive than before, requiring an additional call
to Instances. This cost will disappear when we fix the
security groups/open-port implementation.
Drive-by: the maas provider now uses a bulk "release"
in StopInstances.
Live tested on ec2, local, virtual MAAS, canonistack, azure.
I lack a Joyent account, but I think it should be fine.
Fixes lp:1260171
Fixes lp:1316272
Description of the change
Change StopInstances to take []instance.Id
StopInstances previously took []instance.
which is unnecessary for most providers, and imposes
an unnecessary cost for callers that only have IDs
initially. We change StopInstances to
The openstack provider's StopInstances is now slightly
more expensive than before, requiring an additional call
to Instances. This cost will disappear when we fix the
security groups/open-port implementation.
Drive-by: the maas provider now uses a bulk "release"
in StopInstances.
Live tested on ec2, local, virtual MAAS, canonistack, azure.
I lack a Joyent account, but I think it should be fine.
Fixes lp:1260171
Fixes lp:1316272
Andrew Wilkins (axwalk) wrote : | # |
Dave Cheney (dave-cheney) wrote : | # |
On 2014/05/13 11:58:10, axw wrote:
> Please take a look.
SGTM. How do you feel about making StopInstance variadic ?
Ian Booth (wallyworld) wrote : | # |
LGTM, agree with Dave's comment so would be good to do that before
landing
https:/
File container/
https:/
container/
comment tweak - "by instance id"
Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
https:/
File container/
https:/
container/
On 2014/05/14 02:37:16, wallyworld wrote:
> comment tweak - "by instance id"
Done.
Go Bot (go-bot) wrote : | # |
The attempt to merge lp:~axwalk/juju-core/lp1260171-stopinstances-ids into lp:juju-core failed. Below is the output from the failed tests.
godeps: cannot update "/home/
bzr update --revision only works for a revision in the branch history
mongod: no process found
Dave Cheney (dave-cheney) wrote : | # |
On Wed, May 14, 2014 at 12:37 PM, <email address hidden> wrote:
> LGTM, agree with Dave's comment so would be good to do that before
> landing
Making StopInstance variadic would make some callers initially more verbose, ie
var i []instance.Instance
p.StopInstances
of the slice, not the slice itself.
but, as I found with the change that I made to Addresses recently,
most of the callers that are creating those slices are doing so to
pass one item, so they can be refactored to be variadic themselves and
everything becomes a lot simpler to read without so much tedious
boxing of single values into slices.
Preview Diff
1 | === modified file 'container/interface.go' | |||
2 | --- container/interface.go 2014-03-19 21:08:58 +0000 | |||
3 | +++ container/interface.go 2014-05-14 03:01:15 +0000 | |||
4 | @@ -20,13 +20,17 @@ | |||
5 | 20 | // Manager is responsible for starting containers, and stopping and listing | 20 | // Manager is responsible for starting containers, and stopping and listing |
6 | 21 | // containers that it has started. | 21 | // containers that it has started. |
7 | 22 | type Manager interface { | 22 | type Manager interface { |
9 | 23 | // CreateContainer creates and starts a new container for the specified machine. | 23 | // CreateContainer creates and starts a new container for the specified |
10 | 24 | // machine. | ||
11 | 24 | CreateContainer( | 25 | CreateContainer( |
12 | 25 | machineConfig *cloudinit.MachineConfig, | 26 | machineConfig *cloudinit.MachineConfig, |
13 | 26 | series string, | 27 | series string, |
14 | 27 | network *NetworkConfig) (instance.Instance, *instance.HardwareCharacteristics, error) | 28 | network *NetworkConfig) (instance.Instance, *instance.HardwareCharacteristics, error) |
17 | 28 | // DestroyContainer stops and destroyes the container identified by Instance. | 29 | |
18 | 29 | DestroyContainer(instance.Instance) error | 30 | // DestroyContainer stops and destroyes the container identified by |
19 | 31 | // instance id. | ||
20 | 32 | DestroyContainer(instance.Id) error | ||
21 | 33 | |||
22 | 30 | // ListContainers return a list of containers that have been started by | 34 | // ListContainers return a list of containers that have been started by |
23 | 31 | // this manager. | 35 | // this manager. |
24 | 32 | ListContainers() ([]instance.Instance, error) | 36 | ListContainers() ([]instance.Instance, error) |
25 | 33 | 37 | ||
26 | === modified file 'container/kvm/kvm.go' | |||
27 | --- container/kvm/kvm.go 2014-04-17 03:41:32 +0000 | |||
28 | +++ container/kvm/kvm.go 2014-05-14 03:01:15 +0000 | |||
29 | @@ -125,8 +125,8 @@ | |||
30 | 125 | return &kvmInstance{kvmContainer, name}, &hardware, nil | 125 | return &kvmInstance{kvmContainer, name}, &hardware, nil |
31 | 126 | } | 126 | } |
32 | 127 | 127 | ||
35 | 128 | func (manager *containerManager) DestroyContainer(instance instance.Instance) error { | 128 | func (manager *containerManager) DestroyContainer(id instance.Id) error { |
36 | 129 | name := string(instance.Id()) | 129 | name := string(id) |
37 | 130 | kvmContainer := KvmObjectFactory.New(name) | 130 | kvmContainer := KvmObjectFactory.New(name) |
38 | 131 | if err := kvmContainer.Stop(); err != nil { | 131 | if err := kvmContainer.Stop(); err != nil { |
39 | 132 | logger.Errorf("failed to stop kvm container: %v", err) | 132 | logger.Errorf("failed to stop kvm container: %v", err) |
40 | 133 | 133 | ||
41 | === modified file 'container/kvm/kvm_test.go' | |||
42 | --- container/kvm/kvm_test.go 2014-04-09 16:36:12 +0000 | |||
43 | +++ container/kvm/kvm_test.go 2014-05-14 03:01:15 +0000 | |||
44 | @@ -98,7 +98,7 @@ | |||
45 | 98 | func (s *KVMSuite) TestDestroyContainer(c *gc.C) { | 98 | func (s *KVMSuite) TestDestroyContainer(c *gc.C) { |
46 | 99 | instance := containertesting.CreateContainer(c, s.manager, "1/lxc/0") | 99 | instance := containertesting.CreateContainer(c, s.manager, "1/lxc/0") |
47 | 100 | 100 | ||
49 | 101 | err := s.manager.DestroyContainer(instance) | 101 | err := s.manager.DestroyContainer(instance.Id()) |
50 | 102 | c.Assert(err, gc.IsNil) | 102 | c.Assert(err, gc.IsNil) |
51 | 103 | 103 | ||
52 | 104 | name := string(instance.Id()) | 104 | name := string(instance.Id()) |
53 | 105 | 105 | ||
54 | === modified file 'container/kvm/live_test.go' | |||
55 | --- container/kvm/live_test.go 2014-04-01 16:27:22 +0000 | |||
56 | +++ container/kvm/live_test.go 2014-05-14 03:01:15 +0000 | |||
57 | @@ -75,7 +75,7 @@ | |||
58 | 75 | instances, err := manager.ListContainers() | 75 | instances, err := manager.ListContainers() |
59 | 76 | c.Assert(err, gc.IsNil) | 76 | c.Assert(err, gc.IsNil) |
60 | 77 | for _, instance := range instances { | 77 | for _, instance := range instances { |
62 | 78 | err := manager.DestroyContainer(instance) | 78 | err := manager.DestroyContainer(instance.Id()) |
63 | 79 | c.Check(err, gc.IsNil) | 79 | c.Check(err, gc.IsNil) |
64 | 80 | } | 80 | } |
65 | 81 | } | 81 | } |
66 | 82 | 82 | ||
67 | === modified file 'container/lxc/lxc.go' | |||
68 | --- container/lxc/lxc.go 2014-03-19 21:08:58 +0000 | |||
69 | +++ container/lxc/lxc.go 2014-05-14 03:01:15 +0000 | |||
70 | @@ -269,9 +269,9 @@ | |||
71 | 269 | return appendToContainerConfig(name, line) | 269 | return appendToContainerConfig(name, line) |
72 | 270 | } | 270 | } |
73 | 271 | 271 | ||
75 | 272 | func (manager *containerManager) DestroyContainer(instance instance.Instance) error { | 272 | func (manager *containerManager) DestroyContainer(id instance.Id) error { |
76 | 273 | start := time.Now() | 273 | start := time.Now() |
78 | 274 | name := string(instance.Id()) | 274 | name := string(id) |
79 | 275 | lxcContainer := LxcObjectFactory.New(name) | 275 | lxcContainer := LxcObjectFactory.New(name) |
80 | 276 | if useRestartDir() { | 276 | if useRestartDir() { |
81 | 277 | // Remove the autostart link. | 277 | // Remove the autostart link. |
82 | 278 | 278 | ||
83 | === modified file 'container/lxc/lxc_test.go' | |||
84 | --- container/lxc/lxc_test.go 2014-03-19 21:08:58 +0000 | |||
85 | +++ container/lxc/lxc_test.go 2014-05-14 03:01:15 +0000 | |||
86 | @@ -274,7 +274,7 @@ | |||
87 | 274 | 274 | ||
88 | 275 | // DestroyContainer stops and then destroys the container, putting it | 275 | // DestroyContainer stops and then destroys the container, putting it |
89 | 276 | // into "unknown" state. | 276 | // into "unknown" state. |
91 | 277 | err := manager.DestroyContainer(instance) | 277 | err := manager.DestroyContainer(instance.Id()) |
92 | 278 | c.Assert(err, gc.IsNil) | 278 | c.Assert(err, gc.IsNil) |
93 | 279 | c.Assert(instance.Status(), gc.Equals, string(golxc.StateUnknown)) | 279 | c.Assert(instance.Status(), gc.Equals, string(golxc.StateUnknown)) |
94 | 280 | } | 280 | } |
95 | @@ -283,7 +283,7 @@ | |||
96 | 283 | manager := s.makeManager(c, "test") | 283 | manager := s.makeManager(c, "test") |
97 | 284 | instance := containertesting.CreateContainer(c, manager, "1/lxc/0") | 284 | instance := containertesting.CreateContainer(c, manager, "1/lxc/0") |
98 | 285 | 285 | ||
100 | 286 | err := manager.DestroyContainer(instance) | 286 | err := manager.DestroyContainer(instance.Id()) |
101 | 287 | c.Assert(err, gc.IsNil) | 287 | c.Assert(err, gc.IsNil) |
102 | 288 | 288 | ||
103 | 289 | name := string(instance.Id()) | 289 | name := string(instance.Id()) |
104 | @@ -302,7 +302,7 @@ | |||
105 | 302 | err := os.MkdirAll(targetDir, 0755) | 302 | err := os.MkdirAll(targetDir, 0755) |
106 | 303 | c.Assert(err, gc.IsNil) | 303 | c.Assert(err, gc.IsNil) |
107 | 304 | 304 | ||
109 | 305 | err = manager.DestroyContainer(instance) | 305 | err = manager.DestroyContainer(instance.Id()) |
110 | 306 | c.Assert(err, gc.IsNil) | 306 | c.Assert(err, gc.IsNil) |
111 | 307 | 307 | ||
112 | 308 | // Check that the container dir is no longer in the container dir | 308 | // Check that the container dir is no longer in the container dir |
113 | @@ -368,7 +368,7 @@ | |||
114 | 368 | func (s *LxcSuite) TestDestroyContainerRemovesAutostartLink(c *gc.C) { | 368 | func (s *LxcSuite) TestDestroyContainerRemovesAutostartLink(c *gc.C) { |
115 | 369 | manager := s.makeManager(c, "test") | 369 | manager := s.makeManager(c, "test") |
116 | 370 | instance := containertesting.CreateContainer(c, manager, "1/lxc/0") | 370 | instance := containertesting.CreateContainer(c, manager, "1/lxc/0") |
118 | 371 | err := manager.DestroyContainer(instance) | 371 | err := manager.DestroyContainer(instance.Id()) |
119 | 372 | c.Assert(err, gc.IsNil) | 372 | c.Assert(err, gc.IsNil) |
120 | 373 | autostartLink := lxc.RestartSymlink(string(instance.Id())) | 373 | autostartLink := lxc.RestartSymlink(string(instance.Id())) |
121 | 374 | c.Assert(autostartLink, jc.SymlinkDoesNotExist) | 374 | c.Assert(autostartLink, jc.SymlinkDoesNotExist) |
122 | @@ -380,7 +380,7 @@ | |||
123 | 380 | 380 | ||
124 | 381 | manager := s.makeManager(c, "test") | 381 | manager := s.makeManager(c, "test") |
125 | 382 | instance := containertesting.CreateContainer(c, manager, "1/lxc/0") | 382 | instance := containertesting.CreateContainer(c, manager, "1/lxc/0") |
127 | 383 | err = manager.DestroyContainer(instance) | 383 | err = manager.DestroyContainer(instance.Id()) |
128 | 384 | c.Assert(err, gc.IsNil) | 384 | c.Assert(err, gc.IsNil) |
129 | 385 | } | 385 | } |
130 | 386 | 386 | ||
131 | 387 | 387 | ||
132 | === modified file 'dependencies.tsv' | |||
133 | --- dependencies.tsv 2014-04-30 22:17:51 +0000 | |||
134 | +++ dependencies.tsv 2014-05-14 03:01:15 +0000 | |||
135 | @@ -13,7 +13,7 @@ | |||
136 | 13 | launchpad.net/goamz bzr roger.peppe@canonical.com-20131218155244-hbnkvlkkzy3vmlh9 44 | 13 | launchpad.net/goamz bzr roger.peppe@canonical.com-20131218155244-hbnkvlkkzy3vmlh9 44 |
137 | 14 | launchpad.net/gocheck bzr gustavo@niemeyer.net-20140225173054-xu9zlkf9kxhvow02 87 | 14 | launchpad.net/gocheck bzr gustavo@niemeyer.net-20140225173054-xu9zlkf9kxhvow02 87 |
138 | 15 | launchpad.net/golxc bzr tim.penhey@canonical.com-20140311005930-b14361bwnocu3krh 8 | 15 | launchpad.net/golxc bzr tim.penhey@canonical.com-20140311005930-b14361bwnocu3krh 8 |
140 | 16 | launchpad.net/gomaasapi bzr martin.packman@canonical.com-20140410114803-rsv7r2a2mgs0z0q4 49 | 16 | launchpad.net/gomaasapi bzr andrew.wilkins@canonical.com-20140513111813-kstzbs2kx1ujl3m3 50 |
141 | 17 | launchpad.net/goose bzr tarmac-20140423072524-vxav71c7ko4lgtcu 119 | 17 | launchpad.net/goose bzr tarmac-20140423072524-vxav71c7ko4lgtcu 119 |
142 | 18 | launchpad.net/goyaml bzr gustavo@niemeyer.net-20131114120802-abe042syx64z2m7s 50 | 18 | launchpad.net/goyaml bzr gustavo@niemeyer.net-20131114120802-abe042syx64z2m7s 50 |
143 | 19 | launchpad.net/gwacl bzr tarmac-20140312041035-ac7gw7kcqjx7db63 234 | 19 | launchpad.net/gwacl bzr tarmac-20140312041035-ac7gw7kcqjx7db63 234 |
144 | 20 | 20 | ||
145 | === modified file 'environs/broker.go' | |||
146 | --- environs/broker.go 2014-04-22 09:23:39 +0000 | |||
147 | +++ environs/broker.go 2014-05-14 03:01:15 +0000 | |||
148 | @@ -50,8 +50,9 @@ | |||
149 | 50 | // id. | 50 | // id. |
150 | 51 | StartInstance(args StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) | 51 | StartInstance(args StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) |
151 | 52 | 52 | ||
154 | 53 | // StopInstances shuts down the given instances. | 53 | // StopInstances shuts down the instances with the specified IDs. |
155 | 54 | StopInstances([]instance.Instance) error | 54 | // Unknown instance IDs are ignored, to enable idempotency. |
156 | 55 | StopInstances(...instance.Id) error | ||
157 | 55 | 56 | ||
158 | 56 | // AllInstances returns all instances currently known to the broker. | 57 | // AllInstances returns all instances currently known to the broker. |
159 | 57 | AllInstances() ([]instance.Instance, error) | 58 | AllInstances() ([]instance.Instance, error) |
160 | 58 | 59 | ||
161 | === modified file 'environs/jujutest/livetests.go' | |||
162 | --- environs/jujutest/livetests.go 2014-04-24 02:27:38 +0000 | |||
163 | +++ environs/jujutest/livetests.go 2014-05-14 03:01:15 +0000 | |||
164 | @@ -208,7 +208,7 @@ | |||
165 | 208 | c.Check(insts[0].Id(), gc.Equals, id0) | 208 | c.Check(insts[0].Id(), gc.Equals, id0) |
166 | 209 | c.Check(insts[1], gc.IsNil) | 209 | c.Check(insts[1], gc.IsNil) |
167 | 210 | 210 | ||
169 | 211 | err = t.Env.StopInstances([]instance.Instance{inst}) | 211 | err = t.Env.StopInstances(inst.Id()) |
170 | 212 | c.Assert(err, gc.IsNil) | 212 | c.Assert(err, gc.IsNil) |
171 | 213 | 213 | ||
172 | 214 | // The machine may not be marked as shutting down | 214 | // The machine may not be marked as shutting down |
173 | @@ -229,7 +229,7 @@ | |||
174 | 229 | 229 | ||
175 | 230 | inst1, _ := testing.AssertStartInstance(c, t.Env, "1") | 230 | inst1, _ := testing.AssertStartInstance(c, t.Env, "1") |
176 | 231 | c.Assert(inst1, gc.NotNil) | 231 | c.Assert(inst1, gc.NotNil) |
178 | 232 | defer t.Env.StopInstances([]instance.Instance{inst1}) | 232 | defer t.Env.StopInstances(inst1.Id()) |
179 | 233 | ports, err := inst1.Ports("1") | 233 | ports, err := inst1.Ports("1") |
180 | 234 | c.Assert(err, gc.IsNil) | 234 | c.Assert(err, gc.IsNil) |
181 | 235 | c.Assert(ports, gc.HasLen, 0) | 235 | c.Assert(ports, gc.HasLen, 0) |
182 | @@ -239,7 +239,7 @@ | |||
183 | 239 | ports, err = inst2.Ports("2") | 239 | ports, err = inst2.Ports("2") |
184 | 240 | c.Assert(err, gc.IsNil) | 240 | c.Assert(err, gc.IsNil) |
185 | 241 | c.Assert(ports, gc.HasLen, 0) | 241 | c.Assert(ports, gc.HasLen, 0) |
187 | 242 | defer t.Env.StopInstances([]instance.Instance{inst2}) | 242 | defer t.Env.StopInstances(inst2.Id()) |
188 | 243 | 243 | ||
189 | 244 | // Open some ports and check they're there. | 244 | // Open some ports and check they're there. |
190 | 245 | err = inst1.OpenPorts("1", []instance.Port{{"udp", 67}, {"tcp", 45}}) | 245 | err = inst1.OpenPorts("1", []instance.Port{{"udp", 67}, {"tcp", 45}}) |
191 | @@ -332,7 +332,7 @@ | |||
192 | 332 | 332 | ||
193 | 333 | // Create instances and check open ports on both instances. | 333 | // Create instances and check open ports on both instances. |
194 | 334 | inst1, _ := testing.AssertStartInstance(c, t.Env, "1") | 334 | inst1, _ := testing.AssertStartInstance(c, t.Env, "1") |
196 | 335 | defer t.Env.StopInstances([]instance.Instance{inst1}) | 335 | defer t.Env.StopInstances(inst1.Id()) |
197 | 336 | ports, err := t.Env.Ports() | 336 | ports, err := t.Env.Ports() |
198 | 337 | c.Assert(err, gc.IsNil) | 337 | c.Assert(err, gc.IsNil) |
199 | 338 | c.Assert(ports, gc.HasLen, 0) | 338 | c.Assert(ports, gc.HasLen, 0) |
200 | @@ -341,7 +341,7 @@ | |||
201 | 341 | ports, err = t.Env.Ports() | 341 | ports, err = t.Env.Ports() |
202 | 342 | c.Assert(err, gc.IsNil) | 342 | c.Assert(err, gc.IsNil) |
203 | 343 | c.Assert(ports, gc.HasLen, 0) | 343 | c.Assert(ports, gc.HasLen, 0) |
205 | 344 | defer t.Env.StopInstances([]instance.Instance{inst2}) | 344 | defer t.Env.StopInstances(inst2.Id()) |
206 | 345 | 345 | ||
207 | 346 | err = t.Env.OpenPorts([]instance.Port{{"udp", 67}, {"tcp", 45}, {"tcp", 89}, {"tcp", 99}}) | 346 | err = t.Env.OpenPorts([]instance.Port{{"udp", 67}, {"tcp", 45}, {"tcp", 89}, {"tcp", 99}}) |
208 | 347 | c.Assert(err, gc.IsNil) | 347 | c.Assert(err, gc.IsNil) |
209 | @@ -865,7 +865,7 @@ | |||
210 | 865 | MachineConfig: machineConfig, | 865 | MachineConfig: machineConfig, |
211 | 866 | }) | 866 | }) |
212 | 867 | if inst != nil { | 867 | if inst != nil { |
214 | 868 | err := t.Env.StopInstances([]instance.Instance{inst}) | 868 | err := t.Env.StopInstances(inst.Id()) |
215 | 869 | c.Check(err, gc.IsNil) | 869 | c.Check(err, gc.IsNil) |
216 | 870 | } | 870 | } |
217 | 871 | c.Assert(inst, gc.IsNil) | 871 | c.Assert(inst, gc.IsNil) |
218 | 872 | 872 | ||
219 | === modified file 'environs/jujutest/tests.go' | |||
220 | --- environs/jujutest/tests.go 2014-05-06 21:40:29 +0000 | |||
221 | +++ environs/jujutest/tests.go 2014-05-14 03:01:15 +0000 | |||
222 | @@ -114,7 +114,7 @@ | |||
223 | 114 | c.Assert(insts, gc.HasLen, 2) | 114 | c.Assert(insts, gc.HasLen, 2) |
224 | 115 | c.Assert(insts[0].Id(), gc.Not(gc.Equals), insts[1].Id()) | 115 | c.Assert(insts[0].Id(), gc.Not(gc.Equals), insts[1].Id()) |
225 | 116 | 116 | ||
227 | 117 | err = e.StopInstances([]instance.Instance{inst0}) | 117 | err = e.StopInstances(inst0.Id()) |
228 | 118 | c.Assert(err, gc.IsNil) | 118 | c.Assert(err, gc.IsNil) |
229 | 119 | 119 | ||
230 | 120 | insts, err = e.Instances([]instance.Id{id0, id1}) | 120 | insts, err = e.Instances([]instance.Id{id0, id1}) |
231 | 121 | 121 | ||
232 | === modified file 'provider/azure/environ.go' | |||
233 | --- provider/azure/environ.go 2014-05-06 16:08:51 +0000 | |||
234 | +++ provider/azure/environ.go 2014-05-14 03:01:15 +0000 | |||
235 | @@ -475,7 +475,7 @@ | |||
236 | 475 | var inst instance.Instance | 475 | var inst instance.Instance |
237 | 476 | defer func() { | 476 | defer func() { |
238 | 477 | if inst != nil && resultErr != nil { | 477 | if inst != nil && resultErr != nil { |
240 | 478 | if err := env.StopInstances([]instance.Instance{inst}); err != nil { | 478 | if err := env.StopInstances(inst.Id()); err != nil { |
241 | 479 | // Failure upon failure. Log it, but return the original error. | 479 | // Failure upon failure. Log it, but return the original error. |
242 | 480 | logger.Errorf("error releasing failed instance: %v", err) | 480 | logger.Errorf("error releasing failed instance: %v", err) |
243 | 481 | } | 481 | } |
244 | @@ -760,7 +760,7 @@ | |||
245 | 760 | } | 760 | } |
246 | 761 | 761 | ||
247 | 762 | // StopInstances is specified in the InstanceBroker interface. | 762 | // StopInstances is specified in the InstanceBroker interface. |
249 | 763 | func (env *azureEnviron) StopInstances(instances []instance.Instance) error { | 763 | func (env *azureEnviron) StopInstances(ids ...instance.Id) error { |
250 | 764 | context, err := env.getManagementAPI() | 764 | context, err := env.getManagementAPI() |
251 | 765 | if err != nil { | 765 | if err != nil { |
252 | 766 | return err | 766 | return err |
253 | @@ -769,18 +769,18 @@ | |||
254 | 769 | 769 | ||
255 | 770 | // Map services to role names we want to delete. | 770 | // Map services to role names we want to delete. |
256 | 771 | serviceInstances := make(map[string]map[string]bool) | 771 | serviceInstances := make(map[string]map[string]bool) |
269 | 772 | for _, instance := range instances { | 772 | for _, id := range ids { |
270 | 773 | instance, ok := instance.(*azureInstance) | 773 | serviceName, roleName := env.splitInstanceId(id) |
271 | 774 | if !ok { | 774 | if roleName == "" { |
272 | 775 | continue | 775 | serviceInstances[serviceName] = nil |
273 | 776 | } | 776 | } else { |
274 | 777 | serviceName := instance.hostedService.ServiceName | 777 | deleteRoleNames, ok := serviceInstances[serviceName] |
275 | 778 | deleteRoleNames, ok := serviceInstances[serviceName] | 778 | if !ok { |
276 | 779 | if !ok { | 779 | deleteRoleNames = make(map[string]bool) |
277 | 780 | deleteRoleNames = make(map[string]bool) | 780 | serviceInstances[serviceName] = deleteRoleNames |
278 | 781 | serviceInstances[serviceName] = deleteRoleNames | 781 | } |
279 | 782 | } | 782 | deleteRoleNames[roleName] = true |
280 | 783 | deleteRoleNames[instance.roleName] = true | 783 | } |
281 | 784 | } | 784 | } |
282 | 785 | 785 | ||
283 | 786 | // Load the properties of each service, so we know whether to | 786 | // Load the properties of each service, so we know whether to |
284 | @@ -806,8 +806,9 @@ | |||
285 | 806 | } | 806 | } |
286 | 807 | } | 807 | } |
287 | 808 | // If we're deleting all the roles, we need to delete the | 808 | // If we're deleting all the roles, we need to delete the |
290 | 809 | // entire cloud service or we'll get an error. | 809 | // entire cloud service or we'll get an error. deleteRoleNames |
291 | 810 | if len(deleteRoleNames) == roleNames.Size() { | 810 | // is nil if we're dealing with a legacy deployment. |
292 | 811 | if deleteRoleNames == nil || len(deleteRoleNames) == roleNames.Size() { | ||
293 | 811 | if err := context.DeleteHostedService(serviceName); err != nil { | 812 | if err := context.DeleteHostedService(serviceName); err != nil { |
294 | 812 | return err | 813 | return err |
295 | 813 | } | 814 | } |
296 | 814 | 815 | ||
297 | === modified file 'provider/azure/environ_test.go' | |||
298 | --- provider/azure/environ_test.go 2014-04-30 23:18:40 +0000 | |||
299 | +++ provider/azure/environ_test.go 2014-05-14 03:01:15 +0000 | |||
300 | @@ -762,10 +762,11 @@ | |||
301 | 762 | 762 | ||
302 | 763 | func (s *environSuite) TestStopInstancesDestroysMachines(c *gc.C) { | 763 | func (s *environSuite) TestStopInstancesDestroysMachines(c *gc.C) { |
303 | 764 | env := makeEnviron(c) | 764 | env := makeEnviron(c) |
304 | 765 | prefix := env.getEnvPrefix() | ||
305 | 765 | service1Name := "service1" | 766 | service1Name := "service1" |
307 | 766 | service1 := makeLegacyDeployment(env, service1Name) | 767 | service1 := makeLegacyDeployment(env, prefix+service1Name) |
308 | 767 | service2Name := "service2" | 768 | service2Name := "service2" |
310 | 768 | service2 := makeDeployment(env, service2Name) | 769 | service2 := makeDeployment(env, prefix+service2Name) |
311 | 769 | 770 | ||
312 | 770 | inst1, err := env.getInstance(service1, "") | 771 | inst1, err := env.getInstance(service1, "") |
313 | 771 | c.Assert(err, gc.IsNil) | 772 | c.Assert(err, gc.IsNil) |
314 | @@ -781,7 +782,7 @@ | |||
315 | 781 | responses = append(responses, buildGetServicePropertiesResponses(c, service2)...) | 782 | responses = append(responses, buildGetServicePropertiesResponses(c, service2)...) |
316 | 782 | responses = append(responses, buildStatusOKResponses(c, 1)...) // DeleteHostedService | 783 | responses = append(responses, buildStatusOKResponses(c, 1)...) // DeleteHostedService |
317 | 783 | requests := gwacl.PatchManagementAPIResponses(responses) | 784 | requests := gwacl.PatchManagementAPIResponses(responses) |
319 | 784 | err = env.StopInstances([]instance.Instance{inst1, inst2, inst3}) | 785 | err = env.StopInstances(inst1.Id(), inst2.Id(), inst3.Id()) |
320 | 785 | c.Check(err, gc.IsNil) | 786 | c.Check(err, gc.IsNil) |
321 | 786 | 787 | ||
322 | 787 | // One GET and DELETE per service | 788 | // One GET and DELETE per service |
323 | @@ -795,7 +796,7 @@ | |||
324 | 795 | 796 | ||
325 | 796 | func (s *environSuite) TestStopInstancesServiceSubset(c *gc.C) { | 797 | func (s *environSuite) TestStopInstancesServiceSubset(c *gc.C) { |
326 | 797 | env := makeEnviron(c) | 798 | env := makeEnviron(c) |
328 | 798 | service := makeDeployment(env, "service") | 799 | service := makeDeployment(env, env.getEnvPrefix()+"service") |
329 | 799 | 800 | ||
330 | 800 | role1Name := service.Deployments[0].RoleList[0].RoleName | 801 | role1Name := service.Deployments[0].RoleList[0].RoleName |
331 | 801 | inst1, err := env.getInstance(service, role1Name) | 802 | inst1, err := env.getInstance(service, role1Name) |
332 | @@ -804,7 +805,7 @@ | |||
333 | 804 | responses := buildGetServicePropertiesResponses(c, service) | 805 | responses := buildGetServicePropertiesResponses(c, service) |
334 | 805 | responses = append(responses, buildStatusOKResponses(c, 1)...) // DeleteRole | 806 | responses = append(responses, buildStatusOKResponses(c, 1)...) // DeleteRole |
335 | 806 | requests := gwacl.PatchManagementAPIResponses(responses) | 807 | requests := gwacl.PatchManagementAPIResponses(responses) |
337 | 807 | err = env.StopInstances([]instance.Instance{inst1}) | 808 | err = env.StopInstances(inst1.Id()) |
338 | 808 | c.Check(err, gc.IsNil) | 809 | c.Check(err, gc.IsNil) |
339 | 809 | 810 | ||
340 | 810 | // One GET for the service, and one DELETE for the role. | 811 | // One GET for the service, and one DELETE for the role. |
341 | @@ -817,8 +818,9 @@ | |||
342 | 817 | 818 | ||
343 | 818 | func (s *environSuite) TestStopInstancesWhenStoppingMachinesFails(c *gc.C) { | 819 | func (s *environSuite) TestStopInstancesWhenStoppingMachinesFails(c *gc.C) { |
344 | 819 | env := makeEnviron(c) | 820 | env := makeEnviron(c) |
347 | 820 | service1 := makeDeployment(env, "service1") | 821 | prefix := env.getEnvPrefix() |
348 | 821 | service2 := makeDeployment(env, "service2") | 822 | service1 := makeDeployment(env, prefix+"service1") |
349 | 823 | service2 := makeDeployment(env, prefix+"service2") | ||
350 | 822 | service1Role1Name := service1.Deployments[0].RoleList[0].RoleName | 824 | service1Role1Name := service1.Deployments[0].RoleList[0].RoleName |
351 | 823 | inst1, err := env.getInstance(service1, service1Role1Name) | 825 | inst1, err := env.getInstance(service1, service1Role1Name) |
352 | 824 | c.Assert(err, gc.IsNil) | 826 | c.Assert(err, gc.IsNil) |
353 | @@ -831,8 +833,7 @@ | |||
354 | 831 | responses = append(responses, gwacl.NewDispatcherResponse(nil, http.StatusConflict, nil)) | 833 | responses = append(responses, gwacl.NewDispatcherResponse(nil, http.StatusConflict, nil)) |
355 | 832 | requests := gwacl.PatchManagementAPIResponses(responses) | 834 | requests := gwacl.PatchManagementAPIResponses(responses) |
356 | 833 | 835 | ||
359 | 834 | instances := []instance.Instance{inst1, inst2} | 836 | err = env.StopInstances(inst1.Id(), inst2.Id()) |
358 | 835 | err = env.StopInstances(instances) | ||
360 | 836 | c.Check(err, gc.ErrorMatches, ".*Conflict.*") | 837 | c.Check(err, gc.ErrorMatches, ".*Conflict.*") |
361 | 837 | 838 | ||
362 | 838 | c.Check(len(*requests), gc.Equals, len(responses)) | 839 | c.Check(len(*requests), gc.Equals, len(responses)) |
363 | @@ -843,9 +844,7 @@ | |||
364 | 843 | 844 | ||
365 | 844 | func (s *environSuite) TestStopInstancesWithZeroInstance(c *gc.C) { | 845 | func (s *environSuite) TestStopInstancesWithZeroInstance(c *gc.C) { |
366 | 845 | env := makeEnviron(c) | 846 | env := makeEnviron(c) |
370 | 846 | instances := []instance.Instance{} | 847 | err := env.StopInstances() |
368 | 847 | |||
369 | 848 | err := env.StopInstances(instances) | ||
371 | 849 | c.Check(err, gc.IsNil) | 848 | c.Check(err, gc.IsNil) |
372 | 850 | } | 849 | } |
373 | 851 | 850 | ||
374 | 852 | 851 | ||
375 | === modified file 'provider/common/bootstrap.go' | |||
376 | --- provider/common/bootstrap.go 2014-04-30 23:18:40 +0000 | |||
377 | +++ provider/common/bootstrap.go 2014-05-14 03:01:15 +0000 | |||
378 | @@ -128,7 +128,7 @@ | |||
379 | 128 | 128 | ||
380 | 129 | if inst != nil { | 129 | if inst != nil { |
381 | 130 | fmt.Fprintln(ctx.GetStderr(), "Stopping instance...") | 130 | fmt.Fprintln(ctx.GetStderr(), "Stopping instance...") |
383 | 131 | if stoperr := env.StopInstances([]instance.Instance{inst}); stoperr != nil { | 131 | if stoperr := env.StopInstances(inst.Id()); stoperr != nil { |
384 | 132 | logger.Errorf("cannot stop failed bootstrap instance %q: %v", inst.Id(), stoperr) | 132 | logger.Errorf("cannot stop failed bootstrap instance %q: %v", inst.Id(), stoperr) |
385 | 133 | } else { | 133 | } else { |
386 | 134 | // set to nil so we know we can safely delete the state file | 134 | // set to nil so we know we can safely delete the state file |
387 | 135 | 135 | ||
388 | === modified file 'provider/common/bootstrap_test.go' | |||
389 | --- provider/common/bootstrap_test.go 2014-04-24 08:15:45 +0000 | |||
390 | +++ provider/common/bootstrap_test.go 2014-05-14 03:01:15 +0000 | |||
391 | @@ -117,9 +117,9 @@ | |||
392 | 117 | return &mockInstance{id: "i-blah"}, nil, nil, nil | 117 | return &mockInstance{id: "i-blah"}, nil, nil, nil |
393 | 118 | } | 118 | } |
394 | 119 | 119 | ||
398 | 120 | var stopped []instance.Instance | 120 | var stopped []instance.Id |
399 | 121 | stopInstances := func(instances []instance.Instance) error { | 121 | stopInstances := func(ids []instance.Id) error { |
400 | 122 | stopped = append(stopped, instances...) | 122 | stopped = append(stopped, ids...) |
401 | 123 | return nil | 123 | return nil |
402 | 124 | } | 124 | } |
403 | 125 | 125 | ||
404 | @@ -134,7 +134,7 @@ | |||
405 | 134 | err := common.Bootstrap(ctx, env, environs.BootstrapParams{}) | 134 | err := common.Bootstrap(ctx, env, environs.BootstrapParams{}) |
406 | 135 | c.Assert(err, gc.ErrorMatches, "cannot save state: suddenly a wild blah") | 135 | c.Assert(err, gc.ErrorMatches, "cannot save state: suddenly a wild blah") |
407 | 136 | c.Assert(stopped, gc.HasLen, 1) | 136 | c.Assert(stopped, gc.HasLen, 1) |
409 | 137 | c.Assert(stopped[0].Id(), gc.Equals, instance.Id("i-blah")) | 137 | c.Assert(stopped[0], gc.Equals, instance.Id("i-blah")) |
410 | 138 | } | 138 | } |
411 | 139 | 139 | ||
412 | 140 | func (s *BootstrapSuite) TestCannotRecordThenCannotStop(c *gc.C) { | 140 | func (s *BootstrapSuite) TestCannotRecordThenCannotStop(c *gc.C) { |
413 | @@ -150,8 +150,8 @@ | |||
414 | 150 | return &mockInstance{id: "i-blah"}, nil, nil, nil | 150 | return &mockInstance{id: "i-blah"}, nil, nil, nil |
415 | 151 | } | 151 | } |
416 | 152 | 152 | ||
419 | 153 | var stopped []instance.Instance | 153 | var stopped []instance.Id |
420 | 154 | stopInstances := func(instances []instance.Instance) error { | 154 | stopInstances := func(instances []instance.Id) error { |
421 | 155 | stopped = append(stopped, instances...) | 155 | stopped = append(stopped, instances...) |
422 | 156 | return fmt.Errorf("bork bork borken") | 156 | return fmt.Errorf("bork bork borken") |
423 | 157 | } | 157 | } |
424 | @@ -171,7 +171,7 @@ | |||
425 | 171 | err := common.Bootstrap(ctx, env, environs.BootstrapParams{}) | 171 | err := common.Bootstrap(ctx, env, environs.BootstrapParams{}) |
426 | 172 | c.Assert(err, gc.ErrorMatches, "cannot save state: suddenly a wild blah") | 172 | c.Assert(err, gc.ErrorMatches, "cannot save state: suddenly a wild blah") |
427 | 173 | c.Assert(stopped, gc.HasLen, 1) | 173 | c.Assert(stopped, gc.HasLen, 1) |
429 | 174 | c.Assert(stopped[0].Id(), gc.Equals, instance.Id("i-blah")) | 174 | c.Assert(stopped[0], gc.Equals, instance.Id("i-blah")) |
430 | 175 | c.Assert(tw.Log, jc.LogMatches, []jc.SimpleMessage{{ | 175 | c.Assert(tw.Log, jc.LogMatches, []jc.SimpleMessage{{ |
431 | 176 | loggo.ERROR, `cannot stop failed bootstrap instance "i-blah": bork bork borken`, | 176 | loggo.ERROR, `cannot stop failed bootstrap instance "i-blah": bork bork borken`, |
432 | 177 | }}) | 177 | }}) |
433 | 178 | 178 | ||
434 | === modified file 'provider/common/destroy.go' | |||
435 | --- provider/common/destroy.go 2014-03-14 20:38:20 +0000 | |||
436 | +++ provider/common/destroy.go 2014-05-14 03:01:15 +0000 | |||
437 | @@ -5,6 +5,7 @@ | |||
438 | 5 | 5 | ||
439 | 6 | import ( | 6 | import ( |
440 | 7 | "launchpad.net/juju-core/environs" | 7 | "launchpad.net/juju-core/environs" |
441 | 8 | "launchpad.net/juju-core/instance" | ||
442 | 8 | ) | 9 | ) |
443 | 9 | 10 | ||
444 | 10 | // Destroy is a common implementation of the Destroy method defined on | 11 | // Destroy is a common implementation of the Destroy method defined on |
445 | @@ -15,7 +16,11 @@ | |||
446 | 15 | instances, err := env.AllInstances() | 16 | instances, err := env.AllInstances() |
447 | 16 | switch err { | 17 | switch err { |
448 | 17 | case nil: | 18 | case nil: |
450 | 18 | if err := env.StopInstances(instances); err != nil { | 19 | ids := make([]instance.Id, len(instances)) |
451 | 20 | for i, inst := range instances { | ||
452 | 21 | ids[i] = inst.Id() | ||
453 | 22 | } | ||
454 | 23 | if err := env.StopInstances(ids...); err != nil { | ||
455 | 19 | return err | 24 | return err |
456 | 20 | } | 25 | } |
457 | 21 | fallthrough | 26 | fallthrough |
458 | 22 | 27 | ||
459 | === modified file 'provider/common/destroy_test.go' | |||
460 | --- provider/common/destroy_test.go 2014-04-14 12:36:13 +0000 | |||
461 | +++ provider/common/destroy_test.go 2014-05-14 03:01:15 +0000 | |||
462 | @@ -41,10 +41,10 @@ | |||
463 | 41 | &mockInstance{id: "another"}, | 41 | &mockInstance{id: "another"}, |
464 | 42 | }, nil | 42 | }, nil |
465 | 43 | }, | 43 | }, |
470 | 44 | stopInstances: func(instances []instance.Instance) error { | 44 | stopInstances: func(ids []instance.Id) error { |
471 | 45 | c.Assert(instances, gc.HasLen, 2) | 45 | c.Assert(ids, gc.HasLen, 2) |
472 | 46 | c.Assert(instances[0].Id(), gc.Equals, instance.Id("one")) | 46 | c.Assert(ids[0], gc.Equals, instance.Id("one")) |
473 | 47 | c.Assert(instances[1].Id(), gc.Equals, instance.Id("another")) | 47 | c.Assert(ids[1], gc.Equals, instance.Id("another")) |
474 | 48 | return fmt.Errorf("nah") | 48 | return fmt.Errorf("nah") |
475 | 49 | }, | 49 | }, |
476 | 50 | } | 50 | } |
477 | @@ -61,10 +61,10 @@ | |||
478 | 61 | &mockInstance{id: "another"}, | 61 | &mockInstance{id: "another"}, |
479 | 62 | }, nil | 62 | }, nil |
480 | 63 | }, | 63 | }, |
485 | 64 | stopInstances: func(instances []instance.Instance) error { | 64 | stopInstances: func(ids []instance.Id) error { |
486 | 65 | c.Assert(instances, gc.HasLen, 2) | 65 | c.Assert(ids, gc.HasLen, 2) |
487 | 66 | c.Assert(instances[0].Id(), gc.Equals, instance.Id("one")) | 66 | c.Assert(ids[0], gc.Equals, instance.Id("one")) |
488 | 67 | c.Assert(instances[1].Id(), gc.Equals, instance.Id("another")) | 67 | c.Assert(ids[1], gc.Equals, instance.Id("another")) |
489 | 68 | return nil | 68 | return nil |
490 | 69 | }, | 69 | }, |
491 | 70 | } | 70 | } |
492 | @@ -84,9 +84,9 @@ | |||
493 | 84 | &mockInstance{id: "one"}, | 84 | &mockInstance{id: "one"}, |
494 | 85 | }, nil | 85 | }, nil |
495 | 86 | }, | 86 | }, |
499 | 87 | stopInstances: func(instances []instance.Instance) error { | 87 | stopInstances: func(ids []instance.Id) error { |
500 | 88 | c.Assert(instances, gc.HasLen, 1) | 88 | c.Assert(ids, gc.HasLen, 1) |
501 | 89 | c.Assert(instances[0].Id(), gc.Equals, instance.Id("one")) | 89 | c.Assert(ids[0], gc.Equals, instance.Id("one")) |
502 | 90 | return nil | 90 | return nil |
503 | 91 | }, | 91 | }, |
504 | 92 | } | 92 | } |
505 | 93 | 93 | ||
506 | === modified file 'provider/common/mock_test.go' | |||
507 | --- provider/common/mock_test.go 2014-04-24 02:27:38 +0000 | |||
508 | +++ provider/common/mock_test.go 2014-05-14 03:01:15 +0000 | |||
509 | @@ -19,7 +19,7 @@ | |||
510 | 19 | 19 | ||
511 | 20 | type allInstancesFunc func() ([]instance.Instance, error) | 20 | type allInstancesFunc func() ([]instance.Instance, error) |
512 | 21 | type startInstanceFunc func(string, constraints.Value, []string, []string, tools.List, *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) | 21 | type startInstanceFunc func(string, constraints.Value, []string, []string, tools.List, *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) |
514 | 22 | type stopInstancesFunc func([]instance.Instance) error | 22 | type stopInstancesFunc func([]instance.Id) error |
515 | 23 | type getToolsSourcesFunc func() ([]simplestreams.DataSource, error) | 23 | type getToolsSourcesFunc func() ([]simplestreams.DataSource, error) |
516 | 24 | type configFunc func() *config.Config | 24 | type configFunc func() *config.Config |
517 | 25 | type setConfigFunc func(*config.Config) error | 25 | type setConfigFunc func(*config.Config) error |
518 | @@ -60,8 +60,8 @@ | |||
519 | 60 | args.MachineConfig) | 60 | args.MachineConfig) |
520 | 61 | } | 61 | } |
521 | 62 | 62 | ||
524 | 63 | func (env *mockEnviron) StopInstances(instances []instance.Instance) error { | 63 | func (env *mockEnviron) StopInstances(ids ...instance.Id) error { |
525 | 64 | return env.stopInstances(instances) | 64 | return env.stopInstances(ids) |
526 | 65 | } | 65 | } |
527 | 66 | 66 | ||
528 | 67 | func (env *mockEnviron) Config() *config.Config { | 67 | func (env *mockEnviron) Config() *config.Config { |
529 | 68 | 68 | ||
530 | === modified file 'provider/dummy/environs.go' | |||
531 | --- provider/dummy/environs.go 2014-05-06 16:08:51 +0000 | |||
532 | +++ provider/dummy/environs.go 2014-05-14 03:01:15 +0000 | |||
533 | @@ -130,8 +130,8 @@ | |||
534 | 130 | } | 130 | } |
535 | 131 | 131 | ||
536 | 132 | type OpStopInstances struct { | 132 | type OpStopInstances struct { |
539 | 133 | Env string | 133 | Env string |
540 | 134 | Instances []instance.Instance | 134 | Ids []instance.Id |
541 | 135 | } | 135 | } |
542 | 136 | 136 | ||
543 | 137 | type OpOpenPorts struct { | 137 | type OpOpenPorts struct { |
544 | @@ -829,7 +829,7 @@ | |||
545 | 829 | return i, hc, networkInfo, nil | 829 | return i, hc, networkInfo, nil |
546 | 830 | } | 830 | } |
547 | 831 | 831 | ||
549 | 832 | func (e *environ) StopInstances(is []instance.Instance) error { | 832 | func (e *environ) StopInstances(ids ...instance.Id) error { |
550 | 833 | defer delay() | 833 | defer delay() |
551 | 834 | if err := e.checkBroken("StopInstance"); err != nil { | 834 | if err := e.checkBroken("StopInstance"); err != nil { |
552 | 835 | return err | 835 | return err |
553 | @@ -840,12 +840,12 @@ | |||
554 | 840 | } | 840 | } |
555 | 841 | estate.mu.Lock() | 841 | estate.mu.Lock() |
556 | 842 | defer estate.mu.Unlock() | 842 | defer estate.mu.Unlock() |
559 | 843 | for _, i := range is { | 843 | for _, id := range ids { |
560 | 844 | delete(estate.insts, i.(*dummyInstance).id) | 844 | delete(estate.insts, id) |
561 | 845 | } | 845 | } |
562 | 846 | estate.ops <- OpStopInstances{ | 846 | estate.ops <- OpStopInstances{ |
565 | 847 | Env: e.name, | 847 | Env: e.name, |
566 | 848 | Instances: is, | 848 | Ids: ids, |
567 | 849 | } | 849 | } |
568 | 850 | return nil | 850 | return nil |
569 | 851 | } | 851 | } |
570 | 852 | 852 | ||
571 | === modified file 'provider/ec2/ec2.go' | |||
572 | --- provider/ec2/ec2.go 2014-05-06 16:08:51 +0000 | |||
573 | +++ provider/ec2/ec2.go 2014-05-14 03:01:15 +0000 | |||
574 | @@ -537,11 +537,7 @@ | |||
575 | 537 | return inst, &hc, nil, nil | 537 | return inst, &hc, nil, nil |
576 | 538 | } | 538 | } |
577 | 539 | 539 | ||
583 | 540 | func (e *environ) StopInstances(insts []instance.Instance) error { | 540 | func (e *environ) StopInstances(ids ...instance.Id) error { |
579 | 541 | ids := make([]instance.Id, len(insts)) | ||
580 | 542 | for i, inst := range insts { | ||
581 | 543 | ids[i] = inst.(*ec2Instance).Id() | ||
582 | 544 | } | ||
584 | 545 | return e.terminateInstances(ids) | 541 | return e.terminateInstances(ids) |
585 | 546 | } | 542 | } |
586 | 547 | 543 | ||
587 | 548 | 544 | ||
588 | === modified file 'provider/ec2/live_test.go' | |||
589 | --- provider/ec2/live_test.go 2014-04-09 16:36:12 +0000 | |||
590 | +++ provider/ec2/live_test.go 2014-05-14 03:01:15 +0000 | |||
591 | @@ -116,7 +116,7 @@ | |||
592 | 116 | 116 | ||
593 | 117 | func (t *LiveTests) TestInstanceAttributes(c *gc.C) { | 117 | func (t *LiveTests) TestInstanceAttributes(c *gc.C) { |
594 | 118 | inst, hc := testing.AssertStartInstance(c, t.Env, "30") | 118 | inst, hc := testing.AssertStartInstance(c, t.Env, "30") |
596 | 119 | defer t.Env.StopInstances([]instance.Instance{inst}) | 119 | defer t.Env.StopInstances(inst.Id()) |
597 | 120 | // Sanity check for hardware characteristics. | 120 | // Sanity check for hardware characteristics. |
598 | 121 | c.Assert(hc.Arch, gc.NotNil) | 121 | c.Assert(hc.Arch, gc.NotNil) |
599 | 122 | c.Assert(hc.Mem, gc.NotNil) | 122 | c.Assert(hc.Mem, gc.NotNil) |
600 | @@ -140,7 +140,7 @@ | |||
601 | 140 | func (t *LiveTests) TestStartInstanceConstraints(c *gc.C) { | 140 | func (t *LiveTests) TestStartInstanceConstraints(c *gc.C) { |
602 | 141 | cons := constraints.MustParse("mem=2G") | 141 | cons := constraints.MustParse("mem=2G") |
603 | 142 | inst, hc := testing.AssertStartInstanceWithConstraints(c, t.Env, "30", cons) | 142 | inst, hc := testing.AssertStartInstanceWithConstraints(c, t.Env, "30", cons) |
605 | 143 | defer t.Env.StopInstances([]instance.Instance{inst}) | 143 | defer t.Env.StopInstances(inst.Id()) |
606 | 144 | ec2inst := ec2.InstanceEC2(inst) | 144 | ec2inst := ec2.InstanceEC2(inst) |
607 | 145 | c.Assert(ec2inst.InstanceType, gc.Equals, "m1.medium") | 145 | c.Assert(ec2inst.InstanceType, gc.Equals, "m1.medium") |
608 | 146 | c.Assert(*hc.Arch, gc.Equals, "amd64") | 146 | c.Assert(*hc.Arch, gc.Equals, "amd64") |
609 | @@ -187,14 +187,14 @@ | |||
610 | 187 | c.Assert(err, gc.IsNil) | 187 | c.Assert(err, gc.IsNil) |
611 | 188 | 188 | ||
612 | 189 | inst0, _ := testing.AssertStartInstance(c, t.Env, "98") | 189 | inst0, _ := testing.AssertStartInstance(c, t.Env, "98") |
614 | 190 | defer t.Env.StopInstances([]instance.Instance{inst0}) | 190 | defer t.Env.StopInstances(inst0.Id()) |
615 | 191 | 191 | ||
616 | 192 | // Create a same-named group for the second instance | 192 | // Create a same-named group for the second instance |
617 | 193 | // before starting it, to check that it's reused correctly. | 193 | // before starting it, to check that it's reused correctly. |
618 | 194 | oldMachineGroup := createGroup(c, ec2conn, groups[2].Name, "old machine group") | 194 | oldMachineGroup := createGroup(c, ec2conn, groups[2].Name, "old machine group") |
619 | 195 | 195 | ||
620 | 196 | inst1, _ := testing.AssertStartInstance(c, t.Env, "99") | 196 | inst1, _ := testing.AssertStartInstance(c, t.Env, "99") |
622 | 197 | defer t.Env.StopInstances([]instance.Instance{inst1}) | 197 | defer t.Env.StopInstances(inst1.Id()) |
623 | 198 | 198 | ||
624 | 199 | groupsResp, err := ec2conn.SecurityGroups(groups, nil) | 199 | groupsResp, err := ec2conn.SecurityGroups(groups, nil) |
625 | 200 | c.Assert(err, gc.IsNil) | 200 | c.Assert(err, gc.IsNil) |
626 | @@ -343,7 +343,7 @@ | |||
627 | 343 | inst1 := ec2.FabricateInstance(inst0, "i-aaaaaaaa") | 343 | inst1 := ec2.FabricateInstance(inst0, "i-aaaaaaaa") |
628 | 344 | inst2, _ := testing.AssertStartInstance(c, t.Env, "41") | 344 | inst2, _ := testing.AssertStartInstance(c, t.Env, "41") |
629 | 345 | 345 | ||
631 | 346 | err := t.Env.StopInstances([]instance.Instance{inst0, inst1, inst2}) | 346 | err := t.Env.StopInstances(inst0.Id(), inst1.Id(), inst2.Id()) |
632 | 347 | c.Check(err, gc.IsNil) | 347 | c.Check(err, gc.IsNil) |
633 | 348 | 348 | ||
634 | 349 | var insts []instance.Instance | 349 | var insts []instance.Instance |
635 | 350 | 350 | ||
636 | === modified file 'provider/joyent/environ_instance.go' | |||
637 | --- provider/joyent/environ_instance.go 2014-05-06 16:08:51 +0000 | |||
638 | +++ provider/joyent/environ_instance.go 2014-05-14 03:01:15 +0000 | |||
639 | @@ -232,17 +232,17 @@ | |||
640 | 232 | return instance.Address{}, errors.NotImplementedf("AllocateAddress") | 232 | return instance.Address{}, errors.NotImplementedf("AllocateAddress") |
641 | 233 | } | 233 | } |
642 | 234 | 234 | ||
644 | 235 | func (env *joyentEnviron) StopInstances(instances []instance.Instance) error { | 235 | func (env *joyentEnviron) StopInstances(ids ...instance.Id) error { |
645 | 236 | // Remove all the instances in parallel so that we incur less round-trips. | 236 | // Remove all the instances in parallel so that we incur less round-trips. |
646 | 237 | var wg sync.WaitGroup | 237 | var wg sync.WaitGroup |
647 | 238 | //var err error | 238 | //var err error |
652 | 239 | wg.Add(len(instances)) | 239 | wg.Add(len(ids)) |
653 | 240 | errc := make(chan error, len(instances)) | 240 | errc := make(chan error, len(ids)) |
654 | 241 | for _, inst := range instances { | 241 | for _, id := range ids { |
655 | 242 | inst := inst.(*joyentInstance) | 242 | id := id // copy to new free var for closure |
656 | 243 | go func() { | 243 | go func() { |
657 | 244 | defer wg.Done() | 244 | defer wg.Done() |
659 | 245 | if err := inst.Stop(); err != nil { | 245 | if err := env.stopInstance(string(id)); err != nil { |
660 | 246 | errc <- err | 246 | errc <- err |
661 | 247 | } | 247 | } |
662 | 248 | }() | 248 | }() |
663 | @@ -257,6 +257,39 @@ | |||
664 | 257 | return nil | 257 | return nil |
665 | 258 | } | 258 | } |
666 | 259 | 259 | ||
667 | 260 | func (env *joyentEnviron) stopInstance(id string) error { | ||
668 | 261 | // wait for machine to be running | ||
669 | 262 | // if machine is still provisioning stop will fail | ||
670 | 263 | for !env.pollMachineState(id, "running") { | ||
671 | 264 | time.Sleep(1 * time.Second) | ||
672 | 265 | } | ||
673 | 266 | |||
674 | 267 | err := env.compute.cloudapi.StopMachine(id) | ||
675 | 268 | if err != nil { | ||
676 | 269 | return fmt.Errorf("cannot stop instance %s: %v", id, err) | ||
677 | 270 | } | ||
678 | 271 | |||
679 | 272 | // wait for machine to be stopped | ||
680 | 273 | for !env.pollMachineState(id, "stopped") { | ||
681 | 274 | time.Sleep(1 * time.Second) | ||
682 | 275 | } | ||
683 | 276 | |||
684 | 277 | err = env.compute.cloudapi.DeleteMachine(id) | ||
685 | 278 | if err != nil { | ||
686 | 279 | return fmt.Errorf("cannot delete instance %s: %v", id, err) | ||
687 | 280 | } | ||
688 | 281 | |||
689 | 282 | return nil | ||
690 | 283 | } | ||
691 | 284 | |||
692 | 285 | func (env *joyentEnviron) pollMachineState(machineId, state string) bool { | ||
693 | 286 | machineConfig, err := env.compute.cloudapi.GetMachine(machineId) | ||
694 | 287 | if err != nil { | ||
695 | 288 | return false | ||
696 | 289 | } | ||
697 | 290 | return strings.EqualFold(machineConfig.State, state) | ||
698 | 291 | } | ||
699 | 292 | |||
700 | 260 | func (env *joyentEnviron) listInstanceTypes() ([]instances.InstanceType, error) { | 293 | func (env *joyentEnviron) listInstanceTypes() ([]instances.InstanceType, error) { |
701 | 261 | packages, err := env.compute.cloudapi.ListPackages(nil) | 294 | packages, err := env.compute.cloudapi.ListPackages(nil) |
702 | 262 | if err != nil { | 295 | if err != nil { |
703 | 263 | 296 | ||
704 | === modified file 'provider/joyent/instance.go' | |||
705 | --- provider/joyent/instance.go 2014-04-02 11:35:49 +0000 | |||
706 | +++ provider/joyent/instance.go 2014-05-14 03:01:15 +0000 | |||
707 | @@ -4,10 +4,6 @@ | |||
708 | 4 | package joyent | 4 | package joyent |
709 | 5 | 5 | ||
710 | 6 | import ( | 6 | import ( |
711 | 7 | "fmt" | ||
712 | 8 | "strings" | ||
713 | 9 | "time" | ||
714 | 10 | |||
715 | 11 | "github.com/joyent/gosdc/cloudapi" | 7 | "github.com/joyent/gosdc/cloudapi" |
716 | 12 | 8 | ||
717 | 13 | "launchpad.net/juju-core/instance" | 9 | "launchpad.net/juju-core/instance" |
718 | @@ -63,40 +59,3 @@ | |||
719 | 63 | func (inst *joyentInstance) WaitDNSName() (string, error) { | 59 | func (inst *joyentInstance) WaitDNSName() (string, error) { |
720 | 64 | return common.WaitDNSName(inst) | 60 | return common.WaitDNSName(inst) |
721 | 65 | } | 61 | } |
722 | 66 | |||
723 | 67 | // Stop will stop and delete the machine | ||
724 | 68 | // Stopped machines are still billed for in the Joyent Public Cloud | ||
725 | 69 | func (inst *joyentInstance) Stop() error { | ||
726 | 70 | id := string(inst.Id()) | ||
727 | 71 | |||
728 | 72 | // wait for machine to be running | ||
729 | 73 | // if machine is still provisioning stop will fail | ||
730 | 74 | for !inst.pollMachineState(id, "running") { | ||
731 | 75 | time.Sleep(1 * time.Second) | ||
732 | 76 | } | ||
733 | 77 | |||
734 | 78 | err := inst.env.compute.cloudapi.StopMachine(id) | ||
735 | 79 | if err != nil { | ||
736 | 80 | return fmt.Errorf("cannot stop instance %s: %v", id, err) | ||
737 | 81 | } | ||
738 | 82 | |||
739 | 83 | // wait for machine to be stopped | ||
740 | 84 | for !inst.pollMachineState(id, "stopped") { | ||
741 | 85 | time.Sleep(1 * time.Second) | ||
742 | 86 | } | ||
743 | 87 | |||
744 | 88 | err = inst.env.compute.cloudapi.DeleteMachine(id) | ||
745 | 89 | if err != nil { | ||
746 | 90 | return fmt.Errorf("cannot delete instance %s: %v", id, err) | ||
747 | 91 | } | ||
748 | 92 | |||
749 | 93 | return nil | ||
750 | 94 | } | ||
751 | 95 | |||
752 | 96 | func (inst *joyentInstance) pollMachineState(machineId, state string) bool { | ||
753 | 97 | machineConfig, err := inst.env.compute.cloudapi.GetMachine(machineId) | ||
754 | 98 | if err != nil { | ||
755 | 99 | return false | ||
756 | 100 | } | ||
757 | 101 | return strings.EqualFold(machineConfig.State, state) | ||
758 | 102 | } | ||
759 | 103 | 62 | ||
760 | === modified file 'provider/joyent/local_test.go' | |||
761 | --- provider/joyent/local_test.go 2014-04-24 12:33:19 +0000 | |||
762 | +++ provider/joyent/local_test.go 2014-05-14 03:01:15 +0000 | |||
763 | @@ -193,7 +193,7 @@ | |||
764 | 193 | err := bootstrap.Bootstrap(bootstrapContext(c), env, environs.BootstrapParams{}) | 193 | err := bootstrap.Bootstrap(bootstrapContext(c), env, environs.BootstrapParams{}) |
765 | 194 | c.Assert(err, gc.IsNil) | 194 | c.Assert(err, gc.IsNil) |
766 | 195 | inst, _ := testing.AssertStartInstance(c, env, "100") | 195 | inst, _ := testing.AssertStartInstance(c, env, "100") |
768 | 196 | err = env.StopInstances([]instance.Instance{inst}) | 196 | err = env.StopInstances(inst.Id()) |
769 | 197 | c.Assert(err, gc.IsNil) | 197 | c.Assert(err, gc.IsNil) |
770 | 198 | } | 198 | } |
771 | 199 | 199 | ||
772 | @@ -261,7 +261,7 @@ | |||
773 | 261 | envtesting.UploadFakeTools(c, env.Storage()) | 261 | envtesting.UploadFakeTools(c, env.Storage()) |
774 | 262 | inst, _ := testing.AssertStartInstance(c, env, "100") | 262 | inst, _ := testing.AssertStartInstance(c, env, "100") |
775 | 263 | c.Assert(inst.Status(), gc.Equals, "running") | 263 | c.Assert(inst.Status(), gc.Equals, "running") |
777 | 264 | err := env.StopInstances([]instance.Instance{inst}) | 264 | err := env.StopInstances(inst.Id()) |
778 | 265 | c.Assert(err, gc.IsNil) | 265 | c.Assert(err, gc.IsNil) |
779 | 266 | } | 266 | } |
780 | 267 | 267 | ||
781 | @@ -274,7 +274,7 @@ | |||
782 | 274 | id1 := inst1.Id() | 274 | id1 := inst1.Id() |
783 | 275 | c.Logf("id0: %s, id1: %s", id0, id1) | 275 | c.Logf("id0: %s, id1: %s", id0, id1) |
784 | 276 | defer func() { | 276 | defer func() { |
786 | 277 | err := env.StopInstances([]instance.Instance{inst0, inst1}) | 277 | err := env.StopInstances(inst0.Id(), inst1.Id()) |
787 | 278 | c.Assert(err, gc.IsNil) | 278 | c.Assert(err, gc.IsNil) |
788 | 279 | }() | 279 | }() |
789 | 280 | 280 | ||
790 | 281 | 281 | ||
791 | === modified file 'provider/local/environ.go' | |||
792 | --- provider/local/environ.go 2014-05-06 16:08:51 +0000 | |||
793 | +++ provider/local/environ.go 2014-05-14 03:01:15 +0000 | |||
794 | @@ -354,12 +354,12 @@ | |||
795 | 354 | } | 354 | } |
796 | 355 | 355 | ||
797 | 356 | // StopInstances is specified in the InstanceBroker interface. | 356 | // StopInstances is specified in the InstanceBroker interface. |
801 | 357 | func (env *localEnviron) StopInstances(instances []instance.Instance) error { | 357 | func (env *localEnviron) StopInstances(ids ...instance.Id) error { |
802 | 358 | for _, inst := range instances { | 358 | for _, id := range ids { |
803 | 359 | if inst.Id() == bootstrapInstanceId { | 359 | if id == bootstrapInstanceId { |
804 | 360 | return fmt.Errorf("cannot stop the bootstrap instance") | 360 | return fmt.Errorf("cannot stop the bootstrap instance") |
805 | 361 | } | 361 | } |
807 | 362 | if err := env.containerManager.DestroyContainer(inst); err != nil { | 362 | if err := env.containerManager.DestroyContainer(id); err != nil { |
808 | 363 | return err | 363 | return err |
809 | 364 | } | 364 | } |
810 | 365 | } | 365 | } |
811 | @@ -462,7 +462,7 @@ | |||
812 | 462 | return err | 462 | return err |
813 | 463 | } | 463 | } |
814 | 464 | for _, inst := range containers { | 464 | for _, inst := range containers { |
816 | 465 | if err := env.containerManager.DestroyContainer(inst); err != nil { | 465 | if err := env.containerManager.DestroyContainer(inst.Id()); err != nil { |
817 | 466 | return err | 466 | return err |
818 | 467 | } | 467 | } |
819 | 468 | } | 468 | } |
820 | 469 | 469 | ||
821 | === modified file 'provider/maas/environ.go' | |||
822 | --- provider/maas/environ.go 2014-05-07 19:09:23 +0000 | |||
823 | +++ provider/maas/environ.go 2014-05-14 03:01:15 +0000 | |||
824 | @@ -434,7 +434,7 @@ | |||
825 | 434 | } | 434 | } |
826 | 435 | defer func() { | 435 | defer func() { |
827 | 436 | if err != nil { | 436 | if err != nil { |
829 | 437 | if err := environ.releaseInstance(inst); err != nil { | 437 | if err := environ.StopInstances(inst.Id()); err != nil { |
830 | 438 | logger.Errorf("error releasing failed instance: %v", err) | 438 | logger.Errorf("error releasing failed instance: %v", err) |
831 | 439 | } | 439 | } |
832 | 440 | } | 440 | } |
833 | @@ -546,32 +546,18 @@ | |||
834 | 546 | } | 546 | } |
835 | 547 | 547 | ||
836 | 548 | // StopInstances is specified in the InstanceBroker interface. | 548 | // StopInstances is specified in the InstanceBroker interface. |
838 | 549 | func (environ *maasEnviron) StopInstances(instances []instance.Instance) error { | 549 | func (environ *maasEnviron) StopInstances(ids ...instance.Id) error { |
839 | 550 | // Shortcut to exit quickly if 'instances' is an empty slice or nil. | 550 | // Shortcut to exit quickly if 'instances' is an empty slice or nil. |
841 | 551 | if len(instances) == 0 { | 551 | if len(ids) == 0 { |
842 | 552 | return nil | 552 | return nil |
843 | 553 | } | 553 | } |
865 | 554 | // Tell MAAS to release each of the instances. If there are errors, | 554 | // TODO(axw) 2014-05-13 #1319016 |
866 | 555 | // return only the first one (but release all instances regardless). | 555 | // Nodes that have been removed out of band will cause |
867 | 556 | // Note that releasing instances also turns them off. | 556 | // the release call to fail. We should parse the error |
868 | 557 | var firstErr error | 557 | // returned from MAAS and retry, or otherwise request |
869 | 558 | for _, instance := range instances { | 558 | // an enhancement to MAAS to ignore unknown node IDs. |
870 | 559 | err := environ.releaseInstance(instance) | 559 | nodes := environ.getMAASClient().GetSubObject("nodes") |
871 | 560 | if firstErr == nil { | 560 | _, err := nodes.CallPost("release", getSystemIdValues("nodes", ids)) |
851 | 561 | firstErr = err | ||
852 | 562 | } | ||
853 | 563 | } | ||
854 | 564 | return firstErr | ||
855 | 565 | } | ||
856 | 566 | |||
857 | 567 | // releaseInstance releases a single instance. | ||
858 | 568 | func (environ *maasEnviron) releaseInstance(inst instance.Instance) error { | ||
859 | 569 | maasInst := inst.(*maasInstance) | ||
860 | 570 | maasObj := maasInst.maasObject | ||
861 | 571 | _, err := maasObj.CallPost("release", nil) | ||
862 | 572 | if err != nil { | ||
863 | 573 | logger.Debugf("error releasing instance %v", maasInst) | ||
864 | 574 | } | ||
872 | 575 | return err | 561 | return err |
873 | 576 | } | 562 | } |
874 | 577 | 563 | ||
875 | @@ -580,7 +566,7 @@ | |||
876 | 580 | // "ids" matches all instances (not none as you might expect). | 566 | // "ids" matches all instances (not none as you might expect). |
877 | 581 | func (environ *maasEnviron) instances(ids []instance.Id) ([]instance.Instance, error) { | 567 | func (environ *maasEnviron) instances(ids []instance.Id) ([]instance.Instance, error) { |
878 | 582 | nodeListing := environ.getMAASClient().GetSubObject("nodes") | 568 | nodeListing := environ.getMAASClient().GetSubObject("nodes") |
880 | 583 | filter := getSystemIdValues(ids) | 569 | filter := getSystemIdValues("id", ids) |
881 | 584 | filter.Add("agent_name", environ.ecfg().maasAgentName()) | 570 | filter.Add("agent_name", environ.ecfg().maasAgentName()) |
882 | 585 | listNodeObjects, err := nodeListing.CallGet("list", filter) | 571 | listNodeObjects, err := nodeListing.CallGet("list", filter) |
883 | 586 | if err != nil { | 572 | if err != nil { |
884 | 587 | 573 | ||
885 | === modified file 'provider/maas/environ_whitebox_test.go' | |||
886 | --- provider/maas/environ_whitebox_test.go 2014-04-24 12:33:19 +0000 | |||
887 | +++ provider/maas/environ_whitebox_test.go 2014-05-14 03:01:15 +0000 | |||
888 | @@ -420,24 +420,27 @@ | |||
889 | 420 | func (suite *environSuite) TestStopInstancesReturnsIfParameterEmpty(c *gc.C) { | 420 | func (suite *environSuite) TestStopInstancesReturnsIfParameterEmpty(c *gc.C) { |
890 | 421 | suite.getInstance("test1") | 421 | suite.getInstance("test1") |
891 | 422 | 422 | ||
893 | 423 | err := suite.makeEnviron().StopInstances([]instance.Instance{}) | 423 | err := suite.makeEnviron().StopInstances() |
894 | 424 | c.Check(err, gc.IsNil) | 424 | c.Check(err, gc.IsNil) |
895 | 425 | operations := suite.testMAASObject.TestServer.NodeOperations() | 425 | operations := suite.testMAASObject.TestServer.NodeOperations() |
896 | 426 | c.Check(operations, gc.DeepEquals, map[string][]string{}) | 426 | c.Check(operations, gc.DeepEquals, map[string][]string{}) |
897 | 427 | } | 427 | } |
898 | 428 | 428 | ||
899 | 429 | func (suite *environSuite) TestStopInstancesStopsAndReleasesInstances(c *gc.C) { | 429 | func (suite *environSuite) TestStopInstancesStopsAndReleasesInstances(c *gc.C) { |
902 | 430 | instance1 := suite.getInstance("test1") | 430 | suite.getInstance("test1") |
903 | 431 | instance2 := suite.getInstance("test2") | 431 | suite.getInstance("test2") |
904 | 432 | suite.getInstance("test3") | 432 | suite.getInstance("test3") |
909 | 433 | instances := []instance.Instance{instance1, instance2} | 433 | // mark test1 and test2 as being allocated, but not test3. |
910 | 434 | 434 | // The release operation will ignore test3. | |
911 | 435 | err := suite.makeEnviron().StopInstances(instances) | 435 | suite.testMAASObject.TestServer.OwnedNodes()["test1"] = true |
912 | 436 | 436 | suite.testMAASObject.TestServer.OwnedNodes()["test2"] = true | |
913 | 437 | |||
914 | 438 | err := suite.makeEnviron().StopInstances("test1", "test2", "test3") | ||
915 | 437 | c.Check(err, gc.IsNil) | 439 | c.Check(err, gc.IsNil) |
919 | 438 | operations := suite.testMAASObject.TestServer.NodeOperations() | 440 | operations := suite.testMAASObject.TestServer.NodesOperations() |
920 | 439 | expectedOperations := map[string][]string{"test1": {"release"}, "test2": {"release"}} | 441 | c.Check(operations, gc.DeepEquals, []string{"release"}) |
921 | 440 | c.Check(operations, gc.DeepEquals, expectedOperations) | 442 | c.Assert(suite.testMAASObject.TestServer.OwnedNodes()["test1"], jc.IsFalse) |
922 | 443 | c.Assert(suite.testMAASObject.TestServer.OwnedNodes()["test2"], jc.IsFalse) | ||
923 | 441 | } | 444 | } |
924 | 442 | 445 | ||
925 | 443 | func (suite *environSuite) TestStateInfo(c *gc.C) { | 446 | func (suite *environSuite) TestStateInfo(c *gc.C) { |
926 | @@ -472,6 +475,7 @@ | |||
927 | 472 | func (suite *environSuite) TestDestroy(c *gc.C) { | 475 | func (suite *environSuite) TestDestroy(c *gc.C) { |
928 | 473 | env := suite.makeEnviron() | 476 | env := suite.makeEnviron() |
929 | 474 | suite.getInstance("test1") | 477 | suite.getInstance("test1") |
930 | 478 | suite.testMAASObject.TestServer.OwnedNodes()["test1"] = true // simulate acquire | ||
931 | 475 | data := makeRandomBytes(10) | 479 | data := makeRandomBytes(10) |
932 | 476 | suite.testMAASObject.TestServer.NewFile("filename", data) | 480 | suite.testMAASObject.TestServer.NewFile("filename", data) |
933 | 477 | stor := env.Storage() | 481 | stor := env.Storage() |
934 | @@ -480,9 +484,9 @@ | |||
935 | 480 | c.Check(err, gc.IsNil) | 484 | c.Check(err, gc.IsNil) |
936 | 481 | 485 | ||
937 | 482 | // Instances have been stopped. | 486 | // Instances have been stopped. |
941 | 483 | operations := suite.testMAASObject.TestServer.NodeOperations() | 487 | operations := suite.testMAASObject.TestServer.NodesOperations() |
942 | 484 | expectedOperations := map[string][]string{"test1": {"release"}} | 488 | c.Check(operations, gc.DeepEquals, []string{"release"}) |
943 | 485 | c.Check(operations, gc.DeepEquals, expectedOperations) | 489 | c.Check(suite.testMAASObject.TestServer.OwnedNodes()["test1"], jc.IsFalse) |
944 | 486 | // Files have been cleaned up. | 490 | // Files have been cleaned up. |
945 | 487 | listing, err := storage.List(stor, "") | 491 | listing, err := storage.List(stor, "") |
946 | 488 | c.Assert(err, gc.IsNil) | 492 | c.Assert(err, gc.IsNil) |
947 | 489 | 493 | ||
948 | === modified file 'provider/maas/util.go' | |||
949 | --- provider/maas/util.go 2013-08-02 05:13:41 +0000 | |||
950 | +++ provider/maas/util.go 2014-05-14 03:01:15 +0000 | |||
951 | @@ -24,12 +24,12 @@ | |||
952 | 24 | } | 24 | } |
953 | 25 | 25 | ||
954 | 26 | // getSystemIdValues returns a url.Values object with all the 'system_ids' | 26 | // getSystemIdValues returns a url.Values object with all the 'system_ids' |
956 | 27 | // from the given instanceIds stored under the key 'id'. This is used | 27 | // from the given instanceIds stored under the specified key. This is used |
957 | 28 | // to filter out instances when listing the nodes objects. | 28 | // to filter out instances when listing the nodes objects. |
959 | 29 | func getSystemIdValues(instanceIds []instance.Id) url.Values { | 29 | func getSystemIdValues(key string, instanceIds []instance.Id) url.Values { |
960 | 30 | values := url.Values{} | 30 | values := url.Values{} |
961 | 31 | for _, instanceId := range instanceIds { | 31 | for _, instanceId := range instanceIds { |
963 | 32 | values.Add("id", extractSystemId(instanceId)) | 32 | values.Add(key, extractSystemId(instanceId)) |
964 | 33 | } | 33 | } |
965 | 34 | return values | 34 | return values |
966 | 35 | } | 35 | } |
967 | 36 | 36 | ||
968 | === modified file 'provider/maas/util_test.go' | |||
969 | --- provider/maas/util_test.go 2013-08-09 04:58:34 +0000 | |||
970 | +++ provider/maas/util_test.go 2014-05-14 03:01:15 +0000 | |||
971 | @@ -30,7 +30,7 @@ | |||
972 | 30 | instanceId2 := instance.Id("/MAAS/api/1.0/nodes/system_id2/") | 30 | instanceId2 := instance.Id("/MAAS/api/1.0/nodes/system_id2/") |
973 | 31 | instanceIds := []instance.Id{instanceId1, instanceId2} | 31 | instanceIds := []instance.Id{instanceId1, instanceId2} |
974 | 32 | 32 | ||
976 | 33 | values := getSystemIdValues(instanceIds) | 33 | values := getSystemIdValues("id", instanceIds) |
977 | 34 | 34 | ||
978 | 35 | c.Check(values["id"], gc.DeepEquals, []string{"system_id1", "system_id2"}) | 35 | c.Check(values["id"], gc.DeepEquals, []string{"system_id1", "system_id2"}) |
979 | 36 | } | 36 | } |
980 | 37 | 37 | ||
981 | === modified file 'provider/manual/environ.go' | |||
982 | --- provider/manual/environ.go 2014-05-06 16:08:51 +0000 | |||
983 | +++ provider/manual/environ.go 2014-05-14 03:01:15 +0000 | |||
984 | @@ -70,7 +70,7 @@ | |||
985 | 70 | return nil, nil, nil, errNoStartInstance | 70 | return nil, nil, nil, errNoStartInstance |
986 | 71 | } | 71 | } |
987 | 72 | 72 | ||
989 | 73 | func (*manualEnviron) StopInstances([]instance.Instance) error { | 73 | func (*manualEnviron) StopInstances(...instance.Id) error { |
990 | 74 | return errNoStopInstance | 74 | return errNoStopInstance |
991 | 75 | } | 75 | } |
992 | 76 | 76 | ||
993 | 77 | 77 | ||
994 | === modified file 'provider/openstack/local_test.go' | |||
995 | --- provider/openstack/local_test.go 2014-04-24 12:33:19 +0000 | |||
996 | +++ provider/openstack/local_test.go 2014-05-14 03:01:15 +0000 | |||
997 | @@ -264,7 +264,7 @@ | |||
998 | 264 | err = bootstrap.Bootstrap(coretesting.Context(c), env, environs.BootstrapParams{}) | 264 | err = bootstrap.Bootstrap(coretesting.Context(c), env, environs.BootstrapParams{}) |
999 | 265 | c.Assert(err, gc.IsNil) | 265 | c.Assert(err, gc.IsNil) |
1000 | 266 | inst, _ := testing.AssertStartInstance(c, env, "100") | 266 | inst, _ := testing.AssertStartInstance(c, env, "100") |
1002 | 267 | err = env.StopInstances([]instance.Instance{inst}) | 267 | err = env.StopInstances(inst.Id()) |
1003 | 268 | c.Assert(err, gc.IsNil) | 268 | c.Assert(err, gc.IsNil) |
1004 | 269 | } | 269 | } |
1005 | 270 | 270 | ||
1006 | @@ -288,7 +288,7 @@ | |||
1007 | 288 | env, err := environs.New(cfg) | 288 | env, err := environs.New(cfg) |
1008 | 289 | c.Assert(err, gc.IsNil) | 289 | c.Assert(err, gc.IsNil) |
1009 | 290 | inst, _ := testing.AssertStartInstance(c, env, "100") | 290 | inst, _ := testing.AssertStartInstance(c, env, "100") |
1011 | 291 | err = env.StopInstances([]instance.Instance{inst}) | 291 | err = env.StopInstances(inst.Id()) |
1012 | 292 | c.Assert(err, gc.IsNil) | 292 | c.Assert(err, gc.IsNil) |
1013 | 293 | } | 293 | } |
1014 | 294 | 294 | ||
1015 | @@ -363,7 +363,7 @@ | |||
1016 | 363 | // group, one group for the entire environment, and another for the | 363 | // group, one group for the entire environment, and another for the |
1017 | 364 | // new instance. | 364 | // new instance. |
1018 | 365 | assertSecurityGroups(c, env, []string{"default", fmt.Sprintf("juju-%v", env.Name()), fmt.Sprintf("juju-%v-%v", env.Name(), instanceName)}) | 365 | assertSecurityGroups(c, env, []string{"default", fmt.Sprintf("juju-%v", env.Name()), fmt.Sprintf("juju-%v-%v", env.Name(), instanceName)}) |
1020 | 366 | err = env.StopInstances([]instance.Instance{inst}) | 366 | err = env.StopInstances(inst.Id()) |
1021 | 367 | c.Assert(err, gc.IsNil) | 367 | c.Assert(err, gc.IsNil) |
1022 | 368 | // The security group for this instance is now removed. | 368 | // The security group for this instance is now removed. |
1023 | 369 | assertSecurityGroups(c, env, []string{"default", fmt.Sprintf("juju-%v", env.Name())}) | 369 | assertSecurityGroups(c, env, []string{"default", fmt.Sprintf("juju-%v", env.Name())}) |
1024 | @@ -391,7 +391,7 @@ | |||
1025 | 391 | inst, _ := testing.AssertStartInstance(c, env, instanceName) | 391 | inst, _ := testing.AssertStartInstance(c, env, instanceName) |
1026 | 392 | allSecurityGroups := []string{"default", fmt.Sprintf("juju-%v", env.Name()), fmt.Sprintf("juju-%v-%v", env.Name(), instanceName)} | 392 | allSecurityGroups := []string{"default", fmt.Sprintf("juju-%v", env.Name()), fmt.Sprintf("juju-%v-%v", env.Name(), instanceName)} |
1027 | 393 | assertSecurityGroups(c, env, allSecurityGroups) | 393 | assertSecurityGroups(c, env, allSecurityGroups) |
1029 | 394 | err = env.StopInstances([]instance.Instance{inst}) | 394 | err = env.StopInstances(inst.Id()) |
1030 | 395 | c.Assert(err, gc.IsNil) | 395 | c.Assert(err, gc.IsNil) |
1031 | 396 | assertSecurityGroups(c, env, allSecurityGroups) | 396 | assertSecurityGroups(c, env, allSecurityGroups) |
1032 | 397 | } | 397 | } |
1033 | @@ -478,7 +478,7 @@ | |||
1034 | 478 | // goose's test service always returns ACTIVE state. | 478 | // goose's test service always returns ACTIVE state. |
1035 | 479 | inst, _ := testing.AssertStartInstance(c, env, "100") | 479 | inst, _ := testing.AssertStartInstance(c, env, "100") |
1036 | 480 | c.Assert(inst.Status(), gc.Equals, nova.StatusActive) | 480 | c.Assert(inst.Status(), gc.Equals, nova.StatusActive) |
1038 | 481 | err := env.StopInstances([]instance.Instance{inst}) | 481 | err := env.StopInstances(inst.Id()) |
1039 | 482 | c.Assert(err, gc.IsNil) | 482 | c.Assert(err, gc.IsNil) |
1040 | 483 | } | 483 | } |
1041 | 484 | 484 | ||
1042 | @@ -489,7 +489,7 @@ | |||
1043 | 489 | inst1, _ := testing.AssertStartInstance(c, env, "101") | 489 | inst1, _ := testing.AssertStartInstance(c, env, "101") |
1044 | 490 | id1 := inst1.Id() | 490 | id1 := inst1.Id() |
1045 | 491 | defer func() { | 491 | defer func() { |
1047 | 492 | err := env.StopInstances([]instance.Instance{inst0, inst1}) | 492 | err := env.StopInstances(inst0.Id(), inst1.Id()) |
1048 | 493 | c.Assert(err, gc.IsNil) | 493 | c.Assert(err, gc.IsNil) |
1049 | 494 | }() | 494 | }() |
1050 | 495 | 495 | ||
1051 | @@ -534,7 +534,7 @@ | |||
1052 | 534 | defer cleanup() | 534 | defer cleanup() |
1053 | 535 | stateInst, _ := testing.AssertStartInstance(c, env, "100") | 535 | stateInst, _ := testing.AssertStartInstance(c, env, "100") |
1054 | 536 | defer func() { | 536 | defer func() { |
1056 | 537 | err := env.StopInstances([]instance.Instance{stateInst}) | 537 | err := env.StopInstances(stateInst.Id()) |
1057 | 538 | c.Assert(err, gc.IsNil) | 538 | c.Assert(err, gc.IsNil) |
1058 | 539 | }() | 539 | }() |
1059 | 540 | found := make(map[instance.Id]instance.Instance) | 540 | found := make(map[instance.Id]instance.Instance) |
1060 | @@ -559,7 +559,7 @@ | |||
1061 | 559 | defer cleanup() | 559 | defer cleanup() |
1062 | 560 | stateInst, _ := testing.AssertStartInstance(c, env, "100") | 560 | stateInst, _ := testing.AssertStartInstance(c, env, "100") |
1063 | 561 | defer func() { | 561 | defer func() { |
1065 | 562 | err := env.StopInstances([]instance.Instance{stateInst}) | 562 | err := env.StopInstances(stateInst.Id()) |
1066 | 563 | c.Assert(err, gc.IsNil) | 563 | c.Assert(err, gc.IsNil) |
1067 | 564 | }() | 564 | }() |
1068 | 565 | 565 | ||
1069 | 566 | 566 | ||
1070 | === modified file 'provider/openstack/provider.go' | |||
1071 | --- provider/openstack/provider.go 2014-05-06 16:08:51 +0000 | |||
1072 | +++ provider/openstack/provider.go 2014-05-14 03:01:15 +0000 | |||
1073 | @@ -6,7 +6,6 @@ | |||
1074 | 6 | package openstack | 6 | package openstack |
1075 | 7 | 7 | ||
1076 | 8 | import ( | 8 | import ( |
1077 | 9 | "errors" | ||
1078 | 10 | "fmt" | 9 | "fmt" |
1079 | 11 | "io/ioutil" | 10 | "io/ioutil" |
1080 | 12 | "net/http" | 11 | "net/http" |
1081 | @@ -902,28 +901,33 @@ | |||
1082 | 902 | return inst, inst.hardwareCharacteristics(), nil, nil | 901 | return inst, inst.hardwareCharacteristics(), nil, nil |
1083 | 903 | } | 902 | } |
1084 | 904 | 903 | ||
1100 | 905 | func (e *environ) StopInstances(insts []instance.Instance) error { | 904 | func (e *environ) StopInstances(ids ...instance.Id) error { |
1101 | 906 | ids := make([]instance.Id, len(insts)) | 905 | // If in instance firewall mode, gather the security group names. |
1102 | 907 | securityGroupNames := make([]string, len(insts)) | 906 | var securityGroupNames []string |
1103 | 908 | for i, inst := range insts { | 907 | if e.Config().FirewallMode() == config.FwInstance { |
1104 | 909 | instanceValue, ok := inst.(*openstackInstance) | 908 | instances, err := e.Instances(ids) |
1105 | 910 | if !ok { | 909 | if err == environs.ErrNoInstances { |
1106 | 911 | return errors.New("Incompatible instance.Instance supplied") | 910 | return nil |
1107 | 912 | } | 911 | } |
1108 | 913 | ids[i] = instanceValue.Id() | 912 | securityGroupNames = make([]string, 0, len(ids)) |
1109 | 914 | openstackName := instanceValue.getServerDetail().Name | 913 | for _, inst := range instances { |
1110 | 915 | lastDashPos := strings.LastIndex(openstackName, "-") | 914 | if inst == nil { |
1111 | 916 | if lastDashPos == -1 { | 915 | continue |
1112 | 917 | return fmt.Errorf("cannot identify instance ID in openstack server name %q", openstackName) | 916 | } |
1113 | 918 | } | 917 | openstackName := inst.(*openstackInstance).getServerDetail().Name |
1114 | 919 | securityGroupNames[i] = e.machineGroupName(openstackName[lastDashPos+1:]) | 918 | lastDashPos := strings.LastIndex(openstackName, "-") |
1115 | 919 | if lastDashPos == -1 { | ||
1116 | 920 | return fmt.Errorf("cannot identify machine ID in openstack server name %q", openstackName) | ||
1117 | 921 | } | ||
1118 | 922 | securityGroupName := e.machineGroupName(openstackName[lastDashPos+1:]) | ||
1119 | 923 | securityGroupNames = append(securityGroupNames, securityGroupName) | ||
1120 | 924 | } | ||
1121 | 920 | } | 925 | } |
1122 | 921 | logger.Debugf("terminating instances %v", ids) | 926 | logger.Debugf("terminating instances %v", ids) |
1125 | 922 | err := e.terminateInstances(ids) | 927 | if err := e.terminateInstances(ids); err != nil { |
1124 | 923 | if err != nil { | ||
1126 | 924 | return err | 928 | return err |
1127 | 925 | } | 929 | } |
1129 | 926 | if e.Config().FirewallMode() == config.FwInstance { | 930 | if securityGroupNames != nil { |
1130 | 927 | return e.deleteSecurityGroups(securityGroupNames) | 931 | return e.deleteSecurityGroups(securityGroupNames) |
1131 | 928 | } | 932 | } |
1132 | 929 | return nil | 933 | return nil |
1133 | 930 | 934 | ||
1134 | === modified file 'state/apiserver/client/destroy.go' | |||
1135 | --- state/apiserver/client/destroy.go 2014-01-17 16:41:32 +0000 | |||
1136 | +++ state/apiserver/client/destroy.go 2014-05-14 03:01:15 +0000 | |||
1137 | @@ -104,28 +104,7 @@ | |||
1138 | 104 | if err != nil { | 104 | if err != nil { |
1139 | 105 | return err | 105 | return err |
1140 | 106 | } | 106 | } |
1163 | 107 | // TODO(axw) 2013-12-12 #1260171 | 107 | return env.StopInstances(ids...) |
1142 | 108 | // Modify InstanceBroker.StopInstances to take | ||
1143 | 109 | // a slice of IDs rather than Instances. | ||
1144 | 110 | instances, err := env.Instances(ids) | ||
1145 | 111 | switch err { | ||
1146 | 112 | case nil: | ||
1147 | 113 | default: | ||
1148 | 114 | return err | ||
1149 | 115 | case environs.ErrNoInstances: | ||
1150 | 116 | return nil | ||
1151 | 117 | case environs.ErrPartialInstances: | ||
1152 | 118 | var nonNilInstances []instance.Instance | ||
1153 | 119 | for i, inst := range instances { | ||
1154 | 120 | if inst == nil { | ||
1155 | 121 | logger.Warningf("unknown instance ID: %v", ids[i]) | ||
1156 | 122 | continue | ||
1157 | 123 | } | ||
1158 | 124 | nonNilInstances = append(nonNilInstances, inst) | ||
1159 | 125 | } | ||
1160 | 126 | instances = nonNilInstances | ||
1161 | 127 | } | ||
1162 | 128 | return env.StopInstances(instances) | ||
1164 | 129 | } | 108 | } |
1165 | 130 | 109 | ||
1166 | 131 | // checkManualMachines checks if any of the machines in the slice were | 110 | // checkManualMachines checks if any of the machines in the slice were |
1167 | 132 | 111 | ||
1168 | === modified file 'worker/provisioner/kvm-broker.go' | |||
1169 | --- worker/provisioner/kvm-broker.go 2014-05-09 13:24:50 +0000 | |||
1170 | +++ worker/provisioner/kvm-broker.go 2014-05-14 03:01:15 +0000 | |||
1171 | @@ -105,11 +105,11 @@ | |||
1172 | 105 | } | 105 | } |
1173 | 106 | 106 | ||
1174 | 107 | // StopInstances shuts down the given instances. | 107 | // StopInstances shuts down the given instances. |
1176 | 108 | func (broker *kvmBroker) StopInstances(instances []instance.Instance) error { | 108 | func (broker *kvmBroker) StopInstances(ids ...instance.Id) error { |
1177 | 109 | // TODO: potentially parallelise. | 109 | // TODO: potentially parallelise. |
1181 | 110 | for _, instance := range instances { | 110 | for _, id := range ids { |
1182 | 111 | kvmLogger.Infof("stopping kvm container for instance: %s", instance.Id()) | 111 | kvmLogger.Infof("stopping kvm container for instance: %s", id) |
1183 | 112 | if err := broker.manager.DestroyContainer(instance); err != nil { | 112 | if err := broker.manager.DestroyContainer(id); err != nil { |
1184 | 113 | kvmLogger.Errorf("container did not stop: %v", err) | 113 | kvmLogger.Errorf("container did not stop: %v", err) |
1185 | 114 | return err | 114 | return err |
1186 | 115 | } | 115 | } |
1187 | 116 | 116 | ||
1188 | === modified file 'worker/provisioner/kvm-broker_test.go' | |||
1189 | --- worker/provisioner/kvm-broker_test.go 2014-05-09 13:24:50 +0000 | |||
1190 | +++ worker/provisioner/kvm-broker_test.go 2014-05-14 03:01:15 +0000 | |||
1191 | @@ -102,13 +102,13 @@ | |||
1192 | 102 | kvm1 := s.startInstance(c, "1/kvm/1") | 102 | kvm1 := s.startInstance(c, "1/kvm/1") |
1193 | 103 | kvm2 := s.startInstance(c, "1/kvm/2") | 103 | kvm2 := s.startInstance(c, "1/kvm/2") |
1194 | 104 | 104 | ||
1196 | 105 | err := s.broker.StopInstances([]instance.Instance{kvm0}) | 105 | err := s.broker.StopInstances(kvm0.Id()) |
1197 | 106 | c.Assert(err, gc.IsNil) | 106 | c.Assert(err, gc.IsNil) |
1198 | 107 | s.assertInstances(c, kvm1, kvm2) | 107 | s.assertInstances(c, kvm1, kvm2) |
1199 | 108 | c.Assert(s.kvmContainerDir(kvm0), jc.DoesNotExist) | 108 | c.Assert(s.kvmContainerDir(kvm0), jc.DoesNotExist) |
1200 | 109 | c.Assert(s.kvmRemovedContainerDir(kvm0), jc.IsDirectory) | 109 | c.Assert(s.kvmRemovedContainerDir(kvm0), jc.IsDirectory) |
1201 | 110 | 110 | ||
1203 | 111 | err = s.broker.StopInstances([]instance.Instance{kvm1, kvm2}) | 111 | err = s.broker.StopInstances(kvm1.Id(), kvm2.Id()) |
1204 | 112 | c.Assert(err, gc.IsNil) | 112 | c.Assert(err, gc.IsNil) |
1205 | 113 | s.assertInstances(c) | 113 | s.assertInstances(c) |
1206 | 114 | } | 114 | } |
1207 | @@ -118,7 +118,7 @@ | |||
1208 | 118 | kvm1 := s.startInstance(c, "1/kvm/1") | 118 | kvm1 := s.startInstance(c, "1/kvm/1") |
1209 | 119 | s.assertInstances(c, kvm0, kvm1) | 119 | s.assertInstances(c, kvm0, kvm1) |
1210 | 120 | 120 | ||
1212 | 121 | err := s.broker.StopInstances([]instance.Instance{kvm1}) | 121 | err := s.broker.StopInstances(kvm1.Id()) |
1213 | 122 | c.Assert(err, gc.IsNil) | 122 | c.Assert(err, gc.IsNil) |
1214 | 123 | kvm2 := s.startInstance(c, "1/kvm/2") | 123 | kvm2 := s.startInstance(c, "1/kvm/2") |
1215 | 124 | s.assertInstances(c, kvm0, kvm2) | 124 | s.assertInstances(c, kvm0, kvm2) |
1216 | 125 | 125 | ||
1217 | === modified file 'worker/provisioner/lxc-broker.go' | |||
1218 | --- worker/provisioner/lxc-broker.go 2014-05-09 13:24:50 +0000 | |||
1219 | +++ worker/provisioner/lxc-broker.go 2014-05-14 03:01:15 +0000 | |||
1220 | @@ -102,11 +102,11 @@ | |||
1221 | 102 | } | 102 | } |
1222 | 103 | 103 | ||
1223 | 104 | // StopInstances shuts down the given instances. | 104 | // StopInstances shuts down the given instances. |
1225 | 105 | func (broker *lxcBroker) StopInstances(instances []instance.Instance) error { | 105 | func (broker *lxcBroker) StopInstances(ids ...instance.Id) error { |
1226 | 106 | // TODO: potentially parallelise. | 106 | // TODO: potentially parallelise. |
1230 | 107 | for _, instance := range instances { | 107 | for _, id := range ids { |
1231 | 108 | lxcLogger.Infof("stopping lxc container for instance: %s", instance.Id()) | 108 | lxcLogger.Infof("stopping lxc container for instance: %s", id) |
1232 | 109 | if err := broker.manager.DestroyContainer(instance); err != nil { | 109 | if err := broker.manager.DestroyContainer(id); err != nil { |
1233 | 110 | lxcLogger.Errorf("container did not stop: %v", err) | 110 | lxcLogger.Errorf("container did not stop: %v", err) |
1234 | 111 | return err | 111 | return err |
1235 | 112 | } | 112 | } |
1236 | 113 | 113 | ||
1237 | === modified file 'worker/provisioner/lxc-broker_test.go' | |||
1238 | --- worker/provisioner/lxc-broker_test.go 2014-05-09 13:24:50 +0000 | |||
1239 | +++ worker/provisioner/lxc-broker_test.go 2014-05-14 03:01:15 +0000 | |||
1240 | @@ -131,13 +131,13 @@ | |||
1241 | 131 | lxc1 := s.startInstance(c, "1/lxc/1") | 131 | lxc1 := s.startInstance(c, "1/lxc/1") |
1242 | 132 | lxc2 := s.startInstance(c, "1/lxc/2") | 132 | lxc2 := s.startInstance(c, "1/lxc/2") |
1243 | 133 | 133 | ||
1245 | 134 | err := s.broker.StopInstances([]instance.Instance{lxc0}) | 134 | err := s.broker.StopInstances(lxc0.Id()) |
1246 | 135 | c.Assert(err, gc.IsNil) | 135 | c.Assert(err, gc.IsNil) |
1247 | 136 | s.assertInstances(c, lxc1, lxc2) | 136 | s.assertInstances(c, lxc1, lxc2) |
1248 | 137 | c.Assert(s.lxcContainerDir(lxc0), jc.DoesNotExist) | 137 | c.Assert(s.lxcContainerDir(lxc0), jc.DoesNotExist) |
1249 | 138 | c.Assert(s.lxcRemovedContainerDir(lxc0), jc.IsDirectory) | 138 | c.Assert(s.lxcRemovedContainerDir(lxc0), jc.IsDirectory) |
1250 | 139 | 139 | ||
1252 | 140 | err = s.broker.StopInstances([]instance.Instance{lxc1, lxc2}) | 140 | err = s.broker.StopInstances(lxc1.Id(), lxc2.Id()) |
1253 | 141 | c.Assert(err, gc.IsNil) | 141 | c.Assert(err, gc.IsNil) |
1254 | 142 | s.assertInstances(c) | 142 | s.assertInstances(c) |
1255 | 143 | } | 143 | } |
1256 | @@ -147,7 +147,7 @@ | |||
1257 | 147 | lxc1 := s.startInstance(c, "1/lxc/1") | 147 | lxc1 := s.startInstance(c, "1/lxc/1") |
1258 | 148 | s.assertInstances(c, lxc0, lxc1) | 148 | s.assertInstances(c, lxc0, lxc1) |
1259 | 149 | 149 | ||
1261 | 150 | err := s.broker.StopInstances([]instance.Instance{lxc1}) | 150 | err := s.broker.StopInstances(lxc1.Id()) |
1262 | 151 | c.Assert(err, gc.IsNil) | 151 | c.Assert(err, gc.IsNil) |
1263 | 152 | lxc2 := s.startInstance(c, "1/lxc/2") | 152 | lxc2 := s.startInstance(c, "1/lxc/2") |
1264 | 153 | s.assertInstances(c, lxc0, lxc2) | 153 | s.assertInstances(c, lxc0, lxc2) |
1265 | 154 | 154 | ||
1266 | === modified file 'worker/provisioner/provisioner_task.go' | |||
1267 | --- worker/provisioner/provisioner_task.go 2014-05-08 06:58:42 +0000 | |||
1268 | +++ worker/provisioner/provisioner_task.go 2014-05-14 03:01:15 +0000 | |||
1269 | @@ -397,7 +397,11 @@ | |||
1270 | 397 | if len(instances) == 0 { | 397 | if len(instances) == 0 { |
1271 | 398 | return nil | 398 | return nil |
1272 | 399 | } | 399 | } |
1274 | 400 | if err := task.broker.StopInstances(instances); err != nil { | 400 | ids := make([]instance.Id, len(instances)) |
1275 | 401 | for i, inst := range instances { | ||
1276 | 402 | ids[i] = inst.Id() | ||
1277 | 403 | } | ||
1278 | 404 | if err := task.broker.StopInstances(ids...); err != nil { | ||
1279 | 401 | logger.Errorf("broker failed to stop instances: %v", err) | 405 | logger.Errorf("broker failed to stop instances: %v", err) |
1280 | 402 | return err | 406 | return err |
1281 | 403 | } | 407 | } |
1282 | @@ -484,7 +488,7 @@ | |||
1283 | 484 | } | 488 | } |
1284 | 485 | // We need to stop the instance right away here, set error status and go on. | 489 | // We need to stop the instance right away here, set error status and go on. |
1285 | 486 | task.setErrorStatus("cannot register instance for machine %v: %v", machine, err) | 490 | task.setErrorStatus("cannot register instance for machine %v: %v", machine, err) |
1287 | 487 | if err := task.broker.StopInstances([]instance.Instance{inst}); err != nil { | 491 | if err := task.broker.StopInstances(inst.Id()); err != nil { |
1288 | 488 | // We cannot even stop the instance, log the error and quit. | 492 | // We cannot even stop the instance, log the error and quit. |
1289 | 489 | logger.Errorf("cannot stop instance %q for machine %v: %v", inst.Id(), machine, err) | 493 | logger.Errorf("cannot stop instance %q for machine %v: %v", inst.Id(), machine, err) |
1290 | 490 | return err | 494 | return err |
1291 | 491 | 495 | ||
1292 | === modified file 'worker/provisioner/provisioner_test.go' | |||
1293 | --- worker/provisioner/provisioner_test.go 2014-04-30 23:18:40 +0000 | |||
1294 | +++ worker/provisioner/provisioner_test.go 2014-05-14 03:01:15 +0000 | |||
1295 | @@ -254,8 +254,8 @@ | |||
1296 | 254 | case o := <-s.op: | 254 | case o := <-s.op: |
1297 | 255 | switch o := o.(type) { | 255 | switch o := o.(type) { |
1298 | 256 | case dummy.OpStopInstances: | 256 | case dummy.OpStopInstances: |
1301 | 257 | for _, stoppedInstance := range o.Instances { | 257 | for _, id := range o.Ids { |
1302 | 258 | instId := string(stoppedInstance.Id()) | 258 | instId := string(id) |
1303 | 259 | instanceIdsToStop.Remove(instId) | 259 | instanceIdsToStop.Remove(instId) |
1304 | 260 | if instanceIdsToKeep.Contains(instId) { | 260 | if instanceIdsToKeep.Contains(instId) { |
1305 | 261 | c.Errorf("provisioner unexpectedly stopped instance %s", instId) | 261 | c.Errorf("provisioner unexpectedly stopped instance %s", instId) |
Reviewers: mp+219347_ code.launchpad. net,
Message:
Please take a look.
Description:
Change StopInstances to take []instance.Id
StopInstances previously took []instance. Instance,
which is unnecessary for most providers, and imposes
an unnecessary cost for callers that only have IDs
initially. We change StopInstances to
The openstack provider's StopInstances is now slightly
more expensive than before, requiring an additional call
to Instances. This cost will disappear when we fix the
security groups/open-port implementation.
Drive-by: the maas provider now uses a bulk "release"
in StopInstances.
Live tested on ec2, local, virtual MAAS, canonistack, azure.
I lack a Joyent account, but I think it should be fine.
Fixes lp:1260171
Fixes lp:1316272
https:/ /code.launchpad .net/~axwalk/ juju-core/ lp1260171- stopinstances- ids/+merge/ 219347
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/97400043/
Affected files (+234, -257 lines): interface. go kvm/kvm. go kvm/kvm_ test.go kvm/live_ test.go lxc/lxc. go lxc/lxc_ test.go jujutest/ livetests. go jujutest/ tests.go azure/environ. go azure/environ_ test.go common/ bootstrap. go common/ bootstrap_ test.go common/ destroy. go common/ destroy_ test.go common/ mock_test. go dummy/environs. go ec2/live_ test.go joyent/ environ_ instance. go joyent/ instance. go joyent/ local_test. go local/environ. go maas/environ. go maas/environ_ whitebox_ test.go maas/util. go maas/util_ test.go manual/ environ. go openstack/ local_test. go openstack/ provider. go /client/ destroy. go provisioner/ kvm-broker. go provisioner/ kvm-broker_ test.go provisioner/ lxc-broker. go provisioner/ lxc-broker_ test.go provisioner/ provisioner_ task.go provisioner/ provisioner_ test.go
A [revision details]
M container/
M container/
M container/
M container/
M container/
M container/
M dependencies.tsv
M environs/broker.go
M environs/
M environs/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/ec2/ec2.go
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M state/apiserver
M worker/
M worker/
M worker/
M worker/
M worker/
M worker/