Skip to content

Commit

Permalink
Use reconcile predicates to prevent reconcile on status update
Browse files Browse the repository at this point in the history
  • Loading branch information
matthchr committed Aug 26, 2021
1 parent 6917a6e commit d6d1c4a
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 5 deletions.
4 changes: 4 additions & 0 deletions hack/generated/controllers/generic_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/Azure/azure-service-operator/hack/generated/pkg/armclient"
Expand Down Expand Up @@ -142,6 +143,9 @@ func register(mgr ctrl.Manager, reconciledResourceLookup map[schema.GroupKind]sc

c, err := ctrl.NewControllerManagedBy(mgr).
For(obj).
// Note: These predicates prevent status updates from triggering a reconcile.
// to learn more look at https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/predicate#GenerationChangedPredicate
WithEventFilter(predicate.Or(predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{})).
WithOptions(options.Options).
Build(reconciler)

Expand Down
15 changes: 10 additions & 5 deletions hack/generated/pkg/reconcilers/azure_deployment_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,8 @@ func (r *AzureDeploymentReconciler) StartDeleteOfResource(ctx context.Context) (
return ctrl.Result{}, err
}

// Note: We requeue here because we've only changed the status and status updates don't trigger another reconcile
// because we use predicate.GenerationChangedPredicate and predicate.AnnotationChangedPredicate
// delete has started, check back to seen when the finalizer can be removed
r.log.V(3).Info("Resource deletion started")

Expand Down Expand Up @@ -488,8 +490,7 @@ func (r *AzureDeploymentReconciler) MonitorDelete(ctx context.Context) (ctrl.Res
// TODO: Transfer the below into controller?
err = r.deleteResourceSucceeded(ctx)

// patcher will try to fetch the object after patching, so ignore not found errors
return ctrl.Result{}, client.IgnoreNotFound(err)
return ctrl.Result{}, err
}

func (r *AzureDeploymentReconciler) CreateDeployment(ctx context.Context) (ctrl.Result, error) {
Expand Down Expand Up @@ -930,11 +931,15 @@ func (r *AzureDeploymentReconciler) deleteResourceSucceeded(ctx context.Context)
controllerutil.RemoveFinalizer(r.obj, GenericControllerFinalizer)
err := r.CommitUpdate(ctx)

r.log.V(0).Info("Deleted resource")

// We must also ignore conflict here because updating a resource that
// doesn't exist returns conflict unfortunately: https://github.com/kubernetes/kubernetes/issues/89985
return ignoreNotFoundAndConflict(err)
err = ignoreNotFoundAndConflict(err)
if err != nil {
return err
}

r.log.V(0).Info("Deleted resource")
return nil
}

func ignoreNotFoundAndConflict(err error) error {
Expand Down

0 comments on commit d6d1c4a

Please sign in to comment.