Skip to content

<feature>[storage]: register and take over sblk#3744

Open
zstack-robot-2 wants to merge 2 commits intozsv_5.0.0from
sync/tao.gan/ZSV-11559@@3
Open

<feature>[storage]: register and take over sblk#3744
zstack-robot-2 wants to merge 2 commits intozsv_5.0.0from
sync/tao.gan/ZSV-11559@@3

Conversation

@zstack-robot-2
Copy link
Copy Markdown
Collaborator

APIImpact

Resolves: ZSV-11559

Change-Id: I70637377776e777070676c6a6c616e74786b6667

sync from gitlab !9612

APIImpact

Resolves: ZSV-11559

Change-Id: I70637377776e777070676c6a6c616e74786b6667
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 14, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

新增三条主存储相关 REST 接口及其 SDK 支持:检查主存储一致性(GET /primary-storage/{uuid}/consistency)、接管主存储(PUT /primary-storage/{uuid}/takeover)和发现共享块组卷组(GET /primary-storage/sharedblockgroup/vgs);包含请求/响应消息、事件类、SDK Action/Result、中文文档、SourceClassMap 映射、ReconnectResult 类型与测试辅助方法,并在 PrimaryStorageBase 中加入默认“不支持”处理。

Changes

Cohort / File(s) Summary
API 消息/回复/事件
header/src/main/java/org/zstack/header/storage/primary/APICheckPrimaryStorageConsistencyMsg.java, .../APICheckPrimaryStorageConsistencyReply.java, .../APITakeoverPrimaryStorageMsg.java, .../APITakeoverPrimaryStorageEvent.java
新增检查一致性与接管主存储的请求/回复/事件类,包含 UUID 参数、响应字段(如 consistentreconnectResult)、PrimaryStorageMessage 实现与 __example__() 工厂方法。
API 中文文档
header/src/main/java/org/zstack/header/storage/primary/APICheckPrimaryStorageConsistencyMsgDoc_zh_cn.groovy, ...APICheckPrimaryStorageConsistencyReplyDoc_zh_cn.groovy, ...APITakeoverPrimaryStorageMsgDoc_zh_cn.groovy, ...APITakeoverPrimaryStorageEventDoc_zh_cn.groovy, .../ReconnectResultDoc_zh_cn.groovy
为新增端点、回复与 ReconnectResult 添加中文文档,声明路径、参数位置(path/query/body)、返回结构、字段注释与版本信息。
PrimaryStorage 基类处理
storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java
在 handleApiMessage 中新增对 APITakeoverPrimaryStorageMsgAPICheckPrimaryStorageConsistencyMsg 的分发;提供默认“不支持”的处理,实现回复 Event 或 Reply(包含错误/不支持信息)。
SDK Actions 与 Results
sdk/src/main/java/org/zstack/sdk/CheckPrimaryStorageConsistencyAction.java, .../CheckPrimaryStorageConsistencyResult.java, TakeoverPrimaryStorageAction.java, TakeoverPrimaryStorageResult.java, DiscoverSharedBlockGroupVgsAction.java, DiscoverSharedBlockGroupVgsResult.java
新增 SDK Action 类(同步/异步调用、参数声明、Rest 路由信息)与对应结果载体类,封装三个新 API 的客户端调用与返回类型。
SDK 数据模型 与 映射
sdk/src/main/java/org/zstack/sdk/SharedBlockGroupVgInfo.java, sdk/src/main/java/SourceClassMap.java
新增 SharedBlockGroupVgInfo 数据类,并在 SourceClassMap 中加入双向映射(如 ReconnectResultSharedBlockGroupVgInfo),支持源端与 SDK 类型互转。
ReconnectResult 类型
header/src/main/java/org/zstack/header/storage/primary/ReconnectResult.java, sdk/src/main/java/org/zstack/sdk/ReconnectResult.java
新增 ReconnectResult 源端与 SDK 版本,表示重连成功/失败与错误信息并提供工厂方法(源端)。
测试辅助方法
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
新增测试封装方法 checkPrimaryStorageConsistencydiscoverSharedBlockGroupVgstakeoverPrimaryStorage,支持在 apipath 模式下记录 API 路径并统一调用/错误封装。

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client/SDK
    participant API as API Server
    participant PSBase as PrimaryStorageBase
    participant Bus as MessageBus
    Client->>API: GET /primary-storage/{uuid}/consistency
    API->>PSBase: APICheckPrimaryStorageConsistencyMsg
    PSBase->>PSBase: 检查是否支持该操作(默认不支持)
    PSBase->>API: APICheckPrimaryStorageConsistencyReply (error: not supported)
    API->>Client: HTTP 200 + body (consistent + error)
Loading
sequenceDiagram
    participant Client as Client/SDK
    participant API as API Server
    participant PSBase as PrimaryStorageBase
    participant Bus as MessageBus
    Client->>API: PUT /primary-storage/{uuid}/takeover
    API->>PSBase: APITakeoverPrimaryStorageMsg
    PSBase->>PSBase: 判断是否支持接管(默认不支持)
    PSBase->>API: APITakeoverPrimaryStorageEvent (inventory + reconnectResult with error)
    API->>Client: HTTP 200 + event payload
Loading
sequenceDiagram
    participant Client as Client/SDK
    participant API as API Server
    participant PSBase as PrimaryStorageBase
    Client->>API: GET /primary-storage/sharedblockgroup/vgs?clusterUuid=...
    API->>PSBase: DiscoverSharedBlockGroupVgs request -> 转发至处理逻辑
    PSBase->>API: DiscoverSharedBlockGroupVgsResult (vgInfos map)
    API->>Client: HTTP 200 + vgInfos
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 我是小兔来报到,接口新开又热闹,
一查一致一接管,卷组发现跳脚笑,
SDK 文档与测试到位,存储小径春色俏 🌱

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed 标题清晰准确地反映了主要变更内容,涉及Sblk(Shared Block)存储的注册和接管功能。
Description check ✅ Passed 描述虽然简洁但与变更集相关,提及API影响、关联问题ZSV-11559和GitLab同步信息。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/tao.gan/ZSV-11559@@3

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

@MatheMatrix MatheMatrix force-pushed the sync/tao.gan/ZSV-11559@@3 branch 4 times, most recently from 131dc21 to ad13a93 Compare April 15, 2026 05:36
@MatheMatrix
Copy link
Copy Markdown
Owner

Comment from jin.ma:

API 评审意见(对抗式三轮评审 + 抽象层审计)

一、抽象层泄露

本 MR 新增的两个 API 本身命名良好(APICheckPrimaryStorageConsistencyMsgAPITakeoverPrimaryStorageMsg),使用的是平台级抽象,无泄露问题

APICheckPrimaryStorageConsistencyReply 的内部实现 ConsistencyCheckResult 中:

字段 问题 建议
vgFound 暴露了 LVM VG 概念 改为 sharedBlockGroupFoundstorageGroupFound

注意:如果 vgFound 仅在 Java 后端内部使用(不序列化到 API Reply),可容忍;若会透传到用户可见的返回值中,必须改名。


二、设计层面建议

经过精简派 vs 保留派三轮对抗评审,Tech Lead 最终裁决如下:

P0 — 建议修改

1. reconnectSuccess + reconnectError 字段建议移除

APITakeoverPrimaryStorageEvent 已返回 inventory,其中 PrimaryStorageInventory.status(Connected / Disconnected)足以表达 reconnect 结果。新增两个专用字段的问题:

  • 永久 API 债务:一旦发布无法回收
  • inventory.status=Disconnected 对用户的含义清晰(需要手动 reconnect)
  • reconnectError 信息价值有限——用户唯一操作就是重试 reconnect,重试时 APIReconnectPrimaryStorageMsg 自带详细错误

2. 建议将 APICheckPrimaryStorageConsistencyMsg 改为 APITakeoverPrimaryStorageMsgdryRun 参数

三轮辩论的核心结论:

  • consistent=false 是 takeover 的前置条件(不一致才需要接管),不是阻止执行的安全检查。与 APICheckVmMigrationMsg(check=true 才执行)语义相反
  • TOCTOU 风险:灾备场景 check 和 takeover 之间可能间隔数小时(审批流程),check 结果过期
  • takeover 内部已经重做 check,外部 check 结果必然被丢弃
  • dryRun=true 可在同一条代码路径完成预检,消除 TOCTOU,同时保留 UI "先看一眼"的能力

如果保留独立 Check API,建议至少确保 takeover 内部的 check 与外部 API 的 check 使用同一份逻辑,避免不一致。

P1 — 注意事项

3. API 定义在 PrimaryStorageBase 基类 — 可以保留

模板方法 + 默认返回 operr("not supported") 是 ZStack 标准做法,与 attachHookdetachHook 一致。未来其他存储类型如需接管可复用。

@MatheMatrix MatheMatrix force-pushed the sync/tao.gan/ZSV-11559@@3 branch 11 times, most recently from 3e65b86 to 3d34c2d Compare April 16, 2026 11:56
Resolves: ZSV-11559

Change-Id: I7274786b636c6d716e7773666170756e6a786d72
@MatheMatrix MatheMatrix force-pushed the sync/tao.gan/ZSV-11559@@3 branch from 3d34c2d to a561762 Compare April 16, 2026 12:27
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.

3 participants