[Backport] NETOBSERV-2693 Open Flowcollector status page from OLM page - #1670
[Backport] NETOBSERV-2693 Open Flowcollector status page from OLM page#1670jpinsonneau wants to merge 2 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughFlowCollector routing now supports setup, edit, YAML, cluster, and status paths. Status handling recognizes deletion and not-found states. ResourceWatcher confirms deletion through API polling, while the status page and delete modal provide deletion, loading, missing-resource, and asynchronous action handling. ChangesFlowCollector lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
web/src/components/status/__tests__/flowcollector-status.spec.tsx (1)
101-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for object-shaped k8s errors.
The suite only covers strings and
Errors. The shape Console actually rejects with (a plain object carryingjson.code/json.reason) is untested, and currently unhandled byisK8sNotFoundError. See the related comment inweb/src/components/forms/utils.ts.🤖 Prompt for 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. In `@web/src/components/status/__tests__/flowcollector-status.spec.tsx` around lines 101 - 108, Update isK8sNotFoundError and its tests to support plain Kubernetes error objects containing json.code/json.reason, recognizing the not-found code or reason while preserving existing string, Error, null, and unrelated-error behavior. Add a test in the isK8sNotFoundError suite using the Console rejection shape.
🤖 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 `@web/src/components/forms/flowCollector-status.tsx`:
- Around line 196-217: Normalize ctx.loadError to a human-readable string before
passing it as the error interpolation value in the hasLoadError branch of
flowCollectorMissing. Preserve the existing translated message and ensure
object-shaped errors do not render as “[object Object]”.
In `@web/src/components/forms/resource-watcher.tsx`:
- Around line 306-339: Update both catch blocks in the resource watcher promise
chains to normalize rejected values before calling setErrors, using the error
message, nested JSON message, or String fallback in that order. Replace the
direct e.message usage while preserving rethrow behavior and existing error
handling.
- Around line 168-199: Update the delete-absence polling effect around
pollUntilGone and k8sGet to enforce a maximum attempt, deadline, or backoff
rather than rescheduling indefinitely. When the object remains present or k8sGet
returns a non-404 error after the limit, stop polling and transition the
existing form state to a surfaced delete-failure error; retain
setDeletingSnapshot(null) for confirmed 404 responses and clean up timers on
effect teardown.
In `@web/src/components/forms/utils.ts`:
- Around line 10-16: Normalize Kubernetes errors through one shared helper. In
web/src/components/forms/utils.ts#L10-L16, add and export k8sErrorMessage,
detect code/json.code 404, response.status 404, and json.reason "NotFound"
before matching message text, and update isK8sNotFoundError to use it. In
web/src/components/forms/resource-watcher.tsx#L306-L339, use k8sErrorMessage(e)
in both setErrors calls; in
web/src/components/forms/flowCollector-status.tsx#L196-L217, pass
k8sErrorMessage(ctx.loadError) to the error interpolation. In
web/src/components/status/__tests__/flowcollector-status.spec.tsx#L101-L108, add
object-rejection cases verifying 404/NotFound is true and 403 is false.
In `@web/src/utils/url.ts`:
- Around line 63-64: Update isFlowCollectorCreatePath to recognize both the
existing "~new" segment and the "setup" segment as create routes, preserving the
current behavior for other paths. Add regression coverage verifying both route
forms are identified as create paths.
---
Nitpick comments:
In `@web/src/components/status/__tests__/flowcollector-status.spec.tsx`:
- Around line 101-108: Update isK8sNotFoundError and its tests to support plain
Kubernetes error objects containing json.code/json.reason, recognizing the
not-found code or reason while preserving existing string, Error, null, and
unrelated-error behavior. Add a test in the isK8sNotFoundError suite using the
Console rejection shape.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2cd666a9-215e-4e4b-b584-7b4796a121f4
📒 Files selected for processing (11)
web/locales/en/plugin__netobserv-plugin.jsonweb/src/components/forms/flowCollector-status.tsxweb/src/components/forms/flowCollector.tsxweb/src/components/forms/resource-delete-modal.tsxweb/src/components/forms/resource-watcher.tsxweb/src/components/forms/utils.tsweb/src/components/status/__tests__/flowcollector-status.spec.tsxweb/src/components/status/flowcollector-status-indicator.tsxweb/src/utils/k8s-models-hook.tsweb/src/utils/url.tsweb/webpack.config.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netobserv/netobserv-operator(manual)netobserv/flowlogs-pipeline(manual)
|
Addressed rabbit feedback 054f39d Sadly that was not suggested in pf5 branch. We should consider enabling code rabbit when a human create a PR against any branch |
|
@jpinsonneau: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
Backport of #1570
Summary by CodeRabbit
New Features
Bug Fixes