create a shutdown_command method in distro classes (#567)

Under FreeBSD, we want to use "shutdown -p" for poweroff.

Alpine Linux also has some specificities.

We choose to define a method that returns the shutdown command line to
use, rather than a method that actually does the shutdown. This makes it
easier to have the tests in test_handler_power_state do their
verifications.

Two tests are added for the special behaviours that are known so far.

6 files changed, 102 insertions, 62 deletions

diff --git a/cloudinit/config/cc_power_state_change.py b/cloudinit/config/cc_power_state_change.py
index 6fcb8a7d..b0cfafcd 100644
--- a/cloudinit/config/cc_power_state_change.py
+++ b/cloudinit/config/cc_power_state_change.py
@@ -117,7 +117,7 @@ def check_condition(cond, log=None):
 
 def handle(_name, cfg, cloud, log, _args):
     try:
-        (args, timeout, condition) = load_power_state(cfg, cloud.distro.name)
+        (args, timeout, condition) = load_power_state(cfg, cloud.distro)
         if args is None:
             log.debug("no power_state provided. doing nothing")
             return
@@ -144,19 +144,7 @@ def handle(_name, cfg, cloud, log, _args):
                  condition, execmd, [args, devnull_fp])
 
 
-def convert_delay(delay, fmt=None, scale=None):
-    if not fmt:
-        fmt = "+%s"
-    if not scale:
-        scale = 1
-
-    if delay != "now":
-        delay = fmt % int(int(delay) * int(scale))
-
-    return delay
-
-
-def load_power_state(cfg, distro_name):
+def load_power_state(cfg, distro):
     # returns a tuple of shutdown_command, timeout
     # shutdown_command is None if no config found
     pstate = cfg.get('power_state')
@@ -167,44 +155,16 @@ def load_power_state(cfg, distro_name):
     if not isinstance(pstate, dict):
         raise TypeError("power_state is not a dict.")
 
-    opt_map = {'halt': '-H', 'poweroff': '-P', 'reboot': '-r'}
-
+    modes_ok = ['halt', 'poweroff', 'reboot']
     mode = pstate.get("mode")
-    if mode not in opt_map:
+    if mode not in distro.shutdown_options_map:
         raise TypeError(
             "power_state[mode] required, must be one of: %s. found: '%s'." %
-            (','.join(opt_map.keys()), mode))
-
-    delay = pstate.get("delay", "now")
-    message = pstate.get("message")
-    scale = 1
-    fmt = "+%s"
-    command = ["shutdown", opt_map[mode]]
-
-    if distro_name == 'alpine':
-        # Convert integer 30 or string '30' to '1800' (seconds) as Alpine's
-        # halt/poweroff/reboot commands take seconds rather than minutes.
-        scale = 60
-        # No "+" in front of delay value as not supported by Alpine's commands.
-        fmt = "%s"
-        if delay == "now":
-            # Alpine's commands do not understand "now".
-            delay = "0"
-        command = [mode, "-d"]
-        # Alpine's commands don't support a message.
-        message = None
-
-    try:
-        delay = convert_delay(delay, fmt=fmt, scale=scale)
-    except ValueError as e:
-        raise TypeError(
-            "power_state[delay] must be 'now' or '+m' (minutes)."
-            " found '%s'." % delay
-        ) from e
+            (','.join(modes_ok), mode))
 
-    args = command + [delay]
-    if message:
-        args.append(message)
+    args = distro.shutdown_command(mode=mode,
+                                   delay=pstate.get("delay", "now"),
+                                   message=pstate.get("message"))
 
     try:
         timeout = float(pstate.get('timeout', 30.0))
diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index 2bd9bae8..fac8cf67 100755
--- a/cloudinit/distros/__init__.py
+++ b/cloudinit/distros/__init__.py
@@ -73,6 +73,9 @@ class Distro(metaclass=abc.ABCMeta):
     renderer_configs = {}
     _preferred_ntp_clients = None
     networking_cls = LinuxNetworking
+    # This is used by self.shutdown_command(), and can be overridden in
+    # subclasses
+    shutdown_options_map = {'halt': '-H', 'poweroff': '-P', 'reboot': '-r'}
 
     def __init__(self, name, cfg, paths):
         self._paths = paths
@@ -750,6 +753,22 @@ class Distro(metaclass=abc.ABCMeta):
                 subp.subp(['usermod', '-a', '-G', name, member])
                 LOG.info("Added user '%s' to group '%s'", member, name)
 
+    def shutdown_command(self, *, mode, delay, message):
+        # called from cc_power_state_change.load_power_state
+        command = ["shutdown", self.shutdown_options_map[mode]]
+        try:
+            if delay != "now":
+                delay = "+%d" % int(delay)
+        except ValueError as e:
+            raise TypeError(
+                "power_state[delay] must be 'now' or '+m' (minutes)."
+                " found '%s'." % (delay,)
+            ) from e
+        args = command + [delay]
+        if message:
+            args.append(message)
+        return args
+
 
 def _apply_hostname_transformations_to_url(url: str, transformations: list):
     """
diff --git a/cloudinit/distros/alpine.py b/cloudinit/distros/alpine.py
index e42443fc..e92ff3fb 100644
--- a/cloudinit/distros/alpine.py
+++ b/cloudinit/distros/alpine.py
@@ -162,4 +162,30 @@ class Distro(distros.Distro):
 
         return self._preferred_ntp_clients
 
+    def shutdown_command(self, mode='poweroff', delay='now', message=None):
+        # called from cc_power_state_change.load_power_state
+        # Alpine has halt/poweroff/reboot, with the following specifics:
+        # - we use them rather than the generic "shutdown"
+        # - delay is given with "-d [integer]"
+        # - the integer is in seconds, cannot be "now", and takes no "+"
+        # - no message is supported (argument ignored, here)
+
+        command = [mode, "-d"]
+
+        # Convert delay from minutes to seconds, as Alpine's
+        # halt/poweroff/reboot commands take seconds rather than minutes.
+        if delay == "now":
+            # Alpine's commands do not understand "now".
+            command += ['0']
+        else:
+            try:
+                command.append(str(int(delay) * 60))
+            except ValueError as e:
+                raise TypeError(
+                    "power_state[delay] must be 'now' or '+m' (minutes)."
+                    " found '%s'." % (delay,)
+                ) from e
+
+        return command
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/distros/bsd.py b/cloudinit/distros/bsd.py
index 2ed7a7d5..f717a667 100644
--- a/cloudinit/distros/bsd.py
+++ b/cloudinit/distros/bsd.py
@@ -17,6 +17,10 @@ class BSD(distros.Distro):
     hostname_conf_fn = '/etc/rc.conf'
     rc_conf_fn = "/etc/rc.conf"
 
+    # This differs from the parent Distro class, which has -P for
+    # poweroff.
+    shutdown_options_map = {'halt': '-H', 'poweroff': '-p', 'reboot': '-r'}
+
     # Set in BSD distro subclasses
     group_add_cmd_prefix = []
     pkg_cmd_install_prefix = []
diff --git a/tests/unittests/test_handler/test_handler_power_state.py b/tests/unittests/test_handler/test_handler_power_state.py
index 93b24fdc..4ac49424 100644
--- a/tests/unittests/test_handler/test_handler_power_state.py
+++ b/tests/unittests/test_handler/test_handler_power_state.py
@@ -4,72 +4,102 @@ import sys
 
 from cloudinit.config import cc_power_state_change as psc
 
+from cloudinit import distros
+from cloudinit import helpers
+
 from cloudinit.tests import helpers as t_help
 from cloudinit.tests.helpers import mock
 
 
 class TestLoadPowerState(t_help.TestCase):
+    def setUp(self):
+        super(TestLoadPowerState, self).setUp()
+        cls = distros.fetch('ubuntu')
+        paths = helpers.Paths({})
+        self.dist = cls('ubuntu', {}, paths)
+
     def test_no_config(self):
         # completely empty config should mean do nothing
-        (cmd, _timeout, _condition) = psc.load_power_state({}, 'ubuntu')
+        (cmd, _timeout, _condition) = psc.load_power_state({}, self.dist)
         self.assertIsNone(cmd)
 
     def test_irrelevant_config(self):
         # no power_state field in config should return None for cmd
         (cmd, _timeout, _condition) = psc.load_power_state({'foo': 'bar'},
-                                                           'ubuntu')
+                                                           self.dist)
         self.assertIsNone(cmd)
 
     def test_invalid_mode(self):
+
         cfg = {'power_state': {'mode': 'gibberish'}}
-        self.assertRaises(TypeError, psc.load_power_state, cfg, 'ubuntu')
+        self.assertRaises(TypeError, psc.load_power_state, cfg, self.dist)
 
         cfg = {'power_state': {'mode': ''}}
-        self.assertRaises(TypeError, psc.load_power_state, cfg, 'ubuntu')
+        self.assertRaises(TypeError, psc.load_power_state, cfg, self.dist)
 
     def test_empty_mode(self):
         cfg = {'power_state': {'message': 'goodbye'}}
-        self.assertRaises(TypeError, psc.load_power_state, cfg, 'ubuntu')
+        self.assertRaises(TypeError, psc.load_power_state, cfg, self.dist)
 
     def test_valid_modes(self):
         cfg = {'power_state': {}}
         for mode in ('halt', 'poweroff', 'reboot'):
             cfg['power_state']['mode'] = mode
-            check_lps_ret(psc.load_power_state(cfg, 'ubuntu'), mode=mode)
+            check_lps_ret(psc.load_power_state(cfg, self.dist), mode=mode)
 
     def test_invalid_delay(self):
         cfg = {'power_state': {'mode': 'poweroff', 'delay': 'goodbye'}}
-        self.assertRaises(TypeError, psc.load_power_state, cfg, 'ubuntu')
+        self.assertRaises(TypeError, psc.load_power_state, cfg, self.dist)
 
     def test_valid_delay(self):
         cfg = {'power_state': {'mode': 'poweroff', 'delay': ''}}
         for delay in ("now", "+1", "+30"):
             cfg['power_state']['delay'] = delay
-            check_lps_ret(psc.load_power_state(cfg, 'ubuntu'))
+            check_lps_ret(psc.load_power_state(cfg, self.dist))
 
     def test_message_present(self):
         cfg = {'power_state': {'mode': 'poweroff', 'message': 'GOODBYE'}}
-        ret = psc.load_power_state(cfg, 'ubuntu')
-        check_lps_ret(psc.load_power_state(cfg, 'ubuntu'))
+        ret = psc.load_power_state(cfg, self.dist)
+        check_lps_ret(psc.load_power_state(cfg, self.dist))
         self.assertIn(cfg['power_state']['message'], ret[0])
 
     def test_no_message(self):
         # if message is not present, then no argument should be passed for it
         cfg = {'power_state': {'mode': 'poweroff'}}
-        (cmd, _timeout, _condition) = psc.load_power_state(cfg, 'ubuntu')
+        (cmd, _timeout, _condition) = psc.load_power_state(cfg, self.dist)
         self.assertNotIn("", cmd)
-        check_lps_ret(psc.load_power_state(cfg, 'ubuntu'))
+        check_lps_ret(psc.load_power_state(cfg, self.dist))
         self.assertTrue(len(cmd) == 3)
 
     def test_condition_null_raises(self):
         cfg = {'power_state': {'mode': 'poweroff', 'condition': None}}
-        self.assertRaises(TypeError, psc.load_power_state, cfg, 'ubuntu')
+        self.assertRaises(TypeError, psc.load_power_state, cfg, self.dist)
 
     def test_condition_default_is_true(self):
         cfg = {'power_state': {'mode': 'poweroff'}}
-        _cmd, _timeout, cond = psc.load_power_state(cfg, 'ubuntu')
+        _cmd, _timeout, cond = psc.load_power_state(cfg, self.dist)
         self.assertEqual(cond, True)
 
+    def test_freebsd_poweroff_uses_lowercase_p(self):
+        cls = distros.fetch('freebsd')
+        paths = helpers.Paths({})
+        freebsd = cls('freebsd', {}, paths)
+        cfg = {'power_state': {'mode': 'poweroff'}}
+        ret = psc.load_power_state(cfg, freebsd)
+        self.assertIn('-p', ret[0])
+
+    def test_alpine_delay(self):
+        # alpine takes delay in seconds.
+        cls = distros.fetch('alpine')
+        paths = helpers.Paths({})
+        alpine = cls('alpine', {}, paths)
+        cfg = {'power_state': {'mode': 'poweroff', 'delay': ''}}
+        for delay, value in (('now', 0), ("+1", 60), ("+30", 1800)):
+            cfg['power_state']['delay'] = delay
+            ret = psc.load_power_state(cfg, alpine)
+            self.assertEqual('-d', ret[0][1])
+            self.assertEqual(str(value), ret[0][2])
+
 
 class TestCheckCondition(t_help.TestCase):
     def cmd_with_exit(self, rc):
diff --git a/tools/.github-cla-signers b/tools/.github-cla-signers
index aa946511..f01e9b66 100644
--- a/tools/.github-cla-signers
+++ b/tools/.github-cla-signers
@@ -6,6 +6,7 @@ candlerb
 dermotbradley
 dhensby
 eandersson
+emmanuelthome
 izzyleung
 johnsonshi
 jqueuniet