Skip to content

[ceph]: skip ceph ops on non-enabled cluster timeout#3947

Open
zstack-robot-1 wants to merge 1 commit into
5.5.22from
sync/songtao.liu/fix/ZSTAC-80275-v2
Open

[ceph]: skip ceph ops on non-enabled cluster timeout#3947
zstack-robot-1 wants to merge 1 commit into
5.5.22from
sync/songtao.liu/fix/ZSTAC-80275-v2

Conversation

@zstack-robot-1
Copy link
Copy Markdown
Collaborator

Resolves: ZSTAC-80275

sync from gitlab !9838

Skip Ceph secret creation when cluster is not Enabled to prevent reconnect timeout.
Add 5-minute await timeout to FutureCompletion to prevent permanent blocking.

Resolves: ZSTAC-80275

Change-Id: Iaa61109103cc5b4ee9bf86485e1777456b172b56
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Review Change Stack

概览

修改CephKvmExtension.connectionReestablished方法以在密钥创建前检查集群状态,当集群非启用态时跳过操作;同时为异步完成等待添加5分钟的显式超时限制。

变更说明

集群状态保护与超时管理

层 / 文件 说明
集群状态守卫与超时修改
plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java
增加导入声明(CLogger、Utils、ClusterState、ClusterVO)以支持日志记录与集群状态查询;在connectionReestablished方法中添加守卫逻辑,通过主机的集群UUID查询ClusterVO,若集群存在但处于非启用态则记录信息日志并返回,避免执行Ceph密钥创建;将密钥创建完成等待从无限等待改为5分钟固定超时。

代码审查工作量估计

🎯 2 (简单) | ⏱️ ~10 分钟

集群若未启,Ceph无需唤,
五分钟之约,不再无尽等,
一行行守卫,守护存储心。
🐰✨


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 标题格式不符合要求,缺少冒号分隔符。应为'[scope]: '格式,但标题中'cluster'后没有冒号,且包含多余的'timeout'词汇。 将标题改为'[ceph]: skip ceph ops on non-enabled cluster'或添加冒号使其符合'[scope]: '的格式要求。
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 (3 passed)
Check name Status Explanation
Description check ✅ Passed 描述与变更集相关,提供了JIRA票号ZSTAC-80275和GitLab同步源信息,虽然简洁但与代码改动相关联。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/songtao.liu/fix/ZSTAC-80275-v2

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java (1)

82-82: ⚡ Quick win

将超时时间提取为具名常量,避免魔法值。

Line 82 直接写 5 可读性偏弱,后续调整也容易遗漏;建议提取为类级常量。

♻️ 建议修改
 public class CephKvmExtension implements KVMHostConnectExtensionPoint, HostConnectionReestablishExtensionPoint {
     private static final CLogger logger = Utils.getLogger(CephKvmExtension.class);
+    private static final long SECRET_CREATION_AWAIT_TIMEOUT_MILLIS = TimeUnit.MINUTES.toMillis(5);

@@
-        completion.await(TimeUnit.MINUTES.toMillis(5));
+        completion.await(SECRET_CREATION_AWAIT_TIMEOUT_MILLIS);

As per coding guidelines "避免使用魔法值(Magic Value):直接使用未经定义的数值或字符串应替换为枚举或常量。"

🤖 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
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java`
at line 82, Replace the magic literal 5 in the
completion.await(TimeUnit.MINUTES.toMillis(5)) call by introducing a descriptive
class-level constant (e.g., COMPLETION_AWAIT_TIMEOUT_MINUTES or TIMEOUT_MINUTES)
in CephKvmExtension and use that constant in the await call
(completion.await(TimeUnit.MINUTES.toMillis(COMPLETION_AWAIT_TIMEOUT_MINUTES)));
ensure the constant is static final and documented so future adjustments are
centralized.
🤖 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.

Nitpick comments:
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java`:
- Line 82: Replace the magic literal 5 in the
completion.await(TimeUnit.MINUTES.toMillis(5)) call by introducing a descriptive
class-level constant (e.g., COMPLETION_AWAIT_TIMEOUT_MINUTES or TIMEOUT_MINUTES)
in CephKvmExtension and use that constant in the await call
(completion.await(TimeUnit.MINUTES.toMillis(COMPLETION_AWAIT_TIMEOUT_MINUTES)));
ensure the constant is static final and documented so future adjustments are
centralized.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro

Run ID: 426ed645-091f-47b6-b49d-d76d35791157

📥 Commits

Reviewing files that changed from the base of the PR and between 57a496a and 2ababed.

📒 Files selected for processing (1)
  • plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephKvmExtension.java

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