veyron/services/mgmt/node/impl: fix flakiness in testNodeManager
Should fix https://github.com/veyron/release-issues/issues/477
testNodeManager runs a candidate node manager as a subprocess, as part of
Update(). The node manager subprocess is started via a shell script (created by
generateScript()). The old behavior of testNodeManager was to issue a Revert()
to the subprocess node manager. Revert() has the effect that the subprocess node
manager calls Stop() on itself, which eventually causes the subprocess to
exit. However, the Revert() call returns before the subprocess exits, and hence
the subprocess may still be running by the time Update() on the parent returns.
The parent eventually exits, but the child may still be running. This causes a
race as far as the mount entry created by the subprocess node manager is
concerend: the mount entry may or may not still be present by the time the
parent has exited, depending on whether the subprocess may or may not still be
running. Hence, resolveExpectNotFound on the subprocess node manager's mount
name may or may not flake. Note, the parent does run handle.Clean() on the
subprocess handle before returning from Update(), but that only has the effect
of killing the shell that runs the subprocess node manager, and not the node
manager process itself. Either way, killing the subprocess node manager is not
the right solution, since we want to give it a chance to shut itself down
cleanly, including unmounting its mount entries.
The fix is to Wait for the subprocess to exit cleanly (as a result of the Stop
call that it makes on itself as part of Revert). That ensures that the
subprocess is no longer running by the time Update on the parent returns, and in
particular, that all mount entries have been cleaned up.
While at it, also improve the error message from resolveExpectNotFound to reveal
the call site.
Change-Id: Icfa2a314f43ab78de542bbef07be139d62c5f033
3 files changed
