From d40132ad7105ec1b039d025587a82510ea9698f4 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Sat, 11 Jan 2020 18:27:13 +0100 Subject: [PATCH] Omit configdrive and system_logs from logging Since they are large and base64-encoded, they bloat ramdisk logs. Change-Id: I2e995ef356075be2a7f5b0a1906d02f90fe98a06 --- ironic_python_agent/extensions/base.py | 15 ++++++++++----- ironic_python_agent/tests/unit/test_utils.py | 11 +++++++++++ ironic_python_agent/utils.py | 17 +++++++++++++++++ 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/ironic_python_agent/extensions/base.py b/ironic_python_agent/extensions/base.py index 69f113517..b15c844b0 100644 --- a/ironic_python_agent/extensions/base.py +++ b/ironic_python_agent/extensions/base.py @@ -22,6 +22,7 @@ from oslo_utils import uuidutils from ironic_python_agent import encoding from ironic_python_agent import errors +from ironic_python_agent import utils LOG = log.getLogger() @@ -59,9 +60,10 @@ class BaseCommandResult(encoding.SerializableComparable): return ("Command name: %(name)s, " "params: %(params)s, status: %(status)s, result: " "%(result)s." % - {"name": self.command_name, "params": self.command_params, + {"name": self.command_name, + "params": utils.remove_large_keys(self.command_params), "status": self.command_status, - "result": self.command_result}) + "result": utils.remove_large_keys(self.command_result)}) def is_done(self): """Checks to see if command is still RUNNING. @@ -160,7 +162,8 @@ class AsyncCommandResult(BaseCommandResult): if isinstance(result, (bytes, str)): result = {'result': '{}: {}'.format(self.command_name, result)} LOG.info('Command: %(name)s, result: %(result)s', - {'name': self.command_name, 'result': result}) + {'name': self.command_name, + 'result': utils.remove_large_keys(result)}) with self.command_state_lock: self.command_result = result self.command_status = AgentCommandStatus.SUCCEEDED @@ -234,7 +237,8 @@ class ExecuteCommandMixin(object): """Execute an agent command.""" with self.command_lock: LOG.debug('Executing command: %(name)s with args: %(args)s', - {'name': command_name, 'args': kwargs}) + {'name': command_name, + 'args': utils.remove_large_keys(kwargs)}) extension_part, command_part = self.split_command(command_name) if len(self.command_results) > 0: @@ -264,7 +268,8 @@ class ExecuteCommandMixin(object): LOG.exception('Command execution error: %s', e) result = SyncCommandResult(command_name, kwargs, False, e) LOG.info('Command %(name)s completed: %(result)s', - {'name': command_name, 'result': result}) + {'name': command_name, + 'result': utils.remove_large_keys(result)}) self.command_results[result.id] = result return result diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py index 3fec355c4..94c3a31f3 100644 --- a/ironic_python_agent/tests/unit/test_utils.py +++ b/ironic_python_agent/tests/unit/test_utils.py @@ -639,3 +639,14 @@ class TestUtils(testtools.TestCase): mocked_execute.assert_has_calls( [mock.call('parted', '-s', '/dev/sda', '--', 'print')] ) + + +class TestRemoveKeys(testtools.TestCase): + def test_remove_keys(self): + value = {'system_logs': 'abcd', + 'key': 'value', + 'other': [{'configdrive': 'foo'}, 'string', 0]} + expected = {'system_logs': '<...>', + 'key': 'value', + 'other': [{'configdrive': '<...>'}, 'string', 0]} + self.assertEqual(expected, utils.remove_large_keys(value)) diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index b768da94e..d3eb08bd3 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from collections import abc import copy import errno import glob @@ -469,3 +470,19 @@ def get_efi_part_on_device(device): else: LOG.debug("No efi partition found on device %s", device) return efi_part + + +_LARGE_KEYS = frozenset(['configdrive', 'system_logs']) + + +def remove_large_keys(var): + """Remove specific keys from the var, recursing into dicts and lists.""" + if isinstance(var, abc.Mapping): + return var.__class__( + (key, remove_large_keys(value) + if key not in _LARGE_KEYS else '<...>') + for key, value in var.items()) + elif isinstance(var, abc.Sequence) and not isinstance(var, str): + return var.__class__(map(remove_large_keys, var)) + else: + return var