Add functional tests for grub lib

.github/workflows/build-and-test.yaml

@@ -32,22 +32,21 @@ jobs:
       - name: Run tests
         run: tox -e unit
   integration-test:
-    name: Integration tests
+    name: Ubuntu integration tests
     runs-on: ubuntu-22.04
     steps:
       - name: Checkout
         uses: actions/checkout@v3
-      - uses: canonical/[email protected]
-        with:
-          channel: 5.0/stable
       - name: Install dependencies
-        run: python -m pip install tox
-      - name: Run integration tests
-        run: tox -e integration
+        run: sudo python -m pip install tox
+      - name: Run integration tests for Ubuntu
+        # tests must be run as root because they configure the system
+        run: sudo tox -e integration-ubuntu
       - name: Setup operator environment
         uses: charmed-kubernetes/actions-operator@main
         with:
           provider: lxd
           juju-channel: 3.1/stable
       - name: Run integration tests (juju-systemd-notices)
         run: tox run -e integration-juju-systemd-notices
+

lib/charms/operator_libs_linux/v0/grub.py

@@ -54,7 +54,6 @@ def _on_remove(self, _):
 import filecmp
 import io
 import logging
-import os
 import shlex
 import subprocess
 from pathlib import Path
@@ -70,7 +69,7 @@ def _on_remove(self, _):
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 2
+LIBPATCH = 3
 
 GRUB_DIRECTORY = Path("/etc/default/grub.d/")
 CHARM_CONFIG_PREFIX = "90-juju"
@@ -161,13 +160,24 @@ def _save_config(path: Path, config: Dict[str, str], header: str = CONFIG_HEADER
     if path.exists():
         logger.debug("GRUB config %s already exist and it will overwritten", path)
 
-    context = [f"{key}={shlex.quote(value)}" for key, value in config.items()]
     with open(path, "w", encoding="UTF-8") as file:
-        file.writelines([header, *context])
+        file.write(header)
+        for key, value in config.items():
+            file.write(f"{key}={shlex.quote(value)}\n")
 
     logger.info("GRUB config file %s was saved", path)
 
 
+def _update_config(path: Path, config: Dict[str, str], header: str = CONFIG_HEADER) -> None:
+    """Update existing GRUB config file."""
+    if path.exists():
+        original_config = _load_config(path)
+        original_config.update(config)
+        config = original_config.copy()
+
+    _save_config(path, config, header)
+
+
 def check_update_grub() -> bool:
     """Report whether an update to /boot/grub/grub.cfg is available."""
     main_grub_cfg = Path("/boot/grub/grub.cfg")
@@ -178,7 +188,7 @@ def check_update_grub() -> bool:
         )
     except subprocess.CalledProcessError as error:
         logger.exception(error)
-        raise
+        raise ApplyError from error
 
     return not filecmp.cmp(main_grub_cfg, tmp_path)
 
@@ -240,7 +250,7 @@ def _save_grub_configuration(self) -> None:
         """Save current GRUB configuration."""
         logger.info("saving new GRUB config to %s", GRUB_CONFIG)
         applied_configs = {self.path, *self.applied_configs}  # using set to drop duplicity
