Skip to content

fix(INF2-migrate-to-self-hosted): migrate workflows to use self hosted Runners - #1726

Open
vineetguptadev wants to merge 5 commits into
mainfrom
INF2-migrate-to-self-hosted
Open

fix(INF2-migrate-to-self-hosted): migrate workflows to use self hosted Runners#1726
vineetguptadev wants to merge 5 commits into
mainfrom
INF2-migrate-to-self-hosted

Conversation

@vineetguptadev

Copy link
Copy Markdown
Contributor

Motivation:
Migrate GitHub Actions workflows to organization self-hosted ARC runners to reduce reliance on GitHub-hosted runners.
Modifications:
Switched runs-on from ubuntu-latest to self-hosted for automation, certora-prover, claude-code-review, checks/typos, and remove-stale-branches workflows; added tool install steps (gh CLI, Python) where needed. Docker-dependent jobs (go-bindings) remain on ubuntu-latest.
Result:
Eligible workflows now run on ARC self-hosted runners in the EKS cluster, with Docker-dependent jobs unchanged.

Copilot AI review requested due to automatic review settings February 9, 2026 12:44
@vineetguptadev vineetguptadev added 📜 Chore Maintenance related changes (CI, QOL, etc.). 🚗 CI Continuous Integration labels Feb 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Migrates multiple GitHub Actions workflows from GitHub-hosted runners to org self-hosted ARC runners, adding installation steps for missing tooling on the self-hosted environment.

Changes:

  • Switched eligible jobs to runs-on: self-hosted.
  • Added runtime tool installation steps (GitHub CLI, Python) where required on self-hosted runners.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
.github/workflows/remove-stale-branches.yml Moves stale-branch cleanup job to self-hosted runners.
.github/workflows/claude-code-review.yml Moves Claude review jobs to self-hosted and installs GitHub CLI.
.github/workflows/checks.yml Moves typo linting job to self-hosted runners.
.github/workflows/certora-prover.yml Moves Certora workflow to self-hosted and installs Python.
.github/workflows/automation.yml Moves PR title linting job to self-hosted runners.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/remove-stale-branches.yml Outdated
Comment thread .github/workflows/claude-code-review.yml Outdated
Comment thread .github/workflows/claude-code-review.yml Outdated
Comment thread .github/workflows/claude-code-review.yml Outdated
Comment thread .github/workflows/claude-code-review.yml Outdated
Comment thread .github/workflows/certora-prover.yml Outdated
Comment thread .github/workflows/certora-prover.yml Outdated
Comment thread .github/workflows/checks.yml Outdated
@claude

claude Bot commented Feb 9, 2026

Copy link
Copy Markdown

Claude finished @vineetguptadev's task in 1m 3s —— View job


PR Review Complete ✅

I've reviewed the workflow migration changes. Overall, this is a solid migration to self-hosted runners with good attention to detail. Here are my findings:


✅ What's Working Well

  1. Consistent runner labels: Three of four workflows now use [self-hosted, linux, x64] which properly constrains runner selection
  2. Smart Python setup: Using actions/setup-python@v5 in certora-prover.yml is the right approach
  3. Removed harden-runner: Good decision to remove step-security/harden-runner from the typos job - this action requires privileged access that's typically unavailable in Kubernetes-based ARC runners

⚠️ Issues Found

1. Missing Runner Labels in automation.yml (Line 29)

The lint-pr-title job uses bare self-hosted without additional labels, while all other workflows use [self-hosted, linux, x64]. This inconsistency could cause the job to be scheduled on incompatible runners.

Location: .github/workflows/automation.yml:29

  lint-pr-title:
-   runs-on: self-hosted
+   runs-on: [self-hosted, linux, x64]
    name: Title

Impact: Medium - Job could fail if scheduled on non-Linux runner

Fix this inconsistency →


2. wget Installation Approach (checks.yml:19-23)

The wget installation step uses apt-get which:

  • Is Debian/Ubuntu specific
  • Requires sudo permissions
  • Runs on every job execution (inefficient)

While the author confirmed wget is needed for crate-ci/typos, there are better approaches:

Current approach:

- name: Install wget
  run: |
    if ! command -v wget &> /dev/null; then
      sudo apt-get update && sudo apt-get install -y wget
    fi

Better alternatives (in priority order):

  1. Pre-install in runner image (Recommended): Add wget to your ARC runner base image so it's always available
  2. If runtime install is required: The current implementation with the command -v check is acceptable, but document why wget is needed (reference the typos action requirement)

Impact: Low - Works but not optimal; adds ~5-10 seconds per run


📋 Summary

Files Changed: 4 workflows

  • remove-stale-branches.yml - Perfect
  • certora-prover.yml - Perfect
  • ⚠️ automation.yml - Missing runner labels
  • ⚠️ checks.yml - wget install works but suboptimal

