diff --git a/docs/controller/addon-controller-root-cause-vs-fail-fast-fix-direction-guide.md b/docs/controller/addon-controller-root-cause-vs-fail-fast-fix-direction-guide.md new file mode 100644 index 0000000..da04121 --- /dev/null +++ b/docs/controller/addon-controller-root-cause-vs-fail-fast-fix-direction-guide.md @@ -0,0 +1,91 @@ +# Fix Direction: Single-Entry Fail-Fast vs Root-Cause Chain Fix + +> **Audience**: KubeBlocks controller / DP / Configure / Ops / lifecycle / syncer 开发, XP pair reviewer, maintainer +> **Status**: stable +> **Applies to**: 任何 controller bug 修复设计阶段, reviewer 决定 PR 是否合并阶段 +> **Applies to KB version**: any +> **Affected by version skew**: no + +修一个 controller bug 时, 最常见的两种方向: 一是在 "已经撞到 bug 的那个入口" 提前拦截 (fail-fast 入口校验), 二是在 "真正拥有状态机和写入语义的链路" 里加保护 (root-cause fail-closed)。两个方向都能让具体场景的 bug 消失。但只在入口拦截的 PR 没修真正的链路 — 后续任何其它入口写入同样的非法状态, bug 就会复现。这篇 guide 给出 reviewer 在 review 时的第一个判断: 这是单入口 fail-fast 还是根因 fail-closed; 以及作者在设计阶段应该选哪个方向。 + +## 先用白话理解这篇文档 + +打个比方: 一个水箱漏水, 你看见水从前门流出来, 不去补水箱本身的洞, 而是在前门外面立一道墙挡水。前门是不漏了, 但水箱后门也漏, 侧门也漏, 第二天来个新的门就又漏了。 + +修 controller bug 也一样。**只在调用入口处加一个"检查到非法就 fail"** 是堵前门; 真正应该做的是修 controller 处理状态的代码本身, 让它拒绝接受非法状态, 不管谁通过什么入口写进来。 + +## 适用场景 + +| 我在做什么 | 这篇 guide 怎么用 | +|---|---| +| 开 PR 修 controller bug | 设计阶段先问"我修在入口还是修在根因链路", 优先根因 | +| pair review controller PR | 第一个问题"这是单入口 fail-fast 还是链路 fail-closed", 不是"代码做的事和声明一致" | +| 必须做 fail-fast 当短期 workaround | PR body 必须明写"长期治本方向" + "为何当下选治标 (例: timeline / scope)" | +| reviewer 看到 fail-fast PR 没标治本方向 | 挡 PR, 要求作者补根因方案或承诺时间表 | + +## 通用方法论 + +### 硬规则 + +1. **同样的 bug, 治本 always 优于治标**。原因: 治标只覆盖当前撞到的入口; 治本覆盖所有入口 (包括未来新加的入口)。一个永久的修法胜过 N 个临时拦截。 +2. **判断根因位置**: bug 的状态最终落在哪个 controller / CR 上, 根因就在那个 controller 处理这个 CR 的链路里。修法应该在那条链路加 fail-closed。 +3. **入口 fail-fast 只能作"短期 workaround"**: 仅在 (a) 治本需要长期 refactor 且 user 已经撞到生产问题 (b) 当下需要快速缓解 — 这两个条件同时成立时考虑, 必须在 PR body 明写"长期治本方向"。 +4. **reviewer review controller bug fix PR 的第一问**: "这是修在入口处的提前拦截, 还是修在真正拥有状态语义的链路里"; 如果是前者, 要求作者证明治本需要更大 scope, 否则挡。 +5. **治标 PR 没标治本方向 → 必挡**: 因为下一个入口出现时 bug 复现, 没人记得这是 short-term workaround, 治本工作永远不做。 +6. **入口 fail-fast 和 链路 fail-closed 不互斥但优先级不同**: 同时做是 defense in depth, 但绝不能用入口 fail-fast 代替链路 fail-closed; 反之治本做完后, 入口 fail-fast 是优化用户体验 (early failure, 更快反馈), 不是 fix 本身。 + +### 决策树 + +```text +看到 controller bug 撞在某入口 (例: Ops, API, controller reconcile, webhook) + │ + ├── 问 1: 非法状态最终落在哪个 CR? 哪个 controller 拥有这个 CR 的写入和状态机? + │ │ + │ └── 答案 = root-cause controller + │ + ├── 问 2: root-cause controller 能不能在它自己的处理链路里 fail-closed? + │ ├── 能 → 治本: 在 root-cause controller 处理路径加 fail-closed, 任何入口写入非法状态都会失败 + │ └── 不能 (例: 跨多个 controller 写同一字段, 需要架构 refactor) + │ │ + │ └── 短期 fail-fast 兜底入口, 但 PR body 必须含 "长期治本方向 + ETA" + │ + └── reviewer 把"是否修在 root-cause controller 链路上"作为 review 第一问, 不是"代码能不能挡住这次现场" +``` + +### 反模式表 + +| 反模式 | 后果 | 修法 | +|---|---|---| +| PR 在入口加 fail-fast, 不修 root-cause controller | 下个入口或新加 controller 调用同样路径 → bug 复现 | review 时挡, 要求治本 | +| PR body 写"修了 Valkey 这条问题" | 没把根因暴露, maintainer 以为是 Valkey-specific | PR body 应写"controller-level bug, 当前在 Valkey/Reconfigure 入口先撞到" | +| 治标 PR 不标治本方向 | 长期没人做治本, 治标变事实标准 | PR body 必含 "long-term root cause direction" 段, 即使是 TBD | +| reviewer 只看"代码逻辑对不对" | 漏掉 scope 边界, 让治标 PR 当治本 PR 合 | review 第一问改成 "这是治标还是治本", 第二问才是 "代码逻辑对不对" | +| 双层都做 (入口 + 链路) 又把入口当主要修法 | 入口 fail-fast 抢占注意力, 链路 fail-closed scope 被忽略或推迟 | 治本必须先做, 入口 fail-fast 之后再加作 UX 优化 | + +## 与其他 doc / skill 的关系 + +- [`addon-controller-patch-identity-preservation-guide.md`](addon-controller-patch-identity-preservation-guide.md) — reviewer feedback 阶段的 patch identity 保护; 多轮 rework 时 SHA chain 保留经验 +- [`addon-controller-pr-author-motivation-and-review-channel-guide.md`](addon-controller-pr-author-motivation-and-review-channel-guide.md) — PR body 必须写"具体问题 + 现场例子 + 不修会误导什么"; 与本 guide 的"治本方向也要写"叠加 +- [`addon-controller-diagnostic-pr-scope-guide.md`](addon-controller-diagnostic-pr-scope-guide.md) — 诊断 PR 不推 main; 与本 guide 的"治本 vs 治标"是不同 scope 决策 +- [`../code-submission/addon-design-contract-review-during-xp-guide.md`](../code-submission/addon-design-contract-review-during-xp-guide.md) — XP review 8 类合同检查; 本 guide 加第 9 类: "治本 vs 治标 scope 边界" + +## 案例附录 + +PR #10254 (Lily, ComponentParameter invalid reconfigure false-success bug) 是 2026-05-19 走通这条原则的完整案例: + +| 版本 | head SHA | 修法位置 | scope 性质 | 结果 | +|---|---|---|---|---| +| v1 | `27fa72416` | Ops `reconfigure.go` 入口 `ActionStartedCondition` 早失败 | **治标**: 只挡 Reconfigure Ops 这一个入口, 其它写 `ComponentParameter.Spec.Desired` 的入口未保护 | 三轮架构 pushback, 不合 | +| v2 | `fdf83656a` | parameters 包内 typed error + Ops 仍是触发方 | scope 转移但仍治标 (Ops 触发) | 继续 pushback | +| v3 | `08a7c6482` | ComponentParameter controller 处理 `Spec.Desired` 时 `ValidateComponentParameterAssignments` → `CMergeFailedPhase` | **治本**: 任何写 Desired 入口都过这层校验; Ops 完全删除入口拦截 | review 通过 | +| v4.1 / v4.2 | `927f0afd0` / `aa6e73d53` | 修 CI test 构造问题 | test-only follow-up | review 通过 | +| v5 | `c3a11df66` | schemaValidator 真校验 (Leon review 抓出 v4 用 empty schema 是 placebo) | bug fix 暴露旧 placebo test, 需 v6 follow-up | review 通过 | +| v6 | `e3429a673` | 修 `TestSchemaValidatorWithOpenSchema` 用真实 SchemaInJSON | test follow-up, production 不变 | review 通过 | + +westonnnn 在 PR thread 给出关键 5 句 pushback (msg 092c86d6 / 593c0c8e / f69e4e29 / 0ab2f87c / 490c7a13), 核心是"按是否影响进程/可用性判断, 不按入口是否撞到 bug 判断"。Edward 14:49 (msg 6bce947b) 同步给出"修法应在拥有状态语义的链路里"的等价表述。 + +v1 到 v3 经历的 review 翻转 (LGTM → 撤回 → 重做) 是 reviewer 漏看根因层导致, 不是 author 问题。新增 review check (本 guide 硬规则 4): 第一问"治本 vs 治标", 第二问才是"代码逻辑对不对"。 + +## 一句话总结 + +修 controller bug 时, 先问"非法状态落在哪个 CR, 哪个 controller 拥有它", 把保护加到那条链路 (治本); 单入口 fail-fast 只能当 short-term workaround, PR body 必写"长期治本方向"; reviewer 第一问"这是治本还是治标", 治标 PR 没标治本方向必挡。