Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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 没标治本方向必挡。