Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow deployment rollback when in the process of cannary/partition #91

Merged
merged 1 commit into from
Feb 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 53 additions & 6 deletions pkg/internal/polymorphichelpers/rollback.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
kruiseclientsets "github.com/openkruise/kruise-api/client/clientset/versioned"
internalapps "github.com/openkruise/kruise-tools/pkg/internal/apps"

utils "github.com/openkruise/kruise-tools/pkg/utils"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
Expand Down Expand Up @@ -143,9 +144,6 @@ func (r *DeploymentRollbacker) Rollback(obj runtime.Object, updatedAnnotations m
if dryRunStrategy == cmdutil.DryRunClient {
return printTemplate(&rsForRevision.Spec.Template)
}
if deployment.Spec.Paused {
return "", fmt.Errorf("you cannot rollback a paused deployment; resume it first with 'kubectl rollout resume deployment/%s' and try again", name)
}

// Skip if the revision already matches current Deployment
if equalIgnoreHash(&rsForRevision.Spec.Template, &deployment.Spec.Template) {
Expand All @@ -157,13 +155,18 @@ func (r *DeploymentRollbacker) Rollback(obj runtime.Object, updatedAnnotations m

// compute deployment annotations
annotations := map[string]string{}
for k := range annotationsToSkip {
if v, ok := deployment.Annotations[k]; ok {

// In the same vein as annotationsToSkip, which records annotations to exclude,
// IsKruiseRolloutsAnnotation checks whether an annotation is generated by kruise-rollout.
// Annotations identified as generated by kruise-rollout
// will be skipped when copying from ReplicaSet annotations to Deployment annotations.
for k, v := range deployment.Annotations {
if annotationsToSkip[k] || utils.IsKruiseRolloutsAnnotation(&k) {
annotations[k] = v
}
}
for k, v := range rsForRevision.Annotations {
if !annotationsToSkip[k] {
if !annotationsToSkip[k] && !utils.IsKruiseRolloutsAnnotation(&k) {
annotations[k] = v
}
}
Expand Down Expand Up @@ -270,6 +273,50 @@ func deploymentRevision(deployment *appsv1.Deployment, c kubernetes.Interface, t
return nil, revisionNotFoundErr(toRevision)
}

// even a deployment is paused, rollout history can update if the current rollout progress is
// actually a rollback, for example:

// Step1. Initially, there's only one revision for the deployment:
// REVISION CHANGE-CAUSE
// 1 <none>
// before the first canary release is completed, the rollout history won't update since the original deployment
// is paused. If someone decides to rollback during this release (before it is completed),
// we should return the latestReplicaSet instead of the previousReplicaSet.

// Step2. After a canary release completed, the rollout history will be like:
// REVISION CHANGE-CAUSE
// 1 <none>
// 2 <none>

// Step3. Someone decides to rollback after this release using rollout undo.
// It is ok, and it will be seen as a new canary release.
// However, the rollout history will update though the original deployment is paused, because
// the "new" canary revision can be found from the rollout history, which will be like:
// REVISION CHANGE-CAUSE
// 2 <none>
// 3 <none>

// Step4. Someone decides to rollback during this rollback progress (before it is completed),
// that means rolling back to the revision referred in the Step2.
// We should return the previousReplicaSet instead of the latestReplicaSet.
if utils.InCanaryProgress(deployment) {
if latestReplicaSet == nil {
return nil, fmt.Errorf("no rollout history found for deployment %q", deployment.Name)
}
// only one revison found, then return it, equalIgnoreHash will be called later in Rollback function
if previousReplicaSet == nil {
return latestReplicaSet, nil
}
// possibly attempting to rollback during a rollback progress, so we return previousReplicaSet,
// since the rollout history is updated
if equalIgnoreHash(&latestReplicaSet.Spec.Template, &deployment.Spec.Template) {
return previousReplicaSet, nil
}
// attempting to rollback during a normal publication,
// so we return latestReplicaSet since the rollout history hasn't updated
return latestReplicaSet, nil
}

if previousReplicaSet == nil {
return nil, fmt.Errorf("no rollout history found for deployment %q", deployment.Name)
}
Expand Down
33 changes: 33 additions & 0 deletions pkg/utils/misc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package utils

import (
"strings"

appsv1 "k8s.io/api/apps/v1"
)

const InRolloutProgressingAnnotation = "rollouts.kruise.io/in-progressing"
const DeploymentStrategyAnnotation = "rollouts.kruise.io/deployment-strategy"

func IsKruiseRolloutsAnnotation(s *string) bool {
if s == nil {
return false
}
const prefix = "rollouts.kruise.io/"
return strings.Contains(*s, prefix)
}

func InCanaryProgress(deployment *appsv1.Deployment) bool {
if !deployment.Spec.Paused {
return false
}
//if deployment has InRolloutProgressingAnnotation, it is under kruise-rollout control
if _, ok := deployment.Annotations[InRolloutProgressingAnnotation]; !ok {
return false
}
// only if deployment strategy is 'partition', webhook would add DeploymentStrategyAnnotation
if _, ok := deployment.Annotations[DeploymentStrategyAnnotation]; ok {
return false
}
return true
}
Loading