Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[plugins] maxage and all_logs on all plugins #3681

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[plugins] maxage and all_logs on all plugins #3681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d810c0
[PluginOpt] Support different parameter name
Akrog Jun 26, 2024
8bc2ccb
[plugins] maxhours and all_logs on all plugins
Akrog Jun 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions sos/report/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ class SoSReport(SoSComponent):
'cmd_timeout': TIMEOUT_DEFAULT,
'profiles': [],
'since': None,
'maxhours': None,
'verify': False,
'allow_system_changes': False,
'usernames': [],
Expand Down Expand Up @@ -200,6 +201,12 @@ def add_parser_options(cls, parser):
help="Escapes archived files older than date. "
"This will also affect --all-logs. "
"Format: YYYYMMDD[HHMMSS]")
report_grp.add_argument("--maxhours", action="store",
dest="maxhours", default=None, type=int,
help="Escapes archived files older (according "
"to `mktime`) than this many hours. This will "
"also affect --all-logs (sets the default for "
"all plugins)")
report_grp.add_argument("--build", action="store_true",
dest="build", default=False,
help="preserve the temporary directory and do "
Expand Down Expand Up @@ -904,6 +911,12 @@ def _set_all_options(self):
opt.value = True

def _set_tunables(self):
# Set plugin option's defaults to the global argument values
for pluginname, plugin in self.loaded_plugins:
for optname in plugin.options:
if hasattr(self.opts, optname):
plugin.options[optname].value = getattr(self.opts, optname)
TurboTurtle marked this conversation as resolved.
Show resolved Hide resolved

if self.opts.plugopts:
opts = {}
for opt in self.opts.plugopts:
Expand Down Expand Up @@ -940,13 +953,17 @@ def _set_tunables(self):

for plugname, plug in self.loaded_plugins:
if plugname in opts:
for opt in opts[plugname]:
if opt not in plug.options:
self.soslog.error(f'no such option "{opt}" for '
# Map from parameter names to internal option names
plug_param_map = {o.param_name: o.name
for o in plug.options.values()}
for param in opts[plugname]:
opt = plug_param_map.get(param)
if not opt:
self.soslog.error(f'no such option "{param}" for '
f'plugin ({plugname})')
self._exit(1)
try:
plug.options[opt].set_value(opts[plugname][opt])
plug.options[opt].set_value(opts[plugname][param])
self.soslog.debug(
f"Set {plugname} plugin option to "
f"{plug.options[opt]}")
Expand Down Expand Up @@ -1059,12 +1076,13 @@ def list_plugins(self):
val = TIMEOUT_DEFAULT
if opt.name == 'postproc':
val = not self.opts.no_postproc
self.ui_log.info(f"{opt.name:<25} {val:<15} {opt.desc}")
self.ui_log.info(
f"{opt.param_name:<25} {val!s:<15} {opt.desc}")
self.ui_log.info("")

self.ui_log.info(_("The following plugin options are available:"))
for opt in self.all_options:
if opt.name in ('timeout', 'postproc', 'cmd-timeout'):
if opt.name in _defaults:
if opt.value == opt.default:
continue
# format option value based on its type (int or bool)
Expand All @@ -1079,7 +1097,7 @@ def list_plugins(self):
if tmpopt is None:
tmpopt = 0

self.ui_log.info(f" {f'{opt.plugin}.{opt.name}':<25} "
self.ui_log.info(f" {f'{opt.plugin}.{opt.param_name}':<25} "
f"{tmpopt:<15} {opt.desc}")
else:
self.ui_log.info(_("No plugin options available."))
Expand Down
57 changes: 41 additions & 16 deletions sos/report/plugins/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,8 @@ class PluginOpt():
to define options alongside their distro-specific classes in order to add
support for user-controlled changes in Plugin behavior.

:param name: The name of the plugin option
:param name: The name of the plugin option as used by the code.
Equivalent to the `dest` in arg parser terminology.
:type name: ``str``

:param default: The default value of the option
Expand All @@ -417,9 +418,15 @@ class PluginOpt():
:param val_type: The type of object the option accepts for values. If
not provided, auto-detect from the type of ``default``
:type val_type: A single type or a ``list`` of types

:param param_name: The name of the parameter in the command line. Defaults
to ``name`` if unset.
:type param_name: ``str``

"""

name = ''
param_name = ''
default = None
enabled = False
desc = ''
Expand All @@ -429,8 +436,9 @@ class PluginOpt():
plugin = ''

def __init__(self, name='undefined', default=None, desc='', long_desc='',
val_type=None):
val_type=None, param_name=None):
self.name = name
self.param_name = param_name or name
self.default = default
self.desc = desc
self.long_desc = long_desc
Expand All @@ -445,6 +453,7 @@ def __init__(self, name='undefined', default=None, desc='', long_desc='',
def __str__(self):
items = [
f'name={self.name}',
f'param_name={self.param_name}',
f'desc=\'{self.desc}\'',
f'value={self.value}',
f'default={self.default}'
Expand Down Expand Up @@ -585,7 +594,19 @@ def __init__(self, commons):
self.set_predicate(SoSPredicate(self))

def get_default_plugin_opts(self):
# The default of options that have a global argument counterpart with
# the same name must use the same default in both places.
return {
'maxhours': PluginOpt(
'maxhours', default=None, val_type=int,
desc='Escapes archived files older (according to `mktime`) '
'than this many hours'
),
'all_logs': PluginOpt(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We recently standardized on using hyphens for plugin options, to match the "top level" options which are all hyphens. So this will need to be all-logs (though the internal dest naming will of course still be all_logs).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm new to this, but I assumed looking at the kernel plugin with-timer definition and how it is accessed as with-timer and not with_timer that there is no translation on the PluginOpt.

I looked a bit into the get_option and PluginOpt and didn't find any translation mechanism so I went with the solution that required fewer code changes.

But it is true that it would be weird to call it --all-logs only to have to use all_logs in the plugins.

I'll see if I can use all-logs everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

'all_logs', default=False, val_type=bool,
param_name='all-logs',
desc='collect all available logs regardless of size',
),
'timeout': PluginOpt(
'timeout', default=-1, val_type=int,
desc='Timeout in seconds for plugin to finish all collections'
Expand Down Expand Up @@ -861,7 +882,7 @@ def display_plugin_help(cls, section):
_def = "True/On"
else:
_def = "False/Off"
_ln = f"{' ':<4}{opt.name:<20}{_def:<30}{opt.desc:<20}"
_ln = f"{' ':<4}{opt.param_name:<20}{_def:<30}{opt.desc:<20}"
optsec.add_text(
textwrap.fill(_ln, width=TERMSIZE,
subsequent_indent=opt_indent),
Expand Down Expand Up @@ -1558,9 +1579,12 @@ def get_option(self, optionname, default=0):
:returns: The value of `optionname` if found, else `default`
"""

# These are Global options that can only be set globally. Options that
# can be set globally and per-plugin (those also returned by
# get_default_plugin_opts) are handled by the `_set_tunables` method.
global_options = (
'all_logs', 'allow_system_changes', 'cmd_timeout', 'journal_size',
'log_size', 'plugin_timeout', 'since', 'verify'
'allow_system_changes', 'cmd_timeout', 'journal_size', 'log_size',
'plugin_timeout', 'since', 'verify'
)

if optionname in global_options:
Expand Down Expand Up @@ -1627,7 +1651,7 @@ def generate_copyspec_tags(self):
manifest_data['files_copied'] = matched_files
self.manifest.files.append(manifest_data)

def add_copy_spec(self, copyspecs, sizelimit=None, maxage=None,
def add_copy_spec(self, copyspecs, sizelimit=None, maxhours=None,
tailit=True, pred=None, tags=[], container=None):
"""Add a file, directory, or globs matching filepaths to the archive

Expand All @@ -1638,9 +1662,9 @@ def add_copy_spec(self, copyspecs, sizelimit=None, maxage=None,
to this size in MB
:type sizelimit: ``int``

:param maxage: Collect files with `mtime` not older than this many
hours
:type maxage: ``int``
:param maxhours: Collect files with `mtime` not older than this many
hours
:type maxhours: ``int``

:param tailit: Should a file that exceeds `sizelimit` be tail'ed to fit
the remaining space to meet `sizelimit`
Expand Down Expand Up @@ -1670,9 +1694,10 @@ def add_copy_spec(self, copyspecs, sizelimit=None, maxage=None,
sos will collect up to 25MB worth of files within `/etc/foo`, and will
collect the last 25MB of `/etc/bar.conf`.
"""
since = None
if self.get_option('since'):
since = self.get_option('since')
since = self.get_option('since') or None

# User's plugin option argument takes precedence over plugin passed arg
maxhours = self.get_option('maxhours') or maxhours

logarchive_pattern = re.compile(r'.*((\.(zip|gz|bz2|xz))|[-.][\d]+)$')
configfile_pattern = re.compile(fr"^{self.path_join('etc')}/*")
Expand Down Expand Up @@ -1789,8 +1814,8 @@ def getmtime(path):
return 0

def time_filter(path):
""" When --since is passed, or maxage is coming from the
plugin, we need to filter out older files """
""" When --since is passed, or maxhours is coming from the
plugin or its options, we need to filter out older files """

# skip config files or not-logarchive files from the filter
if ((logarchive_pattern.search(path) is None) or
Expand All @@ -1799,11 +1824,11 @@ def time_filter(path):
filetime = getmtime(path)
filedatetime = datetime.fromtimestamp(filetime)
if ((since and filedatetime < since) or
(maxage and (time()-filetime < maxage*3600))):
(maxhours and (time()-filetime < maxhours*3600))):
return False
return True

if since or maxage:
if since or maxhours:
files = list(filter(lambda f: time_filter(f), files))

files.sort(key=getmtime, reverse=True)
Expand Down
4 changes: 2 additions & 2 deletions sos/report/plugins/kdump.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def setup(self):

# collect the latest vmcore created in the last 24hrs <= 2GB
if self.get_option("get-vm-core"):
self.add_copy_spec(f"{path}/*/vmcore", sizelimit=2048, maxage=24)
self.add_copy_spec(f"{path}/*/vmcore", sizelimit=2048, maxhours=24)


class DebianKDump(KDump, DebianPlugin, UbuntuPlugin):
Expand Down Expand Up @@ -178,6 +178,6 @@ def setup(self):

# collect the latest vmcore created in the last 24hrs <= 2GB
if self.get_option("get-vm-core"):
self.add_copy_spec(f"{path}/*/vmcore", sizelimit=2048, maxage=24)
self.add_copy_spec(f"{path}/*/vmcore", sizelimit=2048, maxhours=24)

# vim: set et ts=4 sw=4 :
3 changes: 2 additions & 1 deletion tests/report_tests/options_tests/options_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@ def test_plugins_only_from_config(self):
def test_plugopts_logged_from_config(self):
self.assertSosLogContains(
r"Set kernel plugin option to \(name=with-timer, "
r"param_name=with-timer, "
r"desc='gather /proc/timer\* statistics', value=True, "
r"default=False\)"
)
self.assertSosLogContains(
r"Set kernel plugin option to \(name=trace, "
r"Set kernel plugin option to \(name=trace, param_name=trace, "
"desc='gather /sys/kernel/debug/tracing/trace file', "
r"value=True, default=False\)"
)
Expand Down
Loading