Merge ~mertkirpici/charm-hw-health:lp/1928403 into charm-hw-health:master

[Mert Kirpici]

Close LP #1928403

Since the ipmi-sel option "--date-range" has a resolution of 1 day, the ack-sel action used to fail acknowledging the entries on the same day. With this change we are trying to figure out the last entry id and use that to exclude the entries up to that point.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:62a7337ebc7e619e430dd4fcc04cc8a678034a11
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/72/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-unit/315/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1537/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/72//rebuild

review: Approve (continuous-integration)

[JamesLin (jneo8)]

Appropriate update to fix the issue. But I would like have more information inside the source code's comment.

review: Needs Information

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:ab5a1f83995301e613f8a5e363599fdebd0336fe
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/73/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-unit/316/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1540/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/73//rebuild

review: Approve (continuous-integration)

[Mert Kirpici (mertkirpici)]

Hey James, thanks for the review. I pushed an update that adds typing info and docstrings like you suggested.

[JamesLin (jneo8)]

Nice

review: Approve

[Eric Chen (eric-chen)]

Please see my inline question. thanks

review: Needs Fixing

[Eric Chen (eric-chen)]

One of my comment disappear.. weird.. I will leave it again. Please wait a moment.

[Eric Chen (eric-chen)]

Please see my inline question. thanks

review: Needs Information

[Mert Kirpici (mertkirpici)]

Hi Eric, thanks for the review. I tried to answer your questions/comments to the best of my understanding. Please see below.

[Eric Chen (eric-chen)]

LGTM, but wait for 1 more COE to review it.

review: Needs Information

[Facundo Ciccioli (fandanbango)]

This looks amazing to me. The only caveat I can think of is that a config change may implicitly ack SELs since the last ack-sel. I'd say this is very unlikely, but could happen:

If this is done within the same UTC day:
1. New SEL entries
2. Operator runs the ack-sel action
3. New SEL entires
4. A config change is made

then the new SEL entries on 3. are implicitly acked.

Then again, this would seldom happen, as whenever we receive an IPMI alert we check it and it'd be a huge coincidence if a config change is made while we are investigating (can only think of a colleague doing some unrelated work concurrently).

Still, it is unexpected and we've spotted it, hence if there's a simple way to fix it then maybe we might?

I believe if we can easily check if the code's running as part of an action then that check could be added to line 805 on tools.py:

    def install_cronjob(self, cron_user="nagios"):
        date_filter = unitdata.kv().get(self.SEL_DATE_FILTER_START_KEY)
805 -> if date_filter:
            self.add_exclude_display_range_to_cron_args(
                record_id=self._get_last_sel_record_id(date=date_filter)
            )
            hookenv.log("Added record id filter to ipmi sensors cmdline", hookenv.DEBUG)
        super().install_cronjob(cron_user)

Thoughts?

[Mert Kirpici (mertkirpici)]

Hi Facundo thanks for the review and thoughts.

> Thoughts?
This is a very good catch! Thanks for bringing this up.
As rare as this scenario might be I think if we could avoid it, we should. I am thinking around two solutions:

1 - Introducing a `force=False` parameter in the Ipmi::install_cronjob() signature . That will be checked alongside the date_filter variable that you're pointing out. Only the action code will set it to True when calling, i.e. Ipmi().install_cronjob(..., force=True). I believe this should be enough to fix this however it smells like bad design to me.

2 - Delegating the max record id getting code to the action code and storing _that_ in the unit data rather than the date. This will render everything static(nothing getting figured out during charm runtime after the action call completes). This is somewhat a bigger change(unit data key, purpose changing, etc.) however I think it is the proper way to implement this change.

I am inclined to push an update towards option 2. Is there anything I am missing here?

[Facundo Ciccioli (fandanbango)]

