[hrak]

Summary

Three small error-handling defects found while surveying pkg/errors usage for #133. Each is independent and in its own commit.

1. ResolveZone appended the same error twice

pkg/cloud/zone.go appended an identical Wrapf of the GetZoneID error to retErr both before and after the metrics call:

retErr = multierror.Append(retErr, errors.Wrapf(err, "could not get Zone ID from %v", zSpec.Name))
c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err)
retErr = multierror.Append(retErr, errors.Wrapf(err, "could not get Zone ID from %v", zSpec.Name))

So a zone that could not be resolved by name reported could not get Zone ID from <name> twice in the returned multierror. Now one append, metrics first — matching the GetNetworkByName block directly below it.

No test asserted on the duplication, so nothing had pinned the behaviour.

2. ResolveNetworkForZone counted the wrong branch

The ACS error counter was called in the count != 1 branch of the GetNetworkByID check, where err is nil by construction. EvaluateErrorAndIncrementAcsReconciliationErrorCounter ignores nil, so that call did nothing — while a genuine error from GetNetworkByID returned early without ever being counted.

Moved into the err != nil branch, matching both the GetNetworkByName block at the top of the same function and the GetZoneByID block in ResolveZone: count the ACS error, do not count a well-formed response that simply matched no network.

Effect: real ACS failures on that path now show up in acs_reconciliation_error_count, where previously they were invisible.

3. The LB attachment error was formatted, not wrapped

internal/controllers/cloudstackmachine_controller.go:214:

return ctrl.Result{}, errors.Errorf("failed to reconcile LB attachment: %+v", err)

Two problems:

• Errorf does not wrap. The original error was flattened into a string, so errors.Is/errors.As could not see through it — a caller wanting to match a sentinel underneath (cloud.ErrNotFound, say) silently could not.
• %+v on a pkg/errors value renders a full stack trace, so the returned error's message carried that trace into the controller logs on every failed load balancer detach.

pkgerrors.Wrap(err, "failed to reconcile LB attachment") fixes both: same message prefix, cause rendered without a stack trace, and Wrap implements Unwrap (via withStack/withMessage, pkg/errors v0.9.1), so Is/As now traverse the chain.

This was the only production call formatting an error into a new one. The remaining %v cases are panics in envtest setup and one Info log, where there is no error to propagate.

Test plan

• make build
• make lint — 0 issues
• make test — all ginkgo suites plus the internal/controllers envtest suite. pkg/cloud/zone_test.go asserts on the exact multierror text of both resolve paths and still passes, confirming the message shapes are unchanged apart from the removed duplicate

🤖 Generated with Claude Code