feat(utidb): 接入 UTiDB CLI#114
Conversation
Episkey-G
left a comment
Episkey-G
left a comment
There was a problem hiding this comment.
接入规范评审
整体骨架规范,平台契约(import 白名单 / 鉴权工厂 cli.NewServiceClient / 输出通道 EmitResult+ProgressWriter+PrintList / golden / 命名 --utidb-id)用得很干净。但当前不可直接合并,请先处理 1 个阻断项 + 若干建议。
🔴 阻断项:产品未注册
本 PR 含 products/utidb/product.yaml 却未提交再生成后的 cmd/products.gen.go。命令挂载与 golden 校验都走 registeredProducts()(cmd/snapshot_export.go:43),该列表由 hack/gen-products 从 products/*/product.yaml 生成。后果:①组合构建后 ucloud utidb 不存在;②TestProductGoldens/utidb 不会执行,本 PR 的 golden 是孤儿、CI 也不会报红。请按规范 §2.7/§3 跑 go run ./hack/gen-products 并把更新后的 cmd/products.gen.go 提进本 PR。
🟠 需澄清 / 改进
- create/delete/scale-node/resize-disk/modify-spec/list-backup 绕过 typed request 改用
GenericInvoke+ 手搓map[string]interface{},偏离规范 §5「请求一律用client.NewXxxRequest()逐字段赋值」。请在描述里说明原因(SDK 对嵌套NodeConfig/Labels/SecGroupInfo序列化有缺陷?)。 client.go是 grab-bag 文件,违反 §2.2 意图(仅因client不在 check-product 规则 10 禁名单里才过闸)。- 长耗时写操作缺
--async(§5.6/§7)。
🟡 需对真实 API 核验
- 轮询终态串大半未实测(
Available/Running/CreateFailed/UpgradeFailed/Deleted/DeleteFailed);若失败态串不符,一次快速失败会拖成 20–30 分钟超时。 NodeCount对所有节点类型强制 ≥4,可能拦掉 pd=3 / tiflash=0|2 的标准拓扑。
说明:审阅者本地 go.mod 锁定 go1.25 工具链 + SDK
v0.22.89(离线环境缺失),未能本地go build/check-product,构建期正确性以 CI 为准;以上均为静态 + 规范分析结论。详见逐行评论。
| // invokeAPI calls a TiDB API action via GenericRequest. | ||
| // String arrays like NodeTypes use flat keys (NodeTypes0). Nested objects like NodeConfig | ||
| // should be passed as maps/slices in params so the SDK encoder emits NodeConfig.0.Field. | ||
| func invokeAPI(ctx *cli.Context, action string, params map[string]interface{}) (map[string]interface{}, error) { |
There was a problem hiding this comment.
🟠 grab-bag 文件,违反 §2.2。 本文件混装了 invokeAPI/mergeCommonParams/format*/*ToMap/parse*FromPayload/extractServerIDs/stringVal/intVal 等互不相关的横切工具。它能过 check-product 规则 10 仅因 client 不在禁名单(helpers/utils/util/common/misc),属绕过机器闸。建议按 concern 拆分(payload 编解码 / 类型转换 / 响应解析分文件)。
| } | ||
| params["NodeConfig"] = nodeConfigMaps | ||
|
|
||
| payload, err := invokeAPI(ctx, "CreateTiDBClusterService", params) |
There was a problem hiding this comment.
🟠 绕过 typed request(§5)。 这里已 NewCreateTiDBClusterServiceRequest() 却只用它绑公共 flag,真正下发走 generic map + 手搓 key/类型。手写字符串 key、手动 float64/int 解析很脆弱,拼错只在运行期暴露。请在 PR 描述说明为何不用 typed(SDK 嵌套参数序列化缺陷?),能 typed 就 typed。同款问题见 delete/scale_node/resize_disk/modify_spec/list_backup。
| flags.IntSliceVar(&alertStrategyIDs, "alert-strategy-ids", nil, "Optional. Alert strategy IDs") | ||
| flags.StringSliceVar(&labels, "labels", nil, "Optional. Resource labels, format: key=value, repeatable") | ||
| flags.StringArrayVar(&secGroupInfo, "sec-group-info", nil, "Optional. Security group info, format: SecGroupId=xxx,Priority=N, repeatable") | ||
| flags.BoolVar(&async, "async", false, "Optional. Do not wait for creation to finish") |
There was a problem hiding this comment.
🟡 只有 create 有 --async。scale-node/resize-disk/modify-spec(轮询超时 30 min)与 delete 都强制同步等待、无 --async,违反 §7「长耗时操作加 --async」。请补齐。
|
|
||
| // minNodeCountPerType is the minimum NodeCount per node type enforced by the TiDB API (> 3). | ||
| const minNodeCountPerType = 4 | ||
|
|
There was a problem hiding this comment.
🟡 可能拦掉合法拓扑。 对 tidb/tikv/pd/tiflash 一视同仁要求 >3。但标准 TiDB 里 pd 常为 3、tiflash 可为 0/2。若托管 API 并不对 pd/tiflash 强制 >3,用户将无法创建标准集群(pd=3 直接被拒)。请逐节点类型核对 API 真实下限,按类型区分校验。
| const ( | ||
| stateAvailable = "Available" | ||
| stateRunning = "Running" | ||
| stateStopped = "Stopped" |
There was a problem hiding this comment.
🔵 未用常量:stateStopped/stateFailed/stateCreating/stateDeleting/stateBackupFail/stateUpgrading/stateAltering/stateDeploying 多数无引用,请精简为实际用到的。
| BackupEndTime int | ||
| } | ||
|
|
||
| func newBackupRowFromData(d tidb.BackupData) backupRow { |
There was a problem hiding this comment.
🔵 死代码。 newBackupRowFromData 无任何调用点(list-backup 走 parseBackupRowsFromPayload 直接构造 backupRow)。删除,或让 list-backup 改用它以复用类型映射。
|
|
||
| cmd.MarkFlagRequired("utidb-id") | ||
| command.SetCompletion(cmd, "utidb-id", func() []string { | ||
| return listResourceIDs(ctx, nil, *req.Region, *req.Zone, *req.ProjectId) |
There was a problem hiding this comment.
🔵 脆弱且不一致的解引用。 用 *req.Region 裸解引用,而 backup.go/list_backup.go 用 nil 安全的 req.GetRegion()。当前不会 panic(Bind* 经 SetRegionRef(®ion) 在 SetCompletion 前把字段置非 nil,已核 pkg/command/bind.go),但一旦调整调用顺序即崩。请统一改 req.GetRegion()/GetZone()/GetProjectId()。同款见 describe.go:62、modify_spec.go:101、resize_disk.go:110、scale_node.go:124。
|
|
||
| _, err = invokeAPI(ctx, "ModifyTiDBClusterNode", params) | ||
| if err != nil { | ||
| handleAPIError(ctx, err) |
There was a problem hiding this comment.
🔵 错误处理不一致:backup/scale-node 用 handleAPIError(补 RetCode 202555 提示),create/delete/resize/modify/list-backup 用 ctx.HandleError。建议统一一条路径。
| ctx.BindRegion(cmd, req) | ||
| ctx.BindZone(cmd, req) | ||
| ctx.BindProjectID(cmd, req) | ||
| flags.StringVar(&limit, "limit", "", "Optional. The maximum number of resources per page") |
There was a problem hiding this comment.
🔵 list 用逐字段 Bind* + 手绑 string 型 limit/offset,而 list-backup 用 int 型塞 generic map,两处 limit/offset 类型/绑定不一致。§8 首选 ctx.BindCommonParams(cmd, req)(自动带 limit/offset + 补全)。建议 list 改用聚合 binder。
3 participants
Features:
Fixes: