K8s controller update status and condition

Im having k8s controller which needs to install some resources and update the status and condition accordantly

The flow in the reconcile is like following:

1. Install the resource and don’t wait
2. Call to the function checkAvailability and update the status accordantly if ready/ pending install/ error

I’ve two main questions:

1. This is the first time that I use status and conditions, is it right way or do I miss something
2. Sometimes when I do the update r.Status().Update I got error :Operation cannot be fulfilled on eds.core.vtw.bmw.com “resouce01”: the object has been modified; please apply your changes to the latest version and try again , so I’ve added the check conditionChanged` which solve the problem but not sure if its correct as I update the status once and if it doesn’t chanted I don’t touch it so user can see status ready from a while ago and the reconcile doesn’t update the date and time for the ready condition as it skip it when its already “ready”

I use the following

func (r *ebdReconciler) checkHealth(ctx context.Context, req ctrl.Request, ebd ebdmanv1alpha1.ebd) (bool, error) {
vfmReady, err := r.mbr.IsReady(ctx, req.Name, req.Namespace)
condition := metav1.Condition{
Type:               ebdmanv1alpha1.KubernetesvfmHealthy,
Observebdneration: ebd.Generation,
LastTransitionTime: metav1.Now(),
}
if err != nil {
// There was an error checking readiness - Set status to false
condition.Status = metav1.ConditionFalse
condition.Reason = ebdmanv1alpha1.ReasonError
condition.Message = fmt.Sprintf("Failed to check  vfm readiness: %v", err)
} else if vfmReady {
// The vfm is ready - Set status to true
condition.Status = metav1.ConditionTrue
condition.Reason = ebdmanv1alpha1.ReasonReady
condition.Message = "vfm custom resource is ready"
} else {
// The vfm is not ready - Set status to false
condition.Status = metav1.ConditionFalse
condition.Reason = ebdmanv1alpha1.ResourceProgressing
condition.Message = "vfm custom resource is not ready"
}
// Get the latest version of the ebd
latestebd := ebdmanv1alpha1.ebd{}
if err := r.Get(ctx, req.NamespacedName, &latestebd); err != nil {
return vfmReady, err
}
oldConditions := latestebd.Status.Conditions
meta.SetStatusCondition(&latestebd.Status.Conditions, condition)
if !conditionChanged(&oldConditions, &latestebd.Status.Conditions, ebdmanv1alpha1.KubernetesvfmHealthy) {
return vfmReady, nil
}
if err := r.Status().Update(ctx, &latestebd); err != nil {
r.Log.Error(err, "failed to update vfm status")
return vfmReady, err
}
return vfmReady, nil
}
func conditionChanged(oldConditions, newConditions *[]metav1.Condition, conditionType string) bool {
newCondition := meta.FindStatusCondition(*newConditions, conditionType)
oldCondition := meta.FindStatusCondition(*oldConditions, conditionType)
if oldCondition == nil && newCondition == nil {
return false
}
if oldCondition == nil || newCondition == nil {
return true
}
return oldCondition.Status != newCondition.Status || oldCondition.Reason != newCondition.Reason || oldCondition.Message != newCondition.Message
}

To your questions:

1. This is the first time that I use status and conditions, is it right way or do I miss something?

   Your approach to managing the status and conditions of Kubernetes resources is generally fine. The status subresource in a Kubernetes API object is typically used to represent the current state of the system, and it can include conditions.

   A condition is a collection of fields that describe the state of an object in a more detailed way than just true or false. Each condition typically has a type, status, reason, message, and lastTransitionTime. Your code correctly sets these fields based on whether the vfm custom resource is ready or not.

   It is good to note that conditions should be leveled - meaning they should be set to their current observed value regardless of their previous value. They should also be set (either true, false, or unknown) for all the significant or user-meaningful aspects of the component's current state. This makes conditions a good mechanism to indicate "transient states" like Progressing or Degraded that might be expected to change over time or based on external state.

2. Sometimes when I do the update r.Status().Update I got error: Operation cannot be fulfilled on eds.core.vtw.bmw.com “resource01”: the object has been modified; please apply your changes to the latest version and try again.

   This error occurs because another client updated the same object while your controller was processing it. This could be another controller or even another instance of the same controller (if you run more than one).

   One possible way to handle this is to use a retry mechanism that re-attempts the status update when this error occurs. In your case, you have implemented a conditionChanged check to only attempt the status update if the condition has changed. This is a good approach to avoid unnecessary updates, but it does not completely prevent the error, because another client could still update the object between your Get call and Status().Update call.

   You could also consider using Patch instead of Update to modify the status, which reduces the risk of conflicting with other updates. Patching allows for partial updates to an object, so you are less likely to encounter conflicts.

   Regarding the timing issue, you could consider updating the LastTransitionTime only when the status actually changes, instead of every time the health check is done. This would mean the LastTransitionTime reflects when the status last changed, rather than the last time the check was performed.

   One thing to keep in mind is that frequent updates to the status subresource, even if the status does not change, can cause unnecessary API server load. You should strive to update the status only when it changes.

A possible updated version of your checkHealth function considering those points could be:

func (r *ebdReconciler) checkHealth(ctx context.Context, req ctrl.Request, ebd ebdmanv1alpha1.ebd) (bool, error) {
    vfmReady, err := r.mbr.IsReady(ctx, req.Name, req.Namespace)
    condition := metav1.Condition{
        Type:   ebdmanv1alpha1.KubernetesvfmHealthy,
        Status: metav1.ConditionUnknown, // start with unknown status
    }

    latestebd := ebdmanv1alpha1.ebd{}
    if err := r.Get(ctx, req.NamespacedName, &latestebd); err != nil {
        return vfmReady, err
    }
    oldCondition := meta.FindStatusCondition(latestebd.Status.Conditions, ebdmanv1alpha1.KubernetesvfmHealthy)

    if err != nil {
        // There was an error checking readiness - Set status to false
        condition.Status = metav1.ConditionFalse
        condition.Reason = ebdmanv1alpha1.ReasonError
        condition.Message = fmt.Sprintf("Failed to check  vfm readiness: %v", err)
    } else if vfmReady {
        // The vfm is ready - Set status to true
        condition.Status = metav1.ConditionTrue
        condition.Reason = ebdmanv1alpha1.ReasonReady
        condition.Message = "vfm custom resource is ready"
    } else {
        // The vfm is not ready - Set status to false
        condition.Status = metav1.ConditionFalse
        condition.Reason = ebdmanv1alpha1.ResourceProgressing
        condition.Message = "vfm custom resource is not ready"
    }

    // Only update the LastTransitionTime if the status has changed
    if oldCondition == nil || oldCondition.Status != condition.Status {
        condition.LastTransitionTime = metav1.Now()
    } else {
        condition.LastTransitionTime = oldCondition.LastTransitionTime
    }

    meta.SetStatusCondition(&latestebd.Status.Conditions, condition)

    if oldCondition != nil && condition.Status == oldCondition.Status && condition.Reason == oldCondition.Reason && condition.Message == oldCondition.Message {
        return vfmReady, nil
    }

    // Retry on conflict
    retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
        // Retrieve the latest version of ebd before attempting update
        // RetryOnConflict uses exponential backoff to avoid exhausting the apiserver
        if getErr := r.Get(ctx, req.NamespacedName, &latestebd); getErr != nil {
            return getErr
        }
        if updateErr := r.Status().Update(ctx, &latestebd); updateErr != nil {
            return updateErr
        }
        return nil
    })

    if retryErr != nil {
        r.Log.Error(retryErr, "Failed to update vfm status after retries")
        return vfmReady, retryErr
    }

    return vfmReady, nil
}

In this updated version:

• The LastTransitionTime field is updated only when the condition's status changes. This will ensure that the LastTransitionTime accurately reflects when the status was last changed rather than when the checkHealth function was last run. This should provide a more accurate timeline of when the resource's status actually changed, rather than when the reconciliation loop was run.

• A retry mechanism is added using retry.RetryOnConflict to re-attempt the status update when a conflict error occurs. Note that you'll need to import the "k8s.io/client-go/util/retry" package for this.
  This is a common pattern for dealing with the Operation cannot be fulfilled... error.

These changes should help to address the issues you were facing with updating the status and conditions of your Kubernetes resources.
Remember that you may still occasionally get conflict errors, especially if there are other clients updating the same object. In these cases, the RetryOnConflict function will retry the update with the latest version of the object.