Skip to content

Conversation

junhaoliao
Copy link
Member

@junhaoliao junhaoliao commented Oct 3, 2025

Description

This change updates the Docker cleanup logic to ignore errors that may occur when attempting to remove the clp-package development image. The adjustment prevents the build process from failing if the image cannot be removed, addressing issue #1370.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  1. Ran task to build the CLP package and the docker image.
  2. cd build/clp-package/; ./sbin/start-clp.sh
  3. Made a dummy change (e.g. add a space in any string in the clp-py-utils) so that the package assets are expected to change in the next build.
  4. Ran task again and observe the build was able to complete successfully, although warnings were printing about the build cannot be removed:
    junhao@GIGABYTE:~/workspace/clp$ task
    ...
    Removing previous image sha256:af2520c2d003f19464685b491004ae8a75c33d9bb9dcf98b9943e3fb631a9a7a.
    Error response from daemon: conflict: unable to delete af2520c2d003 (cannot be forced) - image is being used by running container 7cceeee1d08a
    task: [docker-images:package] rsync --archive '/home/junhao/workspace/clp/build/clp-package-image.id' '/home/junhao/workspace/clp/build/clp-package'
    junhao@GIGABYTE:~/workspace/clp$ echo $?
    0

Summary by CodeRabbit

  • Bug Fixes

    • Prevents intermittent build failures by tolerating errors during old image removal, allowing packaging to proceed reliably.
  • Chores

    • Adds a guard to skip image removal when none exists, streamlining the packaging workflow and reducing noise in build logs.
    • Enhances release pipeline reliability and reduces downtime during deployments.
    • No user-facing changes; improves build stability and developer experience.

@junhaoliao junhaoliao requested a review from a team as a code owner October 3, 2025 00:02
@junhaoliao junhaoliao self-assigned this Oct 3, 2025
Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

Walkthrough

Updates tools/docker-images/clp-package/build.sh to guard image removal: exit early if the previous image is absent; if present, print a message and attempt removal. The removal command tolerates failure (using “|| true”) to prevent the script from stopping on errors.

Changes

Cohort / File(s) Summary of Changes
Docker image cleanup guard
tools/docker-images/clp-package/build.sh
Added early return when previous image is absent; if present, echo and attempt docker rmi; make removal non-fatal with `

Sequence Diagram(s)

sequenceDiagram
    participant S as build.sh
    participant D as Docker Daemon

    S->>D: Check if previous image exists
    alt Image exists
        S->>D: docker rmi <prev-image>
        note right of S: Removal is tolerant (|| true)
        D-->>S: Success or failure (ignored)
    else No image
        S-->>S: Return early (skip removal)
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit's high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the change to ignore errors when removing the clp-package development Docker image and references the relevant issue, aligning with the PR’s main purpose.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 632eecb and 197b5a3.

📒 Files selected for processing (1)
  • tools/docker-images/clp-package/build.sh (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh:3-5
Timestamp: 2025-07-07T17:43:04.349Z
Learning: In CLP project build scripts (specifically build.sh files in docker-images directories), maintain consistency with the established pattern of using separate `set -eu` and `set -o pipefail` commands rather than combining them into `set -euo pipefail`, to ensure uniform script structure across all platform build scripts.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86/build.sh:18-24
Timestamp: 2025-07-01T14:52:02.418Z
Learning: In the CLP project, consistency across platform build scripts is prioritized over defensive programming when it comes to git remote handling. All build.sh files in docker-images directories should follow the same pattern for git metadata injection.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86/build.sh:18-24
Timestamp: 2025-07-01T14:52:02.418Z
Learning: In the CLP project, consistency across platform build scripts is prioritized over defensive programming when it comes to git remote handling. All build.sh files in docker-images directories should follow the same pattern for git metadata injection.
📚 Learning: 2025-07-01T14:52:02.418Z
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86/build.sh:18-24
Timestamp: 2025-07-01T14:52:02.418Z
Learning: In the CLP project, consistency across platform build scripts is prioritized over defensive programming when it comes to git remote handling. All build.sh files in docker-images directories should follow the same pattern for git metadata injection.

Applied to files:

  • tools/docker-images/clp-package/build.sh
⏰ 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). (5)
  • GitHub Check: package-image
  • GitHub Check: rust-checks (macos-15)
  • GitHub Check: lint-check (macos-15)
  • GitHub Check: rust-checks (ubuntu-24.04)
  • GitHub Check: rust-checks (ubuntu-22.04)

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.

@junhaoliao junhaoliao changed the title fix(docker): Ignore errors during dev image removal fix(docker): Ignore errors from clp-package dev image removal (fixes y-scope#1370). Oct 3, 2025
@junhaoliao junhaoliao changed the title fix(docker): Ignore errors from clp-package dev image removal (fixes y-scope#1370). fix(docker): Ignore errors from clp-package dev image removal (fixes #1370). Oct 3, 2025
@junhaoliao junhaoliao removed their assignment Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant