ci: fix upload-artifact EACCES in all downstream test reusables#5608
ci: fix upload-artifact EACCES in all downstream test reusables#5608renecannao merged 1 commit intoGH-Actionsfrom
Conversation
Every downstream test reusable ran actions/upload-artifact@v4 on its ci_*_logs/ output path inside an `if: failure() && !cancelled()` block. When a test actually failed, the upload would die with: ##[error]EACCES: permission denied, scandir '/home/runner/work/ proxysql/proxysql/proxysql/ci_infra_logs/infra-mariadb10-ci-legacy-g3/ mariadb1/mysql' ...because ci_*_logs/ contains files created inside docker build containers as root, which the host-side runner user cannot scandir. Net effect: ZERO post-mortem artifacts for failed test runs. Every maintainer trying to investigate a CI test failure from the Actions page got "no artifacts found" and had to reproduce the failure locally, which is slow and often impossible (CI-specific races). This is the same bug we fixed in ci-builds.yml with sudo (PR #5605), but the fix there only addressed the cache-shrink cleanup paths - it missed every downstream reusable that also wrote to ci_*_logs/ from inside docker and then tried to upload. Fix: before each actions/upload-artifact@v4 step whose path is proxysql/ci_*_logs/, insert a `Fix artifact permissions` step that runs `sudo chmod -R a+rX proxysql/ci_*_logs/` to make the tree world-readable. sudo is required because the files are root-owned. 2>/dev/null + || true because the path may not exist on all failure paths (e.g. a cache-restore failure before the test even runs). Applied to 22 reusable workflow files: - ci-basictests.yml - ci-legacy-clickhouse-g1.yml - ci-legacy-g1.yml, ci-legacy-g2.yml, ci-legacy-g2-genai.yml, ci-legacy-g3.yml, ci-legacy-g4.yml, ci-legacy-g5.yml - ci-mysql84-g1.yml ... ci-mysql84-g5.yml - ci-repltests.yml, ci-shuntest.yml, ci-selftests.yml - ci-taptests.yml, ci-taptests-asan.yml, ci-taptests-old.yml, ci-taptests-ssl.yml, ci-taptests-groups.yml - ci-unittests.yml Not touched: - ci-maketest.yml - uploads ./build-*.log, created by the runner user outside docker, no EACCES - ci-3p-*.yml - these do not upload ci_*_logs/; their artifact uploads (if any) target 3p-specific paths created outside docker - ci-builds.yml - already handled by PR #5605 (sudo rm on test/deps, sudo find on unit binaries) How to verify on a future CI run: if a TAP test fails in any of the patched workflows, `gh api repos/sysown/proxysql/actions/runs/ <run_id>/artifacts` should now return a non-empty list, with the ci_*_logs/ tarball containing the per-test .log.gz files for post-mortem analysis. Before this fix, it returned an empty array for any failed run (because upload-artifact died before emitting anything) - see run 24281031531 on 2026-04-11 as the proof case.
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (22)
📜 Recent review details🔇 Additional comments (22)
📝 WalkthroughWalkthroughAdds a conditional failure-only step across 22 GitHub Actions workflows that recursively changes permissions on Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
1 participant
Summary
Every downstream test reusable on
GH-Actions(ci-legacy-g*,ci-mysql84-g*,ci-basictests,ci-repltests,ci-shuntest, etc.) uploadsproxysql/ci_*_logs/as a post-mortem artifact when a test fails, viaactions/upload-artifact@v4insideif: failure() && !cancelled(). That upload dies every single time with:because
ci_*_logs/contains files that the docker build containers wrote as root, and the host-side runner user cannotscandirinto those directories.Net effect: for every failed test in the CI cascade, zero artifacts are uploaded. Maintainers trying to investigate CI failures from the Actions page get "no artifacts found" and have to reproduce locally.
Concrete proof case from today: runs
24281031531,24281031522,24281031524on PR #5596 (commit09b97547f) — all three failed with real TAP test failures (test_flush_logs-t,pgsql-servers_ssl_params-t,pgsql-ssl_keylog-t,test_read_only_actions_offline_hard_servers-t), and all three haveartifacts: []when queried viagh api. The TAP.log.gzfiles that would have told us why the tests failed were lost.Fix
Insert a new
Fix artifact permissionsstep before everyactions/upload-artifact@v4step whose path isproxysql/ci_*_logs/. The new step runs:sudobecause the files are root-owned (created inside docker containers)a+rXmakes everything world-readable, with directory-traverse on directories (capital X)2>/dev/null || truebecause the path may not exist on some failure paths (e.g. a cache-restore failure before any test runs)Same pattern as the
sudo rm -rf test/deps+sudo find ... -deletefix we shipped in PR #5605 forci-builds.yml.Files touched (22)
ci-basictests.ymlci-legacy-clickhouse-g1.ymlci-legacy-g1.yml,ci-legacy-g2.yml,ci-legacy-g2-genai.yml,ci-legacy-g3.yml,ci-legacy-g4.yml,ci-legacy-g5.ymlci-mysql84-g1.ymlthroughci-mysql84-g5.ymlci-repltests.yml,ci-shuntest.yml,ci-selftests.ymlci-taptests.yml,ci-taptests-asan.yml,ci-taptests-old.yml,ci-taptests-ssl.yml,ci-taptests-groups.ymlci-unittests.ymlFiles intentionally NOT touched
ci-maketest.yml— uploads./build-*.log, created by the runner user outside docker, no EACCES riskci-3p-*.yml— these do not uploadci_*_logs/; their artifact uploads (if any) target 3p-specific paths created outside dockerci-builds.yml— already handled by PR ci: use sudo for cache-prune cleanup to avoid root-owned file errors #5605Test plan
yaml.safe_load(no syntax breakage from the insertion)ci-legacy-g1.ymlmanually — insertion is at the correct indentation and immediately precedes the existingArchive artifacts logsstepartifacts: [...]ingh api repos/sysown/proxysql/actions/runs/<id>/artifacts, with theci_*_logs/tarball containing the per-test.log.gzfilesactions/upload-artifact@v4is unchanged — this PR is orthogonal to thatSummary by CodeRabbit