From 12ed35ed7eae09101f1a4631a97d55d9ada92b27 Mon Sep 17 00:00:00 2001 From: Eniko Dif Date: Tue, 14 Apr 2026 17:15:33 +0200 Subject: [PATCH 1/6] Implement Rollback --- api/v1alpha1/workerdeployment_types.go | 31 ++ api/v1alpha1/workerdeployment_webhook.go | 57 ++- api/v1alpha1/workerdeployment_webhook_test.go | 83 +++++ api/v1alpha1/zz_generated.deepcopy.go | 25 ++ ...temporal.io_temporalworkerdeployments.yaml | 24 ++ internal/controller/genplan.go | 76 ++-- internal/k8s/deployments_test.go | 10 +- internal/planner/planner.go | 61 ++- internal/planner/planner_test.go | 350 ++++++++++++++++++ internal/temporal/worker_deployment.go | 9 + internal/testhelpers/make.go | 42 ++- internal/testhelpers/test_builder.go | 48 ++- internal/testhelpers/workers.go | 16 + .../tests/internal/deployment_controller.go | 44 ++- internal/tests/internal/integration_test.go | 80 +++- 15 files changed, 884 insertions(+), 72 deletions(-) diff --git a/api/v1alpha1/workerdeployment_types.go b/api/v1alpha1/workerdeployment_types.go index 2f4930de..d0077317 100644 --- a/api/v1alpha1/workerdeployment_types.go +++ b/api/v1alpha1/workerdeployment_types.go @@ -83,6 +83,10 @@ type WorkerDeploymentSpec struct { // How to rollout new workflow executions to the target version. RolloutStrategy RolloutStrategy `json:"rollout"` + // How to rollback to a previous version. If not specified, defaults to AllAtOnce strategy. + // +optional + RollbackStrategy *RollbackStrategy `json:"rollback,omitempty"` + // How to manage sunsetting drained versions. SunsetStrategy SunsetStrategy `json:"sunset"` @@ -361,6 +365,18 @@ const ( UpdateProgressive DefaultVersionUpdateStrategy = "Progressive" ) +// DefaultVersionRollbackStrategy describes how to cut over during rollback to a previous version. +// +kubebuilder:validation:Enum=AllAtOnce;Progressive +type DefaultVersionRollbackStrategy string + +const ( + // RollbackAllAtOnce immediately switches 100% of traffic back to the previous version. + RollbackAllAtOnce DefaultVersionRollbackStrategy = "AllAtOnce" + + // RollbackProgressive gradually ramps traffic back to the previous version. + RollbackProgressive DefaultVersionRollbackStrategy = "Progressive" +) + type GateWorkflowConfig struct { WorkflowType string `json:"workflowType"` // Input is an arbitrary JSON object passed as the first parameter to the gate workflow. @@ -405,6 +421,21 @@ type RolloutStrategy struct { Steps []RolloutStep `json:"steps,omitempty" protobuf:"bytes,3,rep,name=steps"` } +// RollbackStrategy defines strategy to apply when rolling back to a previous version. +// This is separate from RolloutStrategy because rollbacks have different requirements: +// - No gate workflow (already trusted version) +// - No manual mode (rollbacks should be automatic) +// - Default to AllAtOnce for fast recovery +type RollbackStrategy struct { + // Strategy for rollback. Valid values are "AllAtOnce" or "Progressive". + // Defaults to "AllAtOnce" for fast recovery. + Strategy DefaultVersionRollbackStrategy `json:"strategy"` + + // Steps to execute progressive rollbacks. Only required when strategy is "Progressive". + // +optional + Steps []RolloutStep `json:"steps,omitempty"` +} + // SunsetStrategy defines strategy to apply when sunsetting k8s deployments of drained versions. type SunsetStrategy struct { // ScaledownDelay specifies how long to wait after a version is drained before scaling its Deployment to zero. diff --git a/api/v1alpha1/workerdeployment_webhook.go b/api/v1alpha1/workerdeployment_webhook.go index f8d8faba..2ab88fad 100644 --- a/api/v1alpha1/workerdeployment_webhook.go +++ b/api/v1alpha1/workerdeployment_webhook.go @@ -7,6 +7,7 @@ package v1alpha1 import ( "context" "fmt" + "time" "github.com/temporalio/temporal-worker-controller/internal/defaults" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -53,6 +54,11 @@ func (s *WorkerDeploymentSpec) Default(ctx context.Context) error { s.SunsetStrategy.DeleteDelay = &v1.Duration{Duration: defaults.DeleteDelay} } + if s.RollbackStrategy == nil { + s.RollbackStrategy = &RollbackStrategy{Strategy: RollbackAllAtOnce} + } else if s.RollbackStrategy.Strategy == "" { + s.RollbackStrategy.Strategy = RollbackAllAtOnce + } return nil } @@ -81,7 +87,11 @@ func (r *WorkerDeployment) validateForUpdateOrCreate(ctx context.Context, obj ru } func validateForUpdateOrCreate(old, new *WorkerDeployment) (admission.Warnings, error) { - allErrs := validateRolloutStrategy(new.Spec.RolloutStrategy) + var allErrs field.ErrorList + allErrs = append(allErrs, validateRolloutStrategy(new.Spec.RolloutStrategy)...) + if new.Spec.RollbackStrategy != nil { + allErrs = append(allErrs, validateRollbackStrategy(*new.Spec.RollbackStrategy)...) + } if len(allErrs) > 0 { return nil, newInvalidErr(new, allErrs) } @@ -96,15 +106,7 @@ func validateRolloutStrategy(s RolloutStrategy) []*field.Error { var allErrs []*field.Error if s.Strategy == UpdateProgressive { - var lastRamp int - for i, step := range s.Steps { - if step.RampPercentage <= lastRamp { - allErrs = append(allErrs, - field.Invalid(field.NewPath(fmt.Sprintf("spec.rollout.steps[%d].rampPercentage", i)), step.RampPercentage, "rampPercentage must increase between each step"), - ) - } - lastRamp = step.RampPercentage - } + allErrs = append(allErrs, validateProgressiveStrategySteps("spec.rollout.steps", s.Steps)...) } if s.Gate != nil && s.Gate.Input != nil && s.Gate.InputFrom != nil { @@ -117,6 +119,41 @@ func validateRolloutStrategy(s RolloutStrategy) []*field.Error { return allErrs } +func validateRollbackStrategy(s RollbackStrategy) []*field.Error { + var allErrs []*field.Error + if s.Strategy == RollbackProgressive { + allErrs = append(allErrs, validateProgressiveStrategySteps("spec.rollback.steps", s.Steps)...) + } + return allErrs +} + +func validateProgressiveStrategySteps(specName string, steps []RolloutStep) []*field.Error { + var allErrs []*field.Error + + if len(steps) == 0 { + allErrs = append(allErrs, + field.Invalid(field.NewPath(specName), steps, "steps are required for Progressive strategy"), + ) + } + + var lastRamp int + for i, step := range steps { + if step.PauseDuration.Duration < 30*time.Second { + allErrs = append(allErrs, + field.Invalid(field.NewPath(fmt.Sprintf("%s[%d].pauseDuration", specName, i)), step.PauseDuration.Duration.String(), "pause duration must be at least 30s"), + ) + } + if step.RampPercentage <= lastRamp { + allErrs = append(allErrs, + field.Invalid(field.NewPath(fmt.Sprintf("%s[%d].rampPercentage", specName, i)), step.RampPercentage, "rampPercentage must increase between each step"), + ) + } + lastRamp = step.RampPercentage + } + + return allErrs +} + func newInvalidErr(dep *WorkerDeployment, errs field.ErrorList) *apierrors.StatusError { return apierrors.NewInvalid(dep.GroupVersionKind().GroupKind(), dep.GetName(), errs) } diff --git a/api/v1alpha1/workerdeployment_webhook_test.go b/api/v1alpha1/workerdeployment_webhook_test.go index 725927e3..e0b2a591 100644 --- a/api/v1alpha1/workerdeployment_webhook_test.go +++ b/api/v1alpha1/workerdeployment_webhook_test.go @@ -50,6 +50,52 @@ func TestWorkerDeployment_ValidateCreate(t *testing.T) { }), errorMsg: "[spec.rollout.steps[2].rampPercentage: Invalid value: 9: rampPercentage must increase between each step, spec.rollout.steps[4].rampPercentage: Invalid value: 50: rampPercentage must increase between each step]", }, + "rollback strategy - valid Progressive with steps": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-progressive", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {50, metav1.Duration{Duration: 30 * time.Second}}, + }, + } + return obj + }), + }, + "rollback strategy - invalid Progressive without steps": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-progressive-no-steps", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackProgressive, + Steps: nil, + } + return obj + }), + errorMsg: "steps are required for Progressive strategy", + }, + "rollback strategy - invalid Progressive pause duration < 30s": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-progressive-invalid", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {50, metav1.Duration{Duration: 10 * time.Second}}, + }, + } + return obj + }), + errorMsg: "pause duration must be at least 30s", + }, + "rollback strategy - invalid Progressive with non-increasing ramp": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-progressive-decreasing", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {50, metav1.Duration{Duration: time.Minute}}, + {25, metav1.Duration{Duration: time.Minute}}, + }, + } + return obj + }), + errorMsg: "rampPercentage must increase between each step", + }, } for name, tc := range tests { @@ -139,6 +185,43 @@ func TestWorkerDeployment_Default(t *testing.T) { assert.Equal(t, 24*time.Hour, obj.Spec.SunsetStrategy.DeleteDelay.Duration) }, }, + "rollback strategy initialized when nil": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("default-rollback-nil", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RollbackStrategy = nil + return obj + }), + expected: func(t *testing.T, obj *temporaliov1alpha1.WorkerDeployment) { + require.NotNil(t, obj.Spec.RollbackStrategy, "expected RollbackStrategy to be initialized by webhook") + assert.Equal(t, temporaliov1alpha1.RollbackAllAtOnce, obj.Spec.RollbackStrategy.Strategy, "expected RollbackStrategy.Strategy to default to AllAtOnce") + }, + }, + "rollback strategy defaults empty strategy field to AllAtOnce": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("default-rollback-empty", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: "", + } + return obj + }), + expected: func(t *testing.T, obj *temporaliov1alpha1.WorkerDeployment) { + require.NotNil(t, obj.Spec.RollbackStrategy) + assert.Equal(t, temporaliov1alpha1.RollbackAllAtOnce, obj.Spec.RollbackStrategy.Strategy, "expected RollbackStrategy.Strategy to default to AllAtOnce") + }, + }, + "rollback strategy preserves explicit strategy": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("explicit-rollback-progressive", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {50, metav1.Duration{Duration: 30 * time.Second}}, + }, + } + return obj + }), + expected: func(t *testing.T, obj *temporaliov1alpha1.WorkerDeployment) { + require.NotNil(t, obj.Spec.RollbackStrategy) + assert.Equal(t, temporaliov1alpha1.RollbackProgressive, obj.Spec.RollbackStrategy.Strategy, "expected RollbackStrategy.Strategy to remain Progressive") + }, + }, } for name, tc := range tests { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index f4b8a3c5..79f4b37b 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -286,6 +286,26 @@ func (in *ManualRolloutStrategy) DeepCopy() *ManualRolloutStrategy { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RollbackStrategy) DeepCopyInto(out *RollbackStrategy) { + *out = *in + if in.Steps != nil { + in, out := &in.Steps, &out.Steps + *out = make([]RolloutStep, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RollbackStrategy. +func (in *RollbackStrategy) DeepCopy() *RollbackStrategy { + if in == nil { + return nil + } + out := new(RollbackStrategy) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RolloutStep) DeepCopyInto(out *RolloutStep) { *out = *in @@ -611,6 +631,11 @@ func (in *TemporalWorkerDeploymentSpec) DeepCopyInto(out *TemporalWorkerDeployme **out = **in } in.RolloutStrategy.DeepCopyInto(&out.RolloutStrategy) + if in.RollbackStrategy != nil { + in, out := &in.RollbackStrategy, &out.RollbackStrategy + *out = new(RollbackStrategy) + (*in).DeepCopyInto(*out) + } in.SunsetStrategy.DeepCopyInto(&out.SunsetStrategy) out.WorkerOptions = in.WorkerOptions } diff --git a/helm/temporal-worker-controller-crds/templates/temporal.io_temporalworkerdeployments.yaml b/helm/temporal-worker-controller-crds/templates/temporal.io_temporalworkerdeployments.yaml index adaaa797..6b222cf7 100644 --- a/helm/temporal-worker-controller-crds/templates/temporal.io_temporalworkerdeployments.yaml +++ b/helm/temporal-worker-controller-crds/templates/temporal.io_temporalworkerdeployments.yaml @@ -61,6 +61,30 @@ spec: replicas: format: int32 type: integer + rollback: + properties: + steps: + items: + properties: + pauseDuration: + type: string + rampPercentage: + maximum: 99 + minimum: 1 + type: integer + required: + - pauseDuration + - rampPercentage + type: object + type: array + strategy: + enum: + - AllAtOnce + - Progressive + type: string + required: + - strategy + type: object rollout: properties: gate: diff --git a/internal/controller/genplan.go b/internal/controller/genplan.go index fe0656f0..55a89147 100644 --- a/internal/controller/genplan.go +++ b/internal/controller/genplan.go @@ -92,43 +92,18 @@ func (r *WorkerDeploymentReconciler) generatePlan( } rolloutStrategy := w.Spec.RolloutStrategy + rollbackStrategy := w.Spec.RollbackStrategy - // Resolve gate input if gate is configured - var gateInput []byte - var isGateInputSecret bool - if rolloutStrategy.Gate != nil { - // Fetch ConfigMap or Secret data if needed - var configMapData map[string]string - var configMapBinaryData map[string][]byte - var secretData map[string][]byte - - if rolloutStrategy.Gate.InputFrom != nil { - if cmRef := rolloutStrategy.Gate.InputFrom.ConfigMapKeyRef; cmRef != nil { - cm := &corev1.ConfigMap{} - if err := r.Client.Get(ctx, types.NamespacedName{Namespace: w.Namespace, Name: cmRef.Name}, cm); err != nil { - return nil, fmt.Errorf("failed to get ConfigMap %s/%s: %w", w.Namespace, cmRef.Name, err) - } - configMapData = cm.Data - configMapBinaryData = cm.BinaryData - } - if secRef := rolloutStrategy.Gate.InputFrom.SecretKeyRef; secRef != nil { - sec := &corev1.Secret{} - if err := r.Client.Get(ctx, types.NamespacedName{Namespace: w.Namespace, Name: secRef.Name}, sec); err != nil { - return nil, fmt.Errorf("failed to get Secret %s/%s: %w", w.Namespace, secRef.Name, err) - } - secretData = sec.Data - } - } - - gateInput, isGateInputSecret, err = planner.ResolveGateInput(rolloutStrategy.Gate, w.Namespace, configMapData, configMapBinaryData, secretData) - if err != nil { - return nil, fmt.Errorf("unable to resolve gate input: %w", err) - } + // Resolve gate workflow if needed + gateInput, isGateInputSecret, err := r.resolveGateWorkflow(ctx, l, w, rolloutStrategy, temporalState) + if err != nil { + return nil, fmt.Errorf("unable to resolve gate input: %w", err) } // Generate the plan using the planner package plannerConfig := &planner.Config{ - RolloutStrategy: rolloutStrategy, + RolloutStrategy: rolloutStrategy, + RollbackStrategy: rollbackStrategy, } // Fetch all WorkerResourceTemplates that reference this TWD so that the planner @@ -197,6 +172,43 @@ func (r *WorkerDeploymentReconciler) generatePlan( return plan, nil } +func (r *TemporalWorkerDeploymentReconciler) resolveGateWorkflow( + ctx context.Context, + l logr.Logger, + w *temporaliov1alpha1.TemporalWorkerDeployment, + rolloutStrategy temporaliov1alpha1.RolloutStrategy, + temporalState *temporal.TemporalWorkerState, +) (gateInput []byte, isSecret bool, err error) { + if rolloutStrategy.Gate == nil { + return nil, false, nil + } + + // Fetch ConfigMap or Secret data if needed + var configMapData map[string]string + var configMapBinaryData map[string][]byte + var secretData map[string][]byte + + if rolloutStrategy.Gate.InputFrom != nil { + if cmRef := rolloutStrategy.Gate.InputFrom.ConfigMapKeyRef; cmRef != nil { + cm := &corev1.ConfigMap{} + if err := r.Client.Get(ctx, types.NamespacedName{Namespace: w.Namespace, Name: cmRef.Name}, cm); err != nil { + return nil, false, fmt.Errorf("failed to get ConfigMap %s/%s: %w", w.Namespace, cmRef.Name, err) + } + configMapData = cm.Data + configMapBinaryData = cm.BinaryData + } + if secRef := rolloutStrategy.Gate.InputFrom.SecretKeyRef; secRef != nil { + sec := &corev1.Secret{} + if err := r.Client.Get(ctx, types.NamespacedName{Namespace: w.Namespace, Name: secRef.Name}, sec); err != nil { + return nil, false, fmt.Errorf("failed to get Secret %s/%s: %w", w.Namespace, secRef.Name, err) + } + secretData = sec.Data + } + } + + return planner.ResolveGateInput(rolloutStrategy.Gate, w.Namespace, configMapData, configMapBinaryData, secretData) +} + // Create a new deployment with owner reference func (r *WorkerDeploymentReconciler) newDeployment( w *temporaliov1alpha1.WorkerDeployment, diff --git a/internal/k8s/deployments_test.go b/internal/k8s/deployments_test.go index 37ed22f4..4e5a0235 100644 --- a/internal/k8s/deployments_test.go +++ b/internal/k8s/deployments_test.go @@ -233,8 +233,8 @@ func TestGenerateBuildID(t *testing.T) { pod1 := testhelpers.MakePodSpec([]corev1.Container{{Image: img}}, map[string]string{"pod": "1"}, "") pod2 := testhelpers.MakePodSpec([]corev1.Container{{Image: img}}, map[string]string{"pod": "2"}, "") - twd1 := testhelpers.MakeWD("", "", 1, pod1, nil, nil, nil) - twd2 := testhelpers.MakeWD("", "", 1, pod2, nil, nil, nil) + twd1 := testhelpers.MakeWD("", "", 1, pod1, nil, nil, nil, nil) + twd2 := testhelpers.MakeWD("", "", 1, pod2, nil, nil, nil, nil) return twd1, twd2 }, expectedPrefix: "my.test_image", @@ -246,8 +246,8 @@ func TestGenerateBuildID(t *testing.T) { generateInputs: func() (*temporaliov1alpha1.WorkerDeployment, *temporaliov1alpha1.WorkerDeployment) { img := "my.test_image" pod := testhelpers.MakePodSpec([]corev1.Container{{Image: img}}, nil, "") - twd1 := testhelpers.MakeWD("", "", 1, pod, nil, nil, nil) - twd2 := testhelpers.MakeWD("", "", 2, pod, nil, nil, nil) + twd1 := testhelpers.MakeWD("", "", 1, pod, nil, nil, nil, nil) + twd2 := testhelpers.MakeWD("", "", 2, pod, nil, nil, nil, nil) return twd1, twd2 }, expectedPrefix: "my.test_image", @@ -257,7 +257,7 @@ func TestGenerateBuildID(t *testing.T) { { name: "no containers", generateInputs: func() (*temporaliov1alpha1.WorkerDeployment, *temporaliov1alpha1.WorkerDeployment) { - twd := testhelpers.MakeWD("", "", 1, testhelpers.MakePodSpec(nil, nil, ""), nil, nil, nil) + twd := testhelpers.MakeWD("", "", 1, testhelpers.MakePodSpec(nil, nil, ""), nil, nil, nil, nil) return twd, nil // only check 1 result, no need to compare }, expectedPrefix: "", diff --git a/internal/planner/planner.go b/internal/planner/planner.go index fd96d58d..04dc0e4d 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -118,6 +118,8 @@ type WorkflowConfig struct { type Config struct { // RolloutStrategy to use RolloutStrategy temporaliov1alpha1.RolloutStrategy + // RollbackStrategy to use + RollbackStrategy *temporaliov1alpha1.RollbackStrategy } // GeneratePlan creates a plan for updating the worker deployment @@ -759,7 +761,34 @@ func getTestWorkflows( return testWorkflows } -// getVersionConfigDiff determines the version configuration based on the rollout strategy +func isRollbackScenario( + l logr.Logger, + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus, + temporalState *temporal.TemporalWorkerState, +) bool { + if temporalState == nil { + return false + } + + targetVersionInfo, exists := temporalState.Versions[status.TargetVersion.BuildID] + if !exists { + return false + } + + if targetVersionInfo.LastCurrentTime == nil { + return false + } + + l.Info("Detected rollback scenario using LastCurrentTime. "+ + "Warning: Auto-upgrade workflows that upgraded from a previous version to the current version may fail during this rollback, "+ + "as they may not handle downgrades properly. Monitor workflow executions for failures.", + "targetBuildID", status.TargetVersion.BuildID, + "lastCurrentTime", targetVersionInfo.LastCurrentTime) + + return true +} + +// getVersionConfigDiff determines the version configuration based on the rollout/rollback strategies func getVersionConfigDiff( l logr.Logger, status *temporaliov1alpha1.WorkerDeploymentStatus, @@ -767,9 +796,14 @@ func getVersionConfigDiff( config *Config, workerDeploymentName string, ) *VersionConfig { - strategy := config.RolloutStrategy - conflictToken := status.VersionConflictToken + var strategy temporaliov1alpha1.RolloutStrategy + if isRollbackScenario(l, status, temporalState) { + strategy = convertRollbackToRolloutStrategy(*config.RollbackStrategy) + } else { + strategy = config.RolloutStrategy + } + // Manual strategy check (only relevant for rollout) if strategy.Strategy == temporaliov1alpha1.UpdateManual { return nil } @@ -800,7 +834,7 @@ func getVersionConfigDiff( managerIdentity = temporalState.ManagerIdentity } vcfg := &VersionConfig{ - ConflictToken: conflictToken, + ConflictToken: status.VersionConflictToken, BuildID: status.TargetVersion.BuildID, ManagerIdentity: managerIdentity, } @@ -840,6 +874,25 @@ func getVersionConfigDiff( return nil } +// Convert to reuse rollout logic with different settings +func convertRollbackToRolloutStrategy(rb temporaliov1alpha1.RollbackStrategy) temporaliov1alpha1.RolloutStrategy { + var strategy temporaliov1alpha1.DefaultVersionUpdateStrategy + switch rb.Strategy { + case temporaliov1alpha1.RollbackAllAtOnce: + strategy = temporaliov1alpha1.UpdateAllAtOnce + case temporaliov1alpha1.RollbackProgressive: + strategy = temporaliov1alpha1.UpdateProgressive + default: + strategy = temporaliov1alpha1.UpdateAllAtOnce + } + + return temporaliov1alpha1.RolloutStrategy{ + Strategy: strategy, + Steps: rb.Steps, + Gate: nil, // Rollbacks don't have gates + } +} + // handleProgressiveRollout handles the progressive rollout strategy logic func handleProgressiveRollout( steps []temporaliov1alpha1.RolloutStep, diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index e57f2dfe..530e8626 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -3581,3 +3581,353 @@ func TestGetWRTOwnerRefPatches(t *testing.T) { require.Len(t, patches, 1) }) } + +func TestIsRollbackScenario(t *testing.T) { + lastCurrentTime := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) + testCases := []struct { + name string + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus + temporalState *temporal.TemporalWorkerState + expectedResult bool + }{ + { + name: "rollback detected via LastCurrentTime", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v1", + }, + }, + }, + temporalState: &temporal.TemporalWorkerState{ + Versions: map[string]*temporal.VersionInfo{ + "build-v1": { + BuildID: "build-v1", + LastCurrentTime: &lastCurrentTime, + }, + }, + }, + expectedResult: true, + }, + { + name: "rollout when LastCurrentTime is nil", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v2", + }, + }, + }, + temporalState: &temporal.TemporalWorkerState{ + Versions: map[string]*temporal.VersionInfo{ + "build-v2": { + BuildID: "build-v2", + LastCurrentTime: nil, + }, + }, + }, + expectedResult: false, + }, + { + name: "rollout when target version not in temporal state", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v3", + }, + }, + }, + temporalState: &temporal.TemporalWorkerState{ + Versions: map[string]*temporal.VersionInfo{ + "build-v1": { + BuildID: "build-v1", + LastCurrentTime: &lastCurrentTime, + }, + }, + }, + expectedResult: false, + }, + { + name: "rollout when temporalState is nil", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v1", + }, + }, + }, + temporalState: nil, + expectedResult: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + logger := logr.Discard() + result := isRollbackScenario(logger, tc.status, tc.temporalState) + assert.Equal(t, tc.expectedResult, result) + }) + } +} + +func TestConvertRollbackToRolloutStrategy(t *testing.T) { + testCases := []struct { + name string + rollbackStrategy temporaliov1alpha1.RollbackStrategy + expectedStrategy temporaliov1alpha1.DefaultVersionUpdateStrategy + expectedSteps []temporaliov1alpha1.RolloutStep + expectedGate *temporaliov1alpha1.GateWorkflowConfig + }{ + { + name: "AllAtOnce rollback converts to AllAtOnce rollout", + rollbackStrategy: temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + }, + expectedStrategy: temporaliov1alpha1.UpdateAllAtOnce, + expectedSteps: nil, + expectedGate: nil, + }, + { + name: "Progressive rollback preserves all steps in order", + rollbackStrategy: temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {RampPercentage: 25, PauseDuration: metav1.Duration{Duration: 30 * time.Second}}, + {RampPercentage: 50, PauseDuration: metav1.Duration{Duration: time.Minute}}, + {RampPercentage: 75, PauseDuration: metav1.Duration{Duration: 2 * time.Minute}}, + }, + }, + expectedStrategy: temporaliov1alpha1.UpdateProgressive, + expectedSteps: []temporaliov1alpha1.RolloutStep{ + {RampPercentage: 25, PauseDuration: metav1.Duration{Duration: 30 * time.Second}}, + {RampPercentage: 50, PauseDuration: metav1.Duration{Duration: time.Minute}}, + {RampPercentage: 75, PauseDuration: metav1.Duration{Duration: 2 * time.Minute}}, + }, + expectedGate: nil, + }, + { + name: "empty strategy defaults to AllAtOnce", + rollbackStrategy: temporaliov1alpha1.RollbackStrategy{ + Strategy: "", + }, + expectedStrategy: temporaliov1alpha1.UpdateAllAtOnce, + expectedSteps: nil, + expectedGate: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := convertRollbackToRolloutStrategy(tc.rollbackStrategy) + + assert.Equal(t, tc.expectedStrategy, result.Strategy) + assert.Equal(t, tc.expectedSteps, result.Steps) + assert.Equal(t, tc.expectedGate, result.Gate) + }) + } +} + +func TestGetVersionConfigDiff_RollbackScenario(t *testing.T) { + logger := logr.Discard() + workerDeploymentName := "test-deployment" + now := time.Now() + zeroRamp := float32(0) + + testCases := []struct { + name string + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus + temporalState *temporal.TemporalWorkerState + config *Config + expectSetCurrent bool + description string + }{ + { + name: "rollback with default AllAtOnce strategy", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v1", + Status: temporaliov1alpha1.VersionStatusInactive, + HealthySince: &metav1.Time{Time: now}, + }, + }, + VersionConflictToken: []byte("token123"), + }, + temporalState: &temporal.TemporalWorkerState{ + Versions: map[string]*temporal.VersionInfo{ + "build-v1": { + BuildID: "build-v1", + LastCurrentTime: &now, + Status: temporaliov1alpha1.VersionStatusInactive, + }, + }, + }, + config: &Config{ + RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {RampPercentage: 25, PauseDuration: metav1.Duration{Duration: 5 * time.Minute}}, + {RampPercentage: 50, PauseDuration: metav1.Duration{Duration: 5 * time.Minute}}, + }, + }, + RollbackStrategy: &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + }, + }, + expectSetCurrent: true, + description: "Rollback with AllAtOnce should immediately set version as current", + }, + { + name: "rollback with Progressive strategy", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v2", + Status: temporaliov1alpha1.VersionStatusCurrent, + HealthySince: &metav1.Time{Time: now}, + }, + }, + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v1", + Status: temporaliov1alpha1.VersionStatusInactive, + HealthySince: &metav1.Time{Time: now}, + }, + RampPercentage: &zeroRamp, + }, + VersionConflictToken: []byte("token123"), + }, + temporalState: &temporal.TemporalWorkerState{ + Versions: map[string]*temporal.VersionInfo{ + "build-v1": { + BuildID: "build-v1", + LastCurrentTime: &now, + Status: temporaliov1alpha1.VersionStatusInactive, + }, + "build-v2": { + BuildID: "build-v2", + LastCurrentTime: nil, + Status: temporaliov1alpha1.VersionStatusCurrent, + }, + }, + }, + config: &Config{ + RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateAllAtOnce, + }, + RollbackStrategy: &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {RampPercentage: 50, PauseDuration: metav1.Duration{Duration: time.Minute}}, + }, + }, + }, + expectSetCurrent: false, + description: "Rollback with Progressive should not immediately set as current", + }, + { + name: "normal rollout when LastCurrentTime is nil", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v1", + Status: temporaliov1alpha1.VersionStatusCurrent, + HealthySince: &metav1.Time{Time: now}, + }, + }, + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v2", + Status: temporaliov1alpha1.VersionStatusInactive, + HealthySince: &metav1.Time{Time: now}, + }, + }, + VersionConflictToken: []byte("token456"), + }, + temporalState: &temporal.TemporalWorkerState{ + Versions: map[string]*temporal.VersionInfo{ + "build-v1": { + BuildID: "build-v1", + LastCurrentTime: &now, + Status: temporaliov1alpha1.VersionStatusCurrent, + }, + "build-v2": { + BuildID: "build-v2", + LastCurrentTime: nil, + Status: temporaliov1alpha1.VersionStatusInactive, + }, + }, + }, + config: &Config{ + RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {RampPercentage: 50, PauseDuration: metav1.Duration{Duration: time.Minute}}, + }, + }, + RollbackStrategy: &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackAllAtOnce, // would set current immediately if wrongly used + }, + }, + expectSetCurrent: false, + description: "New version (nil LastCurrentTime) should use RolloutStrategy, not rollback AllAtOnce", + }, + { + name: "rollback when target version is drained", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v2", + Status: temporaliov1alpha1.VersionStatusCurrent, + HealthySince: &metav1.Time{Time: now}, + }, + }, + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v1", + Status: temporaliov1alpha1.VersionStatusDrained, + HealthySince: &metav1.Time{Time: now}, + }, + }, + VersionConflictToken: []byte("token789"), + }, + temporalState: &temporal.TemporalWorkerState{ + Versions: map[string]*temporal.VersionInfo{ + "build-v1": { + BuildID: "build-v1", + LastCurrentTime: &now, + Status: temporaliov1alpha1.VersionStatusDrained, + }, + "build-v2": { + BuildID: "build-v2", + LastCurrentTime: nil, + Status: temporaliov1alpha1.VersionStatusCurrent, + }, + }, + }, + config: &Config{ + RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateAllAtOnce, + }, + RollbackStrategy: &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + }, + }, + expectSetCurrent: true, + description: "Rollback to a previously-drained version should be detected via LastCurrentTime and immediately set as current", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := getVersionConfigDiff(logger, tc.status, tc.temporalState, tc.config, workerDeploymentName) + + if result == nil { + t.Fatal("expected non-nil VersionConfig") + } + + assert.Equal(t, tc.expectSetCurrent, result.SetCurrent, tc.description) + assert.Equal(t, tc.status.VersionConflictToken, result.ConflictToken) + }) + } +} diff --git a/internal/temporal/worker_deployment.go b/internal/temporal/worker_deployment.go index b05eaf40..a839b8b2 100644 --- a/internal/temporal/worker_deployment.go +++ b/internal/temporal/worker_deployment.go @@ -38,6 +38,10 @@ type VersionInfo struct { // - Strategy is Progressive, and // - Presence of unversioned pollers in all task queues of target version cannot be confirmed. AllTaskQueuesHaveUnversionedPoller bool + // LastCurrentTime is the timestamp when this version last became current. + // Used to determine if this is a rollback scenario (version was previously current). + // Nil if the version was never current or if the server doesn't support this field. + LastCurrentTime *time.Time } // TemporalWorkerState represents the state of a worker deployment in Temporal @@ -177,6 +181,11 @@ func GetWorkerDeploymentState( } + if lct := version.GetLastCurrentTime(); lct != nil { + t := lct.AsTime() + versionInfo.LastCurrentTime = &t + } + state.Versions[version.DeploymentVersion.BuildId] = versionInfo } diff --git a/internal/testhelpers/make.go b/internal/testhelpers/make.go index 609c0552..8c12ee5f 100644 --- a/internal/testhelpers/make.go +++ b/internal/testhelpers/make.go @@ -6,6 +6,7 @@ import ( "github.com/pborman/uuid" temporaliov1alpha1 "github.com/temporalio/temporal-worker-controller/api/v1alpha1" + "github.com/temporalio/temporal-worker-controller/internal/defaults" "github.com/temporalio/temporal-worker-controller/internal/k8s" "go.temporal.io/server/common/worker_versioning" corev1 "k8s.io/api/core/v1" @@ -23,6 +24,7 @@ func MakeWD( replicas int32, podSpec corev1.PodTemplateSpec, rolloutStrategy *temporaliov1alpha1.RolloutStrategy, + rollbackStrategy *temporaliov1alpha1.RollbackStrategy, sunsetStrategy *temporaliov1alpha1.SunsetStrategy, workerOpts *temporaliov1alpha1.WorkerOptions, ) *temporaliov1alpha1.WorkerDeployment { @@ -51,17 +53,43 @@ func MakeWD( Labels: map[string]string{"app": "test-worker"}, }, Spec: temporaliov1alpha1.WorkerDeploymentSpec{ - Replicas: &replicas, - Template: podSpec, - RolloutStrategy: r, - SunsetStrategy: s, - WorkerOptions: w, + Replicas: &replicas, + Template: podSpec, + RolloutStrategy: r, + RollbackStrategy: rollbackStrategy, + SunsetStrategy: s, + WorkerOptions: w, }, } twd.Name = twd.ObjectMeta.Name return twd } +func MakeDefaultTWD( + name string, + namespace string, + replicas int32, + podSpec corev1.PodTemplateSpec, +) *temporaliov1alpha1.WorkerDeployment { + rolloutStrategy := &temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateAllAtOnce, + } + rollbackStrategy := &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + } + sunsetStrategy := &temporaliov1alpha1.SunsetStrategy{ + ScaledownDelay: &metav1.Duration{Duration: defaults.ScaledownDelay}, + DeleteDelay: &metav1.Duration{Duration: defaults.DeleteDelay}, + } + workerOpts := &temporaliov1alpha1.WorkerOptions{ + ConnectionRef: temporaliov1alpha1.ConnectionReference{ + Name: "default-connection", + }, + TemporalNamespace: "default", + } + return MakeWD(name, namespace, replicas, podSpec, rolloutStrategy, rollbackStrategy, sunsetStrategy, workerOpts) +} + // MakePodSpec creates a pod spec with the given containers, labels, and task queue func MakePodSpec(containers []corev1.Container, labels map[string]string, taskQueue string) corev1.PodTemplateSpec { for i := range containers { @@ -106,7 +134,7 @@ func SetTaskQueue(podSpec corev1.PodTemplateSpec, taskQueue string) corev1.PodTe } func MakeWDWithImage(name, namespace, imageName string) *temporaliov1alpha1.WorkerDeployment { - return MakeWD(name, namespace, 1, MakePodSpec([]corev1.Container{{Image: imageName}}, nil, ""), nil, nil, nil) + return MakeWD(name, namespace, 1, MakePodSpec([]corev1.Container{{Image: imageName}}, nil, ""), nil, nil, nil, nil) } // MakeBuildID computes a build id based on the image and @@ -132,7 +160,7 @@ func MakeBuildID(twdName, imageName, unsafeCustomBuildID string, podSpec *corev1 } func MakeWDWithName(name, namespace string) *temporaliov1alpha1.WorkerDeployment { - twd := MakeWD(name, namespace, 1, MakePodSpec(nil, nil, ""), nil, nil, nil) + twd := MakeWD(name, namespace, 1, MakePodSpec(nil, nil, ""), nil, nil, nil, nil) twd.ObjectMeta.Name = name twd.Name = name return twd diff --git a/internal/testhelpers/test_builder.go b/internal/testhelpers/test_builder.go index 0c214903..4aeecfe4 100644 --- a/internal/testhelpers/test_builder.go +++ b/internal/testhelpers/test_builder.go @@ -58,6 +58,23 @@ func (b *WorkerDeploymentBuilder) WithProgressiveStrategy(steps ...temporaliov1a return b } +// WithRollbackAllAtOnceStrategy sets the rollback strategy to all-at-once +func (b *TemporalWorkerDeploymentBuilder) WithRollbackAllAtOnceStrategy() *TemporalWorkerDeploymentBuilder { + b.twd.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + } + return b +} + +// WithRollbackProgressiveStrategy sets the rollback strategy to progressive with given steps +func (b *TemporalWorkerDeploymentBuilder) WithRollbackProgressiveStrategy(steps ...temporaliov1alpha1.RolloutStep) *TemporalWorkerDeploymentBuilder { + b.twd.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackProgressive, + Steps: steps, + } + return b +} + // WithGate sets the rollout strategy have a gate workflow func (b *WorkerDeploymentBuilder) WithGate(expectSuccess bool) *WorkerDeploymentBuilder { if expectSuccess { @@ -233,6 +250,11 @@ type TestCase struct { // If starting from a particular state, specify that in input.Status. twd *temporaliov1alpha1.WorkerDeployment + // previouslyCurrentImages is a list of image names whose corresponding versions should be + // registered in Temporal and briefly promoted to current (giving them a LastCurrentTime) + // before the test runs. The actual current version is restored afterward. + previouslyCurrentImages []string + // existingDeploymentReplicas specifies the number of replicas for each deprecated build. // WorkerDeploymentStatus only tracks the names of the Deployments for deprecated // versions, so for test scenarios that start with existing deprecated version Deployments, @@ -315,6 +337,10 @@ func (tc *TestCase) GetValidatorFunc() func(t *testing.T, ctx context.Context, t return tc.validatorFunc } +func (tc *TestCase) GetPreviouslyCurrentImages() []string { + return tc.previouslyCurrentImages +} + // TestCaseBuilder provides a fluent interface for building test cases type TestCaseBuilder struct { name string @@ -327,6 +353,8 @@ type TestCaseBuilder struct { expectedDeploymentInfos []DeploymentInfo waitTime *time.Duration + previouslyCurrentImages []string + setupFunc func(t *testing.T, ctx context.Context, tc TestCase, env TestEnv) twdMutatorFunc func(*temporaliov1alpha1.WorkerDeployment) postTWDCreateFunc func(t *testing.T, ctx context.Context, tc TestCase, env TestEnv) @@ -357,6 +385,15 @@ func NewTestCaseWithValues(name, k8sNamespace, temporalNamespace string) *TestCa } } +// WithPreviouslyCurrentVersions specifies images whose versions should be registered in Temporal and +// briefly promoted to current (giving them a LastCurrentTime) before the test runs. The actual +// current version declared in WithStatus is restored afterward. Use this for rollback test scenarios +// where the rollback target was previously the current version. +func (tcb *TestCaseBuilder) WithPreviouslyCurrentVersions(imageNames ...string) *TestCaseBuilder { + tcb.previouslyCurrentImages = imageNames + return tcb +} + // WithSetupFunction defines a function that the test case will call while setting up the state, after creating the initial Status. func (tcb *TestCaseBuilder) WithSetupFunction(f func(t *testing.T, ctx context.Context, tc TestCase, env TestEnv)) *TestCaseBuilder { tcb.setupFunc = f @@ -467,11 +504,12 @@ func (tcb *TestCaseBuilder) WithExpectedStatus(statusBuilder *StatusBuilder) *Te // Build returns the constructed test case func (tcb *TestCaseBuilder) Build() TestCase { ret := TestCase{ - setupFunc: tcb.setupFunc, - twdMutatorFunc: tcb.twdMutatorFunc, - postTWDCreateFunc: tcb.postTWDCreateFunc, - validatorFunc: tcb.validatorFunc, - waitTime: tcb.waitTime, + setupFunc: tcb.setupFunc, + twdMutatorFunc: tcb.twdMutatorFunc, + postTWDCreateFunc: tcb.postTWDCreateFunc, + validatorFunc: tcb.validatorFunc, + waitTime: tcb.waitTime, + previouslyCurrentImages: tcb.previouslyCurrentImages, twd: tcb.twdBuilder. WithName(tcb.name). WithNamespace(tcb.k8sNamespace). diff --git a/internal/testhelpers/workers.go b/internal/testhelpers/workers.go index d16d4ce0..f90903bf 100644 --- a/internal/testhelpers/workers.go +++ b/internal/testhelpers/workers.go @@ -55,6 +55,22 @@ func newVersionedWorker(ctx context.Context, podTemplateSpec corev1.PodTemplateS return NewWorker(ctx, temporalDeploymentName, workerBuildID, temporalTaskQueue, temporalHostPort, temporalNamespace, true) } +// StartVersionedWorker creates a versioned worker and registers a dummy workflow on it. +// This is used to register a build ID with a Temporal worker deployment to set LastCurrentTime. +// Returns a stop function that must be called when done. +func StartVersionedWorker(ctx context.Context, temporalDeploymentName, workerBuildID, temporalTaskQueue, temporalHostPort, temporalNamespace string) (stopFunc func(), err error) { + w, stop, err := NewWorker(ctx, temporalDeploymentName, workerBuildID, temporalTaskQueue, temporalHostPort, temporalNamespace, true) + if err != nil { + return nil, err + } + w.RegisterWorkflow(func(workflow.Context) error { return nil }) + if err := w.Start(); err != nil { + stop() + return nil, err + } + return stop, nil +} + func NewWorker( ctx context.Context, temporalDeploymentName, workerBuildID, temporalTaskQueue, temporalHostPort, temporalNamespace string, diff --git a/internal/tests/internal/deployment_controller.go b/internal/tests/internal/deployment_controller.go index a3768fdf..09d0a192 100644 --- a/internal/tests/internal/deployment_controller.go +++ b/internal/tests/internal/deployment_controller.go @@ -32,13 +32,16 @@ func waitTimeout(wg *sync.WaitGroup, timeout time.Duration) bool { } } -func startAndStopWorker(t *testing.T, ctx context.Context, k8sClient client.Client, deploymentName, namespace string) { +// startWorker starts a single worker for the given deployment and returns a stop function. +// The caller is responsible for calling the stop function when done. +func startWorker(t *testing.T, ctx context.Context, k8sClient client.Client, deploymentName, namespace string) func() { var deployment appsv1.Deployment if err := k8sClient.Get(ctx, types.NamespacedName{ Name: deploymentName, Namespace: namespace, }, &deployment); err != nil { t.Fatalf("failed to get deployment: %v", err) + return func() {} } startedCh := make(chan struct{}) @@ -56,11 +59,10 @@ func startAndStopWorker(t *testing.T, ctx context.Context, k8sClient client.Clie // wait for worker to start <-startedCh - time.Sleep(1 * time.Second) - - // kill worker - if stop != nil { - stop() + return func() { + if stop != nil { + stop() + } } } @@ -157,6 +159,7 @@ func makePreliminaryStatusTrue( t *testing.T, env testhelpers.TestEnv, twd *temporaliov1alpha1.WorkerDeployment, + previouslyCurrentImages []string, ) { t.Logf("Creating starting test env based on input.Status") @@ -169,6 +172,25 @@ func makePreliminaryStatusTrue( loopDefers = append(loopDefers, func() { handleStopFuncs(workerStopFuncs) }) } + // Register versions that were previously current so they have LastCurrentTime set in Temporal + workerDeploymentName := k8s.ComputeWorkerDeploymentName(twd) + for _, image := range previouslyCurrentImages { + buildID := testhelpers.MakeBuildID(twd.Name, image, "", nil) + t.Logf("Setting LastCurrentTime for previously-current version %q (buildID %q)", image, buildID) + stopFunc, err := testhelpers.StartVersionedWorker(ctx, workerDeploymentName, buildID, twd.Name, + env.Ts.GetFrontendHostPort(), env.Ts.GetDefaultNamespace()) + if err != nil { + t.Errorf("failed to start worker for previously-current build %q: %v", buildID, err) + continue + } + waitForVersionRegistrationInDeployment(t, ctx, env.Ts, &worker.WorkerDeploymentVersion{ + DeploymentName: workerDeploymentName, + BuildID: buildID, + }) + setCurrentVersion(t, ctx, env.Ts, workerDeploymentName, buildID) + loopDefers = append(loopDefers, stopFunc) + } + if tv := twd.Status.TargetVersion; tv.BuildID != "" { t.Logf("Setting up target version %v with status %v", tv.BuildID, tv.Status) var rampPercentage *float32 @@ -225,9 +247,15 @@ func createStatus( setRampingVersion(t, ctx, env.Ts, v.DeploymentName, "", 0) case temporaliov1alpha1.VersionStatusDrained: if env.ExistingDeploymentReplicas[v.BuildID] == 0 { - startAndStopWorker(t, ctx, env.K8sClient, expectedDeploymentName, prevTWD.Namespace) + // Keep the temporary worker alive until setCurrentVersion completes. + // Stopping it before the call risks the Worker Deployment entry being + // cleaned up (under short PollerHistoryTTL) before registration is confirmed. + stopTemporaryWorker := startWorker(t, ctx, env.K8sClient, expectedDeploymentName, prevTWD.Namespace) + setCurrentVersion(t, ctx, env.Ts, v.DeploymentName, v.BuildID) + stopTemporaryWorker() + } else { + setCurrentVersion(t, ctx, env.Ts, v.DeploymentName, v.BuildID) } - setCurrentVersion(t, ctx, env.Ts, v.DeploymentName, v.BuildID) setCurrentVersion(t, ctx, env.Ts, v.DeploymentName, "") } } diff --git a/internal/tests/internal/integration_test.go b/internal/tests/internal/integration_test.go index 194421bf..c2013f90 100644 --- a/internal/tests/internal/integration_test.go +++ b/internal/tests/internal/integration_test.go @@ -712,6 +712,73 @@ func TestIntegration(t *testing.T) { }) } + rollbackStrategyTestCases := []testCase{ + { + name: "all-at-once-rollback-expect-immediate-promotion", + builder: testhelpers.NewTestCase(). + WithInput( + testhelpers.NewTemporalWorkerDeploymentBuilder(). + WithAllAtOnceStrategy(). + WithRollbackAllAtOnceStrategy(). + WithTargetTemplate("v1"). + WithStatus( + testhelpers.NewStatusBuilder(). + WithTargetVersion("v2", temporaliov1alpha1.VersionStatusCurrent, -1, true, true). + WithCurrentVersion("v2", true, true), + ), + ). + WithExistingDeployments( + testhelpers.NewDeploymentInfo("v2", 1), + ). + WithPreviouslyCurrentVersions("v1"). + WithExpectedStatus( + testhelpers.NewStatusBuilder(). + WithTargetVersion("v1", temporaliov1alpha1.VersionStatusCurrent, -1, true, false). + WithCurrentVersion("v1", true, false). + WithDeprecatedVersions( + testhelpers.NewDeprecatedVersionInfo("v2", temporaliov1alpha1.VersionStatusDrained, true, false, true), + ), + ). + WithExpectedDeployments( + testhelpers.NewDeploymentInfo("v2", 1), + ), + }, + { + name: "progressive-rollback-expect-ramp-at-first-step", + builder: testhelpers.NewTestCase(). + WithInput( + testhelpers.NewTemporalWorkerDeploymentBuilder(). + WithAllAtOnceStrategy(). + WithRollbackProgressiveStrategy(testhelpers.ProgressiveStep(50, time.Hour)). + WithTargetTemplate("v1"). + WithStatus( + testhelpers.NewStatusBuilder(). + WithTargetVersion("v2", temporaliov1alpha1.VersionStatusCurrent, -1, true, true). + WithCurrentVersion("v2", true, true), + ), + ). + WithExistingDeployments( + testhelpers.NewDeploymentInfo("v2", 1), + ). + WithPreviouslyCurrentVersions("v1"). + WithExpectedStatus( + testhelpers.NewStatusBuilder(). + WithTargetVersion("v1", temporaliov1alpha1.VersionStatusRamping, 50, true, false). + WithCurrentVersion("v2", true, false), + ). + WithExpectedDeployments( + testhelpers.NewDeploymentInfo("v2", 1), + ), + }, + } + + for _, tc := range rollbackStrategyTestCases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + testTemporalWorkerDeploymentCreation(ctx, t, k8sClient, mgr, ts, tc.builder.BuildWithValues(tc.name, testNamespace.Name, ts.GetDefaultNamespace())) + }) + } + // Create short TTL test Temporal server and client dcShortTTL := dynamicconfig.NewMemoryClient() // make versions eligible for deletion faster @@ -1004,7 +1071,7 @@ func testWorkerDeploymentCreation( ExpectedDeploymentReplicas: tc.GetExpectedDeploymentReplicas(), } - makePreliminaryStatusTrue(ctx, t, env, twd) + makePreliminaryStatusTrue(ctx, t, env, twd, tc.GetPreviouslyCurrentImages()) // verify that temporal state matches the preliminary status, to confirm that makePreliminaryStatusTrue worked verifyTemporalStateMatchesStatusEventually(t, ctx, ts, twd, twd.Status, 30*time.Second, 5*time.Second) @@ -1024,6 +1091,17 @@ func testWorkerDeploymentCreation( t.Fatalf("failed to create WorkerDeployment: %v", err) } + // Immediately apply the input status before the controller's first reconcile. + // k8sClient.Create strips the status subresource, so without this the first reconcile + // always sees an empty status (CurrentVersion == nil), which triggers the fast-track. + // The controller reconcile is queued asynchronously via the watch/informer + // path, so this synchronous Status().Update() reliably precedes it. + if twd.Status.TargetVersion.BuildID != "" { + if err := k8sClient.Status().Update(ctx, twd); err != nil { + t.Fatalf("failed to pre-apply TWD status: %v", err) + } + } + // Hook: runs after TWD creation but before waiting for the target Deployment. // Use this to assert blocking behaviour and then unblock the rollout. if f := tc.GetPostTWDCreateFunc(); f != nil { From 2a60a9399c4abcf8cb70ec6f8df807ab4934fb9d Mon Sep 17 00:00:00 2001 From: Eniko Dif Date: Mon, 20 Apr 2026 10:41:18 +0200 Subject: [PATCH 2/6] Implement max version age --- api/v1alpha1/workerdeployment_types.go | 6 + api/v1alpha1/workerdeployment_webhook.go | 3 + api/v1alpha1/workerdeployment_webhook_test.go | 21 ++++ api/v1alpha1/zz_generated.deepcopy.go | 5 + ...temporal.io_temporalworkerdeployments.yaml | 2 + internal/defaults/defaults.go | 1 + internal/planner/planner.go | 19 ++- internal/planner/planner_test.go | 111 ++++++++++++++++-- 8 files changed, 159 insertions(+), 9 deletions(-) diff --git a/api/v1alpha1/workerdeployment_types.go b/api/v1alpha1/workerdeployment_types.go index d0077317..0a5addf3 100644 --- a/api/v1alpha1/workerdeployment_types.go +++ b/api/v1alpha1/workerdeployment_types.go @@ -434,6 +434,12 @@ type RollbackStrategy struct { // Steps to execute progressive rollbacks. Only required when strategy is "Progressive". // +optional Steps []RolloutStep `json:"steps,omitempty"` + + // MaxVersionAge limits which versions are eligible as rollback targets. + // A version is only considered a rollback target if it was last current within this duration. + // If nil, there is no age limit. + // +optional + MaxVersionAge *metav1.Duration `json:"maxVersionAge,omitempty"` } // SunsetStrategy defines strategy to apply when sunsetting k8s deployments of drained versions. diff --git a/api/v1alpha1/workerdeployment_webhook.go b/api/v1alpha1/workerdeployment_webhook.go index 2ab88fad..a18ed9fc 100644 --- a/api/v1alpha1/workerdeployment_webhook.go +++ b/api/v1alpha1/workerdeployment_webhook.go @@ -59,6 +59,9 @@ func (s *WorkerDeploymentSpec) Default(ctx context.Context) error { } else if s.RollbackStrategy.Strategy == "" { s.RollbackStrategy.Strategy = RollbackAllAtOnce } + if s.RollbackStrategy.MaxVersionAge == nil { + s.RollbackStrategy.MaxVersionAge = &v1.Duration{Duration: defaults.RollbackMaxVersionAge} + } return nil } diff --git a/api/v1alpha1/workerdeployment_webhook_test.go b/api/v1alpha1/workerdeployment_webhook_test.go index e0b2a591..449eb880 100644 --- a/api/v1alpha1/workerdeployment_webhook_test.go +++ b/api/v1alpha1/workerdeployment_webhook_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" temporaliov1alpha1 "github.com/temporalio/temporal-worker-controller/api/v1alpha1" + "github.com/temporalio/temporal-worker-controller/internal/defaults" "github.com/temporalio/temporal-worker-controller/internal/testhelpers" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -193,6 +194,8 @@ func TestWorkerDeployment_Default(t *testing.T) { expected: func(t *testing.T, obj *temporaliov1alpha1.WorkerDeployment) { require.NotNil(t, obj.Spec.RollbackStrategy, "expected RollbackStrategy to be initialized by webhook") assert.Equal(t, temporaliov1alpha1.RollbackAllAtOnce, obj.Spec.RollbackStrategy.Strategy, "expected RollbackStrategy.Strategy to default to AllAtOnce") + require.NotNil(t, obj.Spec.RollbackStrategy.MaxVersionAge) + assert.Equal(t, defaults.RollbackMaxVersionAge, obj.Spec.RollbackStrategy.MaxVersionAge.Duration) }, }, "rollback strategy defaults empty strategy field to AllAtOnce": { @@ -205,6 +208,8 @@ func TestWorkerDeployment_Default(t *testing.T) { expected: func(t *testing.T, obj *temporaliov1alpha1.WorkerDeployment) { require.NotNil(t, obj.Spec.RollbackStrategy) assert.Equal(t, temporaliov1alpha1.RollbackAllAtOnce, obj.Spec.RollbackStrategy.Strategy, "expected RollbackStrategy.Strategy to default to AllAtOnce") + require.NotNil(t, obj.Spec.RollbackStrategy.MaxVersionAge) + assert.Equal(t, defaults.RollbackMaxVersionAge, obj.Spec.RollbackStrategy.MaxVersionAge.Duration) }, }, "rollback strategy preserves explicit strategy": { @@ -220,6 +225,22 @@ func TestWorkerDeployment_Default(t *testing.T) { expected: func(t *testing.T, obj *temporaliov1alpha1.WorkerDeployment) { require.NotNil(t, obj.Spec.RollbackStrategy) assert.Equal(t, temporaliov1alpha1.RollbackProgressive, obj.Spec.RollbackStrategy.Strategy, "expected RollbackStrategy.Strategy to remain Progressive") + require.NotNil(t, obj.Spec.RollbackStrategy.MaxVersionAge) + assert.Equal(t, defaults.RollbackMaxVersionAge, obj.Spec.RollbackStrategy.MaxVersionAge.Duration) + }, + }, + "rollback strategy preserves explicit MaxVersionAge": { + obj: testhelpers.ModifyObj(testhelpers.MakeTWDWithName("explicit-rollback-max-version-age", ""), func(obj *temporaliov1alpha1.TemporalWorkerDeployment) *temporaliov1alpha1.TemporalWorkerDeployment { + obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + MaxVersionAge: &metav1.Duration{Duration: 30 * time.Minute}, + } + return obj + }), + expected: func(t *testing.T, obj *temporaliov1alpha1.TemporalWorkerDeployment) { + require.NotNil(t, obj.Spec.RollbackStrategy) + require.NotNil(t, obj.Spec.RollbackStrategy.MaxVersionAge) + assert.Equal(t, 30*time.Minute, obj.Spec.RollbackStrategy.MaxVersionAge.Duration, "expected explicit MaxVersionAge to be preserved") }, }, } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 79f4b37b..b3e5723f 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -294,6 +294,11 @@ func (in *RollbackStrategy) DeepCopyInto(out *RollbackStrategy) { *out = make([]RolloutStep, len(*in)) copy(*out, *in) } + if in.MaxVersionAge != nil { + in, out := &in.MaxVersionAge, &out.MaxVersionAge + *out = new(metav1.Duration) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RollbackStrategy. diff --git a/helm/temporal-worker-controller-crds/templates/temporal.io_temporalworkerdeployments.yaml b/helm/temporal-worker-controller-crds/templates/temporal.io_temporalworkerdeployments.yaml index 6b222cf7..e6ae87f1 100644 --- a/helm/temporal-worker-controller-crds/templates/temporal.io_temporalworkerdeployments.yaml +++ b/helm/temporal-worker-controller-crds/templates/temporal.io_temporalworkerdeployments.yaml @@ -63,6 +63,8 @@ spec: type: integer rollback: properties: + maxVersionAge: + type: string steps: items: properties: diff --git a/internal/defaults/defaults.go b/internal/defaults/defaults.go index 50f6c2f0..11448e6a 100644 --- a/internal/defaults/defaults.go +++ b/internal/defaults/defaults.go @@ -9,6 +9,7 @@ import "time" const ( ScaledownDelay = 1 * time.Hour DeleteDelay = 24 * time.Hour + RollbackMaxVersionAge = 1 * time.Hour ServerMaxVersions = 100 MaxVersionsIneligibleForDeletion = int32(ServerMaxVersions * 0.75) diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 04dc0e4d..ff175285 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -765,7 +765,16 @@ func isRollbackScenario( l logr.Logger, status *temporaliov1alpha1.TemporalWorkerDeploymentStatus, temporalState *temporal.TemporalWorkerState, + config *Config, ) bool { + if config.RollbackStrategy == nil { + return false + } + + if config.RolloutStrategy.Strategy == temporaliov1alpha1.UpdateManual { + return false + } + if temporalState == nil { return false } @@ -779,6 +788,14 @@ func isRollbackScenario( return false } + if config.RollbackStrategy.MaxVersionAge != nil && time.Since(*targetVersionInfo.LastCurrentTime) > config.RollbackStrategy.MaxVersionAge.Duration { + l.Info("Skipping rollback: the version's last current time exceeds MaxVersionAge", + "targetBuildID", status.TargetVersion.BuildID, + "lastCurrentTime", targetVersionInfo.LastCurrentTime, + "maxVersionAge", config.RollbackStrategy.MaxVersionAge.Duration) + return false + } + l.Info("Detected rollback scenario using LastCurrentTime. "+ "Warning: Auto-upgrade workflows that upgraded from a previous version to the current version may fail during this rollback, "+ "as they may not handle downgrades properly. Monitor workflow executions for failures.", @@ -797,7 +814,7 @@ func getVersionConfigDiff( workerDeploymentName string, ) *VersionConfig { var strategy temporaliov1alpha1.RolloutStrategy - if isRollbackScenario(l, status, temporalState) { + if isRollbackScenario(l, status, temporalState, config) { strategy = convertRollbackToRolloutStrategy(*config.RollbackStrategy) } else { strategy = config.RolloutStrategy diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index 530e8626..e6400399 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -3583,15 +3583,26 @@ func TestGetWRTOwnerRefPatches(t *testing.T) { } func TestIsRollbackScenario(t *testing.T) { - lastCurrentTime := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) + recentLastCurrentTime := time.Now().Add(-5 * time.Minute) + oldLastCurrentTime := time.Now().Add(-2 * time.Hour) + + defaultConfig := &Config{ + RolloutStrategy: temporaliov1alpha1.RolloutStrategy{Strategy: temporaliov1alpha1.UpdateAllAtOnce}, + RollbackStrategy: &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + MaxVersionAge: &metav1.Duration{Duration: defaults.RollbackMaxVersionAge}, + }, + } + testCases := []struct { name string status *temporaliov1alpha1.TemporalWorkerDeploymentStatus temporalState *temporal.TemporalWorkerState + config *Config expectedResult bool }{ { - name: "rollback detected via LastCurrentTime", + name: "rollback detected via LastCurrentTime within MaxVersionAge", status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ @@ -3603,10 +3614,11 @@ func TestIsRollbackScenario(t *testing.T) { Versions: map[string]*temporal.VersionInfo{ "build-v1": { BuildID: "build-v1", - LastCurrentTime: &lastCurrentTime, + LastCurrentTime: &recentLastCurrentTime, }, }, }, + config: defaultConfig, expectedResult: true, }, { @@ -3626,6 +3638,7 @@ func TestIsRollbackScenario(t *testing.T) { }, }, }, + config: defaultConfig, expectedResult: false, }, { @@ -3641,10 +3654,11 @@ func TestIsRollbackScenario(t *testing.T) { Versions: map[string]*temporal.VersionInfo{ "build-v1": { BuildID: "build-v1", - LastCurrentTime: &lastCurrentTime, + LastCurrentTime: &recentLastCurrentTime, }, }, }, + config: defaultConfig, expectedResult: false, }, { @@ -3657,6 +3671,83 @@ func TestIsRollbackScenario(t *testing.T) { }, }, temporalState: nil, + config: defaultConfig, + expectedResult: false, + }, + { + name: "rollout when version age exceeds MaxVersionAge", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v1", + }, + }, + }, + temporalState: &temporal.TemporalWorkerState{ + Versions: map[string]*temporal.VersionInfo{ + "build-v1": { + BuildID: "build-v1", + LastCurrentTime: &oldLastCurrentTime, + }, + }, + }, + config: defaultConfig, + expectedResult: false, + }, + { + name: "rollback disabled when MaxVersionAge is zero", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v1", + }, + }, + }, + temporalState: &temporal.TemporalWorkerState{ + Versions: map[string]*temporal.VersionInfo{ + "build-v1": { + BuildID: "build-v1", + LastCurrentTime: &recentLastCurrentTime, + }, + }, + }, + config: &Config{ + RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateAllAtOnce, + }, + RollbackStrategy: &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + MaxVersionAge: &metav1.Duration{Duration: 0}, + }, + }, + expectedResult: false, + }, + { + name: "rollback skipped when rollout strategy is Manual", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "build-v1", + }, + }, + }, + temporalState: &temporal.TemporalWorkerState{ + Versions: map[string]*temporal.VersionInfo{ + "build-v1": { + BuildID: "build-v1", + LastCurrentTime: &recentLastCurrentTime, + }, + }, + }, + config: &Config{ + RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateManual, + }, + RollbackStrategy: &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + MaxVersionAge: &metav1.Duration{Duration: defaults.RollbackMaxVersionAge}, + }, + }, expectedResult: false, }, } @@ -3664,7 +3755,7 @@ func TestIsRollbackScenario(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { logger := logr.Discard() - result := isRollbackScenario(logger, tc.status, tc.temporalState) + result := isRollbackScenario(logger, tc.status, tc.temporalState, tc.config) assert.Equal(t, tc.expectedResult, result) }) } @@ -3771,7 +3862,8 @@ func TestGetVersionConfigDiff_RollbackScenario(t *testing.T) { }, }, RollbackStrategy: &temporaliov1alpha1.RollbackStrategy{ - Strategy: temporaliov1alpha1.RollbackAllAtOnce, + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + MaxVersionAge: &metav1.Duration{Duration: defaults.RollbackMaxVersionAge}, }, }, expectSetCurrent: true, @@ -3820,6 +3912,7 @@ func TestGetVersionConfigDiff_RollbackScenario(t *testing.T) { Steps: []temporaliov1alpha1.RolloutStep{ {RampPercentage: 50, PauseDuration: metav1.Duration{Duration: time.Minute}}, }, + MaxVersionAge: &metav1.Duration{Duration: defaults.RollbackMaxVersionAge}, }, }, expectSetCurrent: false, @@ -3866,7 +3959,8 @@ func TestGetVersionConfigDiff_RollbackScenario(t *testing.T) { }, }, RollbackStrategy: &temporaliov1alpha1.RollbackStrategy{ - Strategy: temporaliov1alpha1.RollbackAllAtOnce, // would set current immediately if wrongly used + Strategy: temporaliov1alpha1.RollbackAllAtOnce, // would set current immediately if wrongly used + MaxVersionAge: &metav1.Duration{Duration: defaults.RollbackMaxVersionAge}, }, }, expectSetCurrent: false, @@ -3910,7 +4004,8 @@ func TestGetVersionConfigDiff_RollbackScenario(t *testing.T) { Strategy: temporaliov1alpha1.UpdateAllAtOnce, }, RollbackStrategy: &temporaliov1alpha1.RollbackStrategy{ - Strategy: temporaliov1alpha1.RollbackAllAtOnce, + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + MaxVersionAge: &metav1.Duration{Duration: defaults.RollbackMaxVersionAge}, }, }, expectSetCurrent: true, From cca2c35547844432d4231d54d2264404da348493 Mon Sep 17 00:00:00 2001 From: Eniko Dif Date: Mon, 20 Apr 2026 11:17:52 +0200 Subject: [PATCH 3/6] Add warning message if rollback is set slower than rollout --- api/v1alpha1/workerdeployment_webhook.go | 29 ++++++ api/v1alpha1/workerdeployment_webhook_test.go | 95 +++++++++++++++++-- 2 files changed, 117 insertions(+), 7 deletions(-) diff --git a/api/v1alpha1/workerdeployment_webhook.go b/api/v1alpha1/workerdeployment_webhook.go index a18ed9fc..2312fc3c 100644 --- a/api/v1alpha1/workerdeployment_webhook.go +++ b/api/v1alpha1/workerdeployment_webhook.go @@ -98,6 +98,9 @@ func validateForUpdateOrCreate(old, new *WorkerDeployment) (admission.Warnings, if len(allErrs) > 0 { return nil, newInvalidErr(new, allErrs) } + if new.Spec.RollbackStrategy != nil { + return warnRollbackSlowerThanRollout(new.Spec.RolloutStrategy, *new.Spec.RollbackStrategy), nil + } return nil, nil } @@ -130,6 +133,32 @@ func validateRollbackStrategy(s RollbackStrategy) []*field.Error { return allErrs } +func warnRollbackSlowerThanRollout(rollout RolloutStrategy, rollback RollbackStrategy) admission.Warnings { + switch rollout.Strategy { + case UpdateAllAtOnce: + if rollback.Strategy != RollbackAllAtOnce { + return admission.Warnings{"rollback strategy is slower than rollout: rollout is AllAtOnce, but rollback is Progressive — is that intended?"} + } + case UpdateProgressive: + if rollback.Strategy == RollbackProgressive { + var rolloutTotal, rollbackTotal time.Duration + for _, s := range rollout.Steps { + rolloutTotal += s.PauseDuration.Duration + } + for _, s := range rollback.Steps { + rollbackTotal += s.PauseDuration.Duration + } + if rollbackTotal > rolloutTotal { + return admission.Warnings{fmt.Sprintf( + "rollback strategy is slower than rollout: progressive rollback total duration (%s) exceeds progressive rollout total duration (%s) — is that intended?", + rollbackTotal, rolloutTotal, + )} + } + } + } + return nil +} + func validateProgressiveStrategySteps(specName string, steps []RolloutStep) []*field.Error { var allErrs []*field.Error diff --git a/api/v1alpha1/workerdeployment_webhook_test.go b/api/v1alpha1/workerdeployment_webhook_test.go index 449eb880..9067318e 100644 --- a/api/v1alpha1/workerdeployment_webhook_test.go +++ b/api/v1alpha1/workerdeployment_webhook_test.go @@ -24,6 +24,7 @@ func TestWorkerDeployment_ValidateCreate(t *testing.T) { tests := map[string]struct { obj runtime.Object errorMsg string + warnMsg string }{ "valid temporal worker deployment": { obj: testhelpers.MakeWDWithName("valid-worker", ""), @@ -51,6 +52,18 @@ func TestWorkerDeployment_ValidateCreate(t *testing.T) { }), errorMsg: "[spec.rollout.steps[2].rampPercentage: Invalid value: 9: rampPercentage must increase between each step, spec.rollout.steps[4].rampPercentage: Invalid value: 50: rampPercentage must increase between each step]", }, + "rollout strategy - invalid Progressive pause duration < 30s": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("prog-rollout-short-pause", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RolloutStrategy.Strategy = temporaliov1alpha1.UpdateProgressive + obj.Spec.RolloutStrategy.Steps = []temporaliov1alpha1.RolloutStep{ + {10, metav1.Duration{Duration: time.Minute}}, + {25, metav1.Duration{Duration: 10 * time.Second}}, + {50, metav1.Duration{Duration: time.Minute}}, + } + return obj + }), + errorMsg: `spec.rollout.steps[1].pauseDuration: Invalid value: "10s": pause duration must be at least 30s`, + }, "rollback strategy - valid Progressive with steps": { obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-progressive", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ @@ -62,6 +75,72 @@ func TestWorkerDeployment_ValidateCreate(t *testing.T) { return obj }), }, + "rollback strategy - Progressive rollback with AllAtOnce rollout warns": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-progressive-allatonce-rollout", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {50, metav1.Duration{Duration: 30 * time.Second}}, + }, + } + return obj + }), + warnMsg: "rollback strategy is slower than rollout", + }, + "rollback strategy - AllAtOnce rollback with Progressive rollout is valid": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-allatonce-prog-rollout", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RolloutStrategy = temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {50, metav1.Duration{Duration: time.Minute}}, + {75, metav1.Duration{Duration: time.Minute}}, + }, + } + obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackAllAtOnce, + } + return obj + }), + }, + "rollback strategy - Progressive rollback faster than Progressive rollout is valid": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-prog-faster", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RolloutStrategy = temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {50, metav1.Duration{Duration: 2 * time.Minute}}, + {75, metav1.Duration{Duration: 2 * time.Minute}}, + }, + } + obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {50, metav1.Duration{Duration: time.Minute}}, + {75, metav1.Duration{Duration: time.Minute}}, + }, + } + return obj + }), + }, + "rollback strategy - Progressive rollback slower than Progressive rollout warns": { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-prog-slower", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RolloutStrategy = temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {50, metav1.Duration{Duration: time.Minute}}, + {75, metav1.Duration{Duration: time.Minute}}, + }, + } + obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ + Strategy: temporaliov1alpha1.RollbackProgressive, + Steps: []temporaliov1alpha1.RolloutStep{ + {50, metav1.Duration{Duration: 2 * time.Minute}}, + {75, metav1.Duration{Duration: 2 * time.Minute}}, + }, + } + return obj + }), + warnMsg: "rollback strategy is slower than rollout", + }, "rollback strategy - invalid Progressive without steps": { obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-progressive-no-steps", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ @@ -73,7 +152,7 @@ func TestWorkerDeployment_ValidateCreate(t *testing.T) { errorMsg: "steps are required for Progressive strategy", }, "rollback strategy - invalid Progressive pause duration < 30s": { - obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-progressive-invalid", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-progressive-short-pause", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ Strategy: temporaliov1alpha1.RollbackProgressive, Steps: []temporaliov1alpha1.RolloutStep{ @@ -112,8 +191,12 @@ func TestWorkerDeployment_ValidateCreate(t *testing.T) { require.NoError(t, err) } - // Warnings should always be nil for this implementation - assert.Nil(t, warnings) + if tc.warnMsg != "" { + require.NotEmpty(t, warnings) + assert.Contains(t, warnings[0], tc.warnMsg) + } else { + assert.Empty(t, warnings) + } } // Verify that create and update enforce the same rules @@ -149,7 +232,6 @@ func TestWorkerDeployment_ValidateUpdate(t *testing.T) { require.NoError(t, err) } - // Warnings should always be nil for this implementation assert.Nil(t, warnings) }) } @@ -167,7 +249,6 @@ func TestWorkerDeployment_ValidateDelete(t *testing.T) { warnings, err := webhook.ValidateDelete(ctx, obj) - // ValidateDelete should always return nil, nil assert.NoError(t, err) assert.Nil(t, warnings) } @@ -230,14 +311,14 @@ func TestWorkerDeployment_Default(t *testing.T) { }, }, "rollback strategy preserves explicit MaxVersionAge": { - obj: testhelpers.ModifyObj(testhelpers.MakeTWDWithName("explicit-rollback-max-version-age", ""), func(obj *temporaliov1alpha1.TemporalWorkerDeployment) *temporaliov1alpha1.TemporalWorkerDeployment { + obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("explicit-rollback-max-version-age", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ Strategy: temporaliov1alpha1.RollbackAllAtOnce, MaxVersionAge: &metav1.Duration{Duration: 30 * time.Minute}, } return obj }), - expected: func(t *testing.T, obj *temporaliov1alpha1.TemporalWorkerDeployment) { + expected: func(t *testing.T, obj *temporaliov1alpha1.WorkerDeployment) { require.NotNil(t, obj.Spec.RollbackStrategy) require.NotNil(t, obj.Spec.RollbackStrategy.MaxVersionAge) assert.Equal(t, 30*time.Minute, obj.Spec.RollbackStrategy.MaxVersionAge.Duration, "expected explicit MaxVersionAge to be preserved") From cb0252bc29bb04a1140c4b7f508dca7e6efd9dac Mon Sep 17 00:00:00 2001 From: Eniko Dif Date: Mon, 20 Apr 2026 11:32:54 +0200 Subject: [PATCH 4/6] Update the docs --- docs/configuration.md | 63 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index f4d6ebc3..0ff5bc0e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -5,10 +5,11 @@ This document provides comprehensive configuration options for the Temporal Work ## Table of Contents 1. [Rollout Strategies](#rollout-strategies) -2. [Sunset Configuration](#sunset-configuration) -3. [Worker Options](#worker-options) -4. [Gate Configuration](#gate-configuration) -5. [Advanced Configuration](#advanced-configuration) +2. [Rollback Strategy](#rollback-strategy) +3. [Sunset Configuration](#sunset-configuration) +4. [Worker Options](#worker-options) +5. [Gate Configuration](#gate-configuration) +6. [Advanced Configuration](#advanced-configuration) ## Rollout Strategies @@ -110,6 +111,60 @@ rollout: pauseDuration: 0s # Full rollout after canary validation ``` +## Rollback Strategy + +The controller automatically detects rollbacks: if the target version was previously the current version (within `maxVersionAge`), it applies the rollback strategy instead of the rollout strategy. This means setting your target version back to a prior build ID is enough to trigger a rollback — no special action required. + +> **Note:** Rollback is suppressed when the rollout strategy is `Manual`, since manual mode implies full user control. + +### Default Rollback Configuration + +```yaml +rollback: + strategy: AllAtOnce # Default: immediately switch 100% of traffic back + maxVersionAge: 1h # Only treat versions as rollback targets if they were + # current within the last hour. Set to 0s to disable rollback. +``` + +### Rollback Strategies + +**AllAtOnce (default):** Immediately routes 100% of traffic back to the previous version. Recommended for fast recovery. + +```yaml +rollback: + strategy: AllAtOnce + maxVersionAge: 1h +``` + +**Progressive:** Gradually ramps traffic back, using the same step-based mechanism as progressive rollouts. Use this when you want a controlled rollback, for example to observe metrics during the reversion. + +```yaml +rollback: + strategy: Progressive + maxVersionAge: 1h + steps: + - rampPercentage: 50 + pauseDuration: 5m + - rampPercentage: 75 + pauseDuration: 5m +``` + +> **Warning:** The controller will warn at admission time if the rollback strategy is slower than the rollout strategy, since a slow rollback defeats the purpose of fast recovery. If you configure a progressive rollback with a longer total duration than your rollout steps, verify this is intentional. + +### `maxVersionAge` + +Controls which versions are eligible as rollback targets. A version is only considered a rollback target if the last time it was the current version falls within this duration. Set to a short duration (e.g. `1h`) to limit rollbacks to recently-promoted versions. + +### Disabling Rollbacks + +Set `maxVersionAge` to `0s` to disable automatic rollback detection entirely. The controller will treat every target version as a fresh rollout, regardless of its history. + +```yaml +rollback: + strategy: AllAtOnce + maxVersionAge: 0s +``` + ## Sunset Configuration Controls how old versions are scaled down and cleaned up after they're no longer receiving new traffic: From 64a1cbdbba817dc2b04b8215b8a8c1bb3bd3e05e Mon Sep 17 00:00:00 2001 From: Eniko Dif Date: Mon, 20 Apr 2026 11:59:59 +0200 Subject: [PATCH 5/6] Address Jacob's PR comments --- api/v1alpha1/workerdeployment_types.go | 34 +++++++++++++++++--------- internal/planner/planner.go | 2 +- internal/planner/planner_test.go | 2 +- internal/temporal/worker_deployment.go | 2 +- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/api/v1alpha1/workerdeployment_types.go b/api/v1alpha1/workerdeployment_types.go index 0a5addf3..1067e1e4 100644 --- a/api/v1alpha1/workerdeployment_types.go +++ b/api/v1alpha1/workerdeployment_types.go @@ -84,6 +84,18 @@ type WorkerDeploymentSpec struct { RolloutStrategy RolloutStrategy `json:"rollout"` // How to rollback to a previous version. If not specified, defaults to AllAtOnce strategy. + // + // A rollback is triggered automatically when the target version's pod spec is updated and + // the resulting build ID has previously been set as the default (current) version of the + // worker deployment. The controller detects this by checking whether Temporal recorded a + // non-nil LastCurrentTime for that build ID. + // + // The rollback strategy controls routing of NEW workflow executions only. Workflows already + // running are pinned to the version they started on and continue executing there; they are + // not affected by the rollback. Only new workflow executions will be routed to the rollback + // target version. + // + // Rollback is suppressed when the rollout strategy is Manual. // +optional RollbackStrategy *RollbackStrategy `json:"rollback,omitempty"` @@ -342,17 +354,17 @@ type DeprecatedWorkerDeploymentVersion struct { EligibleForDeletion bool `json:"eligibleForDeletion,omitempty"` } -// DefaultVersionUpdateStrategy describes how to cut over new workflow executions +// VersionRolloutStrategy describes how to cut over new workflow executions // to the target worker deployment version. // +kubebuilder:validation:Enum=Manual;AllAtOnce;Progressive -type DefaultVersionUpdateStrategy string +type VersionRolloutStrategy string const ( // UpdateManual scales worker resources up or down, but does not update the current or ramping worker deployment version. - UpdateManual DefaultVersionUpdateStrategy = "Manual" + UpdateManual VersionRolloutStrategy = "Manual" // UpdateAllAtOnce starts 100% of new workflow executions on the new worker deployment version as soon as it's healthy. - UpdateAllAtOnce DefaultVersionUpdateStrategy = "AllAtOnce" + UpdateAllAtOnce VersionRolloutStrategy = "AllAtOnce" // UpdateProgressive ramps up the percentage of new workflow executions targeting the new worker deployment version over time. // @@ -362,19 +374,19 @@ const ( // Sending a percentage of traffic to a "nil" version means that traffic will be sent to unversioned workers. If // there are no unversioned workers, those tasks will get stuck. This behavior ensures that all traffic on the task // queues in this worker deployment can be handled by an active poller. - UpdateProgressive DefaultVersionUpdateStrategy = "Progressive" + UpdateProgressive VersionRolloutStrategy = "Progressive" ) -// DefaultVersionRollbackStrategy describes how to cut over during rollback to a previous version. +// VersionRollbackStrategy describes how to cut over during rollback to a previous version. // +kubebuilder:validation:Enum=AllAtOnce;Progressive -type DefaultVersionRollbackStrategy string +type VersionRollbackStrategy string const ( // RollbackAllAtOnce immediately switches 100% of traffic back to the previous version. - RollbackAllAtOnce DefaultVersionRollbackStrategy = "AllAtOnce" + RollbackAllAtOnce VersionRollbackStrategy = "AllAtOnce" // RollbackProgressive gradually ramps traffic back to the previous version. - RollbackProgressive DefaultVersionRollbackStrategy = "Progressive" + RollbackProgressive VersionRollbackStrategy = "Progressive" ) type GateWorkflowConfig struct { @@ -409,7 +421,7 @@ type RolloutStrategy struct { // - "Manual" // - "AllAtOnce" // - "Progressive" - Strategy DefaultVersionUpdateStrategy `json:"strategy"` + Strategy VersionRolloutStrategy `json:"strategy"` // Gate specifies a workflow type that must run once to completion on the new worker deployment version before // any traffic is directed to the new version. @@ -429,7 +441,7 @@ type RolloutStrategy struct { type RollbackStrategy struct { // Strategy for rollback. Valid values are "AllAtOnce" or "Progressive". // Defaults to "AllAtOnce" for fast recovery. - Strategy DefaultVersionRollbackStrategy `json:"strategy"` + Strategy VersionRollbackStrategy `json:"strategy"` // Steps to execute progressive rollbacks. Only required when strategy is "Progressive". // +optional diff --git a/internal/planner/planner.go b/internal/planner/planner.go index ff175285..536be674 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -893,7 +893,7 @@ func getVersionConfigDiff( // Convert to reuse rollout logic with different settings func convertRollbackToRolloutStrategy(rb temporaliov1alpha1.RollbackStrategy) temporaliov1alpha1.RolloutStrategy { - var strategy temporaliov1alpha1.DefaultVersionUpdateStrategy + var strategy temporaliov1alpha1.VersionRolloutStrategy switch rb.Strategy { case temporaliov1alpha1.RollbackAllAtOnce: strategy = temporaliov1alpha1.UpdateAllAtOnce diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index e6400399..deeb52c1 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -3765,7 +3765,7 @@ func TestConvertRollbackToRolloutStrategy(t *testing.T) { testCases := []struct { name string rollbackStrategy temporaliov1alpha1.RollbackStrategy - expectedStrategy temporaliov1alpha1.DefaultVersionUpdateStrategy + expectedStrategy temporaliov1alpha1.VersionRolloutStrategy expectedSteps []temporaliov1alpha1.RolloutStep expectedGate *temporaliov1alpha1.GateWorkflowConfig }{ diff --git a/internal/temporal/worker_deployment.go b/internal/temporal/worker_deployment.go index a839b8b2..95e0026f 100644 --- a/internal/temporal/worker_deployment.go +++ b/internal/temporal/worker_deployment.go @@ -67,7 +67,7 @@ func GetWorkerDeploymentState( namespace string, k8sDeployments map[string]*appsv1.Deployment, targetBuildID string, - strategy temporaliov1alpha1.DefaultVersionUpdateStrategy, + strategy temporaliov1alpha1.VersionRolloutStrategy, controllerIdentity string, ) (*TemporalWorkerState, error) { state := &TemporalWorkerState{ From a5d1e3305878720e5c79826071a65acab69bc7e4 Mon Sep 17 00:00:00 2001 From: Eniko Dif Date: Wed, 27 May 2026 17:52:02 +0200 Subject: [PATCH 6/6] Rebase fixes --- api/v1alpha1/workerdeployment_webhook_test.go | 3 +++ api/v1alpha1/zz_generated.deepcopy.go | 10 +++---- internal/controller/genplan.go | 4 +-- internal/planner/planner.go | 2 +- internal/planner/planner_test.go | 26 +++++++++---------- internal/testhelpers/test_builder.go | 4 +-- 6 files changed, 26 insertions(+), 23 deletions(-) diff --git a/api/v1alpha1/workerdeployment_webhook_test.go b/api/v1alpha1/workerdeployment_webhook_test.go index 9067318e..cd8638a6 100644 --- a/api/v1alpha1/workerdeployment_webhook_test.go +++ b/api/v1alpha1/workerdeployment_webhook_test.go @@ -77,6 +77,9 @@ func TestWorkerDeployment_ValidateCreate(t *testing.T) { }, "rollback strategy - Progressive rollback with AllAtOnce rollout warns": { obj: testhelpers.ModifyObj(testhelpers.MakeWDWithName("rollback-progressive-allatonce-rollout", ""), func(obj *temporaliov1alpha1.WorkerDeployment) *temporaliov1alpha1.WorkerDeployment { + obj.Spec.RolloutStrategy = temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateAllAtOnce, + } obj.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ Strategy: temporaliov1alpha1.RollbackProgressive, Steps: []temporaliov1alpha1.RolloutStep{ diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index b3e5723f..6dac14e9 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -636,11 +636,6 @@ func (in *TemporalWorkerDeploymentSpec) DeepCopyInto(out *TemporalWorkerDeployme **out = **in } in.RolloutStrategy.DeepCopyInto(&out.RolloutStrategy) - if in.RollbackStrategy != nil { - in, out := &in.RollbackStrategy, &out.RollbackStrategy - *out = new(RollbackStrategy) - (*in).DeepCopyInto(*out) - } in.SunsetStrategy.DeepCopyInto(&out.SunsetStrategy) out.WorkerOptions = in.WorkerOptions } @@ -788,6 +783,11 @@ func (in *WorkerDeploymentSpec) DeepCopyInto(out *WorkerDeploymentSpec) { **out = **in } in.RolloutStrategy.DeepCopyInto(&out.RolloutStrategy) + if in.RollbackStrategy != nil { + in, out := &in.RollbackStrategy, &out.RollbackStrategy + *out = new(RollbackStrategy) + (*in).DeepCopyInto(*out) + } in.SunsetStrategy.DeepCopyInto(&out.SunsetStrategy) out.WorkerOptions = in.WorkerOptions } diff --git a/internal/controller/genplan.go b/internal/controller/genplan.go index 55a89147..3017d506 100644 --- a/internal/controller/genplan.go +++ b/internal/controller/genplan.go @@ -172,10 +172,10 @@ func (r *WorkerDeploymentReconciler) generatePlan( return plan, nil } -func (r *TemporalWorkerDeploymentReconciler) resolveGateWorkflow( +func (r *WorkerDeploymentReconciler) resolveGateWorkflow( ctx context.Context, l logr.Logger, - w *temporaliov1alpha1.TemporalWorkerDeployment, + w *temporaliov1alpha1.WorkerDeployment, rolloutStrategy temporaliov1alpha1.RolloutStrategy, temporalState *temporal.TemporalWorkerState, ) (gateInput []byte, isSecret bool, err error) { diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 536be674..08e3aa40 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -763,7 +763,7 @@ func getTestWorkflows( func isRollbackScenario( l logr.Logger, - status *temporaliov1alpha1.TemporalWorkerDeploymentStatus, + status *temporaliov1alpha1.WorkerDeploymentStatus, temporalState *temporal.TemporalWorkerState, config *Config, ) bool { diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index deeb52c1..7b14a305 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -3596,14 +3596,14 @@ func TestIsRollbackScenario(t *testing.T) { testCases := []struct { name string - status *temporaliov1alpha1.TemporalWorkerDeploymentStatus + status *temporaliov1alpha1.WorkerDeploymentStatus temporalState *temporal.TemporalWorkerState config *Config expectedResult bool }{ { name: "rollback detected via LastCurrentTime within MaxVersionAge", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + status: &temporaliov1alpha1.WorkerDeploymentStatus{ TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ BuildID: "build-v1", @@ -3623,7 +3623,7 @@ func TestIsRollbackScenario(t *testing.T) { }, { name: "rollout when LastCurrentTime is nil", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + status: &temporaliov1alpha1.WorkerDeploymentStatus{ TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ BuildID: "build-v2", @@ -3643,7 +3643,7 @@ func TestIsRollbackScenario(t *testing.T) { }, { name: "rollout when target version not in temporal state", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + status: &temporaliov1alpha1.WorkerDeploymentStatus{ TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ BuildID: "build-v3", @@ -3663,7 +3663,7 @@ func TestIsRollbackScenario(t *testing.T) { }, { name: "rollout when temporalState is nil", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + status: &temporaliov1alpha1.WorkerDeploymentStatus{ TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ BuildID: "build-v1", @@ -3676,7 +3676,7 @@ func TestIsRollbackScenario(t *testing.T) { }, { name: "rollout when version age exceeds MaxVersionAge", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + status: &temporaliov1alpha1.WorkerDeploymentStatus{ TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ BuildID: "build-v1", @@ -3696,7 +3696,7 @@ func TestIsRollbackScenario(t *testing.T) { }, { name: "rollback disabled when MaxVersionAge is zero", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + status: &temporaliov1alpha1.WorkerDeploymentStatus{ TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ BuildID: "build-v1", @@ -3724,7 +3724,7 @@ func TestIsRollbackScenario(t *testing.T) { }, { name: "rollback skipped when rollout strategy is Manual", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + status: &temporaliov1alpha1.WorkerDeploymentStatus{ TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ BuildID: "build-v1", @@ -3826,7 +3826,7 @@ func TestGetVersionConfigDiff_RollbackScenario(t *testing.T) { testCases := []struct { name string - status *temporaliov1alpha1.TemporalWorkerDeploymentStatus + status *temporaliov1alpha1.WorkerDeploymentStatus temporalState *temporal.TemporalWorkerState config *Config expectSetCurrent bool @@ -3834,7 +3834,7 @@ func TestGetVersionConfigDiff_RollbackScenario(t *testing.T) { }{ { name: "rollback with default AllAtOnce strategy", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + status: &temporaliov1alpha1.WorkerDeploymentStatus{ TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ BuildID: "build-v1", @@ -3871,7 +3871,7 @@ func TestGetVersionConfigDiff_RollbackScenario(t *testing.T) { }, { name: "rollback with Progressive strategy", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + status: &temporaliov1alpha1.WorkerDeploymentStatus{ CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ BuildID: "build-v2", @@ -3920,7 +3920,7 @@ func TestGetVersionConfigDiff_RollbackScenario(t *testing.T) { }, { name: "normal rollout when LastCurrentTime is nil", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + status: &temporaliov1alpha1.WorkerDeploymentStatus{ CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ BuildID: "build-v1", @@ -3968,7 +3968,7 @@ func TestGetVersionConfigDiff_RollbackScenario(t *testing.T) { }, { name: "rollback when target version is drained", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + status: &temporaliov1alpha1.WorkerDeploymentStatus{ CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ BuildID: "build-v2", diff --git a/internal/testhelpers/test_builder.go b/internal/testhelpers/test_builder.go index 4aeecfe4..a62d6512 100644 --- a/internal/testhelpers/test_builder.go +++ b/internal/testhelpers/test_builder.go @@ -59,7 +59,7 @@ func (b *WorkerDeploymentBuilder) WithProgressiveStrategy(steps ...temporaliov1a } // WithRollbackAllAtOnceStrategy sets the rollback strategy to all-at-once -func (b *TemporalWorkerDeploymentBuilder) WithRollbackAllAtOnceStrategy() *TemporalWorkerDeploymentBuilder { +func (b *WorkerDeploymentBuilder) WithRollbackAllAtOnceStrategy() *WorkerDeploymentBuilder { b.twd.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ Strategy: temporaliov1alpha1.RollbackAllAtOnce, } @@ -67,7 +67,7 @@ func (b *TemporalWorkerDeploymentBuilder) WithRollbackAllAtOnceStrategy() *Tempo } // WithRollbackProgressiveStrategy sets the rollback strategy to progressive with given steps -func (b *TemporalWorkerDeploymentBuilder) WithRollbackProgressiveStrategy(steps ...temporaliov1alpha1.RolloutStep) *TemporalWorkerDeploymentBuilder { +func (b *WorkerDeploymentBuilder) WithRollbackProgressiveStrategy(steps ...temporaliov1alpha1.RolloutStep) *WorkerDeploymentBuilder { b.twd.Spec.RollbackStrategy = &temporaliov1alpha1.RollbackStrategy{ Strategy: temporaliov1alpha1.RollbackProgressive, Steps: steps,