Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to force NOT to call Reconcile func again after updating the status subresource? #618

Closed
angrosz opened this issue Mar 4, 2019 · 11 comments

Comments

@angrosz
Copy link

angrosz commented Mar 4, 2019

Hello,

I'm getting and updating status in my Reconciler code as it is stated on https://book.kubebuilder.io/basics/status_subresource.html page. I noticed that r.Status().Update(context.Background(), instance) triggers new call of Reconcile func. Is it possible NOT to call Reconcile func again after the status subresource is successfully finished?
I need to update the status to READY and finish Reconcile func. Calling again Reconcile func creates the infinitive loop in my code (for PATCH).

Best Regards,
Ania

@jwierzbo
Copy link

you can check status value and skip the update if the value is already in the correct state. This will help you avoid the infinity loop.

But yes reconcile after subresource update is frustrating.

@vhosakot
Copy link

vhosakot commented Jun 4, 2019

@vhosakot
Copy link

vhosakot commented Jun 5, 2019

I see this issue too. Is there a way to skip the Reconcile loop when the status subresource is updated?

I'm following these links:

https://book-v1.book.kubebuilder.io/basics/status_subresource.html

https://book.kubebuilder.io/cronjob-tutorial/controller-implementation.html

Comments below for the reconciled object in cronjob_types.go:

