[3003.2] Add server alive (#60573)

* add server alive

* rename log

* change default alive time

* add requested changes

* format string

* reformat string again

* run pre

* customize

* space

* remove EOF dead space

* fix pre-commit

* run pre

Co-authored-by: Megan Wilhite <megan.wilhite@gmail.com>

Author: cmcmarrow

changelog/60216.fixed

@@ -0,0 +1 @@
+Stop SSH from hanging if connection is lost. Also added args to customize grace period.

salt/utils/cloud.py

@@ -124,6 +124,9 @@
     r"(?:.*sudo)(?:.*)[Pp]assword(?: for .*)?:", re.M
 )
 
+SERVER_ALIVE_INTERVAL = 60
+SERVER_ALIVE_COUNT_MAX = 3
+
 # Get logging started
 log = logging.getLogger(__name__)
 
@@ -183,18 +186,24 @@ def __ssh_gateway_arguments(kwargs):
         ssh_gateway_user = kwargs.get("ssh_gateway_user", "root")
 
         # Setup ProxyCommand
-        extended_arguments = '-oProxyCommand="ssh {} {} {} {} {}@{} -p {} {}"'.format(
-            # Don't add new hosts to the host key database
-            "-oStrictHostKeyChecking=no",
-            # Set hosts key database path to /dev/null, i.e., non-existing
-            "-oUserKnownHostsFile=/dev/null",
-            # Don't re-use the SSH connection. Less failures.
-            "-oControlPath=none",
-            ssh_gateway_key,
-            ssh_gateway_user,
-            ssh_gateway,
-            ssh_gateway_port,
-            ssh_gateway_command,
+        extended_arguments = " ".join(
+            (
+                "ssh",
+                "-oStrictHostKeyChecking=no",
+                "-oServerAliveInterval={}".format(
+                    kwargs.get("server_alive_interval", SERVER_ALIVE_INTERVAL)
+                ),
+                "-oServerAliveCountMax={}".format(
+                    kwargs.get("server_alive_count_max", SERVER_ALIVE_COUNT_MAX)
+                ),
+                "-oUserKnownHostsFile=/dev/null",
+                "-oControlPath=none",
+                str(ssh_gateway_key),
+                "{}@{}".format(ssh_gateway_user, ssh_gateway),
+                "-p",
+                str(ssh_gateway_port),
+                str(ssh_gateway_command),
+            )
         )
 
         log.info(
@@ -605,7 +614,7 @@ def bootstrap(vm_, opts=None):
         "event",
         "executing deploy script",
         "salt/cloud/{}/deploying".format(vm_["name"]),
-        args={"kwargs": event_kwargs},
+        args={"kwargs": salt.utils.data.simple_types_filter(event_kwargs)},
         sock_dir=opts.get("sock_dir", os.path.join(__opts__["sock_dir"], "master")),
         transport=opts.get("transport", "zeromq"),
     )
@@ -691,7 +700,14 @@ def wait_for_fun(fun, timeout=900, **kwargs):
             return False
 
 
-def wait_for_port(host, port=22, timeout=900, gateway=None):
+def wait_for_port(
+    host,
+    port=22,
+    timeout=900,
+    gateway=None,
+    server_alive_interval=SERVER_ALIVE_INTERVAL,
+    server_alive_count_max=SERVER_ALIVE_COUNT_MAX,
+):
     """
     Wait until a connection to the specified port can be made on a specified
     host. This is usually port 22 (for SSH), but in the case of Windows
@@ -765,6 +781,9 @@ def wait_for_port(host, port=22, timeout=900, gateway=None):
         [
             # Don't add new hosts to the host key database
             "-oStrictHostKeyChecking=no",
+            # make sure ssh can time out on connection lose
+            "-oServerAliveInterval={}".format(server_alive_interval),
+            "-oServerAliveCountMax={}".format(server_alive_count_max),
             # Set hosts key database path to /dev/null, i.e., non-existing
             "-oUserKnownHostsFile=/dev/null",
             # Don't re-use the SSH connection. Less failures.
@@ -2146,6 +2165,13 @@ def scp_file(dest_path, contents=None, kwargs=None, local_file=None):
         ssh_args = [
             # Don't add new hosts to the host key database
             "-oStrictHostKeyChecking=no",
+            # make sure ssh can time out on connection lose
+            "-oServerAliveInterval={}".format(
+                kwargs.get("server_alive_interval", SERVER_ALIVE_INTERVAL)
+            ),
+            "-oServerAliveCountMax={}".format(
+                kwargs.get("server_alive_count_max", SERVER_ALIVE_COUNT_MAX)
+            ),
             # Set hosts key database path to /dev/null, i.e., non-existing
             "-oUserKnownHostsFile=/dev/null",
             # Don't re-use the SSH connection. Less failures.
@@ -2263,6 +2289,13 @@ def sftp_file(dest_path, contents=None, kwargs=None, local_file=None):
         ssh_args = [
             # Don't add new hosts to the host key database
             "-oStrictHostKeyChecking=no",
+            # make sure ssh can time out on connection lose
+            "-oServerAliveInterval={}".format(
+                kwargs.get("server_alive_interval", SERVER_ALIVE_INTERVAL)
+            ),
+            "-oServerAliveCountMax={}".format(
+                kwargs.get("server_alive_count_max", SERVER_ALIVE_COUNT_MAX)
+            ),
             # Set hosts key database path to /dev/null, i.e., non-existing
             "-oUserKnownHostsFile=/dev/null",
             # Don't re-use the SSH connection. Less failures.

tests/pytests/unit/utils/test_cloud.py

@@ -12,6 +12,7 @@
 
 import pytest
 import salt.utils.cloud as cloud
+from salt.utils.cloud import __ssh_gateway_arguments as ssh_gateway_arguments
 from tests.support.mock import MagicMock, patch
 
 
@@ -222,6 +223,143 @@ def test_winrm_pinnned_version():
     # fmt: on
 
 
+def test_ssh_gateway_arguments_default_alive_args():
+    server_alive_interval = 60
+    server_alive_count_max = 3
+    arguments = ssh_gateway_arguments({"ssh_gateway": "host"})
+    assert "-oServerAliveInterval={}".format(server_alive_interval) in arguments
+    assert "-oServerAliveCountMax={}".format(server_alive_count_max) in arguments
+
+
+def test_ssh_gateway_arguments_alive_args():
+    server_alive_interval = 10
+    server_alive_count_max = 8
+    arguments = ssh_gateway_arguments(
+        {
+            "ssh_gateway": "host",
+            "server_alive_interval": server_alive_interval,
+            "server_alive_count_max": server_alive_count_max,
+        }
+    )
+    assert "-oServerAliveInterval={}".format(server_alive_interval) in arguments
+    assert "-oServerAliveCountMax={}".format(server_alive_count_max) in arguments
+
+
+def test_wait_for_port_default_alive_args():
+    server_alive_interval = 60
+    server_alive_count_max = 3
+    with patch("salt.utils.cloud.socket", autospec=True), patch(
+        "salt.utils.cloud._exec_ssh_cmd", autospec=True, return_value=0
+    ) as exec_ssh_cmd:
+        cloud.wait_for_port(
+            "127.0.0.1", gateway={"ssh_gateway": "host", "ssh_gateway_user": "user"},
+        )
+        assert exec_ssh_cmd.call_count == 2
+        ssh_call = exec_ssh_cmd.call_args[0][0]
+        assert "-oServerAliveInterval={}".format(server_alive_interval) in ssh_call
+        assert "-oServerAliveCountMax={}".format(server_alive_count_max) in ssh_call
+
+
+def test_wait_for_port_alive_args():
+    server_alive_interval = 66
+    server_alive_count_max = 1
+    with patch("salt.utils.cloud.socket", autospec=True), patch(
+        "salt.utils.cloud._exec_ssh_cmd", autospec=True, return_value=0
+    ) as exec_ssh_cmd:
+        cloud.wait_for_port(
+            "127.0.0.1",
+            server_alive_interval=server_alive_interval,
+            server_alive_count_max=server_alive_count_max,
+            gateway={"ssh_gateway": "host", "ssh_gateway_user": "user"},
+        )
+        assert exec_ssh_cmd.call_count == 2
+        ssh_call = exec_ssh_cmd.call_args[0][0]
+        assert "-oServerAliveInterval={}".format(server_alive_interval) in ssh_call
+        assert "-oServerAliveCountMax={}".format(server_alive_count_max) in ssh_call
+
+
+def test_scp_file_default_alive_args():
+    server_alive_interval = 60
+    server_alive_count_max = 3
+    with patch("salt.utils.cloud.socket", autospec=True), patch(
+        "salt.utils.cloud._exec_ssh_cmd", autospec=True, return_value=0
+    ) as exec_ssh_cmd:
+        cloud.scp_file(
+            "/salt.txt",
+            contents=None,
+            kwargs={"hostname": "127.0.0.1", "username": "user"},
+            local_file="/salt.txt",
+        )
+        assert exec_ssh_cmd.call_count == 1
+        ssh_call = exec_ssh_cmd.call_args[0][0]
+        assert "-oServerAliveInterval={}".format(server_alive_interval) in ssh_call
+        assert "-oServerAliveCountMax={}".format(server_alive_count_max) in ssh_call
+
+
+def test_scp_file_alive_args():
+    server_alive_interval = 64
+    server_alive_count_max = 4
+    with patch("salt.utils.cloud.socket", autospec=True), patch(
+        "salt.utils.cloud._exec_ssh_cmd", autospec=True, return_value=0
+    ) as exec_ssh_cmd:
+        cloud.scp_file(
+            "/salt.txt",
+            contents=None,
+            kwargs={
+                "hostname": "127.0.0.1",
+                "username": "user",
+                "server_alive_interval": server_alive_interval,
+                "server_alive_count_max": server_alive_count_max,
+            },
+            local_file="/salt.txt",
+        )
+        assert exec_ssh_cmd.call_count == 1
+        ssh_call = exec_ssh_cmd.call_args[0][0]
+        assert "-oServerAliveInterval={}".format(server_alive_interval) in ssh_call
+        assert "-oServerAliveCountMax={}".format(server_alive_count_max) in ssh_call
+
+
+def test_sftp_file_default_alive_args():
+    server_alive_interval = 60
+    server_alive_count_max = 3
+    with patch("salt.utils.cloud.socket", autospec=True), patch(
+        "salt.utils.cloud._exec_ssh_cmd", autospec=True, return_value=0
+    ) as exec_ssh_cmd:
+        cloud.sftp_file(
+            "/salt.txt",
+            contents=None,
+            kwargs={"hostname": "127.0.0.1", "username": "user"},
+            local_file="/salt.txt",
+        )
+        assert exec_ssh_cmd.call_count == 1
+        ssh_call = exec_ssh_cmd.call_args[0][0]
+        assert "-oServerAliveInterval={}".format(server_alive_interval) in ssh_call
+        assert "-oServerAliveCountMax={}".format(server_alive_count_max) in ssh_call
+
+
+def test_sftp_file_alive_args():
+    server_alive_interval = 62
+    server_alive_count_max = 6
+    with patch("salt.utils.cloud.socket", autospec=True), patch(
+        "salt.utils.cloud._exec_ssh_cmd", autospec=True, return_value=0
+    ) as exec_ssh_cmd:
+        cloud.sftp_file(
+            "/salt.txt",
+            contents=None,
+            kwargs={
+                "hostname": "127.0.0.1",
+                "username": "user",
+                "server_alive_interval": server_alive_interval,
+                "server_alive_count_max": server_alive_count_max,
+            },
+            local_file="/salt.txt",
+        )
+        assert exec_ssh_cmd.call_count == 1
+        ssh_call = exec_ssh_cmd.call_args[0][0]
+        assert "-oServerAliveInterval={}".format(server_alive_interval) in ssh_call
+        assert "-oServerAliveCountMax={}".format(server_alive_count_max) in ssh_call
+
+
 def test_deploy_script_ssh_timeout():
     with patch("salt.utils.cloud.root_cmd", return_value=False) as root_cmd, patch(
         "salt.utils.cloud.wait_for_port", return_value=True