✨ Route controller logs through shared helpers #2971

MaxRink · 2025-11-25T15:50:00Z

What this PR does / why we need it

Route every controller reconcile path through the shared logging helpers, ensuring consistent outcome/error/logic-gate instrumentation across metal3cluster, metal3machine, metal3data, metal3datatemplate, metal3machinetemplate, metal3labelsync, and metal3remediation.
Update controller unit tests so each helper invocation receives an explicit logger after the signature changes, keeping regression coverage in place.

Why this matters

Before this change, many reconcile branches emitted no structured output at all—several delete helpers only logged fatal errors while success paths were silent, and other branches mixed ad-hoc Info/Error calls without tying together controller name, gate decisions, or the final outcome. That made it difficult to correlate events in debug sessions, especially when helper functions (reconcileNormal, reconcileDelete, metadata association, pause handling, etc.) returned early. By funneling each controller through logReconcileOutcome, logLogicGate, and logAndReturnError, every decision point now leaves a predictable debug-level trail, and all error returns are logged exactly once with contextual keys. The updated tests ensure helpers keep receiving loggers so future refactors can’t drop this coverage inadvertently.

Fixes

Fixes #

Checklist

Documentation has been updated, if necessary.
Unit tests have been added, if necessary.
E2E tests have been added, if necessary.
Integration tests have been added, if necessary.

metal3-io-bot · 2025-11-25T15:50:07Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sunnatillo for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

metal3-io-bot · 2025-11-25T15:50:12Z

Hi @MaxRink. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions 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.

Copilot

Pull request overview

This pull request introduces a centralized logging infrastructure to improve observability and consistency across all Metal3 controllers. By routing every reconciliation path through shared logging helpers (logReconcileOutcome, logLogicGate, and logAndReturnError), the changes ensure that controller logic gates, error conditions, and reconciliation outcomes are uniformly instrumented.

Key Changes:

Introduces three new logging helper functions in controllers/logging_helpers.go to standardize debug-level lifecycle logging, error logging, and decision point tracking
Updates all seven controllers (metal3cluster, metal3machine, metal3machinetemplate, metal3data, metal3datatemplate, metal3labelsync, metal3remediation) to use deferred outcome logging and route errors through centralized helpers
Modifies reconcile helper function signatures (reconcileNormal, reconcileDelete) across all controllers to accept explicit logger parameters, with corresponding test updates to pass klogr.New() loggers

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
controllers/logging_helpers.go	Defines three new shared logging utilities: `logReconcileOutcome` for deferred reconciliation completion/failure logging, `logLogicGate` for debug-level decision point tracking, and `logAndReturnError` for compact error logging and return
controllers/metal3cluster_controller.go	Adds deferred outcome logging, routes errors through `logAndReturnError`, adds logic gate tracking, and updates `reconcileNormal`/`reconcileDelete` signatures to accept logger parameter
controllers/metal3cluster_controller_test.go	Updates test calls to `reconcileNormal` and `reconcileDelete` to pass `klogr.New()` logger matching new signatures
controllers/metal3machine_controller.go	Adds deferred outcome logging, routes errors through `logAndReturnError`, adds extensive logic gate tracking throughout normal and delete flows, and updates reconcile function signatures
controllers/metal3machine_controller_test.go	Updates test calls to `reconcileNormal` and `reconcileDelete` to pass `klogr.New()` logger
controllers/metal3machinetemplate_controller.go	Adds deferred outcome logging, converts Info to Error for patch failures, routes errors through `logAndReturnError`, adds logic gate tracking, and updates `reconcileNormal` signature
controllers/metal3machinetemplate_controller_test.go	Updates test call to `reconcileNormal` to pass `klogr.New()` logger
controllers/metal3data_controller.go	Adds deferred outcome logging, converts Info to Error for patch failures, routes errors through `logAndReturnError`, adds logic gate tracking, and updates reconcile function signatures
controllers/metal3data_controller_test.go	Updates test calls to `reconcileNormal` and `reconcileDelete` to pass `klogr.New()` logger
controllers/metal3datatemplate_controller.go	Adds deferred outcome logging, converts Info to Error for patch failures, routes errors through `logAndReturnError`, adds logic gate tracking, updates reconcile signatures, and modifies `checkReconcileError` to accept logger
controllers/metal3datatemplate_controller_test.go	Updates test calls to `reconcileNormal`, `reconcileDelete`, and `checkReconcileError` to pass `klogr.New()` logger
controllers/metal3labelsync_controller.go	Adds deferred outcome logging, converts Info to Error for patch and fetch failures, routes errors through `logAndReturnError`, and adds logic gate tracking
controllers/metal3remediation_controller.go	Adds deferred outcome logging, routes errors through `logAndReturnError`, replaces `r.Log` with passed logger in `reconcileNormal`, adds logic gate tracking, and updates `reconcileNormal` signature
controllers/metal3remediation_controller_test.go	Updates test call to `reconcileNormal` to pass `klogr.New()` logger

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

controllers/metal3machinetemplate_controller.go

controllers/metal3labelsync_controller.go

controllers/metal3data_controller.go

controllers/metal3machine_controller.go

controllers/metal3cluster_controller.go

controllers/metal3machinetemplate_controller.go

controllers/metal3labelsync_controller.go