canonical / hardware-observer-operator

A charm to setup prometheus exporter for IPMI, RedFish and RAID devices from different vendors.
Apache License 2.0
7 stars 14 forks source link

When to install exporter service required dependency/resource? #233

Open jneo8 opened 2 months ago

jneo8 commented 2 months ago

On branch /dev/refactor-multiple-exporter-support

The current install function of BaseExporter class is:

class BaseExporter:
    def install(self) -> bool:
        """Install the exporter."""
        logger.info("Installing %s.", self.exporter_name)
        config_success = self.render_config()
        service_success = self.render_service(str(self.charm_dir), str(self.exporter_config_path))
        if not (config_success and service_success):
            logger.error("Failed to install %s.", self.exporter_name)
            return False
        systemd.daemon_reload()

        # Verify installed
        if not (self.exporter_config_path.exists() and self.exporter_service_path.exists()):
            logger.error(f"{self.exporter_name} is not installed properly.")
            return False

        logger.info("%s installed.", self.exporter_name)
        return True

and the timing to trigger this install function is on


def get_exporter(exporter: BaseExporter):
    """Install and get an exporter"""
    if exporter.exporter_service_path.exists():
        return exporter
    installed = exporter.install()
    if not installed:
        logger.error(f"Failed to install {exporter.exporter_name}")
        return
    return exporter

class HardwareObserverCharm(ops.CharmBase):
    def __init__(self, *args: Any) -> None:
        ...
        self.exporters = []

        hardware_exporter = exporter_helpers.get_exporter(
            HardwareExporter(self.charm_dir, self.model.config)
        )
        if hardware_exporter:
            self.exporters.append(hardware_exporter)
        ...

The logic only include the systemd service config file generate, which is because we hide the prometheus-hardware-exporter dependency in charm's packaging, which is working fine.

But this become a issue when now we need to include more exporter services which are not a python package. There is no hook we can use to handle the install process of the required resource/dependency for exporter.

chanchiwai-ray commented 2 months ago

I suggest we split the get_exporter into get_installed_exporters (for getting the installed exporters instances, used in places other than install even) and install_exporters (for installation only, used in install or upgrade events) functions