Bump sigs.k8s.io/controller-runtime from 0.21.0 to 0.22.0 #39
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![dependabot[bot]](https://avatars.githubusercontent.com/in/29110?s=60&v=4){kind=small}
dependabot
bot
added
dependencies
|
|
升级 controller-runtime 至 v0.22.0 并更新相关依赖变更概述
变更文件
时序图sequenceDiagram
participant User
participant Watcher
participant Backend
participant OS
User->>Watcher: Add(path)
Watcher->>Backend: AddWith(path, opts)
Backend->>OS: Register watch (e.g., inotify_add_watch)
OS-->>Backend: Watch descriptor
Backend->>Watcher: Return
OS->>Backend: Event (e.g., IN_CREATE)
Backend->>Watcher: Send Event
Watcher->>User: Events channel
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
|
![lingma-agents[bot]](https://avatars.githubusercontent.com/in/1198481?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔎 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题，需立即修复。例如：系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 1 | 严重问题，高优先级修复。例如：核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 4 | 主要问题，建议修复。例如：非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 1 | 次要问题，酬情优化。例如：代码格式不规范或注释缺失。 |
总计: 6 个问题
📋 评审意见详情
💡 代码实现建议
以下是文件级别的代码建议，聚焦于代码的可读性、可维护性和潜在问题。
🔹 vendor/github.com/fsnotify/fsnotify/backend_fen.go (2 💬)
- 应在 `handleEvent` 中统一处理所有事件发送失败的情况，以避免事件丢失。 (L240-L246)
- 应正确处理目录更新期间的错误，以防止在处理不存在的文件时中断事件循环。 (L374-L381)
🔹 vendor/github.com/fsnotify/fsnotify/backend_inotify.go (2 💬)
- 应检查 `inotify_add_watch` 返回的错误并适当处理。 (L264-L278)
- 应在发送事件失败时记录日志或采取其他措施。 (L396-L518)
🔹 vendor/github.com/fsnotify/fsnotify/backend_kqueue.go (2 💬)
- 应确保在关闭 kqueue 时正确处理所有资源释放。 (L243-L308)
- 应在读取 kevent 事件时增加超时机制以避免无限等待。 (L688-L705)
🚀 架构设计建议
以下是对代码架构和设计的综合分析，聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍1. 依赖升级引入的潜在兼容性问题
本次PR将 sigs.k8s.io/controller-runtime 从 v0.21.0 升级到 v0.22.0，并随之引入了多个 Kubernetes 相关依赖的更新（如 k8s.io/api, k8s.io/apimachinery 等升级至 v0.34.0）。这类大版本升级可能引入不兼容的API变更或行为变更，尤其是在 controller-runtime 这种核心库上。建议进行充分的集成测试以确保与现有代码的兼容性。
可能导致运行时错误、控制器行为异常或与Kubernetes集群交互失败。
🔍2. 新增依赖库的潜在风险
升级后引入了新的依赖库，如 github.com/emicklei/go-restful/v3 和 github.com/fsnotify/fsnotify。尤其是 fsnotify 的更新日志显示修复了多个 race condition 和平台特定问题。虽然修复是积极的，但新版本的行为变化可能影响依赖它的模块。建议确认这些新依赖是否与项目现有逻辑存在潜在冲突。
📌 关键代码
可能引入未预见的运行时错误、竞态条件或平台兼容性问题。
🔍3. fsnotify 库在多个平台上的行为变更
fsnotify 库在 v1.9.0 和 v1.8.0 版本中进行了多项修复和变更，包括对 inotify 和 kqueue 后端的行为调整。这些变更可能影响文件监听的准确性或性能，特别是在高并发或复杂文件系统操作场景下。建议评估这些变更对当前文件监听逻辑的影响。
📌 关键代码
可能导致文件事件丢失、重复或延迟，影响依赖文件监听功能的模块。
🔍4. go-restful 库的行为变更
go-restful 库在 v3.12.x 版本中引入了对空请求体的处理变更和路由匹配修复。这些变更可能影响 REST API 的行为，特别是对空 POST/PUT 请求的处理。建议确认这些变更不会破坏现有的API端点行为。
📌 关键代码
可能导致API端点对特定请求的处理异常，影响客户端交互。
审查详情
📒 文件清单 （100 个文件）
✅ 新增: 27 个文件
❌ 删除: 5 个文件
🔄 重命名: 19 个文件
📝 变更: 49 个文件
✅ 新增文件:
vendor/github.com/fsnotify/fsnotify/internal/darwin.govendor/github.com/fsnotify/fsnotify/internal/debug_darwin.govendor/github.com/fsnotify/fsnotify/internal/debug_dragonfly.govendor/github.com/fsnotify/fsnotify/internal/debug_freebsd.govendor/github.com/fsnotify/fsnotify/internal/debug_kqueue.govendor/github.com/fsnotify/fsnotify/internal/debug_linux.govendor/github.com/fsnotify/fsnotify/internal/debug_netbsd.govendor/github.com/fsnotify/fsnotify/internal/debug_openbsd.govendor/github.com/fsnotify/fsnotify/internal/debug_solaris.govendor/github.com/fsnotify/fsnotify/internal/debug_windows.govendor/github.com/fsnotify/fsnotify/internal/freebsd.govendor/github.com/fsnotify/fsnotify/internal/internal.govendor/github.com/fsnotify/fsnotify/internal/unix.govendor/github.com/fsnotify/fsnotify/internal/unix2.govendor/github.com/fsnotify/fsnotify/internal/windows.govendor/github.com/fsnotify/fsnotify/shared.govendor/github.com/fsnotify/fsnotify/staticcheck.confvendor/github.com/fxamacker/cbor/v2/omitzero_go124.govendor/github.com/fxamacker/cbor/v2/omitzero_pre_go124.govendor/github.com/spf13/pflag/.golangci.yamlvendor/github.com/spf13/pflag/ipnet_slice.govendor/go.yaml.in/yaml/v2/.travis.ymlvendor/go.yaml.in/yaml/v3/LICENSEvendor/go.yaml.in/yaml/v3/README.mdvendor/go.yaml.in/yaml/v3/apic.govendor/go.yaml.in/yaml/v3/decode.govendor/go.yaml.in/yaml/v3/emitterc.go
❌ 删除文件:
vendor/github.com/emicklei/go-restful/v3/json.govendor/github.com/emicklei/go-restful/v3/jsoniter.govendor/github.com/fsnotify/fsnotify/.gitattributesvendor/github.com/fsnotify/fsnotify/mkdoc.zshvendor/github.com/fxamacker/cbor/v2/encode_map_go117.go
🔄 重命名文件:
vendor/github.com/fsnotify/fsnotify/.editorconfig→vendor/github.com/spf13/pflag/.editorconfigvendor/sigs.k8s.io/structured-merge-diff/v4/LICENSE→vendor/go.yaml.in/yaml/v2/LICENSEvendor/sigs.k8s.io/yaml/goyaml.v2/LICENSE.libyaml→vendor/go.yaml.in/yaml/v2/LICENSE.libyamlvendor/sigs.k8s.io/yaml/goyaml.v2/NOTICE→vendor/go.yaml.in/yaml/v2/NOTICEvendor/sigs.k8s.io/yaml/goyaml.v2/README.md→vendor/go.yaml.in/yaml/v2/README.mdvendor/sigs.k8s.io/yaml/goyaml.v2/apic.go→vendor/go.yaml.in/yaml/v2/apic.govendor/sigs.k8s.io/yaml/goyaml.v2/decode.go→vendor/go.yaml.in/yaml/v2/decode.govendor/sigs.k8s.io/yaml/goyaml.v2/emitterc.go→vendor/go.yaml.in/yaml/v2/emitterc.govendor/sigs.k8s.io/yaml/goyaml.v2/encode.go→vendor/go.yaml.in/yaml/v2/encode.govendor/sigs.k8s.io/yaml/goyaml.v2/parserc.go→vendor/go.yaml.in/yaml/v2/parserc.govendor/sigs.k8s.io/yaml/goyaml.v2/readerc.go→vendor/go.yaml.in/yaml/v2/readerc.govendor/sigs.k8s.io/yaml/goyaml.v2/resolve.go→vendor/go.yaml.in/yaml/v2/resolve.govendor/sigs.k8s.io/yaml/goyaml.v2/scannerc.go→vendor/go.yaml.in/yaml/v2/scannerc.govendor/sigs.k8s.io/yaml/goyaml.v2/sorter.go→vendor/go.yaml.in/yaml/v2/sorter.govendor/sigs.k8s.io/yaml/goyaml.v2/writerc.go→vendor/go.yaml.in/yaml/v2/writerc.govendor/sigs.k8s.io/yaml/goyaml.v2/yaml.go→vendor/go.yaml.in/yaml/v2/yaml.govendor/sigs.k8s.io/yaml/goyaml.v2/yamlh.go→vendor/go.yaml.in/yaml/v2/yamlh.govendor/sigs.k8s.io/yaml/goyaml.v2/yamlprivateh.go→vendor/go.yaml.in/yaml/v2/yamlprivateh.govendor/k8s.io/client-go/listers/networking/v1alpha1/expansion_generated.go→vendor/go.yaml.in/yaml/v3/NOTICE
📝 变更文件:
go.modgo.sumvendor/github.com/emicklei/go-restful/v3/CHANGES.mdvendor/github.com/emicklei/go-restful/v3/README.mdvendor/github.com/emicklei/go-restful/v3/compress.govendor/github.com/emicklei/go-restful/v3/curly.govendor/github.com/emicklei/go-restful/v3/entity_accessors.govendor/github.com/emicklei/go-restful/v3/jsr311.govendor/github.com/emicklei/go-restful/v3/route.govendor/github.com/fsnotify/fsnotify/.cirrus.ymlvendor/github.com/fsnotify/fsnotify/.gitignorevendor/github.com/fsnotify/fsnotify/CHANGELOG.mdvendor/github.com/fsnotify/fsnotify/CONTRIBUTING.mdvendor/github.com/fsnotify/fsnotify/README.mdvendor/github.com/fsnotify/fsnotify/backend_fen.govendor/github.com/fsnotify/fsnotify/backend_inotify.govendor/github.com/fsnotify/fsnotify/backend_kqueue.govendor/github.com/fsnotify/fsnotify/backend_other.govendor/github.com/fsnotify/fsnotify/backend_windows.govendor/github.com/fsnotify/fsnotify/fsnotify.govendor/github.com/fsnotify/fsnotify/system_bsd.govendor/github.com/fsnotify/fsnotify/system_darwin.govendor/github.com/fxamacker/cbor/v2/README.mdvendor/github.com/fxamacker/cbor/v2/bytestring.govendor/github.com/fxamacker/cbor/v2/cache.govendor/github.com/fxamacker/cbor/v2/common.govendor/github.com/fxamacker/cbor/v2/decode.govendor/github.com/fxamacker/cbor/v2/doc.govendor/github.com/fxamacker/cbor/v2/encode.govendor/github.com/fxamacker/cbor/v2/encode_map.govendor/github.com/fxamacker/cbor/v2/simplevalue.govendor/github.com/fxamacker/cbor/v2/stream.govendor/github.com/fxamacker/cbor/v2/structfields.govendor/github.com/fxamacker/cbor/v2/tag.govendor/github.com/google/gnostic-models/compiler/context.govendor/github.com/google/gnostic-models/compiler/extensions.govendor/github.com/google/gnostic-models/compiler/helpers.govendor/github.com/google/gnostic-models/compiler/reader.govendor/github.com/google/gnostic-models/jsonschema/models.govendor/github.com/google/gnostic-models/jsonschema/reader.govendor/github.com/google/gnostic-models/jsonschema/writer.govendor/github.com/google/gnostic-models/openapiv2/OpenAPIv2.govendor/github.com/google/gnostic-models/openapiv2/document.govendor/github.com/google/gnostic-models/openapiv3/OpenAPIv3.govendor/github.com/google/gnostic-models/openapiv3/document.govendor/github.com/modern-go/reflect2/safe_type.govendor/github.com/spf13/pflag/flag.govendor/github.com/spf13/pflag/ip.govendor/github.com/spf13/pflag/string_array.go
💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论，lingma-agents 将自动处理您的请求。例如：
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案，并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如：
-
@lingma-agents 分析这个方法的性能瓶颈并提供优化建议。
-
@lingma-agents 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如：
-
@lingma-agents 请总结上述讨论并提出解决方案。
-
@lingma-agents 请根据讨论内容生成优化代码。
Comment on lines
+240
to
+246
| if !w.sendEvent(Event{Name: path, Op: Remove}) { | ||
| return nil | ||
| } | ||
| reRegister = false | ||
| } | ||
| if events&unix.FILE_RENAME_FROM != 0 { | ||
| if !w.sendEvent(path, Rename) { | ||
| if !w.sendEvent(Event{Name: path, Op: Rename}) { |
There was a problem hiding this comment.
应在 handleEvent 中统一处理所有事件发送失败的情况，以避免事件丢失。
🟡 Major | 🐞 Bugs
📋 问题详情
在 handleEvent 函数中，多个地方调用了 w.sendEvent 并在失败时直接返回 nil，这可能导致事件未被正确处理。应统一处理事件发送失败的情况，确保事件不会因发送失败而丢失，同时避免中断事件循环。
💡 解决方案
建议在事件发送失败时记录错误或采取其他恢复措施，而不是直接返回 nil。可以通过引入重试机制或记录日志来增强事件处理的健壮性。
- if !w.sendEvent(Event{Name: path, Op: Remove}) {\n return nil\n}
+ if !w.sendEvent(Event{Name: path, Op: Remove}) {\n // TODO: Add logging or retry mechanism for event loss prevention\n return nil\n}您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
Comment on lines
+374
to
+381
| if errors.Is(err, fs.ErrNotExist) { | ||
| // File may have disappeared between getting the dir listing and | ||
| // adding the port: that's okay to ignore. | ||
| continue | ||
| } | ||
| if !w.sendEvent(path, Create) { | ||
| if !w.sendError(err) { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
应正确处理目录更新期间的错误，以防止在处理不存在的文件时中断事件循环。
🟡 Major | 🐞 Bugs
📋 问题详情
在 updateDirectory 函数中，当尝试关联新文件时，如果文件在目录列表获取后被删除，则可能会出现 fs.ErrNotExist 错误。虽然当前代码忽略了该错误，但未处理的其他错误可能会导致事件循环中断，影响文件监控的稳定性。应确保所有非预期错误都被正确记录或处理，而不是直接终止。
💡 解决方案
建议将错误处理逻辑调整为明确区分可忽略的错误和需要上报的错误。对于非 fs.ErrNotExist 的错误，应当记录或上报，而不是直接返回 nil 导致事件循环终止。
- err = w.associateFile(path, finfo, false)
- if errors.Is(err, fs.ErrNotExist) {
- // File may have disappeared between getting the dir listing and
- // adding the port: that's okay to ignore.
- continue
- }
- if !w.sendError(err) {
- return nil
- }
+ err = w.associateFile(path, finfo, false)
+ if err != nil {
+ if !errors.Is(err, fs.ErrNotExist) {
+ // File may have disappeared between getting the dir listing and
+ // adding the port: that's okay to ignore.
+ if !w.sendError(err) {
+ return nil
+ }
+ }
+ continue
+ }您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
Comment on lines
+264
to
+278
| wd, err := unix.InotifyAddWatch(w.fd, path, flags) | ||
| if wd == -1 { | ||
| return nil, err | ||
| } | ||
|
|
||
| if e, ok := w.watches.wd[uint32(wd)]; ok { | ||
| return e, nil | ||
| } | ||
|
|
||
| if existing == nil { | ||
| return &watch{ | ||
| wd: uint32(wd), | ||
| path: name, | ||
| flags: flags, | ||
| wd: uint32(wd), | ||
| path: path, | ||
| flags: flags, | ||
| recurse: recurse, |
There was a problem hiding this comment.
应检查 inotify_add_watch 返回的错误并适当处理。
🟡 Major | 🐞 Bugs
📋 问题详情
在 register 方法中调用 unix.InotifyAddWatch 时，仅检查了返回值是否为 -1，但未充分处理可能的错误情况，如权限不足或路径不存在等。应该更详细地处理这些错误。
💡 解决方案
增强错误处理逻辑，针对特定错误类型提供更明确的信息。
+ import \"errors\"
- if wd == -1 {
- return nil, err
- }
+ if wd == -1 {
+ if errors.Is(err, unix.EACCES) || errors.Is(err, unix.ENOENT) {
+ return nil, fmt.Errorf(\"failed to add watch for %s: %w\", path, err)
+ }
+ return nil, err
+ }您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
Comment on lines
+396
to
+518
| if !w.sendEvent(ev) { | ||
| return | ||
| } | ||
|
|
||
| var name string | ||
| if watch != nil { | ||
| name = watch.path | ||
| } | ||
| if nameLen > 0 { | ||
| // Point "bytes" at the first byte of the filename | ||
| bytes := (*[unix.PathMax]byte)(unsafe.Pointer(&buf[offset+unix.SizeofInotifyEvent]))[:nameLen:nameLen] | ||
| // The filename is padded with NULL bytes. TrimRight() gets rid of those. | ||
| name += "/" + strings.TrimRight(string(bytes[0:nameLen]), "\000") | ||
| // Move to the next event in the buffer | ||
| offset += unix.SizeofInotifyEvent + inEvent.Len | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func (w *inotify) handleEvent(inEvent *unix.InotifyEvent, buf *[65536]byte, offset uint32) (Event, bool) { | ||
| w.mu.Lock() | ||
| defer w.mu.Unlock() | ||
|
|
||
| /// If the event happened to the watched directory or the watched file, the | ||
| /// kernel doesn't append the filename to the event, but we would like to | ||
| /// always fill the the "Name" field with a valid filename. We retrieve the | ||
| /// path of the watch from the "paths" map. | ||
| /// | ||
| /// Can be nil if Remove() was called in another goroutine for this path | ||
| /// inbetween reading the events from the kernel and reading the internal | ||
| /// state. Not much we can do about it, so just skip. See #616. | ||
| watch := w.watches.byWd(uint32(inEvent.Wd)) | ||
| if watch == nil { | ||
| return Event{}, true | ||
| } | ||
|
|
||
| var ( | ||
| name = watch.path | ||
| nameLen = uint32(inEvent.Len) | ||
| ) | ||
| if nameLen > 0 { | ||
| /// Point "bytes" at the first byte of the filename | ||
| bb := *buf | ||
| bytes := (*[unix.PathMax]byte)(unsafe.Pointer(&bb[offset+unix.SizeofInotifyEvent]))[:nameLen:nameLen] | ||
| /// The filename is padded with NULL bytes. TrimRight() gets rid of those. | ||
| name += "/" + strings.TrimRight(string(bytes[0:nameLen]), "\x00") | ||
| } | ||
|
|
||
| if debug { | ||
| internal.Debug(name, inEvent.Mask, inEvent.Cookie) | ||
| } | ||
|
|
||
| if inEvent.Mask&unix.IN_IGNORED != 0 || inEvent.Mask&unix.IN_UNMOUNT != 0 { | ||
| w.watches.remove(watch) | ||
| return Event{}, true | ||
| } | ||
|
|
||
| // inotify will automatically remove the watch on deletes; just need | ||
| // to clean our state here. | ||
| if inEvent.Mask&unix.IN_DELETE_SELF == unix.IN_DELETE_SELF { | ||
| w.watches.remove(watch) | ||
| } | ||
|
|
||
| // We can't really update the state when a watched path is moved; only | ||
| // IN_MOVE_SELF is sent and not IN_MOVED_{FROM,TO}. So remove the watch. | ||
| if inEvent.Mask&unix.IN_MOVE_SELF == unix.IN_MOVE_SELF { | ||
| if watch.recurse { // Do nothing | ||
| return Event{}, true | ||
| } | ||
|
|
||
| err := w.remove(watch.path) | ||
| if err != nil && !errors.Is(err, ErrNonExistentWatch) { | ||
| if !w.sendError(err) { | ||
| return Event{}, false | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Skip if we're watching both this path and the parent; the parent will | ||
| /// already send a delete so no need to do it twice. | ||
| if inEvent.Mask&unix.IN_DELETE_SELF != 0 { | ||
| _, ok := w.watches.path[filepath.Dir(watch.path)] | ||
| if ok { | ||
| return Event{}, true | ||
| } | ||
| } | ||
|
|
||
| event := w.newEvent(name, mask) | ||
| ev := w.newEvent(name, inEvent.Mask, inEvent.Cookie) | ||
| // Need to update watch path for recurse. | ||
| if watch.recurse { | ||
| isDir := inEvent.Mask&unix.IN_ISDIR == unix.IN_ISDIR | ||
| /// New directory created: set up watch on it. | ||
| if isDir && ev.Has(Create) { | ||
| err := w.register(ev.Name, watch.flags, true) | ||
| if !w.sendError(err) { | ||
| return Event{}, false | ||
| } | ||
|
|
||
| // Send the events that are not ignored on the events channel | ||
| if mask&unix.IN_IGNORED == 0 { | ||
| if !w.sendEvent(event) { | ||
| return | ||
| // This was a directory rename, so we need to update all the | ||
| // children. | ||
| // | ||
| // TODO: this is of course pretty slow; we should use a better data | ||
| // structure for storing all of this, e.g. store children in the | ||
| // watch. I have some code for this in my kqueue refactor we can use | ||
| // in the future. For now I'm okay with this as it's not publicly | ||
| // available. Correctness first, performance second. | ||
| if ev.renamedFrom != "" { | ||
| for k, ww := range w.watches.wd { | ||
| if k == watch.wd || ww.path == ev.Name { | ||
| continue | ||
| } | ||
| if strings.HasPrefix(ww.path, ev.renamedFrom) { | ||
| ww.path = strings.Replace(ww.path, ev.renamedFrom, ev.Name, 1) | ||
| w.watches.wd[k] = ww | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Move to the next event in the buffer | ||
| offset += unix.SizeofInotifyEvent + nameLen | ||
| } | ||
| } | ||
|
|
||
| return ev, true | ||
| } | ||
|
|
||
| func (w *inotify) isRecursive(path string) bool { | ||
| ww := w.watches.byPath(path) | ||
| if ww == nil { // path could be a file, so also check the Dir. | ||
| ww = w.watches.byPath(filepath.Dir(path)) | ||
| } | ||
| return ww != nil && ww.recurse | ||
| } | ||
|
|
||
| // newEvent returns an platform-independent Event based on an inotify mask. | ||
| func (w *Watcher) newEvent(name string, mask uint32) Event { | ||
| func (w *inotify) newEvent(name string, mask, cookie uint32) Event { |
There was a problem hiding this comment.
应在发送事件失败时记录日志或采取其他措施。
🟢 Minor | 🧹 Code Smells
📋 问题详情
在 handleEvent 方法中，当 sendEvent 或 sendError 返回 false 时，方法直接返回而不做任何记录或处理，可能导致问题难以追踪。应该记录日志或采取其他措施。
💡 解决方案
添加日志记录以便在事件发送失败时能够追踪问题。
- if !w.sendEvent(ev) {
- return
- }
+ if !w.sendEvent(ev) {
+ fmt.Fprintf(os.Stderr, \"Failed to send event: %v\\n\", ev)
+ return
+ }您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
Comment on lines
+243
to
+308
| func (w *kqueue) Close() error { | ||
| if w.shared.close() { | ||
| return nil | ||
| } | ||
| w.isClosed = true | ||
|
|
||
| // copy paths to remove while locked | ||
| pathsToRemove := make([]string, 0, len(w.watches)) | ||
| for name := range w.watches { | ||
| pathsToRemove = append(pathsToRemove, name) | ||
| } | ||
| w.mu.Unlock() // Unlock before calling Remove, which also locks | ||
| pathsToRemove := w.watches.listPaths(false) | ||
| for _, name := range pathsToRemove { | ||
| w.Remove(name) | ||
| } | ||
|
|
||
| // Send "quit" message to the reader goroutine. | ||
| unix.Close(w.closepipe[1]) | ||
| close(w.done) | ||
|
|
||
| unix.Close(w.closepipe[1]) // Send "quit" message to readEvents | ||
| return nil | ||
| } | ||
|
|
||
| // Add starts monitoring the path for changes. | ||
| // | ||
| // A path can only be watched once; watching it more than once is a no-op and will | ||
| // not return an error. Paths that do not yet exist on the filesystem cannot be | ||
| // watched. | ||
| // | ||
| // A watch will be automatically removed if the watched path is deleted or | ||
| // renamed. The exception is the Windows backend, which doesn't remove the | ||
| // watcher on renames. | ||
| // | ||
| // Notifications on network filesystems (NFS, SMB, FUSE, etc.) or special | ||
| // filesystems (/proc, /sys, etc.) generally don't work. | ||
| // | ||
| // Returns [ErrClosed] if [Watcher.Close] was called. | ||
| // | ||
| // See [Watcher.AddWith] for a version that allows adding options. | ||
| // | ||
| // # Watching directories | ||
| // | ||
| // All files in a directory are monitored, including new files that are created | ||
| // after the watcher is started. Subdirectories are not watched (i.e. it's | ||
| // non-recursive). | ||
| // | ||
| // # Watching files | ||
| // | ||
| // Watching individual files (rather than directories) is generally not | ||
| // recommended as many programs (especially editors) update files atomically: it | ||
| // will write to a temporary file which is then moved to to destination, | ||
| // overwriting the original (or some variant thereof). The watcher on the | ||
| // original file is now lost, as that no longer exists. | ||
| // | ||
| // The upshot of this is that a power failure or crash won't leave a | ||
| // half-written file. | ||
| // | ||
| // Watch the parent directory and use Event.Name to filter out files you're not | ||
| // interested in. There is an example of this in cmd/fsnotify/file.go. | ||
| func (w *Watcher) Add(name string) error { return w.AddWith(name) } | ||
| func (w *kqueue) Add(name string) error { return w.AddWith(name) } | ||
|
|
||
| // AddWith is like [Watcher.Add], but allows adding options. When using Add() | ||
| // the defaults described below are used. | ||
| // | ||
| // Possible options are: | ||
| // | ||
| // - [WithBufferSize] sets the buffer size for the Windows backend; no-op on | ||
| // other platforms. The default is 64K (65536 bytes). | ||
| func (w *Watcher) AddWith(name string, opts ...addOpt) error { | ||
| _ = getOptions(opts...) | ||
| func (w *kqueue) AddWith(name string, opts ...addOpt) error { | ||
| if debug { | ||
| fmt.Fprintf(os.Stderr, "FSNOTIFY_DEBUG: %s AddWith(%q)\n", | ||
| time.Now().Format("15:04:05.000000000"), name) | ||
| } | ||
|
|
||
| w.mu.Lock() | ||
| w.userWatches[name] = struct{}{} | ||
| w.mu.Unlock() | ||
| _, err := w.addWatch(name, noteAllEvents) | ||
| return err | ||
| with := getOptions(opts...) | ||
| if !w.xSupports(with.op) { | ||
| return fmt.Errorf("%w: %s", xErrUnsupported, with.op) | ||
| } | ||
|
|
||
| _, err := w.addWatch(name, noteAllEvents, false) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| w.watches.addUserWatch(name) | ||
| return nil | ||
| } | ||
|
|
||
| // Remove stops monitoring the path for changes. | ||
| // | ||
| // Directories are always removed non-recursively. For example, if you added | ||
| // /tmp/dir and /tmp/dir/subdir then you will need to remove both. | ||
| // | ||
| // Removing a path that has not yet been added returns [ErrNonExistentWatch]. | ||
| // | ||
| // Returns nil if [Watcher.Close] was called. | ||
| func (w *Watcher) Remove(name string) error { | ||
| func (w *kqueue) Remove(name string) error { | ||
| if debug { | ||
| fmt.Fprintf(os.Stderr, "FSNOTIFY_DEBUG: %s Remove(%q)\n", | ||
| time.Now().Format("15:04:05.000000000"), name) | ||
| } | ||
| return w.remove(name, true) | ||
| } | ||
|
|
||
| func (w *Watcher) remove(name string, unwatchFiles bool) error { | ||
| name = filepath.Clean(name) | ||
| w.mu.Lock() | ||
| if w.isClosed { | ||
| w.mu.Unlock() | ||
| func (w *kqueue) remove(name string, unwatchFiles bool) error { | ||
| if w.isClosed() { | ||
| return nil | ||
| } | ||
| watchfd, ok := w.watches[name] | ||
| w.mu.Unlock() | ||
|
|
||
| name = filepath.Clean(name) | ||
| info, ok := w.watches.byPath(name) | ||
| if !ok { | ||
| return fmt.Errorf("%w: %s", ErrNonExistentWatch, name) | ||
| } | ||
|
|
||
| err := w.register([]int{watchfd}, unix.EV_DELETE, 0) | ||
| err := w.register([]int{info.wd}, unix.EV_DELETE, 0) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| unix.Close(watchfd) | ||
|
|
||
| w.mu.Lock() | ||
| isDir := w.paths[watchfd].isDir | ||
| delete(w.watches, name) | ||
| delete(w.userWatches, name) | ||
|
|
||
| parentName := filepath.Dir(name) | ||
| delete(w.watchesByDir[parentName], watchfd) | ||
|
|
||
| if len(w.watchesByDir[parentName]) == 0 { | ||
| delete(w.watchesByDir, parentName) | ||
| } | ||
| unix.Close(info.wd) | ||
|
|
||
| delete(w.paths, watchfd) | ||
| delete(w.dirFlags, name) | ||
| delete(w.fileExists, name) | ||
| w.mu.Unlock() | ||
| isDir := w.watches.remove(info.wd, name) | ||
|
|
||
| // Find all watched paths that are in this directory that are not external. | ||
| if unwatchFiles && isDir { | ||
| var pathsToRemove []string | ||
| w.mu.Lock() | ||
| for fd := range w.watchesByDir[name] { | ||
| path := w.paths[fd] | ||
| if _, ok := w.userWatches[path.name]; !ok { | ||
| pathsToRemove = append(pathsToRemove, path.name) | ||
| } | ||
| } | ||
| w.mu.Unlock() | ||
| pathsToRemove := w.watches.watchesInDir(name) |
There was a problem hiding this comment.
应确保在关闭 kqueue 时正确处理所有资源释放。
🟡 Major | 🐞 Bugs
📋 问题详情
在 Close 方法中，虽然尝试关闭所有监视路径，但未检查 Remove 操作是否成功，可能导致部分资源未能正确释放。此外，unix.Close(w.closepipe[1]) 调用后未检查错误，若关闭失败可能引发资源泄露。
💡 解决方案
建议在关闭 closepipe 和移除监视路径时检查错误，并记录或返回错误信息，以确保资源被正确释放。
func (w *kqueue) Close() error {
if w.shared.close() {
return nil
}
pathsToRemove := w.watches.listPaths(false)
for _, name := range pathsToRemove {
- w.Remove(name)
+ if err := w.Remove(name); err != nil {
+ // Log the error or handle it appropriately
+ }
}
- unix.Close(w.closepipe[1]) // Send "quit" message to readEvents
+ if err := unix.Close(w.closepipe[1]); err != nil {
+ // Log the error or handle it appropriately
+ }
return nil
}您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
Comment on lines
+688
to
+705
| func (w *kqueue) read(events []unix.Kevent_t) ([]unix.Kevent_t, error) { | ||
| n, err := unix.Kevent(w.kq, nil, events, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return events[0:n], nil | ||
| } | ||
|
|
||
| func (w *kqueue) xSupports(op Op) bool { | ||
| //if runtime.GOOS == "freebsd" { | ||
| // return true // Supports everything. | ||
| //} | ||
| if op.Has(xUnportableOpen) || op.Has(xUnportableRead) || | ||
| op.Has(xUnportableCloseWrite) || op.Has(xUnportableCloseRead) { | ||
| return false | ||
| } | ||
| return true | ||
| } |
There was a problem hiding this comment.
应在读取 kevent 事件时增加超时机制以避免无限等待。
🟠 Critical | 🐞 Bugs
📋 问题详情
当前 read 方法调用 unix.Kevent 时传入的 timeout 参数为 nil，这会导致在没有事件时无限期阻塞。在某些情况下（如程序需要优雅退出），这可能导致程序无法及时响应关闭信号。
💡 解决方案
建议为 unix.Kevent 调用提供一个合理的超时时间，例如 100ms，以便在无事件时能够定期检查退出条件。
func (w *kqueue) read(events []unix.Kevent_t) ([]unix.Kevent_t, error) {
+ var timeout unix.Timespec
+ timeout.Sec = 0
+ timeout.Nsec = 100 * 1000 * 1000 // 100ms
- n, err := unix.Kevent(w.kq, nil, events, nil)
+ n, err := unix.Kevent(w.kq, nil, events, &timeout)
if err != nil {
return nil, err
}
return events[0:n], nil
}您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
Bumps [sigs.k8s.io/controller-runtime](https://github.com/kubernetes-sigs/controller-runtime) from 0.21.0 to 0.22.0. - [Release notes](https://github.com/kubernetes-sigs/controller-runtime/releases) - [Changelog](https://github.com/kubernetes-sigs/controller-runtime/blob/main/RELEASE.md) - [Commits](kubernetes-sigs/controller-runtime@v0.21.0...v0.22.0) --- updated-dependencies: - dependency-name: sigs.k8s.io/controller-runtime dependency-version: 0.22.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
dependabot
bot
force-pushed
the
dependabot/go_modules/sigs.k8s.io/controller-runtime-0.22.0
branch
from
df37da6 to
b6d51fa
Compare
|
Superseded by #42. |
dependabot
bot
deleted the
dependabot/go_modules/sigs.k8s.io/controller-runtime-0.22.0
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bumps sigs.k8s.io/controller-runtime from 0.21.0 to 0.22.0.
Release notes
Sourced from sigs.k8s.io/controller-runtime's releases.
... (truncated)
Commits
fc84a60Merge pull request #3300 from troy0820/troy0820/k8s-deps-1.34c430462update k8s.io dependencies to v0.34.07085be7Merge pull request #3299 from sbueringer/pr-clarify-state-of-warmup5fd7ff6Clarify state of Warmup featuree922805Merge pull request #3293 from s-z-z/certwatcher-patch41feb4ffeat(certwatcher): add instance-specific logger with cert/key context0f4e99eMerge pull request #3296 from alvaroaleman/reconciliationtimeoutf8db32f✨ Add a ReconciliationTimeout option9d3997b✨ envtest: search the assets index for latest of a release series (#3280)9f93124Merge pull request #3290 from alvaroaleman/addafterDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)