logrotate config (#4721)

This feature adds support for log rotation (logrotate.d) for cloud-init logs located in /var/log.

Fixes GH-4509

cloudinit/cmd/devel/logs.py

@@ -17,11 +17,17 @@
 
 from cloudinit.cmd.devel import read_cfg_paths
 from cloudinit.helpers import Paths
+from cloudinit.stages import Init
 from cloudinit.subp import ProcessExecutionError, subp
 from cloudinit.temp_utils import tempdir
-from cloudinit.util import chdir, copy, ensure_dir, write_file
+from cloudinit.util import (
+    chdir,
+    copy,
+    ensure_dir,
+    get_config_logfiles,
+    write_file,
+)
 
-CLOUDINIT_LOGS = ["/var/log/cloud-init.log", "/var/log/cloud-init-output.log"]
 CLOUDINIT_RUN_DIR = "/run/cloud-init"
 
 
@@ -209,6 +215,8 @@ def collect_logs(tarfile, include_userdata: bool, verbosity=0):
             " Try sudo cloud-init collect-logs\n"
         )
         return 1
+
+    init = Init(ds_deps=[])
     tarfile = os.path.abspath(tarfile)
     log_dir = datetime.utcnow().date().strftime("cloud-init-logs-%Y-%m-%d")
     with tempdir(dir="/tmp") as tmp_dir:
@@ -242,7 +250,8 @@ def collect_logs(tarfile, include_userdata: bool, verbosity=0):
             verbosity=verbosity,
         )
 
-        for log in CLOUDINIT_LOGS:
+        init.read_cfg()
+        for log in get_config_logfiles(init.cfg):
             _collect_file(log, log_dir, verbosity)
         if include_userdata:
             user_data_file = _get_user_data_file()

cloudinit/util.py

@@ -1748,6 +1748,7 @@ def get_config_logfiles(cfg):
     @param cfg: The cloud-init merged configuration dictionary.
     """
     logs = []
+    rotated_logs = []
     if not cfg or not isinstance(cfg, dict):
         return logs
     default_log = cfg.get("def_log_file")
@@ -1765,7 +1766,16 @@ def get_config_logfiles(cfg):
             logs.append(target)
         elif ["tee", "-a"] == parts[:2]:
             logs.append(parts[2])
-    return list(set(logs))
+
+    # add rotated log files
+    for logfile in logs:
+        for rotated_logfile in glob.glob(f"{logfile}*"):
+            # Check that log file exists and is rotated.
+            # Do not add current one
+            if os.path.isfile(rotated_logfile) and rotated_logfile != logfile:
+                rotated_logs.append(rotated_logfile)
+
+    return list(set(logs + rotated_logs))
 
 
 def logexc(log, msg, *args):

packages/debian/cloud-init.logrotate

@@ -0,0 +1,10 @@
+/var/log/cloud-init.log
+/var/log/cloud-init-output.log
+{
+    missingok
+    nocreate
+    notifempty
+    rotate 5
+    compress
+    size 1M   
+}

tests/integration_tests/cmd/test_clean.py

@@ -4,7 +4,7 @@
 import pytest
 
 from tests.integration_tests.instances import IntegrationInstance
-from tests.integration_tests.releases import CURRENT_RELEASE
+from tests.integration_tests.releases import CURRENT_RELEASE, IS_UBUNTU
 
 USER_DATA = """\
 #cloud-config
@@ -26,11 +26,35 @@
   content: '#!/bin/sh\necho DID NOT RUN BECAUSE NO EXEC PERMS'
   permissions: '0644'
   owner: root:root