Critical Issues: 1 (missing runner labels)
Improvements: 1 (wget installation approach)

Recommendation: Fix the missing runner labels in automation.yml before merging to ensure consistency and prevent potential runtime failures.


Comment thread .github/workflows/automation.yml
Comment thread .github/workflows/certora-prover.yml Outdated
Comment thread .github/workflows/certora-prover.yml
Comment thread .github/workflows/checks.yml
Comment thread .github/workflows/checks.yml
Comment thread .github/workflows/checks.yml Outdated
Comment thread .github/workflows/certora-prover.yml Outdated
Comment thread .github/workflows/checks.yml
Comment thread .github/workflows/checks.yml
Comment thread .github/workflows/checks.yml Outdated
Comment thread .github/workflows/checks.yml
@vineetguptadev
vineetguptadev force-pushed the INF2-migrate-to-self-hosted branch from ea8aabd to 8d272ea Compare February 9, 2026 18:59
@elhajin

elhajin commented Feb 10, 2026

Copy link
Copy Markdown
Contributor

@claude check the latest changes of that PR , and if any of your previous review are un-addressed or invalid

@claude

claude Bot commented Feb 10, 2026

Copy link
Copy Markdown

Claude finished @elhajin's task in 1m 0s —— View job


Review Verification Complete ✅

I've checked the latest PR changes against all previous review comments. Here's the status:


✅ Issues That Have Been Addressed

  1. certora-prover.yml - Runner labels added (Line 39)

    • FIXED: Now uses runs-on: [self-hosted, linux, x64]
    • Previous issue: Was using bare self-hosted without labels
    • This is now consistent with other workflows
  2. certora-prover.yml - Python installation improved (Line 46-49)

    • FIXED: Now uses actions/setup-python@v5 with pinned version 3.11
    • Previous issue: Multiple reviewers suggested avoiding apt-get install python3
    • The current approach is cross-platform, reproducible, and doesn't require sudo
  3. checks.yml - Harden-runner removed from typos job

    • FIXED: The step-security/harden-runner step has been removed from the typos job
    • Previous issue: This action requires privileged access incompatible with K8s-based ARC runners
    • The go-bindings job (line 34) still has it, which is correct since that job runs on ubuntu-latest (GitHub-hosted)

⚠️ Issues That Remain Unaddressed

  1. automation.yml - Missing runner labels (Line 29)

    • STILL UNADDRESSED: Uses bare self-hosted instead of [self-hosted, linux, x64]

    Current code:

    lint-pr-title:
      runs-on: self-hosted

    Should be:

    lint-pr-title:
      runs-on: [self-hosted, linux, x64]

    Impact: This job could be scheduled on incompatible runners (macOS, Windows, ARM), causing failures. This is inconsistent with all other workflows in this PR.

    Fix this inconsistency →

  2. checks.yml - wget installation approach (Lines 19-23)

    • ⚠️ REMAINS SUBOPTIMAL: Still uses apt-get to install wget at runtime

    Current status: The author confirmed wget is needed for crate-ci/typos to work, which validates the necessity. However, the implementation approach is still suboptimal:

    • Platform-specific (Debian/Ubuntu only)
    • Requires sudo permissions
    • Adds runtime overhead on every job execution

    Recommendation: Pre-install wget in your ARC runner base image. Most Linux base images include wget by default, but if your custom image doesn't, adding it to the image would be more efficient than installing on every run.

    However, if runtime installation is the only option (e.g., you can't modify the runner image), the current implementation with the command -v check is acceptable and protects against errors if wget is already present.


📊 Summary

Files Checked: 4 workflows

  • remove-stale-branches.yml - Perfect (line 13: [self-hosted, linux, x64])
  • certora-prover.yml - All issues fixed (runner labels added, Python setup improved)
  • ⚠️ automation.yml - 1 critical issue remains (missing runner labels on line 29)
  • ⚠️ checks.yml - 1 improvement remains (wget installation approach is suboptimal but functional)

Critical Issues Remaining: 1 (missing runner labels in automation.yml)
Improvements Suggested: 1 (pre-install wget in runner image)

Recommendation: The automation.yml runner labels issue should be fixed before merging to ensure consistency and prevent potential runtime failures. The wget installation approach is acceptable as-is if modifying the runner image isn't feasible.


@elhajin elhajin closed this Mar 16, 2026
@vineetguptadev
vineetguptadev force-pushed the INF2-migrate-to-self-hosted branch from 7df9aa5 to 2172a20 Compare March 19, 2026 13:49

@elhajin elhajin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

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

Labels

📜 Chore Maintenance related changes (CI, QOL, etc.). 🚗 CI Continuous Integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants