Conversation
WalkthroughAdds a new GitHub Actions workflow that provisions an EC2-based self-hosted runner via OIDC/AWS, runs an OpenShift docling E2E job on that runner, then stops the EC2 instance as a cleanup step. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant OIDC as GitHub OIDC
participant AWS as AWS (STS / EC2)
participant Runner as EC2 Self-hosted Runner
GH->>OIDC: Request OIDC token (workflow)
OIDC-->>GH: Return token
GH->>AWS: Assume IAM role using token (STS)
AWS-->>GH: Return temporary credentials
GH->>AWS: Start EC2 instance (AMI, subnet, tags) -> returns instance-id & label
AWS-->>GH: Provide instance metadata
GH->>Runner: Dispatch `openshift-docling-e2e` job to runner label
Runner-->>GH: Checkout repo, run steps, emit logs
GH->>AWS: stop-ec2-runner job calls stop/terminate with instance-id
AWS-->>GH: Stop/terminate instance (cleanup)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (12)
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 |
| push: | ||
| branches: | ||
| - main | ||
| - ec2-github-runner-RHAIENG-2487 |
There was a problem hiding this comment.
I've left this in for easy testing. You can remove it.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/openshift-docling-e2e.yaml (1)
90-113: Consider handling the case where launch-ec2-runner fails.The
stop-ec2-runnerjob usesif: always()to ensure cleanup, which is good. However, if thelaunch-ec2-runnerjob fails before successfully starting an instance, the outputslabelandec2-instance-idwill be empty, potentially causing this job to fail with confusing error messages.While there's no instance to stop if launch fails, consider adding a conditional check to improve error handling and clarity.
🔎 Proposed enhancement
stop-ec2-runner: permissions: id-token: write # This is required for OIDC (AWS auth) contents: read needs: - launch-ec2-runner - openshift-docling-e2e runs-on: ubuntu-latest - if: ${{ always() }} + if: ${{ always() && needs.launch-ec2-runner.outputs.ec2-instance-id != '' }} steps:This ensures the stop job only runs if an instance was actually created.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/openshift-docling-e2e.yaml(1 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: e2e-test (v1.27.16)
- GitHub Check: build-helm-chart
- GitHub Check: launch-ec2-runner
🔇 Additional comments (6)
.github/workflows/openshift-docling-e2e.yaml (6)
10-13: Acknowledged: Future use input defined.The
pr_or_branchinput is defined but not yet utilized in the workflow. This aligns with the PR description indicating that test commands will be filled out in future commits.
19-25: LGTM!The environment variable and shell defaults are configured appropriately.
38-39: LGTM!Repository checkout is appropriately configured.
41-46: LGTM!AWS credentials configuration using OIDC is properly set up with appropriate permissions and session naming for CloudTrail tracking.
84-88: LGTM!The placeholder test commands are appropriate given the PR description states that "the test commands will be filled out" in a future commit.
100-105: LGTM!AWS credentials configuration for the cleanup job is correctly configured and consistent with the launch job.
ff08646 to
14dbd2b
Compare
Introduce a new job that runs on an ephemeral AWS ec2 runner. In a future commit we will fill out the test commands.
14dbd2b to
fa056ec
Compare
|
I found an error in the |
Introduce a new job that runs on an ephemeral AWS ec2 runner.
In a future commit we will fill out the test commands.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.