Skip to content

Add failingHost to CustomRole status, drop noisy connect log#366

Merged
tmablunar merged 4 commits intomasterfrom
customrole-failing-host-status
Apr 28, 2026
Merged

Add failingHost to CustomRole status, drop noisy connect log#366
tmablunar merged 4 commits intomasterfrom
customrole-failing-host-status

Conversation

@tmablunar
Copy link
Copy Markdown
Contributor

@tmablunar tmablunar commented Apr 27, 2026

Summary

  • Add status.failingHost to CustomRole so operators can see which PostgreSQL host caused a reconcile failure without grepping controller logs.
  • Remove the Connecting to database log line in pkg/postgres/postgres.go. It fired on every connect — admin DB plus per-database connections for grants and functions — so each resync cycle produced many lines per CustomRole.

Notes

  • persistStatus now takes the failing host and clears it on success.
  • CRD and deepcopy regenerated via make generate manifests.
  • Existing CustomRoles already in Failed won't get failingHost populated until they next reconcile.

Test plan

  • Apply the updated CRD; existing CustomRoles still validate.
  • Trigger a reconcile failure on one host (e.g. wrong credentials) and confirm status.failingHost reports it and status.error matches.
  • Resolve the failure and confirm status.failingHost clears on the next successful reconcile.
  • Confirm controller logs no longer emit Connecting to database.

🤖 Generated with Claude Code


Note

Medium Risk
Medium risk because it updates the CustomRole CRD/status schema and refactors the widely-used postgres.Connect API, which could affect controller behavior and diagnostics if any call sites were missed.

Overview
Adds status.failingHost to CustomRole and updates reconciliation to persist the host that triggered a failure (and clear it on success/invalid), including an IsUnchanged helper to avoid no-op status updates.

Removes the per-connection "Connecting to database" log by changing postgres.Connect to no longer accept a logger, then updates all controller code and tests to use the new signature.

Reviewed by Cursor Bugbot for commit 9d12464. Configure here.

Surfaces which PostgreSQL host caused reconciliation to fail via
status.failingHost, and removes the per-connect "Connecting to
database" log line that fired on every reconcile cycle for every
targeted database.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tmablunar tmablunar requested a review from a team as a code owner April 27, 2026 07:50
Comment thread internal/controller/customrole_controller.go Outdated
Comment thread pkg/postgres/postgres.go
… check

- Remove `log logr.Logger` from `postgres.Connect` — the only usage was
  the log line removed in the previous commit; update all call sites.
- Extract the status-unchanged early-return in `persistStatus` into
  `CustomRoleStatus.IsUnchanged` per reviewer suggestion.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@tmablunar
Copy link
Copy Markdown
Contributor Author

bugbot run

Comment thread internal/controller/customrole_controller.go
tmablunar and others added 2 commits April 27, 2026 13:09
Invalid errors are spec-level and not host-specific; leaving failingHost
set from a prior Failed reconcile would be misleading.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@tmablunar
Copy link
Copy Markdown
Contributor Author

bugbot run

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 9d12464. Configure here.

@tmablunar tmablunar merged commit 1523713 into master Apr 28, 2026
6 checks passed
@tmablunar tmablunar deleted the customrole-failing-host-status branch April 28, 2026 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants