diff --git a/cloudinit/cmd/clean.py b/cloudinit/cmd/clean.py index 5666a4eab0f..81c81f19027 100755 --- a/cloudinit/cmd/clean.py +++ b/cloudinit/cmd/clean.py @@ -106,9 +106,10 @@ def get_parser(parser=None): return parser -def remove_artifacts(remove_logs, remove_seed=False, remove_config=None): +def remove_artifacts(init, remove_logs, remove_seed=False, remove_config=None): """Helper which removes artifacts dir and optionally log files. + @param: init: Init object to use @param: remove_logs: Boolean. Set True to delete the cloud_dir path. False preserves them. @param: remove_seed: Boolean. Set True to also delete seed subdir in @@ -117,7 +118,6 @@ def remove_artifacts(remove_logs, remove_seed=False, remove_config=None): Can be any of: all, network, ssh_config. @returns: 0 on success, 1 otherwise. """ - init = Init(ds_deps=[]) init.read_cfg() if remove_logs: for log_file in get_config_logfiles(init.cfg): @@ -158,8 +158,9 @@ def remove_artifacts(remove_logs, remove_seed=False, remove_config=None): def handle_clean_args(name, args): """Handle calls to 'cloud-init clean' as a subcommand.""" + init = Init(ds_deps=[]) exit_code = remove_artifacts( - args.remove_logs, args.remove_seed, args.remove_config + init, args.remove_logs, args.remove_seed, args.remove_config ) if args.machine_id: if uses_systemd(): @@ -169,7 +170,9 @@ def handle_clean_args(name, args): # Non-systemd like FreeBSD regen machine-id when file is absent del_file(ETC_MACHINE_ID) if exit_code == 0 and args.reboot: - cmd = ["shutdown", "-r", "now"] + cmd = init.distro.shutdown_command( + mode="reboot", delay="now", message=None + ) try: subp(cmd, capture=False) except ProcessExecutionError as e: diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index 8289048399f..d15519c8ef4 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -1128,9 +1128,10 @@ def create_group(self, name, members=None): subp.subp(["usermod", "-a", "-G", name, member]) LOG.info("Added user '%s' to group '%s'", member, name) - def shutdown_command(self, *, mode, delay, message): + @classmethod + def shutdown_command(cls, *, mode, delay, message): # called from cc_power_state_change.load_power_state - command = ["shutdown", self.shutdown_options_map[mode]] + command = ["shutdown", cls.shutdown_options_map[mode]] try: if delay != "now": delay = "+%d" % int(delay) diff --git a/tests/unittests/cmd/test_clean.py b/tests/unittests/cmd/test_clean.py index 6569214ba0d..5e1f9d46114 100644 --- a/tests/unittests/cmd/test_clean.py +++ b/tests/unittests/cmd/test_clean.py @@ -7,6 +7,8 @@ import cloudinit.settings from cloudinit.cmd import clean +from cloudinit.distros import Distro +from cloudinit.stages import Init from cloudinit.util import ensure_dir, sym_link, write_file from tests.unittests.helpers import mock, wrap_and_call @@ -30,21 +32,14 @@ def clean_paths(tmpdir): @pytest.fixture(scope="function") def init_class(clean_paths): - class FakeInit: - cfg = { - "def_log_file": clean_paths.log, - "output": {"all": f"|tee -a {clean_paths.output_log}"}, - } - # Ensure cloud_dir has a trailing slash, to match real behaviour - paths = MyPaths(cloud_dir=f"{clean_paths.cloud_dir}/") - - def __init__(self, ds_deps): - pass - - def read_cfg(self): - pass - - return FakeInit + init = mock.Mock(spec=Init) + init.paths = MyPaths(cloud_dir=f"{clean_paths.cloud_dir}/") + init.distro.shutdown_command = Distro.shutdown_command + init.cfg = { + "def_log_file": clean_paths.log, + "output": {"all": f"|tee -a {clean_paths.output_log}"}, + } + return init class TestClean: @@ -56,10 +51,8 @@ def test_remove_artifacts_removes_logs(self, clean_paths, init_class): assert ( os.path.exists(clean_paths.cloud_dir) is False ), "Unexpected cloud_dir" - retcode = wrap_and_call( - "cloudinit.cmd.clean", - {"Init": {"side_effect": init_class}}, - clean.remove_artifacts, + retcode = clean.remove_artifacts( + init=init_class, remove_logs=True, ) assert ( @@ -93,10 +86,8 @@ def test_remove_net_conf(self, clean_paths, init_class): "cloudinit.cmd.clean.GEN_NET_CONFIG_FILES", [f.strpath for f in TEST_GEN_NET_CONFIG_FILES], ): - retcode = wrap_and_call( - "cloudinit.cmd.clean", - {"Init": {"side_effect": init_class}}, - clean.remove_artifacts, + retcode = clean.remove_artifacts( + init_class, remove_logs=False, remove_config=["network"], ) @@ -130,10 +121,8 @@ def test_remove_ssh_conf(self, clean_paths, init_class): "cloudinit.cmd.clean.GEN_SSH_CONFIG_FILES", TEST_GEN_SSH_CONFIG_FILES, ): - retcode = wrap_and_call( - "cloudinit.cmd.clean", - {"Init": {"side_effect": init_class}}, - clean.remove_artifacts, + retcode = clean.remove_artifacts( + init_class, remove_logs=False, remove_config=["ssh_config"], ) @@ -170,10 +159,8 @@ def test_remove_all_conf(self, clean_paths, init_class): "cloudinit.cmd.clean.GEN_SSH_CONFIG_FILES", TEST_GEN_SSH_CONFIG_FILES, ): - retcode = wrap_and_call( - "cloudinit.cmd.clean", - {"Init": {"side_effect": init_class}}, - clean.remove_artifacts, + retcode = clean.remove_artifacts( + init_class, remove_logs=False, remove_config=["all"], ) @@ -200,10 +187,8 @@ def test_keep_net_conf(self, clean_paths, init_class): "cloudinit.cmd.clean.GEN_NET_CONFIG_FILES", TEST_GEN_NET_CONFIG_FILES, ): - retcode = wrap_and_call( - "cloudinit.cmd.clean", - {"Init": {"side_effect": init_class}}, - clean.remove_artifacts, + retcode = clean.remove_artifacts( + init_class, remove_logs=False, remove_config=[], ) @@ -225,12 +210,8 @@ def test_remove_artifacts_runparts_clean_d(self, clean_paths, init_class): with mock.patch.object( cloudinit.settings, "CLEAN_RUNPARTS_DIR", clean_paths.clean_dir ): - retcode = wrap_and_call( - "cloudinit.cmd.clean", - { - "Init": {"side_effect": init_class}, - }, - clean.remove_artifacts, + retcode = clean.remove_artifacts( + init_class, remove_logs=False, ) assert ( @@ -243,10 +224,8 @@ def test_remove_artifacts_preserves_logs(self, clean_paths, init_class): clean_paths.log.write("cloud-init-log") clean_paths.output_log.write("cloud-init-output-log") - retcode = wrap_and_call( - "cloudinit.cmd.clean", - {"Init": {"side_effect": init_class}}, - clean.remove_artifacts, + retcode = clean.remove_artifacts( + init_class, remove_logs=False, ) assert 0 == retcode @@ -269,10 +248,8 @@ def test_remove_artifacts_removes_unlinks_symlinks( with mock.patch.object( cloudinit.settings, "CLEAN_RUNPARTS_DIR", clean_paths.clean_dir ): - retcode = wrap_and_call( - "cloudinit.cmd.clean", - {"Init": {"side_effect": init_class}}, - clean.remove_artifacts, + retcode = clean.remove_artifacts( + init_class, remove_logs=False, ) assert 0 == retcode @@ -295,10 +272,8 @@ def test_remove_artifacts_removes_artifacts_skipping_seed( with mock.patch.object( cloudinit.settings, "CLEAN_RUNPARTS_DIR", clean_paths.clean_dir ): - retcode = wrap_and_call( - "cloudinit.cmd.clean", - {"Init": {"side_effect": init_class}}, - clean.remove_artifacts, + retcode = clean.remove_artifacts( + init_class, remove_logs=False, ) assert 0 == retcode @@ -323,10 +298,8 @@ def test_remove_artifacts_removes_artifacts_removes_seed( with mock.patch.object( cloudinit.settings, "CLEAN_RUNPARTS_DIR", clean_paths.clean_dir ): - retcode = wrap_and_call( - "cloudinit.cmd.clean", - {"Init": {"side_effect": init_class}}, - clean.remove_artifacts, + retcode = clean.remove_artifacts( + init_class, remove_logs=False, remove_seed=True, ) @@ -346,15 +319,13 @@ def test_remove_artifacts_returns_one_on_errors( ensure_dir(clean_paths.cloud_dir) ensure_dir(clean_paths.cloud_dir.join("dir1")) - retcode = wrap_and_call( - "cloudinit.cmd.clean", - { - "del_dir": {"side_effect": OSError("oops")}, - "Init": {"side_effect": init_class}, - }, - clean.remove_artifacts, - remove_logs=False, - ) + with mock.patch( + "cloudinit.cmd.clean.del_dir", side_effect=OSError("oops") + ): + retcode = clean.remove_artifacts( + init_class, + remove_logs=False, + ) assert 1 == retcode _out, err = capsys.readouterr() assert ( @@ -368,7 +339,7 @@ def test_handle_clean_args_reboots(self, init_class): called_cmds = [] def fake_subp(cmd, capture): - called_cmds.append((cmd, capture)) + called_cmds.append(cmd) return "", "" myargs = namedtuple( @@ -385,14 +356,14 @@ def fake_subp(cmd, capture): "cloudinit.cmd.clean", { "subp": {"side_effect": fake_subp}, - "Init": {"side_effect": init_class}, + "Init": {"return_value": init_class}, }, clean.handle_clean_args, name="does not matter", args=cmdargs, ) assert 0 == retcode - assert [(["shutdown", "-r", "now"], False)] == called_cmds + assert [["shutdown", "-r", "now"]] == called_cmds @pytest.mark.parametrize( "machine_id,systemd_val", @@ -428,16 +399,13 @@ def test_handle_clean_args_removed_machine_id( with mock.patch.object( cloudinit.cmd.clean, "ETC_MACHINE_ID", machine_id_path.strpath ): - retcode = wrap_and_call( - "cloudinit.cmd.clean", - { - "Init": {"side_effect": init_class}, - }, - clean.handle_clean_args, - name="does not matter", - args=cmdargs, - ) - assert 0 == retcode + with mock.patch( + "cloudinit.cmd.clean.Init", return_value=init_class + ): + assert 0 == clean.handle_clean_args( + name="does not matter", + args=cmdargs, + ) if systemd_val: if machine_id: assert "uninitialized\n" == machine_id_path.read() @@ -453,7 +421,7 @@ def test_status_main(self, clean_paths, init_class): wrap_and_call( "cloudinit.cmd.clean", { - "Init": {"side_effect": init_class}, + "Init": {"return_value": init_class}, "sys.argv": {"new": ["clean", "--logs"]}, }, clean.main,