Skip to content

Conversation

@dillon-cullinan
Copy link
Contributor

@dillon-cullinan dillon-cullinan commented Oct 30, 2025

Overview:

Testing new CI runner image that has tools pre-installed

Summary by CodeRabbit

  • Chores
    • Updated GitHub Actions workflow configurations with optimized test runner specifications
    • Streamlined deployment workflow steps by removing redundant dependency installations
    • Simplified build pipeline infrastructure for improved efficiency

Signed-off-by: Dillon Cullinan <[email protected]>
@dillon-cullinan dillon-cullinan requested a review from a team as a code owner October 30, 2025 17:22
@github-actions github-actions bot added the ci Issues/PRs that reference CI build/test label Oct 30, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 30, 2025

Walkthrough

Removed AWS CLI installation steps from GitHub Actions workflows and updated runner configurations. Multiple workflow jobs transitioned from standard runners to "-test" variants. Namespace setup steps were simplified by removing dependency installation procedures, streamlining the container validation backend workflow.

Changes

Cohort / File(s) Summary
AWS CLI Installation Removal
.github/actions/docker-build/action.yml, .github/workflows/container-validation-backends.yml
Deleted explicit AWS CLI download, extraction, and installation steps. AWS CLI is no longer provisioned during workflow execution.
Runner Configuration Updates
.github/workflows/container-validation-backends.yml
Updated matrix runner names and runs-on configurations across operator, vllm, sglang, trtllm, and deploy-test jobs from standard runners (e.g., -2xlarge) to test variants (e.g., -2xlarge-test, -test).
Namespace Setup Simplification
.github/workflows/container-validation-backends.yml
Renamed step from "Set namespace and install dependencies" to "Set namespace". Removed dependency installation block including curl, apt-get, yq, Helm, and kubectl setup.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Runner name updates across multiple jobs are homogeneous, repetitive changes requiring minimal per-file reasoning
  • AWS CLI removal is a consistent pattern deletion across files
  • Namespace setup simplification involves straightforward step removal
  • Key attention area: Verify that AWS CLI removal does not break ECR login or other workflow steps that may depend on AWS CLI availability; confirm test runners provide necessary environment

Poem

🐰 The runners hop to their test corrals now,
AWS CLI steps fade to memory's bow,
Dependencies pruned, workflows grow lean,
Container validation, cleaner and keen!
Simplified paths through the GitHub machine! 🚀

Pre-merge checks

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description is largely incomplete and missing several required sections from the template. While an Overview section is present with a brief explanation about testing a new CI runner image, the description lacks the Details section (describing specific changes), the "Where should the reviewer start?" section (calling out specific files), and the Related Issues section (with action keywords). Given that only one of four major template sections is provided, the description does not meet the repository's documentation standards for pull requests. The author should expand the PR description by adding the missing sections: provide a detailed list of changes made across .github/actions/docker-build/action.yml and .github/workflows/container-validation-backends.yml, specify which files reviewers should focus on first, and include a Related Issues section that formally links to the OPS-1744 ticket using an action keyword like "Closes" or "Relates to".
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "ci: OPS-1744: Remove tool installs and test new CI image" accurately reflects the main changes in the pull request. The summary shows that tool installations (AWS CLI) were removed from the workflow and runner names were updated to test-variants, which aligns directly with the title's mention of removing tool installs and testing a new CI image. The title is concise, specific, and provides meaningful context about the primary change without vague terminology.

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.

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/container-validation-backends.yml (1)

366-522: Critical: "-test" runner image is missing required tools (yq, helm, kubectl).

Verification confirms that the -test runner does not have yq, helm, or kubectl pre-installed. The workflow uses all three tools in the "Deploy Operator" step but removed dependency installation in the namespace setup (lines 366–379), causing the workflow to fail at runtime.

Tools missing:

  • yq (lines 435, 437, 449)
  • helm (lines 415–419)
  • kubectl (lines 378, 393, 426, 449, etc.)

Required fixes:

  1. Restore dependency installation in the "Set namespace" step or add a separate "Install tools" step to the "Deploy Operator" job, or
  2. Ensure the -test runner image is built with these tools pre-installed before merging.

Verify the intended approach with the infrastructure team and update accordingly.

📜 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 1898601 and 856e430.

📒 Files selected for processing (2)
  • .github/actions/docker-build/action.yml (0 hunks)
  • .github/workflows/container-validation-backends.yml (8 hunks)
💤 Files with no reviewable changes (1)
  • .github/actions/docker-build/action.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/container-validation-backends.yml

345-345: label "cpu-amd-m5-2xlarge-test" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


547-547: label "cpu-amd-m5-2xlarge-test" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


567-567: label "cpu-amd-m5-2xlarge-test" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

⏰ 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). (9)
  • GitHub Check: sglang (amd64)
  • GitHub Check: sglang (arm64)
  • GitHub Check: trtllm (amd64)
  • GitHub Check: trtllm (arm64)
  • GitHub Check: vllm (arm64)
  • GitHub Check: operator (arm64)
  • GitHub Check: operator (amd64)
  • GitHub Check: vllm (amd64)
  • GitHub Check: Build and Test - dynamo

Signed-off-by: Dillon Cullinan <[email protected]>
Signed-off-by: Dillon Cullinan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Issues/PRs that reference CI build/test size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants