Merge ~al3jandrosg/charm-telegraf:LP1976521 into charm-telegraf:master

[Alejandro Santoyo Gonzalez]

Adding support for intel_powerstat

This patch adds support for intel_powerstat by:

- fixing the permissions issues when attempting to read
`/sys/devices/virtual/powercap/intel-rapl/`
- adding charm configs: `collect_intel_powerstat_metrics`,
`package_metrics`, `cpu_metrics`

Fixes [LP#1976521](https://bugs.launchpad.net/charm-telegraf/+bug/1976521)

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

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

[Alejandro Santoyo Gonzalez (al3jandrosg)]

Lint tests failing but that has nothing to do with the patch:

Lint tests without patch applied:

~/MyData/Projects/charm-telegraf-lp-fork$ make lint
Running lint checks
lint: install_deps> python -I -m pip install black colorama flake8 flake8-colors flake8-docstrings isort pep8-naming pyproject-flake8
lint: commands[0]> pflake8
lint: commands[1]> black --check --diff --color .
--- /home/alejandro/MyData/Projects/charm-telegraf-lp-fork/src/tests/unit/test_telegraf.py 2024-02-08 12:03:03.378682+00:00
+++ /home/alejandro/MyData/Projects/charm-telegraf-lp-fork/src/tests/unit/test_telegraf.py 2024-02-08 12:08:34.248687+00:00
@@ -1736,12 +1736,11 @@
         mock_nrpe.assert_has_calls(calls)

 class TestGrafanaDashboard:
     @patch("reactive.telegraf.endpoint_from_flag")
- def test_register_grafana_dashboard_n(self, mock_endpoint_from_flag):
- ...
+ def test_register_grafana_dashboard_n(self, mock_endpoint_from_flag): ...

     @patch("reactive.telegraf.render_custom")
     @patch("reactive.telegraf.endpoint_from_flag")
     @patch("reactive.telegraf.hookenv")
     @patch("reactive.telegraf.json")
would reformat /home/alejandro/MyData/Projects/charm-telegraf-lp-fork/src/tests/unit/test_telegraf.py

Oh no! 💥 💔 💥
1 file would be reformatted, 15 files would be left unchanged.
lint: exit 1 (0.81 seconds) /home/alejandro/MyData/Projects/charm-telegraf-lp-fork/src> black --check --diff --color . pid=2237953
  lint: FAIL code 1 (3.60=setup[2.39]+cmd[0.40,0.81] seconds)
  evaluation failed :( (3.66 seconds)
make: *** [Makefile:75: lint] Error 1

after the patch was applied:

alejandro@alejandropc:~/MyData/Projects/charm-telegraf-lp-fork$ make lint
Running lint checks
lint: commands[0]> pflake8
lint: commands[1]> black --check --diff --color .
--- /home/alejandro/MyData/Projects/charm-telegraf-lp-fork/src/tests/unit/test_telegraf.py 2024-02-08 12:12:45.928446+00:00
+++ /home/alejandro/MyData/Projects/charm-telegraf-lp-fork/src/tests/unit/test_telegraf.py 2024-02-08 12:14:23.682646+00:00
@@ -1736,12 +1736,11 @@
         mock_nrpe.assert_has_calls(calls)

 class TestGrafanaDashboard:
     @patch("reactive.telegraf.endpoint_from_flag")
- def test_register_grafana_dashboard_n(self, mock_endpoint_from_flag):
- ...
+ def test_register_grafana_dashboard_n(self, mock_endpoint_from_flag): ...

     @patch("reactive.telegraf.render_custom")
     @patch("reactive.telegraf.endpoint_from_flag")
     @patch("reactive.telegraf.hookenv")
     @patch("reactive.telegraf.json")
would reformat /home/alejandro/MyData/Projects/charm-telegraf-lp-fork/src/tests/unit/test_telegraf.py

Oh no! 💥 💔 💥
1 file would be reformatted, 15 files would be left unchanged.
lint: exit 1 (0.83 seconds) /home/alejandro/MyData/Projects/charm-telegraf-lp-fork/src> black --check --diff --color . pid=2238896
  lint: FAIL code 1 (1.26=setup[0.03]+cmd[0.41,0.83] seconds)
  evaluation failed :( (1.30 seconds)
make: *** [Makefile:75: lint] Error 1

[Alejandro Santoyo Gonzalez (al3jandrosg)]

Lots of func test issues due to charmcraft problems. Eventually
managed to get that going and the Zaza model deployed
correctly with the below versioning on a Focal host:

sudo apt update; sudo snap refresh lxd --channel=5.0/stable; mkdir -p ~/.local/share/juju
sudo snap install juju --channel=3.1/stable --classic
juju bootstrap localhost controller; juju add-model test

sudo snap install charmcraft --channel=2.0/stable --classic
sudo apt install -y make tox

git clone https://git.launchpad.net/charm-telegraf; cd charm-telegraf
make functional

However, this now fails as neutron-gateway is unable to start
because of the below:

Feb 8 10:37:22 juju-32f109-7 ovs-ctl[9436]: modprobe: FATAL: Module openvswitch not found in directory /lib/modules/5.4.0-169-generic

However, this is not related to the patch (the steps above
fail with the out-of-the-box charm), hence why I submitted
it as is. I did check that the patch was working, i.e.,
permissions changed correctly, charm config options doing
their job.

[Robert Gildein (rgildein)]

In general, it looks fine to me, but I would really like to see more unit tests,
since we do not have proper func either.

[Chi Wai CHAN (raychan96)]

It might be related to the environment (lxd cloud) not your patch, because telegraf charm functional test requires openstack related charms, and that may not work well on container. We will try it from our side with VMs. Or if you could use VMs, you can also try it again without waiting for us.

[Chi Wai CHAN (raychan96)]

I've tested your patch in our environment with VMs, it doesn't have the issue you mentioned in your functional test. https://pastebin.ubuntu.com/p/5CZ79Q3rJ5/ (note you will need an Ubuntu account to view the pastebin).

The charm is in maintenance mode, so you should build and test the charm with the following setup at least (though there might be some other versions still work, I did not test it thoroughly):

```
juju 3.1.7 25751 3.1/stable canonical✓ -
charmcraft 2.0.0 1033 2.0/stable canonical✓ classic
```

and a juju controller at version 3.1.7

[Alejandro Santoyo Gonzalez (al3jandrosg)]

Using the recommended versioning but the deployment (with the unchanged charm-telegraf repo, no patch added yet) fails as the charm cannot install:

```
charm 3.0.6 712 latest/stable canonical** classic
charmcraft 2.0.0 1033 2.0/stable canonical** classic
juju 3.1.7 25751 3.1/stable canonical** -
```
status:

Unit Workload Agent Machine Public address Ports Message
ubuntu/0* active idle 0 10.5.1.103
  telegraf/0* error idle 10.5.1.103 hook failed: "install"

I'm facing lots of dependency errors. First it complains about:

2024-03-25 15:14:18 WARNING unit.telegraf/0.install logger.go:60 ERROR: Could not find a version that satisfies the requirement setuptools>=42 (from versions: 41.6.0)

I will upload the unit log.

This is unfortunately taking quite a bit of time. Is the automated/repo func testing going to be fixed any time soon?

[Alejandro Santoyo Gonzalez (al3jandrosg)]

2024-03-25 15:17:39 INFO juju.worker.uniter resolver.go:161 awaiting error resolution for "install" hook
2024-03-25 15:21:41 INFO juju.worker.uniter resolver.go:161 awaiting error resolution for "install" hook
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 ERROR: Command errored out with exit status 1:
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 command: /var/lib/juju/agents/unit-telegraf-0/.venv/bin/python /var/lib/juju/agents/unit-telegraf-0/.venv/lib/python3.8/site-packages/pip install --ignore-installed --no-user --prefix /tmp/pip-build-env-y9wnzkmz/overlay --no-warn-script-location --no-binary :none: --only-binary :none: --no-index --find-links wheelhouse -- 'setuptools>=42' wheel
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 cwd: None
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 Complete output (3 lines):
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 Looking in links: wheelhouse
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 ERROR: Could not find a version that satisfies the requirement setuptools>=42 (from versions: 41.6.0)
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 ERROR: No matching distribution found for setuptools>=42
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 ----------------------------------------
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 ERROR: Command errored out with exit status 1: /var/lib/juju/agents/unit-telegraf-0/.venv/bin/python /var/lib/juju/agents/unit-telegraf-0/.venv/lib/python3.8/site-packages/pip install --ignore-installed --no-user --prefix /tmp/pip-build-env-y9wnzkmz/overlay --no-warn-script-location --no-binary :none: --only-binary :none: --no-index --find-links wheelhouse -- 'setuptools>=42' wheel Check the logs for full command output.
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 Traceback (most recent call last):
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 File "/var/lib/juju/agents/unit-telegraf-0/charm/hooks/install", line 8, in <module>
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 basic.bootstrap_charm_deps()
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 File "/var/lib/juju/agents/unit-telegraf-0/charm/lib/charms/layer/basic.py", line 226, in bootstrap_charm_deps
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 check_call([pip, 'install', '-U', reinstall_flag, '--no-index',
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 File "/usr/lib/python3.8/subprocess.py", line 364, in check_call
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 raise CalledProcessError(retcode, cmd)
2024-03-25 15:21:58 WARNING unit.telegraf/0.install logger.go:60 subprocess.CalledProcessError: Command '['/var/lib/juju/agents/unit-telegraf-0/.venv/bin/pip', 'install', '-U', '--force-reinstall', '--no-index', '--no-cache-dir', '-f', 'wheelhouse', 'lockfile==0.12.2', 'immutables==0.19', 'charms.reactive==1.5.2', 'pbr==6.0.0', 'sniffio==1.2.0', 'contextvars==2.4', 'defusedxml==0.5.0', 'six==1....

[Eric Chen (eric-chen)]

Unfortunately, we currently have no plans to restore the CI server due to the complete loss of our infrastructure. Additionally, this project is in maintenance mode and is likely to be deprecated within the year. All system metrics should be provided with node exporter. I recommend that you also raise your request there.

https://github.com/canonical/grafana-agent-operator/blob/main/src/charm.py#L362

I could find a member to take a look again since it's close to the time of 24.04 release.

[Chi Wai CHAN (raychan96)]

This is unfortunately caused by external dependency: https://github.com/canonical/layer-basic/issues/223

Unmerged commits

bb7a195... by Alejandro Santoyo Gonzalez on 2024-02-08

Adding support for intel_powerstat

This patch adds support for intel_powerstat by:

- fixing the permissions issues when attempting to read
`/sys/devices/virtual/powercap/intel-rapl/`
- adding charm configs: `collect_intel_powerstat_metrics`,
`package_metrics`, `cpu_metrics`

Fixes [LP#1976521](https://bugs.launchpad.net/charm-telegraf/+bug/1976521)

diff --git a/src/config.yaml b/src/config.yaml
index 282a163..a66b279 100644
--- a/src/config.yaml
+++ b/src/config.yaml
@@ -268,3 +268,31 @@ options:
         intel-cmt-cat package on ppa:canonical-bootstack/public.  Also, Telegraf needs
         to include a patch for https://github.com/influxdata/telegraf/issues/9380, currently
         available in ppa:guoqiao/telegraf.
+  collect_intel_powerstat_metrics:
+    default: false
+    type: boolean
+    description: >
+        Enable the collection of power statistics on Intel-based platforms using 
+        the telegraf intel_powerstat plugin. 
+        .
+        There are certain requisites to run this plugin, including having a
+        kernel >= v3.13, for `uncore_frequency` metrics `intel-uncore-frequency` 
+        module is required (available since kernel v5.6).
+        .
+        See https://github.com/influxdata/telegraf/blob/master/plugins/inputs/intel_powerstat/README.md
+  package_metrics:
+    default: "current_power_consumption"
+    type: string
+    description: >
+        If set, the comma-separated list of package metrics to be  monitored by the 
+        plugin.
+        
+        ex: "current_power_consumption,current_dram_power_consumption,thermal_design_power"
+  cpu_metrics:
+    default: "cpu_temperature"
+    type: string
+    description: >
+        If set, the comma-separated list of per-CPU metrics to be  monitored by the 
+        plugin.
+        
+        ex: "cpu_frequency,cpu_temperature"
\ No newline at end of file
diff --git a/src/reactive/telegraf.py b/src/reactive/telegraf.py
index eea17e6..3df37a0 100644
--- a/src/reactive/telegraf.py
+++ b/src/reactive/telegraf.py
@@ -116,6 +116,16 @@ RDT_KERNEL_MODULE_NAME = "msr"
 TLS_CERT = pathlib.Path("/etc/telegraf/tls/server.crt")
 TLS_KEY = pathlib.Path("/etc/telegraf/tls/server.key")
 
+# constants related to Intel Powerstat metrics support
+POWERSTAT_MINIMUM_KERNEL_VERSION = (3, 13, 0)
+POWERSTAT_KERNEL_VERSION_RESTRICTED_PERMS = (5, 4, 77)
+POWERSTAT_KERNEL_MODULE_NAMES = {  # keys are kernel versions, values are module names
+    (4, 0, 0): ["msr", "intel_rapl"],
+    (5, 0, 0): ["rapl", "msr", "intel_rapl_common", "intel_rapl_msr"],
+    (5, 6, 0): ["intel-uncore-frequency"],
+}
+POWERSTAT_ADD_PERMS_PATH = "/sys/devices/virtual/powercap/intel-rapl/"
+
 
 class InvalidInstallMethodError(Exception):
     pass
@@ -129,6 +139,10 @@ class InvalidIntelRDTConfigurationError(Exception):
     pass
 
 
+class InvalidIntelPowerstatConfigurationError(Exception):
+    pass
+
+
 class Address(Enum):
     IPV4 = 1
     IPV6 = 2
@@ -140,6 +154,17 @@ def is_intel_rdt_enabled():
     return config.get("collect_intel_rdt_metrics", False) and is_metal()
 
 
+def is_intel_powerstat_enabled():
+    config = hookenv.config()
+    if not is_metal():
+        hookenv.log(
+            """WARNING: Intel Powerstat can only be enabled on baremetal. 
+            Skipping."""
+        )
+
+    return config.get("collect_intel_powerstat_metrics", False) and is_metal()
+
+
 def write_telegraf_file(path, content):
     return host.write_file(
         path,
@@ -403,6 +428,77 @@ def check_valid_intel_rdt_configuration():
     return None
 
 
+def check_valid_intel_powerstat_configuration():
+    """
+    Check that the requirements for Intel Powerstat are met.
+
+    Will raise a InvalidIntelPowerstatConfigurationError exception when a
+    validation issue is encountered, otherwise None
+    """
+    current_kernel_version = get_current_kern_ver()
+    if current_kernel_version:
+        # check the minimum kernel version requirement
+        if current_kernel_version < POWERSTAT_MINIMUM_KERNEL_VERSION:
+            raise InvalidIntelPowerstatConfigurationError(
+                "unsupported kernel version: {}, need version higher than {}".format(
+                    current_kernel_version, POWERSTAT_MINIMUM_KERNEL_VERSION
+                )
+            )
+
+        # check that the required modules are loaded
+        kernel_version = get_intel_powerstat_kern_ver()
+        if not kernel_version:
+            raise InvalidIntelPowerstatConfigurationError(
+                "Kernel version '{}' is not supported".format(
+                    ".".join(map(str, current_kernel_version))
+                )
+            )
+
+        for mod in POWERSTAT_KERNEL_MODULE_NAMES[kernel_version]:
+            if not kernel.is_module_loaded(mod):
+                raise InvalidIntelPowerstatConfigurationError(
+                    "required module '{}' is not loaded".format(mod)
+                )
+    else:
+        linux_release = platform.release()  # format is like '5.4.0-73-generic'
+        raise InvalidIntelPowerstatConfigurationError(
+            "Incompatible platform.release output: {}".format(linux_release)
+        )
+    return None
+
+
+def get_current_kern_ver():
+    # check that we meet the minimum kernel version
+    linux_release = platform.release()  # format is like '5.4.0-73-generic'
+    re_kernel_version = r"^(\d+)\.(\d+)\.(\d+)"
+    match = re.match(re_kernel_version, linux_release)
+
+    if match:
+        return tuple(int(d) for d in match.groups())
+
+    return None
+
+
+def get_intel_powerstat_kern_ver():
+    """
+    Return kernel version.
+
+    Return which kernel version to use as key to parse
+    POWERSTAT_KERNEL_MODULE_NAMES in order to check
+    which modules need to be loaded.
+    """
+    current_kernel_version = get_current_kern_ver()
+    kernel_version = None
+    if current_kernel_version >= (5, 6, 0):
+        kernel_version = (5, 6, 0)
+    elif current_kernel_version >= (5, 0, 0):
+        kernel_version = (5, 0, 0)
+    elif current_kernel_version >= (4, 0, 0):
+        kernel_version = (4, 0, 0)
+
+    return kernel_version
+
+
 def get_cpu_cores():
     """Get the list of available cores for the cpu(s)."""
     # should return something like ["0-23"]
@@ -473,6 +569,15 @@ def get_base_inputs():
     else:
         intel_rdt_cores = None
 
+    # handle the Intel Powerstat collection parameters
+    intel_powerstat = is_intel_powerstat_enabled()
+    if intel_powerstat:
+        package_metrics = config["package_metrics"]
+        cpu_metrics = config["cpu_metrics"]
+    else:
+        package_metrics = None
+        cpu_metrics = None
+
     return {
         "extra_options": extra_options["inputs"],
         "bcache": is_bcache(),
@@ -485,6 +590,9 @@ def get_base_inputs():
         "ipmi_sensor": ipmi_sensor,
         "intel_rdt": intel_rdt,
         "intel_rdt_cores": intel_rdt_cores,
+        "intel_powerstat": intel_powerstat,
+        "package_metrics": package_metrics,
+        "cpu_metrics": cpu_metrics,
     }
 
 
@@ -893,6 +1001,36 @@ def configure_telegraf():  # noqa: C901
     else:
         remove_sudoers_file(sudoers_filename)
 
+    if is_intel_powerstat_enabled():
+        hookenv.log("Intel Powerstat enabled, enabling modules and running checks")
+        # load and persist the required module(s)
+        kernel_version = get_intel_powerstat_kern_ver()
+        try:
+            for mod in POWERSTAT_KERNEL_MODULE_NAMES[kernel_version]:
+                kernel.modprobe(mod, persist=True)
+        except subprocess.CalledProcessError:
+            error_msg = "modprobe {} failed".format(mod)
+            hookenv.log(error_msg, level=hookenv.ERROR)
+            hookenv.status_set("blocked", error_msg)
+            return
+
+        try:
+            check_valid_intel_powerstat_configuration()
+        except InvalidIntelPowerstatConfigurationError() as e:
+            # on error we abort configuration and block the charm
+            error_msg = "Intel Powerstat configuration failed."
+            hookenv.log(e, level=hookenv.ERROR)
+            hookenv.status_set("blocked", error_msg)
+            return
+
+        # change the permissions for POWERSTAT_ADD_PERMS_PATH
+        if kernel_version >= POWERSTAT_KERNEL_VERSION_RESTRICTED_PERMS:
+            cmd = ["chmod", "-R", "a+rx", POWERSTAT_ADD_PERMS_PATH]
+            hookenv.log(
+                "Setting permissions for Powerstat to work: {}".format(" ".join(cmd))
+            )
+            subprocess.call(cmd)
+
     telegraf_exec_metrics = os.path.join(get_files_dir(), "telegraf_exec_metrics.py")
     cmd = [
         telegraf_exec_metrics,
@@ -1072,6 +1210,12 @@ def handle_config_changes():
     else:
         clear_flag("telegraf.intel_rdt.enabled")
 
+    # handle the Intel Powerstat metrics collection
+    if is_intel_powerstat_enabled():
+        set_flag("telegraf.intel_powerstat.enabled")
+    else:
+        clear_flag("telegraf.intel_powerstat.enabled")
+
     clear_flag("telegraf.configured")
     clear_flag("telegraf.apt.configured")
     clear_flag("telegraf.snap.configured")
diff --git a/src/templates/base_inputs.conf b/src/templates/base_inputs.conf
index 9e178be..117acdd 100644
--- a/src/templates/base_inputs.conf
+++ b/src/templates/base_inputs.conf
@@ -179,6 +179,18 @@ use_sudo = true
 {%- endif %}
 {% endif %}
 
+{% if "intel_powerstat" not in disabled_plugins %}
+{% if intel_powerstat -%}
+[[inputs.intel_powerstat]]
+{% if package_metrics %}
+package_metrics = {{ package_metrics.split(',') | tojson }}
+{% endif %}
+{% if cpu_metrics %}
+cpu_metrics = {{ cpu_metrics.split(',') | tojson }}
+{% endif %}
+{%- endif %}
+{% endif %}
+
 [[inputs.exec]]
 commands = [
     "/usr/bin/awk '{ print $1 }' /proc/sys/fs/file-nr"
diff --git a/src/tests/unit/test_telegraf.py b/src/tests/unit/test_telegraf.py
index 2ba2a0e..0493a94 100644
--- a/src/tests/unit/test_telegraf.py
+++ b/src/tests/unit/test_telegraf.py
@@ -2044,6 +2044,38 @@ def test_check_valid_intel_rdt_configuration_pqos(monkeypatch):
         telegraf.check_valid_intel_rdt_configuration()
 
 
+def test_collect_intel_powerstat_metrics(monkeypatch, config):
+    monkeypatch.setattr(telegraf, "is_container", lambda: False)
+    monkeypatch.setattr(telegraf, "is_metal", lambda: True)
+    config["collect_intel_powerstat_metrics"] = True
+    config["package_metrics"] = "current_power_consumption,thermal_design_power"
+    config["cpu_metrics"] = "cpu_frequency"
+    monkeypatch.setattr(
+        telegraf, "check_valid_intel_powerstat_configuration", lambda: ""
+    )
+    monkeypatch.setattr(kernel, "modprobe", lambda module, persist: "msr")
+    telegraf.configure_telegraf()
+
+    expected = """
+[[inputs.intel_powerstat]]
+package_metrics = ["current_power_consumption", "thermal_design_power"]
+cpu_metrics = ["cpu_frequency"]
+"""
+    config_file = base_dir().join("telegraf.conf")
+    assert expected in config_file.read()
+
+
+def test_check_valid_intel_powerstat_configuration_kernel_version(monkeypatch):
+    monkeypatch.setattr(telegraf, "is_container", lambda: False)
+    monkeypatch.setattr(telegraf, "is_metal", lambda: True)
+    monkeypatch.setattr(platform, "release", lambda: "3.3.0-73-generic")
+    with pytest.raises(
+        telegraf.InvalidIntelPowerstatConfigurationError,
+        match="unsupported kernel version",
+    ):
+        telegraf.check_valid_intel_powerstat_configuration()
+
+
 def test_configure_tls(monkeypatch):
     tnp_key_location = pathlib.Path("/tmp/fake-key")
     tmp_cert_location = pathlib.Path("/tmp/fake-crt")