From 663f4d240e150f767e5bc4ec0149d12b1e694141 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 24 Jul 2026 02:00:54 +0900 Subject: [PATCH 1/8] feat(launcher): honor configured JENKINS_HOME in agent script Removes the hardcoded /var/lib/jenkins path in the launcher script and uses the JENKINS_HOME environment variable. The unit already exports the configured home. The script still falls back to /var/lib/jenkins if invoked directly, preserving backward compatibility. --- src/service.py | 7 +++++-- templates/jenkins_agent.sh.j2 | 2 +- tests/unit/test_service.py | 37 +++++++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/service.py b/src/service.py index 80a74c4..8e593bf 100644 --- a/src/service.py +++ b/src/service.py @@ -184,10 +184,13 @@ def _sync_service_files(self) -> bool: ) unit_changed = self._write_if_changed(JENKINS_AGENT_SYSTEMD_PATH, service_content, 0o644) - # Render the agent script template with websocket_mode config + # Render the agent script template with websocket_mode config and home dir websocket_mode = self.state.websocket_mode script_template = self._template_loader.get_template("jenkins_agent.sh.j2") - script_content = script_template.render(websocket_mode=websocket_mode) + script_content = script_template.render( + websocket_mode=websocket_mode, + jenkins_home=str(self.state.jenkins_home), + ) script_changed = self._write_if_changed( JENKINS_AGENT_START_SCRIPT_PATH, script_content, 0o755 ) diff --git a/templates/jenkins_agent.sh.j2 b/templates/jenkins_agent.sh.j2 index d1c2fb4..f353df1 100755 --- a/templates/jenkins_agent.sh.j2 +++ b/templates/jenkins_agent.sh.j2 @@ -16,7 +16,7 @@ set -eu -o pipefail export LC_ALL=C export TERM=xterm -readonly JENKINS_HOME="/var/lib/jenkins" +JENKINS_HOME="${JENKINS_HOME:-{{ jenkins_home|default('/var/lib/jenkins', true) }}}" info "Installing on arch: $(uname -m)" diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 0a26c0c..71108a4 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -208,6 +208,43 @@ def test_install_renders_custom_user_and_workdir( assert "ExecStopPost=rm -rf /srv/jenkins/.ready" in unit_text +def test_install_renders_script_with_jenkins_home( + harness: ops.testing.Harness, monkeypatch: pytest.MonkeyPatch, tmp_path: Path +): + """ + arrange: Harness with patched install paths and templates. + act: run the install hook. + assert: the launcher script references the configured JENKINS_HOME. + """ + host = _mock_install_host(monkeypatch, tmp_path) + apt_add_package_mock = MagicMock() + monkeypatch.setattr(apt, "add_package", apt_add_package_mock) + + harness.update_config({"jenkins_home": "/srv/jenkins"}) + harness.begin_with_initial_hooks() + script_text = Path(host.script_path).read_text() + + assert 'JENKINS_HOME="${JENKINS_HOME:-/srv/jenkins}"' in script_text + + +def test_install_renders_script_with_default_jenkins_home( + harness: ops.testing.Harness, monkeypatch: pytest.MonkeyPatch, tmp_path: Path +): + """ + arrange: Harness without any jenkins_home override. + act: run the install hook. + assert: the launcher script references the default JENKINS_HOME. + """ + host = _mock_install_host(monkeypatch, tmp_path) + apt_add_package_mock = MagicMock() + monkeypatch.setattr(apt, "add_package", apt_add_package_mock) + + harness.begin_with_initial_hooks() + script_text = Path(host.script_path).read_text() + + assert 'JENKINS_HOME="${JENKINS_HOME:-/var/lib/jenkins}"' in script_text + + def test_restart_service( harness: ops.testing.Harness, monkeypatch: pytest.MonkeyPatch, From bca43468a70b547f4dee5a91afa55565e049a26b Mon Sep 17 00:00:00 2001 From: Yanks Yoon <37652070+yanksyoon@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:30:00 +0800 Subject: [PATCH 2/8] feat(service): ensure agent user exists and owns JENKINS_HOME (#176) * feat(service): ensure agent user exists and owns JENKINS_HOME When agent_user is non-root, the charm now creates the user if missing and ensures JENKINS_HOME is owned by that user. Failures are logged as warnings so that pre-created users/homes do not block reconcile, matching the warn-and-continue preference. * feat(service): ensure agent user exists and owns JENKINS_HOME Adds _ensure_user_and_home() to create the configured agent_user and chown jenkins_home on every reconcile. Failures are logged and continue, honoring the warn-and-continue preference. Unit tests use a fake useradd/pwd lookup and assert os.chown arguments so they pass on macOS and in CI. * feat(service): parameterize file ownership in _render_file (#177) * feat(service): parameterize file ownership in _render_file The previous implementation always chowned rendered files to root. This is correct for systemd unit files and the launcher script (systemd runs as root), but made it impossible for user-owned files to keep their owner. The helper now accepts an optional owner argument so future code paths can render files as the configured agent user while the service files remain root-owned. * ci: debug * feat: jenkins user w/ passwordless sudo * ci: debug * ci: revert debug * ci(workflow): disable tmate debugging sessions Tmate creates interactive tmux sessions that cannot be driven by automation. Rely on captured Juju and pytest logs for diagnostics instead. * test(integration): add diagnostics to traefik ingress test Capture Jenkins client URL, Juju status, model debug logs and traefik proxied endpoints when the Jenkins API connection drops during the agent job-execution assertion. This helps identify whether the failure is the server pod IP, ingress routing, or agent connectivity. --- .github/workflows/integration_test.yaml | 3 + charmcraft.yaml | 10 +- src/charm_state.py | 8 +- src/service.py | 168 +++++++++++----- tests/integration/test_agent.py | 71 +++++-- tests/unit/test_service.py | 247 +++++++++++++++++++++++- 6 files changed, 437 insertions(+), 70 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 1229d82..fb79d45 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -17,6 +17,9 @@ jobs: channel: 1.34-strict/edge self-hosted-runner: true with-uv: true + # Explicitly disable tmate debugging sessions. They are interactive and + # cannot be driven by automation; rely on captured logs instead. + tmate-debug: false integration-tests-non-amd64: strategy: matrix: diff --git a/charmcraft.yaml b/charmcraft.yaml index c4f3071..b89e97f 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -63,12 +63,12 @@ config: default to the agents hardware identifier, e.g.: 'x86_64' agent_user: type: string - default: root + default: jenkins description: | - OS user that runs the Jenkins agent systemd service. Defaults to root. - If changed to a non-root user, the charm will ensure the user exists and - that JENKINS_HOME is owned by this user. Note that the systemd unit file - itself is always owned by root. + OS user that runs the Jenkins agent systemd service. Defaults to jenkins. + The charm ensures the user exists, owns JENKINS_HOME, and is granted + passwordless sudo. Note that the systemd unit file itself is always owned + by root. jenkins_home: type: string default: /var/lib/jenkins diff --git a/src/charm_state.py b/src/charm_state.py index 2969702..ae0133c 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -133,7 +133,7 @@ class State: unit_data: UnitData websocket_mode: bool jenkins_agent_service_name: str = "jenkins-agent" - agent_user: str = "root" + agent_user: str = "jenkins" jenkins_home: Path = Path("/var/lib/jenkins") @classmethod @@ -181,8 +181,10 @@ def from_charm(cls, charm: ops.CharmBase) -> "State": websocket_mode = bool(charm.model.config.get("websocket_mode", True)) # Get user/home config - agent_user = str(charm.model.config.get("agent_user", "root") or "root") - jenkins_home = Path(str(charm.model.config.get("jenkins_home", "/var/lib/jenkins") or "/var/lib/jenkins")) + agent_user = str(charm.model.config.get("agent_user", "jenkins") or "jenkins") + jenkins_home = Path( + str(charm.model.config.get("jenkins_home", "/var/lib/jenkins") or "/var/lib/jenkins") + ) return cls( agent_meta=agent_meta, diff --git a/src/service.py b/src/service.py index 8e593bf..4aa40e0 100644 --- a/src/service.py +++ b/src/service.py @@ -7,6 +7,9 @@ import os import pwd import re + +# Bandit flags the subprocess import; useradd/visudo are trusted fixed-path system binaries. +import subprocess # nosec: B404 import time import typing from pathlib import Path @@ -19,7 +22,7 @@ logger = logging.getLogger(__name__) AGENT_SERVICE_NAME = "jenkins-agent" -REQUIRED_PACKAGES = ["openjdk-21-jre"] +REQUIRED_PACKAGES = ["openjdk-21-jre", "sudo"] SYSTEMD_SERVICE_CONF_DIR = "/etc/systemd/system/jenkins-agent.service.d/" STARTUP_CHECK_TIMEOUT = 30 STARTUP_CHECK_INTERVAL = 2 @@ -27,6 +30,7 @@ JENKINS_AGENT_SYSTEMD_PATH = Path("/etc/systemd/system/jenkins-agent.service") JENKINS_AGENT_START_SCRIPT_PATH = Path("/usr/bin/jenkins-agent") AGENT_READY_PATH = Path(JENKINS_HOME / ".ready") +SUDOERS_DROP_IN_DIR = Path("/etc/sudoers.d") # Pattern for systemd Environment="KEY=VALUE" lines. _SYSTEMD_ENV_PATTERN = re.compile(r'^Environment="([^=]+)=(.*)"$') @@ -79,25 +83,21 @@ def __init__(self, state: State): state: The Jenkins agent state. """ self.state = state - # The templates render systemd/shell configuration, not HTML/XML. HTML - # autoescaping would corrupt credential values containing characters such - # as & < > ' " (e.g. turning a token's '&' into '&'), so escaping is - # disabled for every template extension. self._template_loader = jinja2.Environment( - loader=jinja2.FileSystemLoader(searchpath="templates"), - autoescape=jinja2.select_autoescape( - enabled_extensions=(), default_for_string=False, default=False - ), + loader=jinja2.FileSystemLoader("templates"), autoescape=True ) - def _render_file(self, path: Path, content: str, mode: int) -> None: - """Write a content rendered from a template to a file. + def _render_file(self, path: Path, content: str, mode: int, owner: str = "root") -> None: + """Write a file to disk, setting its mode and ownership. Args: - path: Path object to the file. - content: the data to be written to the file. - mode: access permission mask applied to the - file using chmod (e.g. 0o640). + path: target file path. + content: file content to write. + mode: permission bits to apply (e.g. 0o640). + owner: POSIX username the file should be owned by. Defaults to root, + which is appropriate for systemd unit files and the launcher script + because systemd itself runs as root and only drops privileges when + executing the service (see the `User=` directive in the unit). Raises: FileRenderError: if interaction with the filesystem fails @@ -105,15 +105,34 @@ def _render_file(self, path: Path, content: str, mode: int) -> None: try: path.write_text(content) os.chmod(path, mode) - # Get the uid/gid for the root user (running the service). - # TODO: the user running the jenkins agent is currently root - # we should replace this by defining a dedicated user in the apt package - u = pwd.getpwnam("root") - # Set the correct ownership for the file. - os.chown(path, uid=u.pw_uid, gid=u.pw_gid) + user_info = pwd.getpwnam(owner) + os.chown(path, uid=user_info.pw_uid, gid=user_info.pw_gid) except (OSError, KeyError, TypeError) as exc: raise FileRenderError(f"Error rendering file:\n{exc}") from exc + def _write_if_changed(self, path: Path, content: str, mode: int, owner: str = "root") -> bool: + """Render content to a file only when it differs from what is on disk. + + Args: + path: Destination file path. + content: Desired file content. + mode: Access permission mask applied when the file is (re)written. + owner: POSIX username the file should be owned by. + + Returns: + True if the file was created or its content changed, False otherwise. + + Raises: + FileRenderError: if reading the existing file from disk fails. + """ + try: + if path.exists() and path.read_text(encoding="utf-8") == content: + return False + except OSError as exc: + raise FileRenderError(f"Error reading file:\n{exc}") from exc + self._render_file(path, content, mode, owner=owner) + return True + @property def is_active(self) -> bool: """Indicate if the jenkins agent service is active.""" @@ -143,28 +162,6 @@ def credentials_changed(self, credentials: Credentials) -> bool: or current_env.get("JENKINS_TOKEN") != credentials.secret ) - def _write_if_changed(self, path: Path, content: str, mode: int) -> bool: - """Render content to a file only when it differs from what is on disk. - - Args: - path: Destination file path. - content: Desired file content. - mode: Access permission mask applied when the file is (re)written. - - Returns: - True if the file was created or its content changed, False otherwise. - - Raises: - FileRenderError: if reading the existing file from disk fails. - """ - try: - if path.exists() and path.read_text(encoding="utf-8") == content: - return False - except OSError as exc: - raise FileRenderError(f"Error reading file:\n{exc}") from exc - self._render_file(path, content, mode) - return True - def _sync_service_files(self) -> bool: """Write the systemd unit and its launcher script if they've changed. @@ -222,6 +219,7 @@ def install(self) -> None: failed. """ unit_file_changed = self._sync_service_files() + self._ensure_user_and_home() if unit_file_changed: try: systemd.daemon_reload() @@ -238,6 +236,86 @@ def install(self) -> None: except (apt.PackageError, apt.PackageNotFoundError) as exc: raise PackageInstallError("Error installing the Java package") from exc + def _ensure_user_and_home(self) -> None: + """Ensure the configured agent user exists, owns the home, and can sudo. + + Does nothing for the root user. For non-root users, the user is created + (regular user, not a system account) with the configured home directory if + missing, the home directory is created and owned by the user, and a + passwordless sudo entry is written to /etc/sudoers.d. Failures are logged + but not raised, to keep the charm from hard-blocking when an operator has + pre-created the user/home. + """ + username = self.state.agent_user + home = self.state.jenkins_home + if username == "root": + return + + try: + pwd.getpwnam(username) + except KeyError: + logger.info("Creating user %s", username) + try: + subprocess.run( # nosec: B603 - fixed-path system binary (useradd) + [ + "/usr/sbin/useradd", + "--home-dir", + str(home), + "--create-home", + "--shell", + "/bin/bash", + username, + ], + check=True, + capture_output=True, + ) + except (subprocess.CalledProcessError, FileNotFoundError) as exc: + logger.warning("Failed to create user %s: %s", username, exc) + return + try: + home.mkdir(parents=True, exist_ok=True) + user_info = pwd.getpwnam(username) + os.chown(home, uid=user_info.pw_uid, gid=user_info.pw_gid) + # Also chown existing contents if the directory already existed with + # different ownership, as the agent expects write access to its home. + for path in home.rglob("*"): + os.chown(path, uid=user_info.pw_uid, gid=user_info.pw_gid) + except (OSError, KeyError) as exc: + logger.warning("Failed to set ownership of %s to %s: %s", home, username, exc) + + self._grant_passwordless_sudo(username) + + def _grant_passwordless_sudo(self, username: str) -> None: + """Write a sudoers drop-in granting the user passwordless sudo. + + The file is checked with visudo before it is installed so a syntax error + does not lock operators out of sudo. Failures are logged and do not block + reconcile. + + Args: + username: the POSIX username to grant sudo. + """ + sudoers_content = f"{username} ALL=(ALL:ALL) NOPASSWD: ALL\n" + drop_in_path = SUDOERS_DROP_IN_DIR / f"99-jenkins-agent-{username}" + try: + subprocess.run( # nosec: B603 - fixed-path system binary (visudo) + ["/usr/sbin/visudo", "-cf", "-"], + input=sudoers_content, + check=True, + capture_output=True, + text=True, + ) + except (subprocess.CalledProcessError, FileNotFoundError) as exc: + logger.warning("Generated sudoers content failed validation: %s", exc) + return + try: + SUDOERS_DROP_IN_DIR.mkdir(parents=True, exist_ok=True) + drop_in_path.write_text(sudoers_content) + os.chmod(drop_in_path, 0o440) + os.chown(drop_in_path, uid=0, gid=0) + except (OSError, KeyError) as exc: + logger.warning("Failed to write sudoers drop-in %s: %s", drop_in_path, exc) + def restart(self) -> None: """Start the agent service. @@ -277,7 +355,7 @@ def restart(self) -> None: "Error interacting with the filesystem when rendering configuration file" ) from exc - # Check if the service is running after startup + # Check if the service running after startup if not self._startup_check(): raise ServiceRestartError("Error waiting for the agent service to start") @@ -289,7 +367,7 @@ def reset_failed_state(self) -> None: so we need to do it manually. """ try: - # Disable protected-access here because reset-failed is not implemented in the lib + # Disable protected_access here because reset-failed is not implemented in the lib systemd._systemctl("reset-failed", AGENT_SERVICE_NAME) # pylint: disable=W0212 except systemd.SystemdError: # We only log the exception here as this is not critical diff --git a/tests/integration/test_agent.py b/tests/integration/test_agent.py index 3247537..f559569 100644 --- a/tests/integration/test_agent.py +++ b/tests/integration/test_agent.py @@ -194,6 +194,8 @@ def test_agent_traefik_ingress( jenkins_agent_requirer: str, jenkins_client: jenkinsapi.jenkins.Jenkins, juju: jubilant.Juju, + microk8s_juju: jubilant.Juju, + traefik_k8s_application: str, ): """ Verify agent connects successfully through traefik ingress using WebSocket. @@ -217,6 +219,63 @@ def test_agent_traefik_ingress( - logs do NOT show "port:50000 is not reachable" (the bug from issue #165) - agent can execute jobs successfully (functional verification) """ + # ruff: noqa: C901 + # Diagnostic-heavy test; complexity comes from explicit failure logging. + + def _dump_diagnostics(): + """Dump model and application state to aid debugging connection failures.""" + logger.error("=== Jenkins API connection failure diagnostics ===") + logger.error("Jenkins client URL: %s", jenkins_client.base_server_url()) + try: + logger.error("LXD model status:\n%s", juju.status()) + except Exception as exc: # nosec B110 + logger.error("Failed to dump LXD model status: %s", exc) + try: + logger.error( + "LXD model debug log:\n%s", + juju.cli("debug-log", "--replay", "--no-tail", "--limit", "200"), + ) + except Exception as exc: # nosec B110 + logger.error("Failed to dump LXD model debug log: %s", exc) + try: + logger.error("MicroK8s model status:\n%s", microk8s_juju.status()) + except Exception as exc: # nosec B110 + logger.error("Failed to dump MicroK8s model status: %s", exc) + try: + logger.error( + "MicroK8s model debug log:\n%s", + microk8s_juju.cli("debug-log", "--replay", "--no-tail", "--limit", "200"), + ) + except Exception as exc: # nosec B110 + logger.error("Failed to dump MicroK8s model debug log: %s", exc) + try: + traefik_status = microk8s_juju.run( + f"{traefik_k8s_application}/0", "show-proxied-endpoints" + ) + logger.error("Traefik proxied endpoints:\n%s", traefik_status) + except Exception as exc: # nosec B110 + logger.error("Failed to dump traefik proxied endpoints: %s", exc) + logger.error("=== end diagnostics ===") + + def _run_test_job(agent_name: str): + """Run the Jenkins test job and dump diagnostics on connection failure.""" + logger.info("Agent %s is online, running test job...", agent_name) + try: + assert_job_success( + client=jenkins_client, + agent_name=agent_name, + test_target_label="machine", + ) + except requests.exceptions.ConnectionError as exc: + _dump_diagnostics() + raise AssertionError( + f"Jenkins API connection failed while running test job against " + f"{jenkins_client.base_server_url()}: {exc}" + ) from exc + logger.info( + "✓ Traefik ingress test passed: agent connected via WebSocket and executed job" + ) + # Relate agent to ingressed Jenkins server (if not already related) logger.info("Ensuring jenkins-agent is related to ingressed jenkins-k8s...") try: @@ -284,17 +343,7 @@ def test_agent_traefik_ingress( assert len(agent_nodes) == 1, f"Expected one agent node, found {len(agent_nodes)}" agent_name = agent_nodes[0].name - logger.info("Agent %s is online, running test job...", agent_name) - - # Run a test job to verify the agent can execute work - assert_job_success( - client=jenkins_client, - agent_name=agent_name, - test_target_label="machine", - ) - logger.info( - "✓ Traefik ingress test passed: agent connected via WebSocket and executed job" - ) + _run_test_job(agent_name) except requests.exceptions.HTTPError as e: # Jenkins API access may be limited through ingress - the core test (WebSocket connection) passed logger.warning("Jenkins API access limited through ingress (expected): %s", e) diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 71108a4..cf4e87f 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -7,7 +7,12 @@ from __future__ import annotations +import logging import os +import pwd + +# Bandit flags subprocess in tests; it is only used to build command lists for unit-test mocks. +import subprocess # nosec: B404 from pathlib import Path from types import SimpleNamespace from typing import TYPE_CHECKING @@ -26,7 +31,11 @@ def _mock_install_host( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path, *, package_installed: bool = False + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + *, + package_installed: bool = False, + mock_fs_ownership: bool = True, ) -> SimpleNamespace: """Mock host syscalls so JenkinsAgentService.install runs off-host. @@ -34,6 +43,9 @@ def _mock_install_host( monkeypatch: pytest monkeypatch fixture. tmp_path: temporary directory the service files are written to. package_installed: whether the required apt package is already present. + mock_fs_ownership: whether to mock os.chmod/os.chown. Disabling it leaves + os.chmod untouched but still stubs os.chown, so tests can assert real + filesystem writes without requiring root privileges to change owners. Returns: Namespace of the patched systemd entry points (daemon_reload, service_enable) @@ -43,8 +55,11 @@ def _mock_install_host( script_path = tmp_path / "jenkins-agent" monkeypatch.setattr(service, "JENKINS_AGENT_SYSTEMD_PATH", unit_path) monkeypatch.setattr(service, "JENKINS_AGENT_START_SCRIPT_PATH", script_path) - monkeypatch.setattr(os, "chmod", MagicMock()) - monkeypatch.setattr(os, "chown", MagicMock()) + if mock_fs_ownership: + monkeypatch.setattr(os, "chmod", MagicMock()) + monkeypatch.setattr(os, "chown", MagicMock()) + else: + monkeypatch.setattr(os, "chown", MagicMock()) daemon_reload = MagicMock() service_enable = MagicMock() monkeypatch.setattr(systemd, "daemon_reload", daemon_reload) @@ -101,7 +116,7 @@ def test_on_install(harness: ops.testing.Harness, monkeypatch: pytest.MonkeyPatc # The package is absent (mock always raises), so every reconcile installs it # with the expected package list. assert apt_add_package_mock.call_count >= 1 - assert apt_add_package_mock.call_args_list[0][0][0] == ["openjdk-21-jre"] + assert apt_add_package_mock.call_args_list[0][0][0] == ["openjdk-21-jre", "sudo"] # The unit file is newly written, so the service is reloaded and enabled for # automatic start on reboot. assert host.service_enable.call_count >= 1 @@ -179,8 +194,8 @@ def test_install_renders_user_and_workdir( harness.begin_with_initial_hooks() unit_text = host.unit_path.read_text() - assert "User=root" in unit_text - assert "Group=root" in unit_text + assert "User=jenkins" in unit_text + assert "Group=jenkins" in unit_text assert "WorkingDirectory=/var/lib/jenkins" in unit_text assert 'Environment="JENKINS_HOME=/var/lib/jenkins"' in unit_text @@ -245,6 +260,226 @@ def test_install_renders_script_with_default_jenkins_home( assert 'JENKINS_HOME="${JENKINS_HOME:-/var/lib/jenkins}"' in script_text +def test_install_creates_user_and_home( + harness: ops.testing.Harness, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +): + """ + arrange: Harness with agent_user=jenkins and jenkins_home pointing inside tmp_path. + act: run the install hook. + assert: the home directory is created and the charm calls os.chown using the + created jenkins uid/gid. + """ + home = tmp_path / "jenkins-home" + _mock_install_host(monkeypatch, tmp_path) + monkeypatch.setattr(apt, "add_package", MagicMock()) + _make_fake_useradd(monkeypatch) + chown_mock = MagicMock() + monkeypatch.setattr(os, "chown", chown_mock) + + harness.update_config({"agent_user": "jenkins", "jenkins_home": str(home)}) + harness.begin_with_initial_hooks() + + assert home.exists() + jenkins_uid = pwd.getpwnam("jenkins").pw_uid + jenkins_gid = pwd.getpwnam("jenkins").pw_gid + chown_mock.assert_any_call(home, uid=jenkins_uid, gid=jenkins_gid) + + +def test_install_warns_but_continues_on_useradd_failure( + harness: ops.testing.Harness, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +): + """ + arrange: Harness with agent_user=nonexistent-testuser and useradd mocked to fail. + act: run the install hook. + assert: the charm emits a warning but does not error. + """ + username = "nonexistent-testuser" + home = tmp_path / f"{username}-home" + _mock_install_host(monkeypatch, tmp_path) + monkeypatch.setattr(apt, "add_package", MagicMock()) + monkeypatch.setattr( + subprocess, "run", MagicMock(side_effect=subprocess.CalledProcessError(1, ["useradd"])) + ) + + harness.update_config({"agent_user": username, "jenkins_home": str(home)}) + harness.begin_with_initial_hooks() + + warning_messages = [ + message for _, level, message in caplog.record_tuples if level == logging.WARNING + ] + assert any(f"Failed to create user {username}" in message for message in warning_messages) + + +def _make_fake_useradd(monkeypatch: pytest.MonkeyPatch) -> None: + """Provide an in-memory useradd that creates entries seen by pwd.getpwnam.""" + user_db: dict[str, pwd.struct_passwd] = {} + + def fake_run(args, **_kwargs): + # Validate expected useradd shape: + # /usr/sbin/useradd --home-dir HOME --create-home --shell /bin/bash USER + if len(args) < 7 or not args[0].endswith("useradd"): + raise subprocess.CalledProcessError(1, args) + if "--system" in args: + raise subprocess.CalledProcessError(1, args) + if "--create-home" not in args or "--shell" not in args or "/bin/bash" not in args: + raise subprocess.CalledProcessError(1, args) + username = args[-1] + try: + home = args[args.index("--home-dir") + 1] + except (ValueError, IndexError) as exc: + raise subprocess.CalledProcessError(1, args) from exc + if username in user_db: + raise subprocess.CalledProcessError(9, args) + # Pick deterministic fake uid/gid based on username hash to avoid collisions. + uid = 50000 + hash(username) % 10000 + gid = uid + user_db[username] = pwd.struct_passwd((username, "x", uid, gid, "", home, "/bin/bash")) + return subprocess.CompletedProcess(args, 0, "", "") + + real_getpwnam = pwd.getpwnam + + def fake_getpwnam(username: str): + if username in user_db: + return user_db[username] + return real_getpwnam(username) + + monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr(pwd, "getpwnam", fake_getpwnam) + + +def test_render_file_uses_configured_owner( + harness: ops.testing.Harness, monkeypatch: pytest.MonkeyPatch, tmp_path: Path +): + """ + arrange: Harness with agent_user=jenkins and patched install paths. + act: run the install hook. + assert: the systemd unit and launcher script are chowned to root; the home dir + is chowned to jenkins. + """ + from unittest.mock import call + + home = tmp_path / "jenkins-home" + host = _mock_install_host(monkeypatch, tmp_path) + monkeypatch.setattr(apt, "add_package", MagicMock()) + _make_fake_useradd(monkeypatch) + chown_mock = MagicMock() + monkeypatch.setattr(os, "chown", chown_mock) + + harness.update_config({"agent_user": "jenkins", "jenkins_home": str(home)}) + harness.begin_with_initial_hooks() + + jenkins_uid = pwd.getpwnam("jenkins").pw_uid + jenkins_gid = pwd.getpwnam("jenkins").pw_gid + assert call(host.unit_path, uid=0, gid=0) in chown_mock.call_args_list + assert call(host.script_path, uid=0, gid=0) in chown_mock.call_args_list + assert call(home, uid=jenkins_uid, gid=jenkins_gid) in chown_mock.call_args_list + + +def test_ensure_user_chowns_existing_home_contents( + harness: ops.testing.Harness, monkeypatch: pytest.MonkeyPatch, tmp_path: Path +): + """ + arrange: Harness with agent_user=jenkins, jenkins_home pre-existing with a file. + act: run the install hook. + assert: existing contents under the home are chowned to jenkins. + """ + from unittest.mock import call + + home = tmp_path / "jenkins-home" + home.mkdir(parents=True) + existing = home / "existing.txt" + existing.write_text("old") + _mock_install_host(monkeypatch, tmp_path) + monkeypatch.setattr(apt, "add_package", MagicMock()) + _make_fake_useradd(monkeypatch) + chown_mock = MagicMock() + monkeypatch.setattr(os, "chown", chown_mock) + + harness.update_config({"agent_user": "jenkins", "jenkins_home": str(home)}) + harness.begin_with_initial_hooks() + + jenkins_uid = pwd.getpwnam("jenkins").pw_uid + jenkins_gid = pwd.getpwnam("jenkins").pw_gid + assert call(existing, uid=jenkins_uid, gid=jenkins_gid) in chown_mock.call_args_list + assert call(home, uid=jenkins_uid, gid=jenkins_gid) in chown_mock.call_args_list + + +def test_install_grants_passwordless_sudo( + harness: ops.testing.Harness, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +): + """ + arrange: Harness with agent_user=jenkins and a temporary sudoers.d directory. + act: run the install hook. + assert: a sudoers drop-in is written with the correct NOPASSWD rule and is + owned/mode-ed by root. + """ + home = tmp_path / "jenkins-home" + sudoers_d = tmp_path / "sudoers.d" + _mock_install_host(monkeypatch, tmp_path, mock_fs_ownership=False) + monkeypatch.setattr(apt, "add_package", MagicMock()) + monkeypatch.setattr(service, "REQUIRED_PACKAGES", []) + monkeypatch.setattr(service, "SUDOERS_DROP_IN_DIR", sudoers_d) + _make_fake_useradd(monkeypatch) + + real_subprocess_run = subprocess.run + + def fake_subprocess(args, **kwargs): + if args and args[0].endswith("visudo"): + return subprocess.CompletedProcess(args, 0, "", "") + return real_subprocess_run(args, **kwargs) + + monkeypatch.setattr(subprocess, "run", fake_subprocess) + + harness.update_config({"agent_user": "jenkins", "jenkins_home": str(home)}) + harness.begin_with_initial_hooks() + + drop_in_path = sudoers_d / "99-jenkins-agent-jenkins" + assert drop_in_path.exists() + assert drop_in_path.read_text() == "jenkins ALL=(ALL:ALL) NOPASSWD: ALL\n" + + +def test_install_warns_on_visudo_failure( + harness: ops.testing.Harness, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +): + """ + arrange: Harness where visudo rejects the generated sudoers content. + act: run the install hook. + assert: a warning is logged and reconcile does not raise. + """ + home = tmp_path / "jenkins-home" + _mock_install_host(monkeypatch, tmp_path) + monkeypatch.setattr(apt, "add_package", MagicMock()) + monkeypatch.setattr(service, "REQUIRED_PACKAGES", []) + _make_fake_useradd(monkeypatch) + + real_subprocess_run = subprocess.run + + def fake_subprocess(args, **kwargs): + if args and args[0].endswith("visudo"): + raise subprocess.CalledProcessError(1, args) + return real_subprocess_run(args, **kwargs) + + monkeypatch.setattr(subprocess, "run", fake_subprocess) + + harness.update_config({"agent_user": "jenkins", "jenkins_home": str(home)}) + harness.begin_with_initial_hooks() + + warning_messages = [ + message for _, level, message in caplog.record_tuples if level == logging.WARNING + ] + assert any("sudoers content failed validation" in message for message in warning_messages) + + def test_restart_service( harness: ops.testing.Harness, monkeypatch: pytest.MonkeyPatch, From 364c616cc34888a109133ab243f33dc13d776c7a Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 10 Aug 2026 15:27:08 +0000 Subject: [PATCH 3/8] test: log Jenkins queue state before waiting --- tests/integration/test_agent.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/integration/test_agent.py b/tests/integration/test_agent.py index f559569..6068322 100644 --- a/tests/integration/test_agent.py +++ b/tests/integration/test_agent.py @@ -77,6 +77,23 @@ def assert_job_success( """ job = client.create_job(agent_name, _gen_test_job_xml(test_target_label)) queue_item = job.invoke() + try: + queue_item.poll() + node = client.get_node(agent_name) + logger.info( + "Queued Jenkins job %s: queue_id=%s age=%.1fs why=%r blocked=%s " + "stuck=%s buildable=%s agent_online=%s", + job.name, + queue_item.queue_id, + queue_item.get_age(), + queue_item.why, + queue_item.is_blocked, + queue_item.is_stuck, + queue_item.is_buildable, + node.is_online(), + ) + except Exception as exc: # nosec B110 - diagnostics must not mask the test result + logger.warning("Unable to collect Jenkins queue diagnostics: %s", exc) queue_item.block_until_complete() build: jenkinsapi.build.Build = queue_item.get_build() assert build.get_status() == "SUCCESS" From c4688a5bc83ffe7200eb18aba84fa85dd921e4bf Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 10 Aug 2026 20:37:56 +0000 Subject: [PATCH 4/8] test: capture Jenkins API diagnostics on status failures --- tests/integration/test_agent.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/integration/test_agent.py b/tests/integration/test_agent.py index 6068322..5f1bab0 100644 --- a/tests/integration/test_agent.py +++ b/tests/integration/test_agent.py @@ -361,6 +361,11 @@ def _run_test_job(agent_name: str): agent_name = agent_nodes[0].name _run_test_job(agent_name) + except requests.exceptions.ConnectionError as exc: + _dump_diagnostics() + raise AssertionError( + f"Jenkins API connection failed while checking agent status: {exc}" + ) from exc except requests.exceptions.HTTPError as e: # Jenkins API access may be limited through ingress - the core test (WebSocket connection) passed logger.warning("Jenkins API access limited through ingress (expected): %s", e) From e21c4ea905e2299a9f9b1387f6f3802603cadb3f Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 10 Aug 2026 21:03:41 +0000 Subject: [PATCH 5/8] test: capture wrapped Jenkins API failures --- tests/integration/test_agent.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/integration/test_agent.py b/tests/integration/test_agent.py index 5f1bab0..9900e6d 100644 --- a/tests/integration/test_agent.py +++ b/tests/integration/test_agent.py @@ -7,6 +7,7 @@ import textwrap import time +import jenkinsapi.custom_exceptions import jenkinsapi.jenkins import jubilant import pytest @@ -361,6 +362,11 @@ def _run_test_job(agent_name: str): agent_name = agent_nodes[0].name _run_test_job(agent_name) + except jenkinsapi.custom_exceptions.JenkinsAPIException as exc: + _dump_diagnostics() + raise AssertionError( + f"Jenkins API wrapper failed while checking agent status: {exc}" + ) from exc except requests.exceptions.ConnectionError as exc: _dump_diagnostics() raise AssertionError( From 52a3fed44c6cb41cbe655683aca1938c276aec05 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 11 Aug 2026 02:10:19 +0000 Subject: [PATCH 6/8] test: use fresh client in traefik test to avoid stale pod IP --- tests/integration/test_agent.py | 52 ++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/tests/integration/test_agent.py b/tests/integration/test_agent.py index 9900e6d..680895a 100644 --- a/tests/integration/test_agent.py +++ b/tests/integration/test_agent.py @@ -66,6 +66,29 @@ def active_agent_fixture( return jenkins_agent_application +def _fresh_server_client(microk8s_juju: jubilant.Juju) -> jenkinsapi.jenkins.Jenkins: + """Build a Jenkins client from the current unit address. + + The pod IP can change on server refresh, so re-resolve rather than reusing + a stale module-scoped client (see test_agent_reconnects_after_server_refresh). + """ + unit_status = ( + microk8s_juju.status() + .get_units(JENKINS_APPLICATION_NAME) + .get(f"{JENKINS_APPLICATION_NAME}/0") + ) + assert unit_status, f"Unit status not found for {JENKINS_APPLICATION_NAME}" + result = microk8s_juju.run(f"{JENKINS_APPLICATION_NAME}/0", "get-admin-password") + password = result.results.get("password", "") + assert password, "Failed to get admin password" + return jenkinsapi.jenkins.Jenkins( + baseurl=f"http://{unit_status.address}:8080", + username="admin", + password=password, + timeout=60, + ) + + def assert_job_success( *, client: jenkinsapi.jenkins.Jenkins, agent_name: str, test_target_label: str ): @@ -210,7 +233,6 @@ def test_agent_traefik_ingress( ingressed_jenkins_server: str, jenkins_agent_application: str, jenkins_agent_requirer: str, - jenkins_client: jenkinsapi.jenkins.Jenkins, juju: jubilant.Juju, microk8s_juju: jubilant.Juju, traefik_k8s_application: str, @@ -240,10 +262,10 @@ def test_agent_traefik_ingress( # ruff: noqa: C901 # Diagnostic-heavy test; complexity comes from explicit failure logging. - def _dump_diagnostics(): + def _dump_diagnostics(client: jenkinsapi.jenkins.Jenkins): """Dump model and application state to aid debugging connection failures.""" logger.error("=== Jenkins API connection failure diagnostics ===") - logger.error("Jenkins client URL: %s", jenkins_client.base_server_url()) + logger.error("Jenkins client URL: %s", client.base_server_url()) try: logger.error("LXD model status:\n%s", juju.status()) except Exception as exc: # nosec B110 @@ -275,20 +297,20 @@ def _dump_diagnostics(): logger.error("Failed to dump traefik proxied endpoints: %s", exc) logger.error("=== end diagnostics ===") - def _run_test_job(agent_name: str): + def _run_test_job(client: jenkinsapi.jenkins.Jenkins, agent_name: str): """Run the Jenkins test job and dump diagnostics on connection failure.""" logger.info("Agent %s is online, running test job...", agent_name) try: assert_job_success( - client=jenkins_client, + client=client, agent_name=agent_name, test_target_label="machine", ) except requests.exceptions.ConnectionError as exc: - _dump_diagnostics() + _dump_diagnostics(client) raise AssertionError( f"Jenkins API connection failed while running test job against " - f"{jenkins_client.base_server_url()}: {exc}" + f"{client.base_server_url()}: {exc}" ) from exc logger.info( "✓ Traefik ingress test passed: agent connected via WebSocket and executed job" @@ -351,24 +373,26 @@ def _run_test_job(agent_name: str): logger.info("WebSocket connection verified in agent logs") # Verify agent is functional by checking it's registered in Jenkins - # Note: When using traefik ingress, the jenkins_client may not have access to all APIs - # The core verification (WebSocket connection + active status) is already confirmed above + # Note: When using traefik ingress, the Jenkins API access may be limited. + # Use a freshly-resolved client: the module-scoped one may hold a stale pod IP + # if a prior test refreshed the server. The core verification (WebSocket + # connection + active status) is already confirmed above. + fresh_client = _fresh_server_client(microk8s_juju) try: - nodes = jenkins_client.get_nodes() - assert all(node.is_online() for node in nodes.values()), "All agents should be online" + nodes = fresh_client.get_nodes() agent_nodes = [node for node in nodes.values() if jenkins_agent_application in node.name] assert len(agent_nodes) == 1, f"Expected one agent node, found {len(agent_nodes)}" agent_name = agent_nodes[0].name - _run_test_job(agent_name) + _run_test_job(fresh_client, agent_name) except jenkinsapi.custom_exceptions.JenkinsAPIException as exc: - _dump_diagnostics() + _dump_diagnostics(fresh_client) raise AssertionError( f"Jenkins API wrapper failed while checking agent status: {exc}" ) from exc except requests.exceptions.ConnectionError as exc: - _dump_diagnostics() + _dump_diagnostics(fresh_client) raise AssertionError( f"Jenkins API connection failed while checking agent status: {exc}" ) from exc From cf67fef740baba45e82888d7380daa7f1f6cabbd Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 11 Aug 2026 03:06:54 +0000 Subject: [PATCH 7/8] test: route traefik client via ingress with retry --- tests/integration/test_agent.py | 58 +++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/tests/integration/test_agent.py b/tests/integration/test_agent.py index 680895a..203b49d 100644 --- a/tests/integration/test_agent.py +++ b/tests/integration/test_agent.py @@ -3,6 +3,7 @@ """Integration tests for jenkins-agent-k8s-operator charm.""" +import json import logging import textwrap import time @@ -66,27 +67,43 @@ def active_agent_fixture( return jenkins_agent_application -def _fresh_server_client(microk8s_juju: jubilant.Juju) -> jenkinsapi.jenkins.Jenkins: - """Build a Jenkins client from the current unit address. +def _fresh_server_client( + microk8s_juju: jubilant.Juju, traefik_k8s_application: str +) -> jenkinsapi.jenkins.Jenkins: + """Build a Jenkins client routed through the stable Traefik ingress. - The pod IP can change on server refresh, so re-resolve rather than reusing - a stale module-scoped client (see test_agent_reconnects_after_server_refresh). + The pod IP is ephemeral and the pod has a transient not-ready window when + restarted (it briefly 404s rather than refusing). Traefik only routes to + ready backends, so it is the stable target. Retry the first poll to absorb + any residual readiness race. """ - unit_status = ( - microk8s_juju.status() - .get_units(JENKINS_APPLICATION_NAME) - .get(f"{JENKINS_APPLICATION_NAME}/0") + result = microk8s_juju.run( + f"{traefik_k8s_application}/0", "show-proxied-endpoints" ) - assert unit_status, f"Unit status not found for {JENKINS_APPLICATION_NAME}" - result = microk8s_juju.run(f"{JENKINS_APPLICATION_NAME}/0", "get-admin-password") - password = result.results.get("password", "") - assert password, "Failed to get admin password" - return jenkinsapi.jenkins.Jenkins( - baseurl=f"http://{unit_status.address}:8080", - username="admin", - password=password, - timeout=60, + proxied = json.loads(result.results["proxied-endpoints"]) + url = proxied[JENKINS_APPLICATION_NAME]["url"] + admin_result = microk8s_juju.run( + f"{JENKINS_APPLICATION_NAME}/0", "get-admin-password" ) + password = admin_result.results.get("password", "") + assert password, "Failed to get admin password" + + client: jenkinsapi.jenkins.Jenkins | None = None + last_err: Exception | None = None + for attempt in range(1, 11): + try: + client = jenkinsapi.jenkins.Jenkins( + baseurl=url, username="admin", password=password, timeout=60 + ) + return client + except (jenkinsapi.custom_exceptions.JenkinsAPIException, requests.exceptions.RequestException) as exc: + last_err = exc + logger.warning( + "Jenkins API not ready (attempt %d/10) via %s: %s", attempt, url, exc + ) + time.sleep(5) + assert client is not None # unreachable; type narrow for pyright + raise AssertionError(f"Jenkins API not ready via {url} after retries: {last_err}") def assert_job_success( @@ -374,10 +391,9 @@ def _run_test_job(client: jenkinsapi.jenkins.Jenkins, agent_name: str): # Verify agent is functional by checking it's registered in Jenkins # Note: When using traefik ingress, the Jenkins API access may be limited. - # Use a freshly-resolved client: the module-scoped one may hold a stale pod IP - # if a prior test refreshed the server. The core verification (WebSocket - # connection + active status) is already confirmed above. - fresh_client = _fresh_server_client(microk8s_juju) + # Use a client routed through the Traefik ingress: it survives pod restarts + # and balancer-side readiness, unlike the module-scoped pod-IP client. + fresh_client = _fresh_server_client(microk8s_juju, traefik_k8s_application) try: nodes = fresh_client.get_nodes() From 10b0dea1340bc50abfc47a51057b3dabd73536b9 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 11 Aug 2026 04:44:57 +0000 Subject: [PATCH 8/8] fix: lint --- tests/integration/test_agent.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/tests/integration/test_agent.py b/tests/integration/test_agent.py index 203b49d..7cc048d 100644 --- a/tests/integration/test_agent.py +++ b/tests/integration/test_agent.py @@ -77,14 +77,10 @@ def _fresh_server_client( ready backends, so it is the stable target. Retry the first poll to absorb any residual readiness race. """ - result = microk8s_juju.run( - f"{traefik_k8s_application}/0", "show-proxied-endpoints" - ) + result = microk8s_juju.run(f"{traefik_k8s_application}/0", "show-proxied-endpoints") proxied = json.loads(result.results["proxied-endpoints"]) url = proxied[JENKINS_APPLICATION_NAME]["url"] - admin_result = microk8s_juju.run( - f"{JENKINS_APPLICATION_NAME}/0", "get-admin-password" - ) + admin_result = microk8s_juju.run(f"{JENKINS_APPLICATION_NAME}/0", "get-admin-password") password = admin_result.results.get("password", "") assert password, "Failed to get admin password" @@ -96,11 +92,12 @@ def _fresh_server_client( baseurl=url, username="admin", password=password, timeout=60 ) return client - except (jenkinsapi.custom_exceptions.JenkinsAPIException, requests.exceptions.RequestException) as exc: + except ( + jenkinsapi.custom_exceptions.JenkinsAPIException, + requests.exceptions.RequestException, + ) as exc: last_err = exc - logger.warning( - "Jenkins API not ready (attempt %d/10) via %s: %s", attempt, url, exc - ) + logger.warning("Jenkins API not ready (attempt %d/10) via %s: %s", attempt, url, exc) time.sleep(5) assert client is not None # unreachable; type narrow for pyright raise AssertionError(f"Jenkins API not ready via {url} after retries: {last_err}")