typo

Disruption in Deployments and DaemonSets is controlled by a field in the Strategy of each controller: MaxUnavailable. There has been discussion (kubernetes/kubernetes#35318) about unifying that field with PDB but I don't think there is any consensus for this one yet. Based on kubernetes/kubernetes#35318 (comment) and kubernetes/kubernetes#35318 (comment) I think there is room for both.

The only issue for StatefulSet is that with PDBs if we ever want to move faster than one Pod at a time we will have to create evictions instead of deleting Pods.

Does this mean that other parts of the PodTemplateSpec will be gated?

I think that for a first pass we should support just the above and then (potentially) relax the allowable mutable portions of .Template. Right now you can update Replicas and Containers. I think that is good enough to support configuration, resources, and image updates. Granted, you can cause an update by adding some arbitrary annotation to a Container.

Hrm - I'm having trouble coming up with a reason to gate template mutation, since we don't do it on any other "updatable" controller.

The current behavior will fail mutation on validation if anything other than Replicas and Containers is updated. We can change it to open it up in a backward compatible way.

The current behavior will fail mutation on validation if anything other than Replicas and Containers is updated. We can change it to open it up in a backward compatible way

What I mean is that allowing for more mutable fields can not break someones existing StatefulSet's. However, once me make something mutable, we can't take it back without breaking backward compatibility.

Why would we want to take it back? Eventually a StatefulSet should be updatable as any other controller.

If allowed Volumes to be updated, what would that semantically mean ? Is that an identity change ?

The changes we are considering now would be mutation to the VolumeClaimsTemplate with respect to resizing the storage. It would not be an identity change because we would not mutate the contents of the storage.

Can we clarify here that the existing validation that is applied in the StatefulSet pod template will be lifted?

CurrentTemplateRevision

Is this a usability issue with Deployments that we need to fix?

Is this a usability issue with Deployments that we need to fix?

The fact that we don't output the revisions in the status, both the one we roll from and the one we roll to.

Hrm - why wouldn't this be generation?

@smarterclayton Daemon set uses TemplateGeneration. I will update the proposal such that StatefulSet will use the same.
@Kargakis I would think that, for all controllers, we would want to express the full current and target state as part of the status, but that its more important for StatefulSet than for Deployment.

RevisionHistoryLimit to be consistent with Deployments?

Makes sense

typos

Can you explain why is this field needed? Is it for running canaries? We want something similar for the rest of the workload controllers but it's going to differ from this one.

Its for canaries and staged roll outs. I think StatefulSet should be different from Deployments and ReplicaSets in this regard. Deployments and ReplicaSets are probabilistic in nature and StatefulSets are ordered and deterministic. I think that specifying a percentage makes more sense for Deployments and ReplicaSets while partitioning a StatefulSet based on ordinal makes the most sense. For a Deployment or ReplicaSet I want to say give me x% with a target configuration. For a StatefulSet I want to say update these members (which in this case live in a contiguous partition).

This feels like it is part of a strategy struct (like deployments). Can we imagine a different approach to update in the future? Custom rollout strategies? If so a strategy struct is a good hedge.

That has precedence in DaemonSet, and it is a small level of effort for the benefit of the hedge

Why not rely on owner references?

To select them. OwnerReference is needed as well to avoid revision history overlap.

What if you apply the same labels to the PodTemplates that you do to your Pods, then you select them with your normal controller selector, and filter by ControllerRef to handle overlapping selectors?

I was assuming @enisoc's suggestion. +1 on that

SGTM

Shouldn't we rely on the StatefulSet PodTemplateSpec for the desired state? Are PodTemplates going to be read-only?

In essence we still will. The target revision of the StatefulSet's history will be semantically, deeply equivalent to its Spec.Template with the exception that it will carry the annotation for revision history that is applied to the PodTemplate. That is reason that the PodTemplate and not the Spec.Template will be used for creation.

When you say , confirm to exactly one or two PodTemplates you mean two because some pods can be with old template and some can be with new template ?

No. A Pod conforms to exactly one of two templates.

@kow3ns are you talking about the canary case ?

typo

This doesn't feel right. How is the controller creating the PodTemplate in the first place?

The controller generates PodTemplates based on its current Template, when modified, in order to keep track of the modifications made to the Template. The idea is that for every revision to the StatefulSet's .Spec.Template we will generate a new PodTemplate that represents that particular revision and add it to the containers revision history.

The way this point is worded, it seems to me that the controller is mutating the StatefulSet.

Nevermind, you are using this wording elsewhere. I can get used to it:)

I will make this more clear

I think that there should be at least a link between the Template Update and PodTemplate Creation sections to make it clear that a Template Update is what triggers PodTemplate Creation.

Why the owner ref from the previous step is not enough and an additional label is needed?

Are you suggesting that we just list all PodTemplates and then filter by OwnerRef?

Yes, basically that's what @enisoc did for all the rest of the controllers.

I have no objection to doing it that way. It seems more efficient to select based on a label to get only the subset that I care about, and then filter those based on ControllerRef. Is there a benefit to listing all PodTemplates? The only reason I can think of to select them all is to continue to consider PodTemplates that have had the label intentionally removed by the user, but if the user intentionally removes the label isn't she indicating by explicit action that the PodTemplate should be removed?

In case a PodTemplate A-1 is not selectable by a StatefulSet A anymore but A-1 has an owner reference to A, the StatefulSet controller needs to remove that owner reference.
https://github.com/kubernetes/community/blob/31d62c70214d66ca2c21716c02b83d9087fdf3ff/contributors/design-proposals/controller-ref.md#orphaning

The point above still applies. The only way this should happen is if the user explicitly mutates the label applied to the PodTemplate, or, if StatefulSet.Spec.Selector becomes mutable, by mutating the selector. Are we worried about orphan's occurring due to unintentional user error? If so, lets not make the selector mutable.
Also, I was not planning on having controllers adopt PodTemplates in the same way they adopt orphan Pods. A controller taking ownership of an orphaned Pod makes sense, a controller taking ownership of another controllers revision history, imo, does not. I don't think we need to solve for the same constraints as we did for Pods.

Shouldn't this be handled by the GC?

I wasn't sure if GC would work for PodTemplates, but it appears that it will, provided the OwnerReference is set. I will update to indicate that we will use GC. That is the direction I wanted to go eventually.

On second thoughts, PodTemplates is not really a resource on the api server right, its just an array/field inside StatefulSet Spec. Not sure if GC will handle this @caesarxuchao ?

I have the same question @krmayankk asked, isn't PodTemplate a field?

OwnerReferneces is a field in Objectmeta, which is only included in each top-level API object.

PodTemplates are top-level objects, but they are not used anywhere in Kubernetes today. PodTemplateSpec is the field that every controller has.

Thanks. It's good to know. Then GC can handle PodTemplate.

This may be confusing because once a StatefulSet is updated and a new PodTemplate is created, CurrentReplicas is really NeedToBeReplacedReplicas. I can't think of a better name now.

I couldn't think of a better name either, but I'm very open to suggestion here. Also, do you have thoughts on the naming of PartitionRevision? I'd be happy to have a better name for this also.

UpdatedReplicas to stay consistent with what we have in Deployments?

SGTM

typo

Which component is going to increase this field?

API Server

Why isn't this a generation?

Will update for consistency with DaemonSet.

In DeamonSet we check if pod is Available (Ready + MinReadySeconds)

Is that done primarily to tolerate infant mortality?

You may want to read #478

Basically I am fine with not handling MinReadySeconds for StatefulSets since we are moving it in the PodSpec. Eventually, we should add the additional status field (StableReplicas).

Yeah stable pods will be supported, but I didn't think adding that to StatefulSetStatus was relevant for this proposal. You already have an issue open for consistent Status objects for controllers, and I thought that work could be performed under that issue, or a StatefulSet specific sub issue.

Is this a final decision? In DaemonSets we didn't decided yet how to store a history. cc @Kargakis @janetkuo @smarterclayton

Yeah, we haven't yet. I think we need to get a quick list of options written up and discussed:

1. PodTemplate (collision between use for different controller types)
2. Specific type for each controller: DaemonSetHistory, StatefulSetHistory, etc (no reuse between controllers)
3. Storing a limited subset of specs on the object (size limits)
4. ???

I'm obviously partial to 1. My primary reason is that controllers should not be special and require magic. If we get TPRs right then PodTemplates can be leveraged by arbitrary operators/controllers, and they, therefore, provide a primitive upon which others can build.

So for pod template here's what we need to handle:

1. Collisions across controllers by naming
2. Idempotency of product versions (the same pod template in a controller object should map to exactly one PodTemplate across multiple upgrades)
3. Human readability - a human should be able to find a PodTemplate
4. Collisions across controllers by pod template spec - if two controllers deploy the exact same pod template (daemon set vs stateful set) the PodTemplate has two owners, not just one, so garbage collection won't work
5. Future versions of controllers may involve renames of the controller resource name, so we would still have to be able to locate old PodTemplate revisions even if the resource name changes
6. Immutability of PodTemplate - a template used in this fashion needs to prevent updates, or at least strongly discourage them (if they are mutable, we can't compare the pods against the pod template version).

I think the real issue is 1 - the rest of the problems apply to ReplicaSets owned by Deployments too.

For clarity, I am talking about the UID of the parent.

@smarterclayton @janetkuo @Kargakis @lukaszo

Before we go forward with PodTemplate creation we should also discuss adding a History sub resource to the controllers (I believe that is something like option 3 above).

Having a per Kind History sub resource mitigates History overlap across Kinds with the same .Name in the same namespace. My argument about PodTemplate being a primitive that is usable without controller specific magic is valid (imo), but perhaps its also a straw man.

Right now the category of controllers could loosely be defined like this.
A generative controller takes, as an argument, an Object that represents the declared target state. The Object must have the following properties

1. A .Spec.Template that is a template for Objects of the generated Kind.
2. A .Spec.Replicas that expresses the desired cardinality of the generated Kind.
3. A Generation that indicates the version of the declared target state.
4. A .Status.Replicas that indicates the number of created replicas.
5. If the Objects of created Kind are not immediately available upon creation, a .Status.ReadyReplicas to indicate the number of available created resources.
   1. A .Satus.ObservedGeneration that indicates the version of the target state specification that the controller has observed.

I confirmed with @foxish that the plan to support third party controller and operators is to make these properties well supported. That is, we want them to create custom Kinds that contain these sub resources. Given that, adding a History sub resource might be the best fit.

We could say that controllers that implement revision history must have

1. A History.Revision where the type of this object is an []PodTemplate or []PodTemplateSpec.
2. A History.RevisionLimit that indicates the number of stored revisions.
3. A Spec.TemplateGeneration that is incremented by the API server upon mutation of the .Spec.Template.

There has been some discussion before and I think there is general agreement that we want a history subresource. I am not sure whether we need to have this discussion in this proposal or in a separate one since it 1. affects all of the workload controllers and 2. is not required for having StatefulSet upgrades.

I don't have a problem with putting uids in names of generated resources provided they are ONLY used for uniquification and NEVER used for discovery/searching.

Prevention of mutation and deletion is also desired in other contexts (e.g., addons): kubernetes/kubernetes#10179

This applies to deployments as well - maybe indicate it's more likely, but that we're doing exactly what deployments do here.

SGTM

Does this mean unlike Deployments, where you can rollback to any revision you want, StatefulSet can only be rolled back to the previous version only ?