in_kubernetes_events: bound watch connection lifetime - #12105
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Kubernetes events input adds a configurable watch timeout, applies it to Kubernetes watch requests and streaming HTTP read idle handling, advances the tracked resource version from streamed events, and adds integration coverage for stalled-watch reconnection. ChangesKubernetes watch timeout handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant KubernetesEventsInput
participant KubernetesAPI
participant StreamingHTTPClient
KubernetesEventsInput->>KubernetesAPI: Request watch URL with timeoutSeconds
KubernetesEventsInput->>StreamingHTTPClient: Set read idle timeout
KubernetesAPI-->>StreamingHTTPClient: Stream event
KubernetesEventsInput->>KubernetesEventsInput: Process event and advance resourceVersion
KubernetesAPI-->>StreamingHTTPClient: Close stalled watch
KubernetesEventsInput->>KubernetesAPI: Reconnect watch
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 602e7948c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| flb_sds_printf(&url, "?watch=1&resourceVersion=%" PRIu64, max_resource_version); | ||
| if (ctx->watch_timeout > 0) { | ||
| flb_sds_printf(&url, "&timeoutSeconds=%d", ctx->watch_timeout); |
There was a problem hiding this comment.
Advance resource versions before timing out watches
With the new default positive watch timeout, every watch request asks the API server to close after 600s, which makes the next collector cycle perform a full list again. In DB-less configurations, watched events are not used to advance ctx->last_resource_version (it is only updated from list metadata in check_and_init_stream()), so any events delivered by the watch and still retained by Kubernetes have resource versions greater than the stale filter and are emitted a second time on each forced relist. Please update the tracked resource version from watched events or avoid enabling periodic relists by default.
Useful? React with 👍 / 👎.
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
602e794 to
b0c1a32
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@tests/integration/scenarios/in_kubernetes_events/tests/test_in_kubernetes_events_001.py`:
- Around line 52-59: Update the Kubernetes event recovery test around the server
response logic and its existing duplicate/resource-version assertions so the
first watch emits only the initial event, while the second watch or relist (when
watch_requests >= 2) emits a distinct event. Extend the client-side wait and
assertions to require that this post-recovery event is processed, while
preserving the existing duplicate/resource-version checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 12a2ad88-86ef-42ce-ba30-b70492c7a5cb
📒 Files selected for processing (3)
plugins/in_kubernetes_events/kubernetes_events.cplugins/in_kubernetes_events/kubernetes_events.htests/integration/scenarios/in_kubernetes_events/tests/test_in_kubernetes_events_001.py
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/in_kubernetes_events/kubernetes_events.h
- plugins/in_kubernetes_events/kubernetes_events.c
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
b0c1a32 to
7c8f175
Compare
Problem
The Kubernetes events watch request can remain silently half-open forever when the API server or an intermediary stops sending data without closing the connection. The plugin accepts the incomplete HTTP response as an active streaming client, while the default network I/O timeout is disabled, so the collector never reconnects.
Fixes #12019.
Changes
kube_watch_timeout, defaulting to10m; setting it to0disables the timeout.timeoutSecondsquery parameter to watch requests.The default bounds recovery time while avoiding frequent API relists. Existing behavior can be retained with
kube_watch_timeout 0.Validation
All checks passed:
The strict Valgrind run reported zero leaks and zero errors.
Summary by CodeRabbit
New Features
0to disable.Bug Fixes
resourceVersionafter handling events.Tests