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

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