-        registered_configs = os.linesep.join(
+        registered_configs = "\n".join(
             FILE_LINE_IN_DESCRIPTION.format(path=path) for path in applied_configs
         )
         header = CONFIG_HEADER + CONFIG_DESCRIPTION.format(configs=registered_configs)
@@ -378,5 +388,5 @@ def update(self, config: Dict[str, str], apply: bool = True) -> Set[str]:
             raise
 
         logger.debug("[%s] saving copy of charm config to %s", self.charm_name, GRUB_DIRECTORY)
-        _save_config(self.path, config)
+        _update_config(self.path, config)
         return changed_keys

tests/integration/test_apt.py

@@ -55,7 +55,7 @@ def test_install_package_external_repository():
     apt.update()
     apt.add_package("terraform")
 
-    assert get_command_path("terraform") == "/usr/bin/terraform"
+    assert get_command_path("terraform") == "/usr/local/bin/terraform"
 
 
 def test_list_file_generation_external_repository():

tests/integration/test_grub.py

@@ -0,0 +1,201 @@
+#!/usr/bin/env python3
+# Copyright 2023 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+import logging
+
+import pytest
+from charms.operator_libs_linux.v0 import grub
+
+logger = logging.getLogger(__name__)
+
+
+@pytest.fixture(autouse=True)
+def clean_configs():
+    """Clean main and charms configs after each test."""
+    yield  # run test
+    grub.GRUB_CONFIG.unlink(missing_ok=True)
+    for charm_config in grub.GRUB_DIRECTORY.glob(f"{grub.CHARM_CONFIG_PREFIX}-*"):
+        charm_config.unlink(missing_ok=True)
+
+
+@pytest.mark.parametrize(
+    "config",
+    [
+        {"GRUB_CMDLINE_LINUX_DEFAULT": "$GRUB_CMDLINE_LINUX_DEFAULT hugepagesz=1G"},
+        {
+            "GRUB_CMDLINE_LINUX_DEFAULT": "$GRUB_CMDLINE_LINUX_DEFAULT hugepages=64 hugepagesz=1G",
+            "GRUB_DEFAULT": "0",
+        },
+        {"GRUB_TIMEOUT": "0"},
+    ],
+)
+def test_single_charm_valid_update(config):
+    """Test single charm update GRUB configuration."""
+    grub_conf = grub.Config("test-charm")
+    grub_conf.update(config)
+    # check that config was set for charm config file
+    assert config == grub_conf
+    assert config == grub._load_config(grub_conf.path)
+    # check the main config
+    assert config == grub._load_config(grub.GRUB_CONFIG)
+
+
+@pytest.mark.parametrize("config", [{"TEST_WRONG_KEY:test": "1"}])
+def test_single_charm_update_apply_failure(config):
+    """Test single charm update GRUB configuration with ApplyError."""
+    # create empty grub config
+    grub.GRUB_CONFIG.touch()
+    grub_conf = grub.Config("test-charm")
+
+    with pytest.raises(grub.ApplyError):
+        grub_conf.update(config)
+
+    # check that charm file was not configured
+    assert not grub_conf.path.exists()
+    # check the main config
+    main_config = grub._load_config(grub.GRUB_CONFIG)
+    for key in config:
+        assert key not in main_config
+
+
+def test_single_charm_multiple_update():
+    """Test that charm can do multiple updates and update it's own configuration."""
+    # charms using this config to make update
+    configs = [
+        {"GRUB_TIMEOUT": "0"},
+        {
+            "GRUB_TIMEOUT": "0",
+            "GRUB_CMDLINE_LINUX_DEFAULT": "$GRUB_CMDLINE_LINUX_DEFAULT hugepagesz=1G",
+        },
+        {"GRUB_TIMEOUT": "1"},
+    ]
+    # charms configs in time
+    exp_charms_configs = [
+        {"GRUB_TIMEOUT": "0"},
+        {
+            "GRUB_TIMEOUT": "0",
+            "GRUB_CMDLINE_LINUX_DEFAULT": "$GRUB_CMDLINE_LINUX_DEFAULT hugepagesz=1G",
+        },
+        {
+            "GRUB_TIMEOUT": "1",
+            "GRUB_CMDLINE_LINUX_DEFAULT": "$GRUB_CMDLINE_LINUX_DEFAULT hugepagesz=1G",
+        },
+    ]
+    exp_main_config = {
+        "GRUB_TIMEOUT": "1",
+        "GRUB_CMDLINE_LINUX_DEFAULT": "$GRUB_CMDLINE_LINUX_DEFAULT hugepagesz=1G",
+    }
+    grub_conf = grub.Config("test-charm")
+
+    for config, exp_conf in zip(configs, exp_charms_configs):
+        grub_conf.update(config)
+        assert exp_conf == grub_conf
+        assert exp_conf == grub._load_config(grub_conf.path)
+
+    # check the main config
+    assert exp_main_config == grub._load_config(grub.GRUB_CONFIG)
+
+
+@pytest.mark.parametrize(
+    "config_1, config_2",
+    [
+        ({"GRUB_TIMEOUT": "0"}, {"GRUB_TIMEOUT": "0"}),
+        (
+            {"GRUB_TIMEOUT": "0"},
+            {"GRUB_CMDLINE_LINUX_DEFAULT": "$GRUB_CMDLINE_LINUX_DEFAULT hugepagesz=1G"},
+        ),
+        (
+            {"GRUB_CMDLINE_LINUX_DEFAULT": "$GRUB_CMDLINE_LINUX_DEFAULT hugepagesz=1G"},
+            {"GRUB_TIMEOUT": "0"},
+        ),
+    ],
+)
+def test_two_charms_no_conflict(config_1, config_2):
+    """Test two charms update GRUB configuration without any conflict."""
+    for name, config in [("test-charm-1", config_1), ("test-charm-2", config_2)]:
+        grub_conf = grub.Config(name)
+        grub_conf.update(config)
+        assert config == grub._load_config(grub_conf.path)
+
+    # check the main config
+    assert {**config_1, **config_2} == grub._load_config(grub.GRUB_CONFIG)
+
+
+@pytest.mark.parametrize(
+    "config_1, config_2",
+    [
+        ({"GRUB_TIMEOUT": "0"}, {"GRUB_TIMEOUT": "1"}),
+        (
+            {"GRUB_TIMEOUT": "0"},
+            {
+                "GRUB_TIMEOUT": "1",
+                "GRUB_CMDLINE_LINUX_DEFAULT": "$GRUB_CMDLINE_LINUX_DEFAULT hugepagesz=1G",
+            },
+        ),
+    ],
+)
+def test_two_charms_with_conflict(config_1, config_2):
+    """Test two charms update GRUB configuration with conflict."""
+    # configure charm 1
+    grub_conf_1 = grub.Config("test-charm-1")
+    grub_conf_1.update(config_1)
+    assert config_1 == grub._load_config(grub_conf_1.path)
+
+    # configure charm 2
+    grub_conf_2 = grub.Config("test-charm-2")
+    with pytest.raises(grub.ValidationError):
+        grub_conf_2.update(config_2)
+
+    assert not grub_conf_2.path.exists()
+    # check the main config
+    assert config_1 == grub._load_config(grub.GRUB_CONFIG)
+
+
+def test_charm_remove_configuration():
+    """Test removing charm configuration."""
+    config = {"GRUB_TIMEOUT": "0"}
+    grub_conf = grub.Config("test-charm")
+    grub_conf.update(config)
+
+    assert grub_conf.path.exists(), "Config file is missing, check test_single_charm_valid_update"
+    assert config == grub._load_config(grub_conf.path)
+    assert config == grub._load_config(grub.GRUB_CONFIG)
+
+    grub_conf.remove()
+    assert not grub_conf.path.exists()
+    assert {} == grub._load_config(grub.GRUB_CONFIG)
+
+
+@pytest.mark.parametrize(
+    "config_1, config_2",
+    [
+        (
+            {"GRUB_TIMEOUT": "0"},
+            {
+                "GRUB_TIMEOUT": "0",
+                "GRUB_CMDLINE_LINUX_DEFAULT": "$GRUB_CMDLINE_LINUX_DEFAULT hugepagesz=1G",
+            },
+        ),
+        (
+            {
+                "GRUB_TIMEOUT": "0",
+                "GRUB_CMDLINE_LINUX_DEFAULT": "$GRUB_CMDLINE_LINUX_DEFAULT hugepagesz=1G",
+            },
+            {"GRUB_TIMEOUT": "0"},
+        ),
+    ],
+)
+def test_charm_remove_configuration_without_changing_others(config_1, config_2):
+    """Test removing charm configuration and do not touch other."""
+    grub_conf_1 = grub.Config("test-charm-1")
+    grub_conf_1.update(config_1)
+    grub_conf_2 = grub.Config("test-charm-2")
+    grub_conf_2.update(config_2)
+
+    assert grub_conf_1.path.exists()
+    assert grub_conf_2.path.exists()
+
+    grub_conf_1.remove()
+    assert not grub_conf_1.path.exists()
+    assert config_2 == grub._load_config(grub.GRUB_CONFIG)