This adds a return when aborting from closeConnections, which should
have happened as a part of #1369.
Note that it's currently inconsequential that it's not happening right
now, save some misleading log entries. setCloseTimeForResponse will
effectively no-op on empty or nil input maps.
This backports some test work that is being added in #1340 to assert
that the worker is actually marking connections closed in its *local*
state on a failure to contact the controller. This was missed in #1357
during some late-stage refactoring.
Summary of the changes:
* If we can't actually contact the controller during connection closure
to mark the connections as closed, we actually construct a "fake"
response that just simply includes a closed state for all connections
requested. This is then used to update the local connection state.
* Added a 3 second timeout to the CloseConnection request to the
controller to ensure the call to close connections completed in a
timely manner and did not hang indefinitely.
Testing enhancements:
* Added a new helper in TestHelper called LookupSession that allows the
lookup of a session from the worker's internal session state.
* This is used in the new ExpectConnectionStateOnWorker check in the E2E
test, which is part of a larger amount of enhancements to the E2E test
that are coming in #1340.
* Added a ConnectionStatusFromProtoVal helper function to the session
package to allow the conversion of protobuf connection status values to
their higher-level counterparts; essentially it is a reverse of
ConnectionStatus.ProtoVal. This is used in some of our tests (with more
coming in #1340) to just help with keeping things more idiomatic.
Finally, there is some minor error refactoring, and we also change the
timeout on internal status and connection close requests to the
controller to 5 seconds.
This commit adds some rudimentary functionality for cleaning up
connections on controller failure, or more specifically, a failure to
communicate with the controllers.
In this scenario, when a status cannot be sent for any reason for longer
than 30 seconds, the worker will close all connections that are normally
not cleaned up (so for sessions that currently are *not* in the progress
of being canceled or terminated). Status requests now time out after 10
seconds if they are not made successfully.
Session status is not touched during this process, but natural cleanup
or connectionless sessions will still occur. This should be okay, as the
session should be added on re-connect if need be.
A few other changes are made to cancelConnections: first, error logging
has been moved to the function so that we can handle errors requesting
cancellation directly within the function; this entails simply just
locally marking these connections as closed with a warning. Second,
there was a minor logging bug where closed connections were not properly
being noted in the last trace log, this has been fixed.
Additional changes:
* Circle: Set acceptance test no_output_timeout to 30m
BK clued me into the fact that for some types, similar alignment
problems exist as with the normal atomic package (e.g. 64-bit types on
32-bit architectures) if they are not pointer values. So change the few
places they weren't pointers to pointers.
None of these are 64-bit, but it feels like best practice for using the
library and should just standardize on it.
* Attempt canceling sessions by the client
In some circumstances (like a Ctrl-C), the client knows it's shutting
down and the TOFU token will be lost. In this case it makes sense to
cancel the session rather than have it time out. This introduces a
command enum to the handshake message which can be used for the client
to send commands to the worker -- in this case the only specified
command is to cancel the session. It doesn't occur if the session is
already timed out or if we can't authorize connections as this generally
means the session is already invalid.
Go's TLS stack doesn't need a separate CA cert and in fact for sessions
we don't use this. So simplify the worker connection to remove an
unnecessary extra key generation.
It's up to the worker to understand what a change in state means, so
there was no functional difference between simply sending a state change
request and a cancel request.
* Possible api changes.
* Worker API update.
* Updating import alias name.
* Moving session down a level as a member of the GetSessionResponse.
* Marshal the auth flag into the GetSessionResponse proto instead of the session.
This was copied from Vault code but shouldn't be necessary any more as a
change in hclog a while back meant that derived loggers share the log
level of the parent.
Jeff Mitchell
Chris Marchesi
Jim
Michael GaffneyandJeff Mitchell
Christian Frichot
Todd Knight