Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: apt install freeipmi-tools failed on focal #143

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/apt_helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
"""Apt helper module for missing features in operator_libs_linux."""
import re
from subprocess import PIPE, CalledProcessError, check_output
from typing import Optional

from charms.operator_libs_linux.v0 import apt


def get_candidate_version(package: str) -> Optional[str]:
"""Get candiate version of package from apt-cache.
Related issue: https://github.com/canonical/operator-libs-linux/issues/113
"""
try:
output = check_output(
["apt-cache", "policy", package], stderr=PIPE, universal_newlines=True
)
except CalledProcessError as e:
raise apt.PackageError(f"Could not list packages in apt-cache: {e.output}") from None

lines = [line.strip() for line in output.strip().split("\n")]
for line in lines:
candidate_matcher = re.compile(r"^Candidate:\s(?P<version>(.*))")
matches = candidate_matcher.search(line)
if matches:
return matches.groupdict().get("version")
raise apt.PackageError(f"Could not find candidate version package in apt-cache: {output}")


def add_pkg_with_candidate_version(pkg: str) -> None:
"""Install package with apt-cache candidate version.
Related issue: https://github.com/canonical/operator-libs-linux/issues/113
"""
version = get_candidate_version(pkg)
apt.add_package(pkg, version=version, update_cache=False)
12 changes: 6 additions & 6 deletions src/hw_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
SessionCreationError,
)

