Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not hardcode reboot command #5208

Merged
merged 2 commits into from
May 1, 2024
Merged

Do not hardcode reboot command #5208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b6d6383
fix(cmd): Do not hardcode reboot command
holmanb Apr 24, 2024
3d8c176
comments
holmanb Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions cloudinit/cmd/clean.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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():
Expand All @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions cloudinit/distros/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
128 changes: 48 additions & 80 deletions tests/unittests/cmd/test_clean.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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:
Expand All @@ -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 (
Expand Down Expand Up @@ -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"],
)
Expand Down Expand Up @@ -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"],
)
Expand Down Expand Up @@ -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"],
)
Expand 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=[],
)
Expand All @@ -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 (
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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,
)
Expand All @@ -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 (
Expand All @@ -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(
Expand All @@ -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",
Expand Down Expand Up @@ -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()
Expand All @@ -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,
Expand Down
Loading