Skip to content

Commit

Permalink
refactor: replace verbosity with log levels in logs.py (canonical#5414)
Browse files Browse the repository at this point in the history
  • Loading branch information
@TheRealFalcon @holmanb
TheRealFalcon authored and holmanb committed Aug 2, 2024
1 parent b6617ba commit db242a7
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 77 deletions.
116 changes: 59 additions & 57 deletions cloudinit/cmd/devel/logs.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@
"""Define 'collect-logs' utility and handler to include in cloud-init cmd."""

import argparse
import logging
import os
import pathlib
import shutil
import subprocess
import sys
from datetime import datetime, timezone
from pathlib import Path
from typing import NamedTuple, Optional
from typing import NamedTuple, Optional, cast

from cloudinit import log
from cloudinit.cmd.devel import read_cfg_paths
from cloudinit.stages import Init
from cloudinit.subp import ProcessExecutionError, subp
Expand All @@ -27,6 +30,8 @@
write_file,
)

LOG = cast(log.CustomLoggerType, logging.getLogger(__name__))


class LogPaths(NamedTuple):
userdata_raw: str
Expand Down Expand Up @@ -152,111 +157,103 @@ def _get_copytree_ignore_files(paths: LogPaths):
return ignored_files


def _write_command_output_to_file(cmd, filename, msg, verbosity):
def _write_command_output_to_file(cmd, filename, msg):
"""Helper which runs a command and writes output or error to filename."""
ensure_dir(os.path.dirname(filename))
try:
output = subp(cmd).stdout
except ProcessExecutionError as e:
write_file(filename, str(e))
_debug("collecting %s failed.\n" % msg, 1, verbosity)
LOG.debug("collecting %s failed.", msg)
else:
write_file(filename, output)
_debug("collected %s\n" % msg, 1, verbosity)
LOG.debug("collected %s", msg)
return output


def _stream_command_output_to_file(cmd, filename, msg, verbosity):
"""Helper which runs a command and writes output or error to filename."""
def _stream_command_output_to_file(cmd, filename, msg):
"""Helper which runs a command and writes output or error to filename.
`subprocess.call` is invoked directly here to stream output to the file.
Otherwise memory usage can be high for large outputs.
"""
ensure_dir(os.path.dirname(filename))
try:
with open(filename, "w") as f:
subprocess.call(cmd, stdout=f, stderr=f) # nosec B603
except OSError as e:
write_file(filename, str(e))
_debug("collecting %s failed.\n" % msg, 1, verbosity)
LOG.debug("collecting %s failed.", msg)
else:
_debug("collected %s\n" % msg, 1, verbosity)


def _debug(msg, level, verbosity):
if level <= verbosity:
sys.stderr.write(msg)
LOG.debug("collected %s", msg)


def _collect_file(path, out_dir, verbosity):
def _collect_file(path: str, out_dir: str) -> None:
if os.path.isfile(path):
copy(path, out_dir)
_debug("collected file: %s\n" % path, 1, verbosity)
LOG.debug("collected file: %s", path)
else:
_debug("file %s did not exist\n" % path, 2, verbosity)
LOG.trace("file %s did not exist", path)


def _collect_installer_logs(
log_dir: str, include_userdata: bool, verbosity: int
):
def _collect_installer_logs(log_dir: str, include_userdata: bool) -> None:
"""Obtain subiquity logs and config files."""
for src_file in INSTALLER_APPORT_FILES:
destination_dir = Path(log_dir + src_file.path).parent
if not destination_dir.exists():
ensure_dir(str(destination_dir))
_collect_file(src_file.path, str(destination_dir), verbosity)
_collect_file(src_file.path, str(destination_dir))
if include_userdata:
for src_file in INSTALLER_APPORT_SENSITIVE_FILES:
destination_dir = Path(log_dir + src_file.path).parent
if not destination_dir.exists():
ensure_dir(str(destination_dir))
_collect_file(src_file.path, str(destination_dir), verbosity)
_collect_file(src_file.path, str(destination_dir))


def _collect_version_info(log_dir: str, verbosity: int):
def _collect_version_info(log_dir: str) -> None:
version = _write_command_output_to_file(
cmd=["cloud-init", "--version"],
filename=os.path.join(log_dir, "version"),
msg="cloud-init --version",
verbosity=verbosity,
)
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,
)
if not version:
version = dpkg_ver if dpkg_ver else "not-available"
_debug("collected cloud-init version: %s\n" % version, 1, verbosity)
LOG.debug("collected cloud-init version: %s", version)


def _collect_system_logs(log_dir: str, verbosity: int):
def _collect_system_logs(log_dir: str) -> None:
_stream_command_output_to_file(
cmd=["dmesg"],
filename=os.path.join(log_dir, "dmesg.txt"),
msg="dmesg output",
verbosity=verbosity,
)
_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",
verbosity=verbosity,
)


def _collect_cloudinit_logs(
log_dir: str,
verbosity: int,
init: Init,
paths: LogPaths,
include_userdata: bool,
):
for log in get_config_logfiles(init.cfg):
_collect_file(log, log_dir, verbosity)
) -> None:
for logfile in get_config_logfiles(init.cfg):
_collect_file(logfile, log_dir)
if include_userdata:
user_data_file = paths.userdata_raw
_collect_file(user_data_file, log_dir, verbosity)
_collect_file(user_data_file, log_dir)


def _collect_run_dir(log_dir: str, verbosity: int, paths: LogPaths):
def _collect_run_dir(log_dir: str, paths: LogPaths) -> None:
run_dir = os.path.join(log_dir, "run")
ensure_dir(run_dir)
if os.path.exists(paths.run_dir):
Expand All @@ -267,15 +264,10 @@ def _collect_run_dir(log_dir: str, verbosity: int, paths: LogPaths):
ignore=lambda _, __: _get_copytree_ignore_files(paths),
)
except shutil.Error as e:
sys.stderr.write("Failed collecting file(s) due to error:\n")
sys.stderr.write(str(e) + "\n")
_debug("collected dir %s\n" % paths.run_dir, 1, verbosity)
LOG.warning("Failed collecting file(s) due to error: %s", e)
LOG.debug("collected directory: %s", paths.run_dir)
else:
_debug(
"directory '%s' did not exist\n" % paths.run_dir,
1,
verbosity,
)
LOG.debug("directory '%s' did not exist", paths.run_dir)
if os.path.exists(os.path.join(paths.run_dir, "disabled")):
# Fallback to grab previous cloud/data
cloud_data_dir = Path(paths.cloud_data)
Expand All @@ -286,19 +278,17 @@ def _collect_run_dir(log_dir: str, verbosity: int, paths: LogPaths):
)