// +k8s:openapi-gen=true
// +kubebuilder:subresource:status
// CronJob is the Schema for the cronjobs API
type CronJob struct {

Comments below for the Reconcile loop in cronjob_controller.go:

// +kubebuilder:rbac:groups=batch.tutorial.kubebuilder.io,resources=cronjobs,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=batch.tutorial.kubebuilder.io,resources=cronjobs/status,verbs=get;update;patch
func (r *CronJobReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {

status subresource in CRD:

spec:
...
  subresources:
    status: {}
...

Returning reconciler's Result object with Requeue: false set (see below) in the Reconcile loop does not help:

return ctrl.Result{Requeue: false}, r.Status().Update(ctx, &cronJob)

@angrosz
Copy link
Author

angrosz commented Jun 6, 2019

You can use predicate functions to decide when to skip Reconcile:

p := predicate.Funcs{
	UpdateFunc: func(e event.UpdateEvent) bool {
		oldObject := e.ObjectOld.(*configurationv1.YourCustomResourceDefinition)
		newObject := e.ObjectNew.(*configurationv1.YourCustomResourceDefinition)
		if oldObject.Status != newObject.Status {
			// NO enqueue request
			return false
		}
		// ENQUEUE request
		return true
	},
}

and use this "p" object in Watch function.

// Watch for changes to YourCustomResourceDefinition
err = c.Watch(&source.Kind{Type: &configurationv1.YourCustomResourceDefinition{}}, &handler.EnqueueRequestForObject{}, p)
if err != nil {
	return err
}

I used the following command to generate api:
kubebuilder create api --group configuration --version v1 --kind YourCustomResourceDefinition

Best Regards,
Ania

@vhosakot
Copy link

vhosakot commented Jun 6, 2019

Thanks @angrosz, will try predicate functions and keep you posted.

@DirectXMan12
Copy link
Contributor

the above answer is ok, except that it prevents you from "correcting" changes to status is something else changes status. This is why we generally recommend making controllers idempotent.

@vhosakot
Copy link

vhosakot commented Jun 7, 2019

I was able to use the metadata.generation value in the CR to check if the CR's status was updated and then to decide if the Reconcile loop should be skipped or not.

https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#status-subresource says the following about the status subresource:

The .metadata.generation value is incremented for all changes, except for changes to .metadata or .status.

cronjob_types.go:

type CronJobStatus struct {
    // generation (metadata.generation in cronJob CR) observed by the controller
    ObservedGeneration int64 `json:"observedGeneration,omitempty"`
    ...
}

Reconcile loop in cronjob_controller.go:

func (r *CronJobReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
    var cronJob batch.CronJob
    if err := r.Get(ctx, req.NamespacedName, &cronJob); err != nil {
        if cronJob.Status.ObservedGeneration != cronJob.ObjectMeta.Generation {
            Istio.Status.ObservedGeneration = Istio.ObjectMeta.Generation
            r.Status().Update(ctx, &Istio)
            // need to reconcile as cronJob CR is updated
            ...
        } else {
            // cronJob.Status is updated, metadata.generation of
            // cronJob CR is not incremented, no need to reconcile
        }
    }
    ...
}

The above code skips the Reconcile loop when the CR's status subresource is updated.

I see k8s' deployments use the ObservedGeneration field in their status subresource:

$ kubectl get deploy -n=kube-system -o yaml| grep 'status:\|observedGeneration'
  status:
    observedGeneration: 1
  status:
    observedGeneration: 1

https://github.com/kubernetes/api/blob/9b8cae951d65ea28a341b3262d38a058d0935f7c/apps/v1/types.go#L379

@djzager
Copy link

djzager commented Mar 6, 2020

Just as a note. You don't necessarily need custom predicate functions. There are a couple in controller-runtime's predicate pkg that may be useful. Specifically, the GenerationChangedPredicate may be what you want.

This is why we generally recommend making controllers idempotent.

This seems like the best advice. One thing that I have found helpful to prevent hotlooping, without using a predicate, is to only update the status once per Reconcile.

@cargaona
Copy link
Contributor

cargaona commented Sep 23, 2020

For further reading, using KubeBuilder v2 I managed to use predicates for not reconcile after updating as shown below.

func (r *RedisReconciler) SetupWithManager(mgr ctrl.Manager) error {
	pred := predicate.GenerationChangedPredicate{}
	return ctrl.NewControllerManagedBy(mgr).For(&redisv1beta1.Redis{}).WithEventFilter(pred).Complete(r)
}

All the examples I found were using kubebuilder v1, and the add Add and Watch functions. WithEventFilter is the method you should use to pass the predicate to your controller on the new version.

iamniting added a commit to iamniting/volume-replication-operator that referenced this issue Mar 26, 2021
Updating status subresource from operator reque the recocile request
again which cause the recocile twice for the same request. Fix the
issue using predicate.

reference:
kubernetes-sigs/kubebuilder#618 (comment)

Signed-off-by: Nitin Goyal <nigoyal@redhat.com>
Madhu-1 pushed a commit to csi-addons/volume-replication-operator that referenced this issue Mar 26, 2021
Updating status subresource from operator reque the recocile request
again which cause the recocile twice for the same request. Fix the
issue using predicate.

reference:
kubernetes-sigs/kubebuilder#618 (comment)

Signed-off-by: Nitin Goyal <nigoyal@redhat.com>
Madhu-1 pushed a commit to Madhu-1/operator that referenced this issue May 4, 2021
Updating status subresource from operator reque the recocile request
again which cause the recocile twice for the same request. Fix the
issue using predicate.

reference:
kubernetes-sigs/kubebuilder#618 (comment)

Signed-off-by: Nitin Goyal <nigoyal@redhat.com>
@alexec
Copy link

alexec commented Jun 4, 2021

Does these filters also apply to owend objects?

	return ctrl.NewControllerManagedBy(mgr).
		For(&dfv1.Step{}).
		WithEventFilter(predicate.GenerationChangedPredicate{}).
		Owns(&corev1.Pod{}).
		Owns(&corev1.Service{}).
		Complete(r)

I care about the pod and service status, but not the step status.

ack-bot pushed a commit to aws-controllers-k8s/runtime that referenced this issue Aug 4, 2021
Fixes: aws-controllers-k8s/community#886

The reconciler will indefinitely requeue even when there have been no changes made to the `spec` or `status`. This is happening in both the adoption and resource reconciler types.

The root cause comes from `controller-runtime` requeue-ing the resource when we patch the `status` subresource - see [this issue](kubernetes-sigs/kubebuilder#618) for details. This bug was introduced for the resource reconciler as part of #39 , since we now call `patchResourceStatus` on every reconcile loop.

This fix adds an event filter to each manager, with a predicate that the resource must have changed generation. The generation is not changed unless there has been a modification to the `spec`.
@jgillich
Copy link

jgillich commented Aug 9, 2021

Took me a while to figure this out, but if you want to only filter a subset of objects, you can pass a predicate to your builder calls. In your example @alexec, to filter status for &dfv1.Step{} only:

return ctrl.NewControllerManagedBy(mgr).
  For(&dfv1.Step{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
  Owns(&corev1.Pod{}).
  Owns(&corev1.Service{}).
  Complete(r)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants