migration: REAP failure handling issues

[Menno Finlay-Smits]

REAPFAILED and an error are returned when the API call to Reap fails. This results in the worker restarting instead of ending up in REAPFAILED.

Also, consider the possibility of the migrationmaster being killed because of model doc removal, before it gets to set the phase to REAPFAILED or DONE (insert a sleep to check).

[Christian Muirhead (2-xtian)]

I don't understand this bug - the code for `doREAP` returns `REAPFAILED, nil` on error (and always has as far as I can tell).

I haven't tried provoking the worker being killed by the model removal yet.

[Christian Muirhead (2-xtian)]

D'oh sorry - it didn't always, but it has since December 15th 2016, before this bug was created.

[Menno Finlay-Smits (menno.smits)]

Sorry for the confusion. I suspect this ticket was created from a list I was maintaining and I probably forgot that I'd already dealt with the first part.

The second part is still a possible issue though. doREAP ends up removing the model doc which means there's a race where `w.killed()` might return true before the migration phase gets set to DONE or REAPFAILED. I suspect the fix is to check w.killed() *after* the `SetPhase` call. Thoughts?

[Christian Muirhead (2-xtian)]

I'm trying it now but I don't think that'll work - the migrationmaster facade is a model-specific one, so calling SetPhase after the model has gone will probably fail.

[Christian Muirhead (2-xtian)]

PR to change the API Reap call to update the migration phase here: https://github.com/juju/juju/pull/7647

[Christian Muirhead (2-xtian)]

PR for 2.2 branch: https://github.com/juju/juju/pull/7652