Re: [kubernetes/community] controller history (#594)

[k8s-ci-robot]

@kow3ns: These labels do not exist in this repository: sig/api-machinery, sig/apps.

In response to this:

@kubernetes/sig-api-machinery-api-reviews
@kubernetes/sig-apps-api-reviews
@smarterclayton
@kargakis
@janetkuo
@enisoc
@foxish
@erictune
@bgrant0607

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[Kenneth Owens]

@kubernetes/sig-api-machinery-api-reviews
@kubernetes/sig-apps-api-reviews
@smarterclayton
@kargakis
@janetkuo
@enisoc
@foxish
@erictune
@bgrant0607

—

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> @@ -0,0 +1,447 @@
+# Controller History
+
+**Author**: kow3ns@
+
+**Status**: Proposal
+
+## Abstract 
+In Kubernetes, in order to update and rollback the configuration and binary 
+images of controller managed Pods, users mutate DaemonSet, StatefulSet, 
+and ReplicaSet (via Deployment) Objects, and the corresponding controllers 

We don't mutate ReplicaSets*, we create new copies when the Deployment is mutated.

• apart from syncing annotations and spec.MinReadySeconds between the two objects

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +1. API Server
+1. Kubectl
+1. Controllers that utilize the feature
+
+## Requirements
+
+1. History is a collection of points in time, and each point in time must be 
+represented by its own Object. While it is tempting to aggregate all of an 
+Object's history into a single container Object, experience with Borg and Mesos 
+has taught us that this inevitably leads to exhausting the single Object size 
+limit of the system's the storage backend. 
+1. We must be able to select the Objects that contain point in time snapshots 
+of versions of an Object to reconstruct the Object's history.
+1. History respects causality. The Object type used to store point in time 
+snapshots must be strictly ordered with respect to creation. CreationTimestamp 
+should not be used, as this is susceptible to clock skew.

Is clock skew between multiple masters in this case?

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +    controllers.
+    - PersistentVolumeClaim is also a generated type for the StatefulSet 
+    controller.
+- The current state of a controller is the union of the states of its generated 
+Objects along with its status.
+    - For ReplicaSet, DaemonSet, and StatefulSet, the current state of the 
+    corresponding controllers can be derived from Pods they contain and the 
+    ReplicasSetStatus, DaemonSetStatus, and StatefulSetStatus objects 
+    respectively.
+- For all specification type Objects for controllers, the target state is the 
+set of fields in the Object that determine the state to which the controller 
+attempts to evolve the system.
+    - This may not necessarily be all fields of Object.
+- The target Object state is the subset of the target state necessary to create 
+a Objects of the generated type(s). 
+- To make the this concrete, for the StatefulSet controller:

typo

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +[reconstruct the Object's revision history](#history-reconstruction).
+    - Note that the process of reconstructing the Object's history filters any
+    AnyStates not owned by the Object.
+1. The controller will filter any AnyStates that represent a live version.
+1. If the number of remaining AnyStates is greater than the configured 
+`RevisionHistoryLimit`, the controller will delete them, in order with respect 
+to the value mapped to their `"ControllerGeneration"` keys, until the number 
+of remaining AnyStates is equal to the `RevisionHistoryLimit`.
+
+This ensures that the number of recorded, non-live revisions is less than or 
+equal to the configured `RevisionHistoryLimit`.
+
+### Version Tracking
+Controllers must track the version of the target Object state that corresponds 
+to their generated Objects. This information is necessary to determine which 
+versions are live, and to track which Objects need to be updated during a 

What do you mean by "and to track which Objects need to be updated during a target state update or rollback"?

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +[reconstruct the Object's revision history](#history-reconstruction).
+    - Note that the process of reconstructing the Object's history filters any
+    AnyStates not owned by the Object.
+1. The controller will filter any AnyStates that represent a live version.
+1. If the number of remaining AnyStates is greater than the configured 
+`RevisionHistoryLimit`, the controller will delete them, in order with respect 
+to the value mapped to their `"ControllerGeneration"` keys, until the number 
+of remaining AnyStates is equal to the `RevisionHistoryLimit`.
+
+This ensures that the number of recorded, non-live revisions is less than or 
+equal to the configured `RevisionHistoryLimit`.
+
+### Version Tracking
+Controllers must track the version of the target Object state that corresponds 
+to their generated Objects. This information is necessary to determine which 
+versions are live, and to track which Objects need to be updated during a 

History objects are immutable, pods are created or deleted, parent objects are updated by users.

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +    - This method has the benefit of providing visibility, via the label, to 
+    users with respect to the historical providence of a generated Object.
+    - The primary drawback is the lack of support for using garbage collection 
+    to ensure that only non-live version snapshots are collected.
+1. Controllers may also use the `OwnerReferences` field of the AnyState to 
+record all Objects that are generated from target Object state version 
+represented by the AnyState as its owners.
+   - A revision is considered to be live while any generated Object that owns 
+   it is live.
+   - This method allows for the implementation of generic garbage collection.
+   - The primary drawback with this method is that the book keeping is complex, 
+   and deciding if a generated Object corresponds to a particular revision 
+   will require testing each Object for membership in the `OwnerReferences` 
+   of all AnyStates.
+   
+At the cost of the complexity of implementing both labeling and ownership, 

Ownership is based on labeling. If an owner cannot match a child, the child is orphaned. If an owner matches an object that is not a child, the object is adopted.

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +not require cryptographically strong collision resistance, and given we use a 
+[collision resolution](#collision-resolution) technique, we can't use the 
+[generated names](#unique-name-generation) of AnyStates to decide equality.
+
+We propose that two AnyStates can be considered equal if their `.Data` is 
+equivalent, but that it is not sufficient to compare the serialized 
+representation of the their `.Data`. Consider that the addition of new fields 
+to the Objects that represent the target Object state may cause the serialized 
+representation of those Objects to be unequal even when they are semantically 
+equivalent. We propose two methods that controllers may use to decide the 
+equivalence of two versions of their target Object state.
+
+1. The controller may deserialize the values of the AnyStates representing their 
+target Object state and perform a deep, semantic equality test. Here all 
+differences that do not constitute a mutation to the target Object state are 
+disregarded during the equivalence test.

Here all differences that do not constitute a mutation to the target Object state are disregarded during the equivalence test.

How is this going to be done? Or are we purely based on defaulting filling out any new fields that may have been introduced since we serialized the template?

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +## Kubectl
+
+Modifications to kubectl to leverage controller history are an optional 
+extension. Users can trigger rolling updates and rollbacks by modifying their 
+manifests and using `kubectl apply`. Controllers will be able to detect 
+revisions to their target Object state and perform 
+[reconciliation](#target-object-state-reconciliation) as necessary.
+
+### Viewing History 
+As controllers can use strategic merge patching as a method for 
+[version equivalence](#version-equivalence) testing, so can clients use it as a 
+method for reconstructing specification type Objects at a particular revision. 
+We propose the following command.
+
+```bash
+> kubectl history <controller-kind> <controller-name>

There is already a kubectl rollout history command.

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +contained in the AnyState with specification type Object in the controller's 
+manifest and to output the resulting manifest.
+
+- This output manifest represents the scale invariant application of a prior 
+target Object state to the current specification type Object.
+- This approach only works because of the encoding we propose for 
+[version snapshot construction](#version-snapshot-creation). Controllers that 
+use a different encoding will not be able to use this feature.
+
+### Rollback
+
+For future work, `kubeclt rollback` can be implemented in the general case as an 
+extension of the [above](#viewing-history ).
+
+```bash
+> kubectl rollback <controller-kind> <controller-name> --revision <any-state-name>

We already have a kubectl rollout undo command.

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +value of `n`. We define a third function 
+`func H''(s string, exists func (string) bool)(int,[HashSize]byte)`. `H''` 
+will start with `n := 0` and compute `s' := H'(s,n)`, incrementing `n` when 
+`exists(s')` returns true, until `exists(s')`  returns false. After this it will 
+return `n,s'`.
+
+For our purposes, the implementation of the `exists` function will attempt to 
+create a `.Named` AnyState via the API Server using a 
+[unique name generation](#unique-name-generation). If creation fails, due to a 
+conflict, the method returns false.
+
+### Unique Name Generation
+We can use our [hash function](#hashsing) and 
+[collision resolution](#collision-resolution) scheme to generate a system 
+wide unique identifier for an Object based on a deterministic non-unique prefix 
+and a serialized representation of the Object. Kuberentes Object's `.Names` must 

Do you mean .Name?

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +return `n,s'`.
+
+For our purposes, the implementation of the `exists` function will attempt to 
+create a `.Named` AnyState via the API Server using a 
+[unique name generation](#unique-name-generation). If creation fails, due to a 
+conflict, the method returns false.
+
+### Unique Name Generation
+We can use our [hash function](#hashsing) and 
+[collision resolution](#collision-resolution) scheme to generate a system 
+wide unique identifier for an Object based on a deterministic non-unique prefix 
+and a serialized representation of the Object. Kuberentes Object's `.Names` must 
+conform to a DNS subdomain. Therefore, the total length of the unique identifier
+must not exceed 255, and in practice 253, characters. We can generate a unique 
+identifier that meets this constraint by selecting a hash function such that the 
+output length is equal to `253-len(prefix)` and applying our [hash](#hashing) 

This is handling names for history resources. In the case of Deployments, the name of the history object (ReplicaSet) is used in generating the name of the Pods. So I guess for Deployments, this needs to differ slightly.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +1. API Server
+1. Kubectl
+1. Controllers that utilize the feature
+
+## Requirements
+
+1. History is a collection of points in time, and each point in time must be 
+represented by its own Object. While it is tempting to aggregate all of an 
+Object's history into a single container Object, experience with Borg and Mesos 
+has taught us that this inevitably leads to exhausting the single Object size 
+limit of the system's the storage backend. 
+1. We must be able to select the Objects that contain point in time snapshots 
+of versions of an Object to reconstruct the Object's history.
+1. History respects causality. The Object type used to store point in time 
+snapshots must be strictly ordered with respect to creation. CreationTimestamp 
+should not be used, as this is susceptible to clock skew.

Multiple API servers is my concern yes.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +[reconstruct the Object's revision history](#history-reconstruction).
+    - Note that the process of reconstructing the Object's history filters any
+    AnyStates not owned by the Object.
+1. The controller will filter any AnyStates that represent a live version.
+1. If the number of remaining AnyStates is greater than the configured 
+`RevisionHistoryLimit`, the controller will delete them, in order with respect 
+to the value mapped to their `"ControllerGeneration"` keys, until the number 
+of remaining AnyStates is equal to the `RevisionHistoryLimit`.
+
+This ensures that the number of recorded, non-live revisions is less than or 
+equal to the configured `RevisionHistoryLimit`.
+
+### Version Tracking
+Controllers must track the version of the target Object state that corresponds 
+to their generated Objects. This information is necessary to determine which 
+versions are live, and to track which Objects need to be updated during a 

If I have 10 Pods, and five of them are at a semantically equivalent version to my current target I only want to restart the 5 that are not. The other 5 may need to be relabeled.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +    - This method has the benefit of providing visibility, via the label, to 
+    users with respect to the historical providence of a generated Object.
+    - The primary drawback is the lack of support for using garbage collection 
+    to ensure that only non-live version snapshots are collected.
+1. Controllers may also use the `OwnerReferences` field of the AnyState to 
+record all Objects that are generated from target Object state version 
+represented by the AnyState as its owners.
+   - A revision is considered to be live while any generated Object that owns 
+   it is live.
+   - This method allows for the implementation of generic garbage collection.
+   - The primary drawback with this method is that the book keeping is complex, 
+   and deciding if a generated Object corresponds to a particular revision 
+   will require testing each Object for membership in the `OwnerReferences` 
+   of all AnyStates.
+   
+At the cost of the complexity of implementing both labeling and ownership, 

It has special semantics and, afiak, you can't select based on it can you?

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +value of `n`. We define a third function 
+`func H''(s string, exists func (string) bool)(int,[HashSize]byte)`. `H''` 
+will start with `n := 0` and compute `s' := H'(s,n)`, incrementing `n` when 
+`exists(s')` returns true, until `exists(s')`  returns false. After this it will 
+return `n,s'`.
+
+For our purposes, the implementation of the `exists` function will attempt to 
+create a `.Named` AnyState via the API Server using a 
+[unique name generation](#unique-name-generation). If creation fails, due to a 
+conflict, the method returns false.
+
+### Unique Name Generation
+We can use our [hash function](#hashsing) and 
+[collision resolution](#collision-resolution) scheme to generate a system 
+wide unique identifier for an Object based on a deterministic non-unique prefix 
+and a serialized representation of the Object. Kuberentes Object's `.Names` must 

Yes, but it is the plural possessive of the field.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +return `n,s'`.
+
+For our purposes, the implementation of the `exists` function will attempt to 
+create a `.Named` AnyState via the API Server using a 
+[unique name generation](#unique-name-generation). If creation fails, due to a 
+conflict, the method returns false.
+
+### Unique Name Generation
+We can use our [hash function](#hashsing) and 
+[collision resolution](#collision-resolution) scheme to generate a system 
+wide unique identifier for an Object based on a deterministic non-unique prefix 
+and a serialized representation of the Object. Kuberentes Object's `.Names` must 
+conform to a DNS subdomain. Therefore, the total length of the unique identifier
+must not exceed 255, and in practice 253, characters. We can generate a unique 
+identifier that meets this constraint by selecting a hash function such that the 
+output length is equal to `253-len(prefix)` and applying our [hash](#hashing) 

I think that Deployments should use the same concept where possible to deal with ReplicaSets, but, in this case, and in others, they will likely need special handling. Also, this is just to demonstrate that the approach is plausible, we will need to think about the naming implementation, and it will have to be based on the new hash function that will be used for deployment.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +not require cryptographically strong collision resistance, and given we use a 
+[collision resolution](#collision-resolution) technique, we can't use the 
+[generated names](#unique-name-generation) of AnyStates to decide equality.
+
+We propose that two AnyStates can be considered equal if their `.Data` is 
+equivalent, but that it is not sufficient to compare the serialized 
+representation of the their `.Data`. Consider that the addition of new fields 
+to the Objects that represent the target Object state may cause the serialized 
+representation of those Objects to be unequal even when they are semantically 
+equivalent. We propose two methods that controllers may use to decide the 
+equivalence of two versions of their target Object state.
+
+1. The controller may deserialize the values of the AnyStates representing their 
+target Object state and perform a deep, semantic equality test. Here all 
+differences that do not constitute a mutation to the target Object state are 
+disregarded during the equivalence test.

Deserialize each object and write a long, Java style, object equivalence comparison. Every time a new field gets added, update the imperative code that performs the comparison. As indicated below, this might get us off the ground but should probably not be the long term solution. Strategic merge patch is the way to go in the long run.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> @@ -0,0 +1,447 @@
+# Controller History
+
+**Author**: kow3ns@

+
+**Status**: Proposal
+
+## Abstract 
+In Kubernetes, in order to update and rollback the configuration and binary 
+images of controller managed Pods, users mutate DaemonSet, StatefulSet, 
+and ReplicaSet (via Deployment) Objects, and the corresponding controllers 

Yes. This is not clear. You mutate Deployments which causes replicaset's to be created, deleted, and updated. It is a more complicated workflow. I will update to reflect this.

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +- The specification type of a controller is the type that contains the 
+specification for the Objects generated by the controller.
+    - For example, the specification types for the ReplicaSet, DaemonSet, 
+     and StatefulSet controllers are ReplicaSetSpec, DaemonSetSpec, 
+     and StatefulSetSpec respectively.
+- The generated type(s) for a controller is/are the type of the Object(s) 
+generated by the controller. 
+    - Pod is a generated type for the ReplicaSet, DaemonSet, and StatefulSet 
+    controllers.
+    - PersistentVolumeClaim is also a generated type for the StatefulSet 
+    controller.
+- The current state of a controller is the union of the states of its generated 
+Objects along with its status.
+    - For ReplicaSet, DaemonSet, and StatefulSet, the current state of the 
+    corresponding controllers can be derived from Pods they contain and the 
+    ReplicasSetStatus, DaemonSetStatus, and StatefulSetStatus objects 

For history, we do care about the status fields ?

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +Mesos Replicate Log (A Multi-Paxos based state machine used by the Mesos 
+Masters). This State Abstraction is a mutable, durable dictionary where keys 
+and values are opaque strings. As controllers only need the capability to 
+persist an immutable point in time snapshot of target Object states to 
+implement a revision history, we propose to use an immutable durable dictionary, 
+and we introduce the AnyState object for this purpose.
+
+``` golang
+// AnyState implements an immutable dictionary of state data to allow for a 
+// point in time snapshot. Clients are responsible for serializing 
+// and deserializing the objects that contain their internal state and mapping 
+// them to a set of keys. 
+// Once an AnyState has been successfully created, it can not be updated. The 
+// API Server will fail validation of all update requests that attempt to mutate 
+// the Data field. AnyStates may, however, be deleted.
+type AnyState struct {

Do you intend to use it for purposes other than history ? AnyState is too generic a name

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +can use AnyState to persist a history of revisions to its specification type 
+Objects. The technique is applicable, without loss of generality, to the 
+existing Kuberentes controllers that have Pod as a generated type.
+
+When a controller detects a revision to the target Object state of a 
+specification type Object it will do the following.
+
+1. The controller will [create a snapshot](#version-snapshot-creation) of the 
+current target Object state.
+1. The controller will [reconstruct the history](#history-reconstruction) of 
+revisions to the Object's target Object state.
+1. The controller will test the current target Object state for 
+[equivalence](#version-equivalence) with all other versions in the Object's 
+revision history.
+    - If the current version is semantically equivalent to its immediate 
+    predecessor no update to the Object's target state has been performed.

how do you determine the immediate predecessor ?

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +The API Server must support the creation and deletion of AnyState objects.
+As we have no mechanism for declarative immutability, the API server must fail 
+any update request that updates the `.Data` field of an AnyState Object.
+
+## Controllers 
+This section is presented as a generalization of how an arbitrary controller 
+can use AnyState to persist a history of revisions to its specification type 
+Objects. The technique is applicable, without loss of generality, to the 
+existing Kuberentes controllers that have Pod as a generated type.
+
+When a controller detects a revision to the target Object state of a 
+specification type Object it will do the following.
+
+1. The controller will [create a snapshot](#version-snapshot-creation) of the 
+current target Object state.
+1. The controller will [reconstruct the history](#history-reconstruction) of 

this history reconstruction will not always happen , only when there exists no history right ?

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +
+1. The controller will [create a snapshot](#version-snapshot-creation) of the 
+current target Object state.
+1. The controller will [reconstruct the history](#history-reconstruction) of 
+revisions to the Object's target Object state.
+1. The controller will test the current target Object state for 
+[equivalence](#version-equivalence) with all other versions in the Object's 
+revision history.
+    - If the current version is semantically equivalent to its immediate 
+    predecessor no update to the Object's target state has been performed.
+    - If the current version is equivalent to a version prior to its immediate 
+    predecessor, this indicates a rollback.
+    - If the current version is not equivalent to any prior version, this 
+    indicates an update or a roll forward.
+    - Controllers should use their status objects for book keeping with respect 
+    to current and prior revisions.

Do you mean revision numbers should be kept in status fields ? Currently revisions are annotations in deployments

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +with the new target Object state.
+1. The controller will [maintain the length of its history](#history-maintenance) 
+to be less than the configured limit.
+
+### Version Snapshot Creation
+To take a snapshot of the target Object state contained in a specification type
+Object, a controller will do the following.
+
+1. The controller will serialize all the fields of the Object's target object 
+state using JSON encoding.
+1. The controller will populate the `.Data` of an AnyState object by 
+associating the serialized fields with keys that correspond to the fields' 
+names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.

Are you proposing we use the generation number of the objects as revision number and their monotonically increasing sequence as a way to determine the point in time in history . Just clarifying.

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +    applicable. 
+    - Alternatively, a Kind unique label may be set to the `.Name` of the 
+    specification type Object.
+1. The controller will add a ControllerRef indicating the specification type 
+Object as the owner of the AnyState in the AnyState's `.OwnerReferences`.
+1. The controller will use the hash from above, along with a user identifiable 
+prefix, to [generate a unique `.Name`](#unique-name-generation) for the AnyState.
+1. The controller will persist the AnyState via the API Server.
+    - Note that, in practice, creation occurs concurrently with 
+    [collision resolution](#collision-resolution).
+    
+### History Reconstruction
+To reconstruct the history of a specification type Object, a controller will do 
+the following. 
+
+1. Select all AnyState Objects labeled as described 

are these namespaced , sorry i missed that part , if you already discussed that earlier

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +1. The controller will persist the AnyState via the API Server.
+    - Note that, in practice, creation occurs concurrently with 
+    [collision resolution](#collision-resolution).
+    
+### History Reconstruction
+To reconstruct the history of a specification type Object, a controller will do 
+the following. 
+
+1. Select all AnyState Objects labeled as described 
+[above](#version-snapshot-creation).
+1. Filter any AnyStates that do not have a ControllerRef in their 
+`.OwnerReferences` indicating ownership by the Object.
+1. Sort the AnyStates by the value associated with the 
+`"ControllerGeneration"` key.
+1. This produces a strictly ordered set of AnyStates that comprises the ordered 
+revision history of the specification type Object.

it would be good to clarify, at what point/cases is the history construction necessary

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +
+1. The controller will 
+[reconstruct the Object's revision history](#history-reconstruction).
+    - Note that the process of reconstructing the Object's history filters any
+    AnyStates not owned by the Object.
+1. The controller will filter any AnyStates that represent a live version.
+1. If the number of remaining AnyStates is greater than the configured 
+`RevisionHistoryLimit`, the controller will delete them, in order with respect 
+to the value mapped to their `"ControllerGeneration"` keys, until the number 
+of remaining AnyStates is equal to the `RevisionHistoryLimit`.
+
+This ensures that the number of recorded, non-live revisions is less than or 
+equal to the configured `RevisionHistoryLimit`.
+
+### Version Tracking
+Controllers must track the version of the target Object state that corresponds 

By Version of the target, you mean revision of the target or APIVersion ?

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +[reconstruct the Object's revision history](#history-reconstruction).
+    - Note that the process of reconstructing the Object's history filters any
+    AnyStates not owned by the Object.
+1. The controller will filter any AnyStates that represent a live version.
+1. If the number of remaining AnyStates is greater than the configured 
+`RevisionHistoryLimit`, the controller will delete them, in order with respect 
+to the value mapped to their `"ControllerGeneration"` keys, until the number 
+of remaining AnyStates is equal to the `RevisionHistoryLimit`.
+
+This ensures that the number of recorded, non-live revisions is less than or 
+equal to the configured `RevisionHistoryLimit`.
+
+### Version Tracking
+Controllers must track the version of the target Object state that corresponds 
+to their generated Objects. This information is necessary to determine which 
+versions are live, and to track which Objects need to be updated during a 

i think my questions is answered, you mean revision not apiversion

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +[reconstruct the Object's revision history](#history-reconstruction).
+    - Note that the process of reconstructing the Object's history filters any
+    AnyStates not owned by the Object.
+1. The controller will filter any AnyStates that represent a live version.
+1. If the number of remaining AnyStates is greater than the configured 
+`RevisionHistoryLimit`, the controller will delete them, in order with respect 
+to the value mapped to their `"ControllerGeneration"` keys, until the number 
+of remaining AnyStates is equal to the `RevisionHistoryLimit`.
+
+This ensures that the number of recorded, non-live revisions is less than or 
+equal to the configured `RevisionHistoryLimit`.
+
+### Version Tracking
+Controllers must track the version of the target Object state that corresponds 
+to their generated Objects. This information is necessary to determine which 
+versions are live, and to track which Objects need to be updated during a 

Yeah, clearer now.

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +1. The most straightforward method is labeling. In this method the generated 
+Objects are labeled with the `.Name` of the AnyState object that corresponds to 
+the version of the target Object state that was used to generate them. As we 
+have taken care to ensure the uniqueness of the `.Names` of the AnyStates, 
+this approach is reasonable.
+    - A revision is considered to be live while any generated Object labeled 
+     with its `.Name` is live.
+    - This method has the benefit of providing visibility, via the label, to 
+    users with respect to the historical providence of a generated Object.
+    - The primary drawback is the lack of support for using garbage collection 
+    to ensure that only non-live version snapshots are collected.
+1. Controllers may also use the `OwnerReferences` field of the AnyState to 
+record all Objects that are generated from target Object state version 
+represented by the AnyState as its owners.
+   - A revision is considered to be live while any generated Object that owns 
+   it is live.

I think what we are saying here is that amongst all AnyState objects for lets say StatefulSets, only the live one have ownership set the StatefulSet ? In terms of ownership , i think all AnyState which represent history should have their ownerreference pointing to the object whose history they represent. This would help in gc'ng history when the object is deleted as well

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +    - This method has the benefit of providing visibility, via the label, to 
+    users with respect to the historical providence of a generated Object.
+    - The primary drawback is the lack of support for using garbage collection 
+    to ensure that only non-live version snapshots are collected.
+1. Controllers may also use the `OwnerReferences` field of the AnyState to 
+record all Objects that are generated from target Object state version 
+represented by the AnyState as its owners.
+   - A revision is considered to be live while any generated Object that owns 
+   it is live.
+   - This method allows for the implementation of generic garbage collection.
+   - The primary drawback with this method is that the book keeping is complex, 
+   and deciding if a generated Object corresponds to a particular revision 
+   will require testing each Object for membership in the `OwnerReferences` 
+   of all AnyStates.
+   
+At the cost of the complexity of implementing both labeling and ownership, 

The current way our controllers work is:

1. select all the generated objects in the namespace for a parent: set A
2. from set A, select all objects that match the parent selector: set B
3. from set B, filter out all objects with an owner ref other than the current parent: set C
4. from set C, adopt all objects that have no owner ref
5. from set A, release all objects whose owner ref matches the parent but their labels don't match the parent selector anymore.

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +target Object state and perform a deep, semantic equality test. Here all 
+differences that do not constitute a mutation to the target Object state are 
+disregarded during the equivalence test.
+    - As it is imperative, this method may be brittle and hard to maintain.
+1. Alternatively, controllers may use a strategic merge patch to ignore fields 
+whose deletion, addition, or mutation does not constitute a revision.
+    - This approach requires all values to be JSON encodings of Objects, but the 
+    declarative nature will likely make the code more maintainable and less 
+    error prone (The business logic for the patch and comparison only needs to 
+    be implemented once).
+    
+Note that, as both methods provide a test for deep, semantic equivalence, 
+controllers that implement one method are not bound to use the same method 
+in the future. 
+    
+### Target Object State Reconciliation

Can you clarify this more ? Is this a new step needed to maintain the history ?

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +not require cryptographically strong collision resistance, and given we use a 
+[collision resolution](#collision-resolution) technique, we can't use the 
+[generated names](#unique-name-generation) of AnyStates to decide equality.
+
+We propose that two AnyStates can be considered equal if their `.Data` is 
+equivalent, but that it is not sufficient to compare the serialized 
+representation of the their `.Data`. Consider that the addition of new fields 
+to the Objects that represent the target Object state may cause the serialized 
+representation of those Objects to be unequal even when they are semantically 
+equivalent. We propose two methods that controllers may use to decide the 
+equivalence of two versions of their target Object state.
+
+1. The controller may deserialize the values of the AnyStates representing their 
+target Object state and perform a deep, semantic equality test. Here all 
+differences that do not constitute a mutation to the target Object state are 
+disregarded during the equivalence test.

Every time a new field gets added, update the imperative code that performs the comparison.

Oh, it's unlike that this will be maintainable. It's going to skew at some point. Maybe a reflective test can help us but I am not sure why we want this over a simple deep equality check.

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +```bash
+> kubectl rollback <controller-kind> <controller-name> --revision <any-state-name>
+```
+
+Here `kubectl rollback` simply applies the manifest generated by the 
+`kubectl history` command. We propose that the `--revision` argument be 
+optional, and that its exclusion indicates that the controller should be 
+reverted to its immediately prior state.
+
+## Tests
+
+1. Controllers can create an AnyState containing a revision of their target 
+Object state.
+1. Controllers can reconstruct their revision history.
+1. Controllers can't update an AnyState's `.Data`.
+1. Controllers can delete an AnyState to maintain their history with respect to 

can a user delete the history using kubectl ?

[krmayankk]

@krmayankk commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +```bash
+> kubectl rollback <controller-kind> <controller-name> --revision <any-state-name>
+```
+
+Here `kubectl rollback` simply applies the manifest generated by the 
+`kubectl history` command. We propose that the `--revision` argument be 
+optional, and that its exclusion indicates that the controller should be 
+reverted to its immediately prior state.
+
+## Tests
+
+1. Controllers can create an AnyState containing a revision of their target 
+Object state.
+1. Controllers can reconstruct their revision history.
+1. Controllers can't update an AnyState's `.Data`.
+1. Controllers can delete an AnyState to maintain their history with respect to 

can we set the revisionhistorylimit as 0 and hence the controllers dont generate AnyState at all ?

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +```bash
+> kubectl rollback <controller-kind> <controller-name> --revision <any-state-name>
+```
+
+Here `kubectl rollback` simply applies the manifest generated by the 
+`kubectl history` command. We propose that the `--revision` argument be 
+optional, and that its exclusion indicates that the controller should be 
+reverted to its immediately prior state.
+
+## Tests
+
+1. Controllers can create an AnyState containing a revision of their target 
+Object state.
+1. Controllers can reconstruct their revision history.
+1. Controllers can't update an AnyState's `.Data`.
+1. Controllers can delete an AnyState to maintain their history with respect to 

can we set the revisionhistorylimit as 0 and hence the controllers dont generate AnyState at all ?

revisionhistorylimit=0 doesn't mean that the controllers are not generating history, eg. a DaemonSet in the middle of a rollout would need both the old and the new history object. revisionHistoryLimit should take effect only for history objects with no links (no running pods in the system generated by them).

[krmayankk]

@kow3ns few other questions: what chnages are needed to consolidate with deployment revisions that are currently stored in ReplicaSets. Will they coexist ? Can you manually delete history ? Will pods created manually (without using rs) also have history ?

[Michail Kargakis]

@kow3ns few other questions: what chnages are needed to consolidate with deployment revisions that are currently stored in ReplicaSets. Will they coexist ?

I don't think we are going to make any changes in Deployments in the short term. I see no issue with coexistence since this will be initially implemented for DaemonSets and StatefulSets.

Can you manually delete history ?

You should be able but you shouldn't do it. You probably want to use revisionHistoryLimit.

Will pods created manually (without using rs) also have history ?

I am not sure we are going to add this for ReplicaSets since they don't support upgrades although I could imagine squashing ReplicaSets into Deployments in the future (after careful evaluation of the option). I don't think we are going to use history for bare pods.

[Michail Kargakis]

@mfojtik @tnozicka @soltysh fyi

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +target state of their generated Objects.
+
+## Affected Components
+
+1. API Machinery
+1. API Server
+1. Kubectl
+1. Controllers that utilize the feature
+
+## Requirements
+
+1. History is a collection of points in time, and each point in time must be 
+represented by its own Object. While it is tempting to aggregate all of an 
+Object's history into a single container Object, experience with Borg and Mesos 
+has taught us that this inevitably leads to exhausting the single Object size 
+limit of the system's the storage backend. 

typo: system's the storage

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +The following terminology is used throughout the rest of this proposal. We 
+make its meaning explicit here.
+- The specification type of a controller is the type that contains the 
+specification for the Objects generated by the controller.
+    - For example, the specification types for the ReplicaSet, DaemonSet, 
+     and StatefulSet controllers are ReplicaSetSpec, DaemonSetSpec, 
+     and StatefulSetSpec respectively.
+- The generated type(s) for a controller is/are the type of the Object(s) 
+generated by the controller. 
+    - Pod is a generated type for the ReplicaSet, DaemonSet, and StatefulSet 
+    controllers.
+    - PersistentVolumeClaim is also a generated type for the StatefulSet 
+    controller.
+- The current state of a controller is the union of the states of its generated 
+Objects along with its status.
+    - For ReplicaSet, DaemonSet, and StatefulSet, the current state of the 

What if another controller is actively mutating those states? I.e. PodPreset?

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +The following terminology is used throughout the rest of this proposal. We 
+make its meaning explicit here.
+- The specification type of a controller is the type that contains the 
+specification for the Objects generated by the controller.
+    - For example, the specification types for the ReplicaSet, DaemonSet, 
+     and StatefulSet controllers are ReplicaSetSpec, DaemonSetSpec, 
+     and StatefulSetSpec respectively.
+- The generated type(s) for a controller is/are the type of the Object(s) 
+generated by the controller. 
+    - Pod is a generated type for the ReplicaSet, DaemonSet, and StatefulSet 
+    controllers.
+    - PersistentVolumeClaim is also a generated type for the StatefulSet 
+    controller.
+- The current state of a controller is the union of the states of its generated 
+Objects along with its status.
+    - For ReplicaSet, DaemonSet, and StatefulSet, the current state of the 

Also initial resources autosizer, pod security policy, etc

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +Objects, we only need to persist snapshots of the target Object states 
+contained in the specification type when they are revised.
+
+One approach would be to, for every specification type,  have a 
+corresponding History type. For example, we could introduce a StatefulSetHistory 
+object that aggregates a PodTemplateSpec and a slice of PersistentVolumeClaims.
+The StatefulSet controller could use this object to store point in time 
+snapshots of versions of StatefulSetSpecs. However, this requires that we 
+introduce a new History Kind for all current and future controllers. It has the 
+benefit of type safety, but, for this benefit, we trade generality.
+
+Another approach would be to use PodTemplate objects. This mechanisms provides 
+the desired generality, but it only provides for the recording of versions of
+PodTemplateSpecs (e.g. For StatefulSet, we can not use PodTemplates to 
+record revisions to PersistentVolumeClaims). Also, it introduces the potential 
+for overlapping histories for two Objects of different Kinds, with the same 

Doesn't AnyState also have this problem?

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +The following terminology is used throughout the rest of this proposal. We 
+make its meaning explicit here.
+- The specification type of a controller is the type that contains the 
+specification for the Objects generated by the controller.
+    - For example, the specification types for the ReplicaSet, DaemonSet, 
+     and StatefulSet controllers are ReplicaSetSpec, DaemonSetSpec, 
+     and StatefulSetSpec respectively.
+- The generated type(s) for a controller is/are the type of the Object(s) 
+generated by the controller. 
+    - Pod is a generated type for the ReplicaSet, DaemonSet, and StatefulSet 
+    controllers.
+    - PersistentVolumeClaim is also a generated type for the StatefulSet 
+    controller.
+- The current state of a controller is the union of the states of its generated 
+Objects along with its status.
+    - For ReplicaSet, DaemonSet, and StatefulSet, the current state of the 

Note that initializers/admission controllers are allowed to both default (fill out) and mutate (override) fields. We can discourage mutation, but cannot disallow it.

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +// AnyState implements an immutable dictionary of state data to allow for a 
+// point in time snapshot. Clients are responsible for serializing 
+// and deserializing the objects that contain their internal state and mapping 
+// them to a set of keys. 
+// Once an AnyState has been successfully created, it can not be updated. The 
+// API Server will fail validation of all update requests that attempt to mutate 
+// the Data field. AnyStates may, however, be deleted.
+type AnyState struct {
+    metav1.TypeMeta
+    // +optional
+    metav1.ObjectMeta
+    // Data is a map of keys to the serialized objects that comprise the state 
+    // snapshot.
+    // Each key must consist of alphanumeric characters, '-', '_' or '.'.
+    // +optional
+    Data map[string]string

I'm confused how this is different than a config map. I guess I was expecting something like:

type ControllerHistory struct {
  metav1.ObjectMeta
  Spec struct{
    OwningController api.ObjectReference
    DesiredState runtime.RawExtension // contains serialization in raw form of the controller with any non-template fields stripped
    ...
  }
}

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +with the new target Object state.
+1. The controller will [maintain the length of its history](#history-maintenance) 
+to be less than the configured limit.
+
+### Version Snapshot Creation
+To take a snapshot of the target Object state contained in a specification type
+Object, a controller will do the following.
+
+1. The controller will serialize all the fields of the Object's target object 
+state using JSON encoding.
+1. The controller will populate the `.Data` of an AnyState object by 
+associating the serialized fields with keys that correspond to the fields' 
+names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.

I don't know why we would go so generic here. If we have to define a schema on Data, we might as well specialize the object and define the schema on that instead.

[Janet Kuo]

@janetkuo commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +contained in the AnyState with specification type Object in the controller's 
+manifest and to output the resulting manifest.
+
+- This output manifest represents the scale invariant application of a prior 
+target Object state to the current specification type Object.
+- This approach only works because of the encoding we propose for 
+[version snapshot construction](#version-snapshot-creation). Controllers that 
+use a different encoding will not be able to use this feature.
+
+### Rollback
+
+For future work, `kubeclt rollback` can be implemented in the general case as an 
+extension of the [above](#viewing-history ).
+
+```bash
+> kubectl rollback <controller-kind> <controller-name> --revision <any-state-name>

and kubectl rollout undo --to-revision

---

In contributors/design-proposals/controller_history.md:

> +
+```bash
+> kubectl history <controller-kind> <controller-name>
+```
+
+In the above `controller-kind` indicates the Kind of the controller (e.g. 
+ReplicaSet, StatefulSet, DaemonSet) and `controller-name` indicates the `.Name` 
+of the request subject. We propose that the output of this command be a list 
+of the `.Names` of AnyStates that comprise the revision history of the 
+controller.
+
+In order to view the application of a particular revision to the controller we 
+propose the following additional flag.
+
+```bash
+> kubectl history <controller-kind> <controller-name> --revision <any-state-name>

Current implementation of --revision asks for an int64. Users get revision number from kubectl history. Switching it to <any-state-name> will be a huge user experience change. Same as kubectl rollout undo --to-revision.

---

In contributors/design-proposals/controller_history.md:

> +with the new target Object state.
+1. The controller will [maintain the length of its history](#history-maintenance) 
+to be less than the configured limit.
+
+### Version Snapshot Creation
+To take a snapshot of the target Object state contained in a specification type
+Object, a controller will do the following.
+
+1. The controller will serialize all the fields of the Object's target object 
+state using JSON encoding.
+1. The controller will populate the `.Data` of an AnyState object by 
+associating the serialized fields with keys that correspond to the fields' 
+names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.

A problem with Generation: a user can easily mess up controller history by deleting a controller non-cascadingly and re-creating it

---

In contributors/design-proposals/controller_history.md:

> +// AnyState implements an immutable dictionary of state data to allow for a 
+// point in time snapshot. Clients are responsible for serializing 
+// and deserializing the objects that contain their internal state and mapping 
+// them to a set of keys. 
+// Once an AnyState has been successfully created, it can not be updated. The 
+// API Server will fail validation of all update requests that attempt to mutate 
+// the Data field. AnyStates may, however, be deleted.
+type AnyState struct {
+    metav1.TypeMeta
+    // +optional
+    metav1.ObjectMeta
+    // Data is a map of keys to the serialized objects that comprise the state 
+    // snapshot.
+    // Each key must consist of alphanumeric characters, '-', '_' or '.'.
+    // +optional
+    Data map[string]string

Would AnyState be harder to read?

---

In contributors/design-proposals/controller_history.md:

> +Objects, we only need to persist snapshots of the target Object states 
+contained in the specification type when they are revised.
+
+One approach would be to, for every specification type,  have a 
+corresponding History type. For example, we could introduce a StatefulSetHistory 
+object that aggregates a PodTemplateSpec and a slice of PersistentVolumeClaims.
+The StatefulSet controller could use this object to store point in time 
+snapshots of versions of StatefulSetSpecs. However, this requires that we 
+introduce a new History Kind for all current and future controllers. It has the 
+benefit of type safety, but, for this benefit, we trade generality.
+
+Another approach would be to use PodTemplate objects. This mechanisms provides 
+the desired generality, but it only provides for the recording of versions of
+PodTemplateSpecs (e.g. For StatefulSet, we can not use PodTemplates to 
+record revisions to PersistentVolumeClaims). Also, it introduces the potential 
+for overlapping histories for two Objects of different Kinds, with the same 

+1, I have the same question

---

In contributors/design-proposals/controller_history.md:

> +    controllers.
+    - PersistentVolumeClaim is also a generated type for the StatefulSet 
+    controller.
+- The current state of a controller is the union of the states of its generated 
+Objects along with its status.
+    - For ReplicaSet, DaemonSet, and StatefulSet, the current state of the 
+    corresponding controllers can be derived from Pods they contain and the 
+    ReplicasSetStatus, DaemonSetStatus, and StatefulSetStatus objects 
+    respectively.
+- For all specification type Objects for controllers, the target state is the 
+set of fields in the Object that determine the state to which the controller 
+attempts to evolve the system.
+    - This may not necessarily be all fields of Object.
+- The target Object state is the subset of the target state necessary to create 
+a Objects of the generated type(s). 
+- To make the this concrete, for the StatefulSet controller:

Suggest merging this with the previous bullet point, since it explains the previous point (target state)

---

In contributors/design-proposals/controller_history.md:

> +names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.
+1. The controller will compute the [hash](#hashing) of the AnyState's `.Data`.
+1. The controller will attach a label to the AnyState so that it is selectable 
+with a low probability of overlap.
+    - ControllerRefs will be used as the authoritative test for ownership.
+    - The specification type Object's `.Selector` should be used where 
+    applicable. 
+    - Alternatively, a Kind unique label may be set to the `.Name` of the 
+    specification type Object.
+1. The controller will add a ControllerRef indicating the specification type 
+Object as the owner of the AnyState in the AnyState's `.OwnerReferences`.
+1. The controller will use the hash from above, along with a user identifiable 
+prefix, to [generate a unique `.Name`](#unique-name-generation) for the AnyState.

What's this prefix? Does each controller decide what the prefix should be, or it'll just use controller .Name?

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +    - This method has the benefit of providing visibility, via the label, to 
+    users with respect to the historical providence of a generated Object.
+    - The primary drawback is the lack of support for using garbage collection 
+    to ensure that only non-live version snapshots are collected.
+1. Controllers may also use the `OwnerReferences` field of the AnyState to 
+record all Objects that are generated from target Object state version 
+represented by the AnyState as its owners.
+   - A revision is considered to be live while any generated Object that owns 
+   it is live.
+   - This method allows for the implementation of generic garbage collection.
+   - The primary drawback with this method is that the book keeping is complex, 
+   and deciding if a generated Object corresponds to a particular revision 
+   will require testing each Object for membership in the `OwnerReferences` 
+   of all AnyStates.
+   
+At the cost of the complexity of implementing both labeling and ownership, 

I've been putting some more thought into this. I did not consider adoption initially because I was coming from the viewpoint that controller history is not adoptable. I still have not convinced myself that it makes sense to adopt the history of another object, but I haven't convinced myself that this is an aweful idea either.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +not require cryptographically strong collision resistance, and given we use a 
+[collision resolution](#collision-resolution) technique, we can't use the 
+[generated names](#unique-name-generation) of AnyStates to decide equality.
+
+We propose that two AnyStates can be considered equal if their `.Data` is 
+equivalent, but that it is not sufficient to compare the serialized 
+representation of the their `.Data`. Consider that the addition of new fields 
+to the Objects that represent the target Object state may cause the serialized 
+representation of those Objects to be unequal even when they are semantically 
+equivalent. We propose two methods that controllers may use to decide the 
+equivalence of two versions of their target Object state.
+
+1. The controller may deserialize the values of the AnyStates representing their 
+target Object state and perform a deep, semantic equality test. Here all 
+differences that do not constitute a mutation to the target Object state are 
+disregarded during the equivalence test.

Deep equality will return false positives that can cause unintentional upgrades. My largest concern is that a new field with a default value causes the a rolling update for all StatefulSet's and DaemonSet's during/after a Master upgrade. As long as rolling update works well it shouldn't actually result in a loss of availability, but it could be the worst kind of surprising.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +- The specification type of a controller is the type that contains the 
+specification for the Objects generated by the controller.
+    - For example, the specification types for the ReplicaSet, DaemonSet, 
+     and StatefulSet controllers are ReplicaSetSpec, DaemonSetSpec, 
+     and StatefulSetSpec respectively.
+- The generated type(s) for a controller is/are the type of the Object(s) 
+generated by the controller. 
+    - Pod is a generated type for the ReplicaSet, DaemonSet, and StatefulSet 
+    controllers.
+    - PersistentVolumeClaim is also a generated type for the StatefulSet 
+    controller.
+- The current state of a controller is the union of the states of its generated 
+Objects along with its status.
+    - For ReplicaSet, DaemonSet, and StatefulSet, the current state of the 
+    corresponding controllers can be derived from Pods they contain and the 
+    ReplicasSetStatus, DaemonSetStatus, and StatefulSetStatus objects 

Only is so much as the current current and update revisions are stored in the Status Kind for the object.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +Mesos Replicate Log (A Multi-Paxos based state machine used by the Mesos 
+Masters). This State Abstraction is a mutable, durable dictionary where keys 
+and values are opaque strings. As controllers only need the capability to 
+persist an immutable point in time snapshot of target Object states to 
+implement a revision history, we propose to use an immutable durable dictionary, 
+and we introduce the AnyState object for this purpose.
+
+``` golang
+// AnyState implements an immutable dictionary of state data to allow for a 
+// point in time snapshot. Clients are responsible for serializing 
+// and deserializing the objects that contain their internal state and mapping 
+// them to a set of keys. 
+// Once an AnyState has been successfully created, it can not be updated. The 
+// API Server will fail validation of all update requests that attempt to mutate 
+// the Data field. AnyStates may, however, be deleted.
+type AnyState struct {

I don't have other purpose right now, but the object is named after what it is. Its a generic, immutable snapshot of state.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +can use AnyState to persist a history of revisions to its specification type 
+Objects. The technique is applicable, without loss of generality, to the 
+existing Kuberentes controllers that have Pod as a generated type.
+
+When a controller detects a revision to the target Object state of a 
+specification type Object it will do the following.
+
+1. The controller will [create a snapshot](#version-snapshot-creation) of the 
+current target Object state.
+1. The controller will [reconstruct the history](#history-reconstruction) of 
+revisions to the Object's target Object state.
+1. The controller will test the current target Object state for 
+[equivalence](#version-equivalence) with all other versions in the Object's 
+revision history.
+    - If the current version is semantically equivalent to its immediate 
+    predecessor no update to the Object's target state has been performed.

Sorting based on the Generation.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +The API Server must support the creation and deletion of AnyState objects.
+As we have no mechanism for declarative immutability, the API server must fail 
+any update request that updates the `.Data` field of an AnyState Object.
+
+## Controllers 
+This section is presented as a generalization of how an arbitrary controller 
+can use AnyState to persist a history of revisions to its specification type 
+Objects. The technique is applicable, without loss of generality, to the 
+existing Kuberentes controllers that have Pod as a generated type.
+
+When a controller detects a revision to the target Object state of a 
+specification type Object it will do the following.
+
+1. The controller will [create a snapshot](#version-snapshot-creation) of the 
+current target Object state.
+1. The controller will [reconstruct the history](#history-reconstruction) of 

No, you will reconstruct the history during every execution of the main control loop. In practice controllers benefit from a shared cache that mediates interaction with API server.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +
+1. The controller will [create a snapshot](#version-snapshot-creation) of the 
+current target Object state.
+1. The controller will [reconstruct the history](#history-reconstruction) of 
+revisions to the Object's target Object state.
+1. The controller will test the current target Object state for 
+[equivalence](#version-equivalence) with all other versions in the Object's 
+revision history.
+    - If the current version is semantically equivalent to its immediate 
+    predecessor no update to the Object's target state has been performed.
+    - If the current version is equivalent to a version prior to its immediate 
+    predecessor, this indicates a rollback.
+    - If the current version is not equivalent to any prior version, this 
+    indicates an update or a roll forward.
+    - Controllers should use their status objects for book keeping with respect 
+    to current and prior revisions.

Yes

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +    applicable. 
+    - Alternatively, a Kind unique label may be set to the `.Name` of the 
+    specification type Object.
+1. The controller will add a ControllerRef indicating the specification type 
+Object as the owner of the AnyState in the AnyState's `.OwnerReferences`.
+1. The controller will use the hash from above, along with a user identifiable 
+prefix, to [generate a unique `.Name`](#unique-name-generation) for the AnyState.
+1. The controller will persist the AnyState via the API Server.
+    - Note that, in practice, creation occurs concurrently with 
+    [collision resolution](#collision-resolution).
+    
+### History Reconstruction
+To reconstruct the history of a specification type Object, a controller will do 
+the following. 
+
+1. Select all AnyState Objects labeled as described 

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +Objects, we only need to persist snapshots of the target Object states 
+contained in the specification type when they are revised.
+
+One approach would be to, for every specification type,  have a 
+corresponding History type. For example, we could introduce a StatefulSetHistory 
+object that aggregates a PodTemplateSpec and a slice of PersistentVolumeClaims.
+The StatefulSet controller could use this object to store point in time 
+snapshots of versions of StatefulSetSpecs. However, this requires that we 
+introduce a new History Kind for all current and future controllers. It has the 
+benefit of type safety, but, for this benefit, we trade generality.
+
+Another approach would be to use PodTemplate objects. This mechanisms provides 
+the desired generality, but it only provides for the recording of versions of
+PodTemplateSpecs (e.g. For StatefulSet, we can not use PodTemplates to 
+record revisions to PersistentVolumeClaims). Also, it introduces the potential 
+for overlapping histories for two Objects of different Kinds, with the same 

Based on the collision avoidance and hashing convention we came up with Name collisions can be resolved. Provided we use ControllerRef's to resolve ownership we should be able to decide ownership deterministically.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +// AnyState implements an immutable dictionary of state data to allow for a 
+// point in time snapshot. Clients are responsible for serializing 
+// and deserializing the objects that contain their internal state and mapping 
+// them to a set of keys. 
+// Once an AnyState has been successfully created, it can not be updated. The 
+// API Server will fail validation of all update requests that attempt to mutate 
+// the Data field. AnyStates may, however, be deleted.
+type AnyState struct {
+    metav1.TypeMeta
+    // +optional
+    metav1.ObjectMeta
+    // Data is a map of keys to the serialized objects that comprise the state 
+    // snapshot.
+    // Each key must consist of alphanumeric characters, '-', '_' or '.'.
+    // +optional
+    Data map[string]string

As discussed previously, its not so different from ConfigMap (except it is immutable). The benefit of using JSON encoding to map the name of the serialized property to its value is that it can be viewed as an encapsulation of the deltas necessary to restore the prior state. It is generic enough that it can be used by any controller to capture its state.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +with the new target Object state.
+1. The controller will [maintain the length of its history](#history-maintenance) 
+to be less than the configured limit.
+
+### Version Snapshot Creation
+To take a snapshot of the target Object state contained in a specification type
+Object, a controller will do the following.
+
+1. The controller will serialize all the fields of the Object's target object 
+state using JSON encoding.
+1. The controller will populate the `.Data` of an AnyState object by 
+associating the serialized fields with keys that correspond to the fields' 
+names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.

Does a controllers history survive deletion and recreation? Should we be able to delete a StatefulSet or a DaemonSet, and then apply a rollback to the prior state of the previous object?

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.
+1. The controller will compute the [hash](#hashing) of the AnyState's `.Data`.
+1. The controller will attach a label to the AnyState so that it is selectable 
+with a low probability of overlap.
+    - ControllerRefs will be used as the authoritative test for ownership.
+    - The specification type Object's `.Selector` should be used where 
+    applicable. 
+    - Alternatively, a Kind unique label may be set to the `.Name` of the 
+    specification type Object.
+1. The controller will add a ControllerRef indicating the specification type 
+Object as the owner of the AnyState in the AnyState's `.OwnerReferences`.
+1. The controller will use the hash from above, along with a user identifiable 
+prefix, to [generate a unique `.Name`](#unique-name-generation) for the AnyState.

Each controller can decide to use whatever it needs to to prefix the state so that the name is reasonable.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +with the new target Object state.
+1. The controller will [maintain the length of its history](#history-maintenance) 
+to be less than the configured limit.
+
+### Version Snapshot Creation
+To take a snapshot of the target Object state contained in a specification type
+Object, a controller will do the following.
+
+1. The controller will serialize all the fields of the Object's target object 
+state using JSON encoding.
+1. The controller will populate the `.Data` of an AnyState object by 
+associating the serialized fields with keys that correspond to the fields' 
+names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.

If this is the case than we can just add a number for Revision that is always 1 greater than the Revision stored in all other AnyStates. I think we should still track Generation, but this scheme works just as well.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +The following terminology is used throughout the rest of this proposal. We 
+make its meaning explicit here.
+- The specification type of a controller is the type that contains the 
+specification for the Objects generated by the controller.
+    - For example, the specification types for the ReplicaSet, DaemonSet, 
+     and StatefulSet controllers are ReplicaSetSpec, DaemonSetSpec, 
+     and StatefulSetSpec respectively.
+- The generated type(s) for a controller is/are the type of the Object(s) 
+generated by the controller. 
+    - Pod is a generated type for the ReplicaSet, DaemonSet, and StatefulSet 
+    controllers.
+    - PersistentVolumeClaim is also a generated type for the StatefulSet 
+    controller.
+- The current state of a controller is the union of the states of its generated 
+Objects along with its status.
+    - For ReplicaSet, DaemonSet, and StatefulSet, the current state of the 

Since we are labeling the Pods to indicate their providence with respect to the revision of the controller's target state we are susceptible to downstream mutations by other controllers changing the controller's product. The best we can do is guarantee that our product meets the specification at the time of creation. If a third party mutates the product downstream (as long as it does so in a consistent and intentional way) we don't want to recall it and make it conform to the original spec.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +with the new target Object state.
+1. The controller will [maintain the length of its history](#history-maintenance) 
+to be less than the configured limit.
+
+### Version Snapshot Creation
+To take a snapshot of the target Object state contained in a specification type
+Object, a controller will do the following.
+
+1. The controller will serialize all the fields of the Object's target object 
+state using JSON encoding.
+1. The controller will populate the `.Data` of an AnyState object by 
+associating the serialized fields with keys that correspond to the fields' 
+names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.

The idea of the ConfigMap like Object is avoid assumptions about the state data that third parties and operators might use. We will use Template and VolumeClaimsTemplate. An Operator might use an arbitrary field. Even tracking Generation as a well defined type won't work at this point for TPRs. We might want to consider going with a Revision field though. Controllers, that have a working Generation can use this. Deployments, Operators, TPC, can use the incremental Revision technique that Deployment currently uses.

[Janet Kuo]

Deployment controller copies a Deployment's annotations to its ReplicaSets. We may want that for controller history too.

[Kenneth Owens]

@kow3ns pushed 1 commit.

• a77bff5 Clarify ReplicaSet mutation

—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +not require cryptographically strong collision resistance, and given we use a 
+[collision resolution](#collision-resolution) technique, we can't use the 
+[generated names](#unique-name-generation) of AnyStates to decide equality.
+
+We propose that two AnyStates can be considered equal if their `.Data` is 
+equivalent, but that it is not sufficient to compare the serialized 
+representation of the their `.Data`. Consider that the addition of new fields 
+to the Objects that represent the target Object state may cause the serialized 
+representation of those Objects to be unequal even when they are semantically 
+equivalent. We propose two methods that controllers may use to decide the 
+equivalence of two versions of their target Object state.
+
+1. The controller may deserialize the values of the AnyStates representing their 
+target Object state and perform a deep, semantic equality test. Here all 
+differences that do not constitute a mutation to the target Object state are 
+disregarded during the equivalence test.

a new field with a default value causes the a rolling update for all StatefulSet's and DaemonSet's during/after a Master upgrade.

Why is that the case? Wouldn't the new default affect both the DaemonSet/StatefulSet and AnyState templates?

—
You are receiving this because you are on a team that was mentioned.

Reply to this email directly, view it on GitHub, or mute the thread.

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +with the new target Object state.
+1. The controller will [maintain the length of its history](#history-maintenance) 
+to be less than the configured limit.
+
+### Version Snapshot Creation
+To take a snapshot of the target Object state contained in a specification type
+Object, a controller will do the following.
+
+1. The controller will serialize all the fields of the Object's target object 
+state using JSON encoding.
+1. The controller will populate the `.Data` of an AnyState object by 
+associating the serialized fields with keys that correspond to the fields' 
+names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.

Yes, this can work similarly to how the Deployment controller decides the next revision for a Deployment (based on the highest revision found in old RSs + 1).

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +1. Controllers can create an AnyState containing a revision of their target 
+Object state.
+1. Controllers can reconstruct their revision history.
+1. Controllers can't update an AnyState's `.Data`.
+1. Controllers can delete an AnyState to maintain their history with respect to 
+the configured `RevisionHistoryLimit`.
+
+## Appendix
+ 
+### Hashing
+We will require a CRHF (collision resistant hash function), but, as we expect 
+no adversaries, such a function need not be resistant to pre-image and 
+secondary pre-image attacks.
+As the property of interest is primarily collision resistance, and as we 
+provide a method of [collision resolution](#collision-resolution), both 
+cryptographically strong functions, such as Secure Hash Algorithm 2 (SHA-2), 

Do you think we need a cryptographic function for hashing?

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +Mesos Replicate Log (A Multi-Paxos based state machine used by the Mesos 
+Masters). This State Abstraction is a mutable, durable dictionary where keys 
+and values are opaque strings. As controllers only need the capability to 
+persist an immutable point in time snapshot of target Object states to 
+implement a revision history, we propose to use an immutable durable dictionary, 
+and we introduce the AnyState object for this purpose.
+
+``` golang
+// AnyState implements an immutable dictionary of state data to allow for a 
+// point in time snapshot. Clients are responsible for serializing 
+// and deserializing the objects that contain their internal state and mapping 
+// them to a set of keys. 
+// Once an AnyState has been successfully created, it can not be updated. The 
+// API Server will fail validation of all update requests that attempt to mutate 
+// the Data field. AnyStates may, however, be deleted.
+type AnyState struct {

I'm not in favor of this, as it's too broad. We already have ConfigMap for "any data". Creating configmap with another name seems wrong.

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +with the new target Object state.
+1. The controller will [maintain the length of its history](#history-maintenance) 
+to be less than the configured limit.
+
+### Version Snapshot Creation
+To take a snapshot of the target Object state contained in a specification type
+Object, a controller will do the following.
+
+1. The controller will serialize all the fields of the Object's target object 
+state using JSON encoding.
+1. The controller will populate the `.Data` of an AnyState object by 
+associating the serialized fields with keys that correspond to the fields' 
+names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.

I can see the argument, but still uncomfortable with the idea of an api object that looks a lot like config map that is explicitly described as "generic key value store". There has to be a really strong reason to go that generic. Also, map vs array comes into play here - I'd rather an array with more structured data about each bit of state, especially if these correlate to actual fields.

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +The following terminology is used throughout the rest of this proposal. We 
+make its meaning explicit here.
+- The specification type of a controller is the type that contains the 
+specification for the Objects generated by the controller.
+    - For example, the specification types for the ReplicaSet, DaemonSet, 
+     and StatefulSet controllers are ReplicaSetSpec, DaemonSetSpec, 
+     and StatefulSetSpec respectively.
+- The generated type(s) for a controller is/are the type of the Object(s) 
+generated by the controller. 
+    - Pod is a generated type for the ReplicaSet, DaemonSet, and StatefulSet 
+    controllers.
+    - PersistentVolumeClaim is also a generated type for the StatefulSet 
+    controller.
+- The current state of a controller is the union of the states of its generated 
+Objects along with its status.
+    - For ReplicaSet, DaemonSet, and StatefulSet, the current state of the 

That's fine, we should call it out here explicitly since PodPresent and how it interplays with StatefulSet is novel and may be surprising to people.

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +// AnyState implements an immutable dictionary of state data to allow for a 
+// point in time snapshot. Clients are responsible for serializing 
+// and deserializing the objects that contain their internal state and mapping 
+// them to a set of keys. 
+// Once an AnyState has been successfully created, it can not be updated. The 
+// API Server will fail validation of all update requests that attempt to mutate 
+// the Data field. AnyStates may, however, be deleted.
+type AnyState struct {
+    metav1.TypeMeta
+    // +optional
+    metav1.ObjectMeta
+    // Data is a map of keys to the serialized objects that comprise the state 
+    // snapshot.
+    // Each key must consist of alphanumeric characters, '-', '_' or '.'.
+    // +optional
+    Data map[string]string

Immutability is going to be a property of many objects, so just being immutable isn't enough to be distinct.

I really don't want to create key value storage for history. That's not in the remit of the proposal and needs sufficiently stronger justification.

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.
+1. The controller will compute the [hash](#hashing) of the AnyState's `.Data`.
+1. The controller will attach a label to the AnyState so that it is selectable 
+with a low probability of overlap.
+    - ControllerRefs will be used as the authoritative test for ownership.
+    - The specification type Object's `.Selector` should be used where 
+    applicable. 
+    - Alternatively, a Kind unique label may be set to the `.Name` of the 
+    specification type Object.
+1. The controller will add a ControllerRef indicating the specification type 
+Object as the owner of the AnyState in the AnyState's `.OwnerReferences`.
+1. The controller will use the hash from above, along with a user identifiable 
+prefix, to [generate a unique `.Name`](#unique-name-generation) for the AnyState.

We should define a convention and require first party controllers to follow it.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +with the new target Object state.
+1. The controller will [maintain the length of its history](#history-maintenance) 
+to be less than the configured limit.
+
+### Version Snapshot Creation
+To take a snapshot of the target Object state contained in a specification type
+Object, a controller will do the following.
+
+1. The controller will serialize all the fields of the Object's target object 
+state using JSON encoding.
+1. The controller will populate the `.Data` of an AnyState object by 
+associating the serialized fields with keys that correspond to the fields' 
+names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.

With an array, how would we map back to the origin field?

The strong reason for this construction is that you can use the JSON encoded version of the Object to merge patch a delta to an arbitrary other Object. You can use it to save a delta that can be reapplied later, and it will work for an arbitrary Controller-like Object.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +The following terminology is used throughout the rest of this proposal. We 
+make its meaning explicit here.
+- The specification type of a controller is the type that contains the 
+specification for the Objects generated by the controller.
+    - For example, the specification types for the ReplicaSet, DaemonSet, 
+     and StatefulSet controllers are ReplicaSetSpec, DaemonSetSpec, 
+     and StatefulSetSpec respectively.
+- The generated type(s) for a controller is/are the type of the Object(s) 
+generated by the controller. 
+    - Pod is a generated type for the ReplicaSet, DaemonSet, and StatefulSet 
+    controllers.
+    - PersistentVolumeClaim is also a generated type for the StatefulSet 
+    controller.
+- The current state of a controller is the union of the states of its generated 
+Objects along with its status.
+    - For ReplicaSet, DaemonSet, and StatefulSet, the current state of the 

I will add the above to the proposal.

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +// AnyState implements an immutable dictionary of state data to allow for a 
+// point in time snapshot. Clients are responsible for serializing 
+// and deserializing the objects that contain their internal state and mapping 
+// them to a set of keys. 
+// Once an AnyState has been successfully created, it can not be updated. The 
+// API Server will fail validation of all update requests that attempt to mutate 
+// the Data field. AnyStates may, however, be deleted.
+type AnyState struct {
+    metav1.TypeMeta
+    // +optional
+    metav1.ObjectMeta
+    // Data is a map of keys to the serialized objects that comprise the state 
+    // snapshot.
+    // Each key must consist of alphanumeric characters, '-', '_' or '.'.
+    // +optional
+    Data map[string]string

The intention is only to provide storage for history using a generic object that provides a JSON encoding allowing for re-application of the state data via a strategic merge patch as per the prior discussion with @erictune and @bgrant0607. The intention is not to provide a generic key value store for controllers.

Is your primary concern that it will be used for unintentional purposes? If so, the construction above doesn't mitigate this. You could serialize arbitrary information into a RawExtension as well.

If you feel the name "AnyState" is misleading with respect to the intention we can change it "ControllerHistory".

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +1. Controllers can create an AnyState containing a revision of their target 
+Object state.
+1. Controllers can reconstruct their revision history.
+1. Controllers can't update an AnyState's `.Data`.
+1. Controllers can delete an AnyState to maintain their history with respect to 
+the configured `RevisionHistoryLimit`.
+
+## Appendix
+ 
+### Hashing
+We will require a CRHF (collision resistant hash function), but, as we expect 
+no adversaries, such a function need not be resistant to pre-image and 
+secondary pre-image attacks.
+As the property of interest is primarily collision resistance, and as we 
+provide a method of [collision resolution](#collision-resolution), both 
+cryptographically strong functions, such as Secure Hash Algorithm 2 (SHA-2), 

I would be hard pressed to prove that we do. Since we have collision resolution the collision probability is performance trade off. More collisions means more calls to the API server during resolution. Less collisions means we spend more CPU time computing strong hashes. I'm voting for FNV, but we can switch to SHA-2 later if necessary.

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +// AnyState implements an immutable dictionary of state data to allow for a 
+// point in time snapshot. Clients are responsible for serializing 
+// and deserializing the objects that contain their internal state and mapping 
+// them to a set of keys. 
+// Once an AnyState has been successfully created, it can not be updated. The 
+// API Server will fail validation of all update requests that attempt to mutate 
+// the Data field. AnyStates may, however, be deleted.
+type AnyState struct {
+    metav1.TypeMeta
+    // +optional
+    metav1.ObjectMeta
+    // Data is a map of keys to the serialized objects that comprise the state 
+    // snapshot.
+    // Each key must consist of alphanumeric characters, '-', '_' or '.'.
+    // +optional
+    Data map[string]string

"AnyState" is too broad. "ControllerHistory" is misleading since an instance of this resource is part of a controller history. This is more like "ControllerRevision"

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +1. Controllers can create an AnyState containing a revision of their target 
+Object state.
+1. Controllers can reconstruct their revision history.
+1. Controllers can't update an AnyState's `.Data`.
+1. Controllers can delete an AnyState to maintain their history with respect to 
+the configured `RevisionHistoryLimit`.
+
+## Appendix
+ 
+### Hashing
+We will require a CRHF (collision resistant hash function), but, as we expect 
+no adversaries, such a function need not be resistant to pre-image and 
+secondary pre-image attacks.
+As the property of interest is primarily collision resistance, and as we 
+provide a method of [collision resolution](#collision-resolution), both 
+cryptographically strong functions, such as Secure Hash Algorithm 2 (SHA-2), 

Based on prior experimentation, I think the gain we get from the difference in stability between sha-2 and fnv is negligible because fnv is already stable enough. Coupled with the uniquifier, we should be foolproof.

[Kenneth Owens]

@kow3ns pushed 1 commit.

• 4369da7 Change AnyState to ControllerHistory

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or mute the thread.

[Kenneth Owens]

@kow3ns pushed 1 commit.

• 6a1465a Remove JSON encodign and use RAW encoding

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +    - This method has the benefit of providing visibility, via the label, to 
+    users with respect to the historical providence of a generated Object.
+    - The primary drawback is the lack of support for using garbage collection 
+    to ensure that only non-live version snapshots are collected.
+1. Controllers may also use the `OwnerReferences` field of the AnyState to 
+record all Objects that are generated from target Object state version 
+represented by the AnyState as its owners.
+   - A revision is considered to be live while any generated Object that owns 
+   it is live.
+   - This method allows for the implementation of generic garbage collection.
+   - The primary drawback with this method is that the book keeping is complex, 
+   and deciding if a generated Object corresponds to a particular revision 
+   will require testing each Object for membership in the `OwnerReferences` 
+   of all AnyStates.
+   
+At the cost of the complexity of implementing both labeling and ownership, 

Renaming is one use case, handing off to a different workload controller type is another (eg. moving away from Deployments into a TPR).

—
You are receiving this because you are on a team that was mentioned.

Reply to this email directly, view it on GitHub, or mute the thread.

[Kenneth Owens]

@kow3ns pushed 1 commit.

• 471d67e Added statement about the limitations wrt the controllers product being

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or mute the thread.

[Kenneth Owens]

@kow3ns pushed 1 commit.

• 449d702 Change several instances of an to a ControllerHistory

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +1. Controllers can create an AnyState containing a revision of their target 
+Object state.
+1. Controllers can reconstruct their revision history.
+1. Controllers can't update an AnyState's `.Data`.
+1. Controllers can delete an AnyState to maintain their history with respect to 
+the configured `RevisionHistoryLimit`.
+
+## Appendix
+ 
+### Hashing
+We will require a CRHF (collision resistant hash function), but, as we expect 
+no adversaries, such a function need not be resistant to pre-image and 
+secondary pre-image attacks.
+As the property of interest is primarily collision resistance, and as we 
+provide a method of [collision resolution](#collision-resolution), both 
+cryptographically strong functions, such as Secure Hash Algorithm 2 (SHA-2), 

My thought is also that the time spent computing SHA-2 is not worth it.

—
You are receiving this because you are on a team that was mentioned.

Reply to this email directly, view it on GitHub, or mute the thread.

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +1. Controllers can create an AnyState containing a revision of their target 
+Object state.
+1. Controllers can reconstruct their revision history.
+1. Controllers can't update an AnyState's `.Data`.
+1. Controllers can delete an AnyState to maintain their history with respect to 
+the configured `RevisionHistoryLimit`.
+
+## Appendix
+ 
+### Hashing
+We will require a CRHF (collision resistant hash function), but, as we expect 
+no adversaries, such a function need not be resistant to pre-image and 
+secondary pre-image attacks.
+As the property of interest is primarily collision resistance, and as we 
+provide a method of [collision resolution](#collision-resolution), both 
+cryptographically strong functions, such as Secure Hash Algorithm 2 (SHA-2), 

Yeah, that's where I end up too.

[Michal Fojtik]

@mfojtik commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +the following command.
+
+```bash
+> kubectl rollout history --revision <revision>
+```
+
+### Rollback
+
+For future work, `kubeclt rollout undo` can be implemented in the general case 
+as an extension of the [above](#viewing-history ).
+
+```bash
+> kubectl rollout undo
+```
+
+Here `kubectl undo` simply uses strategic merge patch to apply the state 

how about ConfigMaps/Secrets attached to the Deployment? Will they be part of the history or we rollback and attach the current versions of them?

[Łukasz Oleś]

@lukaszo commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +To facilitate update and rollback for these controllers, and to provide a 
+primitive that third party controllers can build on, we propose a mechanism 
+that allows controllers to manage a bounded history of revisions to the declared 
+target state of their generated Objects.
+
+## Affected Components
+
+1. API Machinery
+1. API Server
+1. Kubectl
+1. Controllers that utilize the feature
+
+## Requirements
+
+1. History is a collection of points in time, and each point in time must be 
+represented by its own Object. While it is tempting to aggregate all of an 

Event if the points in time are the same we will have separate Objects for them? I'm talking about situation, when changing image: nging -> httpd -> nginx

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +To facilitate update and rollback for these controllers, and to provide a 
+primitive that third party controllers can build on, we propose a mechanism 
+that allows controllers to manage a bounded history of revisions to the declared 
+target state of their generated Objects.
+
+## Affected Components
+
+1. API Machinery
+1. API Server
+1. Kubectl
+1. Controllers that utilize the feature
+
+## Requirements
+
+1. History is a collection of points in time, and each point in time must be 
+represented by its own Object. While it is tempting to aggregate all of an 

No, in such a case the old object is reused.

[Łukasz Oleś]

@lukaszo commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +with the new target Object state.
+1. The controller will [maintain the length of its history](#history-maintenance) 
+to be less than the configured limit.
+
+### Version Snapshot Creation
+To take a snapshot of the target Object state contained in a specification type
+Object, a controller will do the following.
+
+1. The controller will serialize all the fields of the Object's target object 
+state using JSON encoding.
+1. The controller will populate the `.Data` of an AnyState object by 
+associating the serialized fields with keys that correspond to the fields' 
+names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.

In this case a new Controller should adopt the history first. Should this proposal also handle adoption?

[Łukasz Oleś]

@lukaszo commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +conflict, the method returns false.
+
+### Unique Name Generation
+We can use our [hash function](#hashsing) and 
+[collision resolution](#collision-resolution) scheme to generate a system 
+wide unique identifier for an Object based on a deterministic non-unique prefix 
+and a serialized representation of the Object. Kuberentes Object's `.Names` must 
+conform to a DNS subdomain. Therefore, the total length of the unique identifier
+must not exceed 255, and in practice 253, characters. We can generate a unique 
+identifier that meets this constraint by selecting a hash function such that the 
+output length is equal to `253-len(prefix)` and applying our [hash](#hashing) 
+function and [collision-resolution](#collision-resolution) scheme to the 
+serialized representation of the Object's data. The unique hash and integer can
+be combined to produce a unique suffix for the Object's `.Name`.
+
+1. We must also ensure that unique name does contain any bad words.

I hope you meant "doesn't"? :)

[Janet Kuo]

@janetkuo commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +the following command.
+
+```bash
+> kubectl rollout history --revision <revision>
+```
+
+### Rollback
+
+For future work, `kubeclt rollout undo` can be implemented in the general case 
+as an extension of the [above](#viewing-history ).
+
+```bash
+> kubectl rollout undo
+```
+
+Here `kubectl undo` simply uses strategic merge patch to apply the state 

Deployment won't use controller history (AnyState), it uses ReplicaSets instead. ConfigMap/Secret is part of Deployment history (ReplicaSets) since it's included in ReplicaSet template. The ConfigMap/Secret data is stored in the ConfigMap/Secret itself, not in the history.

ConfigMap rollout is discussed in kubernetes/kubernetes#22368

[Kenneth Owens]

@kow3ns commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +// AnyState implements an immutable dictionary of state data to allow for a 
+// point in time snapshot. Clients are responsible for serializing 
+// and deserializing the objects that contain their internal state and mapping 
+// them to a set of keys. 
+// Once an AnyState has been successfully created, it can not be updated. The 
+// API Server will fail validation of all update requests that attempt to mutate 
+// the Data field. AnyStates may, however, be deleted.
+type AnyState struct {
+    metav1.TypeMeta
+    // +optional
+    metav1.ObjectMeta
+    // Data is a map of keys to the serialized objects that comprise the state 
+    // snapshot.
+    // Each key must consist of alphanumeric characters, '-', '_' or '.'.
+    // +optional
+    Data map[string]string

@smarterclayton After trying the encoding using rutime.RawExtension I found that I could use it to encode arbitrary JSON or a registered Kind. Will remove the map[string]runtime.RawEncoding and replace it with runtime.RawEncoding.
@kargakis do you feel strongly about ControllerRevision. I have migrated it to ControllerHistory, but I actually like ControllerRevision better when looking at the implementation.
In order to use the plural for ControllerHistory when registering the type you end up with controllerhistories as the plural in the path. ControllerRevision seems better in the API. I would like to finalize the name today though.

[Kenneth Owens]

@kow3ns pushed 1 commit.

• c90ee9d Changed ControllerHistory to ControllerRevision

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or mute the thread.

[Michail Kargakis]

@kargakis commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +// AnyState implements an immutable dictionary of state data to allow for a 
+// point in time snapshot. Clients are responsible for serializing 
+// and deserializing the objects that contain their internal state and mapping 
+// them to a set of keys. 
+// Once an AnyState has been successfully created, it can not be updated. The 
+// API Server will fail validation of all update requests that attempt to mutate 
+// the Data field. AnyStates may, however, be deleted.
+type AnyState struct {
+    metav1.TypeMeta
+    // +optional
+    metav1.ObjectMeta
+    // Data is a map of keys to the serialized objects that comprise the state 
+    // snapshot.
+    // Each key must consist of alphanumeric characters, '-', '_' or '.'.
+    // +optional
+    Data map[string]string

+1 for ControllerRevision

cc: @smarterclayton @erictune @bgrant0607 @janetkuo @lukaszo @mfojtik @tnozicka @kubernetes/sig-apps-api-reviews thoughts?

—
You are receiving this because you are on a team that was mentioned.

Reply to this email directly, view it on GitHub, or mute the thread.

[Kenneth Owens]

@kow3ns pushed 1 commit.

• debc6e8 Fix ControllerRevision comments

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or mute the thread.

[Kenneth Owens]

@kow3ns pushed 1 commit.

• 4f75e51 Update comments on ControllerRevision.Data

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +with the new target Object state.
+1. The controller will [maintain the length of its history](#history-maintenance) 
+to be less than the configured limit.
+
+### Version Snapshot Creation
+To take a snapshot of the target Object state contained in a specification type
+Object, a controller will do the following.
+
+1. The controller will serialize all the fields of the Object's target object 
+state using JSON encoding.
+1. The controller will populate the `.Data` of an AnyState object by 
+associating the serialized fields with keys that correspond to the fields' 
+names. 
+   - e.g `PodTemplateSpecs` should be stored at the `"Template"` key.
+1. The controller will store the specification type Objects `.Generation` at the 
+`"ControllerGeneration"` key.

Array is:

Templates []History

type History struct {
  Purpose/Name string // i.e. "podTemplate"
  TypeMeta // the api and version of the historical snippet for use by the client
  Value runtime.RawExtension // the snippet
}

Purpose/Name would map back (same as the key in the map).

Another point - a pod template is serializable but a volume claim template is not (no type metadata). So there has to be additional versioning data attached if you want to store things that are not top level objects . You could alternatively require the controller to store a full object PersistentVolumeClaim instead of VolumeClaimTemplate and reconstitute the sub object from the full, or you could create a new VolumeClaimTemplate and attach object metadata (which is not used?). Ideally we would only serialize exactly what we need, but we would need to specify a group and version for the data we specify.

If serialized state can span two api groups/versions, we need to have distinct group versions. If the state must be intrinsic, the controller revision may store a single type/version (and arbitrary snippets that it can understand for that version). I kind of prefer the latter, which means you can serialize subsets of the object.

In proto serialization, do you want to store JSON?

—
You are receiving this because you are on a team that was mentioned.

Reply to this email directly, view it on GitHub, or mute the thread.

[Clayton Coleman]

@smarterclayton commented on this pull request.

---

In contributors/design-proposals/controller_history.md:

> +type ControllerRevision struct {
+    metav1.TypeMeta
+    // +optional
+    metav1.ObjectMeta
+    // Data contains the serialized state.
+    Data runtime.RawExtension
+    // Revision indicates the revision of the state represented by Data.
+    Revision int64
+}
+```
+
+## API Server
+The API Server must support the creation and deletion of ControllerRevision 
+objects. As we have no mechanism for declarative immutability, the API server 
+must fail any update request that updates the `.Data` field of a 
+ControllerRevision Object.

If you want, you can mention that by making it immutable now, in the future we can relax immutability when that construct is introduced.