Skip to content

[WIP] Clean actions cache workflow#5022

Closed
christianvogt wants to merge 2 commits intoopendatahub-io:mainfrom
christianvogt:clean-actions-cache
Closed

[WIP] Clean actions cache workflow#5022
christianvogt wants to merge 2 commits intoopendatahub-io:mainfrom
christianvogt:clean-actions-cache

Conversation

@christianvogt
Copy link
Collaborator

@christianvogt christianvogt commented Sep 26, 2025

Description

An actions to clean up github actions cache when a PR closes

How Has This Been Tested?

Test Impact

Request review criteria:

Self checklist (all need to be checked):

  • The developer has manually tested the changes and verified that the changes work
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has added tests or explained why testing cannot be added (unit or cypress tests for related changes)
  • The code follows our Best Practices (React coding standards, PatternFly usage, performance considerations)

If you have UI changes:

  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change.

After the PR is posted & before it merges:

  • The developer has tested their solution on a cluster by using the image produced by the PR to main

Summary by CodeRabbit

  • Chores
    • Added automated cleanup of caches when pull requests are closed to reduce storage usage.
    • Unified and streamlined CI caching across checks and tests for faster, more consistent runs.
    • Expanded cache path coverage to include all relevant Turbo cache locations.
    • Simplified pipeline logic by removing redundant cache-hit guards, improving reliability without failing on individual cache deletion issues.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress This PR is in WIP state label Sep 26, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

📝 Walkthrough

Walkthrough

  • Adds a new GitHub Actions workflow (.github/workflows/pr-cache-cleanup.yml) triggered on pull_request closed to list and delete caches tied to the PR merge ref using gh cache list/delete, with non-failing deletions and required env vars (GH_TOKEN, GH_REPO, BRANCH).
  • Updates .github/workflows/test.yml to centralize cache key usage via needs.Setup.outputs.modules-cache-key across jobs, removes cache-hit conditional guards, and standardizes restore/use points.
  • Expands Turbo cache paths to a multi-path glob (**/.turbo) and aligns all related cache steps to the unified key and paths across Type-Check, Lint, and Unit-Tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description includes the Description section but leaves critical sections such as How Has This Been Tested? and Test Impact empty, does not reference any related issue, and the self-checklist items are all unchecked, indicating incomplete compliance with the required template. Please provide detailed testing information under How Has This Been Tested?, specify the coverage or impact in the Test Impact section, reference the relevant issue or ticket for this change, and complete the self-checklist by marking the items you have verified. This will ensure that all required template sections are fully documented and validated before merging.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title concisely conveys the primary change of adding a GitHub Actions cache cleanup workflow and aligns directly with the changeset’s purpose without including extraneous details.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 26, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign fedealonso for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9db03c1 and 510d5de.

📒 Files selected for processing (2)
  • .github/workflows/pr-cache-cleanup.yml (1 hunks)
  • .github/workflows/test.yml (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Cypress-Setup
  • GitHub Check: Lint
  • GitHub Check: Type-Check
🔇 Additional comments (2)
.github/workflows/test.yml (2)

52-52: Centralizing the modules cache key is spot on

Referencing needs.Setup.outputs.modules-cache-key across the jobs keeps cache usage consistent and prevents divergent installs. Nicely tightened up.

Also applies to: 79-79, 106-106


56-57: Broader Turbo cache coverage looks great

Switching the Turbo cache path to **/.turbo ensures every workspace gets picked up without extra duplication. Thanks for making the reuse more robust.

Also applies to: 83-84, 110-111

Comment on lines +16 to +24
cacheKeysForPR=$(gh cache list --ref $BRANCH --limit 1000 --json id --jq '.[].id')

## Setting this to not fail the workflow while deleting cache keys.
set +e
echo "Deleting caches..."
for cacheKey in $cacheKeysForPR
do
gh cache delete $cacheKey
done
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Prevent gh cache from failing by using the REST API instead

On GitHub-hosted runners the plain gh cache … subcommand is not available unless the actions/gh-actions-cache extension is pre-installed. As written, the very first gh cache list … invocation exits with “unknown command” and the cleanup job fails every time. Switching to the built-in REST API (gh api) avoids that dependency and keeps the job portable.

Please adjust the script along these lines:

-          cacheKeysForPR=$(gh cache list --ref $BRANCH --limit 1000 --json id --jq '.[].id')
+          cacheKeysForPR=$(gh api repos/$GH_REPO/actions/caches --paginate -F ref=$BRANCH --jq '.actions_caches[].id')
@@
-              gh cache delete $cacheKey
+              gh api --method DELETE repos/$GH_REPO/actions/caches/$cacheKey

This runs everywhere without requiring extra tooling and keeps the cleanup workflow from breaking.

🤖 Prompt for AI Agents
.github/workflows/pr-cache-cleanup.yml around lines 16 to 24: the workflow uses
the `gh cache` subcommand which may not be available on GitHub-hosted runners
and causes the job to fail; replace the `gh cache list` and `gh cache delete`
calls with `gh api` REST requests that list cache entries for the branch
(querying /repos/:owner/:repo/actions/caches with appropriate params and parsing
returned cache ids) and then delete each cache id via `gh api --method DELETE
/repos/:owner/:repo/actions/caches/:cache_id`; keep the `set +e` behavior or add
per-request error handling so failures don’t break the cleanup, and iterate over
the returned ids to delete them using the REST API instead of the subcommand.

@codecov
Copy link

codecov bot commented Sep 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.68%. Comparing base (14ffa56) to head (510d5de).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5022      +/-   ##
==========================================
- Coverage   67.82%   67.68%   -0.14%     
==========================================
  Files        2235     2230       -5     
  Lines       51007    50734     -273     
  Branches    14215    14090     -125     
==========================================
- Hits        34595    34339     -256     
+ Misses      16412    16395      -17     

see 42 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14ffa56...510d5de. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress This PR is in WIP state

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant