ROX-35081: Add instance report admin command - #2764
Conversation
Add `fleet-manager admin central report` command that fetches all centrals from the fleet-manager API and generates a summary report. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kovayur The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds the admin command to ChangesAdmin central reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The change appears merge-ready after normal validation; no actionable merge-blocking risk remains, with lint and verification checks still recommended. Sequence Diagram(s)sequenceDiagram
participant AdminCLI
participant AdminCommand
participant FleetManagerClient
participant FleetManagerAPI
participant ReportWriter
AdminCLI->>AdminCommand: Execute central report
AdminCommand->>FleetManagerClient: Create and retrieve authenticated client
AdminCommand->>FleetManagerAPI: Fetch paginated central instances
FleetManagerAPI-->>AdminCommand: Return central records
AdminCommand->>ReportWriter: Filter, group, and format records
ReportWriter-->>AdminCLI: Write Slack-compatible report
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@internal/central/pkg/api/admin/private/model_central.go`:
- Around line 28-47: Keep the organisation_name field defined in the canonical
OpenAPI specification rather than editing model_central.go directly, then run
make generate and commit the regenerated output. Ensure the generated Central
model retains the organisation_name JSON property.
In `@internal/central/pkg/cmd/admin/centrals/report.go`:
- Around line 94-150: Extend the report generation around the existing
allCentrals classification loop and writeSection calls to aggregate and print
counts by cloud provider, region, plan, and status for all relevant centrals,
rather than only the current recent-region, eval, expiration, and failed tables.
Reuse the central fields and established report formatting helpers, ensure probe
instances follow the existing exclusion behavior, and add tests covering each
aggregate’s counts before release.
- Around line 52-70: Update the pagination loop around api.GetCentrals to return
an error when list.Items is empty while the fetched count is still below
list.Total, preventing further page requests. Compare the counts using a
non-narrowing type instead of converting len(all) to int32, while preserving the
existing completion condition and error context.
🪄 Autofix
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: 0ae94c87-d189-44ae-9fb3-a4e6b55eda72
📒 Files selected for processing (12)
cmd/acsfleetctl/main.gocmd/fleet-manager/main.gointernal/central/pkg/api/admin/private/api/openapi.yamlinternal/central/pkg/api/admin/private/model_central.gointernal/central/pkg/cmd/admin/centrals/cmd.gointernal/central/pkg/cmd/admin/centrals/list.gointernal/central/pkg/cmd/admin/centrals/report.gointernal/central/pkg/cmd/admin/centrals/report_test.gointernal/central/pkg/cmd/admin/cmd.gointernal/central/pkg/cmd/fleetmanagerclient/client.gointernal/central/pkg/presenters/admin_central.goopenapi/fleet-manager-private-admin.yaml
💤 Files with no reviewable changes (1)
- cmd/acsfleetctl/main.go
The pagination loop in fetchAllCentrals only terminated when the accumulated count reached list.Total, which could spin forever if the API returned an empty Items slice. Break early on empty pages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@coderabbitai review |
|
| if err != nil { | ||
| glog.Fatalf("Failed to create connection: %s", err) | ||
| return | ||
| _, _ = fmt.Fprintf(os.Stderr, "Failed to create connection: %s\n", err) |
There was a problem hiding this comment.
Avoid printing stacktrace (glog.Fatalf behavior)
|
/retest |
johannes94
left a comment
johannes94
left a comment
There was a problem hiding this comment.
Good work, only a minor comment. You can merge when the E2E test succeeds. 👍
| auth, err := impl.NewAuth(ctx, authType, impl.OptionFromEnv()) | ||
| if err != nil { | ||
| _, _ = fmt.Fprintf(os.Stderr, "creating auth: %v\n", err) | ||
| os.Exit(1) |
There was a problem hiding this comment.
I think the preferred way of error handling in cobra would be to use PersistentPreRunE function and return an error. Then let the library do the actual exit call.
This allows for shared cleanup code to run.
it's a nit because after all it does not really matter, since there is no cleanup for this CLI.
|
@kovayur: 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. |
2 participants
Adds the
fleet-manager admin central reportcommand. The command retrieves Centrals through the Fleet Manager admin API and reports counts by region, plan, cloud provider, and status.The PR also:
adminsubcommand fromacsfleetctltofleet-manager.organisation_nameto the Central model, presenters, and OpenAPI schemas.This is part one of two. A follow-up will add a scheduled Tekton pipeline that posts report results to Slack.
Comments summary
The PR author requested a review from CodeRabbit. The automated review was not completed because the review service was rate limited.
The following was generated by
@coderabbitaiand may be updated automatically.Summary
fleet-manager admin central report.organisation_nameto the admin Central model and OpenAPI schemas.Checklist (Definition of Done)
Test manual
fleet-manager admin central report.internal/central/pkg/cmd/admin/centrals.