import apt_helpers
from checksum import (
PERCCLI_VERSION_INFOS,
SAS2IRCU_VERSION_INFOS,
Expand Down Expand Up @@ -307,20 +308,19 @@ class IPMIStrategy(APTStrategyABC):
# messages. The installation should be good since all of these
# tools require the same `freeipmi-tools` to be installed.
_name = HWTool.IPMI_SENSOR
pkgs = ["freeipmi-tools"]
pkg = "freeipmi-tools"

def install(self) -> None:
for pkg in self.pkgs:
apt.add_package(pkg)
apt_helpers.add_pkg_with_candidate_version(self.pkg)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please leave a comment . We can use operator-libs-linux after this issue/feature been implemented. Then we don't need to maintain the code in apt-helper.py
canonical/operator-libs-linux#113

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added related issue links in apt_helpers module


def remove(self) -> None:
# Skip removing because we afriad this cause dependency error
# for other services on the same machine.
logger.info("IPMIStrategy skip removing %s", self.pkgs)
logger.info("IPMIStrategy skip removing %s", self.pkg)

def check(self) -> bool:
"""Check package status."""
return check_deb_pkg_installed(self.pkgs[0])
return check_deb_pkg_installed(self.pkg)


class RedFishStrategy(StrategyABC): # pylint: disable=R0903
Expand Down Expand Up @@ -434,7 +434,7 @@ def bmc_hw_verifier() -> t.List[HWTool]:
tools = []

# Check if ipmi services are available
apt.add_package("freeipmi-tools", update_cache=False)
apt_helpers.add_pkg_with_candidate_version("freeipmi-tools")

try:
subprocess.check_output("ipmimonitoring".split())
Expand Down
43 changes: 43 additions & 0 deletions tests/unit/test_apt_helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import unittest
from subprocess import CalledProcessError
from unittest import mock

from charms.operator_libs_linux.v0 import apt

import apt_helpers

APT_CACHE_POLICY_FREEIPMI_TOOLS_OUTPUT = """freeipmi-tools:
Installed: (none)
Candidate: 1.6.4-3ubuntu1.1
Version table:
1.6.9-2~bpo20.04.1 100
100 http://tw.archive.ubuntu.com/ubuntu focal-backports/main amd64 Packages
1.6.4-3ubuntu1.1 500
500 http://tw.archive.ubuntu.com/ubuntu focal-updates/main amd64 Packages
100 /var/lib/dpkg/status
1.6.4-3ubuntu1 500
500 http://tw.archive.ubuntu.com/ubuntu focal/main amd64 Packages
"""


class TestGetCandidateVersion(unittest.TestCase):
@mock.patch("apt_helpers.check_output")
def test_install_freeipmi_tools_on_focal(self, mock_check_output):
mock_check_output.return_value = APT_CACHE_POLICY_FREEIPMI_TOOLS_OUTPUT
version = apt_helpers.get_candidate_version("freeipmi-tools")
self.assertEqual(version, "1.6.4-3ubuntu1.1")

@mock.patch("apt_helpers.check_output")
def test_checkoutput_failed(self, mock_check_output):
mock_check_output.side_effect = CalledProcessError(-1, "cmd")

with self.assertRaises(apt.PackageError):
apt_helpers.get_candidate_version("freeipmi-tools")

@mock.patch("apt_helpers.check_output")
def test_checkoutput_version_not_found_error(self, mock_check_output):
fake_output = APT_CACHE_POLICY_FREEIPMI_TOOLS_OUTPUT.replace("Candidate", "NotCandidate")
mock_check_output.return_value = fake_output

with self.assertRaises(apt.PackageError):
apt_helpers.get_candidate_version("freeipmi-tools")
28 changes: 16 additions & 12 deletions tests/unit/test_hw_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -674,12 +674,16 @@ def test_remove(self, mock_apt):


class TestIPMIStrategy(unittest.TestCase):
@mock.patch("hw_tools.apt")
def test_install(self, mock_apt):
@mock.patch("apt_helpers.get_candidate_version")
@mock.patch("apt_helpers.apt")
def test_install(self, mock_apt, mock_candidate_version):
strategy = IPMIStrategy()
mock_candidate_version.return_value = "some-candidate-version"
strategy.install()

mock_apt.add_package.assert_called_with("freeipmi-tools")
mock_apt.add_package.assert_called_with(
"freeipmi-tools", version="some-candidate-version", update_cache=False
)

@mock.patch("hw_tools.apt")
def test_remove(self, mock_apt):
Expand Down Expand Up @@ -832,11 +836,11 @@ def test_redfish_available_and_login_success(self, mock_bmc_address, mock_redfis

@mock.patch("hw_tools.redfish_available", return_value=True)
@mock.patch("hw_tools.subprocess")
@mock.patch("hw_tools.apt")
def test_bmc_hw_verifier(self, mock_apt, mock_subprocess, mock_redfish_available):
@mock.patch("hw_tools.apt_helpers")
def test_bmc_hw_verifier(self, mock_apt_helpers, mock_subprocess, mock_redfish_available):
bmc_hw_verifier.cache_clear()
output = bmc_hw_verifier()
mock_apt.add_package.assert_called_with("freeipmi-tools", update_cache=False)
mock_apt_helpers.add_pkg_with_candidate_version.assert_called_with("freeipmi-tools")
self.assertCountEqual(
output, [HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, HWTool.IPMI_DCMI, HWTool.REDFISH]
)
Expand All @@ -846,18 +850,18 @@ def test_bmc_hw_verifier(self, mock_apt, mock_subprocess, mock_redfish_available
"hw_tools.subprocess.check_output",
side_effect=subprocess.CalledProcessError(-1, "cmd"),
)
@mock.patch("hw_tools.apt")
@mock.patch("hw_tools.apt_helpers")
def test_bmc_hw_verifier_error_handling(
self, mock_apt, mock_check_output, mock_redfish_available
self, mock_apt_helpers, mock_check_output, mock_redfish_available
):
bmc_hw_verifier.cache_clear()
output = bmc_hw_verifier()
mock_apt.add_package.assert_called_with("freeipmi-tools", update_cache=False)
mock_apt_helpers.add_pkg_with_candidate_version.assert_called_with("freeipmi-tools")
self.assertEqual(output, [])

@mock.patch("hw_tools.redfish_available", return_value=False)
@mock.patch("hw_tools.apt")
def test_bmc_hw_verifier_mixed(self, mock_apt, mock_redfish_available):
@mock.patch("hw_tools.apt_helpers")
def test_bmc_hw_verifier_mixed(self, mock_apt_helpers, mock_redfish_available):
"""Test a mixture of failures and successes for ipmi."""

def mock_get_response_ipmi(ipmi_call):
Expand All @@ -871,5 +875,5 @@ def mock_get_response_ipmi(ipmi_call):
bmc_hw_verifier.cache_clear()
with mock.patch("hw_tools.subprocess.check_output", side_effect=mock_get_response_ipmi):
output = bmc_hw_verifier()
mock_apt.add_package.assert_called_with("freeipmi-tools", update_cache=False)
mock_apt_helpers.add_pkg_with_candidate_version.assert_called_with("freeipmi-tools")
self.assertCountEqual(output, [HWTool.IPMI_SENSOR, HWTool.IPMI_SEL])