+packages:
+- logrotate
 """
 
 
 @pytest.mark.user_data(USER_DATA)
 class TestCleanCommand:
+    @pytest.mark.skipif(
+        not IS_UBUNTU, reason="Hasn't been tested on other distros"
+    )
+    def test_clean_rotated_logs(self, class_client: IntegrationInstance):
+        """Clean with log params alters expected files without error"""
+        assert class_client.execute("cloud-init status --wait").ok
+        assert class_client.execute(
+            "logrotate /etc/logrotate.d/cloud-init.logrotate"
+        ).ok
+        log_paths = (
+            "/var/log/cloud-init.log",
+            "/var/log/cloud-init.log.1.gz",
+            "/var/log/cloud-init-output.log",
+            "/var/log/cloud-init-output.log.1.gz",
+        )
+
+        assert class_client.execute("cloud-init clean --logs").ok
+        for path in log_paths:
+            assert class_client.execute(
+                f"test -f {path}"
+            ).failed, f"Unexpected file found {path}"
+
     def test_clean_by_param(self, class_client: IntegrationInstance):
         """Clean with various params alters expected files without error"""
         result = class_client.execute("cloud-init status --wait --long")

tests/unittests/cmd/devel/test_logs.py

@@ -40,8 +40,12 @@ def test_collect_logs_creates_tarfile(self, m_getuid, mocker, tmpdir):
         m_getuid.return_value = 100
         log1 = tmpdir.join("cloud-init.log")
         write_file(log1, "cloud-init-log")
+        log1_rotated = tmpdir.join("cloud-init.log.1.gz")
+        write_file(log1_rotated, "cloud-init-log-rotated")
         log2 = tmpdir.join("cloud-init-output.log")
         write_file(log2, "cloud-init-output-log")
+        log2_rotated = tmpdir.join("cloud-init-output.log.1.gz")
+        write_file(log2_rotated, "cloud-init-output-log-rotated")
         run_dir = tmpdir.join("run")
         write_file(run_dir.join("results.json"), "results")
         write_file(
@@ -52,6 +56,12 @@ def test_collect_logs_creates_tarfile(self, m_getuid, mocker, tmpdir):
         )
         output_tarfile = str(tmpdir.join("logs.tgz"))
 
+        mocker.patch(M_PATH + "Init", autospec=True)
+        mocker.patch(
+            M_PATH + "get_config_logfiles",
+            return_value=[log1, log1_rotated, log2, log2_rotated],
+        )
+
         date = datetime.utcnow().date().strftime("%Y-%m-%d")
         date_logdir = "cloud-init-logs-{0}".format(date)
 
@@ -98,7 +108,6 @@ def fake_subprocess_call(cmd, stdout=None, stderr=None):
             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)
         mocker.patch(M_PATH + "INSTALLER_APPORT_FILES", [])
         mocker.patch(M_PATH + "INSTALLER_APPORT_SENSITIVE_FILES", [])
@@ -123,9 +132,15 @@ def fake_subprocess_call(cmd, stdout=None, stderr=None):
         assert "cloud-init-log" == load_file(
             os.path.join(out_logdir, "cloud-init.log")
         )
+        assert "cloud-init-log-rotated" == load_file(
+            os.path.join(out_logdir, "cloud-init.log.1.gz")
+        )
         assert "cloud-init-output-log" == load_file(
             os.path.join(out_logdir, "cloud-init-output.log")
         )
+        assert "cloud-init-output-log-rotated" == load_file(
+            os.path.join(out_logdir, "cloud-init-output.log.1.gz")
+        )
         assert "dmesg-out\n" == load_file(
             os.path.join(out_logdir, "dmesg.txt")
         )
@@ -156,6 +171,12 @@ def test_collect_logs_includes_optional_userdata(
         )
         output_tarfile = str(tmpdir.join("logs.tgz"))
 
+        mocker.patch(M_PATH + "Init", autospec=True)
+        mocker.patch(
+            M_PATH + "get_config_logfiles",
+            return_value=[log1, log2],
+        )
+
         date = datetime.utcnow().date().strftime("%Y-%m-%d")
         date_logdir = "cloud-init-logs-{0}".format(date)
 
@@ -200,7 +221,6 @@ def fake_subprocess_call(cmd, stdout=None, stderr=None):
             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)
         mocker.patch(M_PATH + "INSTALLER_APPORT_FILES", [])
         mocker.patch(M_PATH + "INSTALLER_APPORT_SENSITIVE_FILES", [])

tests/unittests/test_util.py

@@ -2344,6 +2344,30 @@ def test_output_logs_parsed_when_appending(self):
             ),
         )
 
+    def test_output_logs_parsed_when_teeing_files_and_rotated(self):
+        """When output configuration is parsed when teeing files and rotated
+        log files are present."""
+        tmpd = self.tmp_dir()
+        log1 = self.tmp_path("my.log", tmpd)
+        log1_rotated = self.tmp_path("my.log.1.gz", tmpd)
+        log2 = self.tmp_path("himom.log", tmpd)
+        log2_rotated = self.tmp_path("himom.log.1.gz", tmpd)
+
+        util.write_file(log1_rotated, "hello")
+        util.write_file(log2_rotated, "hello")
+
+        self.assertEqual(
+            [log2, log2_rotated, log1, log1_rotated],
+            sorted(
+                util.get_config_logfiles(
+                    {
+                        "def_log_file": str(log1),
+                        "output": {"all": f"|tee -a {log2}"},
+                    }
+                )
+            ),
+        )
+
 
 class TestMultiLog(helpers.FilesystemMockingTestCase):
     def _createConsole(self, root):