add debug logs - #52
Conversation
Signed-off-by: Aliaksei Dziauho <a.dziauho@sap.com>
4c396e6 to
6ba475d
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The newly added CloudProfile operation log is currently emitted at Info level (potentially noisy) and the “skipping retry” wording is misleading given the controller’s scheduled requeue behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds additional controller logging to improve observability of the ManagedCloudProfile reconcile flow and the underlying CloudProfile apply/patch behavior.
Changes:
- Add verbose (V=1) log when reconcile is triggered, including generation and resourceVersion.
- Add verbose (V=1) logs around status patching to indicate when status updates are skipped vs patched.
- Capture and log the
controllerutil.CreateOrPatchoperation result forCloudProfile, and log when an invalidCloudProfileleads to skipping error-based retries.
File summaries
| File | Description |
|---|---|
| controllers/managedcloudprofile_controller.go | Adds V(1) debug logs for reconcile entry and status patch skip/issue paths. |
| controllers/cloud_profile.go | Logs CreateOrPatch operation result and adds an explicit log when invalid CloudProfile errors avoid error-based retries. |
Review details
Suppressed comments (1)
controllers/cloud_profile.go:77
- The message
skipping retryis inaccurate here:Reconcilealways returnsRequeueAfter: 5 * time.Minute, so this will still retry on the next scheduled reconcile (it only skips returning an error/backoff). Consider updating the log message to reflect that behavior (and include the operation result for debugging).
if apierrors.IsInvalid(err) {
log.Error(err, "CloudProfile is invalid, skipping retry")
return nil
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Aliaksei Dziauho <a.dziauho@sap.com>
Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. |
No description provided.