A subordinate charm hook scheduled to run(but waiting for the principal charm hook to release the lock) goes to an error state after the principal charm hook triggers a reboot.

[Adrian Vladu]

This scenario needs at least 3 charms:
- one principal and one subordinate charm
- a third charm that services the principal charm

A relation must exist between the principal and the third charm.

This issue happens only when the principal charm executes the relation hook triggered by the third charm(let's name this hook third-relation-joined-hook), while the subordinate charm has at least a charm hook in the queue(let's name this hook secondary-relation-hook). secondary-relation-hook execution must wait for the lock to be released by third-relation-joined-hook.

if third-relation-joined-hook triggers a reboot using the command "juju-reboot --now", after the subsequent reboot, secondary-relation-hook goes into an error state.

This issue happens on Windows 2012 R2 with Juju versions 1.24 and 1.25.

[Gabriel Samfira (gabriel-samfira)]

This I believe happens because of the way hooks are run. The hook run has been split up in 3 steps:

* prepare
* execute
* commit

During prepare, the state of the hook is written, but we only start locking for hook execution in the "execute" phase. So we can have one hook that requires a reboot executing, and another one finishing the prepare phase, ready to execute. If we reboot the machine, when the uniter comes back up, it will see the second hook in the preparing phase, but has no knowledge of this happening, and sets it in error state.

I think this can be fixed by simply locking in the prepare phase and unlocking after execute, instead of locking in execute.

[Gabriel Samfira (gabriel-samfira)]

Here is a quick and dirty fix for this:

https://github.com/juju/juju/compare/1.24...gabriel-samfira:executor-lock?expand=1

I am unsure on how to do this cleanly, or what the deeper implications of locking in "prepare" are. Any advice from @fwreade would be welcome :).

[William Reade (fwereade)]

I think this is essentially sound, but that logic is a bit tangly. Can we do it in operation.executor perhaps? (e.g. add a NeedsGlobalMachineLock() bool method to the Operation interface?, and check/acquire that first of all before even calling prepare?)

[Bogdan Teleaga (bteleaga)]

http://reviews.vapour.ws/r/2058/ <--- Backport