From 8fb29161c25bdf4b94f9f005aaec5647d55ee951 Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Sun, 8 Nov 2026 14:15:27 +0100 Subject: [PATCH] fix(sonar): reduce sharded coverage disk usage (#174) * fix(sonar): bound sharded coverage disk usage * test(signing): detect restricted GPG agent sockets --- .github/workflows/sonarcloud.yml | 23 +++- README.md | 10 ++ tests/test_sign_linux.py | 42 +++++-- tests/test_sign_linux_fixture.py | 60 ++++++++++ tests/test_sonarcloud_disk.py | 192 +++++++++++++++++++++++++++++++ 5 files changed, 310 insertions(+), 17 deletions(-) create mode 100644 tests/test_sign_linux_fixture.py create mode 100644 tests/test_sonarcloud_disk.py diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml index 440633f..b7efd43 100644 --- a/.github/workflows/sonarcloud.yml +++ b/.github/workflows/sonarcloud.yml @@ -282,9 +282,13 @@ jobs: echo "CRAFT_FONTS_DIR=$CRAFT_FONTS_PATH" >> "$GITHUB_ENV" echo "CRAFT_FONTS_REQUIRED=1" >> "$GITHUB_ENV" - name: Free disk space - # The instrumented workspace build overflows the ~14 GB free on a stock runner. - if: steps.token.outputs.available == 'true' && hashFiles('Cargo.toml') != '' && inputs.shards <= 1 - run: sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc /opt/hostedtoolcache/CodeQL + # Both a local coverage build and downloaded shard profiles plus the extracted test + # archive can overflow the free space on a stock runner. Clean before either path. + if: steps.token.outputs.available == 'true' && hashFiles('Cargo.toml') != '' + run: | + df -h . "$RUNNER_TEMP" + sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc /opt/hostedtoolcache/CodeQL + df -h . "$RUNNER_TEMP" - name: Install Linux build dependencies if: steps.token.outputs.available == 'true' && hashFiles('Cargo.toml') != '' && inputs.shards <= 1 uses: storytold/craft-common-actions/actions/install-apt-packages@main @@ -347,11 +351,18 @@ jobs: set -euo pipefail toolchain="$(cat archive/rustc-release.txt)" llvm_bin="$(rustup run "$toolchain" rustc --print sysroot)/lib/rustlib/$(rustup run "$toolchain" rustc -vV | sed -n 's/^host: //p')/bin" + df -h . "$RUNNER_TEMP" + # Do not hold every raw profile beside the expanded archive. Retain the raw inputs + # until the merge succeeds, then release them before unpacking the test binaries. + "$llvm_bin/llvm-profdata" merge -sparse profraw/*.profraw -o coverage.profdata + rm -f profraw/*.profraw + df -h . "$RUNNER_TEMP" # Unpack outside the checkout: the scanner analyses everything under the workspace, and build # output (generated sources) there would be counted as duplicated code. - extracted="$RUNNER_TEMP/coverage-archive" - mkdir -p "$extracted" + extracted="$(mktemp -d "$RUNNER_TEMP/coverage-archive.XXXXXX")" + trap 'rm -rf "$extracted"' EXIT tar --zstd -xf archive/nextest-archive.tar.zst -C "$extracted" + rm -f archive/nextest-archive.tar.zst # Only test binaries run on the target; helpers and proc-macros are not instrumented profile sources. objects=() while IFS= read -r name; do @@ -361,11 +372,11 @@ jobs: echo "::error::No test binaries found in the archive metadata" exit 1 fi - "$llvm_bin/llvm-profdata" merge -sparse profraw/*.profraw -o coverage.profdata extra=() for object in "${objects[@]:1}"; do extra+=(-object "$object"); done "$llvm_bin/llvm-cov" export -format=lcov -instr-profile=coverage.profdata \ "${objects[0]}" "${extra[@]}" -ignore-filename-regex="$COVERAGE_IGNORE" >lcov.info + rm -f coverage.profdata - name: Generate Python coverage if: steps.token.outputs.available == 'true' && (inputs.python-coverage-source != '' || inputs.python-coverage-script != '') shell: bash diff --git a/README.md b/README.md index bb319ba..6715181 100644 --- a/README.md +++ b/README.md @@ -863,6 +863,11 @@ with the rustup `llvm-profdata` and `llvm-cov` tools. Pass `shards: 1` to keep the single `cargo llvm-cov` job. Sharded runs skip doctests, because nextest does not run them. +The scan job frees unused runner SDKs before either Rust coverage path. Sharded +coverage merges and removes raw profiles before extracting the test archive, then +releases the compressed archive and temporary binaries as they are consumed. +Disk-space snapshots in the logs show headroom before cleanup and aggregation. + The new limits leave headroom over sampled Craft app job timings; the existing FreeBSD and ARM64 limits are retained. Treat these as initial limits, review successful cold builds after rollout and lower them when the evidence supports it. @@ -911,6 +916,11 @@ With Python 3 and PyYAML installed, run: python3 -m unittest discover -s tests -v ``` +GnuPG signing fixtures need local Unix sockets for their temporary agents. They +report an explicit skip when a local sandbox denies socket creation or binding; +the same restriction remains a failure on GitHub Actions. Other GPG errors are +not skipped, and the hosted suite exercises the real protected-key fixture. + The tests execute the FreeBSD run template under the host's POSIX shell with fixture build/test commands and real tar archives. They cover failures in each custom test group, default build/test failures, missing/corrupt packages and font diff --git a/tests/test_sign_linux.py b/tests/test_sign_linux.py index 2acf73d..b30e706 100644 --- a/tests/test_sign_linux.py +++ b/tests/test_sign_linux.py @@ -1,7 +1,9 @@ """Sign a fake release's Linux packages with a throwaway key and verify every signature.""" from pathlib import Path +from contextlib import ExitStack import os import shutil +import socket import subprocess import tempfile import unittest @@ -24,24 +26,37 @@ def have(*tools): return all(shutil.which(tool) for tool in tools) +def require_agent_socket(home): + """Skip restricted local executors, without hiding GPG or hosted CI failures.""" + path = home / "agent-socket-check" + try: + with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as probe: + probe.bind(str(path)) + except PermissionError as error: + if os.environ.get("GITHUB_ACTIONS") == "true": + raise + raise unittest.SkipTest(f"GnuPG agent needs Unix sockets: {error}") from error + finally: + path.unlink(missing_ok=True) + + @unittest.skipUnless(have("gpg", "gpgv"), "needs GnuPG") class SignLinuxTests(unittest.TestCase): def setUp(self): self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) self.root = Path(self.temp.name) self.gnupg = self.root / "gnupg" self.gnupg.mkdir(mode=0o700) + require_agent_socket(self.gnupg) self.env = dict(os.environ, GNUPGHOME=str(self.gnupg), OCI_REGISTRY="ghcr.io/storytold/demo") + self.addCleanup(subprocess.run, ["gpgconf", "--kill", "all"], env=self.env, capture_output=True) self.gpg("--passphrase", "", "--quick-gen-key", "Release Test ", "ed25519", "sign", "never") listing = self.gpg("--with-colons", "--list-secret-keys").stdout self.env["SIGNING_FPR"] = next(line.split(":")[9] for line in listing.splitlines() if line.startswith("fpr")) self.dist = self.root / "artifacts" self.dist.mkdir() - def tearDown(self): - subprocess.run(["gpgconf", "--kill", "all"], env=self.env, capture_output=True) - self.temp.cleanup() - def gpg(self, *args): return subprocess.run(["gpg", "--batch", "--quiet", *args], env=self.env, capture_output=True, text=True, check=True) @@ -155,14 +170,21 @@ class SigningWiringTests(unittest.TestCase): @unittest.skipUnless(have("gpg"), "needs GnuPG") def test_the_import_action_refuses_a_key_that_needs_a_passphrase(self): script = yaml.load(IMPORT.read_text(), Loader=yaml.BaseLoader)["runs"]["steps"][0]["run"] - with tempfile.TemporaryDirectory() as directory: + with tempfile.TemporaryDirectory() as directory, ExitStack() as cleanup: root = Path(directory) home = root / "gen" home.mkdir(mode=0o700) + require_agent_socket(home) gen = dict(os.environ, GNUPGHOME=str(home)) - run = lambda *a: subprocess.run(["gpg", "--batch", "--quiet", *a], env=gen, capture_output=True, check=True).stdout + cleanup.callback(subprocess.run, ["gpgconf", "--kill", "all"], env=gen, capture_output=True) + + def run(*args): + proc = subprocess.run(["gpg", "--batch", "--quiet", *args], env=gen, capture_output=True, text=True) + self.assertEqual(proc.returncode, 0, f"GPG fixture setup failed:\n{proc.stderr}") + return proc.stdout + run("--passphrase", "secret", "--quick-gen-key", "Protected ", "ed25519", "sign", "never") - protected = run("--pinentry-mode", "loopback", "--passphrase", "secret", "--armor", "--export-secret-keys").decode() + protected = run("--pinentry-mode", "loopback", "--passphrase", "secret", "--armor", "--export-secret-keys") results = {} for case, key in {"protected": protected, "empty": ""}.items(): out = root / f"{case}.out" @@ -170,14 +192,12 @@ class SigningWiringTests(unittest.TestCase): out.write_text("") env_file.write_text("") (root / case).mkdir() + cleanup.callback(subprocess.run, ["gpgconf", "--kill", "all"], capture_output=True, + env=dict(os.environ, GNUPGHOME=str(root / case / "release-gnupg"))) proc = subprocess.run(["bash", "-e", "-c", script], capture_output=True, text=True, env=dict(os.environ, SIGNING_KEY=key, RUNNER_TEMP=str(root / case), GITHUB_OUTPUT=str(out), GITHUB_ENV=str(env_file))) results[case] = (proc.returncode, out.read_text(), env_file.read_text(), proc.stdout) - subprocess.run(["gpgconf", "--kill", "all"], env=gen, capture_output=True) - for case in results: - subprocess.run(["gpgconf", "--kill", "all"], capture_output=True, - env=dict(os.environ, GNUPGHOME=str(root / case / "release-gnupg"))) self.assertNotEqual(results["protected"][0], 0) self.assertIn("without a passphrase", results["protected"][3]) self.assertEqual(results["empty"][:3], (0, "enabled=false\n", "")) diff --git a/tests/test_sign_linux_fixture.py b/tests/test_sign_linux_fixture.py new file mode 100644 index 0000000..8db9c6c --- /dev/null +++ b/tests/test_sign_linux_fixture.py @@ -0,0 +1,60 @@ +"""Only unsupported local agent sockets may skip the real GnuPG fixtures.""" +import errno +import os +from pathlib import Path +import tempfile +import unittest +from unittest import mock + +import test_sign_linux as signing + + +class AgentSocketTests(unittest.TestCase): + def setUp(self): + temp = tempfile.TemporaryDirectory() + self.addCleanup(temp.cleanup) + self.home = Path(temp.name) + self.env = mock.patch.dict(os.environ, {"GITHUB_ACTIONS": "false"}) + self.env.start() + self.addCleanup(self.env.stop) + self.factory = mock.patch.object(signing.socket, "socket") + self.socket = self.factory.start() + self.addCleanup(self.factory.stop) + + def test_allowed_socket_is_closed_and_removed(self): + probe = self.socket.return_value.__enter__.return_value + probe.bind.side_effect = lambda path: Path(path).touch() + signing.require_agent_socket(self.home) + self.socket.assert_called_once_with(signing.socket.AF_UNIX, signing.socket.SOCK_STREAM) + probe.bind.assert_called_once_with(str(self.home / "agent-socket-check")) + self.socket.return_value.__exit__.assert_called_once() + self.assertEqual(list(self.home.iterdir()), []) + + def test_denied_local_socket_is_an_explicit_skip(self): + for code in (errno.EPERM, errno.EACCES): + with self.subTest(errno=code): + self.socket.side_effect = PermissionError(code, os.strerror(code)) + with self.assertRaisesRegex(unittest.SkipTest, "GnuPG agent needs Unix sockets"): + signing.require_agent_socket(self.home) + + def test_denied_bind_is_an_explicit_local_skip(self): + self.socket.return_value.__enter__.return_value.bind.side_effect = PermissionError(errno.EPERM, "denied") + with self.assertRaises(unittest.SkipTest): + signing.require_agent_socket(self.home) + self.socket.return_value.__exit__.assert_called_once() + + def test_denied_hosted_socket_remains_a_failure(self): + self.socket.side_effect = PermissionError(errno.EPERM, "denied") + with mock.patch.dict(os.environ, {"GITHUB_ACTIONS": "true"}): + with self.assertRaises(PermissionError): + signing.require_agent_socket(self.home) + + def test_resource_errors_remain_failures(self): + self.socket.side_effect = OSError(errno.EMFILE, "too many open files") + with self.assertRaises(OSError) as result: + signing.require_agent_socket(self.home) + self.assertEqual(result.exception.errno, errno.EMFILE) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_sonarcloud_disk.py b/tests/test_sonarcloud_disk.py new file mode 100644 index 0000000..26f3b53 --- /dev/null +++ b/tests/test_sonarcloud_disk.py @@ -0,0 +1,192 @@ +"""Exercise disk headroom and input lifetimes in sharded Sonar coverage.""" + +import json +import os +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + +import yaml + + +ROOT = Path(__file__).resolve().parents[1] +TOOLS = ('bash', 'jq', 'tar', 'zstd') + + +@unittest.skipUnless(all(shutil.which(tool) for tool in TOOLS), + 'requires bash, jq, tar and zstd') +class SonarCloudDiskTests(unittest.TestCase): + def setUp(self): + workflow = yaml.load( + (ROOT / '.github/workflows/sonarcloud.yml').read_text(), + Loader=yaml.BaseLoader, + ) + self.steps = workflow['jobs']['sonarcloud']['steps'] + self.cleanup = next(step for step in self.steps + if step.get('name') == 'Free disk space') + self.combine = next(step for step in self.steps + if step.get('name') == 'Combine shard coverage') + + def test_cleanup_covers_both_rust_paths_before_downloads(self): + self.assertEqual(self.cleanup['if'], + "steps.token.outputs.available == 'true' && " + "hashFiles('Cargo.toml') != ''") + for name in ('Generate Rust coverage', 'Download shard profiles and archive', + 'Download test archive'): + step = next(step for step in self.steps if step.get('name') == name) + self.assertLess(self.steps.index(self.cleanup), self.steps.index(step)) + + def run_shell(self, script, root, **env): + return subprocess.run( + ['bash', '--noprofile', '--norc', '-e', '-o', 'pipefail', '-c', script], + cwd=root, env={**os.environ, **env}, capture_output=True, text=True, + ) + + def test_cleanup_reports_space_and_keeps_the_existing_removal_scope(self): + # Stub privileged cleanup: this test never deletes runner/system directories. + with tempfile.TemporaryDirectory(prefix='sonar cleanup ') as temp: + root = Path(temp) + for name in ('sudo', 'df'): + tool = root / name + tool.write_text('#!/bin/sh\nprintf "%s\\n" "$(basename "$0") $*" >> "$CALLS"\n') + tool.chmod(0o755) + calls = root / 'calls' + result = self.run_shell( + self.cleanup['run'], root, PATH=f'{root}{os.pathsep}{os.environ["PATH"]}', + CALLS=str(calls), RUNNER_TEMP=str(root / 'runner temp'), + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(calls.read_text().splitlines(), [ + f'df -h . {root}/runner temp', + 'sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc ' + '/opt/hostedtoolcache/CodeQL', + f'df -h . {root}/runner temp', + ]) + + def run_combine(self, failure=''): + with tempfile.TemporaryDirectory(prefix='sonar merge ') as temp: + root = Path(temp) + workspace = root / 'workspace' + workspace.mkdir() + runner_temp = root / 'runner temp' + runner_temp.mkdir() + sentinel = runner_temp / 'keep-unrelated.txt' + sentinel.write_text('keep') + profiles = workspace / 'profraw' + profiles.mkdir() + for name in ('one', 'two'): + (profiles / f'{name}.profraw').write_text(name) + archive = workspace / 'archive' + archive.mkdir() + (archive / 'rustc-release.txt').write_text('1.99.1\n') + source = root / 'source' + metadata = source / 'target/nextest/binaries-metadata.json' + metadata.parent.mkdir(parents=True) + metadata.write_text(json.dumps({'rust-binaries': { + 'test': {'build-platform': 'target', 'binary-path': '/target/debug/test-bin'}, + }})) + binary = source / 'target/debug/test-bin' + binary.parent.mkdir(parents=True) + binary.write_text('test binary') + archive_file = archive / 'nextest-archive.tar.zst' + subprocess.run(['tar', '--zstd', '-cf', str(archive_file), '-C', str(source), + 'target'], check=True, capture_output=True) + + tools = root / 'tools' + tools.mkdir() + llvm = root / 'toolchain/lib/rustlib/test-host/bin' + llvm.mkdir(parents=True) + fake_tool = root / 'fake_tool.py' + fake_tool.write_text('''#!/usr/bin/env python3 +import os, pathlib, subprocess, sys +name = pathlib.Path(sys.argv[0]).name +args = sys.argv[1:] +failure = os.environ.get('FAIL_STAGE', '') +with open(os.environ['CALLS'], 'a') as calls: + calls.write(name + '\\n') +if name == 'rustup': + print(os.environ['SYSROOT'] if '--print' in args else 'host: test-host') +elif name == 'llvm-profdata': + assert list(pathlib.Path('profraw').glob('*.profraw')) + if failure == 'merge': + sys.exit(7) + pathlib.Path(args[args.index('-o') + 1]).write_text('merged profile') +elif name == 'tar': + assert pathlib.Path('coverage.profdata').is_file() + assert not list(pathlib.Path('profraw').glob('*.profraw')) + if failure == 'extract': + (pathlib.Path(args[args.index('-C') + 1]) / 'partial').write_text('partial extract') + sys.exit(8) + sys.exit(subprocess.run([os.environ['REAL_TAR'], *args]).returncode) +elif name == 'llvm-cov': + assert pathlib.Path('coverage.profdata').is_file() + assert not pathlib.Path('archive/nextest-archive.tar.zst').exists() + if failure == 'export': + sys.exit(9) + print('TN:\\nSF:src/lib.rs\\nDA:1,2\\nend_of_record') +''') + fake_tool.chmod(0o755) + for name in ('rustup', 'tar'): + (tools / name).symlink_to(fake_tool) + for name in ('llvm-profdata', 'llvm-cov'): + (llvm / name).symlink_to(fake_tool) + calls = root / 'calls' + result = self.run_shell( + self.combine['run'], workspace, + PATH=f'{tools}{os.pathsep}{os.environ["PATH"]}', RUNNER_TEMP=str(runner_temp), + SYSROOT=str(root / 'toolchain'), CALLS=str(calls), FAIL_STAGE=failure, + REAL_TAR=shutil.which('tar'), COVERAGE_IGNORE='tests/', + ) + return { + 'result': result, + 'calls': calls.read_text().splitlines(), + 'raw_profiles': sorted(path.name for path in profiles.glob('*.profraw')), + 'archive_exists': archive_file.exists(), + 'merged_exists': (workspace / 'coverage.profdata').exists(), + 'extracted': list(runner_temp.glob('coverage-archive*')), + 'sentinel': sentinel.read_text(), + 'lcov': (workspace / 'lcov.info').read_text() + if (workspace / 'lcov.info').exists() else None, + } + + def test_profiles_are_released_before_real_archive_extraction(self): + observed = self.run_combine() + self.assertEqual(observed['result'].returncode, 0, observed['result'].stderr) + self.assertLess(observed['calls'].index('llvm-profdata'), observed['calls'].index('tar')) + self.assertEqual(observed['raw_profiles'], []) + self.assertFalse(observed['archive_exists']) + self.assertFalse(observed['merged_exists']) + self.assertEqual(observed['extracted'], []) + self.assertEqual(observed['sentinel'], 'keep') + self.assertEqual(observed['lcov'], 'TN:\nSF:src/lib.rs\nDA:1,2\nend_of_record\n') + + def test_failed_profile_merge_preserves_inputs_and_never_extracts(self): + observed = self.run_combine('merge') + self.assertEqual(observed['result'].returncode, 7, observed['result'].stderr) + self.assertEqual(observed['raw_profiles'], ['one.profraw', 'two.profraw']) + self.assertTrue(observed['archive_exists']) + self.assertNotIn('tar', observed['calls']) + self.assertEqual(observed['extracted'], []) + self.assertIsNone(observed['lcov']) + + def test_failed_extraction_cleans_only_its_temporary_directory(self): + observed = self.run_combine('extract') + self.assertEqual(observed['result'].returncode, 8, observed['result'].stderr) + self.assertTrue(observed['archive_exists']) + self.assertTrue(observed['merged_exists']) + self.assertEqual(observed['extracted'], []) + self.assertEqual(observed['sentinel'], 'keep') + self.assertNotIn('llvm-cov', observed['calls']) + + def test_failed_export_cleans_the_expanded_archive(self): + observed = self.run_combine('export') + self.assertEqual(observed['result'].returncode, 9, observed['result'].stderr) + self.assertTrue(observed['merged_exists']) + self.assertEqual(observed['extracted'], []) + self.assertEqual(observed['sentinel'], 'keep') + + +if __name__ == '__main__': + unittest.main()