Repository navigation
Conversation
1d6ada8 to
1ad001a
Compare
PIKACHUIM
left a comment
There was a problem hiding this comment.
🙏 感谢贡献 @suantea 提交!
🤖 AI 自动审核声明:本评审报告由 AI 自动生成,当前使用 Claude Opus 5 模型进行分析。
🎯 结论
✅ Approve — 自愈守护骨架清晰,仅 2 处 P2 可优化
📖 概要
新增 autoheal 包,周期性探活(30s)frp_client/cftunnel/portforward,未运行则触发 RestartFn 自动重启,证书 ≤7 天剩余发送 callback 告警。
🧭 整体方案
探活回调(ProbeFunc)+ 重启回调(RestartFn)依赖注入设计,解耦具体服务实现;周期 ticker + stopCh 优雅停止;证书预警独立于进程探活。方案清晰可扩展。
📊 变更统计
4 文件(+305 / -34)| 功能 ⭐⭐⭐⭐ | 最小改动 ⭐⭐⭐⭐ | 前向兼容 ⭐⭐⭐⭐⭐ | 方案设计 ⭐⭐⭐⭐
🚨 关键问题
无重大问题。
⚠️ 次要问题(P2,不阻塞)
RestartMaxRetries定义未使用:Config声明了RestartMaxRetries,但probeOne中没有任何重试上限逻辑,重启失败仅写HEAL_FAIL审计。若服务持续崩溃会每 30s 无限重试。建议实现「失败 N 次后暂停告警」或明确移除该字段。RestartFn为 nil 时误报HEAL_OK:probeOne中即使RestartFn为 nil(未注入),仍会走到m.logAudit(..., "HEAL_OK", "已自动重启"),实际未执行任何重启。建议在RestartFn == nil时直接返回,不写 HEAL_OK。
⚠️ 边缘提醒(P2)
Stop()中close(m.stopCh),若Start() → Stop() → Start()重复调用,第二次Start复用已关闭的stopCh可能导致 panic。当前调用方(main.go)通常只启停一次,风险低,但建议防御性重建 channel。
✅ 亮点
- 依赖注入清晰,探活逻辑与重启逻辑解耦。
- 证书过期/临期分级告警(expired / expiring_in_N_days)。
- 失败/成功均写审计日志,可追踪。
🎯 结论:✅ Approve — 建议合并(最终由维护者判定)。
pikachuren
left a comment
There was a problem hiding this comment.
🙏 感谢 @suantea 提交!
🤖 AI 自动审核声明:本评审报告由 AI 自动生成,当前使用 Claude Opus 5 模型进行分析。
🎯 结论
🔄 Request Changes — 自愈守护的整体骨架合理,但存在「重启次数上限未生效」「证书告警每 30 秒重复轰炸」「Start/Stop 生命周期缺陷」3 个 P1,建议修后再合并
📖 概要
feat(autoheal): 系统自愈守护(PR-3 of #59) · 关联 #59 · 周期性探测子进程存活并自动重启,附带证书到期预警
核心改动:新增 service/autoheal(Manager + BuildProbeFunc),每 30s 探测 frp_client / cftunnel / portforward 的启用项,未运行则调用 RestartFn 重启;同时扫描证书有效期并在 7 天内告警,写 syslog 并触发 callback。main.go 完成接线与优雅关闭。
🧭 整体方案
用 ticker + 探针函数 + 注入式重启回调的方式实现通用自愈守护,ProbeFunc/RestartFn 通过依赖注入解耦具体服务,设计上是清晰的。两个需要收敛的点:一是「告警去重」与「重启退避/上限」这两个自愈场景最关键的保护都缺失或未生效;二是本 PR 夹带了两个与自愈无关的提交(CI pr-checks 重构、selector 取消确定性修复),且与 #91 完全重复。
📊 变更统计
4 个文件(+305 / −34 行) | 功能 ⭐⭐⭐⭐ | 最小改动 ⭐⭐⭐⭐ | 前向兼容 ⭐⭐⭐⭐ | 方案设计 ⭐⭐⭐
🚨 关键问题
P0(阻塞合并):无
P1(建议修复):
⚠️ backend/service/autoheal/heal.go:31,74,141-146证书告警重复轰炸:Config.CertWarnDays配置了 7 天,checkCertWarnings()每 30 秒遍历全部DomainCert,对每个临期证书无条件调用alertCert()。一张 7 天后到期的证书,在 7 天内会写约 2 万条 syslog 并触发约 2 万次 callback。建议加去重:按certID + reason记录上次告警时间(例如每天最多一次),或在 reason 里去掉具体天数后按天粒度去重。⚠️ backend/service/autoheal/heal.go:31,141-146RestartMaxRetries定义了但从未使用:probeOne()直接调用m.cfg.RestartFn,既没有重试计数也没有退避。若某个服务持续崩溃(例如配置错误),会每 30 秒无条件重启一次,日志与 callback 同样会被刷爆。建议接入RestartMaxRetries(达到上限后停手并升级为一次告警),并加上退避。⚠️ backend/service/autoheal/heal.go:39,64-72Start()/Stop()生命周期不完整:Stop()里close(m.stopCh)后stopCh永久处于已关闭状态;若之后再次Start(),run()会立刻从<-m.stopCh返回,守护静默失效(ticker已新建但无人消费)。当前startServer()只走一次路径所以暂时安全,但 service 模式下重复调用就会踩到。建议在Start()中重建stopCh(或用context.Context+sync.Once)。⚠️ gofmt 不合规(已用gofmt -l核对):backend/service/autoheal/heal.go、backend/main.go。#109正在给 CI 加gofmt门禁,届时会被拦下,建议先跑gofmt -w。
P2(可选):
- 💡
backend/service/autoheal/heal.go:175-206BuildProbeFunc里 3 处db.Where("enable = ?", true).Find(&xxx)忽略 Find 的 error。DB 出错时会返回空集合,表现为「没有任何服务需要探活」,自愈静默失效。建议至少记录 error。 - 💡
backend/service/autoheal/heal.go:167-174alertCert触发 callback 时只传了Type: "cert_" + reason,没有带证书名/域名/到期时间等上下文;且 reason 里含天数(expiring_in_3_days)意味着每天都会产生一个新的时间类型,回调规则很难配置(需要每天新增一条规则)。建议 reason 收敛为固定枚举(expiring/expired),把天数放进 payload。 - 💡
backend/service/autoheal/heal.go:176-206BuildProbeFunc每次 tick 都全表查询三张配置表。若配置项不多,可接受;建议在 PR 描述里说明预期规模,或后续改为只查启用项 + 加索引说明。 - 💡
backend/service/autoheal/heal.go:186-196判断 cftunnel 运行状态依赖GetStatus()返回 map 里的"running"布尔键,属于弱契约(键名拼写错了会静默判为「未运行」并触发无意义重启)。建议改为返回明确的结构体或枚举。 - 💡 本 PR 夹带的
ci: fix PR checks ...与fix(selector): ProbeLines 取消确定性两个提交与 #91 完全重复,建议提取为独立 PR。
📂 逐文件分析
backend/service/autoheal/heal.go
改动意图:自愈守护主体 —— 周期探活 + 重启 + 证书预警
问题分析:RestartMaxRetries 未使用(P1)、Start/Stop 生命周期缺陷(P1)、忽略 DB error(P2)、callback 事件类型发散(P2)
详细建议:
// 告警去重(示例:同证书同原因每天最多一次)
type alertKey struct{ id uint; reason string }
if last, ok := m.lastAlert[key]; !ok || time.Since(last) > 24*time.Hour {
m.alertCert(reason, c.Name, c.Domains)
m.lastAlert[key] = time.Now()
}Start() 中改为 m.stopCh = make(chan struct{});probeOne 里接入重试计数与退避。
backend/main.go
改动意图:接线 autohealMgr,加入启动序列与 registerStopHandlers
问题分析:gofmt)不合规;接线本身正确,autohealMgr.Stop() 已加入关闭链。
详细建议:gofmt -w。
其余文件:backend/service/selector/selector.go 的 ProbeLines ctx 取消前置检查实现正确(消除了 select 在两条通道均就绪时随机选择导致跳过取消分支的竞态);.github/workflows/pr-checks.yml 的 artifact 传递修复方向正确,但两者都与自愈主题无关,建议拆分。
✅ 待处理清单
- [P1] 证书告警去重(避免 7 天 2 万条)
- [P1] 接入
RestartMaxRetries+ 退避 - [P1]
Start()重建stopCh,修复重复启停缺陷 - [P1]
heal.go/main.go执行gofmt -w - [P2]
BuildProbeFunc检查并记录 Find 的 error - [P2] callback 事件类型收敛为固定枚举,天数放 payload
- [P2] 拆分夹带的 CI / selector 提交
🎯 结论:🔄 Request Changes — 骨架与依赖注入设计合理;但「告警去重」和「重启上限」是自愈功能最关键的两个保护且当前都缺失/未生效,建议补齐后再合并
新增 backend/service/autoheal 模块实现系统级自愈守护: - 周期性探活(frp_client/cftunnel/portforward):探测进程存活状态 - 自动重启:单进程失败后触发 RestartFn,失败写入审计日志 - 证书预警:扫描 DomainCert 表,≤7 天剩余发送 callback 告警 - 与 syslog/callback 集成:自愈动作记录审计条目并通过回调通道推送 - main.go 接入:初始化 autohealMgr 并在 Start/Stop 生命周期注册 验证:go build ./... 通过。
582238c to
a28262c
Compare
pikachuren
left a comment
There was a problem hiding this comment.
🙏 感谢 @suantea 提交!
🤖 AI 自动审核声明。
🎯 结论
🔄 Request Changes — P1 问题需修复(重启无退避+健康检查失效)
📖 概要
系统自愈守护进程 · PR #59-3
核心改动:AutoHeal 引擎、health 端点检查、自动重启
🚨 关键问题
P1:
- 💡
autoheal/engine.goattemptRestart只有 5s 固定延迟,无退避策略,服务必现崩溃时会以最快速度死循环重启耗尽 CPU。 - 💡 健康检查用
GET /system/health,该端点未在 PR #117 里实现,会 404 导致误判。
P2:重启动作无鉴权(engine 内部调用 mgr.Start,未经中间件校验)。
由 AI 助手整理(issue #59 PR-3)。
背景
当前 NetPanel 的 Manager.StartAll() 只在启动时拉起服务,运行时崩溃需手动恢复。
改动
验证:go build ./... 通过。