WIP: ROX-35081: Add weekly ACS instance report CronJob#2752
Conversation
|
[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 |
📝 WalkthroughWalkthroughAdds an authenticated ChangesACS instance reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ACSReportPipeline
participant GitClone
participant FleetManager
participant SlackNotification
ACSReportPipeline->>GitClone: clone main branch
ACSReportPipeline->>FleetManager: build and run admin central report
FleetManager-->>ACSReportPipeline: return report message and exit code
ACSReportPipeline->>SlackNotification: send report message
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
kovayur
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.tekton/acs-instance-report-cron.yaml (1)
123-129: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBuild step memory limit may be too low for a full Go build.
make fleet-managercompiles a large Go module; a 512Mi memory limit can trigger OOM kills during compilation/linking, causing intermittent cron failures. Consider raising the memory limit (e.g., 1–2Gi) and validating against a real run.🤖 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 @.tekton/acs-instance-report-cron.yaml around lines 123 - 129, Increase the build step memory limit in the computeResources block used by make fleet-manager from 512Mi to an appropriate value such as 1Gi–2Gi, while retaining the existing request unless needed; validate the cron job with a real full Go build to confirm it no longer encounters OOM failures.
🤖 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 @.tekton/acs-instance-report-cron.yaml:
- Line 121: Update the printf command in the pipeline report step to use a
format that interprets escaped newline sequences, such as %b, so the separators
in MESSAGE and PIPELINE_LINK render as actual blank lines rather than literal \n
text.
---
Nitpick comments:
In @.tekton/acs-instance-report-cron.yaml:
- Around line 123-129: Increase the build step memory limit in the
computeResources block used by make fleet-manager from 512Mi to an appropriate
value such as 1Gi–2Gi, while retaining the existing request unless needed;
validate the cron job with a real full Go build to confirm it no longer
encounters OOM failures.
🪄 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: Enterprise
Run ID: 93654d42-0add-49d7-bc07-f89f1b7147bd
📒 Files selected for processing (13)
.tekton/acs-instance-report-cron.yamlcmd/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
| fi | ||
|
|
||
| PIPELINE_LINK="<${PIPELINE_RUN_URL}|${PIPELINE_RUN_NAME}>" | ||
| printf '%s' "${MESSAGE}\n\nPipeline run: ${PIPELINE_LINK}" > $(results.message.path) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
printf '%s' will not expand \n; Slack message will contain literal \n.
With %s, the \n\n between the message and pipeline link (and the \n\n embedded in the build-failure MESSAGE) are emitted as literal backslash-n rather than newlines. Use %b (or actual newlines) so the separators render correctly.
🔧 Proposed fix
- printf '%s' "${MESSAGE}\n\nPipeline run: ${PIPELINE_LINK}" > $(results.message.path)
+ printf '%b' "${MESSAGE}\n\nPipeline run: ${PIPELINE_LINK}" > $(results.message.path)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| printf '%s' "${MESSAGE}\n\nPipeline run: ${PIPELINE_LINK}" > $(results.message.path) | |
| printf '%b' "${MESSAGE}\n\nPipeline run: ${PIPELINE_LINK}" > $(results.message.path) |
🤖 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 @.tekton/acs-instance-report-cron.yaml at line 121, Update the printf command
in the pipeline report step to use a format that interprets escaped newline
sequences, such as %b, so the separators in MESSAGE and PIPELINE_LINK render as
actual blank lines rather than literal \n text.
1 participant
PR summary
Adds a weekly ACS instance report CronJob to ACS Fleet Manager.
The following was generated by
@coderabbitaiand may be updated automatically.Summary
Adds a
centrals reportadmin command that generates Slack-formatted ACS instance reports, including new, evaluation, expired, and failed instances. The command is exposed throughfleet-manager, uses authenticated client context, and includes expanded organisation metadata.Adds a weekly Tekton CronJob that runs the report and sends the output to Slack, with repository checkout, retention, authentication, and failure-safe notifications configured.
Checklist (Definition of Done)
Test manual