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
1diff --git a/cloudinit/cmd/devel/logs.py b/cloudinit/cmd/devel/logs.py
2index 35ca478..df72520 100644
3--- a/cloudinit/cmd/devel/logs.py
4+++ b/cloudinit/cmd/devel/logs.py
5@@ -11,6 +11,7 @@ from cloudinit.temp_utils import tempdir
6 from datetime import datetime
7 import os
8 import shutil
9+import sys
10
11
12 CLOUDINIT_LOGS = ['/var/log/cloud-init.log', '/var/log/cloud-init-output.log']
13@@ -31,6 +32,8 @@ def get_parser(parser=None):
14 parser = argparse.ArgumentParser(
15 prog='collect-logs',
16 description='Collect and tar all cloud-init debug info')
17+ parser.add_argument('--verbose', '-v', action='count', default=0,
18+ dest='verbosity', help="Be more verbose.")
19 parser.add_argument(
20 "--tarfile", '-t', default='cloud-init.tar.gz',
21 help=('The tarfile to create containing all collected logs.'
22@@ -43,17 +46,33 @@ def get_parser(parser=None):
23 return parser
24
25
26-def _write_command_output_to_file(cmd, filename):
27+def _write_command_output_to_file(cmd, filename, msg, verbosity):
28 """Helper which runs a command and writes output or error to filename."""
29 try:
30 out, _ = subp(cmd)
31 except ProcessExecutionError as e:
32 write_file(filename, str(e))
33+ _debug("collecting %s failed.\n" % msg, 1, verbosity)
34 else:
35 write_file(filename, out)
36+ _debug("collected %s\n" % msg, 1, verbosity)
37+ return out
38
39
40-def collect_logs(tarfile, include_userdata):
41+def _debug(msg, level, verbosity):
42+ if level <= verbosity:
43+ sys.stderr.write(msg)
44+
45+
46+def _collect_file(path, out_dir, verbosity):
47+ if os.path.isfile(path):
48+ copy(path, out_dir)
49+ _debug("collected file: %s\n" % path, 1, verbosity)
50+ else:
51+ _debug("file %s did not exist\n" % path, 2, verbosity)
52+
53+
54+def collect_logs(tarfile, include_userdata, verbosity=0):
55 """Collect all cloud-init logs and tar them up into the provided tarfile.
56
57 @param tarfile: The path of the tar-gzipped file to create.
58@@ -64,28 +83,46 @@ def collect_logs(tarfile, include_userdata):
59 log_dir = 'cloud-init-logs-{0}'.format(date)
60 with tempdir(dir='/tmp') as tmp_dir:
61 log_dir = os.path.join(tmp_dir, log_dir)
62- _write_command_output_to_file(
63+ version = _write_command_output_to_file(
64+ ['cloud-init', '--version'],
65+ os.path.join(log_dir, 'version'),
66+ "cloud-init --version", verbosity)
67+ dpkg_ver = _write_command_output_to_file(
68 ['dpkg-query', '--show', "-f=${Version}\n", 'cloud-init'],
69- os.path.join(log_dir, 'version'))
70+ os.path.join(log_dir, 'dpkg-version'),
71+ "dpkg version", verbosity)
72+ if not version:
73+ version = dpkg_ver if dpkg_ver else "not-available"
74+ _debug("collected cloud-init version: %s\n" % version, 1, verbosity)
75 _write_command_output_to_file(
76- ['dmesg'], os.path.join(log_dir, 'dmesg.txt'))
77+ ['dmesg'], os.path.join(log_dir, 'dmesg.txt'),
78+ "dmesg output", verbosity)
79 _write_command_output_to_file(
80- ['journalctl', '-o', 'short-precise'],
81- os.path.join(log_dir, 'journal.txt'))
82+ ['journalctl', '--boot=0', '-o', 'short-precise'],
83+ os.path.join(log_dir, 'journal.txt'),
84+ "systemd journal of current boot", verbosity)
85+
86 for log in CLOUDINIT_LOGS:
87- copy(log, log_dir)
88+ _collect_file(log, log_dir, verbosity)
89 if include_userdata:
90- copy(USER_DATA_FILE, log_dir)
91+ _collect_file(USER_DATA_FILE, log_dir, verbosity)
92 run_dir = os.path.join(log_dir, 'run')
93 ensure_dir(run_dir)
94- shutil.copytree(CLOUDINIT_RUN_DIR, os.path.join(run_dir, 'cloud-init'))
95+ if os.path.exists(CLOUDINIT_RUN_DIR):
96+ shutil.copytree(CLOUDINIT_RUN_DIR,
97+ os.path.join(run_dir, 'cloud-init'))
98+ _debug("collected dir %s\n" % CLOUDINIT_RUN_DIR, 1, verbosity)
99+ else:
100+ _debug("directory '%s' did not exist\n" % CLOUDINIT_RUN_DIR, 1,
101+ verbosity)
102 with chdir(tmp_dir):
103 subp(['tar', 'czvf', tarfile, log_dir.replace(tmp_dir + '/', '')])
104+ sys.stderr.write("Wrote %s\n" % tarfile)
105
106
107 def handle_collect_logs_args(name, args):
108 """Handle calls to 'cloud-init collect-logs' as a subcommand."""
109- collect_logs(args.tarfile, args.userdata)
110+ collect_logs(args.tarfile, args.userdata, args.verbosity)
111
112
113 def main():
114diff --git a/cloudinit/cmd/devel/tests/test_logs.py b/cloudinit/cmd/devel/tests/test_logs.py
115index dc4947c..98b4756 100644
116--- a/cloudinit/cmd/devel/tests/test_logs.py
117+++ b/cloudinit/cmd/devel/tests/test_logs.py
118@@ -4,6 +4,7 @@ from cloudinit.cmd.devel import logs
119 from cloudinit.util import ensure_dir, load_file, subp, write_file
120 from cloudinit.tests.helpers import FilesystemMockingTestCase, wrap_and_call
121 from datetime import datetime
122+import mock
123 import os
124
125
126@@ -27,11 +28,13 @@ class TestCollectLogs(FilesystemMockingTestCase):
127 date = datetime.utcnow().date().strftime('%Y-%m-%d')
128 date_logdir = 'cloud-init-logs-{0}'.format(date)
129
130+ version_out = '/usr/bin/cloud-init 18.2fake\n'
131 expected_subp = {
132 ('dpkg-query', '--show', "-f=${Version}\n", 'cloud-init'):
133 '0.7fake\n',
134+ ('cloud-init', '--version'): version_out,
135 ('dmesg',): 'dmesg-out\n',
136- ('journalctl', '-o', 'short-precise'): 'journal-out\n',
137+ ('journalctl', '--boot=0', '-o', 'short-precise'): 'journal-out\n',
138 ('tar', 'czvf', output_tarfile, date_logdir): ''
139 }
140
141@@ -44,9 +47,12 @@ class TestCollectLogs(FilesystemMockingTestCase):
142 subp(cmd) # Pass through tar cmd so we can check output
143 return expected_subp[cmd_tuple], ''
144
145+ fake_stderr = mock.MagicMock()
146+
147 wrap_and_call(
148 'cloudinit.cmd.devel.logs',
149 {'subp': {'side_effect': fake_subp},
150+ 'sys.stderr': {'new': fake_stderr},
151 'CLOUDINIT_LOGS': {'new': [log1, log2]},
152 'CLOUDINIT_RUN_DIR': {'new': self.run_dir}},
153 logs.collect_logs, output_tarfile, include_userdata=False)
154@@ -55,7 +61,9 @@ class TestCollectLogs(FilesystemMockingTestCase):
155 out_logdir = self.tmp_path(date_logdir, self.new_root)
156 self.assertEqual(
157 '0.7fake\n',
158- load_file(os.path.join(out_logdir, 'version')))
159+ load_file(os.path.join(out_logdir, 'dpkg-version')))
160+ self.assertEqual(version_out,
161+ load_file(os.path.join(out_logdir, 'version')))
162 self.assertEqual(
163 'cloud-init-log',
164 load_file(os.path.join(out_logdir, 'cloud-init.log')))
165@@ -72,6 +80,7 @@ class TestCollectLogs(FilesystemMockingTestCase):
166 'results',
167 load_file(
168 os.path.join(out_logdir, 'run', 'cloud-init', 'results.json')))
169+ fake_stderr.write.assert_any_call('Wrote %s\n' % output_tarfile)
170
171 def test_collect_logs_includes_optional_userdata(self):
172 """collect-logs include userdata when --include-userdata is set."""
173@@ -88,11 +97,13 @@ class TestCollectLogs(FilesystemMockingTestCase):
174 date = datetime.utcnow().date().strftime('%Y-%m-%d')
175 date_logdir = 'cloud-init-logs-{0}'.format(date)
176
177+ version_out = '/usr/bin/cloud-init 18.2fake\n'
178 expected_subp = {
179 ('dpkg-query', '--show', "-f=${Version}\n", 'cloud-init'):
180 '0.7fake',
181+ ('cloud-init', '--version'): version_out,
182 ('dmesg',): 'dmesg-out\n',
183- ('journalctl', '-o', 'short-precise'): 'journal-out\n',
184+ ('journalctl', '--boot=0', '-o', 'short-precise'): 'journal-out\n',
185 ('tar', 'czvf', output_tarfile, date_logdir): ''
186 }
187
188@@ -105,9 +116,12 @@ class TestCollectLogs(FilesystemMockingTestCase):
189 subp(cmd) # Pass through tar cmd so we can check output
190 return expected_subp[cmd_tuple], ''
191
192+ fake_stderr = mock.MagicMock()
193+
194 wrap_and_call(
195 'cloudinit.cmd.devel.logs',
196 {'subp': {'side_effect': fake_subp},
197+ 'sys.stderr': {'new': fake_stderr},
198 'CLOUDINIT_LOGS': {'new': [log1, log2]},
199 'CLOUDINIT_RUN_DIR': {'new': self.run_dir},
200 'USER_DATA_FILE': {'new': userdata}},
201@@ -118,3 +132,4 @@ class TestCollectLogs(FilesystemMockingTestCase):
202 self.assertEqual(
203 'user-data',
204 load_file(os.path.join(out_logdir, 'user-data.txt')))
205+ fake_stderr.write.assert_any_call('Wrote %s\n' % output_tarfile)

Subscribers

People subscribed via source and target branches