Skip to content

Commit

Permalink
Fix finalize logging (#417)
Browse files Browse the repository at this point in the history
* Fix finalize logging

Signed-off-by: John Collier <[email protected]>

* Fix up log

Signed-off-by: John Collier <[email protected]>

---------

Signed-off-by: John Collier <[email protected]>
  • Loading branch information
johnmcollier authored Nov 21, 2023
1 parent c3e6afd commit 109a862
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions controllers/application_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (

k8sErrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/util/retry"
"k8s.io/client-go/util/workqueue"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
Expand Down Expand Up @@ -115,30 +116,37 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
if containsString(application.GetFinalizers(), appFinalizerName) {
metrics.ApplicationDeletionTotalReqs.Inc()
// A finalizer is present for the Application CR, so make sure we do the necessary cleanup steps
if err := r.Finalize(ctx, &application, ghClient); err != nil {
if finalizeErr := r.Finalize(ctx, &application, ghClient); finalizeErr != nil {
finalizeCounter, err := getCounterAnnotation(finalizeCount, &application)
if err == nil && finalizeCounter < 5 {
// The Finalize function failed, so increment the finalize count and return
setCounterAnnotation(finalizeCount, &application, finalizeCounter+1)
err := r.Update(ctx, &application)
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
err := r.Update(ctx, &application)
return err
})
if err != nil {
log.Error(err, "Error incrementing finalizer count on resource")
}
return ctrl.Result{}, nil
} else {
// if fail to delete the external dependency here, log the error, but don't return error
// Don't want to get stuck in a cycle of repeatedly trying to delete the repository and failing
log.Error(err, "Unable to delete GitOps repository for application %v in namespace %v", application.GetName(), application.GetNamespace())

// Increment the Application deletion failed metric as the application delete did not fully succeed
log.Error(finalizeErr, "Unable to delete GitOps repository for application")
metrics.ApplicationDeletionFailed.Inc()
}

}

// remove the finalizer from the list and update it.
controllerutil.RemoveFinalizer(&application, appFinalizerName)
if err := r.Update(ctx, &application); err != nil {
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
err := r.Update(ctx, &application)
return err
})
if err != nil {
return ctrl.Result{}, err
} else {
metrics.ApplicationDeletionSucceeded.Inc()
Expand Down

0 comments on commit 109a862

Please sign in to comment.