Skip to content

Commit

Permalink
Re-enable pylint duplicate-code check (#956)
Browse files Browse the repository at this point in the history
The bulk of the changes in this patch are to the plugin_extensions
that implement summary output. Most plugins have some entries in
common and those are now moved to a base class so that they don't
need to be redefined by each plugin.

Also optimises some of the hosthelpers and checks implementations
to satisfy the duplicate-code check. Some disables have been used
where it was deemed that fixing the code would introduce too much
instability and this can be addressed in future patches.
  • Loading branch information
dosaboy authored Jul 29, 2024
1 parent 8d9e0e9 commit dd51c3f
Show file tree
Hide file tree
Showing 46 changed files with 613 additions and 580 deletions.
2 changes: 1 addition & 1 deletion examples/hotsos-example-juju.summary.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ system:
this environment. If maintenance windows are required please consider disabling
unattended upgrades.
juju:
version: 2.9.22
services:
systemd:
enabled:
- jujud-machine-1
ps:
- jujud (1)
version: 2.9.22
machine: '1'
units:
ceph-osd/0:
Expand Down
74 changes: 66 additions & 8 deletions hotsos/core/host_helpers/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
import pickle
import re
from functools import cached_property

from searchkit.utils import MPCache
from hotsos.core.config import HotSOSConfig
Expand All @@ -12,6 +13,7 @@
SourceNotFound,
)
from hotsos.core.log import log
from hotsos.core.utils import sorted_dict


class NullCache():
Expand Down Expand Up @@ -88,6 +90,12 @@ def __init__(self, service_exprs, ps_allow_relative=True):
self._ps_allow_relative = ps_allow_relative
self._service_exprs = set(service_exprs)

@property
@abc.abstractmethod
def _service_manager_type(self):
""" A string name representing the type of service manager e.g.
'systemd' """

def get_cmd_from_ps_line(self, line, expr):
"""
Match a command in ps output line.
Expand All @@ -111,23 +119,73 @@ def get_cmd_from_ps_line(self, line, expr):

@property
@abc.abstractmethod
def services(self):
""" Return a dictionary of identified services and their state. """
def _service_filtered_ps(self):
""" Return a list ps entries corresponding to services. """

@property
@abc.abstractmethod
@cached_property
def processes(self):
"""
Return a dictionary of processes associated with identified
services.
Identify running processes from ps that are associated with resolved
services. The search pattern used to identify a service is also
used to match the process binaryc/cmd name.
Accounts for different types of process cmd path e.g.
/snap/<name>/1830/<svc>
/usr/bin/<svc>
and filter e.g.
/var/lib/<svc> and /var/log/<svc>
Returns a dictionary of process names along with the number of each.
"""
_proc_info = {}
for line in self._service_filtered_ps:
for expr in self._service_exprs:
cmd = self.get_cmd_from_ps_line(line, expr)
if not cmd:
continue

if cmd in _proc_info:
_proc_info[cmd] += 1
else:
_proc_info[cmd] = 1

return _proc_info

@property
@abc.abstractmethod
def services(self):
""" Return a dictionary of identified services and their state. """

@property
def _service_info(self):
"""Return a dictionary of services grouped by state. """
info = {}
for svc, obj in sorted_dict(self.services).items():
state = obj.state
if state not in info:
info[state] = []

info[state].append(svc)

return info

@property
def _process_info(self):
"""Return a list of processes associated with services. """
return [f"{name} ({count})"
for name, count in sorted_dict(self.processes).items()]

@property
def summary(self):
""" Return a dictionary summary of this class i.e. services,
their state and associated processes.
"""
Output a dict summary of this class i.e. services, their state and any
processes run by them.
"""
return {self._service_manager_type: self._service_info,
'ps': self._process_info}


class NullSource():
Expand Down
63 changes: 6 additions & 57 deletions hotsos/core/host_helpers/pebble.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from hotsos.core.factory import FactoryBase
from hotsos.core.host_helpers import CLIHelper
from hotsos.core.host_helpers.common import ServiceManagerBase
from hotsos.core.utils import sorted_dict


class PebbleService():
Expand All @@ -21,6 +20,10 @@ def __repr__(self):
class PebbleHelper(ServiceManagerBase):
""" Helper class used to query pebble services. """

@property
def _service_manager_type(self):
return 'pebble'

@cached_property
def services(self):
""" Return a dict of identified pebble services and their state. """
Expand All @@ -38,62 +41,8 @@ def services(self):
return _services

@cached_property
def processes(self):
"""
Identify running processes from ps that are associated with resolved
pebble services. The search pattern used to identify a service is also
used to match the process binary/cmd name.
Accounts for different types of process cmd path e.g.
/snap/<name>/1830/<svc>
/usr/bin/<svc>
and filter e.g.
/var/lib/<svc> and /var/log/<svc>
Returns a dictionary of process names along with the number of each.
"""
_proc_info = {}
for line in CLIHelper().ps():
for expr in self._service_exprs:
cmd = self.get_cmd_from_ps_line(line, expr)
if cmd:
if cmd in _proc_info:
_proc_info[cmd] += 1
else:
_proc_info[cmd] = 1

