-
Notifications
You must be signed in to change notification settings - Fork 17
Fix CI container name collision for parallel matrix jobs #212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d68c8cc
0d8e639
1b2e1ce
8b60256
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -78,6 +78,7 @@ jobs: | |||||||||
| runs-on: ${{ matrix.runner }} | ||||||||||
| env: | ||||||||||
| ATOM_BASE_NIGTHLY_IMAGE: rocm/atom-dev:latest | ||||||||||
| CONTAINER_NAME: atom_test_${{ strategy.job-index }} | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Follow the co-pilot: Consider change CONTAINER_NAME: atom_test_${{ strategy.job-index }} The reason is strategy.job-index is only unique within a single workflow run. If two PRs trigger CI at the same time and are scheduled on the same runner, both runs may have job-index=0, resulting in both generating atom_test_0 and causing a naming collision. Adding github.run_id. Even if two workflow runs execute concurrently on the same runner, they will not collide. |
||||||||||
| GITHUB_REPO_URL: ${{ github.event.pull_request.head.repo.clone_url || 'https://github.com/ROCm/ATOM.git' }} | ||||||||||
| GITHUB_COMMIT_SHA: ${{ github.event.pull_request.head.sha || github.event.head_commit.id }} | ||||||||||
|
|
||||||||||
|
|
@@ -151,8 +152,8 @@ jobs: | |||||||||
| - name: Start CI container | ||||||||||
| run: | | ||||||||||
| echo "Clean up containers..." | ||||||||||
| (docker ps -aq -f name=atom_test | xargs -r docker stop) || true | ||||||||||
| (docker ps -aq -f name=atom_test | xargs -r docker rm) || true | ||||||||||
| (docker ps -aq -f name="^${CONTAINER_NAME}$" | xargs -r docker stop) || true | ||||||||||
| (docker ps -aq -f name="^${CONTAINER_NAME}$" | xargs -r docker rm) || true | ||||||||||
|
Comment on lines
+155
to
+156
|
||||||||||
| (docker ps -aq -f name="^${CONTAINER_NAME}$" | xargs -r docker stop) || true | |
| (docker ps -aq -f name="^${CONTAINER_NAME}$" | xargs -r docker rm) || true | |
| (docker ps -aq -f name="${CONTAINER_NAME}" | xargs -r docker stop) || true | |
| (docker ps -aq -f name="${CONTAINER_NAME}" | xargs -r docker rm) || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strategy.job-indexis not a valid GitHub Actions context variable. GitHub Actions does not provide a built-in job index for matrix jobs. Consider using a combination of matrix values to create unique container names instead. For example:github.run_idwith a sanitized version ofmatrix.model_namegithub.run_idwithgithub.run_attemptand sanitized matrix valuesExample:
CONTAINER_NAME: atom_test_${{ github.run_id }}_${{ github.run_attempt }}_${{ matrix.model_name }}Note: You'll need to sanitize
matrix.model_nameto remove special characters that are invalid in container names (e.g., replace spaces and special chars with underscores).