> Hi Facundo thanks for the review and thoughts.
>
> > Thoughts?
> This is a very good catch! Thanks for bringing this up.
> As rare as this scenario might be I think if we could avoid it, we should. I
> am thinking around two solutions:
>
> 1 - Introducing a `force=False` parameter in the Ipmi::install_cronjob()
> signature . That will be checked alongside the date_filter variable that
> you're pointing out. Only the action code will set it to True when calling,
> i.e. Ipmi().install_cronjob(..., force=True). I believe this should be enough
> to fix this however it smells like bad design to me.
>
> 2 - Delegating the max record id getting code to the action code and storing
> _that_ in the unit data rather than the date. This will render everything
> static(nothing getting figured out during charm runtime after the action call
> completes). This is somewhat a bigger change(unit data key, purpose changing,
> etc.) however I think it is the proper way to implement this change.
>
> I am inclined to push an update towards option 2. Is there anything I am
> missing here?

I'd really prefer telling you to go for option 1. as it's simpler, and I wouldn't invest time in implementing the "right" option for a charm like this one which has a death sentence in the wake of the COS.

*But*, I believe option 1 doesn't cut, as it breaks in other ways. If a config-changed is triggered, then add_exclude_display_range_to_cron_args() won't be executed (if I understood correctly) and hence the filter won't be rendered to the cron file. This effectively unacks all SEL entries.

Option 2. is, like you say, the way to go. We only care about the record ID up to which we're ignoring, and the action code is the only one which should mess around with it.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:9e8b0d0adc10184830e7b4503d73d40680a9f0f7
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/74/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-unit/317/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1571/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/74//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:2dff736e15bc21b25ae429d0f1dfb66947842cec
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/75/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-unit/318/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1583/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/75//rebuild

