diff --git a/services/node-agent/src/node_agent.py b/services/node-agent/src/node_agent.py index 2cbb3e2..3ed2d59 100644 --- a/services/node-agent/src/node_agent.py +++ b/services/node-agent/src/node_agent.py @@ -543,6 +543,18 @@ class NodeAgent: f"{VPS_EVENTS_PATH}/{self.node_name}/") cmd = [ "rsync", "-az", "--remove-source-files", + # --omit-dir-times: the remote per-node event dir on VPS is owned + # by aerbot:aerbot; the ssh user (oskar) is only a group member, + # so it can write files into the dir but cannot chown/chmod/touch + # the dir itself. -a implies -t (preserve times) which also + # applies to directories, so rsync would otherwise try to set the + # dir's mtime, get EPERM, and return non-zero even though every + # file transferred fine. Files still keep their mtime via -t; + # this only skips the (harmless, doomed) directory mtime set. + # TODO tech-debt: fix ownership of /opt/homelab/events/ on + # VPS so this workaround isn't needed (see docs/backlog.md, + # "Tech-debt: globalny porządek uid/gid/uprawnień"). + "--omit-dir-times", # -F /dev/null: skip ~/.ssh/config entirely. The .ssh dir is # mounted from the host oskar user into the container which runs # as root; OpenSSH rejects config files owned by a different UID. diff --git a/services/node-agent/tests/conftest.py b/services/node-agent/tests/conftest.py new file mode 100644 index 0000000..8b6e6f3 --- /dev/null +++ b/services/node-agent/tests/conftest.py @@ -0,0 +1,28 @@ +"""Shared fixtures for node-agent tests.""" +from __future__ import annotations + +import os +import sys +import tempfile +from pathlib import Path + +import pytest + +_SRC = Path(__file__).resolve().parents[1] / "src" +sys.path.insert(0, str(_SRC)) + +# node_agent.py reads RUNTIME_PATH/NODE_NAME as module-level constants at +# import time, so these must be set before the first `import node_agent`. +_RUNTIME = Path(tempfile.mkdtemp(prefix="node-agent-test-")) +os.environ.setdefault("RUNTIME_PATH", str(_RUNTIME)) +os.environ.setdefault("NODE_NAME", "test-node") + +import node_agent # noqa: E402 + + +@pytest.fixture +def agent(monkeypatch): + monkeypatch.setattr(node_agent, "VPS_EVENTS_HOST", "vps.example.com") + monkeypatch.setattr(node_agent, "VPS_EVENTS_USER", "oskar") + monkeypatch.setattr(node_agent, "VPS_EVENTS_PATH", "/opt/homelab/events") + return node_agent.NodeAgent() diff --git a/services/node-agent/tests/test_ship_events_to_vps.py b/services/node-agent/tests/test_ship_events_to_vps.py new file mode 100644 index 0000000..d8e4242 --- /dev/null +++ b/services/node-agent/tests/test_ship_events_to_vps.py @@ -0,0 +1,59 @@ +"""Tests for NodeAgent._ship_events_to_vps rsync command construction. + +Covers the fix for the false "Event shipping failed" warning: the remote +per-node events dir on VPS is owned by aerbot:aerbot, so the ssh user (a +group member, not the owner) can write files into it but cannot touch the +dir's own mtime. -a implies -t (preserve times), which rsync also applies to +directories, so without --omit-dir-times rsync returns a non-zero exit code +on every run even though the file transfer itself succeeded. +""" +from __future__ import annotations + +from unittest.mock import MagicMock + +import node_agent + + +def test_omits_dir_times_flag(agent, monkeypatch): + captured = {} + + def fake_run(cmd, **kwargs): + captured["cmd"] = cmd + return MagicMock(returncode=0, stderr="") + + monkeypatch.setattr(node_agent.subprocess, "run", fake_run) + agent._ship_events_to_vps() + + assert "--omit-dir-times" in captured["cmd"] + + +def test_skips_when_vps_host_not_set(agent, monkeypatch): + monkeypatch.setattr(node_agent, "VPS_EVENTS_HOST", "") + fake_run = MagicMock() + monkeypatch.setattr(node_agent.subprocess, "run", fake_run) + + agent._ship_events_to_vps() + + fake_run.assert_not_called() + + +def test_skips_on_vps_node(agent, monkeypatch): + agent.node_name = node_agent.VPS_NODE_NAME + fake_run = MagicMock() + monkeypatch.setattr(node_agent.subprocess, "run", fake_run) + + agent._ship_events_to_vps() + + fake_run.assert_not_called() + + +def test_nonzero_returncode_logs_warning(agent, monkeypatch, caplog): + def fake_run(cmd, **kwargs): + return MagicMock(returncode=23, stderr="rsync: some partial transfer error") + + monkeypatch.setattr(node_agent.subprocess, "run", fake_run) + + with caplog.at_level("WARNING"): + agent._ship_events_to_vps() + + assert "Event shipping failed" in caplog.text