Fix worker images, code-upload git dependency, and CI/Cloud Agent Docker bootstrap#5141
Merged
RishabhJain2018 merged 8 commits intoJul 8, 2026
Merged
Conversation
…tored ECR URL Challenges with worker_python_version=3.9 but a stale EvalAI-managed worker_image_url pointing at worker-py3.7 were still deployed on Python 3.7 images, causing runtime import failures such as missing boto3. Resolve EvalAI-managed worker images from worker_python_version while preserving the existing commit tag, sync worker_image_url during task definition refresh, and add worker image import verification to py3.9 Dockerfiles. Co-authored-by: Rishabh Jain <rishabhjain2018@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR refactors EvalAI-managed worker image tag resolution and task-definition refresh logic with expanded test coverage, updates worker Dockerfiles with dependency pins and verification steps, adds git to code-upload-worker runtime images, and introduces a cloud-agent Docker bootstrap script with updated documentation. ChangesWorker image tag resolution and refresh
Worker build and bootstrap updates
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant refresh_task_definition_for_challenge
participant get_worker_image_for_challenge
participant Challenge
Caller->>refresh_task_definition_for_challenge: trigger refresh
refresh_task_definition_for_challenge->>get_worker_image_for_challenge: resolve WORKER_IMAGE
get_worker_image_for_challenge-->>refresh_task_definition_for_challenge: resolved_worker_image
refresh_task_definition_for_challenge->>Challenge: save(update_fields=[task_def_arn, worker_image_url?])
sequenceDiagram
participant EnvironmentSetup
participant InstallScript
participant AptPackageManager
participant DockerDaemon
EnvironmentSetup->>InstallScript: bash cloud-agent/install.sh
InstallScript->>AptPackageManager: detect/install compose package + docker.io
InstallScript->>InstallScript: configure_iptables_legacy_if_available
InstallScript->>InstallScript: configure_docker_daemon (daemon.json)
InstallScript->>DockerDaemon: start_docker_daemon (systemctl or dockerd fallback)
DockerDaemon-->>InstallScript: docker info (poll up to 30s)
InstallScript-->>EnvironmentSetup: print docker/compose versions
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
install_dependencies.sh uses kubectl apply -k with github.com URLs for EFS/EBS CSI drivers, which requires git in the container PATH. Co-authored-by: Rishabh Jain <rishabhjain2018@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/unit/challenges/test_aws_utils.py (1)
5903-5965: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLGTM! The test properly verifies
worker_image_urlsynchronization andsave(update_fields=...)call without mockingget_image_settings_for_challenge, providing good integration coverage.Consider adding a test for the no-op case where a managed
worker_image_urlalready matches the resolved image — that path should save with onlyupdate_fields=["task_def_arn"].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/challenges/test_aws_utils.py` around lines 5903 - 5965, Add a unit test for the no-op branch in refresh_task_definition_for_challenge where the existing managed worker_image_url already matches the resolved image settings, so only task_def_arn should be updated. Reuse the existing test setup around refresh_task_definition_for_challenge and assert that challenge.save is called with update_fields=["task_def_arn"] and that worker_image_url is left unchanged.apps/challenges/aws_utils.py (1)
1103-1111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWrap the conditional to fit black’s 79-char limit.
Suggested reformatting
update_fields = ["task_def_arn"] resolved_worker_image = image_settings["WORKER_IMAGE"] - if challenge.worker_image_url and is_evalai_managed_submission_worker_image( - challenge.worker_image_url - ): + if ( + challenge.worker_image_url + and is_evalai_managed_submission_worker_image( + challenge.worker_image_url + ) + ): if challenge.worker_image_url != resolved_worker_image: challenge.worker_image_url = resolved_worker_image update_fields.append("worker_image_url") challenge.save(update_fields=update_fields)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/challenges/aws_utils.py` around lines 1103 - 1111, The conditional in aws_utils around challenge.worker_image_url and is_evalai_managed_submission_worker_image exceeds Black’s 79-character limit and should be reformatted. Wrap the if statement across multiple lines in the same block that updates update_fields and challenge.worker_image_url, keeping the logic unchanged while making the expression fit the formatter’s line-length rules.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/prod/worker_py3_9/Dockerfile`:
- Around line 75-83: The runtime import check in the worker Dockerfile uses
yaml, but the package is not declared in the worker dependency set. Add PyYAML
to the relevant requirements used by the prod worker build, specifically the
worker_py3_9 requirements chain that feeds submission_worker.py, and ensure the
dependency is included where the Dockerfile’s import verification expects it.
---
Nitpick comments:
In `@apps/challenges/aws_utils.py`:
- Around line 1103-1111: The conditional in aws_utils around
challenge.worker_image_url and is_evalai_managed_submission_worker_image exceeds
Black’s 79-character limit and should be reformatted. Wrap the if statement
across multiple lines in the same block that updates update_fields and
challenge.worker_image_url, keeping the logic unchanged while making the
expression fit the formatter’s line-length rules.
In `@tests/unit/challenges/test_aws_utils.py`:
- Around line 5903-5965: Add a unit test for the no-op branch in
refresh_task_definition_for_challenge where the existing managed
worker_image_url already matches the resolved image settings, so only
task_def_arn should be updated. Reuse the existing test setup around
refresh_task_definition_for_challenge and assert that challenge.save is called
with update_fields=["task_def_arn"] and that worker_image_url is left unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a7bee042-5ae9-4cac-bb92-ac44f348e498
📒 Files selected for processing (4)
apps/challenges/aws_utils.pydocker/dev/worker_py3_9/Dockerfiledocker/prod/worker_py3_9/Dockerfiletests/unit/challenges/test_aws_utils.py
- Pin Cython to 0.29.36 so Python 3.8/3.9 worker images use prebuilt wheels instead of failing to compile cython==0.29 from source during CI setup - Override PyJWT in worker py3.7/py3.8 Docker builds (2.13 requires Python 3.9+) - Add Cloud Agent bootstrap script and invoke it from environment.json before docker compose build Co-authored-by: Rishabh Jain <rishabhjain2018@gmail.com>
RishabhJain2018
marked this pull request as ready for review
July 8, 2026 06:19
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
- Use docker-compose-v2 on Noble (docker-compose-plugin is unavailable) - Skip iptables-legacy when alternatives are not present - Fall back to starting dockerd directly when systemd is unavailable - Use sudo for docker daemon health checks before group membership applies - Apply black formatting fix to aws_utils.py for CI code quality Co-authored-by: Rishabh Jain <rishabhjain2018@gmail.com>
cursor
Bot
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/cloud-agent/bootstrap-docker.sh`:
- Around line 73-85: The Docker bootstrap flow in bootstrap-docker.sh does not
verify the result of ensure_docker_compose_package before using compose_package,
so an unavailable compose package can lead to an empty apt-get install argument.
Update the installation path to check the return status of
ensure_docker_compose_package immediately after calling it, and abort with a
clear error message if no compose package is found. Keep the fix localized to
the Docker install block that calls ensure_docker_compose_package, apt-get
install, and configure_docker_daemon.
- Around line 56-58: The dockerd startup block writes logs to a predictable /tmp
path, which is vulnerable to symlink attacks. Update the bootstrap-docker.sh
logic around the dockerd launch to use a unique temporary file created with
mktemp instead of the hardcoded /tmp/dockerd.log, and direct dockerd output to
that generated path in the background command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b75435b9-c27c-4201-9a26-b767d963553f
📒 Files selected for processing (3)
AGENTS.mdapps/challenges/aws_utils.pyscripts/cloud-agent/bootstrap-docker.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/challenges/aws_utils.py
Relocate the Docker bootstrap script from scripts/cloud-agent/ to a top-level cloud-agent/ folder and update environment.json and AGENTS.md. Co-authored-by: Rishabh Jain <rishabhjain2018@gmail.com>
submission_worker imports yaml and the worker_py3_9 Dockerfile verifies it at build time. Declare PyYaml explicitly in worker_py3_9.txt (matching the code-upload worker pin) instead of relying only on the transitive common.txt dependency. Co-authored-by: Rishabh Jain <rishabhjain2018@gmail.com>
- Use mktemp for dockerd log path in cloud-agent/install.sh - Abort Docker install when no compose package is available - Add unit test for no-op worker_image_url sync on task def refresh Co-authored-by: Rishabh Jain <rishabhjain2018@gmail.com>
Co-authored-by: Rishabh Jain <rishabhjain2018@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5141 +/- ##
==========================================
- Coverage 91.22% 91.21% -0.01%
==========================================
Files 114 114
Lines 8852 8865 +13
==========================================
+ Hits 8075 8086 +11
- Misses 777 779 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes worker image resolution for challenges migrated to Python 3.9, addresses CI build failures, and resolves CodeRabbit review feedback.
Worker image resolution
get_worker_image_for_challenge()now resolves EvalAI-managed ECR images fromworker_python_version, preserving existing tags when appropriate.refresh_task_definition_for_challenge()syncsworker_image_urlwhen the stored URL is an EvalAI-managed image that differs from the resolved image.Docker / CI fixes
cython==0.29.36in all worker Dockerfiles (Python 3.9 wheel compatibility).PyJWTfor py3.7/py3.8 worker builds; addPyYaml==5.4.1toworker_py3_9.txt.boto3,botocore,django,requests,yaml) in py3.9 worker Dockerfiles.gitto code-upload-worker Dockerfiles.Cloud Agent bootstrap
cloud-agent/install.sh.mktempfor dockerd logs; abort if no compose package is found.docker-compose-v2, skip iptables-legacy).Tests
worker_image_urlsync on task definition refresh (including no-op when already synced).Ops note
After deploy, refresh stale task definitions:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation