fix(workflows): clean temp arch tags after merge#18
Conversation
- add Docker Hub tag cleanup after successful manifest publish in pd/store/server flow - keep partial amd64 availability when arm64 fails by cleaning only in publish_manifest job - document the critical pd/store/server release pipeline with an ASCII workflow diagram - align AGENTS.md with the staged dual-arch publish and cleanup semantics
dosubot
bot
added
the
size:L
工作流程说明在 变更
预期代码审查工作量🎯 2 (简单) | ⏱️ ~10 分钟 可能相关的 PR
建议标签
诗歌
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the documentation in AGENTS.md and README.md to reflect changes in the CI/CD workflow for publishing pd/store/server Docker images. The changes document the new behavior of cleaning up temporary architecture-specific tags only after a successful multi-arch manifest creation. The documentation is well-written, and the inclusion of an ASCII diagram in README.md is very helpful for understanding the workflow. I have a couple of suggestions to further improve the clarity and potentially the efficiency of the documented process.
| publish_amd64 (matrix x4 modules) | ||
| +-------------------------------------------------+ | ||
| | pd | store | server-hstore | server-standalone | | ||
| +-------------------------------------------------+ | ||
| push x.y.z-amd64 (or latest-amd64) | ||
| | | ||
| v | ||
| publish_arm64 (matrix x4 modules) | ||
| push x.y.z-arm64 (or latest-arm64) |
There was a problem hiding this comment.
The ASCII diagram shows publish_arm64 running after publish_amd64. If the builds for different architectures are independent, they could be run in parallel to speed up the workflow. The publish_manifest job would then need to depend on both publish_amd64 and publish_arm64 completing successfully.
This would change the diagram to show parallel execution, for example:
integration_precheck (optional)
|
+-----------------+-----------------+
| |
v v
publish_amd64 (matrix x4) publish_arm64 (matrix x4)
| |
+-----------------+-----------------+
|
v
publish_manifest (matrix x4 modules)
If the sequential execution is intentional (e.g., due to runner limitations), consider adding a note to explain why.
|
|
||
| Tag behavior: | ||
|
|
||
| - If only amd64 is published and arm64 fails, manifest is not created and `*-amd64` remains available. |
There was a problem hiding this comment.
This sentence is slightly ambiguous. "If only amd64 is published" could be interpreted in different ways. For clarity, consider rephrasing to focus on the success/failure of the workflow jobs. For example: "If the amd64 publish succeeds but the arm64 publish fails, the manifest is not created and the *-amd64 tag remains available."
There was a problem hiding this comment.
Pull request overview
This PR updates the pd/store/server publishing pipeline to clean up temporary per-architecture Docker Hub tags after a successful multi-arch manifest publish, and documents the intended staged publish semantics.
Changes:
- Add a Docker Hub tag-deletion step in
publish_manifestto remove*-amd64/*-arm64tags after manifest creation. - Document the critical
pd/store/serverpipeline and tag semantics inREADME.mdandAGENTS.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| README.md | Adds an ASCII diagram and tag-behavior notes for the pd/store/server critical publish path. |
| AGENTS.md | Aligns repository guidance with the staged dual-arch publish + cleanup semantics for pd/store/server. |
| .github/workflows/_publish_pd_store_server_reusable.yml | Implements temporary Docker Hub tag cleanup after manifest publish. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [`.github/workflows/_publish_pd_store_server_reusable.yml`](./.github/workflows/_publish_pd_store_server_reusable.yml). | ||
|
|
||
| ```text | ||
| source branch (master / release-x.y.z) |
There was a problem hiding this comment.
The diagram references master, but the rest of the README describes the default branch as main (e.g., “current main branch line”). This is likely inaccurate/confusing; consider changing the diagram to main (or main/master if both are supported).
| source branch (master / release-x.y.z) | |
| source branch (main / release-x.y.z) |
| - name: Delete temporary arch tags after manifest publish (${{ matrix.module }}) | ||
| env: | ||
| IMAGE_REPO: ${{ matrix.image_repo }} | ||
| VERSION_TAG: ${{ env.VERSION_TAG }} | ||
| DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }} | ||
| DOCKERHUB_PASSWORD: ${{ secrets.DOCKERHUB_PASSWORD }} | ||
| run: | |
There was a problem hiding this comment.
If this tag-cleanup step fails (e.g., Docker Hub API hiccup), the entire publish_manifest job will fail even though the manifest may already be created, and update_latest_hash will be skipped. Consider making cleanup non-blocking (e.g., continue-on-error: true) or moving it to a separate job so successful publishes still count as success/have their hash updated.
| status_code="$( | ||
| curl -sS -o /tmp/dockerhub-delete-response.txt -w "%{http_code}" -X DELETE \ | ||
| -H "Authorization: JWT $auth_token" \ | ||
| "https://hub.docker.com/v2/repositories/${namespace}/${repository}/tags/${tag}/" | ||
| )" |
There was a problem hiding this comment.
The retry loop won’t actually retry on transient network failures because set -e will exit the script if the curl command itself returns non-zero inside the command substitution. Consider handling curl failures explicitly (e.g., capture the exit code and treat it as a retryable failure) so the backoff logic is effective.
| status_code="$( | |
| curl -sS -o /tmp/dockerhub-delete-response.txt -w "%{http_code}" -X DELETE \ | |
| -H "Authorization: JWT $auth_token" \ | |
| "https://hub.docker.com/v2/repositories/${namespace}/${repository}/tags/${tag}/" | |
| )" | |
| if ! status_code="$( | |
| curl -sS -o /tmp/dockerhub-delete-response.txt -w "%{http_code}" -X DELETE \ | |
| -H "Authorization: JWT $auth_token" \ | |
| "https://hub.docker.com/v2/repositories/${namespace}/${repository}/tags/${tag}/" | |
| )"; then | |
| # Treat curl failures (e.g., network errors) as a retryable failure | |
| status_code="000" | |
| fi |
| curl --fail-with-body -sS -X POST "https://hub.docker.com/v2/users/login/" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{"username":"'"$DOCKERHUB_USERNAME"'","password":"'"$DOCKERHUB_PASSWORD"'"}' \ |
There was a problem hiding this comment.
The JSON request body is built via shell string concatenation, which will break if the Docker Hub username/password contain quotes or backslashes. Consider generating the JSON safely (e.g., via jq -nc --arg ... or another proper JSON encoder) to avoid auth failures with certain credentials.
| curl --fail-with-body -sS -X POST "https://hub.docker.com/v2/users/login/" \ | |
| -H "Content-Type: application/json" \ | |
| -d '{"username":"'"$DOCKERHUB_USERNAME"'","password":"'"$DOCKERHUB_PASSWORD"'"}' \ | |
| jq -nc --arg u "$DOCKERHUB_USERNAME" --arg p "$DOCKERHUB_PASSWORD" '{username:$u, password:$p}' \ | |
| | curl --fail-with-body -sS -X POST "https://hub.docker.com/v2/users/login/" \ | |
| -H "Content-Type: application/json" \ | |
| -d @- \ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/_publish_pd_store_server_reusable.yml (1)
500-509: 建议使用jq构建 JSON 请求体以避免潜在的注入问题。当前使用 shell 字符串拼接构建 JSON，如果凭据中包含
"或\等特殊字符，可能导致 JSON 格式错误或意外行为。♻️ 使用 jq 安全构建 JSON
auth_token="$( - curl --fail-with-body -sS -X POST "https://hub.docker.com/v2/users/login/" \ - -H "Content-Type: application/json" \ - -d '{"username":"'"$DOCKERHUB_USERNAME"'","password":"'"$DOCKERHUB_PASSWORD"'"}' \ - | jq -r '.token' + jq -n --arg u "$DOCKERHUB_USERNAME" --arg p "$DOCKERHUB_PASSWORD" \ + '{username: $u, password: $p}' \ + | curl --fail-with-body -sS -X POST "https://hub.docker.com/v2/users/login/" \ + -H "Content-Type: application/json" -d `@-` \ + | jq -r '.token' )"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/_publish_pd_store_server_reusable.yml around lines 500 - 509, The current auth_token curl invocation builds the JSON body via shell string concatenation which can break if DOCKERHUB_USERNAME or DOCKERHUB_PASSWORD contain special characters; change the payload construction to use jq to safely create JSON (e.g. use jq -n --arg user "$DOCKERHUB_USERNAME" --arg pass "$DOCKERHUB_PASSWORD" '{username:$user,password:$pass}') and pipe that output into curl -d `@-` so the curl call in the auth_token assignment uses the jq-generated JSON; update the auth_token block (the curl command that sets auth_token and checks it) to use this jq-built body to avoid injection/escaping issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/_publish_pd_store_server_reusable.yml:
- Around line 500-509: The current auth_token curl invocation builds the JSON
body via shell string concatenation which can break if DOCKERHUB_USERNAME or
DOCKERHUB_PASSWORD contain special characters; change the payload construction
to use jq to safely create JSON (e.g. use jq -n --arg user "$DOCKERHUB_USERNAME"
--arg pass "$DOCKERHUB_PASSWORD" '{username:$user,password:$pass}') and pipe
that output into curl -d `@-` so the curl call in the auth_token assignment uses
the jq-generated JSON; update the auth_token block (the curl command that sets
auth_token and checks it) to use this jq-built body to avoid injection/escaping
issues.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b195797c-b813-4ce8-834a-19217c5e5850
📒 Files selected for processing (3)
.github/workflows/_publish_pd_store_server_reusable.ymlAGENTS.mdREADME.md
Summary by CodeRabbit
发布说明
文档
pd/store/server模块使用专用发布流程，包括预检查和分阶段的多架构构建与清理语义。维护