ssh: only rely on timeout/retries is one is set

In a configuration, when either the SSH timeout or the SSH handshake
retries are set, the other attribute would also be set to its default
value.

This in turn could cause a problem where a user expects his choice to be
the one that makes the final decision to abort, but in the end this
choice could cause the build to be cancelled independently as the other
setting may cause a premature cancellation.

To avoid this, we clarify the documentation, and ensure that when one is
set, the other will be ignored.

cmd/packer-sdc/internal/renderdocs/docs-partials/packer-plugin-sdk/communicator/SSH-not-required.mdx

@@ -44,11 +44,12 @@
 - `ssh_timeout` (duration string | ex: "1h5m2s") - The time to wait for SSH to become available. Packer uses this to
   determine when the machine has booted so this is usually quite long.
   Example value: `10m`.
+  This defaults to `5m`, unless `ssh_handshake_attempts` is set.
 
 - `ssh_disable_agent_forwarding` (bool) - If true, SSH agent forwarding will be disabled. Defaults to `false`.
 
-- `ssh_handshake_attempts` (int) - The number of handshakes to attempt with SSH once it can connect. This
-  defaults to `10`.
+- `ssh_handshake_attempts` (int) - The number of handshakes to attempt with SSH once it can connect.
+  This defaults to `10`, unless a `ssh_timeout` is set.
 
 - `ssh_bastion_host` (string) - A bastion host to use for the actual SSH connection.
 

communicator/config.go

@@ -123,6 +123,7 @@ type SSH struct {
 	// The time to wait for SSH to become available. Packer uses this to
 	// determine when the machine has booted so this is usually quite long.
 	// Example value: `10m`.
+	// This defaults to `5m`, unless `ssh_handshake_attempts` is set.
 	SSHTimeout time.Duration `mapstructure:"ssh_timeout"`
 	// Deprecated in favor of SSHTimeout
 	SSHWaitTimeout time.Duration `mapstructure:"ssh_wait_timeout" undocumented:"true"`
@@ -135,8 +136,8 @@ type SSH struct {
 	SSHAgentAuth bool `mapstructure:"ssh_agent_auth" undocumented:"true"`
 	// If true, SSH agent forwarding will be disabled. Defaults to `false`.
 	SSHDisableAgentForwarding bool `mapstructure:"ssh_disable_agent_forwarding"`
-	// The number of handshakes to attempt with SSH once it can connect. This
-	// defaults to `10`.
+	// The number of handshakes to attempt with SSH once it can connect.
+	// This defaults to `10`, unless a `ssh_timeout` is set.
 	SSHHandshakeAttempts int `mapstructure:"ssh_handshake_attempts"`
 	// A bastion host to use for the actual SSH connection.
 	SSHBastionHost string `mapstructure:"ssh_bastion_host"`
@@ -472,18 +473,16 @@ func (c *Config) prepareSSH(ctx *interpolate.Context) []error {
 		c.SSHPort = 22
 	}
 
-	if c.SSHTimeout == 0 {
+	// Only set default values when neither are set
+	if c.SSHTimeout == 0 && c.SSHHandshakeAttempts == 0 {
 		c.SSHTimeout = 5 * time.Minute
+		c.SSHHandshakeAttempts = 10
 	}
 
 	if c.SSHKeepAliveInterval == 0 {
 		c.SSHKeepAliveInterval = 5 * time.Second
 	}
 
-	if c.SSHHandshakeAttempts == 0 {
-		c.SSHHandshakeAttempts = 10
-	}
-
 	if c.SSHBastionHost != "" {
 		if c.SSHBastionPort == 0 {
 			c.SSHBastionPort = 22

communicator/step_connect_ssh.go

@@ -50,7 +50,10 @@ func (s *StepConnectSSH) Run(ctx context.Context, state multistep.StateBag) mult
 	}()
 
 	log.Printf("[INFO] Waiting for SSH, up to timeout: %s", s.Config.SSHTimeout)
-	timeout := time.After(s.Config.SSHTimeout)
+	timeout := make(<-chan time.Time)
+	if s.Config.SSHTimeout > 0 {
+		timeout = time.After(s.Config.SSHTimeout)
+	}
 	for {
 		// Wait for either SSH to become available, a timeout to occur,
 		// or an interrupt to come through.
@@ -231,14 +234,15 @@ func (s *StepConnectSSH) waitForSSH(state multistep.StateBag, ctx context.Contex
 				handshakeAttempts += 1
 			}
 
-			if handshakeAttempts < s.Config.SSHHandshakeAttempts {
-				// Try to connect via SSH a handful of times. We sleep here
-				// so we don't get a ton of authentication errors back to back.
-				time.Sleep(2 * time.Second)
-				continue
+			if s.Config.SSHHandshakeAttempts > 0 &&
+				handshakeAttempts >= s.Config.SSHHandshakeAttempts {
+				return nil, err
 			}
 
-			return nil, err
+			// Try to connect via SSH a handful of times. We sleep here
+			// so we don't get a ton of authentication errors back to back.
+			time.Sleep(2 * time.Second)
+			continue
 		}
 
 		break