Skip to content

Commit 016988a

Browse files
hmlaniganhmlanigan
committed
Check for CodeMachineHasContainers if EnsureDead fails.
A machine with containers can now be marked dying. However it should not be marked dead until all containers have been removed. Allow the worker to retry EnsureDead as the worker's watcher will not trigger once all the containers have been removed. If restarting while machine is dying or dead, no need to set the addresses. Improve comments. Add TestMachinerMachineHasContainers. And update current tests for changed behavior around EnsureDead failing on having containers.
1 parent 5df4da8 commit 016988a

File tree

2 files changed

+63
-35
lines changed

2 files changed

+63
-35
lines changed

‎worker/machiner/machiner.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,21 @@ func (mr *Machiner) SetUp() (watcher.NotifyWatcher, error) {
8585
}
8686
mr.machine = m
8787

88-
if mr.config.ClearMachineAddressesOnStart {
88+
switch {
89+
case corelife.IsNotAlive(m.Life()):
90+
// Can happen when the machiner is restarting after a failure in EnsureDead.
91+
// Since we're dying or dead, no need handle the machine addresses.
92+
logger.Infof("%q not alive", mr.config.Tag)
93+
return m.Watch()
94+
case mr.config.ClearMachineAddressesOnStart:
8995
logger.Debugf("machiner configured to reset machine %q addresses to empty", mr.config.Tag)
9096
if err := m.SetMachineAddresses(nil); err != nil {
9197
return nil, errors.Annotate(err, "resetting machine addresses")
9298
}
93-
} else {
94-
// Set the addresses in state to the host's addresses.
99+
default:
100+
// Set the addresses in state to the host's addresses if the
101+
// machine is alive. No need to set addresses if the machine
102+
// is dying or dead on a worker restart.
95103
if err := setMachineAddresses(mr.config.Tag, m); err != nil {
96104
return nil, errors.Annotate(err, "setting machine addresses")
97105
}
@@ -182,6 +190,7 @@ func (mr *Machiner) Handle(_ <-chan struct{}) error {
182190

183191
return nil
184192
}
193+
185194
logger.Debugf("%q is now %s", mr.config.Tag, life)
186195
if err := mr.machine.SetStatus(status.Stopped, "", nil); err != nil {
187196
return errors.Annotatef(err, "%s failed to set status stopped", mr.config.Tag)
@@ -191,15 +200,22 @@ func (mr *Machiner) Handle(_ <-chan struct{}) error {
191200
// assigned, or storage attached, this will fail with
192201
// CodeHasAssignedUnits or CodeMachineHasAttachedStorage respectively.
193202
// Once units or storage are removed, the watcher will trigger again
194-
// and we'll reattempt.
203+
// and we'll reattempt. If the machine has containers, EnsureDead will
204+
// fail with CodeMachineHasContainers. However the watcher will not
205+
// trigger again, so fail and let the machiner restart and try again.
195206
if err := mr.machine.EnsureDead(); err != nil {
196207
if params.IsCodeHasAssignedUnits(err) {
208+
logger.Tracef("machine still has units")
197209
return nil
198210
}
199211
if params.IsCodeMachineHasAttachedStorage(err) {
200212
logger.Tracef("machine still has storage attached")
201213
return nil
202214
}
215+
if params.IsCodeMachineHasContainers(err) {
216+
logger.Tracef("machine still has containers")
217+
return errors.Annotatef(err, "%q", mr.config.Tag)
218+
}
203219
err = errors.Annotatef(err, "%s failed to set machine to dead", mr.config.Tag)
204220
if e := mr.machine.SetStatus(status.Error, errors.Annotate(err, "destroying machine").Error(), nil); e != nil {
205221
logger.Errorf("failed to set status for error %v ", err)

‎worker/machiner/machiner_test.go

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,6 @@ func (s *MachinerSuite) testMachinerMachineRefreshNotFoundOrUnauthorized(c *gc.C
119119
func (s *MachinerSuite) TestMachinerSetStatusStopped(c *gc.C) {
120120
s.accessor.machine.life = life.Dying
121121
s.accessor.machine.SetErrors(
122-
nil, // SetMachineAddresses
123-
nil, // SetStatus (started)
124122
nil, // Watch
125123
nil, // Refresh
126124
errors.New("cannot set status"), // SetStatus (stopped)
@@ -137,15 +135,14 @@ func (s *MachinerSuite) TestMachinerSetStatusStopped(c *gc.C) {
137135
"machine-123 failed to set status stopped: cannot set status",
138136
)
139137
s.accessor.machine.CheckCallNames(c,
140-
"SetMachineAddresses",
141-
"SetStatus",
138+
"Life",
142139
"Watch",
143140
"Refresh",
144141
"Life",
145142
"SetStatus",
146143
)
147144
s.accessor.machine.CheckCall(
148-
c, 5, "SetStatus",
145+
c, 4, "SetStatus",
149146
status.Stopped,
150147
"",
151148
map[string]interface{}(nil),
@@ -155,8 +152,6 @@ func (s *MachinerSuite) TestMachinerSetStatusStopped(c *gc.C) {
155152
func (s *MachinerSuite) TestMachinerMachineEnsureDeadError(c *gc.C) {
156153
s.accessor.machine.life = life.Dying
157154
s.accessor.machine.SetErrors(
158-
nil, // SetMachineAddresses
159-
nil, // SetStatus
160155
nil, // Watch
161156
nil, // Refresh
162157
nil, // SetStatus
@@ -174,7 +169,7 @@ func (s *MachinerSuite) TestMachinerMachineEnsureDeadError(c *gc.C) {
174169
"machine-123 failed to set machine to dead: cannot ensure machine is dead",
175170
)
176171
s.accessor.machine.CheckCall(
177-
c, 7, "SetStatus",
172+
c, 6, "SetStatus",
178173
status.Error,
179174
"destroying machine: machine-123 failed to set machine to dead: cannot ensure machine is dead",
180175
map[string]interface{}(nil),
@@ -184,8 +179,6 @@ func (s *MachinerSuite) TestMachinerMachineEnsureDeadError(c *gc.C) {
184179
func (s *MachinerSuite) TestMachinerMachineAssignedUnits(c *gc.C) {
185180
s.accessor.machine.life = life.Dying
186181
s.accessor.machine.SetErrors(
187-
nil, // SetMachineAddresses
188-
nil, // SetStatus
189182
nil, // Watch
190183
nil, // Refresh
191184
nil, // SetStatus
@@ -204,8 +197,37 @@ func (s *MachinerSuite) TestMachinerMachineAssignedUnits(c *gc.C) {
204197
c.Check(err, jc.ErrorIsNil)
205198

206199
s.accessor.machine.CheckCallNames(c,
207-
"SetMachineAddresses",
200+
"Life",
201+
"Watch",
202+
"Refresh",
203+
"Life",
208204
"SetStatus",
205+
"EnsureDead",
206+
)
207+
}
208+
209+
func (s *MachinerSuite) TestMachinerMachineHasContainers(c *gc.C) {
210+
s.accessor.machine.life = life.Dying
211+
s.accessor.machine.SetErrors(
212+
nil, // Watch
213+
nil, // Refresh
214+
nil, // SetStatus
215+
&params.Error{Code: params.CodeMachineHasContainers}, // EnsureDead
216+
)
217+
w, err := machiner.NewMachiner(machiner.Config{
218+
MachineAccessor: s.accessor,
219+
Tag: s.machineTag,
220+
})
221+
c.Assert(err, jc.ErrorIsNil)
222+
s.accessor.machine.watcher.changes <- struct{}{}
223+
err = stopWorker(w)
224+
225+
// If EnsureDead fails with "machine has containers", then
226+
// the worker will fail and restart.
227+
c.Check(err, jc.Satisfies, params.IsCodeMachineHasContainers)
228+
229+
s.accessor.machine.CheckCallNames(c,
230+
"Life",
209231
"Watch",
210232
"Refresh",
211233
"Life",
@@ -220,8 +242,6 @@ func (s *MachinerSuite) TestMachinerStorageAttached(c *gc.C) {
220242
// this should not cause an error.
221243
s.accessor.machine.life = life.Dying
222244
s.accessor.machine.SetErrors(
223-
nil, // SetMachineAddresses
224-
nil, // SetStatus
225245
nil, // Watch
226246
nil, // Refresh
227247
nil, // SetStatus
@@ -242,20 +262,7 @@ func (s *MachinerSuite) TestMachinerStorageAttached(c *gc.C) {
242262
}})
243263

244264
s.accessor.machine.CheckCalls(c, []gitjujutesting.StubCall{{
245-
FuncName: "SetMachineAddresses",
246-
Args: []interface{}{
247-
[]corenetwork.MachineAddress{
248-
corenetwork.NewMachineAddress("255.255.255.255"),
249-
corenetwork.NewMachineAddress("0.0.0.0"),
250-
},
251-
},
252-
}, {
253-
FuncName: "SetStatus",
254-
Args: []interface{}{
255-
status.Started,
256-
"",
257-
map[string]interface{}(nil),
258-
},
265+
FuncName: "Life",
259266
}, {
260267
FuncName: "Watch",
261268
}, {
@@ -278,6 +285,7 @@ func (s *MachinerSuite) TestRunStop(c *gc.C) {
278285
mr := s.makeMachiner(c, false)
279286
c.Assert(worker.Stop(mr), jc.ErrorIsNil)
280287
s.accessor.machine.CheckCallNames(c,
288+
"Life",
281289
"SetMachineAddresses",
282290
"SetStatus",
283291
"Watch",
@@ -289,12 +297,13 @@ func (s *MachinerSuite) TestStartSetsStatus(c *gc.C) {
289297
err := stopWorker(mr)
290298
c.Assert(err, jc.ErrorIsNil)
291299
s.accessor.machine.CheckCallNames(c,
300+
"Life",
292301
"SetMachineAddresses",
293302
"SetStatus",
294303
"Watch",
295304
)
296305
s.accessor.machine.CheckCall(
297-
c, 1, "SetStatus",
306+
c, 2, "SetStatus",
298307
status.Started, "", map[string]interface{}(nil),
299308
)
300309
}
@@ -357,7 +366,7 @@ LXC_BRIDGE="ignored"`[1:])
357366

358367
mr := s.makeMachiner(c, false)
359368
c.Assert(stopWorker(mr), jc.ErrorIsNil)
360-
s.accessor.machine.CheckCall(c, 0, "SetMachineAddresses", []corenetwork.MachineAddress{
369+
s.accessor.machine.CheckCall(c, 1, "SetMachineAddresses", []corenetwork.MachineAddress{
361370
corenetwork.NewScopedMachineAddress("10.0.0.1", corenetwork.ScopeCloudLocal),
362371
corenetwork.NewScopedMachineAddress("127.0.0.1", corenetwork.ScopeMachineLocal),
363372
corenetwork.NewScopedMachineAddress("::1", corenetwork.ScopeMachineLocal),
@@ -370,13 +379,13 @@ func (s *MachinerSuite) TestSetMachineAddressesEmpty(c *gc.C) {
370379
mr := s.makeMachiner(c, false)
371380
c.Assert(stopWorker(mr), jc.ErrorIsNil)
372381
// No call to SetMachineAddresses
373-
s.accessor.machine.CheckCallNames(c, "SetStatus", "Watch")
382+
s.accessor.machine.CheckCallNames(c, "Life", "SetStatus", "Watch")
374383
}
375384

376385
func (s *MachinerSuite) TestMachineAddressesWithClearFlag(c *gc.C) {
377386
mr := s.makeMachiner(c, true)
378387
c.Assert(stopWorker(mr), jc.ErrorIsNil)
379-
s.accessor.machine.CheckCall(c, 0, "SetMachineAddresses", []corenetwork.MachineAddress(nil))
388+
s.accessor.machine.CheckCall(c, 1, "SetMachineAddresses", []corenetwork.MachineAddress(nil))
380389
}
381390

382391
func (s *MachinerSuite) TestGetObservedNetworkConfigEmpty(c *gc.C) {
@@ -389,6 +398,7 @@ func (s *MachinerSuite) TestGetObservedNetworkConfigEmpty(c *gc.C) {
389398
c.Assert(stopWorker(mr), jc.ErrorIsNil)
390399

391400
s.accessor.machine.CheckCallNames(c,
401+
"Life",
392402
"SetMachineAddresses",
393403
"SetStatus",
394404
"Watch",
@@ -407,6 +417,7 @@ func (s *MachinerSuite) TestSetObservedNetworkConfig(c *gc.C) {
407417
c.Assert(stopWorker(mr), jc.ErrorIsNil)
408418

409419
s.accessor.machine.CheckCallNames(c,
420+
"Life",
410421
"SetMachineAddresses",
411422
"SetStatus",
412423
"Watch",
@@ -426,6 +437,7 @@ func (s *MachinerSuite) TestAliveErrorGetObservedNetworkConfig(c *gc.C) {
426437
c.Assert(stopWorker(mr), gc.ErrorMatches, "cannot discover observed network config: no config!")
427438

428439
s.accessor.machine.CheckCallNames(c,
440+
"Life",
429441
"SetMachineAddresses",
430442
"SetStatus",
431443
"Watch",

0 commit comments

Comments
 (0)