bug: cleanup swallows removeMaintenanceRefFromServer errors, may leave stale ServerMaintenanceRef

[coderabbitai]

Bug Report

Description

In internal/controller/servermaintenance_controller.go, the cleanup function silently swallows errors returned by removeMaintenanceRefFromServer — it only logs them and allows reconciliation to continue.

This means that if patching Server.Spec.ServerMaintenanceRef fails, the controller may still proceed to remove the ServerMaintenanceFinalizer, leaving a stale ServerMaintenanceRef on the Server object with no way to retry the cleanup.

Affected Code

File: internal/controller/servermaintenance_controller.go

if server.Spec.ServerMaintenanceRef != nil {
    if err := r.removeMaintenanceRefFromServer(ctx, server); err != nil {
        log.Error(err, "Failed to remove ServerMaintenance ref from Server")
        // error is swallowed — reconciliation continues
    }
}

Expected Behavior

If removeMaintenanceRefFromServer returns an error, cleanup should propagate the error so the reconciler retries and does not remove the finalizer while the Server patch is still failing.

Suggested Fix

if server.Spec.ServerMaintenanceRef != nil {
    if err := r.removeMaintenanceRefFromServer(ctx, server); err != nil {
        return fmt.Errorf("failed to remove ServerMaintenance ref from Server: %w", err)
    }
}

References

• Identified in PR Clean up log messages, comments, error wrapping, and test descriptions #783 (comment: Clean up log messages, comments, error wrapping, and test descriptions #783 (comment))
• Reported by @afritzler