diff --git a/cloudinit/cmd/devel/logs.py b/cloudinit/cmd/devel/logs.py index de093b5f3c53..9fb0e3d6d342 100755 --- a/cloudinit/cmd/devel/logs.py +++ b/cloudinit/cmd/devel/logs.py @@ -9,6 +9,7 @@ import argparse import os import shutil +import subprocess import sys from datetime import datetime from pathlib import Path @@ -19,7 +20,6 @@ from cloudinit.subp import ProcessExecutionError, subp from cloudinit.temp_utils import tempdir from cloudinit.util import chdir, copy, ensure_dir, write_file -import subprocess CLOUDINIT_LOGS = ["/var/log/cloud-init.log", "/var/log/cloud-init-output.log"] CLOUDINIT_RUN_DIR = "/run/cloud-init" @@ -142,19 +142,17 @@ def _copytree_rundir_ignore_files(curdir, files): return ignored_files -def _write_command_output_to_file(cmd, filename, msg, verbosity, return_output=False): +def _write_command_output_to_file(cmd, filename, msg, verbosity): """Helper which runs a command and writes output or error to filename.""" - output = None + output = "" try: ensure_dir(os.path.dirname(filename)) - with open(filename, "w") as f: - if return_output: - output = subp(cmd)[0] - f.write(output) - else: - f.write(subp(cmd)[0]) + output = subp(cmd)[0] + # with open(filename, "w") as f: + # f.write(output) + write_file(filename, output) except ProcessExecutionError as e: - write_file(filename, output:=str(e)) + write_file(filename, output := str(e)) _debug("collecting %s failed.\n" % msg, 1, verbosity) return output else: @@ -162,6 +160,19 @@ def _write_command_output_to_file(cmd, filename, msg, verbosity, return_output=F return output +def _stream_command_output_to_file(cmd, filename, msg, verbosity): + """Helper which runs a command and writes output or error to filename.""" + try: + ensure_dir(os.path.dirname(filename)) + with open(filename, "w") as f: + subprocess.call(cmd, stdout=f, stderr=f) + except ProcessExecutionError as e: + # write_file(filename, str(e)) + _debug("collecting %s failed.\n" % msg, 1, verbosity) + else: + _debug("collected %s\n" % msg, 1, verbosity) + + def _debug(msg, level, verbosity): if level <= verbosity: sys.stderr.write(msg) @@ -212,26 +223,24 @@ def collect_logs(tarfile, include_userdata: bool, verbosity=0): filename=os.path.join(log_dir, "version"), msg="cloud-init --version", verbosity=verbosity, - return_output=True, ) dpkg_ver = _write_command_output_to_file( cmd=["dpkg-query", "--show", "-f=${Version}\n", "cloud-init"], filename=os.path.join(log_dir, "dpkg-version"), msg="dpkg version", verbosity=verbosity, - return_output=True, ) if not version: version = dpkg_ver if dpkg_ver else "not-available" print("version: ", version) _debug("collected cloud-init version: %s\n" % version, 1, verbosity) - _write_command_output_to_file( + _stream_command_output_to_file( cmd=["dmesg"], filename=os.path.join(log_dir, "dmesg.txt"), msg="dmesg output", verbosity=verbosity, ) - _write_command_output_to_file( + _stream_command_output_to_file( cmd=["journalctl", "--boot=0", "-o", "short-precise"], filename=os.path.join(log_dir, "journal.txt"), msg="systemd journal of current boot", diff --git a/tests/unittests/cmd/devel/test_logs.py b/tests/unittests/cmd/devel/test_logs.py index 508eb4f43e22..bee6ea7cbc66 100644 --- a/tests/unittests/cmd/devel/test_logs.py +++ b/tests/unittests/cmd/devel/test_logs.py @@ -79,9 +79,24 @@ def fake_subp(cmd): subp(cmd) # Pass through tar cmd so we can check output return expected_subp[cmd_tuple], "" + # the new _stream_command_output_to_file function uses subprocess.call + # instead of subp, so we need to mock that as well + def fake_subprocess_call(cmd, stdout=None, stderr=None): + cmd_tuple = tuple(cmd) + if cmd_tuple not in expected_subp: + raise AssertionError( + "Unexpected command provided to fake subprocess.call(): {0}".format( + cmd + ) + ) + stdout.write(expected_subp[cmd_tuple]) + fake_stderr = mock.MagicMock() mocker.patch(M_PATH + "subp", side_effect=fake_subp) + mocker.patch( + M_PATH + "subprocess.call", side_effect=fake_subprocess_call + ) mocker.patch(M_PATH + "sys.stderr", fake_stderr) mocker.patch(M_PATH + "CLOUDINIT_LOGS", [log1, log2]) mocker.patch(M_PATH + "CLOUDINIT_RUN_DIR", run_dir) @@ -194,7 +209,6 @@ def fake_subp(cmd): ) fake_stderr.write.assert_any_call("Wrote %s\n" % output_tarfile) - def test_write_command_output_to_file(self, m_getuid, tmpdir): # what does this do??? test_str_1 = "test #1" @@ -208,38 +222,35 @@ def test_write_command_output_to_file(self, m_getuid, tmpdir): cmd=["echo", test_str_1], msg="", verbosity=1, - return_output=True, ) - return_output2 = logs._write_command_output_to_file( + logs._stream_command_output_to_file( filename=output_file2, cmd=["echo", test_str_2], msg="", verbosity=1, - return_output=False, - ) - return_output3 = logs._write_command_output_to_file( - filename=output_file3, - cmd=["ls", dir:="/this-directory-does-not-exist"], - msg="", - verbosity=1, - return_output=True, ) + # return_output3 = logs._write_command_output_to_file( + # filename=output_file3, + # cmd=["ls", dir:="/this-directory-does-not-exist"], + # msg="", + # verbosity=1, + # ) assert test_str_1 + "\n" == return_output1 assert test_str_1 + "\n" == load_file(output_file1) # no output should have been returned so this value should be None - assert None == return_output2 + # assert None == return_output2 assert test_str_2 + "\n" == load_file(output_file2) - expected_err_msg = "Unexpected error while running command.\n" + \ - f"Command: ['ls', '{dir}']\n" + \ - "Exit code: 2\n" + \ - "Reason: -\n" + \ - "Stdout: \n" + \ - f"Stderr: ls: cannot access '{dir}': No such file or directory" - assert expected_err_msg == return_output3 - assert expected_err_msg == load_file(output_file3) + # expected_err_msg = "Unexpected error while running command.\n" + \ + # f"Command: ['ls', '{dir}']\n" + \ + # "Exit code: 2\n" + \ + # "Reason: -\n" + \ + # "Stdout: \n" + \ + # f"Stderr: ls: cannot access '{dir}': No such file or directory" + # assert expected_err_msg == return_output3 + # assert expected_err_msg == load_file(output_file3) # def test_write_command_output_to_file_with_return_output(self, m_getuid, tmpdir): # # what does this do??? @@ -252,10 +263,9 @@ def test_write_command_output_to_file(self, m_getuid, tmpdir): # verbosity=0, # return_output=True, # ) - + # assert "test\n" == return_output # assert "test\n" == load_file(output_file) - # def test_write_command_output_to_file_without_return_output(self, m_getuid, tmpdir): # # what does this do???