StatefulSet using OnDelete update strategy have stale currentRevision

[ebensom]

The vmoperator manages the StatefulSets belonging to VMClusters and VMAlertmanagers using OnDelete updateStrategy.type. In such case, K8s statefulset controller does not update the status.currentRevision to the value of status.updateRevision when the last pod was successfully deleted and hence updated to the current controllerrevision. This cause stale information to be exposed in the status block, and moreover the currentReplicas to be missing:

status:
  availableReplicas: 6
  collisionCount: 0
  currentRevision: vmstorage-skr-697c996f7b
  observedGeneration: 38
  readyReplicas: 6
  replicas: 6
  updateRevision: vmstorage-skr-74c4bd7697
  updatedReplicas: 6

vmoperator could patch the statefulset/status subresource to have the currentRevision equal to the updateRevision when the last pod is deleted.

[Haleygo]

Hi @ebensom ,
Could you elaborate why you want to update the currentRevision to updateRevision?
Or, why not set updateStrategy: RollingUpdate if you need the correct currentRevision?
As I can see, k8s doesn't attempt to update it for statefulset with updateStrategy: OnDelete.

[ebensom]

Hi @Haleygo ,

You are right, I see the updateStrategy of vmstorage, vmselect, etc. can be changed. May I ask why the default OnDelete strategy was chosen, i.e. why vmoperator handles manually the statefulsert pod deletions during reconciliation?

Yes, I'm aware of the long outstanding kubernetes/kubernetes#106055 issue, and there the consensus is that K8s will not (and never in the future) update the currentRevision for when updateStrategy: OnDelete is used. Hence my original question/suggestion, because when it comes to the statefulsets owned by CRs managed by vmoperator, this seems like the responsibility of the vmoperator to patch the currentRevision when updateStrategy: OnDelete is used.

However if you say it's absolutely OK to use RollingUpdate strategy with VMCluster components instead of the default OnDelete, we will adjust accordingly.

[Haleygo]

May I ask why the default OnDelete strategy was chosen, i.e. why vmoperator handles manually the statefulsert pod deletions during reconciliation?

You can find the differences in rolling updates between vm-operator and Kubernetes at #389 (comment).
Basically, we think vm-operator can do better than kubernetes, and we allow user to choose.

Yes, I'm aware of the long outstanding kubernetes/kubernetes#106055 issue, and there the consensus is that K8s will not (and never in the future) update the currentRevision for when updateStrategy: OnDelete is used. Hence my original question/suggestion, because when it comes to the statefulsets owned by CRs managed by vmoperator, this seems like the responsibility of the vmoperator to patch the currentRevision when updateStrategy: OnDelete is used.

I'm unsure if the currentRevision needs to be accurate in this case, like you said, Kubernetes allows it to be incorrect for updateStrategy: OnDelete. That's why I asked why you want it updated.
If we update the currentRevision, it must happen after all pods finish updating. It requires an additional update call and we try to avoid unnecessary update to ease pressure on API server.