Merge charm-hw-health:add-ipmiseld into charm-hw-health:master

[James Hebden]

Adds ipmiseld by default. I used the Tools pattern, as that seems the way that this charm has been designed, however it's not strictly necessary as this does not include an NRPE check to ensure seld is running (yet) it just installs the package.

Closes bug 1856625.

[Paul Goins (vultaire)]

Generally looks fine, but I think there's a mismatch in the install/remove actions for IpmiSEL.

review: Needs Fixing

42382a6... by James Hebden on 2020-09-17

Fix copypasta in IpmiSEL tools class

[Peter Sabaini (peter-sabaini)]

The MR itself looks good to me, however I found the freeipmi-ipmiseld packages suffers from this bug:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=793186

We might want to work around this in the charm

c5393ad... by Peter Sabaini on 2021-01-21

Add workaround LP#1912347

Package fails to create the cache dir on install, create dir ourselves

591583e... by Peter Sabaini on 2021-01-21

Fix unittest

[Drew Freiberger (afreiberger)]

Can someone explain the namespace/stack operation for toolset within/around the _get_tools_os() call?

review: Needs Information

[Peter Sabaini (peter-sabaini)]

Answered inline -- toolset is a set() which is already a ref var

[Drew Freiberger (afreiberger)]

Thanks Peter, I see it's passed by reference and is modifying the underlying object in the _get_tools_os() function. This LGTM. +1 Thanks!

review: Approve

[Xav Paice (xavpaice)]

lgtm

review: Approve

diff --git a/src/README.md b/src/README.md
index d332504..e367400 100644
--- a/src/README.md
+++ b/src/README.md
@@ -18,6 +18,8 @@ Hardware-independent tools:
  * Linux software RAID (mdadm)
  * IPMI as implemented by freeipmi (enable_ipmi config option is enabled by
    default)
+ * ipmiseld from the freeipmi suite (enable_ipmiseld is enabled by default)
+   for logging system event log entries to syslog
 
 In the backlog, hp-health logic still needs to be backported to support
 Hewlett-Packard gen8 and older equipment (HP Controllers with hpacucli)
diff --git a/src/config.yaml b/src/config.yaml
index 2203c1a..30fd452 100644
--- a/src/config.yaml
+++ b/src/config.yaml
@@ -22,6 +22,10 @@ options:
     type: boolean
     default: True
     description: Enable the use of freeipmi tools to monitor hardware status.
+  enable_ipmiseld:
+    type: boolean
+    default: True
+    description: Enable the logging of IPMI system event log (SEL) data to syslog
   ipmi_check_options:
     type: string
     default: ''
diff --git a/src/lib/hwhealth/hwdiscovery.py b/src/lib/hwhealth/hwdiscovery.py
index 1bb8dba..e334cf8 100644
--- a/src/lib/hwhealth/hwdiscovery.py
+++ b/src/lib/hwhealth/hwdiscovery.py
@@ -44,6 +44,19 @@ def get_tools(manufacturer="auto"):
         raise NotImplementedError
 
 
+def _get_tools_os(toolset):
+    """Handle freeipmi and mdadm packages, included with Ubuntu."""
+
+    if _supports_mdadm():
+        toolset.add(tools.Mdadm)
+
+    if hookenv.config("enable_ipmi"):
+        toolset.add(tools.Ipmi)
+
+    if hookenv.config("enable_ipmiseld"):
+        toolset.add(tools.IpmiSEL)
+
+
 def _get_tools():
     """Return list of tool classes relevent for the current hardware."""
     hwinfo = Hardware()
@@ -69,12 +82,7 @@ def _get_tools():
                 continue
             hookenv.log("Driver not supported: {}".format(driver), hookenv.DEBUG)
 
-    # SW RAID?
-    if _supports_mdadm():
-        toolset.add(tools.Mdadm)
-
-    if hookenv.config("enable_ipmi"):
-        toolset.add(tools.Ipmi)
+    _get_tools_os(toolset)
 
     system_vendor = hwinfo.get_system.get("vendor")
     tool = SUPPORTED_SYSTEMS.get(system_vendor)
diff --git a/src/lib/hwhealth/tools.py b/src/lib/hwhealth/tools.py
index 1ec3255..3da262a 100644
--- a/src/lib/hwhealth/tools.py
+++ b/src/lib/hwhealth/tools.py
@@ -716,6 +716,26 @@ class Ipmi(Tool):
         return dst
 
 
+class IpmiSEL(Tool):
+    """A class representing the ipmiseld utility.
+
+    This class represents the ipmiseld utility from the freeipmi package.
+    ipmiseld is able to forward hardware system event log entries to the host
+    Operating System's syslog for logging and potentially forwarding alongside
+    regular system logs.
+    """
+
+    def install(self):
+        """Install the ipmiseld package."""
+        # Create cachedir to work around LP#1912347
+        cache_dir = Path("/var/cache/ipmiseld")
+        cache_dir.mkdir(parents=True)
+        fetch.apt_install(["freeipmi-ipmiseld"], fatal=True)
+
+    def remove(self):
+        fetch.apt_purge(["freeipmi-ipmiseld"], fatal=True)
+
+
 class Nvme(Tool):
     """A class representing the nvme tool (userspace tooling to control NVMe).
 
diff --git a/src/tests/unit/test_hwdiscovery.py b/src/tests/unit/test_hwdiscovery.py
index c657ac0..6930898 100644
--- a/src/tests/unit/test_hwdiscovery.py
+++ b/src/tests/unit/test_hwdiscovery.py
@@ -41,7 +41,9 @@ class TestGetTools(unittest.TestCase):
                 lambda: "localhost",
             ),
             mock.patch.object(
-                hwhealth.hwdiscovery.hookenv, "config", lambda x: x != "enable_ipmi"
+                hwhealth.hwdiscovery.hookenv,
+                "config",
+                lambda x: x not in {"enable_ipmi", "enable_ipmiseld"},
             ),
             mock.patch.object(
                 hwhealth.hwdiscovery.hookenv,