Skip to content

Commit

Permalink
Make cpufrequtils optional (#90)
Browse files Browse the repository at this point in the history
cpufrequtils is not available on some architectures,
such as ppc64el.
In these cases, previously the charm would fail to install,
crashing on trying to install cpufrequtils.

So now the charm will install cpufrequtils if it is available
and continue as usual.
If cpufrequtils is not available however,
the charm will skip attempting to install cpufrequtils,
and ignore the governor config option.

Also some tox.ini improvements:
- remove the envdir option from tox.ini;
  this is not a recommended way to share environments.
- switch to flake8-pyproject
  - appears more maintained
  - lets us use the original flake8 command
  • Loading branch information
samuelallan72 authored Feb 19, 2025
1 parent 3dd5d54 commit 9d5635d
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 11 deletions.
3 changes: 3 additions & 0 deletions src/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@ options:
Recommended option when Bios control power is set to the OS.
- 'performance'
- 'powersave'
On some architectures, cpufrequtils is not available.
In these cases, this config option will be ignored.
resolved-cache-mode:
type: string
default: ""
Expand Down
3 changes: 0 additions & 3 deletions src/layer.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,4 @@ options:
include_system_packages: true
packages:
- python3-yaml
apt:
packages:
- cpufrequtils
repo: https://github.com/canonical/charm-sysconfig
15 changes: 15 additions & 0 deletions src/lib/lib_sysconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,21 @@ def install_configured_kernel(self):
apt_install("linux-image-{}".format(configured))
self.boot_resources.set_resource(KERNEL)

def install_cpufrequtils(self):
"""Install cpufrequtils if available.
Return True if cpufrequtils is available
and installation was successful.
"""
apt_update()
if subprocess.check_output(
["apt-cache", "search", "--names-only", "^cpufrequtils$"]
).strip():
apt_install("cpufrequtils", fatal=True)
return True
else:
return False

def update_cpufreq(self):
"""Update /etc/default/cpufrequtils and restart cpufrequtils service."""
if self.governor not in ("", "performance", "powersave"):
Expand Down
23 changes: 19 additions & 4 deletions src/reactive/sysconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,19 @@ def install_sysconfig():
hookenv.status_set("blocked", "configuration parameters not valid.")
return

if syshelper.install_cpufrequtils():
set_flag("sysconfig.cpufrequtils-installed")
else:
hookenv.log(
"cpufrequtils package not found: governor config option will be ignored.",
hookenv.WARNING,
)

syshelper.install_configured_kernel()
syshelper.update_cpufreq()

if is_flag_set("sysconfig.cpufrequtils-installed"):
syshelper.update_cpufreq()

syshelper.update_grub_file()
syshelper.update_systemd_system_file()
syshelper.update_systemd_resolved()
Expand All @@ -86,8 +97,9 @@ def config_changed():
syshelper.install_configured_kernel()

# cpufreq
if syshelper.charm_config.changed("governor") or helpers.any_file_changed(
[CPUFREQUTILS]
if is_flag_set("sysconfig.cpufrequtils-installed") and (
syshelper.charm_config.changed("governor")
or helpers.any_file_changed([CPUFREQUTILS])
):
syshelper.update_cpufreq()

Expand Down Expand Up @@ -215,7 +227,10 @@ def remove_configuration():
"""
hookenv.status_set("maintenance", "removing sysconfig configurations")
syshelper = SysConfigHelper()
syshelper.remove_cpufreq_configuration()

if is_flag_set("sysconfig.cpufrequtils-installed"):
syshelper.remove_cpufreq_configuration()

syshelper.remove_grub_configuration()
syshelper.remove_systemd_configuration()
syshelper.remove_resolved_configuration()
Expand Down
38 changes: 37 additions & 1 deletion src/tests/unit/test_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ def test_update_sysctl(self, check_call, config):

mock_file.assert_called_with(lib_sysconfig.SYSCTL_CONF, "w")
handle = mock_file()
handle.write.has_calls(
handle.write.assert_has_calls(
[mock.call("net.ipv4.ip_forward=1\n"), mock.call("vm.swappiness=60\n")]
)
check_call.assert_called_with(["sysctl", "-p", lib_sysconfig.SYSCTL_CONF])
Expand Down Expand Up @@ -1101,3 +1101,39 @@ def test_remove_irqbalance_configuration(self, render, log, config, restart):
)

restart.assert_called()

@mock.patch("lib_sysconfig.apt_install")
@mock.patch("lib_sysconfig.apt_update")
@mock.patch("lib_sysconfig.subprocess")
@mock.patch("lib_sysconfig.hookenv.config")
def test_install_cpufrequtils_success(
self, _mock_config, mock_subprocess, mock_apt_update, mock_apt_install
):
"""Verify it does install cpufrequtils when it's available."""
mock_subprocess.check_output.return_value = (
"cpufrequtils - utilities to deal with the cpufreq Linux kernel feature"
)
sysh = lib_sysconfig.SysConfigHelper()

result = sysh.install_cpufrequtils()

assert result is True
mock_apt_update.assert_called_once_with()
mock_apt_install.assert_called_once_with("cpufrequtils", fatal=True)

@mock.patch("lib_sysconfig.apt_install")
@mock.patch("lib_sysconfig.apt_update")
@mock.patch("lib_sysconfig.subprocess")
@mock.patch("lib_sysconfig.hookenv.config")
def test_install_cpufrequtils_not_found(
self, _mock_config, mock_subprocess, mock_apt_update, mock_apt_install
):
"""Verify it doesn't install cpufrequtils when it's not available."""
mock_subprocess.check_output.return_value = ""
sysh = lib_sysconfig.SysConfigHelper()

result = sysh.install_cpufrequtils()

assert result is False
mock_apt_update.assert_called_once_with()
mock_apt_install.assert_not_called()
5 changes: 2 additions & 3 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,20 @@ passenv =

[testenv:lint]
commands =
pflake8
flake8
black --check --diff --color .
isort --check --diff --color .
deps =
black<24.0.0 # TODO: Remove pin once we make our linter pass on it
flake8
pyproject-flake8
flake8-pyproject
flake8-docstrings
pep8-naming
flake8-colors
colorama
isort

[testenv:reformat]
envdir = {toxworkdir}/lint
commands =
black .
isort .
Expand Down

0 comments on commit 9d5635d

Please sign in to comment.