return _proc_info

@property
def _service_info(self):
"""Return a dictionary of pebble services grouped by state. """
info = {}
for svc, obj in sorted_dict(self.services).items():
state = obj.state
if state not in info:
info[state] = []

info[state].append(svc)

return info

@property
def _process_info(self):
"""Return a list of processes associated with services. """
return [f"{name} ({count})"
for name, count in sorted_dict(self.processes).items()]

@property
def summary(self):
"""
Output a dict summary of this class i.e. services, their state and any
processes run by them.
"""
return {'pebble': self._service_info,
'ps': self._process_info}
def _service_filtered_ps(self):
return CLIHelper().ps()


class ServiceFactory(FactoryBase):
Expand Down
65 changes: 4 additions & 61 deletions hotsos/core/host_helpers/systemd.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
from hotsos.core.host_helpers import CLIHelper, CLIHelperFile
from hotsos.core.host_helpers.common import ServiceManagerBase
from hotsos.core.log import log
from hotsos.core.utils import sorted_dict


class SystemdService():
Expand Down Expand Up @@ -187,6 +186,10 @@ def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self._cached_unit_files_exprs = {}

@property
def _service_manager_type(self):
return 'systemd'

@cached_property
def _systemctl_list_units(self):
return CLIHelper().systemctl_list_units()
Expand Down Expand Up @@ -350,66 +353,6 @@ def _service_filtered_ps(self):

return ps_filtered

@cached_property
def processes(self):
"""
Identify running processes from ps that are associated with resolved
systemd services. The search pattern used to identify a service is also
used to match the process binaryc/cmd name.
Accounts for different types of process cmd path e.g.
/snap/<name>/1830/<svc>
/usr/bin/<svc>
and filter e.g.
/var/lib/<svc> and /var/log/<svc>
Returns a dictionary of process names along with the number of each.
"""
_proc_info = {}
for line in self._service_filtered_ps:
for expr in self._service_exprs:
cmd = self.get_cmd_from_ps_line(line, expr)
if not cmd:
continue

if cmd in _proc_info:
_proc_info[cmd] += 1
else:
_proc_info[cmd] = 1

return _proc_info

@property
def _service_info(self):
"""Return a dictionary of systemd services grouped by state. """
info = {}
for svc, obj in sorted_dict(self.services).items():
state = obj.state
if state not in info:
info[state] = []

info[state].append(svc)

return info

@property
def _process_info(self):
"""Return a list of processes associated with services. """
return [f"{name} ({count})"
for name, count in sorted_dict(self.processes).items()]

@property
def summary(self):
"""
Output a dict summary of this class i.e. services, their state and any
processes run by them.
"""
return {'systemd': self._service_info,
'ps': self._process_info}


class ServiceFactory(FactoryBase):
"""
Expand Down
7 changes: 7 additions & 0 deletions hotsos/core/plugins/juju/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ def __init__(self, *args, **kwargs):
# this is needed for juju scenarios
self.systemd_processes = self.systemd.processes

@property
def version(self):
if self.machine:
return self.machine.version

return "unknown"

@property
def plugin_runnable(self):
return os.path.exists(self.juju_lib_path)
4 changes: 2 additions & 2 deletions hotsos/core/plugins/mysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ class MySQLChecks(plugintools.PluginPartBase):

def __init__(self, *args, **kwargs):
super().__init__()
self.apt_info = APTPackageHelper(core_pkgs=CORE_APT)
self.apt = APTPackageHelper(core_pkgs=CORE_APT)
self.pebble = PebbleHelper(service_exprs=MYSQL_SVC_EXPRS)
self.systemd = SystemdHelper(service_exprs=MYSQL_SVC_EXPRS)

@property
def plugin_runnable(self):
return self.apt_info.core is not None
return self.apt.core is not None


class MySQLConfig(host_helpers.IniConfigBase):
Expand Down
5 changes: 5 additions & 0 deletions hotsos/core/plugins/storage/bcache.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,11 @@ def cache_available_percent(self):

class BcacheChecks(BcacheBase):
""" Bcache checks. """

@property
def summary_subkey_include_default_entries(self):
return True

@property
def summary_subkey(self):
return 'bcache'
4 changes: 4 additions & 0 deletions hotsos/core/plugins/storage/ceph/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ def __init__(self, *args, **kwargs):
self.systemd = SystemdHelper(service_exprs=CEPH_SERVICES_EXPRS)
self.cluster = CephCluster()

@property
def summary_subkey_include_default_entries(self):
return True

@property
def summary_subkey(self):
return 'ceph'
Expand Down
Loading

0 comments on commit dd51c3f

Please sign in to comment.