Make status compatible with kstatus #325
Conversation
There was a problem hiding this comment.
Based on the documentation, the Stalled state must be complimented with a Reconciling condition (== what we call "Progressing" at the moment).
It looks like this technically also makes it possible to drop the Ready condition as it seems to be that Ready == !Stalled && !Reconciling. For backward compatibility reasons, and because of the limited adoption of kstatus, I would however still keep the condition.
|
To add to my review comment:
|
Signed-off-by: Somtochi Onyekwere <somtochionyekwere@gmail.com>
Signed-off-by: Somtochi Onyekwere <somtochionyekwere@gmail.com>
Signed-off-by: Somtochi Onyekwere <somtochionyekwere@gmail.com>
| // SetKustomizeStalled sets the StalledCondition, ObservedGeneration on the Kustomization. | ||
| func SetKustomizationStalled(k *Kustomization, status metav1.ConditionStatus, reason, message string) { | ||
| meta.SetResourceCondition(k, meta.StalledCondition, status, reason, trimString(message, MaxConditionMessageLength)) | ||
| k.Status.ObservedGeneration = k.Generation |
There was a problem hiding this comment.
Should this be set on Stalled, or on reaching Ready? Or do we want to record it in the ObservedGeneration field of the metav1.Condition Status, but only move the one in the Status on a successful run?
There was a problem hiding this comment.
kstatus seems to return an InProgress status once status.ObserveredGeneration != metadata.generation regardless of the conditions present. So I thought we would want to set just incase,
There was a problem hiding this comment.
Good catch! Which brings us to a new question: does every error mean that the reported condition status is Stalled, and kstatus should thus stop waiting because ObservedGeneration == Generation or Status == Stalled?
If we for example run into a transient error (e.g. transient resource lookup failure that returned an error from the K8s API server). This would mean that kstatus would stop waiting for the resource to reach the expected state if we would set both ObservedGeneration and Stalled, while if we would set Ready==False or left the ObservedGeneration untouched, it would continue to wait for the next reconciliation to (hopefully) succeed: https://github.com/kubernetes-sigs/cli-utils/blob/e351b2bc43cec2107ba1d874c3dec54fd0956c59/pkg/kstatus/status/generic.go#L39-L54
There was a problem hiding this comment.
If we for example run into a transient error (e.g. transient resource lookup failure that returned an error from the K8s API server).
Hmm. We probably have to identify transient errors and real errors, then decide how best to handle the transient. I think we should set it Reconciling status and Ready=false for transient errors. kstatus takes ObservationGeneration != generated to mean that the controller might not be running or unable to update the resource.
Plus a generic message is set when ObservationGeneration != generated. If we use Reconciling condition, kstatus sets the reason and message from the condition .
Signed-off-by: Somtochi Onyekwere <somtochionyekwere@gmail.com>
|
This can be closed in favor of #347 indeed. |
This PR projects the
Readystatus condition toReconcilingandStalledfor better kstatus compliance.Signed-off-by: Somtochi Onyekwere somtochionyekwere@gmail.com