Handle error if SSH service no present.

cloudinit/config/cc_set_passwords.py

@@ -14,7 +14,7 @@
 from cloudinit import log as logging
 from cloudinit import subp, util
 from cloudinit.config.schema import MetaSchema, get_meta_doc
-from cloudinit.distros import ALL_DISTROS, ug_util
+from cloudinit.distros import ALL_DISTROS, Distro, ug_util
 from cloudinit.settings import PER_INSTANCE
 from cloudinit.ssh_util import update_ssh_config
 
@@ -79,14 +79,47 @@
 PW_SET = "".join([x for x in ascii_letters + digits if x not in "loLOI01"])
 
 
-def handle_ssh_pwauth(pw_auth, distro):
+def handle_ssh_pwauth(pw_auth, distro: Distro):
     """Apply sshd PasswordAuthentication changes.
 
     @param pw_auth: config setting from 'pw_auth'.
                     Best given as True, False, or "unchanged".
     @param distro: an instance of the distro class for the target distribution
 
     @return: None"""
+    service = distro.get_option("ssh_svcname", "ssh")
+    restart_ssh = True
+    try:
+        distro.manage_service("status", service)
+    except subp.ProcessExecutionError as e:
+        if e.exit_code == 3:
+            # Service is not running. Write ssh config.
+            LOG.warning(
+                "Writing config 'ssh_pwauth: %s'."
+                " SSH service '%s' will not be restarted because is stopped.",
+                pw_auth,
+                service,
+            )
+            restart_ssh = False
+        elif e.exit_code == 4:
+            # Service status is unknown
+            LOG.warning(
+                "Ignoring config 'ssh_pwauth: %s'."
+                " SSH service '%s' is not installed.",
+                pw_auth,
+                service,
+            )
+            return
+        else:
+            LOG.warning(
+                "Ignoring config 'ssh_pwauth: %s'."
+                " SSH service '%s' is not available. Error: %s.",
+                pw_auth,
+                service,
+                e,
+            )
+            return
+
     cfg_name = "PasswordAuthentication"
 
     if isinstance(pw_auth, str):
@@ -112,8 +145,11 @@ def handle_ssh_pwauth(pw_auth, distro):
         LOG.debug("No need to restart SSH service, %s not updated.", cfg_name)
         return
 
-    distro.manage_service("restart", distro.get_option("ssh_svcname", "ssh"))
-    LOG.debug("Restarted the SSH daemon.")
+    if restart_ssh:
+        distro.manage_service("restart", service)
+        LOG.debug("Restarted the SSH daemon.")
+    else:
+        LOG.debug("Not restarting SSH service: service is stopped.")
 
 
 def handle(_name, cfg, cloud, log, args):
@@ -226,7 +262,7 @@ def handle(_name, cfg, cloud, log, args):
     handle_ssh_pwauth(cfg.get("ssh_pwauth"), cloud.distro)
 
     if len(errors):
-        log.debug("%s errors occured, re-raising the last one", len(errors))
+        log.debug("%s errors occurred, re-raising the last one", len(errors))
         raise errors[-1]
 
 

cloudinit/distros/__init__.py

@@ -850,7 +850,7 @@ def shutdown_command(self, *, mode, delay, message):
             args.append(message)
         return args
 
