From 2dceffa5a634fa64d52f05ced5dc27de991de710 Mon Sep 17 00:00:00 2001 From: Michael Still Date: Wed, 21 Nov 2012 21:43:12 +1100 Subject: [PATCH] Truncate large console logs in libvirt. Resolves bug 1081436 where they were returning a 5.5gb console log to the user. Change-Id: I0d98c83e801f8936d35789b5e8f8283f49483c4e --- nova/tests/fake_libvirt_utils.py | 5 ++- nova/tests/test_libvirt.py | 39 +++++++++++++-------- nova/tests/test_utils.py | 60 ++++++++++++++++++++++++++++---- nova/tests/test_virt_drivers.py | 5 +++ nova/utils.py | 21 +++++++++++ nova/virt/libvirt/driver.py | 18 ++++++++-- 6 files changed, 125 insertions(+), 23 deletions(-) diff --git a/nova/tests/fake_libvirt_utils.py b/nova/tests/fake_libvirt_utils.py index 51077493959e..353b6ef1f208 100644 --- a/nova/tests/fake_libvirt_utils.py +++ b/nova/tests/fake_libvirt_utils.py @@ -85,7 +85,10 @@ def extract_snapshot(disk_path, source_fmt, snapshot_name, out_path, dest_fmt): class File(object): def __init__(self, path, mode=None): - self.fp = StringIO.StringIO(files[path]) + if path in files: + self.fp = StringIO.StringIO(files[path]) + else: + self.fp = StringIO.StringIO(files[os.path.split(path)[-1]]) def __enter__(self): return self.fp diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py index bc741909d2f1..54a355a39321 100644 --- a/nova/tests/test_libvirt.py +++ b/nova/tests/test_libvirt.py @@ -2214,6 +2214,7 @@ class LibvirtConnTestCase(test.TestCase): FLAGS.base_dir_name)) def test_get_console_output_file(self): + fake_libvirt_utils.files['console.log'] = '01234567890' with utils.tempdir() as tmpdir: self.flags(instances_path=tmpdir) @@ -2223,11 +2224,7 @@ class LibvirtConnTestCase(test.TestCase): instance = db.instance_create(self.context, instance_ref) console_dir = (os.path.join(tmpdir, instance['name'])) - os.mkdir(console_dir) console_log = '%s/console.log' % (console_dir) - f = open(console_log, "w") - f.write("foo") - f.close() fake_dom_xml = """ @@ -2250,10 +2247,18 @@ class LibvirtConnTestCase(test.TestCase): libvirt_driver.libvirt_utils = fake_libvirt_utils conn = libvirt_driver.LibvirtDriver(False) - output = conn.get_console_output(instance) - self.assertEquals("foo", output) + + try: + prev_max = libvirt_driver.MAX_CONSOLE_BYTES + libvirt_driver.MAX_CONSOLE_BYTES = 5 + output = conn.get_console_output(instance) + finally: + libvirt_driver.MAX_CONSOLE_BYTES = prev_max + + self.assertEquals('67890', output) def test_get_console_output_pty(self): + fake_libvirt_utils.files['pty'] = '01234567890' with utils.tempdir() as tmpdir: self.flags(instances_path=tmpdir) @@ -2263,11 +2268,7 @@ class LibvirtConnTestCase(test.TestCase): instance = db.instance_create(self.context, instance_ref) console_dir = (os.path.join(tmpdir, instance['name'])) - os.mkdir(console_dir) pty_file = '%s/fake_pty' % (console_dir) - f = open(pty_file, "w") - f.write("foo") - f.close() fake_dom_xml = """ @@ -2286,17 +2287,27 @@ class LibvirtConnTestCase(test.TestCase): return FakeVirtDomain(fake_dom_xml) def _fake_flush(self, fake_pty): - with open(fake_pty, 'r') as fp: - return fp.read() + return 'foo' + + def _fake_append_to_file(self, data, fpath): + return 'pty' self.create_fake_libvirt_mock() libvirt_driver.LibvirtDriver._conn.lookupByName = fake_lookup libvirt_driver.LibvirtDriver._flush_libvirt_console = _fake_flush + libvirt_driver.LibvirtDriver._append_to_file = _fake_append_to_file libvirt_driver.libvirt_utils = fake_libvirt_utils conn = libvirt_driver.LibvirtDriver(False) - output = conn.get_console_output(instance) - self.assertEquals("foo", output) + + try: + prev_max = libvirt_driver.MAX_CONSOLE_BYTES + libvirt_driver.MAX_CONSOLE_BYTES = 5 + output = conn.get_console_output(instance) + finally: + libvirt_driver.MAX_CONSOLE_BYTES = prev_max + + self.assertEquals('67890', output) def test_get_host_ip_addr(self): conn = libvirt_driver.LibvirtDriver(False) diff --git a/nova/tests/test_utils.py b/nova/tests/test_utils.py index 53177821253f..adc62d8f6cd1 100644 --- a/nova/tests/test_utils.py +++ b/nova/tests/test_utils.py @@ -768,9 +768,57 @@ class DiffDict(test.TestCase): self.assertEqual(diff, dict(b=['-'])) -class EnsureTree(test.TestCase): - def test_ensure_tree(self): - with utils.tempdir() as tmpdir: - testdir = '%s/foo/bar/baz' % (tmpdir,) - utils.ensure_tree(testdir) - self.assertTrue(os.path.isdir(testdir)) +class MkfsTestCase(test.TestCase): + + @test.skip_test("Skip because utils.mkfs is not available.") + def test_mkfs(self): + self.mox.StubOutWithMock(utils, 'execute') + utils.execute('mkfs', '-t', 'ext4', '-F', '/my/block/dev') + utils.execute('mkfs', '-t', 'msdos', '/my/msdos/block/dev') + utils.execute('mkswap', '/my/swap/block/dev') + self.mox.ReplayAll() + + utils.mkfs('ext4', '/my/block/dev') + utils.mkfs('msdos', '/my/msdos/block/dev') + utils.mkfs('swap', '/my/swap/block/dev') + + @test.skip_test("Skip because utils.mkfs is not available.") + def test_mkfs_with_label(self): + self.mox.StubOutWithMock(utils, 'execute') + utils.execute('mkfs', '-t', 'ext4', '-F', + '-L', 'ext4-vol', '/my/block/dev') + utils.execute('mkfs', '-t', 'msdos', + '-n', 'msdos-vol', '/my/msdos/block/dev') + utils.execute('mkswap', '-L', 'swap-vol', '/my/swap/block/dev') + self.mox.ReplayAll() + + utils.mkfs('ext4', '/my/block/dev', 'ext4-vol') + utils.mkfs('msdos', '/my/msdos/block/dev', 'msdos-vol') + utils.mkfs('swap', '/my/swap/block/dev', 'swap-vol') + + +class LastBytesTestCase(test.TestCase): + """Test the last_bytes() utility method.""" + + def setUp(self): + super(LastBytesTestCase, self).setUp() + self.f = StringIO.StringIO('1234567890') + + def test_truncated(self): + self.f.seek(0, os.SEEK_SET) + out, remaining = utils.last_bytes(self.f, 5) + self.assertEqual(out, '67890') + self.assertTrue(remaining > 0) + + def test_read_all(self): + self.f.seek(0, os.SEEK_SET) + out, remaining = utils.last_bytes(self.f, 1000) + self.assertEqual(out, '1234567890') + self.assertFalse(remaining > 0) + + def test_seek_too_far_real_file(self): + # StringIO doesn't raise IOError if you see past the start of the file. + flo = tempfile.TemporaryFile() + content = '1234567890' + flo.write(content) + self.assertEqual((content, 0), utils.last_bytes(flo, 1000)) diff --git a/nova/tests/test_virt_drivers.py b/nova/tests/test_virt_drivers.py index 18b6bdc7dd94..c0ec422ba75c 100644 --- a/nova/tests/test_virt_drivers.py +++ b/nova/tests/test_virt_drivers.py @@ -14,8 +14,11 @@ # License for the specific language governing permissions and limitations # under the License. +import __builtin__ import base64 +import mox import netaddr +import StringIO import sys import traceback @@ -24,6 +27,7 @@ from nova import exception from nova.openstack.common import importutils from nova.openstack.common import log as logging from nova import test +from nova.tests import fake_libvirt_utils from nova.tests.image import fake as fake_image from nova.tests import utils as test_utils @@ -424,6 +428,7 @@ class _VirtDriverTestCase(_FakeDriverBackendTestCase): @catch_notimplementederror def test_get_console_output(self): + fake_libvirt_utils.files['dummy.log'] = '' instance_ref, network_info = self._get_running_instance() console_output = self.connection.get_console_output(instance_ref) self.assertTrue(isinstance(console_output, basestring)) diff --git a/nova/utils.py b/nova/utils.py index 015ff915a509..5722cf5621bc 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -1328,3 +1328,24 @@ def ensure_tree(path): raise else: raise + + +def last_bytes(file_like_object, num): + """Return num bytes from the end of the file, and remaining byte count. + + :param file_like_object: The file to read + :param num: The number of bytes to return + + :returns (data, remaining) + """ + + try: + file_like_object.seek(-num, os.SEEK_END) + except IOError, e: + if e.errno == 22: + file_like_object.seek(0, os.SEEK_SET) + else: + raise + + remaining = file_like_object.tell() + return (file_like_object.read(), remaining) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 30a1f94ff921..8c3761ca7984 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -200,6 +200,8 @@ DEFAULT_FIREWALL_DRIVER = "%s.%s" % ( libvirt_firewall.__name__, libvirt_firewall.IptablesFirewallDriver.__name__) +MAX_CONSOLE_BYTES = 102400 + def patch_tpool_proxy(): """eventlet.tpool.Proxy doesn't work with old-style class in __str__() @@ -1126,7 +1128,14 @@ class LibvirtDriver(driver.ComputeDriver): if not path: continue libvirt_utils.chown(path, os.getuid()) - return libvirt_utils.load_file(path) + + with libvirt_utils.file_open(path, 'rb') as fp: + log_data, remaining = utils.last_bytes(fp, + MAX_CONSOLE_BYTES) + if remaining > 0: + LOG.info(_('Truncated console log returned, %d bytes ' + 'ignored'), remaining, instance=instance) + return log_data # Try 'pty' types if console_types.get('pty'): @@ -1147,7 +1156,12 @@ class LibvirtDriver(driver.ComputeDriver): console_log = self._get_console_log_path(instance['name']) fpath = self._append_to_file(data, console_log) - return libvirt_utils.load_file(fpath) + with libvirt_utils.file_open(fpath, 'rb') as fp: + log_data, remaining = utils.last_bytes(fp, MAX_CONSOLE_BYTES) + if remaining > 0: + LOG.info(_('Truncated console log returned, %d bytes ignored'), + remaining, instance=instance) + return log_data @staticmethod def get_host_ip_addr():