def collect_logs(
tarfile: str, include_userdata: bool, verbosity: int = 0
) -> int:
def collect_logs(tarfile: str, include_userdata: bool) -> int:
"""Collect all cloud-init logs and tar them up into the provided tarfile.
@param tarfile: The path of the tar-gzipped file to create.
@param include_userdata: Boolean, true means include user-data.
@return: 0 on success, 1 on failure.
"""
if include_userdata and os.getuid() != 0:
sys.stderr.write(
"To include userdata, root user is required."
" Try sudo cloud-init collect-logs\n"
LOG.error(
"To include userdata, root user is required. "
"Try sudo cloud-init collect-logs"
)
return 1

Expand All @@ -312,25 +302,37 @@ def collect_logs(
init.read_cfg()
paths = get_log_paths(init)

_collect_version_info(log_dir, verbosity)
_collect_system_logs(log_dir, verbosity)
_collect_cloudinit_logs(
log_dir, verbosity, init, paths, include_userdata
)
_collect_installer_logs(log_dir, include_userdata, verbosity)
_collect_run_dir(log_dir, verbosity, paths)
_collect_version_info(log_dir)
_collect_system_logs(log_dir)
_collect_cloudinit_logs(log_dir, init, paths, include_userdata)
_collect_installer_logs(log_dir, include_userdata)
_collect_run_dir(log_dir, paths)
with chdir(tmp_dir):
subp(["tar", "czvf", tarfile, log_dir.replace(f"{tmp_dir}/", "")])
sys.stderr.write("Wrote %s\n" % tarfile)
LOG.info("Wrote %s", tarfile)
return 0


def _setup_logger(verbosity: int) -> None:
log.reset_logging()
if verbosity == 0:
level = logging.INFO
elif verbosity == 1:
level = logging.DEBUG
else:
level = log.TRACE
LOG.setLevel(level)
handler = logging.StreamHandler()
handler.setFormatter(logging.Formatter("%(message)s"))
LOG.addHandler(handler)


def handle_collect_logs_args(name, args):
"""Handle calls to 'cloud-init collect-logs' as a subcommand."""
_setup_logger(args.verbosity)
return collect_logs(
tarfile=args.tarfile,
include_userdata=args.userdata,
verbosity=args.verbosity,
)


Expand Down
27 changes: 7 additions & 20 deletions tests/unittests/cmd/devel/test_logs.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import os
import re
from datetime import datetime
from io import StringIO

import pytest

Expand All @@ -21,22 +20,19 @@
@mock.patch("cloudinit.cmd.devel.logs.os.getuid")
class TestCollectLogs:
def test_collect_logs_with_userdata_requires_root_user(
self, m_getuid, tmpdir
self, m_getuid, tmpdir, caplog
):
"""collect-logs errors when non-root user collects userdata ."""
m_getuid.return_value = 100 # non-root
output_tarfile = tmpdir.join("logs.tgz")
with mock.patch("sys.stderr", new_callable=StringIO) as m_stderr:
assert 1 == logs.collect_logs(
output_tarfile, include_userdata=True
)
assert 1 == logs.collect_logs(output_tarfile, include_userdata=True)
assert (
"To include userdata, root user is required."
" Try sudo cloud-init collect-logs\n" == m_stderr.getvalue()
" Try sudo cloud-init collect-logs" in caplog.text
)

def test_collect_logs_creates_tarfile(
self, m_getuid, m_log_paths, mocker, tmpdir
self, m_getuid, m_log_paths, mocker, tmpdir, caplog
):
"""collect-logs creates a tarfile with all related cloud-init info."""
m_getuid.return_value = 100
Expand Down Expand Up @@ -101,13 +97,10 @@ def fake_subprocess_call(cmd, stdout=None, stderr=None):
)
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 + "INSTALLER_APPORT_FILES", [])
mocker.patch(M_PATH + "INSTALLER_APPORT_SENSITIVE_FILES", [])
logs.collect_logs(output_tarfile, include_userdata=False)
Expand Down Expand Up @@ -151,10 +144,10 @@ def fake_subprocess_call(cmd, stdout=None, stderr=None):
assert "results" == load_text_file(
os.path.join(out_logdir, "run", "cloud-init", "results.json")
)
fake_stderr.write.assert_any_call("Wrote %s\n" % output_tarfile)
assert f"Wrote {output_tarfile}" in caplog.text

def test_collect_logs_includes_optional_userdata(
self, m_getuid, mocker, tmpdir, m_log_paths
self, m_getuid, mocker, tmpdir, m_log_paths, caplog
):
"""collect-logs include userdata when --include-userdata is set."""
m_getuid.return_value = 0
Expand Down Expand Up @@ -215,13 +208,10 @@ def fake_subprocess_call(cmd, stdout=None, stderr=None):
)
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 + "INSTALLER_APPORT_FILES", [])
mocker.patch(M_PATH + "INSTALLER_APPORT_SENSITIVE_FILES", [])
logs.collect_logs(output_tarfile, include_userdata=True)
Expand All @@ -239,7 +229,7 @@ def fake_subprocess_call(cmd, stdout=None, stderr=None):
m_log_paths.instance_data_sensitive.name,
)
)
fake_stderr.write.assert_any_call("Wrote %s\n" % output_tarfile)
assert f"Wrote {output_tarfile}" in caplog.text

@pytest.mark.parametrize(
"cmd, expected_file_contents, expected_return_value",
Expand Down Expand Up @@ -278,7 +268,6 @@ def test_write_command_output_to_file(
filename=output_file,
cmd=cmd,
msg="",
verbosity=1,
)

assert expected_return_value == return_output
Expand All @@ -301,7 +290,6 @@ def test_stream_command_output_to_file(
filename=output_file,
cmd=cmd,
msg="",
verbosity=1,
)

assert expected_file_contents == load_text_file(output_file)
Expand Down Expand Up @@ -382,7 +370,6 @@ def test_include_installer_logs_when_present(
logs._collect_installer_logs(
log_dir=tmpdir.strpath,
include_userdata=include_userdata,
verbosity=0,
)
expect_userdata = bool(include_userdata and apport_sensitive_files)
# when subiquity artifacts exist, and userdata set true, expect logs
Expand Down

0 comments on commit db242a7

Please sign in to comment.