Skip to content

<fix>[storage]: honor force flag to clean image cache for existing images with no VMs (ZSTAC-79628)#3404

Open
zstack-robot-1 wants to merge 1 commit into5.5.12from
sync/ye.zou/fix/ZSTAC-79628
Open

<fix>[storage]: honor force flag to clean image cache for existing images with no VMs (ZSTAC-79628)#3404
zstack-robot-1 wants to merge 1 commit into5.5.12from
sync/ye.zou/fix/ZSTAC-79628

Conversation

@zstack-robot-1
Copy link
Collaborator

Problem

When calling APICleanUpImageCacheOnPrimaryStorageMsg with force=true, the intention is to also clean up image cache entries for images that still exist but have no VMs using them. However, createShadowImageCacheVOsForNewDeletedAndOld in both the base class (ImageCacheCleaner) and CephImageCacheCleaner hardcoded false when calling getStaleImageCacheIds, ignoring the param.includeReadyImage flag.

Fix

Pass param.includeReadyImage to getStaleImageCacheIds in:

  • ImageCacheCleaner.createShadowImageCacheVOsForNewDeletedAndOld (covers NFS, SMP, SharedBlock)
  • CephImageCacheCleaner.createShadowImageCacheVOsForNewDeletedAndOld

With this fix, calling the cleanup API with force=true will now correctly clean image caches that meet the criteria:

  1. Image still exists (in ready state)
  2. No VMs are using the image
  3. No volume snapshot trees reference the image

Note: LocalStorageImageCleaner already used param.includeReadyImage correctly.

Resolves: ZSTAC-79628

sync from gitlab !9252

…ages with no VMs

Resolves: ZSTAC-79628

Change-Id: I5a7a0941a59bcea132ea97df52bbebdc9a227508
@coderabbitai
Copy link

coderabbitai bot commented Mar 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e734c2f and 944c7a8.

📒 Files selected for processing (2)
  • plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephImageCacheCleaner.java
  • storage/src/main/java/org/zstack/storage/primary/ImageCacheCleaner.java

总体说明

将图像缓存清理逻辑中的硬编码布尔值替换为参数 includeReadyImage,使得清理流程可根据是否强制清理来动态决定是否包含处于 READY 状态的镜像。

变更清单

变更内容分组 / 文件路径 变更摘要
镜像缓存清理参数化
plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephImageCacheCleaner.java, storage/src/main/java/org/zstack/storage/primary/ImageCacheCleaner.java
getStaleImageCacheIds() 调用中的硬编码 false 参数替换为 param.includeReadyImage,允许根据清理策略动态决定是否将 READY 状态的镜像视为陈旧镜像,同时更新相关注释以反映新行为。

代码审查工作量估算

🎯 2 (Simple) | ⏱️ ~10 分钟

诗歌颂

参数轻轻起舞,布尔值翩翩然,🐰
镜像缓存智慧增,清理策略更灵活,
READY 状态被尊重,硬编码已远离,
两处修改保一致,代码优雅且通达!✨


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Title check ❌ Error PR标题超过了72字符的限制(99字符),不符合格式要求。 缩短标题长度至72字符以内,例如:[storage]: honor force flag for stale image cache (ZSTAC-79628)
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed PR描述清晰完整地说明了问题、修复方案和相关文件,与代码变更高度相关。
✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sync/ye.zou/fix/ZSTAC-79628

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

2 participants