Mimic LeaderWorkerSet status as StatefulSet or Deployment

[liurupeng]

For rolling update related status' fields, should we mimic Statefulset or Deployment. My prefer using the StatefulSet one, since it could provide more information.

[liurupeng]

@ahg-g @kerthcet any thoughts

[ahg-g]

For rolling update related status' fields, should we mimic Statefulset or Deployment

+1, we should have this.

[liurupeng]

but do you think we should have the same one as StatefulSet or Deployment though @ahg-g

[ahg-g]

can you describe what each one covers?

[kerthcet]

We're now quite similar to deployment. Glad to have one observedGeneration to reflect the current generation.

[liurupeng]

In StatefulSet, Link, there are ObservedGeneration, CurrentReplicas, UpdatedReplicas, CurrentRevision, UpdatedRevision etc.
In Deployment, Link, there are ObservedGeneration, UpdatedReplicas.

Both could be supported which we could change the CurrentRevision to CurrentTemplateHash or remove that. I mainly want to support the CurrentReplicas and UpdatedReplicas, for now, the UpdatedReplicas==Lws.Spec.Replicas when upgrade finished which make it hard to calculate the upgrade latency (the same in deployment). In statefulSet, when upgrade finished, UpdatedReplcias == 0 and currentReplicas==lws.Spec.Replicas. This should give more information as well for many cases

[liurupeng]

@ahg-g and @kerthcet thoughts?

[ahg-g]

Adding currentReplicas and changing the semantics of updatedReplicas sounds good to me.

[ahg-g]

but we need to do that before the release

[liurupeng]

I'll submit a change today @ahg-g

[liurupeng]

/assign

[liurupeng]

actually it's hard to calculate currentReplicas if we don't keep a record of the current template hash somewhere. Does it make sense to only update the semantics of updatedReplicas if we don't add the currentReplicas though

[kerthcet]

In statefulSet, when upgrade finished, UpdatedReplcias == 0 and currentReplicas==lws.Spec.Replicas.

I rechecked, but the updatedReplicas == spec.Replicas. But when rolling update starts, the updatedReplicas will change to 0.

[kerthcet]

If we set the updatedReplicas to 0 when rolling update finished, then if we have a sts of replicas-10, when two replicas down, what's the value of updatedReplicas right now? eight? then it seems a little weird .. since we jumped from 0 to 8 then to 0 again.

calculate the upgrade latency

Can we start when updatedReplicas changes to 0? and end when updatedReplicas = spec.Replicas. I didn't think too deep about this, have no idea whether this works or not.

[nayihz]

If we set the updatedReplicas to 0 when rolling update finished, then if we have a sts of replicas-10, when two replicas down, what's the value of updatedReplicas right now? eight?

Yes. updatedReplicas will change when pod restart.

calculate the upgrade latency

There are currentRevision and updateRevision in statefulSet to record the template hash. Maybe we can use them to calculate the upgrade latency.

[kerthcet]

We decided to add a new condition type or progressing reason for record.

[ahg-g]

I just noticed that UpdatedReplicas and Replicas semantics are not consistent with Statefulset.

In LWS, UpdatedReplicas is the number of replicas that are ready and updated, while in StatefulSet it is the number of updated replicas whether they are ready or not (i.e., created, but not necessarily ready).

In LWS, Replicas is the number of replicas we aim to create (because we set it to leaderSts.Spec.Replicas); while in StatefulSet it is the number of Replicas we created so far (ready or not). I think we can fix this by setting it to leaderSts.Status.Replicas instead of leaderSts.Spec.Replicas

[kerthcet]

Both make sense.