Refine the Deployment proposal and switch hashing algorithm #477
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
@Kargakis we'd need to support migration if switching hashing algorithm, correct? |
|
@janetkuo correct, I am linking the implementation in the comment above yours. |
|
@janetkuo @kow3ns @bgrant0607 @smarterclayton @erictune updated as per our discussion yesterday cc: @kubernetes/sig-apps-api-reviews |
| @@ -65,7 +67,7 @@ const ( | |||
| // Kill all existing pods before creating new ones. | |||
| RecreateDeploymentStrategyType DeploymentStrategyType = "Recreate" | |||
|
|
|||
| // Replace the old RCs by new one using rolling update i.e gradually scale down the old RCs and scale up the new one. | |||
| // Replace the old RSs by new one using rolling update i.e gradually scale down the old RSs and scale up the new one. | |||
| notice that the second RC exists already which it can ramp up while ramping down | ||
| Users can cancel a rollout by doing a non-cascading deletion of the Deployment | ||
| before it is complete. Recreating the same Deployment will resume it. | ||
| For example, consider the following case: |
There was a problem hiding this comment.
This example shows how to pause and resume a rollout using non-cascading deletion, not cancel a rollout. However the first sentence goes "users can cancel a rollout..."
Plus switch from adler to fnv
|
As a side note: this proposal = |
|
I know that more than one person contributed significant time and ideas to this proposal. I think it would be a good idea to list each of them as authors at the top of the proposal. |
Agreed. Added commenters from the initial proposal plus anybody else that participated in the recent discussions around hashing vs templateGeneration. Let me know if I am missing somebody. |
|
Thanks for updating this. Please don't include me, though, as I didn't
spend much time on this.
…On Thu, Apr 27, 2017 at 3:04 AM, Michail Kargakis ***@***.***> wrote:
I know that more than one person contributed significant time and ideas to
this proposal. I think it would be a good idea to list each of them as
authors at the top of the proposal.
Agreed. Added commenters from the initial proposal plus anybody else that
participated in the recent discussions around hashing vs
templateGeneration. Let me know if I am missing somebody.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#477 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHuudthpJNaHESyjYx8VdGhpuAnpUJL8ks5r0GgmgaJpZM4Mm73M>
.
|
Includes commenters from the initial proposal plus anybody else that participated in the recent discussions around hashing vs templateGeneration. Signed-off-by: Michail Kargakis <mkargaki@redhat.com>
|
All comments addressed, I am going to merge this one and follow-up with the implementation which is in-flight in kubernetes/kubernetes#44774 |
Automatic merge from submit-queue Switch Deployments to new hashing algo w/ collision avoidance mechanism Implements kubernetes/community#477 @kubernetes/sig-apps-api-reviews @kubernetes/sig-apps-pr-reviews Fixes #29735 Fixes #43948 ```release-note Deployments are updated to use (1) a more stable hashing algorithm (fnv) than the previous one (adler) and (2) a hashing collision avoidance mechanism that will ensure new rollouts will not block on hashing collisions anymore. ```
|
So I don't see this in the doc, but I think a rather significant use case for collisions needs to be clarified: initializers on replica sets. Effectively anything that mutates or defaults the replicaset (so that those defaults are applied exactly once, and each version is consistent). We hit this on a prototype initializer that resolves image tags to fully qualified hashes, which causes the deployment controller to not recognize the template as it's own. I would not want collision avoidance to apply in this scenario... |
|
@smarterclayton I commented on this scenario here: kubernetes/kubernetes#1697 (comment) And in the PR that (erroneously) changed image from optional to required. I'd expect the Deployment's pod template to be updated, in which case there would be no collision. The user needs to specify the unresolved image tag in a different field. For instance, we could add an imageStream field (or struct) specifies the repo+image+tag. That would be automatically translated to the digest form into the existing image field in the DeploymentSpec, which would trigger the creation of a new ReplicaSet whose hash wouldn't collide with others'. |
|
I was also thinking of other examples as well - autolabellers or auto annotators, snapshotting config maps (at deployment time automatically copy config maps and set owner refs on them to a particular replica set), possibly sidecar injectors (I think injecting some sidecars at pod scope is wrong). Also node selectors and tolerations, conceivably. Are we ready to eliminate the possibility that replica set templates can be filled in (and thus preserve an atomic snapshot of rollout that can be reverted to under pressure)? I can think of lots of "on demand" policies that we can benefit from doing at the replica set level, where the natural action of the deployment (stamp a new desired RS out) can capture that point in time. |
|
The hash shouldn't be used for comparison. It should just be a uniquifier. Strategic merge diff should be used to determine whether any of the ReplicaSets matches the current Deployment template. The model I had in mind was that any automatic injection that we wanted to trigger a rollout would also modify DeploymentSpec fields that were not specified by the user. |
Automatic merge from submit-queue Switch Deployments to new hashing algo w/ collision avoidance mechanism Implements kubernetes/community#477 @kubernetes/sig-apps-api-reviews @kubernetes/sig-apps-pr-reviews Fixes kubernetes/kubernetes#29735 Fixes kubernetes/kubernetes#43948 ```release-note Deployments are updated to use (1) a more stable hashing algorithm (fnv) than the previous one (adler) and (2) a hashing collision avoidance mechanism that will ensure new rollouts will not block on hashing collisions anymore. ```
…t-proposal Refine the Deployment proposal and switch hashing algorithm
Alternative proposal to #384
@kubernetes/sig-apps-api-reviews @kubernetes/api-reviewers