Merge ~smoser/cloud-init:cleanup/collect-logs-stderr into cloud-init:master

Proposed by Scott Moser
Status: Merged
Approved by: Chad Smith
Approved revision: a6707315e674846bfff079520173d18d50fb0b79
Merge reported by: Chad Smith
Merged at revision: 9f5907e1a14e3a4890fa25e0b1910a902e098d58
Proposed branch: ~smoser/cloud-init:cleanup/collect-logs-stderr
Merge into: cloud-init:master
Diff against target: 205 lines (+66/-14)
2 files modified
cloudinit/cmd/devel/logs.py (+48/-11)
cloudinit/cmd/devel/tests/test_logs.py (+18/-3)
Reviewer Review Type Date Requested Status
Chad Smith Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+344894@code.launchpad.net

Commit message

collect-logs: add -v flag, write to stderr, limit journal to single boot.

With no output at all from collect-logs, users have been confused
on where the output is. By default now, write to stderr what that
file is.

Also
 * add '-v' to increase verbosity. With a single -v flag, mention
   what file/info is being collected.
 * limit the 'journalctl' collection to this boot (--boot=0).
   collecting entire journal seems unnecessary and can be huge.
 * do not fail when collecting files or directories that are not there.

LP: #1766335

Description of the change

see commit message

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:42590af381e9a4fd0e750733f6419d1abfdc18b6
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1083/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1083/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:5abc18c747aa0e52680c7830528ad42ea9f3fccb
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1084/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1084/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:0680529933ec324313d050695d3cac815661b7ce
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1086/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1086/rebuild

review: Approve (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) :
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:c821c5f783747394555977286791d4c94d85a8a3
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1088/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1088/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:a6707315e674846bfff079520173d18d50fb0b79
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1089/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1089/rebuild

review: Approve (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) wrote :

+1 LGTM!