review: Approve (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:188386ec4c31120ab47f1bf35f20db658984c4cb
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/76/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-unit/319/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1585/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/76//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:612a1bd1ee05ce9139aafdd7d86fcf6318eed80e
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/77/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-unit/320/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1586/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-hw-health-ci/77//rebuild

review: Approve (continuous-integration)

[Facundo Ciccioli (fandanbango)]

Looks good! Too bad about the tests going away, the alternatives I can think of are probably worse than not having tests...

review: Approve

[Eric Chen (eric-chen)]

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision cdc5bce8035b5cfceafda258e676bb0119ef29f8

diff --git a/src/actions/actions.py b/src/actions/actions.py
index 62b2b50..6f1e342 100755
--- a/src/actions/actions.py
+++ b/src/actions/actions.py
@@ -59,9 +59,9 @@ def show_sel():
     show_all = action_get("show-all")
     command = [IPMI_SEL, "--output-event-state"]
 
-    date_start = unitdata.kv().get(tools.Ipmi.SEL_DATE_FILTER_START_KEY)
-    if date_start:
-        command.extend(["--date-range", "{}-now".format(date_start)])
+    max_record_id = unitdata.kv().get(tools.Ipmi.SEL_EXCLUDE_RANGE_MAX_KEY)
+    if max_record_id:
+        command.extend(["--exclude-display-range", "1-{}".format(max_record_id)])
 
     try:
         header, body = None, None
@@ -74,8 +74,8 @@ def show_sel():
                 body = [line for line in body if "Nominal" not in line]
         if body:
             final_output = "\n".join([header] + body)
-            if date_start:
-                final_output += "Has been date filtered: {}".format(date_start)
+            if max_record_id:
+                final_output += "Has been id filtered: {}".format(max_record_id)
         else:
             final_output = "No matching entries found"
         log("Action show-sel completed:\n{}".format(final_output))
@@ -84,12 +84,31 @@ def show_sel():
         action_fail("Action failed with {}".format(e))
 
 
+def _get_sel_max_event_id_for_date(date):
+    try:
+        output = subprocess.check_output(
+            args=[
+                IPMI_SEL,
+                "--no-header-output",
+                "--comma-separated-output",
+                "--date-range",
+                "01/01/1970-{}".format(date.strftime("%m/%d/%Y")),
+            ],
+            universal_newlines=True,
+        ).splitlines()
+        max_record_id = output[-1].split(",")[0].strip()
+    except IndexError:
+        max_record_id = None
+
+    return max_record_id
+
+
 def ack_sel():
     """Set the acknowledgement date filter."""
     message = ""
-    if "--date-range" in config("ipmi_check_options"):
+    if "--exclude-display-range" in config("ipmi_check_options"):
         message = (
-            "WARNING: Will override user specified --date-range "
+            "WARNING: Will override user specified --exclude-display-range "
             "option on ipmi_check_options config\n"
         )
 
@@ -107,12 +126,11 @@ def ack_sel():
         date = datetime.utcnow()
 
     try:
-        unitdata.kv().set(
-            tools.Ipmi.SEL_DATE_FILTER_START_KEY, date.strftime("%m/%d/%Y")
-        )
+        max_record_id = _get_sel_max_event_id_for_date(date)
+        unitdata.kv().set(tools.Ipmi.SEL_EXCLUDE_RANGE_MAX_KEY, max_record_id)
         unitdata.kv().flush()
-        message += "Only SEL entries newer than {} will be considered".format(
-            date.strftime("%Y-%m-%d")
+        message += "Only SEL entries with id greater than {} will be considered".format(
+            max_record_id
         )
         action_set({"message": message})
         tools.Ipmi().install_cronjob()
@@ -123,7 +141,7 @@ def ack_sel():
 def unack_sel():
     """Unset the acknowledgement date filter."""
     try:
-        unitdata.kv().unset(tools.Ipmi.SEL_DATE_FILTER_START_KEY)
+        unitdata.kv().unset(tools.Ipmi.SEL_EXCLUDE_RANGE_MAX_KEY)
         unitdata.kv().flush()
         message = "SEL entries from all times will be considered"
         action_set({"message": message})
diff --git a/src/lib/hwhealth/tools.py b/src/lib/hwhealth/tools.py
index 71273d6..3a3e8b4 100644
--- a/src/lib/hwhealth/tools.py
+++ b/src/lib/hwhealth/tools.py
@@ -686,7 +686,7 @@ class Ipmi(Tool):
     OUTPUT_FILE = "/var/lib/nagios/ipmi_sensors.out"
     EXCLUDE_FILE = "/var/lib/nagios/ipmi_exclude"
     SEL_EXCLUDE_FILE = "/var/lib/nagios/sel_exclude"
-    SEL_DATE_FILTER_START_KEY = "ipmi_sel_date_filter_start"
+    SEL_EXCLUDE_RANGE_MAX_KEY = "ipmi_exclude_range_max"
 
     def __init__(self, nrpe_opts=""):
         self.set_exclusions(
@@ -737,7 +737,13 @@ class Ipmi(Tool):
                 perms=0o444,
             )
 
-    def add_date_range_to_cron_args(self, date):
+    def add_exclude_display_range_to_cron_args(self, max_record_id):
+        """Add exclude display range option to the check script.
+
+        This function is used during the "ack-sel" action workflow.
+        The idea is to add --exclude-display-range=1-{last_record_id}
+        to silence(or "ack") the SEL entries up to a certain point.
+        """
         seloptions_argument = ""
         split_cron_args = self._cron_script_args.split()
         if "--seloptions" in split_cron_args:
@@ -745,15 +751,15 @@ class Ipmi(Tool):
             #       init code is adding the --sexclude option.
             seloptions_argument_index = split_cron_args.index("--seloptions") + 1
             seloptions_argument = split_cron_args[seloptions_argument_index]
-            # This could happen if the user specified the date range option
+            # This could happen if the user specified the exclude display range option
             # through the ipmi_check_options config. We override it (the action
             # already warned the user about this).
-            if "--date-range" in seloptions_argument:
+            if "--exclude-display-range" in seloptions_argument:
                 seloptions_argument = ",".join(
                     [
                         x
                         for x in seloptions_argument.split(",")
-                        if "--date-range" not in x
+                        if "--exclude-display-range" not in x
                     ]
                 )
             if seloptions_argument:
@@ -761,15 +767,15 @@ class Ipmi(Tool):
         else:
             split_cron_args.extend(["--seloptions", ""])
             seloptions_argument_index = len(split_cron_args) - 1
-        seloptions_argument += "--date-range=" + date + "-now"
+        seloptions_argument += "--exclude-display-range=1-" + max_record_id
         split_cron_args[seloptions_argument_index] = seloptions_argument
         self._cron_script_args = " ".join(split_cron_args)
 
     def install_cronjob(self, cron_user="nagios"):
-        date_filter = unitdata.kv().get(self.SEL_DATE_FILTER_START_KEY)
-        if date_filter:
-            self.add_date_range_to_cron_args(date_filter)
-            hookenv.log("Added date filter to ipmi sensors cmdline", hookenv.DEBUG)
+        max_record_id = unitdata.kv().get(self.SEL_EXCLUDE_RANGE_MAX_KEY)
+        if max_record_id:
+            self.add_exclude_display_range_to_cron_args(max_record_id=max_record_id)
+            hookenv.log("Added record id filter to ipmi sensors cmdline", hookenv.DEBUG)
         super().install_cronjob(cron_user)
 
     def configure_nrpe_check(self, nrpe_setup):
diff --git a/src/tests/functional/test_hwhealth.py b/src/tests/functional/test_hwhealth.py
index 98e497a..2951b7a 100644
--- a/src/tests/functional/test_hwhealth.py
+++ b/src/tests/functional/test_hwhealth.py
@@ -2,7 +2,6 @@ import asyncio
 import os
 import subprocess
 import sys
-from datetime import datetime
 from os.path import abspath, dirname
 
 sys.path.append("lib")
@@ -444,103 +443,6 @@ async def test_imports(
             )
 
 
-IPMI_CROND_FILE = os.path.join(tools.Ipmi.CROND_DIR, "hwhealth_ipmi")
-
-
-async def test_action_ack_sel(deployed_unit, run_command):
-    action = await deployed_unit.run_action("ack-sel")
-    action = await action.wait()
-
-    assert action.status == "completed", "Action did not complete successfully"
-
-    # NOTE: This test might fail if run very very close to UTC midnight... I'm
-    # purposedly not contemplating that case in favor of test simplicity.
-    today = datetime.utcnow().strftime("%m/%d/%Y")
-    cmd = "grep -E '[ ,]--date-range={}-now([ ,]|$)' {}".format(today, IPMI_CROND_FILE)
-    result = await run_command(cmd, deployed_unit)
-    assert (
-        result["Code"] == "0"
-    ), "--date-range parameter not found on cron file (or it has the wrong date)"
-
-
-async def test_action_ack_sel_date_param(deployed_unit, run_command):
-    action = await deployed_unit.run_action("ack-sel", date="2021-3-9")
-    action = await action.wait()
-
-    assert action.status == "completed", "Action did not complete successfully"
-
-    cmd = "grep -E '[ ,]--date-range=03/09/2021-now([ ,]|$)' {}".format(IPMI_CROND_FILE)
-    result = await run_command(cmd, deployed_unit)
-    assert (
-        result["Code"] == "0"
-    ), "--date-range parameter not found on cron file (or it has the wrong date)"
-
-
-async def test_action_ack_sel_invalid_date_param(deployed_unit, run_command):
-    result = await run_command(
-        "TMP=`mktemp` && cp {} $TMP && echo $TMP".format(IPMI_CROND_FILE), deployed_unit
-    )
-    assert result["Code"] == "0", "Error copying original cron file: {}".format(
-        result["Stderr"]
-    )
-    original_crond = result["Stdout"].strip()
-
-    action = await deployed_unit.run_action("ack-sel", date="03/09/2021")
-    action = await action.wait()
-    assert action.status == "failed", "Action didn't fail"
-    assert (
-        "Invalid date format" in action.message
-    ), "Expected 'Invalid date format' phrase not in action message: {}".format(
-        action.message
-    )
-
-    result = await run_command(
-        "cmp {} {}".format(original_crond, IPMI_CROND_FILE), deployed_unit
-    )
-    assert result["Code"] == "0", "Cron file changed when it shouldn't have"
-
-
-async def test_action_ack_sel_warning(model, deploy_app, deployed_unit, run_command):
-    await deploy_app.set_config(
-        {"ipmi_check_options": "--seloptions --date-range=2/27/2021-now"}
-    )
-    await model.block_until(
-        lambda: deployed_unit.agent_status == "idle", timeout=DEF_TIMEOUT
-    )
-
-    action = await deployed_unit.run_action("ack-sel", date="2021-3-9")
-    action = await action.wait()
-
-    assert action.status == "completed", "Action did not complete successfully"
-
-    message = action.results["message"]
-    # NOTE: We don't check the actual date range value, that's covered on the
-    # unit tests and tests above. We just wanna see if the warning's there.
-    assert (
-        "WARNING: Will override" in message
-    ), "Warning about overriding user config is not there: {}".format(message)
-
-
-async def test_action_unack_sel(model, deploy_app, deployed_unit, run_command):
-    await deploy_app.set_config({"ipmi_check_options": ""})
-    await model.block_until(
-        lambda: deployed_unit.agent_status == "idle", timeout=DEF_TIMEOUT
-    )
-
-    cmd = "grep -Fe '--date-range' {}".format(IPMI_CROND_FILE)
-    result = await run_command(cmd, deployed_unit)
-    assert result["Code"] == "0", "Test expected --date-range to be there but it isn't"
-
-    action = await deployed_unit.run_action("unack-sel")
-    action = await action.wait()
-
-    assert action.status == "completed", "Action did not complete successfully"
-
-    cmd = "grep -Fe '--date-range' {}".format(IPMI_CROND_FILE)
-    result = await run_command(cmd, deployed_unit)
-    assert result["Code"] == "1", "--date-range parameter is still there"
-
-
 async def test_removal(monkeypatch, toolset, model, deploy_app, file_stat):
     """Remove the unit, test that all files have been cleaned up."""
     hw_health_app_name = deploy_app.name
diff --git a/src/tests/unit/test_actions.py b/src/tests/unit/test_actions.py
index 15ee573..7c919ec 100644
--- a/src/tests/unit/test_actions.py
+++ b/src/tests/unit/test_actions.py
@@ -26,6 +26,8 @@ from actions.actions import (  # noqa:E402
     _run_job_file,
     run_cron_action,
     show_sel,
+    ack_sel,
+    _get_sel_max_event_id_for_date,
 )
 
 
@@ -295,3 +297,93 @@ class ListCronTestCase(unittest.TestCase):
         mock_run_job_file.assert_called_once_with(
             pathlib.Path("/etc/cron.d/hwhealth_test")
         )
+
+
+class AckSelTestCase(unittest.TestCase):
+    @mock.patch("subprocess.check_output")
+    def test_get_sel_max_event_id_for_date(self, mock_check_output):
+        mock_date = mock.MagicMock()
+        mock_date.strftime.return_value = "01/01/2022"
+        mock_check_output.return_value = (
+            "1,May-06-2011,20:48:43,Sensor #10,Power Supply,Critical,Power Supply Failure detected\n"  # noqa
+            "2,Sep-21-2013,14:05:03,Sensor #7,Power Supply,Critical,Power Supply Failure detected\n"  # noqa
+            "3,Sep-21-2013,14:05:03,Sensor #10,Power Supply,Critical,Power Supply Failure detected\n"  # noqa
+        )
+
+        ret = _get_sel_max_event_id_for_date(mock_date)
+
+        self.assertEqual(ret, "3")
+        mock_check_output.assert_called_once_with(
+            args=[
+                "/usr/sbin/ipmi-sel",
+                "--no-header-output",
+                "--comma-separated-output",
+                "--date-range",
+                "01/01/1970-01/01/2022",
+            ],
+            universal_newlines=True,
+        )
+
+    @mock.patch("subprocess.check_output")
+    def test_get_sel_max_event_id_for_date_returns_none(self, mock_check_output):
+        mock_date = mock.MagicMock()
+        mock_date.strftime.return_value = "01/01/2022"
+        mock_check_output.return_value = ""
+
+        ret = _get_sel_max_event_id_for_date(mock_date)
+
+        self.assertEqual(ret, None)
+        mock_check_output.assert_called_once_with(
+            args=[
+                "/usr/sbin/ipmi-sel",
+                "--no-header-output",
+                "--comma-separated-output",
+                "--date-range",
+                "01/01/1970-01/01/2022",
+            ],
+            universal_newlines=True,
+        )
+
+    @mock.patch("actions.actions._get_sel_max_event_id_for_date")
+    @mock.patch("hwhealth.tools.Ipmi")
+    @mock.patch("actions.actions.unitdata.kv")
+    @mock.patch("actions.actions.config")
+    @mock.patch("actions.actions.action_set")
+    @mock.patch("actions.actions.action_get")
+    def test_ack_sel(
+        self,
+        mock_action_get,
+        mock_action_set,
+        mock_config,
+        mock_kv,
+        mock_ipmi,
+        mock_get_sel_max_event_id_for_date,
+    ):
+        mock_action_get.return_value = "2022-01-01"
+        mock_config.return_value = ""
+        mock_get_sel_max_event_id_for_date.return_value = "42"
+
+        ack_sel()
+
+        mock_action_get.assert_called_once_with("date")
+        mock_kv.return_value.set.assert_called_once_with(
+            mock_ipmi.SEL_EXCLUDE_RANGE_MAX_KEY, "42"
+        )
+        mock_kv.return_value.flush.assert_called_once()
+        mock_ipmi.return_value.install_cronjob.assert_called_once()
+        mock_action_set.assert_called_once()
+
+    @mock.patch("actions.actions.config")
+    @mock.patch("actions.actions.action_fail")
+    @mock.patch("actions.actions.action_get")
+    def test_ack_sel_fails_on_bad_date(
+        self, mock_action_get, mock_action_fail, mock_config
+    ):
+        mock_action_get.return_value = "foo"
+        mock_config.return_value = ""
+
+        ret = ack_sel()
+
+        mock_action_get.assert_called_once_with("date")
+        mock_action_fail.assert_called_once()
+        self.assertEqual(ret, None)
diff --git a/src/tests/unit/test_ipmi_sensor.py b/src/tests/unit/test_ipmi_sensor.py
index 3048261..5611cdf 100644
--- a/src/tests/unit/test_ipmi_sensor.py
+++ b/src/tests/unit/test_ipmi_sensor.py
@@ -48,19 +48,19 @@ class TestIpmiSensor(unittest.TestCase):
     )
     @mock.patch("hwhealth.tools.write_file")
     @mock.patch("hwhealth.tools.hookenv.config")
-    def test_add_date_range_to_cron_args(self, config, *rest_of_mocks):
+    def test_add_exclude_display_range_to_cron_args(self, config, *rest_of_mocks):
         subtests = [  # (ipmi_check_options, expected _cron_script_args)
             (
                 "--option1 --option2 arg1",
                 "--option1 --option2 arg1 --sexclude {} --selexclude {} "
-                "--seloptions --date-range=03/08/2021-now".format(
+                "--seloptions --exclude-display-range=1-42".format(
                     tools.Ipmi.EXCLUDE_FILE, tools.Ipmi.SEL_EXCLUDE_FILE
                 ),
             ),
             (
                 "--option1 --option2 arg1 --seloptions --some-option",
                 "--option1 --option2 arg1 --seloptions "
-                "--some-option,--date-range=03/08/2021-now --sexclude {} "
+                "--some-option,--exclude-display-range=1-42 --sexclude {} "
                 "--selexclude {}".format(
                     tools.Ipmi.EXCLUDE_FILE, tools.Ipmi.SEL_EXCLUDE_FILE
                 ),
@@ -68,47 +68,47 @@ class TestIpmiSensor(unittest.TestCase):
             (
                 "--option1 --option2 arg1 --seloptions --some-option --option3",
                 "--option1 --option2 arg1 --seloptions "
-                "--some-option,--date-range=03/08/2021-now --option3 "
+                "--some-option,--exclude-display-range=1-42 --option3 "
                 "--sexclude {} --selexclude {}".format(
                     tools.Ipmi.EXCLUDE_FILE, tools.Ipmi.SEL_EXCLUDE_FILE
                 ),
             ),
             (
                 "--seloptions --some-option --option1 --option2 arg1",
-                "--seloptions --some-option,--date-range=03/08/2021-now "
+                "--seloptions --some-option,--exclude-display-range=1-42 "
                 "--option1 --option2 arg1 --sexclude {} --selexclude {}".format(
                     tools.Ipmi.EXCLUDE_FILE, tools.Ipmi.SEL_EXCLUDE_FILE
                 ),
             ),
             # Pre-existing date range options are overriden
             (
-                "--seloptions --some-option,--date-range=09/01/2021 "
+                "--seloptions --some-option,--exclude-display-range=1-2 "
                 "--option1 --option2 arg1",
-                "--seloptions --some-option,--date-range=03/08/2021-now "
+                "--seloptions --some-option,--exclude-display-range=1-42 "
                 "--option1 --option2 arg1 --sexclude {} --selexclude {}".format(
                     tools.Ipmi.EXCLUDE_FILE, tools.Ipmi.SEL_EXCLUDE_FILE
                 ),
             ),
             (
-                "--seloptions --date-range=09/01/2021,--some-option "
+                "--seloptions --exclude-display-range=1-2,--some-option "
                 "--option1 --option2 arg1",
-                "--seloptions --some-option,--date-range=03/08/2021-now "
+                "--seloptions --some-option,--exclude-display-range=1-42 "
                 "--option1 --option2 arg1 --sexclude {} --selexclude {}".format(
                     tools.Ipmi.EXCLUDE_FILE, tools.Ipmi.SEL_EXCLUDE_FILE
                 ),
             ),
             (
-                "--seloptions --some-option,--date-range=09/01/2021,"
+                "--seloptions --some-option,--exclude-display-range=1-2,"
                 "--some-option2 --option1 --option2 arg1",
                 "--seloptions "
-                "--some-option,--some-option2,--date-range=03/08/2021-now "
+                "--some-option,--some-option2,--exclude-display-range=1-42 "
                 "--option1 --option2 arg1 --sexclude {} --selexclude {}".format(
                     tools.Ipmi.EXCLUDE_FILE, tools.Ipmi.SEL_EXCLUDE_FILE
                 ),
             ),
             (
-                "--seloptions --date-range=09/01/2021 --option1 --option2 arg1",
-                "--seloptions --date-range=03/08/2021-now --option1 "
+                "--seloptions --exclude-display-range=1-2 --option1 --option2 arg1",
+                "--seloptions --exclude-display-range=1-42 --option1 "
                 "--option2 arg1 --sexclude {} --selexclude {}".format(
                     tools.Ipmi.EXCLUDE_FILE, tools.Ipmi.SEL_EXCLUDE_FILE
                 ),
@@ -123,5 +123,5 @@ class TestIpmiSensor(unittest.TestCase):
             config.side_effect = lambda x: config_dict[x]
 
             ipmi = tools.Ipmi()
-            ipmi.add_date_range_to_cron_args("03/08/2021")
+            ipmi.add_exclude_display_range_to_cron_args("42")
             self.assertEqual(ipmi._cron_script_args, i[1])
diff --git a/src/tox.ini b/src/tox.ini
index afa9c71..566c5dd 100644
--- a/src/tox.ini
+++ b/src/tox.ini
@@ -69,7 +69,6 @@ commands =
            --cov=reactive \
            --cov=actions \
            --cov=hooks \
-           --cov=src \
            --cov=files \
            --cov-report=term \
            --cov-report=annotate:report/annotated \