diff --git a/config.yaml b/config.yaml index 60924863..a273a583 100644 --- a/config.yaml +++ b/config.yaml @@ -21,6 +21,12 @@ options: description: | Channel to install the DCGM snap if the hardware has NVIDIA GPU. By default, it will install from latest/stable + smartctl-exporter-snap-channel: + type: string + default: "latest/stable" + description: | + Channel to install the Smartctl exporter snap if the hardware has smart disk. By default, it will install + from latest/stable. exporter-log-level: type: string default: "INFO" diff --git a/src/charm.py b/src/charm.py index 948e29b8..11900a41 100755 --- a/src/charm.py +++ b/src/charm.py @@ -12,7 +12,7 @@ from ops.framework import EventBase, StoredState from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus -from hw_tools import HWTool, HWToolHelper, detect_available_tools +from hw_tools import HWTool, HWToolHelper, detect_available_tools, remove_legacy_smartctl_exporter from service import BaseExporter, DCGMExporter, ExporterError, HardwareExporter, SmartCtlExporter logger = logging.getLogger(__name__) @@ -81,7 +81,7 @@ def exporters(self) -> List[BaseExporter]: ) if stored_tools & SmartCtlExporter.hw_tools(): - exporters.append(SmartCtlExporter(self.charm_dir, self.model.config)) + exporters.append(SmartCtlExporter(self.model.config)) if stored_tools & DCGMExporter.hw_tools(): exporters.append(DCGMExporter(self.charm_dir, self.model.config)) @@ -97,6 +97,8 @@ def get_stored_tools(self) -> Set[HWTool]: if not self._stored.stored_tools: # type: ignore[truthy-function] available_tools = detect_available_tools() # type: ignore[unreachable] self._stored.stored_tools = {tool.value for tool in available_tools} + if "smartctl" in self._stored.stored_tools: # type: ignore[operator] + self._stored.stored_tools.remove("smartctl") # type: ignore[attr-defined] return {HWTool(value) for value in self._stored.stored_tools} # type: ignore[attr-defined] def _on_redetect_hardware(self, event: ops.ActionEvent) -> None: @@ -130,6 +132,8 @@ def _on_install_or_upgrade(self, event: EventBase) -> None: """Install or upgrade charm.""" self.model.unit.status = MaintenanceStatus("Installing resources...") + remove_legacy_smartctl_exporter() + stored_tools = self.get_stored_tools() msg: str diff --git a/src/config.py b/src/config.py index 47bcd17b..a5fe935b 100644 --- a/src/config.py +++ b/src/config.py @@ -36,20 +36,6 @@ class HardwareExporterSettings(ExporterSettings): # pylint: disable = too-few-p HARDWARE_EXPORTER_SETTINGS = HardwareExporterSettings() -class SmartCtlExporterSettings(ExporterSettings): # pylint: disable = too-few-public-methods - """Constant settings for SmartCtl Exporter.""" - - name: str = "smartctl-exporter" - config_path: Path = Path(f"/etc/{name}-config.yaml") - service_path: Path = Path(f"/etc/systemd/system/{name}.service") - config_template: str = f"{name}-config.yaml.j2" - service_template: str = f"{name}.service.j2" - crash_msg: str = "SmartCtl exporter crashed unexpectedly, please refer to systemd logs..." - - -SMARTCTL_EXPORTER_SETTINGS = SmartCtlExporterSettings() - - class SystemVendor(str, Enum): """Different hardware system vendor.""" @@ -77,8 +63,7 @@ class HWTool(str, Enum): IPMI_SEL = "ipmi_sel" IPMI_SENSOR = "ipmi_sensor" REDFISH = "redfish" - SMARTCTL = "smartctl" - SMARTCTL_EXPORTER = "smartctl_exporter" + SMARTCTL_EXPORTER = "smartctl-exporter" DCGM = "dcgm" diff --git a/src/hw_tools.py b/src/hw_tools.py index 76e883a8..a3ec7bee 100644 --- a/src/hw_tools.py +++ b/src/hw_tools.py @@ -3,21 +3,19 @@ Define strategy for install, remove and verifier for different hardware. """ -import io import logging import os import shutil import stat import subprocess -import tarfile from abc import ABCMeta, abstractmethod -from http import HTTPStatus from pathlib import Path from typing import Dict, List, Set, Tuple import requests import urllib3 from charms.operator_libs_linux.v0 import apt +from charms.operator_libs_linux.v1 import systemd from charms.operator_libs_linux.v2 import snap from ops.model import ModelError, Resources @@ -63,14 +61,6 @@ def __init__(self, tool: HWTool, path: Path): self.message = f"Tool: {tool} path: {path} size is zero" -class ResourceInstallationError(Exception): - """Exception raised when a hardware tool installation fails.""" - - def __init__(self, tool: HWTool): - """Init.""" - super().__init__(f"Installation failed for tool: {tool}") - - def copy_to_snap_common_bin(source: Path, filename: str) -> None: """Copy file to $SNAP_COMMON/bin folder.""" Path(f"{SNAP_COMMON}/bin").mkdir(parents=False, exist_ok=True) @@ -239,6 +229,16 @@ def __init__(self, channel: str) -> None: self.channel = channel +class SmartCtlExporterStrategy(SnapStrategy): + """SmartCtl strategy class.""" + + _name = HWTool.SMARTCTL_EXPORTER + + def __init__(self, channel: str) -> None: + """Init.""" + self.channel = channel + + class StorCLIStrategy(TPRStrategyABC): """Strategy to install storcli.""" @@ -444,75 +444,6 @@ def check(self) -> bool: return True -class SmartCtlStrategy(APTStrategyABC): - """Strategy for installing ipmi.""" - - pkg = "smartmontools" - _name = HWTool.SMARTCTL - - def install(self) -> None: - apt_helpers.add_pkg_with_candidate_version(self.pkg) - - def remove(self) -> None: - # Skip removing because this may cause dependency error - # for other services on the same machine. - logger.info("%s skip removing %s", self._name, self.pkg) - - def check(self) -> bool: - """Check package status.""" - return check_deb_pkg_installed(self.pkg) - - -class SmartCtlExporterStrategy(StrategyABC): # pylint: disable=R0903 - """Install smartctl exporter binary.""" - - _name = HWTool.SMARTCTL_EXPORTER - - _resource_dir = Path("/opt/SmartCtlExporter/") - _release = ( - "https://github.com/prometheus-community/" - "smartctl_exporter/releases/download/v0.12.0/smartctl_exporter-0.12.0.linux-amd64.tar.gz" - ) - _exporter_name = "smartctl_exporter" - _exporter_path = Path(_resource_dir / "smartctl_exporter") - - def install(self) -> None: - """Install exporter binary from internet.""" - logger.debug("Installing SmartCtlExporter") - self._resource_dir.mkdir(parents=True, exist_ok=True) - - resp = requests.get(self._release, timeout=60) - if resp.status_code != HTTPStatus.OK: - logger.error("Failed to download smartctl exporter binary.") - raise ResourceInstallationError(self._name) - - success = False - fileobj = io.BytesIO(resp.content) - with tarfile.open(fileobj=fileobj, mode="r:gz") as tar: - for member in tar.getmembers(): - if member.name.endswith(self._exporter_name): - with open(self._exporter_path, "wb") as outfile: - member_file = tar.extractfile(member) - if member_file: - outfile.write(member_file.read()) - success = True - if success: - make_executable(self._exporter_path) - if not success: - logger.error("Failed to install SmartCtlExporter binary.") - raise ResourceInstallationError(self._name) - - def remove(self) -> None: - """Remove downloaded exporter binary.""" - logger.debug("Remove SmartCtlExporter") - shutil.rmtree(self._resource_dir) - - def check(self) -> bool: - """Check package status.""" - logger.debug("Check SmartCtlExporter resources") - return self._exporter_path.is_file() - - def _raid_hw_verifier_hwinfo() -> Set[HWTool]: """Verify if a supported RAID card exists on the machine using the hwinfo command.""" hwinfo_output = hwinfo("storage") @@ -650,7 +581,7 @@ def bmc_hw_verifier() -> Set[HWTool]: def disk_hw_verifier() -> Set[HWTool]: """Verify if the disk exists on the machine.""" - return {HWTool.SMARTCTL} if lshw(class_filter="disk") else set() + return {HWTool.SMARTCTL_EXPORTER} if lshw(class_filter="disk") else set() def nvidia_gpu_verifier() -> Set[HWTool]: @@ -664,6 +595,25 @@ def detect_available_tools() -> Set[HWTool]: return raid_hw_verifier() | bmc_hw_verifier() | disk_hw_verifier() | nvidia_gpu_verifier() +def remove_legacy_smartctl_exporter() -> None: + """Remove any legacy tool from older revision. + + Workaround for migrating legacy smartctl exporter to snap package. + """ + name = "smartctl-exporter" + smartctl_exporter = Path("opt/SmartCtlExporter/") + smartctl_exporter_config_path = Path(f"/etc/{name}-config.yaml") + smartctl_exporter_service_path = Path(f"/etc/systemd/system/{name}.service") + if smartctl_exporter_service_path.exists(): + systemd.service_stop(name) + systemd.service_disable(name) + smartctl_exporter_service_path.unlink() + if smartctl_exporter_config_path.exists(): + smartctl_exporter_config_path.unlink() + if smartctl_exporter.exists(): + shutil.rmtree("/opt/SmartCtlExporter/") + + class HWToolHelper: """Helper to install vendor's or hardware related tools.""" @@ -680,7 +630,6 @@ def strategies(self) -> List[StrategyABC]: IPMIDCMIStrategy(), IPMISENSORStrategy(), RedFishStrategy(), - SmartCtlStrategy(), ] def fetch_tools( # pylint: disable=W0102 diff --git a/src/service.py b/src/service.py index f91bc4ab..d71f4e9e 100644 --- a/src/service.py +++ b/src/service.py @@ -18,7 +18,6 @@ from config import ( HARDWARE_EXPORTER_COLLECTOR_MAPPING, HARDWARE_EXPORTER_SETTINGS, - SMARTCTL_EXPORTER_SETTINGS, ExporterSettings, HWTool, ) @@ -310,67 +309,6 @@ def remove_file(path: Path) -> bool: return success -class SmartCtlExporter(RenderableExporter): - """A class representing the smartctl exporter and the metric endpoints.""" - - required_config: bool = False - - def __init__(self, charm_dir: Path, config: ConfigData) -> None: - """Initialize the Hardware Exporter class.""" - super().__init__(charm_dir, config, SMARTCTL_EXPORTER_SETTINGS) - - self.port = int(config["smartctl-exporter-port"]) - self.collect_timeout = int(config["collect-timeout"]) - self.log_level = str(config["exporter-log-level"]) - self.strategy = SmartCtlExporterStrategy() - - def render_service(self) -> bool: - """Render required files for service.""" - service_rendered = self._render_service( - { - "PORT": str(self.port), - "LEVEL": self.log_level, - } - ) - return service_rendered - - def configure(self) -> bool: - """Override base configure to render the service file. - - This is because smartctl_exporter doesn't support providing config file. - The config options need to be provided as flags while exectuting - smartctl_exporter. So, the service file must be re-rendered when a config - value is changed. - """ - service_rendered = self.render_service() - if service_rendered: - systemd.daemon_reload() - return service_rendered - - @staticmethod - def hw_tools() -> Set[HWTool]: - """Return hardware tools to watch.""" - return {HWTool.SMARTCTL} - - def install_resources(self) -> bool: - restart = False - if self.check_active(): - systemd.service_stop(self.exporter_name) - restart = True - self.strategy.install() - if restart: - systemd.service_restart(self.exporter_name) - logger.debug("Finish install resources for %s", self.exporter_name) - return True - - def resources_exist(self) -> bool: - return self.strategy.check() - - def remove_resources(self) -> bool: - self.strategy.remove() - return True - - class SnapExporter(BaseExporter): """A class representing a snap exporter.""" @@ -382,7 +320,11 @@ class SnapExporter(BaseExporter): def __init__(self, config: ConfigData): """Init.""" self.config = config - self.snap_client = snap.SnapCache()[self.strategy.snap] + + @property + def snap_client(self) -> snap.Snap: + """Return the snap client.""" + return snap.SnapCache()[self.strategy.snap] @staticmethod def hw_tools() -> Set[HWTool]: @@ -396,7 +338,6 @@ def install(self) -> bool: """ try: self.strategy.install() - # dcgm-exporter is disabled by default self.enable_and_start() return self.snap_client.present is True except Exception: # pylint: disable=broad-except @@ -430,6 +371,18 @@ def restart(self) -> None: """Restart the exporter daemon.""" self.snap_client.restart(reload=True) + def set(self, snap_config: dict) -> bool: + """Set config options for the snap service. + + Return true if successfully updated snap config, otherwise false. + """ + try: + self.snap_client.set(snap_config, typed=True) + except snap.SnapError as err: + logger.error("Failed to update snap configs %s: %s", self.strategy.snap, err) + return False + return True + def check_health(self) -> bool: """Check if all services are active. @@ -483,6 +436,33 @@ def hw_tools() -> Set[HWTool]: return {HWTool.DCGM} +class SmartCtlExporter(SnapExporter): + """A class representing the smartctl exporter and the metric endpoints.""" + + exporter_name: str = "smartctl-exporter" + + def __init__(self, config: ConfigData) -> None: + """Initialize the SmartctlExporter class.""" + self.port = int(config["smartctl-exporter-port"]) + self.log_level = str(config["exporter-log-level"]) + self.strategy = SmartCtlExporterStrategy(str(config["smartctl-exporter-snap-channel"])) + super().__init__(config) + + @staticmethod + def hw_tools() -> Set[HWTool]: + """Return hardware tools to watch.""" + return {HWTool.SMARTCTL_EXPORTER} + + def configure(self) -> bool: + """Set the necessary exporter configurations or change snap channel.""" + return super().configure() and self.set( + { + "log.level": self.log_level.lower(), + "web.listen-address": f":{self.port}", + } + ) + + class HardwareExporter(RenderableExporter): """A class representing the hardware exporter and the metric endpoints.""" diff --git a/templates/smartctl-exporter.service.j2 b/templates/smartctl-exporter.service.j2 deleted file mode 100644 index ef9fb374..00000000 --- a/templates/smartctl-exporter.service.j2 +++ /dev/null @@ -1,20 +0,0 @@ -[Unit] -Description=smartctl exporter service -After=network-online.target - -[Service] -Type=simple -PIDFile=/run/smartctl_exporter.pid -ExecStart=/opt/SmartCtlExporter/smartctl_exporter --web.listen-address=:{{ PORT }} -User=root -Group=root -SyslogIdentifier=smartctl_exporter -SyslogLevel={{ LEVEL }} -Restart=on-failure -RemainAfterExit=no -RestartSec=100ms -StandardOutput=journal -StandardError=journal - -[Install] -WantedBy=multi-user.target diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 68bc3890..bb643713 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -15,13 +15,7 @@ import charm from charm import ExporterError, HardwareObserverCharm from config import HWTool -from service import ( - HARDWARE_EXPORTER_SETTINGS, - SMARTCTL_EXPORTER_SETTINGS, - DCGMExporter, - HardwareExporter, - SmartCtlExporter, -) +from service import HARDWARE_EXPORTER_SETTINGS, DCGMExporter, HardwareExporter, SmartCtlExporter class TestCharm(unittest.TestCase): @@ -72,12 +66,12 @@ def test_harness(self) -> None: ), ( "Enable two exporters", - {HWTool.IPMI_SEL, HWTool.SMARTCTL}, + {HWTool.IPMI_SEL, HWTool.SMARTCTL_EXPORTER}, {"hardware-exporter", "smartctl-exporter"}, ), ( "Enable all exporters", - {HWTool.IPMI_SEL, HWTool.SMARTCTL, HWTool.DCGM}, + {HWTool.IPMI_SEL, HWTool.SMARTCTL_EXPORTER, HWTool.DCGM}, {"hardware-exporter", "smartctl-exporter", "dcgm"}, ), ] @@ -98,7 +92,7 @@ def test_exporters( mock_hw_exporter.return_value = hw_exporter smart_exporter = mock.MagicMock() - smart_exporter.exporter_name = SMARTCTL_EXPORTER_SETTINGS.name + smart_exporter.exporter_name = SmartCtlExporter.exporter_name mock_smart_exporter.hw_tools.return_value = SmartCtlExporter.hw_tools() mock_smart_exporter.return_value = smart_exporter @@ -117,7 +111,7 @@ def test_exporters( ( "happy case", "install", - {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL}, + {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL_EXPORTER}, (True, ""), [mock.MagicMock(), mock.MagicMock()], [True, True], @@ -125,7 +119,7 @@ def test_exporters( ( "happy case", "upgrade", - {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL}, + {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL_EXPORTER}, (True, ""), [mock.MagicMock(), mock.MagicMock()], [True, True], @@ -133,7 +127,7 @@ def test_exporters( ( "missing resource", "install", - {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL}, + {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL_EXPORTER}, (False, "miss something"), [mock.MagicMock(), mock.MagicMock()], [True, True], @@ -141,7 +135,7 @@ def test_exporters( ( "missing resource", "upgrade", - {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL}, + {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL_EXPORTER}, (False, "miss something"), [mock.MagicMock(), mock.MagicMock()], [True, True], @@ -149,7 +143,7 @@ def test_exporters( ( "Exporter install fail", "install", - {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL}, + {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL_EXPORTER}, (True, ""), [mock.MagicMock(), mock.MagicMock()], [False, True], @@ -157,7 +151,7 @@ def test_exporters( ( "Exporter install fail", "upgrade", - {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL}, + {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL_EXPORTER}, (True, ""), [mock.MagicMock(), mock.MagicMock()], [False, True], @@ -227,14 +221,14 @@ def test_remove(self): self.harness.charm.get_stored_tools.return_value = { HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, - HWTool.SMARTCTL, + HWTool.SMARTCTL_EXPORTER, } self.harness.charm.on.remove.emit() self.harness.charm.hw_tool_helper.remove.assert_called_with( self.harness.charm.model.resources, - {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL}, + {HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.SMARTCTL_EXPORTER}, ) for mock_exporter in mock_exporters: mock_exporter.uninstall.assert_called() @@ -792,3 +786,8 @@ def test_validate_configs( self.harness.begin() result = self.harness.charm.validate_configs() self.assertEqual(result, expect) + + def test_get_stored_tools_remove_legacy_smartctl(self): + self.harness.begin() + self.harness.charm._stored.stored_tools = {"smartctl"} + assert self.harness.charm.get_stored_tools() == set() diff --git a/tests/unit/test_hw_tools.py b/tests/unit/test_hw_tools.py index 4e51fba8..0def3264 100644 --- a/tests/unit/test_hw_tools.py +++ b/tests/unit/test_hw_tools.py @@ -1,8 +1,6 @@ import stat import subprocess -import tempfile import unittest -from http import HTTPStatus from pathlib import Path from unittest import mock @@ -31,11 +29,8 @@ PercCLIStrategy, ResourceChecksumError, ResourceFileSizeZeroError, - ResourceInstallationError, SAS2IRCUStrategy, SAS3IRCUStrategy, - SmartCtlExporterStrategy, - SmartCtlStrategy, SnapStrategy, SSACLIStrategy, StorCLIStrategy, @@ -55,6 +50,7 @@ raid_hw_verifier, redfish_available, remove_deb, + remove_legacy_smartctl_exporter, symlink, ) from keys import HP_KEYS @@ -729,126 +725,6 @@ def test_remove(self, mock_apt): mock_apt.remove_package.assert_not_called() -class TestSmartCtlStrategy(unittest.TestCase): - @mock.patch("apt_helpers.get_candidate_version") - @mock.patch("apt_helpers.apt") - def test_install(self, mock_apt, mock_candidate_version): - strategy = SmartCtlStrategy() - mock_candidate_version.return_value = "some-candidate-version" - strategy.install() - - mock_apt.add_package.assert_called_with( - "smartmontools", version="some-candidate-version", update_cache=False - ) - - @mock.patch("hw_tools.apt") - def test_remove(self, mock_apt): - strategy = SmartCtlStrategy() - strategy.remove() - - mock_apt.remove_package.assert_not_called() - - @mock.patch("hw_tools.check_deb_pkg_installed") - def test_check(self, mock_check_deb_method): - strategy = SmartCtlStrategy() - strategy.check() - - mock_check_deb_method.assert_called_with("smartmontools") - - -class TestSmartCtlExporterStrategy(unittest.TestCase): - def setUp(self): - self.temp_dir = tempfile.TemporaryDirectory() - self.tmp_path = Path(self.temp_dir.name) - - def tearDown(self): - self.temp_dir.cleanup() - - @mock.patch("hw_tools.requests.get") - @mock.patch("hw_tools.tarfile.open") - @mock.patch("hw_tools.make_executable") - def test_install_success( - self, - mock_make_executable, - mock_tar_open, - mock_requests_get, - ): - strategy = SmartCtlExporterStrategy() - strategy._resource_dir = self.tmp_path - strategy._exporter_path = self.tmp_path / "smartctl_exporter" - - mock_response = mock.MagicMock(status_code=HTTPStatus.OK) - mock_response.content = b"dummy content" - mock_requests_get.return_value = mock_response - mock_member = mock.MagicMock(name="member") - mock_member.name = "smartctl_exporter" - mock_member_file = mock.MagicMock() - mock_member_file.read.return_value = b"dummy content" - mock_tar_open.return_value.__enter__.return_value.getmembers.return_value = [mock_member] - mock_tar_open.return_value.__enter__.return_value.extractfile.return_value = ( - mock_member_file # noqa: E501 - ) - - strategy.install() - - mock_requests_get.assert_called_with(strategy._release, timeout=60) - # mock_tar_open.assert_called_with(fileobj=BytesIO(b"dummy content"), mode="r:gz") - mock_make_executable.assert_called_with(strategy._exporter_path) - self.assertTrue(strategy._resource_dir.exists()) - - @mock.patch("hw_tools.requests.get") - def test_install_download_failure(self, mock_requests_get): - strategy = SmartCtlExporterStrategy() - strategy._resource_dir = self.tmp_path - strategy._exporter_path = self.tmp_path / "smartctl_exporter" - - mock_response = mock.MagicMock(status_code=HTTPStatus.NOT_FOUND) - mock_requests_get.return_value = mock_response - - with self.assertRaises(ResourceInstallationError): - strategy.install() - - @mock.patch("hw_tools.requests.get") - @mock.patch("hw_tools.tarfile.open") - def test_install_parse_failure(self, mock_tar_open, mock_requests_get): - strategy = SmartCtlExporterStrategy() - strategy._resource_dir = self.tmp_path - strategy._exporter_path = self.tmp_path / "smartctl_exporter" - - mock_response = mock.MagicMock(status_code=HTTPStatus.OK) - mock_response.content = b"dummy content" - mock_requests_get.return_value = mock_response - mock_member = mock.MagicMock(name="member") - mock_member.name = "random name" - mock_member_file = mock.MagicMock() - mock_member_file.read.return_value = b"dummy content" - mock_tar_open.return_value.__enter__.return_value.getmembers.return_value = [mock_member] - mock_tar_open.return_value.__enter__.return_value.extractfile.return_value = ( - mock_member_file # noqa: E501 - ) - - with self.assertRaises(ResourceInstallationError): - strategy.install() - - @mock.patch("hw_tools.shutil.rmtree") - def test_remove(self, mock_shutil_rmtree): - strategy = SmartCtlExporterStrategy() - - strategy.remove() - - mock_shutil_rmtree.assert_called_with(strategy._resource_dir) - - def test_check(self): - strategy = SmartCtlExporterStrategy() - strategy._exporter_path = mock.MagicMock() - strategy._exporter_path.is_file.return_value = True - - result = strategy.check() - self.assertTrue(result) - - strategy._exporter_path.is_file.assert_called() - - @mock.patch("hw_tools.disk_hw_verifier", return_value={7, 8, 9}) @mock.patch("hw_tools.bmc_hw_verifier", return_value={1, 2, 3}) @mock.patch("hw_tools.raid_hw_verifier", return_value={4, 5, 6}) @@ -982,7 +858,7 @@ class TestDiskHWVerifier(unittest.TestCase): @mock.patch("hw_tools.lshw", return_value=[True]) def test_disk_available(self, mock_lshw): tools = disk_hw_verifier() - self.assertEqual(tools, {HWTool.SMARTCTL}) + self.assertEqual(tools, {HWTool.SMARTCTL_EXPORTER}) @mock.patch("hw_tools.lshw", return_value=[]) def test_disk_not_available(self, mock_lshw): @@ -1265,3 +1141,36 @@ def test_snap_strategy_check(snap_exporter, mock_snap_lib, services, expected): mock_snap_lib.SnapCache.return_value = {"my-snap": mock_snap_client} assert snap_exporter.check() is expected + + +@mock.patch("hw_tools.Path.unlink") +@mock.patch("hw_tools.Path.exists") +@mock.patch("hw_tools.shutil") +@mock.patch("hw_tools.systemd") +def test_remove_legacy_smartctl_exporter_exist( + mock_systemd, mock_shutil, mock_path_exists, mock_path_unlink +): + mock_path_exists.return_value = True + remove_legacy_smartctl_exporter() + + mock_systemd.service_stop.assert_called_once() + mock_systemd.service_disable.assert_called_once() + mock_path_unlink.assert_called() + assert mock_path_unlink.call_count == 2 + mock_shutil.rmtree.assert_called_once() + + +@mock.patch("hw_tools.Path.unlink") +@mock.patch("hw_tools.Path.exists") +@mock.patch("hw_tools.shutil") +@mock.patch("hw_tools.systemd") +def test_remove_legacy_smartctl_exporter_not_exists( + mock_systemd, mock_shutil, mock_path_exists, mock_path_unlink +): + mock_path_exists.return_value = False + remove_legacy_smartctl_exporter() + + mock_systemd.service_stop.assert_not_called() + mock_systemd.service_disable.assert_not_called() + mock_path_unlink.assert_not_called() + mock_shutil.rmtree.assert_not_called() diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 972fc7ea..0d45045a 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -8,6 +8,7 @@ import pytest import yaml +from charms.operator_libs_linux.v2 import snap from parameterized import parameterized from redfish.rest.v1 import InvalidCredentialsError @@ -1018,6 +1019,19 @@ def test_snap_exporter_restart(snap_exporter): snap_exporter.snap_client.restart.assert_called_once_with(reload=True) +def test_snap_exporter_set(snap_exporter): + snap_config = {} + assert snap_exporter.set(snap_config) is True + snap_exporter.snap_client.set.assert_called_once_with(snap_config, typed=True) + + +def test_snap_exporter_set_failed(snap_exporter): + snap_config = {} + snap_exporter.snap_client.set.side_effect = snap.SnapError() + assert snap_exporter.set(snap_config) is False + snap_exporter.snap_client.set.assert_called_once_with(snap_config, typed=True) + + def test_snap_exporter_check_health(snap_exporter): snap_exporter.check_health() snap_exporter.strategy.check.assert_called_once() @@ -1032,5 +1046,23 @@ def test_snap_exporter_configure(mock_install, snap_exporter, install_result, ex mock_install.assert_called_once() +@pytest.mark.parametrize("result, expected_result", [(True, True), (False, False)]) +@mock.patch("service.SnapExporter.install") +@mock.patch("service.SnapExporter.set") +def test_smartctl_exporter_configure(mock_set, mock_install, result, expected_result): + mock_config = { + "smartctl-exporter-port": "10000", + "exporter-log-level": "info", + "smartctl-exporter-snap-channel": "latest/stable", + } + + mock_set.return_value = result + mock_install.return_value = result + exporter = service.SmartCtlExporter(mock_config) + assert exporter.exporter_name == "smartctl-exporter" + assert exporter.hw_tools() == {HWTool.SMARTCTL_EXPORTER} + assert exporter.configure() is expected_result + + if __name__ == "__main__": unittest.main()