review: Approve
Revision history for this message
Chad Smith (chad.smith) wrote :

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=9f5907e1

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/cloudinit/cmd/devel/logs.py b/cloudinit/cmd/devel/logs.py
index 35ca478..df72520 100644
--- a/cloudinit/cmd/devel/logs.py
+++ b/cloudinit/cmd/devel/logs.py
@@ -11,6 +11,7 @@ from cloudinit.temp_utils import tempdir
11from datetime import datetime11from datetime import datetime
12import os12import os
13import shutil13import shutil
14import sys
1415
1516
16CLOUDINIT_LOGS = ['/var/log/cloud-init.log', '/var/log/cloud-init-output.log']17CLOUDINIT_LOGS = ['/var/log/cloud-init.log', '/var/log/cloud-init-output.log']
@@ -31,6 +32,8 @@ def get_parser(parser=None):
31 parser = argparse.ArgumentParser(32 parser = argparse.ArgumentParser(
32 prog='collect-logs',33 prog='collect-logs',
33 description='Collect and tar all cloud-init debug info')34 description='Collect and tar all cloud-init debug info')
35 parser.add_argument('--verbose', '-v', action='count', default=0,
36 dest='verbosity', help="Be more verbose.")
34 parser.add_argument(37 parser.add_argument(
35 "--tarfile", '-t', default='cloud-init.tar.gz',38 "--tarfile", '-t', default='cloud-init.tar.gz',
36 help=('The tarfile to create containing all collected logs.'39 help=('The tarfile to create containing all collected logs.'
@@ -43,17 +46,33 @@ def get_parser(parser=None):
43 return parser46 return parser
4447
4548
46def _write_command_output_to_file(cmd, filename):49def _write_command_output_to_file(cmd, filename, msg, verbosity):
47 """Helper which runs a command and writes output or error to filename."""50 """Helper which runs a command and writes output or error to filename."""
48 try:51 try:
49 out, _ = subp(cmd)52 out, _ = subp(cmd)
50 except ProcessExecutionError as e:53 except ProcessExecutionError as e:
51 write_file(filename, str(e))54 write_file(filename, str(e))
55 _debug("collecting %s failed.\n" % msg, 1, verbosity)
52 else:56 else:
53 write_file(filename, out)57 write_file(filename, out)
58 _debug("collected %s\n" % msg, 1, verbosity)
59 return out
5460
5561
56def collect_logs(tarfile, include_userdata):62def _debug(msg, level, verbosity):
63 if level <= verbosity:
64 sys.stderr.write(msg)
65
66
67def _collect_file(path, out_dir, verbosity):
68 if os.path.isfile(path):
69 copy(path, out_dir)
70 _debug("collected file: %s\n" % path, 1, verbosity)
71 else:
72 _debug("file %s did not exist\n" % path, 2, verbosity)
73
74
75def collect_logs(tarfile, include_userdata, verbosity=0):
57 """Collect all cloud-init logs and tar them up into the provided tarfile.76 """Collect all cloud-init logs and tar them up into the provided tarfile.
5877
59 @param tarfile: The path of the tar-gzipped file to create.78 @param tarfile: The path of the tar-gzipped file to create.
@@ -64,28 +83,46 @@ def collect_logs(tarfile, include_userdata):
64 log_dir = 'cloud-init-logs-{0}'.format(date)83 log_dir = 'cloud-init-logs-{0}'.format(date)
65 with tempdir(dir='/tmp') as tmp_dir:84 with tempdir(dir='/tmp') as tmp_dir:
66 log_dir = os.path.join(tmp_dir, log_dir)85 log_dir = os.path.join(tmp_dir, log_dir)
67 _write_command_output_to_file(86 version = _write_command_output_to_file(
87 ['cloud-init', '--version'],
88 os.path.join(log_dir, 'version'),
89 "cloud-init --version", verbosity)
90 dpkg_ver = _write_command_output_to_file(
68 ['dpkg-query', '--show', "-f=${Version}\n", 'cloud-init'],91 ['dpkg-query', '--show', "-f=${Version}\n", 'cloud-init'],
69 os.path.join(log_dir, 'version'))92 os.path.join(log_dir, 'dpkg-version'),
93 "dpkg version", verbosity)
94 if not version:
95 version = dpkg_ver if dpkg_ver else "not-available"
96 _debug("collected cloud-init version: %s\n" % version, 1, verbosity)
70 _write_command_output_to_file(97 _write_command_output_to_file(
71 ['dmesg'], os.path.join(log_dir, 'dmesg.txt'))98 ['dmesg'], os.path.join(log_dir, 'dmesg.txt'),
99 "dmesg output", verbosity)
72 _write_command_output_to_file(100 _write_command_output_to_file(
73 ['journalctl', '-o', 'short-precise'],101 ['journalctl', '--boot=0', '-o', 'short-precise'],
74 os.path.join(log_dir, 'journal.txt'))102 os.path.join(log_dir, 'journal.txt'),
103 "systemd journal of current boot", verbosity)
104
75 for log in CLOUDINIT_LOGS:105 for log in CLOUDINIT_LOGS:
76 copy(log, log_dir)106 _collect_file(log, log_dir, verbosity)
77 if include_userdata:107 if include_userdata:
78 copy(USER_DATA_FILE, log_dir)108 _collect_file(USER_DATA_FILE, log_dir, verbosity)
79 run_dir = os.path.join(log_dir, 'run')109 run_dir = os.path.join(log_dir, 'run')
80 ensure_dir(run_dir)110 ensure_dir(run_dir)
81 shutil.copytree(CLOUDINIT_RUN_DIR, os.path.join(run_dir, 'cloud-init'))111 if os.path.exists(CLOUDINIT_RUN_DIR):
112 shutil.copytree(CLOUDINIT_RUN_DIR,
113 os.path.join(run_dir, 'cloud-init'))
114 _debug("collected dir %s\n" % CLOUDINIT_RUN_DIR, 1, verbosity)
115 else:
116 _debug("directory '%s' did not exist\n" % CLOUDINIT_RUN_DIR, 1,
117 verbosity)
82 with chdir(tmp_dir):118 with chdir(tmp_dir):
83 subp(['tar', 'czvf', tarfile, log_dir.replace(tmp_dir + '/', '')])119 subp(['tar', 'czvf', tarfile, log_dir.replace(tmp_dir + '/', '')])
120 sys.stderr.write("Wrote %s\n" % tarfile)
84121
85122
86def handle_collect_logs_args(name, args):123def handle_collect_logs_args(name, args):
87 """Handle calls to 'cloud-init collect-logs' as a subcommand."""124 """Handle calls to 'cloud-init collect-logs' as a subcommand."""
88 collect_logs(args.tarfile, args.userdata)125 collect_logs(args.tarfile, args.userdata, args.verbosity)
89126
90127
91def main():128def main():
diff --git a/cloudinit/cmd/devel/tests/test_logs.py b/cloudinit/cmd/devel/tests/test_logs.py
index dc4947c..98b4756 100644
--- a/cloudinit/cmd/devel/tests/test_logs.py
+++ b/cloudinit/cmd/devel/tests/test_logs.py
@@ -4,6 +4,7 @@ from cloudinit.cmd.devel import logs
4from cloudinit.util import ensure_dir, load_file, subp, write_file4from cloudinit.util import ensure_dir, load_file, subp, write_file
5from cloudinit.tests.helpers import FilesystemMockingTestCase, wrap_and_call5from cloudinit.tests.helpers import FilesystemMockingTestCase, wrap_and_call
6from datetime import datetime6from datetime import datetime
7import mock
7import os8import os
89
910
@@ -27,11 +28,13 @@ class TestCollectLogs(FilesystemMockingTestCase):
27 date = datetime.utcnow().date().strftime('%Y-%m-%d')28 date = datetime.utcnow().date().strftime('%Y-%m-%d')
28 date_logdir = 'cloud-init-logs-{0}'.format(date)29 date_logdir = 'cloud-init-logs-{0}'.format(date)
2930
31 version_out = '/usr/bin/cloud-init 18.2fake\n'
30 expected_subp = {32 expected_subp = {
31 ('dpkg-query', '--show', "-f=${Version}\n", 'cloud-init'):33 ('dpkg-query', '--show', "-f=${Version}\n", 'cloud-init'):
32 '0.7fake\n',34 '0.7fake\n',
35 ('cloud-init', '--version'): version_out,
33 ('dmesg',): 'dmesg-out\n',36 ('dmesg',): 'dmesg-out\n',
34 ('journalctl', '-o', 'short-precise'): 'journal-out\n',37 ('journalctl', '--boot=0', '-o', 'short-precise'): 'journal-out\n',
35 ('tar', 'czvf', output_tarfile, date_logdir): ''38 ('tar', 'czvf', output_tarfile, date_logdir): ''
36 }39 }
3740
@@ -44,9 +47,12 @@ class TestCollectLogs(FilesystemMockingTestCase):
44 subp(cmd) # Pass through tar cmd so we can check output47 subp(cmd) # Pass through tar cmd so we can check output
45 return expected_subp[cmd_tuple], ''48 return expected_subp[cmd_tuple], ''
4649
50 fake_stderr = mock.MagicMock()
51
47 wrap_and_call(52 wrap_and_call(
48 'cloudinit.cmd.devel.logs',53 'cloudinit.cmd.devel.logs',
49 {'subp': {'side_effect': fake_subp},54 {'subp': {'side_effect': fake_subp},
55 'sys.stderr': {'new': fake_stderr},
50 'CLOUDINIT_LOGS': {'new': [log1, log2]},56 'CLOUDINIT_LOGS': {'new': [log1, log2]},
51 'CLOUDINIT_RUN_DIR': {'new': self.run_dir}},57 'CLOUDINIT_RUN_DIR': {'new': self.run_dir}},
52 logs.collect_logs, output_tarfile, include_userdata=False)58 logs.collect_logs, output_tarfile, include_userdata=False)
@@ -55,7 +61,9 @@ class TestCollectLogs(FilesystemMockingTestCase):
55 out_logdir = self.tmp_path(date_logdir, self.new_root)61 out_logdir = self.tmp_path(date_logdir, self.new_root)
56 self.assertEqual(62 self.assertEqual(
57 '0.7fake\n',63 '0.7fake\n',
58 load_file(os.path.join(out_logdir, 'version')))64 load_file(os.path.join(out_logdir, 'dpkg-version')))
65 self.assertEqual(version_out,
66 load_file(os.path.join(out_logdir, 'version')))
59 self.assertEqual(67 self.assertEqual(
60 'cloud-init-log',68 'cloud-init-log',
61 load_file(os.path.join(out_logdir, 'cloud-init.log')))69 load_file(os.path.join(out_logdir, 'cloud-init.log')))
@@ -72,6 +80,7 @@ class TestCollectLogs(FilesystemMockingTestCase):
72 'results',80 'results',
73 load_file(81 load_file(
74 os.path.join(out_logdir, 'run', 'cloud-init', 'results.json')))82 os.path.join(out_logdir, 'run', 'cloud-init', 'results.json')))
83 fake_stderr.write.assert_any_call('Wrote %s\n' % output_tarfile)
7584
76 def test_collect_logs_includes_optional_userdata(self):85 def test_collect_logs_includes_optional_userdata(self):
77 """collect-logs include userdata when --include-userdata is set."""86 """collect-logs include userdata when --include-userdata is set."""
@@ -88,11 +97,13 @@ class TestCollectLogs(FilesystemMockingTestCase):
88 date = datetime.utcnow().date().strftime('%Y-%m-%d')97 date = datetime.utcnow().date().strftime('%Y-%m-%d')
89 date_logdir = 'cloud-init-logs-{0}'.format(date)98 date_logdir = 'cloud-init-logs-{0}'.format(date)
9099
100 version_out = '/usr/bin/cloud-init 18.2fake\n'
91 expected_subp = {101 expected_subp = {
92 ('dpkg-query', '--show', "-f=${Version}\n", 'cloud-init'):102 ('dpkg-query', '--show', "-f=${Version}\n", 'cloud-init'):
93 '0.7fake',103 '0.7fake',
104 ('cloud-init', '--version'): version_out,
94 ('dmesg',): 'dmesg-out\n',105 ('dmesg',): 'dmesg-out\n',
95 ('journalctl', '-o', 'short-precise'): 'journal-out\n',106 ('journalctl', '--boot=0', '-o', 'short-precise'): 'journal-out\n',
96 ('tar', 'czvf', output_tarfile, date_logdir): ''107 ('tar', 'czvf', output_tarfile, date_logdir): ''
97 }108 }
98109
@@ -105,9 +116,12 @@ class TestCollectLogs(FilesystemMockingTestCase):
105 subp(cmd) # Pass through tar cmd so we can check output116 subp(cmd) # Pass through tar cmd so we can check output
106 return expected_subp[cmd_tuple], ''117 return expected_subp[cmd_tuple], ''
107118
119 fake_stderr = mock.MagicMock()
120
108 wrap_and_call(121 wrap_and_call(
109 'cloudinit.cmd.devel.logs',122 'cloudinit.cmd.devel.logs',
110 {'subp': {'side_effect': fake_subp},123 {'subp': {'side_effect': fake_subp},
124 'sys.stderr': {'new': fake_stderr},
111 'CLOUDINIT_LOGS': {'new': [log1, log2]},125 'CLOUDINIT_LOGS': {'new': [log1, log2]},
112 'CLOUDINIT_RUN_DIR': {'new': self.run_dir},126 'CLOUDINIT_RUN_DIR': {'new': self.run_dir},
113 'USER_DATA_FILE': {'new': userdata}},127 'USER_DATA_FILE': {'new': userdata}},
@@ -118,3 +132,4 @@ class TestCollectLogs(FilesystemMockingTestCase):
118 self.assertEqual(132 self.assertEqual(
119 'user-data',133 'user-data',
120 load_file(os.path.join(out_logdir, 'user-data.txt')))134 load_file(os.path.join(out_logdir, 'user-data.txt')))
135 fake_stderr.write.assert_any_call('Wrote %s\n' % output_tarfile)

Subscribers

People subscribed via source and target branches