-    def manage_service(self, action, service):
+    def manage_service(self, action: str, service: str):
         """
         Perform the requested action on a service. This handles the common
         'systemctl' and 'service' cases and may be overridden in subclasses
@@ -867,6 +867,7 @@ def manage_service(self, action, service):
                 "restart": ["restart", service],
                 "reload": ["reload-or-restart", service],
                 "try-reload": ["reload-or-try-restart", service],
+                "status": ["status", service],
             }
         else:
             cmds = {
@@ -876,6 +877,7 @@ def manage_service(self, action, service):
                 "restart": [service, "restart"],
                 "reload": [service, "restart"],
                 "try-reload": [service, "restart"],
+                "status": [service, "status"],
             }
         cmd = list(init_cmd) + list(cmds[action])
         return subp.subp(cmd, capture=True)

packages/debian/control.in

@@ -14,7 +14,7 @@ Depends: ${misc:Depends},
          iproute2,
          isc-dhcp-client
 Recommends: eatmydata, sudo, software-properties-common, gdisk
-Suggests: ssh-import-id
+Suggests: ssh-import-id, openssh-server
 Description: Init scripts for cloud instances
  Cloud instances need special scripts to run during initialisation
  to retrieve and install ssh keys and to let the user run various scripts.

tests/unittests/config/test_cc_set_passwords.py

@@ -4,7 +4,7 @@
 
 import pytest
 
-from cloudinit import util
+from cloudinit import subp, util
 from cloudinit.config import cc_set_passwords as setpass
 from cloudinit.config.schema import (
     SchemaValidationError,
@@ -28,7 +28,10 @@ def test_unknown_value_logs_warning(self, m_subp):
         self.assertIn(
             "Unrecognized value: ssh_pwauth=floo", self.logs.getvalue()
         )
-        m_subp.assert_not_called()
+        self.assertEqual(
+            [mock.call(["systemctl", "status", "ssh"], capture=True)],
+            m_subp.call_args_list,
+        )
 
     @mock.patch(MODPATH + "update_ssh_config", return_value=True)
     @mock.patch("cloudinit.distros.subp.subp")
@@ -40,14 +43,18 @@ def test_systemctl_as_service_cmd(self, m_subp, m_update_ssh_config):
         m_subp.assert_called_with(
             ["systemctl", "restart", "ssh"], capture=True
         )
+        self.assertIn("DEBUG: Restarted the SSH daemon.", self.logs.getvalue())
 
     @mock.patch(MODPATH + "update_ssh_config", return_value=False)
     @mock.patch("cloudinit.distros.subp.subp")
     def test_not_restarted_if_not_updated(self, m_subp, m_update_ssh_config):
         """If config is not updated, then no system restart should be done."""
         cloud = self.tmp_cloud(distro="ubuntu")
         setpass.handle_ssh_pwauth(True, cloud.distro)
-        m_subp.assert_not_called()
+        self.assertEqual(
+            [mock.call(["systemctl", "status", "ssh"], capture=True)],
+            m_subp.call_args_list,
+        )
         self.assertIn("No need to restart SSH", self.logs.getvalue())
 
     @mock.patch(MODPATH + "update_ssh_config", return_value=True)
@@ -57,28 +64,132 @@ def test_unchanged_does_nothing(self, m_subp, m_update_ssh_config):
         cloud = self.tmp_cloud(distro="ubuntu")
         setpass.handle_ssh_pwauth("unchanged", cloud.distro)
         m_update_ssh_config.assert_not_called()
-        m_subp.assert_not_called()
+        self.assertEqual(m_update_ssh_config.call_count, 0)
+        self.assertEqual(
+            [mock.call(["systemctl", "status", "ssh"], capture=True)],
+            m_subp.call_args_list,
+        )
 
     @mock.patch("cloudinit.distros.subp.subp")
     def test_valid_change_values(self, m_subp):
         """If value is a valid changen value, then update should be called."""
         cloud = self.tmp_cloud(distro="ubuntu")
         upname = MODPATH + "update_ssh_config"
         optname = "PasswordAuthentication"
-        for value in util.FALSE_STRINGS + util.TRUE_STRINGS:
+        for n, value in enumerate(util.FALSE_STRINGS + util.TRUE_STRINGS, 1):
             optval = "yes" if value in util.TRUE_STRINGS else "no"
             with mock.patch(upname, return_value=False) as m_update:
                 setpass.handle_ssh_pwauth(value, cloud.distro)
-                m_update.assert_called_with({optname: optval})
-        m_subp.assert_not_called()
+                self.assertEqual(
+                    mock.call({optname: optval}), m_update.call_args_list[-1]
+                )
+            self.assertEqual(m_subp.call_count, n)
+        self.assertEqual(
+            mock.call(["systemctl", "status", "ssh"], capture=True),
+            m_subp.call_args_list[-1],
+        )
+
+    @mock.patch(MODPATH + "update_ssh_config", return_value=True)
+    @mock.patch("cloudinit.distros.subp.subp")
+    def test_failed_ssh_service_is_not_runing(
+        self, m_subp, m_update_ssh_config
+    ):
+        """If the ssh service is not running, then the config is updated and
+        no restart.
+        """
+        cloud = self.tmp_cloud(distro="ubuntu")
+        cloud.distro.init_cmd = ["systemctl"]
+        cloud.distro.manage_service = mock.Mock(
+            side_effect=subp.ProcessExecutionError(
+                stderr="Service is not running.", exit_code=3
+            )
+        )
+
+        setpass.handle_ssh_pwauth(True, cloud.distro)
+        self.assertIn(
+            r"WARNING: Writing config 'ssh_pwauth: True'."
+            r" SSH service 'ssh' will not be restarted because is stopped.",
+            self.logs.getvalue(),
+        )
+        self.assertIn(
+            r"DEBUG: Not restarting SSH service: service is stopped.",
+            self.logs.getvalue(),
+        )
+        self.assertEqual(
+            [mock.call("status", "ssh")],
+            cloud.distro.manage_service.call_args_list,
+        )
+        self.assertEqual(m_update_ssh_config.call_count, 1)
+        self.assertEqual(m_subp.call_count, 0)
+
+    @mock.patch(MODPATH + "update_ssh_config", return_value=True)
+    @mock.patch("cloudinit.distros.subp.subp")
+    def test_failed_ssh_service_is_not_installed(
+        self, m_subp, m_update_ssh_config
+    ):
+        """If the ssh service is not installed, then no updates config and
+        no restart.
+        """
+        cloud = self.tmp_cloud(distro="ubuntu")
+        cloud.distro.init_cmd = ["systemctl"]
+        cloud.distro.manage_service = mock.Mock(
+            side_effect=subp.ProcessExecutionError(
+                stderr="Service is not installed.", exit_code=4
+            )
+        )
+
+        setpass.handle_ssh_pwauth(True, cloud.distro)
+        self.assertIn(
+            r"WARNING: Ignoring config 'ssh_pwauth: True'."
+            r" SSH service 'ssh' is not installed.",
+            self.logs.getvalue(),
+        )
+        self.assertEqual(
+            [mock.call("status", "ssh")],
+            cloud.distro.manage_service.call_args_list,
+        )
+        self.assertEqual(m_update_ssh_config.call_count, 0)
+        self.assertEqual(m_subp.call_count, 0)
+
+    @mock.patch(MODPATH + "update_ssh_config", return_value=True)
+    @mock.patch("cloudinit.distros.subp.subp")
+    def test_failed_ssh_service_is_not_available(
+        self, m_subp, m_update_ssh_config
+    ):
+        """If the ssh service is not available, then no updates config and
+        no restart.
+        """
+        cloud = self.tmp_cloud(distro="ubuntu")
+        cloud.distro.init_cmd = ["systemctl"]
+        process_error = "Service is not available."
+        cloud.distro.manage_service = mock.Mock(
+            side_effect=subp.ProcessExecutionError(
+                stderr=process_error, exit_code=2
+            )
+        )
+
+        setpass.handle_ssh_pwauth(True, cloud.distro)
+        self.assertIn(
+            r"WARNING: Ignoring config 'ssh_pwauth: True'."
+            r" SSH service 'ssh' is not available. Error: ",
+            self.logs.getvalue(),
+        )
+        self.assertIn(process_error, self.logs.getvalue())
+        self.assertEqual(
+            [mock.call("status", "ssh")],
+            cloud.distro.manage_service.call_args_list,
+        )
+        self.assertEqual(m_update_ssh_config.call_count, 0)
+        self.assertEqual(m_subp.call_count, 0)
 
 
 class TestSetPasswordsHandle(CiTestCase):
     """Test cc_set_passwords.handle"""
 
     with_logs = True
 
-    def test_handle_on_empty_config(self, *args):
+    @mock.patch(MODPATH + "subp.subp")
+    def test_handle_on_empty_config(self, m_subp):
         """handle logs that no password has changed when config is empty."""
         cloud = self.tmp_cloud(distro="ubuntu")
         setpass.handle(
@@ -89,8 +200,13 @@ def test_handle_on_empty_config(self, *args):
             "ssh_pwauth=None\n",
             self.logs.getvalue(),
         )
+        self.assertEqual(
+            [mock.call(["systemctl", "status", "ssh"], capture=True)],
+            m_subp.call_args_list,
+        )
 
-    def test_handle_on_chpasswd_list_parses_common_hashes(self):
+    @mock.patch(MODPATH + "subp.subp")
+    def test_handle_on_chpasswd_list_parses_common_hashes(self, m_subp):
         """handle parses command password hashes."""
         cloud = self.tmp_cloud(distro="ubuntu")
         valid_hashed_pwds = [
@@ -136,6 +252,7 @@ def test_bsd_calls_custom_pw_cmds_to_set_and_expire_passwords(
                     logstring="chpasswd for ubuntu",
                 ),
                 mock.call(["pw", "usermod", "ubuntu", "-p", "01-Jan-1970"]),
+                mock.call(["systemctl", "status", "sshd"], capture=True),
             ],
             m_subp.call_args_list,
         )