From e2a74918533e35f901919745971e759eebb8a50f Mon Sep 17 00:00:00 2001 From: Raven Kaur Date: Wed, 3 Jun 2026 22:12:45 -0400 Subject: [PATCH] validates external instances storage before starting nova-compute Check whether the default instances path is being used as an externally managed mount point via /etc/fstab. If external storage is configured, the service will only start when: - the instances path is actually mounted - the mounted filesystem is writable If no external storage is configured, startup behavior is unchanged. Signed-off-by: Raven Kaur --- openstack_hypervisor/mount_validation.py | 86 ++++++++++++++ openstack_hypervisor/services.py | 16 +++ tests/unit/test_mount_validation.py | 142 +++++++++++++++++++++++ tests/unit/test_services.py | 28 +++++ 4 files changed, 272 insertions(+) create mode 100644 openstack_hypervisor/mount_validation.py create mode 100644 tests/unit/test_mount_validation.py diff --git a/openstack_hypervisor/mount_validation.py b/openstack_hypervisor/mount_validation.py new file mode 100644 index 0000000..1b64eee --- /dev/null +++ b/openstack_hypervisor/mount_validation.py @@ -0,0 +1,86 @@ +# SPDX-FileCopyrightText: 2026 - Canonical Ltd +# SPDX-License-Identifier: Apache-2.0 + +import logging +import subprocess +import tempfile +from pathlib import Path + +LOG = logging.getLogger(__name__) + + +def path_declared_in_fstab( + path: Path, + fstab_path: Path = Path("/etc/fstab"), +) -> bool: + """Return True if /etc/fstab has an entry targeting the given path.""" + try: + text = fstab_path.read_text() + except OSError as exc: + LOG.warning("Could not read %s (%s), skipping mount validation.", fstab_path, exc) + return False + + for line in text.splitlines(): + line = line.strip() + if not line or line.startswith("#"): + continue + fields = line.split() + if len(fields) >= 2 and Path(fields[1]) == path: + return True + return False + + +def is_mounted(path: Path) -> bool: + """Return True if path is an active mount point.""" + try: + result = subprocess.run( + ["findmnt", "--mountpoint", str(path)], + capture_output=True, + check=False, + ) + except OSError as exc: + LOG.warning("Unable to execute findmnt (%s).", exc) + return False + return result.returncode == 0 + + +def is_usable(path: Path) -> bool: + """Return True if path is a writable directory.""" + if not path.is_dir(): + return False + try: + with tempfile.TemporaryFile(dir=path): + return True + except OSError: + return False + + +def validate_instances_mount( + instances_path: Path, + fstab_path: Path = Path("/etc/fstab"), +) -> bool: + """Validate externally managed storage mount for Nova's instance path.""" + if not path_declared_in_fstab(instances_path, fstab_path): + LOG.debug("No fstab entry for %s, skipping mount validation.", instances_path) + return True + + LOG.info("fstab entry found for %s, validating mount.", instances_path) + + if not is_mounted(instances_path): + LOG.error( + "Instances path %s is declared in /etc/fstab but is not mounted. " + "nova-compute will not start until the path is mounted.", + instances_path, + ) + return False + + if not is_usable(instances_path): + LOG.error( + "Instances path %s is mounted but not a writable directory." + " Check mount status, filesystem health, and permissions. " + "nova-compute will not start.", + instances_path, + ) + return False + + return True diff --git a/openstack_hypervisor/services.py b/openstack_hypervisor/services.py index db4b3b1..7aa24fa 100644 --- a/openstack_hypervisor/services.py +++ b/openstack_hypervisor/services.py @@ -13,6 +13,7 @@ from snaphelpers import Snap, UnknownConfigKey from openstack_hypervisor.log import setup_logging +from openstack_hypervisor.mount_validation import validate_instances_mount def entry_point(service_class): @@ -43,6 +44,9 @@ def run(self, snap: Snap) -> int: """ setup_logging(snap.paths.common / f"{self.executable.name}-{snap.name}.log") + if not self.preflight(snap): + return 1 + args = [] for conf_file in self.conf_files: args.extend( @@ -69,6 +73,13 @@ def run(self, snap: Snap) -> int: logging.info(f"Exiting with code {completed_process.returncode}") return completed_process.returncode + def preflight(self, snap: Snap) -> bool: + """Preflight checks before starting the service. + + Return True to proceed with startup. Return False to abort. + """ + return True + class NovaComputeService(OpenStackService): """A python service object used to run the nova-compute daemon.""" @@ -82,6 +93,11 @@ class NovaComputeService(OpenStackService): executable = Path("usr/bin/nova-compute") + def preflight(self, snap: Snap) -> bool: + """Validate any configured mount for the Nova instances path.""" + instances_path = snap.paths.common / "lib" / "nova" / "instances" + return validate_instances_mount(instances_path) + nova_compute = partial(entry_point, NovaComputeService) diff --git a/tests/unit/test_mount_validation.py b/tests/unit/test_mount_validation.py new file mode 100644 index 0000000..5def62b --- /dev/null +++ b/tests/unit/test_mount_validation.py @@ -0,0 +1,142 @@ +# SPDX-FileCopyrightText: 2026 - Canonical Ltd +# SPDX-License-Identifier: Apache-2.0 + +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from openstack_hypervisor.mount_validation import ( + is_mounted, + is_usable, + path_declared_in_fstab, + validate_instances_mount, +) + +_INSTANCES_PATH = Path("/var/snap/openstack-hypervisor/common/lib/nova/instances") + + +@pytest.fixture +def fstab(tmp_path): + return tmp_path / "fstab" + + +@pytest.fixture +def instances_dir(tmp_path): + d = tmp_path / "instances" + d.mkdir() + return d + + +class TestPathDeclaredInFstab: + """Tests for path_declared_in_fstab().""" + + def test_returns_false_when_no_matching_entry(self, fstab): + """Returns False when no fstab entry targets the instances path.""" + fstab.write_text("/dev/sda1 / ext4 defaults 0 1\n" "/dev/sda2 /boot ext4 defaults 0 2\n") + assert path_declared_in_fstab(_INSTANCES_PATH, fstab) is False + + def test_returns_true_when_matching_entry_exists(self, fstab): + """Returns True when fstab has a line whose mount target matches.""" + fstab.write_text( + "/dev/sda1 / ext4 defaults 0 1\n" f"/dev/sdb1 {_INSTANCES_PATH} ext4 defaults 0 0\n" + ) + assert path_declared_in_fstab(_INSTANCES_PATH, fstab) is True + + def test_returns_false_when_fstab_unreadable(self, tmp_path): + """Returns False when fstab cannot be read.""" + missing = tmp_path / "not_fstab" + assert path_declared_in_fstab(_INSTANCES_PATH, missing) is False + + def test_ignores_comments_and_blank_lines(self, fstab): + fstab.write_text("\n" "# comment\n" f"/dev/sdb {_INSTANCES_PATH} ext4 defaults 0 0\n") + assert path_declared_in_fstab(_INSTANCES_PATH, fstab) is True + + +class TestIsMounted: + """Tests for is_mounted().""" + + def test_returns_true_when_findmnt_succeeds(self): + """Returns True when findmnt exits with returncode 0.""" + mock_result = MagicMock(returncode=0) + with patch( + "openstack_hypervisor.mount_validation.subprocess.run", return_value=mock_result + ) as mock_run: + assert is_mounted(_INSTANCES_PATH) is True + mock_run.assert_called_once_with( + ["findmnt", "--mountpoint", str(_INSTANCES_PATH)], + capture_output=True, + check=False, + ) + + def test_returns_false_when_findmnt_fails(self): + """Returns False when findmnt exits with a non-zero returncode.""" + mock_result = MagicMock(returncode=1) + with patch( + "openstack_hypervisor.mount_validation.subprocess.run", return_value=mock_result + ): + assert is_mounted(_INSTANCES_PATH) is False + + +class TestIsUsable: + """Tests for is_usable().""" + + def test_returns_true_for_writable_directory(self, instances_dir): + """Returns True when the path is a directory and a file can be created.""" + assert is_usable(instances_dir) is True + + def test_returns_false_when_path_does_not_exist(self, tmp_path): + """Returns False when the path does not exist.""" + assert is_usable(tmp_path / "nonexistent") is False + + def test_returns_false_when_path_is_a_file(self, tmp_path): + """Returns False when the path is a regular file, not a directory.""" + f = tmp_path / "notadir" + f.write_text("content") + assert is_usable(f) is False + + def test_returns_false_when_write_fails(self, instances_dir): + """Returns False when a temporary file cannot be created in the directory.""" + with patch( + "openstack_hypervisor.mount_validation.tempfile.TemporaryFile", + side_effect=OSError("read-only"), + ): + assert is_usable(instances_dir) is False + + +class TestValidateInstancesMount: + """Tests for validate_instances_mount().""" + + def test_returns_true_when_no_fstab_entry(self, fstab): + """Returns True without checking mount when no fstab entry exists.""" + fstab.write_text("/dev/sda1 / ext4 defaults 0 1\n") + with patch("openstack_hypervisor.mount_validation.is_mounted") as mock_mounted: + result = validate_instances_mount(_INSTANCES_PATH, fstab) + assert result is True + mock_mounted.assert_not_called() + + def test_returns_true_when_mounted_and_usable(self, fstab): + """Returns True when fstab entry exists, path is mounted, and writable.""" + fstab.write_text(f"/dev/sdb1 {_INSTANCES_PATH} ext4 defaults 0 0\n") + with patch("openstack_hypervisor.mount_validation.is_mounted", return_value=True), patch( + "openstack_hypervisor.mount_validation.is_usable", return_value=True + ): + assert validate_instances_mount(_INSTANCES_PATH, fstab) is True + + def test_returns_false_when_fstab_entry_but_not_mounted(self, fstab): + """Returns False when fstab entry exists but path is not mounted.""" + fstab.write_text(f"/dev/sdb1 {_INSTANCES_PATH} ext4 defaults 0 0\n") + with patch("openstack_hypervisor.mount_validation.is_mounted", return_value=False), patch( + "openstack_hypervisor.mount_validation.LOG" + ) as mock_log: + assert validate_instances_mount(_INSTANCES_PATH, fstab) is False + assert "not mounted" in mock_log.error.call_args.args[0] + + def test_returns_false_when_mounted_but_not_usable(self, fstab): + """Returns False when path is mounted but not writable.""" + fstab.write_text(f"/dev/sdb1 {_INSTANCES_PATH} ext4 defaults 0 0\n") + with patch("openstack_hypervisor.mount_validation.is_mounted", return_value=True), patch( + "openstack_hypervisor.mount_validation.is_usable", return_value=False + ), patch("openstack_hypervisor.mount_validation.LOG") as mock_log: + assert validate_instances_mount(_INSTANCES_PATH, fstab) is False + assert "not a writable directory" in mock_log.error.call_args.args[0] diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index 77c29fd..33ee764 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -10,6 +10,7 @@ from openstack_hypervisor.services import ( FileTransferService, NovaAPIMetadataService, + NovaComputeService, ) _CERT = base64.b64encode(b"CERT").decode() @@ -39,6 +40,33 @@ def config_file(snap): return path +class TestNovaComputeService: + """Tests for NovaComputeService.""" + + def test_preflight_validates_instances_path(self, snap): + """Preflight validates Nova's instances path before startup.""" + with patch( + "openstack_hypervisor.services.validate_instances_mount", + return_value=True, + ) as mock_validate: + service = NovaComputeService() + assert service.preflight(snap) is True + + mock_validate.assert_called_once_with(snap.paths.common / "lib" / "nova" / "instances") + + def test_run_aborts_when_preflight_fails(self, snap, mocker): + """run() aborts startup when instances mount validation fails.""" + mocker.patch( + "openstack_hypervisor.services.validate_instances_mount", + return_value=False, + ) + mock_run = mocker.patch("openstack_hypervisor.services.subprocess.run") + + assert NovaComputeService().run(snap) == 1 + + mock_run.assert_not_called() + + class TestFileTransferService: """Tests for FileTransferService."""