fix: error being silenced during apply

[timonwong]

	defer func() {
		applyErr := u.Apply(ctx, obj)
   		// Problem: err is not assigned to return value  
		if err == nil && !apierrors.IsNotFound(applyErr) {
			err = applyErr
		}
	}()

[porridge]

Nice catch!
Although it feels like such issue should really be caught by unit tests. Could you maybe take a stab at adding such case?

[timonwong]

Nice catch! Although it feels like such issue should really be caught by unit tests. Could you maybe take a stab at adding such case?

sure! i'll then add a case for that

[porridge]

LGTM, but the comments seem a bit off.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.67%. Comparing base (08ab7fb) to head (abbabcb).
Report is 48 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (08ab7fb) and HEAD (abbabcb). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (08ab7fb)	HEAD (abbabcb)
	2	1

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #395      +/-   ##
==========================================
- Coverage   85.06%   79.67%   -5.40%     
==========================================
  Files          19       31      +12     
  Lines        1346     1958     +612     
==========================================
+ Hits         1145     1560     +415     
- Misses        125      310     +185     
- Partials       76       88      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[komish]

Any particular reason why the Apply operation can't be done at the end of the function instead of in a defer clause?

[timonwong]

Any particular reason why the Apply operation can't be done at the end of the function instead of in a defer clause?

Becasue the Apply() is for status update here. When early return (in case of error) from Reconcile loop, you need to set error conditions for your managed CR resources, the defer approch is more ideal than:

1. use goto to handle them in a uniform way
2. call Apply every time before return err

[perdasilva]

lgtm! Thank you <3