<fix>[testlib]: update libcbd on deploying client #2956
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Resolves: ZSTAC-79057 Change-Id: I6471766d6d786871777977736e7073726e64776a
|
Note
|
| 内聚组 / 文件 | 变更摘要 |
|---|---|
ZBS存储控制器核心逻辑 plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java |
引入基于FlowChain的新工作流：添加UPDATE_HOST_DEPENDENCY_PATH常量；新增UpdateHostDependencyCmd和UpdateHostDependencyRsp命令类；将客户端部署流程改为两步链式执行（部署客户端 → 更新主机依赖），包含异步响应处理和错误传播机制；优化数据库查询调用风格 |
测试模拟器端点 testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovy |
在模拟器中注册新的UPDATE_HOST_DEPENDENCY_PATH端点处理器，返回成功响应 |
序列图
sequenceDiagram
participant Caller
participant FlowChain
participant DeployStep as 部署客户端步骤
participant DBQuery as 数据库查询
participant UpdateStep as 更新依赖步骤
participant Host as KVM主机
Caller->>FlowChain: 启动工作流
FlowChain->>DeployStep: 执行部署步骤
DeployStep->>DBQuery: 查询KVM主机
DBQuery-->>DeployStep: 返回主机信息
DeployStep->>Host: 发送DeployClientCmd
Host-->>DeployStep: 命令完成
DeployStep->>FlowChain: ✓ 成功，进行下一步
FlowChain->>UpdateStep: 执行更新依赖步骤
UpdateStep->>Host: 发送UpdateHostDependencyCmd(异步)
Host-->>UpdateStep: 异步响应
UpdateStep->>FlowChain: ✓ 成功
FlowChain-->>Caller: 工作流完成
rect rgb(220, 240, 250)
Note over DeployStep,UpdateStep: 错误处理：任何步骤失败则整个工作流失败
end
代码审查工作量估计
🎯 3 (中等复杂度) | ⏱️ ~20-25 分钟
需要重点关注的区域：
- FlowChain工作流的两步序列逻辑：需验证部署与依赖更新的执行顺序和状态传递
- 异步HTTP调用处理（KVMHostAsyncHttpCallMsg）：确保异步响应正确捕获和错误传播
- 新增命令类
UpdateHostDependencyCmd和UpdateHostDependencyRsp的序列化/反序列化兼容性 - 模拟器端点与实际控制器逻辑的一致性验证
- 数据库查询Q.New()调用方式的重构确保没有引入回归
诗歌
🐰 流程链接一和二，
客户端部署飞驰去，
主机依赖紧随后，
异步魔法轻轻舞，
错误捕捉护周全！
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | 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 |
|---|---|---|
| Title check | ✅ Passed | 标题格式正确，遵循[scope]: 格式，长度为49个字符（在72字符限制内）。 |
| Description check | ✅ Passed | 描述与变更集相关，提及了JIRA issue ZSTAC-79057、Change-Id和gitlab同步信息，说明了变更来源。 |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
sync/yaohua.wu/5.4.6
Tip
📝 Customizable high-level summaries are now available in beta!
You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
- Provide your own instructions using the
high_level_summary_instructionssetting. - Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
- Use
high_level_summary_in_walkthroughto move the summary from the description to the walkthrough section.
Example instruction:
"Divide the high-level summary into five sections:
- 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
- 📓 References — List relevant issues, discussions, documentation, or related PRs.
- 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
- 📊 Contributor Summary — Include a Markdown table showing contributions:
| Contributor | Lines Added | Lines Removed | Files Changed |- ✔️ Additional Notes — Add any extra reviewer context.
Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java (2)
217-222: 移除多余的双分号。第 217 行和第 222 行存在双分号
;;，虽然不影响运行，但应该修正。- trigger.next();; + trigger.next();- trigger.fail(errorCode);; + trigger.fail(errorCode);
1960-1963: 建议保持代码风格一致性：使用私有字段和 getter/setter。
UpdateHostDependencyCmd类使用了 public 字段，而该文件中其他所有 Cmd 类（如DeployClientCmd、CreateVolumeCmd等）都使用私有字段配合 getter/setter 方法。为保持一致性和封装性，建议采用相同模式。public static class UpdateHostDependencyCmd extends AgentCommand { - public String updatePackages; - public String zstackRepo; + private String updatePackages; + private String zstackRepo; + + public String getUpdatePackages() { + return updatePackages; + } + + public void setUpdatePackages(String updatePackages) { + this.updatePackages = updatePackages; + } + + public String getZstackRepo() { + return zstackRepo; + } + + public void setZstackRepo(String zstackRepo) { + this.zstackRepo = zstackRepo; + } }如果采用此修改，需同步更新第 234-235 行的赋值方式：
cmd.setUpdatePackages("libcbd"); cmd.setZstackRepo(AnsibleGlobalProperty.ZSTACK_REPO);
📜 Review details
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java(6 hunks)testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovy(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovyplugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名：
- API 名称必须唯一，不能重复。
- API 消息类需要继承
APIMessage；其返回类必须继承APIReply或APIEvent，并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest，并满足如下规范：
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时，使用
{variableName}格式。- HTTP 方法对应：
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档，并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名：
- 使用 UpperCamelCase 风格。
- 特殊情况：
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量：
- 使用 lowerCamelCase 风格。
常量命名：
- 全部大写，使用下划线分隔单词。
- 要求表达清楚，避免使用含糊或不准确的名称。
包名：
- 统一使用小写，使用点分隔符，每个部分应是一个具有自然语义的英文单词（参考 Spring 框架的结构）。
命名细节：
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量，防止混淆。
- 命名缩写：
- 不允许使用不必要的缩写，如：
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达：
- 避免使用布尔型参数造成含义不明确。例如：
- 对于
stopAgent(boolean ignoreError)，建议拆分为不同函数（如stopAgentIgnoreError()），或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途（例如在常量与变量名称中，将类型词放在末尾）。
- 避免使用魔法值（Magic Value）：
直接使用未经定义的数值或字符串（如 if (status == 5)）应替换为枚举或常量。
示例：
// 错误示例：魔法值
if (user.getStatus() == 5) { ... }
// 正确示例：常量或枚举
public static final int STATUS_ACTIVE = 5;
if (user.getStatus() == STATUS_ACTIVE) { ... }
// 或使用枚举
enum UserStatus { ACTIVE, INACTIVE }
注释：
- 代码应尽量做到自解释，对少于两行的说明可以直接写在代码中。
- 对于较长的注释，需要仔细校对并随代码更新，确保内容正确。
- 接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用：
- 应尽量减少 if...else 结构的使用，建议：
- 限制嵌套层级最多为两层，且内层不应再出现
else分支。- 尽早返回（Early Return），将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断：
- if 条件表达不宜过长或过于复杂，必要时可以将条件抽成 boolean 变量描述。
代码块长度：
...
Files:
plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
🧠 Learnings (6)
📚 Learning: 2025-08-25T03:55:07.988Z
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2504
File: plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java:2046-2078
Timestamp: 2025-08-25T03:55:07.988Z
Learning: The OFFLINE_SNAPSHOT_COMMIT path for NFS primary storage snapshot commit operations is properly handled in the test infrastructure via NfsPrimaryStorageSpec.groovy, which includes both simulator and VFS hook implementations for testing the offline snapshot commit functionality.
Applied to files:
testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovyplugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
📚 Learning: 2025-09-01T08:16:10.006Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2541
File: testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy:30889-30893
Timestamp: 2025-09-01T08:16:10.006Z
Learning: ApiHelper.groovy in testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy is auto-generated and should not be manually modified or receive code change suggestions.
Applied to files:
testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovy
📚 Learning: 2025-10-20T11:27:25.928Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2763
File: sdk/src/main/java/org/zstack/sdk/UpdateHostKernelInterfaceAction.java:31-31
Timestamp: 2025-10-20T11:27:25.928Z
Learning: UpdateHostKernelInterface API (`sdk/src/main/java/org/zstack/sdk/UpdateHostKernelInterfaceAction.java`) 在 PR #2763 时尚未被实际使用,因此对其参数约束的变更(如 name 字段从必需改为可选)不会造成破坏性影响。
Applied to files:
plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
📚 Learning: 2025-08-25T03:53:30.749Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2504
File: plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java:1010-1010
Timestamp: 2025-08-25T03:53:30.749Z
Learning: The OFFLINE_COMMIT_PATH ("/localstorage/snapshot/offlinecommit") for local storage snapshot commit operations is properly handled on both the ZStack management node side (in LocalStorageKvmBackend.java) and the KVM Agent side, providing complete end-to-end offline snapshot commit functionality.
Applied to files:
plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
📚 Learning: 2025-07-22T02:30:46.123Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2311
File: plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/KVMRealizeHardwareVxlanNetworkBackend.java:52-53
Timestamp: 2025-07-22T02:30:46.123Z
Learning: In ZStack, DatabaseFacade.findByUuid() calls for VO objects in contexts like KVMRealizeHardwareVxlanNetworkBackend do not require null checks because the L2NetworkInventory passed to these methods is guaranteed to have a corresponding VO record in the database by the system's design and calling patterns.
Applied to files:
plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
📚 Learning: 2025-08-04T03:35:51.225Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2360
File: compute/src/main/java/org/zstack/compute/vm/VmNicParamBuilder.java:27-28
Timestamp: 2025-08-04T03:35:51.225Z
Learning: 在 VmNicParamBuilder.buildByVmUuid 方法中，上层调用保证传入的 VM UUID 对应的实例一定存在，因此使用 Q.New(VmInstanceVO.class).eq(VmInstanceVO_.uuid, vmInstanceUuid).find() 查询后不需要进行 null 检查。
Applied to files:
plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
🧬 Code graph analysis (1)
plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java (1)
core/src/main/java/org/zstack/core/db/Q.java (1)
Q(16-193)
🔇 Additional comments (5)
plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java (4)
12-17: LGTM!新增的
AnsibleGlobalProperty和Q导入是合理的，分别用于获取 Ansible 仓库配置和简化数据库查询语法。
104-104: LGTM!新增的
UPDATE_HOST_DEPENDENCY_PATH常量命名清晰，与现有的 path 常量风格一致。
228-255: 新增的 update-host-client-dependency 流程逻辑正确。该流程正确地构建了
UpdateHostDependencyCmd命令，通过KVMHostAsyncHttpCallMsg异步发送到目标主机，并妥善处理了回调中的成功/失败情况。
420-440: LGTM!使用
Q.New替代完整包路径org.zstack.core.db.Q.New的重构使代码更简洁，与导入声明保持一致。testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovy (1)
74-79: LGTM!新增的
UPDATE_HOST_DEPENDENCY_PATH模拟器实现正确，与文件中其他简单模拟器的模式一致。它为测试环境提供了必要的端点支持，确保新增的 host dependency 更新流程能够在测试中正常运行。
Comment on lines
+202
to
+207
| public void run(FlowTrigger trigger, Map data) { | ||
| KVMHostVO host = Q.New(KVMHostVO.class).eq(KVMHostVO_.uuid, h.getUuid()).find(); | ||
| if (host == null) { | ||
| comp.fail(operr("cannot found kvm host[uuid:%s], unable to deploy client", h.getUuid())); | ||
| return; | ||
| } |
There was a problem hiding this comment.
严重问题：FlowChain 触发器未正确处理，会导致流程挂起。
当 host == null 时，代码调用了 comp.fail() 并返回，但没有调用 trigger.fail()。这会导致 FlowChain 永远无法完成，因为 FlowTrigger 既没有调用 next() 也没有调用 fail()。
应用以下修复：
public void run(FlowTrigger trigger, Map data) {
KVMHostVO host = Q.New(KVMHostVO.class).eq(KVMHostVO_.uuid, h.getUuid()).find();
if (host == null) {
- comp.fail(operr("cannot found kvm host[uuid:%s], unable to deploy client", h.getUuid()));
+ trigger.fail(operr("cannot find kvm host[uuid:%s], unable to deploy client", h.getUuid()));
return;
}注意：错误消息中 "cannot found" 应改为 "cannot find"（语法修正）。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public void run(FlowTrigger trigger, Map data) { | |
| KVMHostVO host = Q.New(KVMHostVO.class).eq(KVMHostVO_.uuid, h.getUuid()).find(); | |
| if (host == null) { | |
| comp.fail(operr("cannot found kvm host[uuid:%s], unable to deploy client", h.getUuid())); | |
| return; | |
| } | |
| public void run(FlowTrigger trigger, Map data) { | |
| KVMHostVO host = Q.New(KVMHostVO.class).eq(KVMHostVO_.uuid, h.getUuid()).find(); | |
| if (host == null) { | |
| trigger.fail(operr("cannot find kvm host[uuid:%s], unable to deploy client", h.getUuid())); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
around lines 202 to 207, the FlowChain trigger isn't notified when host==null
which can hang the flow; replace the current comp.fail(...) + return behavior
with notifying the trigger and the completion: create an error (operr) with
corrected text "cannot find kvm host[uuid:%s], unable to deploy client", call
trigger.fail(thatError) to unblock the FlowChain, then call
comp.fail(theSameError) and return.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: ZSTAC-79057
Change-Id: I6471766d6d786871777977736e7073726e64776a
sync from gitlab !8756