This commit makes two changes to the provisioner connection block code:
- Change the `port` argument type from string to number, which is
technically more correct and consistent with `bastion_port`;
- Use `uint16` as the struct member type for both ports instead of
`int`, which gets us free range validation from the gocty package.
Includes a test of the validation message when the port number is an
invalid integer.
The connection block schema defines the bastion_port argument as a
number, but we were incorrectly trying to convert it from a string. This
commit fixes that by attempting to convert the cty.Number to the int
result type, returning the error on failure.
An alternative approach would be to change the bastion_port argument in
the schema to be a string, matching the port argument. I'm less sure
about the secondary effects of that change, though.
If there is an error when opening the session for agent forwarding in
the process ssh connention, there is a deadlock when recursively
calling Connect on an internal reattempt. Avoid that, and let the
connection be reattempted externally.
If a connection fails and attempts to reconnect after the keep-alive
loop started, the client will be pulled out from under the keep-alive
requests. Close over a local copy of the client, so that reconnecting
doesn't race with the keepalive loop terminating.
1) Mention the host and port in the "Connecting..." message.
2) Mention the username in the post-connection handshaking message.
3) If handshaking fails, mention the user, host, and port in the error
message that will eventually be returned to the user.
An ssh server should always send a reply packet to the keepalive
request. If we miss those replies for over 2min, consider the connection
dead and abort, rather than block the provisioner indefinitely.
Match the tested behavior, and that of the ssh implementation, where the
communicator automatically connects when starting a command.
Remove unused import from legacy dependency handling.
The error from a remote command is not exported, and only exposed via
the Run method. Otherwise the Run method works exactly like the
runCommand function being removed.
Most of the time an ssh authentication failure would be non-recoverable,
but some host images can start the ssh service before it is properly
configured, or before user authentication data is available.
Log ssh authentication errors and allow the provisioner to retry until
the connection timeout.
Combine the ExitStatus and Err values from remote.Cmd into an error
returned by Wait, better matching the behavior of the os/exec package.
Non-zero exit codes are returned from Wait as a remote.ExitError.
Communicator related errors are returned directly.
Clean up all the error handling in the provisioners using a
communicator. Also remove the extra copyOutput synchronization that was
copied from package to package.
The remote.Cmd struct could not convey any transport related error to
the caller, meaning that interrupted commands would show that they
succeeded.
Change Cmd.SetExited to accept an exit status, as well as an error to
store for the caller. Make the status and error fields internal,
require serialized access through the getter methods.
Users of remote.Cmd should not check both Cmd.Err() and Cmd.ExitStatus()
until after Wait returns.
Require communicators to call Cmd.Init before executing the command.
This will indicate incorrect usage of the remote.Cmd by causing a panic
in SetExitStatus.
Alisdair McDiarmid