From 37a40057b7146785e7569c7f03cc819b3b1a5565 Mon Sep 17 00:00:00 2001 From: Rob Holland Date: Wed, 2 Jul 2025 11:45:38 +0100 Subject: [PATCH 1/4] WIP. --- api/v1alpha1/worker_types.go | 11 +- api/v1alpha1/zz_generated.deepcopy.go | 5 - ...temporal.io_temporalworkerdeployments.yaml | 65 -- internal/controller/genplan.go | 10 +- internal/controller/genstatus.go | 12 +- internal/controller/state_mapper.go | 9 +- internal/controller/worker_controller.go | 17 +- internal/planner/planner.go | 137 +-- internal/planner/planner_test.go | 1028 ++++++++--------- 9 files changed, 588 insertions(+), 706 deletions(-) diff --git a/api/v1alpha1/worker_types.go b/api/v1alpha1/worker_types.go index a93542be..7960ede1 100644 --- a/api/v1alpha1/worker_types.go +++ b/api/v1alpha1/worker_types.go @@ -105,17 +105,12 @@ type TemporalWorkerDeploymentStatus struct { // TargetVersion is the desired next version. If TargetVersion.Deployment is nil, // then the controller should create it. If not nil, the controller should // wait for it to become healthy and then move it to the CurrentVersion. + // This must never be nil. TargetVersion *TargetWorkerDeploymentVersion `json:"targetVersion"` // CurrentVersion is the version that is currently registered with - // Temporal as the current version of its worker deployment. This must never be nil. - CurrentVersion *CurrentWorkerDeploymentVersion `json:"currentVersion"` - - // RampingVersion is the version that is currently registered with - // Temporal as the ramping version of its worker deployment. The controller - // should ensure that this is always equal to the TargetVersion, or, if the - // TargetVersion has been promoted to the current version, this should be nil. - RampingVersion *TargetWorkerDeploymentVersion `json:"rampingVersion,omitempty"` + // Temporal as the current version of its worker deployment. + CurrentVersion *CurrentWorkerDeploymentVersion `json:"currentVersion,omitempty"` // DeprecatedVersions are deployment versions that are no longer the default. Any // deployment versions that are unreachable should be deleted by the controller. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 90646487..a5833fd3 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -442,11 +442,6 @@ func (in *TemporalWorkerDeploymentStatus) DeepCopyInto(out *TemporalWorkerDeploy *out = new(CurrentWorkerDeploymentVersion) (*in).DeepCopyInto(*out) } - if in.RampingVersion != nil { - in, out := &in.RampingVersion, &out.RampingVersion - *out = new(TargetWorkerDeploymentVersion) - (*in).DeepCopyInto(*out) - } if in.DeprecatedVersions != nil { in, out := &in.DeprecatedVersions, &out.DeprecatedVersions *out = make([]*DeprecatedWorkerDeploymentVersion, len(*in)) diff --git a/helm/temporal-worker-controller/templates/crds/temporal.io_temporalworkerdeployments.yaml b/helm/temporal-worker-controller/templates/crds/temporal.io_temporalworkerdeployments.yaml index 98170457..cf3fb571 100644 --- a/helm/temporal-worker-controller/templates/crds/temporal.io_temporalworkerdeployments.yaml +++ b/helm/temporal-worker-controller/templates/crds/temporal.io_temporalworkerdeployments.yaml @@ -3854,71 +3854,6 @@ spec: type: array lastModifierIdentity: type: string - rampingVersion: - properties: - deployment: - properties: - apiVersion: - type: string - fieldPath: - type: string - kind: - type: string - name: - type: string - namespace: - type: string - resourceVersion: - type: string - uid: - type: string - type: object - x-kubernetes-map-type: atomic - healthySince: - format: date-time - type: string - managedBy: - type: string - rampPercentage: - type: number - rampingSince: - format: date-time - type: string - status: - type: string - taskQueues: - items: - properties: - name: - type: string - required: - - name - type: object - type: array - testWorkflows: - items: - properties: - runID: - type: string - status: - type: string - taskQueue: - type: string - workflowID: - type: string - required: - - runID - - status - - taskQueue - - workflowID - type: object - type: array - versionID: - type: string - required: - - status - - versionID - type: object targetVersion: properties: deployment: diff --git a/internal/controller/genplan.go b/internal/controller/genplan.go index c1609625..6cf30be8 100644 --- a/internal/controller/genplan.go +++ b/internal/controller/genplan.go @@ -17,6 +17,7 @@ import ( temporaliov1alpha1 "github.com/DataDog/temporal-worker-controller/api/v1alpha1" "github.com/DataDog/temporal-worker-controller/internal/k8s" "github.com/DataDog/temporal-worker-controller/internal/planner" + "github.com/DataDog/temporal-worker-controller/internal/temporal" ) // plan holds the actions to execute during reconciliation @@ -50,6 +51,7 @@ func (r *TemporalWorkerDeploymentReconciler) generatePlan( l logr.Logger, w *temporaliov1alpha1.TemporalWorkerDeployment, connection temporaliov1alpha1.TemporalConnectionSpec, + temporalState *temporal.TemporalWorkerState, ) (*plan, error) { workerDeploymentName := k8s.ComputeWorkerDeploymentName(w) targetVersionID := k8s.ComputeVersionID(w) @@ -82,17 +84,15 @@ func (r *TemporalWorkerDeploymentReconciler) generatePlan( // Generate the plan using the planner package plannerConfig := &planner.Config{ - Status: &w.Status, - Spec: &w.Spec, RolloutStrategy: rolloutStrategy, - TargetVersionID: targetVersionID, - Replicas: *w.Spec.Replicas, - ConflictToken: w.Status.VersionConflictToken, } planResult, err := planner.GeneratePlan( l, k8sState, + &w.Status, + &w.Spec, + temporalState, plannerConfig, ) if err != nil { diff --git a/internal/controller/genstatus.go b/internal/controller/genstatus.go index 6f22a3e7..90b4b698 100644 --- a/internal/controller/genstatus.go +++ b/internal/controller/genstatus.go @@ -25,6 +25,7 @@ func (r *TemporalWorkerDeploymentReconciler) generateStatus( temporalClient temporalClient.Client, req ctrl.Request, workerDeploy *temporaliov1alpha1.TemporalWorkerDeployment, + temporalState *temporal.TemporalWorkerState, ) (*temporaliov1alpha1.TemporalWorkerDeploymentStatus, error) { workerDeploymentName := computeWorkerDeploymentName(workerDeploy) targetVersionID := computeVersionID(workerDeploy) @@ -41,17 +42,6 @@ func (r *TemporalWorkerDeploymentReconciler) generateStatus( return nil, fmt.Errorf("unable to get Kubernetes deployment state: %w", err) } - // Fetch Temporal worker deployment state - temporalState, err := temporal.GetWorkerDeploymentState( - ctx, - temporalClient, - workerDeploymentName, - workerDeploy.Spec.WorkerOptions.TemporalNamespace, - ) - if err != nil { - return nil, fmt.Errorf("unable to get Temporal worker deployment state: %w", err) - } - // Fetch test workflow status for the desired version if targetVersionID != temporalState.CurrentVersionID { testWorkflows, err := temporal.GetTestWorkflowStatus( diff --git a/internal/controller/state_mapper.go b/internal/controller/state_mapper.go index 103f5ce0..d9bbc3dc 100644 --- a/internal/controller/state_mapper.go +++ b/internal/controller/state_mapper.go @@ -45,21 +45,14 @@ func (m *stateMapper) mapToStatus(targetVersionID string) *v1alpha1.TemporalWork status.TargetVersion.RampPercentage = &rampPercentage } - rampingVersionID := m.temporalState.RampingVersionID - // Set ramping version if it exists - if rampingVersionID != "" { - status.RampingVersion = m.mapTargetWorkerDeploymentVersion(rampingVersionID) - } - // Add deprecated versions var deprecatedVersions []*v1alpha1.DeprecatedWorkerDeploymentVersion for versionID := range m.k8sState.Deployments { // Skip current and target versions - if versionID == currentVersionID || versionID == targetVersionID || versionID == rampingVersionID { + if versionID == currentVersionID || versionID == targetVersionID { continue } - // TODO(rob): We should never see a version here that has VersionStatusCurrent, but should we check? versionStatus := m.mapDeprecatedWorkerDeploymentVersion(versionID) if versionStatus != nil { deprecatedVersions = append(deprecatedVersions, versionStatus) diff --git a/internal/controller/worker_controller.go b/internal/controller/worker_controller.go index 8880bd67..e1f8946a 100644 --- a/internal/controller/worker_controller.go +++ b/internal/controller/worker_controller.go @@ -21,6 +21,7 @@ import ( temporaliov1alpha1 "github.com/DataDog/temporal-worker-controller/api/v1alpha1" "github.com/DataDog/temporal-worker-controller/internal/controller/clientpool" + "github.com/DataDog/temporal-worker-controller/internal/temporal" ) var ( @@ -112,8 +113,20 @@ func (r *TemporalWorkerDeploymentReconciler) Reconcile(ctx context.Context, req temporalClient = c } + // Fetch Temporal worker deployment state + workerDeploymentName := computeWorkerDeploymentName(&workerDeploy) + temporalState, err := temporal.GetWorkerDeploymentState( + ctx, + temporalClient, + workerDeploymentName, + workerDeploy.Spec.WorkerOptions.TemporalNamespace, + ) + if err != nil { + return ctrl.Result{}, fmt.Errorf("unable to get Temporal worker deployment state: %w", err) + } + // Compute a new status from k8s and temporal state - status, err := r.generateStatus(ctx, l, temporalClient, req, &workerDeploy) + status, err := r.generateStatus(ctx, l, temporalClient, req, &workerDeploy, temporalState) if err != nil { return ctrl.Result{}, err } @@ -136,7 +149,7 @@ func (r *TemporalWorkerDeploymentReconciler) Reconcile(ctx context.Context, req workerDeploy.Default() // Generate a plan to get to desired spec from current status - plan, err := r.generatePlan(ctx, l, &workerDeploy, temporalConnection.Spec) + plan, err := r.generatePlan(ctx, l, &workerDeploy, temporalConnection.Spec, temporalState) if err != nil { return ctrl.Result{}, err } diff --git a/internal/planner/planner.go b/internal/planner/planner.go index f9f1e243..8c886a04 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -8,15 +8,15 @@ import ( "time" "github.com/go-logr/logr" - appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" temporaliov1alpha1 "github.com/DataDog/temporal-worker-controller/api/v1alpha1" "github.com/DataDog/temporal-worker-controller/internal/k8s" + "github.com/DataDog/temporal-worker-controller/internal/temporal" ) -// Plan represents the actions to be taken to move the system to the desired state +// Plan holds the actions to execute during reconciliation type Plan struct { // Which actions to take DeleteDeployments []*appsv1.Deployment @@ -26,7 +26,7 @@ type Plan struct { TestWorkflows []WorkflowConfig } -// VersionConfig represents version routing configuration +// VersionConfig defines version configuration for Temporal type VersionConfig struct { // Token to use for conflict detection ConflictToken []byte @@ -41,7 +41,7 @@ type VersionConfig struct { RampPercentage float32 } -// WorkflowConfig represents a workflow to be started +// WorkflowConfig defines a workflow to be started type WorkflowConfig struct { WorkflowType string WorkflowID string @@ -49,20 +49,10 @@ type WorkflowConfig struct { TaskQueue string } -// Config holds the inputs needed to generate a plan +// Config holds the configuration for planning type Config struct { - // Status of the TemporalWorkerDeployment - Status *temporaliov1alpha1.TemporalWorkerDeploymentStatus - // Spec of the TemporalWorkerDeployment - Spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec // RolloutStrategy to use RolloutStrategy temporaliov1alpha1.RolloutStrategy - // Desired version ID to deploy - TargetVersionID string - // Number of replicas desired - Replicas int32 - // Token to use for conflict detection - ConflictToken []byte } // ScaledownDelay returns the scaledown delay from the sunset strategy @@ -85,6 +75,9 @@ func getDeleteDelay(spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec) time. func GeneratePlan( l logr.Logger, k8sState *k8s.DeploymentState, + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus, + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec, + temporalState *temporal.TemporalWorkerState, config *Config, ) (*Plan, error) { plan := &Plan{ @@ -92,15 +85,15 @@ func GeneratePlan( } // Add delete/scale operations based on version status - plan.DeleteDeployments = getDeleteDeployments(k8sState, config) - plan.ScaleDeployments = getScaleDeployments(k8sState, config) - plan.ShouldCreateDeployment = shouldCreateDeployment(k8sState, config) + plan.DeleteDeployments = getDeleteDeployments(k8sState, status, spec) + plan.ScaleDeployments = getScaleDeployments(k8sState, status, spec) + plan.ShouldCreateDeployment = shouldCreateDeployment(status) // Determine if we need to start any test workflows - plan.TestWorkflows = getTestWorkflows(config) + plan.TestWorkflows = getTestWorkflows(status, config) // Determine version config changes - plan.VersionConfig = getVersionConfigDiff(l, config.RolloutStrategy, config.Status, config.ConflictToken) + plan.VersionConfig = getVersionConfigDiff(l, status, spec, temporalState, config) // TODO(jlegrone): generate warnings/events on the TemporalWorkerDeployment resource when buildIDs are reachable // but have no corresponding Deployment. @@ -111,11 +104,12 @@ func GeneratePlan( // getDeleteDeployments determines which deployments should be deleted func getDeleteDeployments( k8sState *k8s.DeploymentState, - config *Config, + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus, + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec, ) []*appsv1.Deployment { var deleteDeployments []*appsv1.Deployment - for _, version := range config.Status.DeprecatedVersions { + for _, version := range status.DeprecatedVersions { if version.Deployment == nil { continue } @@ -131,49 +125,53 @@ func getDeleteDeployments( // Deleting a deployment is only possible when: // 1. The deployment has been drained for deleteDelay + scaledownDelay. // 2. The deployment is scaled to 0 replicas. - if (time.Since(version.DrainedSince.Time) > getDeleteDelay(config.Spec)+getScaledownDelay(config.Spec)) && + if (time.Since(version.DrainedSince.Time) > getDeleteDelay(spec)+getScaledownDelay(spec)) && *d.Spec.Replicas == 0 { deleteDeployments = append(deleteDeployments, d) } case temporaliov1alpha1.VersionStatusNotRegistered: // NotRegistered versions are versions that the server doesn't know about. // Only delete if it's not the target version. - if config.Status.TargetVersion == nil || config.Status.TargetVersion.VersionID != version.VersionID { + if status.TargetVersion == nil || status.TargetVersion.VersionID != version.VersionID { deleteDeployments = append(deleteDeployments, d) } } } - // If the target version ID has changed, delete the latest unregistered deployment - if config.Status.TargetVersion != nil && config.Status.TargetVersion.Deployment != nil && - config.Status.TargetVersion.VersionID != config.TargetVersionID { - if d, exists := k8sState.Deployments[config.Status.TargetVersion.VersionID]; exists { - deleteDeployments = append(deleteDeployments, d) - } - } - return deleteDeployments } // getScaleDeployments determines which deployments should be scaled and to what size func getScaleDeployments( k8sState *k8s.DeploymentState, - config *Config, + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus, + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec, ) map[*v1.ObjectReference]uint32 { scaleDeployments := make(map[*v1.ObjectReference]uint32) + replicas := *spec.Replicas // Scale the current version if needed - if config.Status.CurrentVersion != nil && config.Status.CurrentVersion.Deployment != nil { - ref := config.Status.CurrentVersion.Deployment - if d, exists := k8sState.Deployments[config.Status.CurrentVersion.VersionID]; exists { - if d.Spec.Replicas != nil && *d.Spec.Replicas != config.Replicas { - scaleDeployments[ref] = uint32(config.Replicas) + if status.CurrentVersion != nil && status.CurrentVersion.Deployment != nil { + ref := status.CurrentVersion.Deployment + if d, exists := k8sState.Deployments[status.CurrentVersion.VersionID]; exists { + if d.Spec.Replicas != nil && *d.Spec.Replicas != replicas { + scaleDeployments[ref] = uint32(replicas) + } + } + } + + // Scale the target version if it exists, and isn't current + if (status.CurrentVersion == nil || status.CurrentVersion.VersionID != status.TargetVersion.VersionID) && + status.TargetVersion.Deployment != nil { + if d, exists := k8sState.Deployments[status.TargetVersion.VersionID]; exists { + if d.Spec.Replicas == nil || *d.Spec.Replicas != replicas { + scaleDeployments[status.TargetVersion.Deployment] = uint32(replicas) } } } // Scale other versions based on status - for _, version := range config.Status.DeprecatedVersions { + for _, version := range status.DeprecatedVersions { if version.Deployment == nil { continue } @@ -189,11 +187,11 @@ func getScaleDeployments( temporaliov1alpha1.VersionStatusCurrent: // TODO(carlydf): Consolidate scale up cases and verify that scale up is the correct action for inactive versions // Scale up these deployments - if d.Spec.Replicas != nil && *d.Spec.Replicas != config.Replicas { - scaleDeployments[version.Deployment] = uint32(config.Replicas) + if d.Spec.Replicas != nil && *d.Spec.Replicas != replicas { + scaleDeployments[version.Deployment] = uint32(replicas) } case temporaliov1alpha1.VersionStatusDrained: - if time.Since(version.DrainedSince.Time) > getScaledownDelay(config.Spec) { + if time.Since(version.DrainedSince.Time) > getScaledownDelay(spec) { // TODO(jlegrone): Compute scale based on load? Or percentage of replicas? // Scale down drained deployments after delay if d.Spec.Replicas != nil && *d.Spec.Replicas != 0 { @@ -203,56 +201,31 @@ func getScaleDeployments( } } - // Scale the target version if it exists - if config.Status.TargetVersion != nil && config.Status.TargetVersion.Deployment != nil && - config.Status.TargetVersion.VersionID == config.TargetVersionID { - if d, exists := k8sState.Deployments[config.Status.TargetVersion.VersionID]; exists { - if d.Spec.Replicas == nil || *d.Spec.Replicas != config.Replicas { - scaleDeployments[config.Status.TargetVersion.Deployment] = uint32(config.Replicas) - } - } - } - return scaleDeployments } // shouldCreateDeployment determines if a new deployment needs to be created func shouldCreateDeployment( - k8sState *k8s.DeploymentState, - config *Config, + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus, ) bool { - if config.Status.TargetVersion == nil { - return true - } - - if config.Status.TargetVersion.Deployment == nil { - return true - } - - // If the target version already has a deployment, we don't need to create another one - if config.Status.TargetVersion.VersionID == config.TargetVersionID { - if _, exists := k8sState.Deployments[config.TargetVersionID]; exists { - return false - } - } - - return true + return status.TargetVersion.Deployment == nil } // getTestWorkflows determines which test workflows should be started func getTestWorkflows( + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus, config *Config, ) []WorkflowConfig { var testWorkflows []WorkflowConfig // Skip if there's no gate workflow defined or if the target version is already the current - if config.RolloutStrategy.Gate == nil || config.Status.TargetVersion == nil || - config.Status.CurrentVersion == nil || - config.Status.CurrentVersion.VersionID == config.Status.TargetVersion.VersionID { + if config.RolloutStrategy.Gate == nil || + status.CurrentVersion == nil || + status.CurrentVersion.VersionID == status.TargetVersion.VersionID { return nil } - targetVersion := config.Status.TargetVersion + targetVersion := status.TargetVersion // Create a map of task queues that already have running test workflows taskQueuesWithWorkflows := make(map[string]struct{}) @@ -265,7 +238,7 @@ func getTestWorkflows( if _, ok := taskQueuesWithWorkflows[tq.Name]; !ok { testWorkflows = append(testWorkflows, WorkflowConfig{ WorkflowType: config.RolloutStrategy.Gate.WorkflowType, - WorkflowID: getTestWorkflowID(config, tq.Name, targetVersion.VersionID), + WorkflowID: getTestWorkflowID(tq.Name, targetVersion.VersionID), VersionID: targetVersion.VersionID, TaskQueue: tq.Name, }) @@ -276,19 +249,23 @@ func getTestWorkflows( } // getTestWorkflowID generates an ID for a test workflow -func getTestWorkflowID(config *Config, taskQueue, versionID string) string { +func getTestWorkflowID(taskQueue, versionID string) string { return "test-" + versionID + "-" + taskQueue } // getVersionConfigDiff determines the version configuration based on the rollout strategy func getVersionConfigDiff( l logr.Logger, - strategy temporaliov1alpha1.RolloutStrategy, status *temporaliov1alpha1.TemporalWorkerDeploymentStatus, - conflictToken []byte, + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec, + temporalState *temporal.TemporalWorkerState, + config *Config, ) *VersionConfig { + strategy := config.RolloutStrategy + conflictToken := status.VersionConflictToken + // Do nothing if target version's deployment is not healthy yet - if status == nil || status.TargetVersion == nil || status.TargetVersion.HealthySince == nil { + if status == nil || status.TargetVersion.HealthySince == nil { return nil } @@ -321,7 +298,7 @@ func getVersionConfigDiff( // If the current version is the target version if status.CurrentVersion.VersionID == status.TargetVersion.VersionID { // Reset ramp if needed, this would happen if a ramp has been rolled back before completing - if status.RampingVersion != nil { + if temporalState.RampingVersionID != "" { vcfg.VersionID = "" vcfg.RampPercentage = 0 return vcfg diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index b54d13e0..5de9fd68 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -18,35 +18,42 @@ import ( temporaliov1alpha1 "github.com/DataDog/temporal-worker-controller/api/v1alpha1" "github.com/DataDog/temporal-worker-controller/internal/k8s" + "github.com/DataDog/temporal-worker-controller/internal/temporal" ) func TestGeneratePlan(t *testing.T) { testCases := []struct { name string k8sState *k8s.DeploymentState + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec + state *temporal.TemporalWorkerState config *Config expectDelete int - expectScale int expectCreate bool + expectScale int expectWorkflow int expectConfig bool - expectConfigSetCurrent *bool // pointer to distinguish between false and not set - expectConfigRampPercent *float32 + expectConfigSetCurrent *bool // pointer so we can test nil + expectConfigRampPercent *float32 // pointer so we can test nil }{ { - name: "empty state creates new deployment", - k8sState: &k8s.DeploymentState{ - Deployments: map[string]*appsv1.Deployment{}, - DeploymentsByTime: []*appsv1.Deployment{}, - DeploymentRefs: map[string]*v1.ObjectReference{}, + name: "empty state creates new deployment", + k8sState: &k8s.DeploymentState{}, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusNotRegistered, + }, + }, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: func() *int32 { r := int32(1); return &r }(), }, + state: &temporal.TemporalWorkerState{}, config: &Config{ - TargetVersionID: "test/namespace.123", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{}, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 1, - ConflictToken: []byte{}, }, expectCreate: true, }, @@ -55,37 +62,49 @@ func TestGeneratePlan(t *testing.T) { k8sState: &k8s.DeploymentState{ Deployments: map[string]*appsv1.Deployment{ "test/namespace.123": createDeploymentWithReplicas(0), + "test/namespace.456": createDeploymentWithReplicas(1), }, DeploymentsByTime: []*appsv1.Deployment{ createDeploymentWithReplicas(0), - }, - DeploymentRefs: map[string]*v1.ObjectReference{ - "test/namespace.123": {Name: "test-123"}, + createDeploymentWithReplicas(1), }, }, - config: &Config{ - TargetVersionID: "test/namespace.456", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusDrained, - Deployment: &v1.ObjectReference{Name: "test-123"}, - }, - DrainedSince: &metav1.Time{ - Time: time.Now().Add(-24 * time.Hour), - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.456", + Status: temporaliov1alpha1.VersionStatusCurrent, + Deployment: &v1.ObjectReference{Name: "test-456"}, + }, + }, + CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.456", + Status: temporaliov1alpha1.VersionStatusCurrent, + Deployment: &v1.ObjectReference{Name: "test-456"}, + }, + }, + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusDrained, + Deployment: &v1.ObjectReference{Name: "test-123"}, + }, + DrainedSince: &metav1.Time{ + Time: time.Now().Add(-24 * time.Hour), }, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: func() *int32 { r := int32(1); return &r }(), + }, + state: &temporal.TemporalWorkerState{}, + config: &Config{ RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 1, - ConflictToken: []byte{}, }, expectDelete: 1, - expectCreate: true, }, { name: "deployment needs to be scaled", @@ -96,35 +115,31 @@ func TestGeneratePlan(t *testing.T) { DeploymentsByTime: []*appsv1.Deployment{ createDeploymentWithReplicas(1), }, - DeploymentRefs: map[string]*v1.ObjectReference{ - "test/namespace.123": {Name: "test-123"}, - }, }, - config: &Config{ - TargetVersionID: "test/namespace.123", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusCurrent, - Deployment: &v1.ObjectReference{Name: "test-123"}, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusCurrent, + Deployment: &v1.ObjectReference{Name: "test-123"}, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusCurrent, - Deployment: &v1.ObjectReference{Name: "test-123"}, - }, + }, + CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusCurrent, + Deployment: &v1.ObjectReference{Name: "test-123"}, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: func() *int32 { r := int32(2); return &r }(), + }, + state: &temporal.TemporalWorkerState{}, + config: &Config{ RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 2, - ConflictToken: []byte{}, }, - expectScale: 2, - expectCreate: false, + expectScale: 1, }, { name: "rollback scenario - target equals current but deprecated version is ramping", @@ -142,47 +157,47 @@ func TestGeneratePlan(t *testing.T) { "test/namespace.456": {Name: "test-456"}, }, }, - config: &Config{ - TargetVersionID: "test/namespace.123", // Rolling back to current version - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusCurrent, - Deployment: &v1.ObjectReference{Name: "test-123"}, - HealthySince: &metav1.Time{ - Time: time.Now().Add(-2 * time.Hour), - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusCurrent, + Deployment: &v1.ObjectReference{Name: "test-123"}, + HealthySince: &metav1.Time{ + Time: time.Now().Add(-2 * time.Hour), }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusCurrent, - Deployment: &v1.ObjectReference{Name: "test-123"}, - HealthySince: &metav1.Time{ - Time: time.Now().Add(-2 * time.Hour), - }, + }, + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusCurrent, + Deployment: &v1.ObjectReference{Name: "test-123"}, + HealthySince: &metav1.Time{ + Time: time.Now().Add(-2 * time.Hour), }, }, - RampingVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + }, + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusRamping, Deployment: &v1.ObjectReference{Name: "test-456"}, - HealthySince: &metav1.Time{ - Time: time.Now().Add(-30 * time.Minute), - }, }, - RampPercentage: func() *float32 { f := float32(25); return &f }(), }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: func() *int32 { r := int32(3); return &r }(), + }, + state: &temporal.TemporalWorkerState{ + RampingVersionID: "test/namespace.456", // This is what triggers the reset + }, + config: &Config{ RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ Strategy: temporaliov1alpha1.UpdateAllAtOnce, }, - Replicas: 3, - ConflictToken: []byte("token"), }, expectCreate: false, expectScale: 0, @@ -194,7 +209,7 @@ func TestGeneratePlan(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - plan, err := GeneratePlan(logr.Discard(), tc.k8sState, tc.config) + plan, err := GeneratePlan(logr.Discard(), tc.k8sState, tc.status, tc.spec, tc.state, tc.config) require.NoError(t, err) assert.Equal(t, tc.expectDelete, len(plan.DeleteDeployments), "unexpected number of deletions") @@ -220,6 +235,8 @@ func TestGetDeleteDeployments(t *testing.T) { testCases := []struct { name string k8sState *k8s.DeploymentState + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec config *Config expectDeletes int }{ @@ -230,26 +247,27 @@ func TestGetDeleteDeployments(t *testing.T) { "test/namespace.123": createDeploymentWithReplicas(0), }, }, - config: &Config{ - TargetVersionID: "test/namespace.456", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusDrained, - Deployment: &v1.ObjectReference{Name: "test-123"}, - }, - DrainedSince: &metav1.Time{ - Time: time.Now().Add(-24 * time.Hour), - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusDrained, + Deployment: &v1.ObjectReference{Name: "test-123"}, + }, + DrainedSince: &metav1.Time{ + Time: time.Now().Add(-24 * time.Hour), }, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, // Uses default sunset strategy: ScaledownDelay=0, DeleteDelay=0 - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 1, - ConflictToken: []byte{}, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + SunsetStrategy: temporaliov1alpha1.SunsetStrategy{ + DeleteDelay: &metav1.Duration{ + Duration: 4 * time.Hour, + }, + }, + Replicas: func() *int32 { r := int32(1); return &r }(), }, expectDeletes: 1, }, @@ -260,32 +278,34 @@ func TestGetDeleteDeployments(t *testing.T) { "test/namespace.123": createDeploymentWithReplicas(0), }, }, - config: &Config{ - TargetVersionID: "test/namespace.456", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusDrained, - Deployment: &v1.ObjectReference{Name: "test-123"}, - }, - DrainedSince: &metav1.Time{ - Time: time.Now().Add(-1 * time.Hour), - }, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusCurrent, + Deployment: &v1.ObjectReference{Name: "test-123"}, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ - SunsetStrategy: temporaliov1alpha1.SunsetStrategy{ - DeleteDelay: &metav1.Duration{ - Duration: 4 * time.Hour, + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.456", + Status: temporaliov1alpha1.VersionStatusDrained, + Deployment: &v1.ObjectReference{Name: "test-456"}, + }, + DrainedSince: &metav1.Time{ + Time: time.Now().Add(-1 * time.Hour), }, }, }, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 1, - ConflictToken: []byte{}, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + SunsetStrategy: temporaliov1alpha1.SunsetStrategy{ + DeleteDelay: &metav1.Duration{ + Duration: 4 * time.Hour, + }, + }, + Replicas: func() *int32 { r := int32(1); return &r }(), }, expectDeletes: 0, }, @@ -294,50 +314,29 @@ func TestGetDeleteDeployments(t *testing.T) { k8sState: &k8s.DeploymentState{ Deployments: map[string]*appsv1.Deployment{ "test/namespace.123": createDeploymentWithReplicas(1), + "test/namespace.456": createDeploymentWithReplicas(1), }, }, - config: &Config{ - TargetVersionID: "test/namespace.456", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusNotRegistered, - Deployment: &v1.ObjectReference{Name: "test-123"}, - }, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusCurrent, + Deployment: &v1.ObjectReference{Name: "test-123"}, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 1, - ConflictToken: []byte{}, - }, - expectDeletes: 1, - }, - { - name: "delete target deployment when version ID has changed", - k8sState: &k8s.DeploymentState{ - Deployments: map[string]*appsv1.Deployment{ - "test/namespace.b": createDeploymentWithReplicas(3), - }, - }, - config: &Config{ - TargetVersionID: "test/namespace.c", // Different desired version - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.b", - Status: temporaliov1alpha1.VersionStatusInactive, - Deployment: &v1.ObjectReference{Name: "test-b"}, + VersionID: "test/namespace.456", + Status: temporaliov1alpha1.VersionStatusNotRegistered, + Deployment: &v1.ObjectReference{Name: "test-456"}, }, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 3, - ConflictToken: []byte{}, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: func() *int32 { r := int32(1); return &r }(), }, expectDeletes: 1, }, @@ -345,7 +344,7 @@ func TestGetDeleteDeployments(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - deletes := getDeleteDeployments(tc.k8sState, tc.config) + deletes := getDeleteDeployments(tc.k8sState, tc.status, tc.spec) assert.Equal(t, tc.expectDeletes, len(deletes), "unexpected number of deletes") }) } @@ -355,31 +354,36 @@ func TestGetScaleDeployments(t *testing.T) { testCases := []struct { name string k8sState *k8s.DeploymentState - config *Config + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec + state *temporal.TemporalWorkerState expectScales int }{ { - name: "default version needs scaling", + name: "current version needs scaling", k8sState: &k8s.DeploymentState{ Deployments: map[string]*appsv1.Deployment{ "test/namespace.123": createDeploymentWithReplicas(1), }, }, - config: &Config{ - TargetVersionID: "test/namespace.123", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusCurrent, - Deployment: &v1.ObjectReference{Name: "test-123"}, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusCurrent, + Deployment: &v1.ObjectReference{Name: "test-123"}, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 2, - ConflictToken: []byte{}, + CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusCurrent, + Deployment: &v1.ObjectReference{Name: "test-123"}, + }, + }, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: func() *int32 { r := int32(2); return &r }(), }, expectScales: 1, }, @@ -388,28 +392,32 @@ func TestGetScaleDeployments(t *testing.T) { k8sState: &k8s.DeploymentState{ Deployments: map[string]*appsv1.Deployment{ "test/namespace.123": createDeploymentWithReplicas(1), + "test/namespace.456": createDeploymentWithReplicas(2), }, }, - config: &Config{ - TargetVersionID: "test/namespace.456", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusDrained, - Deployment: &v1.ObjectReference{Name: "test-123"}, - }, - DrainedSince: &metav1.Time{ - Time: time.Now().Add(-24 * time.Hour), - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusCurrent, + Deployment: &v1.ObjectReference{Name: "test-123"}, + }, + }, + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.456", + Status: temporaliov1alpha1.VersionStatusDrained, + Deployment: &v1.ObjectReference{Name: "test-456"}, + }, + DrainedSince: &metav1.Time{ + Time: time.Now().Add(-24 * time.Hour), }, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 2, - ConflictToken: []byte{}, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: func() *int32 { r := int32(1); return &r }(), }, expectScales: 1, }, @@ -423,23 +431,26 @@ func TestGetScaleDeployments(t *testing.T) { "test/namespace.a": {Name: "test-a"}, }, }, - config: &Config{ - TargetVersionID: "test/namespace.b", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.a", - Status: temporaliov1alpha1.VersionStatusInactive, - Deployment: &v1.ObjectReference{Name: "test-a"}, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusInactive, + Deployment: &v1.ObjectReference{Name: "test-123"}, + }, + }, + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.a", + Status: temporaliov1alpha1.VersionStatusInactive, + Deployment: &v1.ObjectReference{Name: "test-a"}, }, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 3, - ConflictToken: []byte{}, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: func() *int32 { r := int32(3); return &r }(), }, expectScales: 1, }, @@ -453,21 +464,20 @@ func TestGetScaleDeployments(t *testing.T) { "test/namespace.b": {Name: "test-b"}, }, }, - config: &Config{ - TargetVersionID: "test/namespace.b", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.b", - Status: temporaliov1alpha1.VersionStatusRamping, - Deployment: &v1.ObjectReference{Name: "test-b"}, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.b", + Status: temporaliov1alpha1.VersionStatusRamping, + Deployment: &v1.ObjectReference{Name: "test-b"}, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 3, - ConflictToken: []byte{}, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: func() *int32 { r := int32(3); return &r }(), + }, + state: &temporal.TemporalWorkerState{ + RampingVersionID: "test/namespace.b", }, expectScales: 1, }, @@ -481,23 +491,26 @@ func TestGetScaleDeployments(t *testing.T) { "test/namespace.a": {Name: "test-a"}, }, }, - config: &Config{ - TargetVersionID: "test/namespace.b", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.a", - Status: temporaliov1alpha1.VersionStatusInactive, - Deployment: &v1.ObjectReference{Name: "test-a"}, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.a", + Status: temporaliov1alpha1.VersionStatusInactive, + Deployment: &v1.ObjectReference{Name: "test-a"}, + }, + }, + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.b", + Status: temporaliov1alpha1.VersionStatusInactive, + Deployment: &v1.ObjectReference{Name: "test-b"}, }, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 3, - ConflictToken: []byte{}, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: func() *int32 { r := int32(3); return &r }(), }, expectScales: 1, }, @@ -511,39 +524,34 @@ func TestGetScaleDeployments(t *testing.T) { "test/namespace.b": {Name: "test-b"}, }, }, - config: &Config{ - TargetVersionID: "test/namespace.a", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.a", + Status: temporaliov1alpha1.VersionStatusInactive, + Deployment: &v1.ObjectReference{Name: "test-a"}, + }, + }, + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.a", - Status: temporaliov1alpha1.VersionStatusCurrent, - Deployment: &v1.ObjectReference{Name: "test-a"}, + VersionID: "test/namespace.b", + Status: temporaliov1alpha1.VersionStatusDrained, + Deployment: &v1.ObjectReference{Name: "test-b"}, }, - }, - DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.b", - Status: temporaliov1alpha1.VersionStatusDrained, - Deployment: &v1.ObjectReference{Name: "test-b"}, - }, - DrainedSince: &metav1.Time{ - Time: time.Now().Add(-1 * time.Hour), // Recently drained - }, + DrainedSince: &metav1.Time{ + Time: time.Now().Add(-1 * time.Hour), // Recently drained }, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ - SunsetStrategy: temporaliov1alpha1.SunsetStrategy{ - ScaledownDelay: &metav1.Duration{ - Duration: 4 * time.Hour, // Longer than 1 hour - }, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + SunsetStrategy: temporaliov1alpha1.SunsetStrategy{ + ScaledownDelay: &metav1.Duration{ + Duration: 4 * time.Hour, // Longer than 1 hour }, }, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 3, - ConflictToken: []byte{}, + Replicas: func() *int32 { r := int32(3); return &r }(), }, expectScales: 0, // No scaling yet because not enough time passed }, @@ -551,7 +559,7 @@ func TestGetScaleDeployments(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - scales := getScaleDeployments(tc.k8sState, tc.config) + scales := getScaleDeployments(tc.k8sState, tc.status, tc.spec) assert.Equal(t, tc.expectScales, len(scales), "unexpected number of scales") }) } @@ -560,72 +568,32 @@ func TestGetScaleDeployments(t *testing.T) { func TestShouldCreateDeployment(t *testing.T) { testCases := []struct { name string - k8sState *k8s.DeploymentState - config *Config + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus expectCreates bool }{ - { - name: "no target version should create", - k8sState: &k8s.DeploymentState{ - Deployments: map[string]*appsv1.Deployment{}, - }, - config: &Config{ - TargetVersionID: "test/namespace.123", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: nil, - }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 1, - ConflictToken: []byte{}, - }, - expectCreates: true, - }, { name: "existing deployment should not create", - k8sState: &k8s.DeploymentState{ - Deployments: map[string]*appsv1.Deployment{ - "test/namespace.123": createDeploymentWithReplicas(1), - }, - }, - config: &Config{ - TargetVersionID: "test/namespace.123", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusInactive, - Deployment: &v1.ObjectReference{Name: "test-123"}, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusInactive, + Deployment: &v1.ObjectReference{Name: "test-123"}, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 1, - ConflictToken: []byte{}, }, expectCreates: false, }, { name: "target version without deployment should create", - k8sState: &k8s.DeploymentState{ - Deployments: map[string]*appsv1.Deployment{}, - }, - config: &Config{ - TargetVersionID: "test/namespace.b", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.b", - Status: temporaliov1alpha1.VersionStatusInactive, - Deployment: nil, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.b", + Status: temporaliov1alpha1.VersionStatusInactive, + Deployment: nil, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 1, - ConflictToken: []byte{}, }, expectCreates: true, }, @@ -633,7 +601,7 @@ func TestShouldCreateDeployment(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - creates := shouldCreateDeployment(tc.k8sState, tc.config) + creates := shouldCreateDeployment(tc.status) assert.Equal(t, tc.expectCreates, creates, "unexpected create decision") }) } @@ -642,147 +610,132 @@ func TestShouldCreateDeployment(t *testing.T) { func TestGetTestWorkflows(t *testing.T) { testCases := []struct { name string + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus config *Config expectWorkflows int }{ { name: "gate workflow needed", - config: &Config{ - TargetVersionID: "test/namespace.123", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusInactive, - TaskQueues: []temporaliov1alpha1.TaskQueue{ - {Name: "queue1"}, - {Name: "queue2"}, - }, - }, - TestWorkflows: []temporaliov1alpha1.WorkflowExecution{ - { - TaskQueue: "queue1", - Status: temporaliov1alpha1.WorkflowExecutionStatusRunning, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusInactive, + TaskQueues: []temporaliov1alpha1.TaskQueue{ + {Name: "queue1"}, + {Name: "queue2"}, }, }, - CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.456", - Status: temporaliov1alpha1.VersionStatusCurrent, + TestWorkflows: []temporaliov1alpha1.WorkflowExecution{ + { + TaskQueue: "queue1", + Status: temporaliov1alpha1.WorkflowExecutionStatusRunning, }, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, + CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.456", + Status: temporaliov1alpha1.VersionStatusCurrent, + }, + }, + }, + config: &Config{ RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ Gate: &temporaliov1alpha1.GateWorkflowConfig{ WorkflowType: "TestWorkflow", }, }, - Replicas: 1, - ConflictToken: []byte{}, }, expectWorkflows: 1, // Only queue2 needs a workflow }, { name: "no gate workflow", - config: &Config{ - TargetVersionID: "test/namespace.123", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusInactive, - TaskQueues: []temporaliov1alpha1.TaskQueue{ - {Name: "queue1"}, - {Name: "queue2"}, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusInactive, + TaskQueues: []temporaliov1alpha1.TaskQueue{ + {Name: "queue1"}, + {Name: "queue2"}, }, }, - CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.456", - Status: temporaliov1alpha1.VersionStatusCurrent, - }, + }, + CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.456", + Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, + }, + config: &Config{ RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 1, - ConflictToken: []byte{}, }, expectWorkflows: 0, }, { name: "gate workflow with empty task queues", - config: &Config{ - TargetVersionID: "test/namespace.123", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusInactive, - TaskQueues: []temporaliov1alpha1.TaskQueue{}, // Empty - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusInactive, + TaskQueues: []temporaliov1alpha1.TaskQueue{}, // Empty }, - CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.456", - Status: temporaliov1alpha1.VersionStatusCurrent, - }, + }, + CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.456", + Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, + }, + config: &Config{ RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ Gate: &temporaliov1alpha1.GateWorkflowConfig{ WorkflowType: "TestWorkflow", }, }, - Replicas: 1, - ConflictToken: []byte{}, }, expectWorkflows: 0, // No task queues, no workflows }, { name: "all test workflows already running", - config: &Config{ - TargetVersionID: "test/namespace.123", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusInactive, - TaskQueues: []temporaliov1alpha1.TaskQueue{ - {Name: "queue1"}, - {Name: "queue2"}, - }, - }, - TestWorkflows: []temporaliov1alpha1.WorkflowExecution{ - { - TaskQueue: "queue1", - Status: temporaliov1alpha1.WorkflowExecutionStatusRunning, - }, - { - TaskQueue: "queue2", - Status: temporaliov1alpha1.WorkflowExecutionStatusCompleted, - }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusInactive, + TaskQueues: []temporaliov1alpha1.TaskQueue{ + {Name: "queue1"}, + {Name: "queue2"}, }, }, - CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.456", - Status: temporaliov1alpha1.VersionStatusCurrent, + TestWorkflows: []temporaliov1alpha1.WorkflowExecution{ + { + TaskQueue: "queue1", + Status: temporaliov1alpha1.WorkflowExecutionStatusRunning, + }, + { + TaskQueue: "queue2", + Status: temporaliov1alpha1.WorkflowExecutionStatusCompleted, }, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, + CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.456", + Status: temporaliov1alpha1.VersionStatusCurrent, + }, + }, + }, + config: &Config{ RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ Gate: &temporaliov1alpha1.GateWorkflowConfig{ WorkflowType: "TestWorkflow", }, }, - Replicas: 1, - ConflictToken: []byte{}, }, expectWorkflows: 0, // All queues have workflows }, @@ -790,7 +743,7 @@ func TestGetTestWorkflows(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - workflows := getTestWorkflows(tc.config) + workflows := getTestWorkflows(tc.status, tc.config) assert.Equal(t, tc.expectWorkflows, len(workflows), "unexpected number of test workflows") }) } @@ -801,7 +754,8 @@ func TestGetVersionConfigDiff(t *testing.T) { name string strategy temporaliov1alpha1.RolloutStrategy status *temporaliov1alpha1.TemporalWorkerDeploymentStatus - conflictToken []byte + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec + state *temporal.TemporalWorkerState expectConfig bool expectSetCurrent bool expectRampPercent *float32 // Made pointer to handle nil case @@ -826,7 +780,7 @@ func TestGetVersionConfigDiff(t *testing.T) { }, }, }, - conflictToken: []byte("token"), + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: true, expectSetCurrent: true, }, @@ -846,8 +800,8 @@ func TestGetVersionConfigDiff(t *testing.T) { status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusInactive, + VersionID: "test/namespace.123", + Status: temporaliov1alpha1.VersionStatusInactive, HealthySince: &metav1.Time{ Time: time.Now().Add(-30 * time.Minute), }, @@ -861,7 +815,7 @@ func TestGetVersionConfigDiff(t *testing.T) { }, }, }, - conflictToken: []byte("token"), + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: true, expectSetCurrent: false, }, @@ -886,21 +840,22 @@ func TestGetVersionConfigDiff(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - RampingVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.456", - Status: temporaliov1alpha1.VersionStatusRamping, - HealthySince: &metav1.Time{ - Time: time.Now().Add(-30 * time.Minute), + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.456", + Status: temporaliov1alpha1.VersionStatusRamping, + HealthySince: &metav1.Time{ + Time: time.Now().Add(-30 * time.Minute), + }, }, }, - RampPercentage: func() *float32 { f := float32(25); return &f }(), }, }, - conflictToken: []byte("token"), - expectConfig: true, - expectSetCurrent: false, - expectRampPercent: func() *float32 { f := float32(0); return &f }(), + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, + state: &temporal.TemporalWorkerState{RampingVersionID: "test/namespace.456"}, + expectConfig: true, + expectSetCurrent: false, }, { name: "roll-forward scenario - target differs from current but different version is ramping", @@ -923,14 +878,16 @@ func TestGetVersionConfigDiff(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - RampingVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.456", - Status: temporaliov1alpha1.VersionStatusRamping, + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.456", + Status: temporaliov1alpha1.VersionStatusRamping, + }, }, }, }, - conflictToken: []byte("token"), + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: true, expectSetCurrent: true, expectRampPercent: func() *float32 { f := float32(0); return &f }(), @@ -939,13 +896,16 @@ func TestGetVersionConfigDiff(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - config := getVersionConfigDiff(logr.Discard(), tc.strategy, tc.status, tc.conflictToken) - assert.Equal(t, tc.expectConfig, config != nil, "unexpected version config presence") + config := &Config{ + RolloutStrategy: tc.strategy, + } + versionConfig := getVersionConfigDiff(logr.Discard(), tc.status, tc.spec, tc.state, config) + assert.Equal(t, tc.expectConfig, versionConfig != nil, "unexpected version config presence") if tc.expectConfig { - assert.NotNil(t, config, "expected version config") - assert.Equal(t, tc.expectSetCurrent, config.SetCurrent, "unexpected set current value") + assert.NotNil(t, versionConfig, "expected version config") + assert.Equal(t, tc.expectSetCurrent, versionConfig.SetCurrent, "unexpected set current value") if tc.expectRampPercent != nil { - assert.Equal(t, *tc.expectRampPercent, config.RampPercentage, "unexpected ramp percentage") + assert.Equal(t, *tc.expectRampPercent, versionConfig.RampPercentage, "unexpected ramp percentage") } } }) @@ -957,6 +917,8 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { name string strategy temporaliov1alpha1.RolloutStrategy status *temporaliov1alpha1.TemporalWorkerDeploymentStatus + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec + state *temporal.TemporalWorkerState expectConfig bool expectSetCurrent bool expectRampPercent float32 @@ -991,6 +953,7 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { }, }, }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: true, expectSetCurrent: true, // Should become current after all steps }, @@ -1018,6 +981,7 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { RampingSince: nil, // Not ramping yet }, }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: true, expectRampPercent: 25, // First step expectSetCurrent: false, @@ -1049,6 +1013,7 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { }, }, }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: true, expectRampPercent: 0, // When set as current, ramp is 0 expectSetCurrent: true, // At exactly 2 hours, it sets as current @@ -1081,6 +1046,7 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { }, }, }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: true, expectRampPercent: 25, // Should maintain previous ramp value expectSetCurrent: false, @@ -1112,6 +1078,7 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { }, }, }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: true, expectRampPercent: 0, expectSetCurrent: true, // Past all steps, should be default @@ -1120,12 +1087,15 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - config := getVersionConfigDiff(logr.Discard(), tc.strategy, tc.status, []byte("token")) - assert.Equal(t, tc.expectConfig, config != nil, "unexpected version config presence") + config := &Config{ + RolloutStrategy: tc.strategy, + } + versionConfig := getVersionConfigDiff(logr.Discard(), tc.status, tc.spec, tc.state, config) + assert.Equal(t, tc.expectConfig, versionConfig != nil, "unexpected version config presence") if tc.expectConfig { - assert.Equal(t, tc.expectSetCurrent, config.SetCurrent, "unexpected set default value") + assert.Equal(t, tc.expectSetCurrent, versionConfig.SetCurrent, "unexpected set default value") if !tc.expectSetCurrent { - assert.Equal(t, tc.expectRampPercent, config.RampPercentage, "unexpected ramp percentage") + assert.Equal(t, tc.expectRampPercent, versionConfig.RampPercentage, "unexpected ramp percentage") } } }) @@ -1137,6 +1107,8 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { name string strategy temporaliov1alpha1.RolloutStrategy status *temporaliov1alpha1.TemporalWorkerDeploymentStatus + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec + state *temporal.TemporalWorkerState expectConfig bool }{ { @@ -1171,6 +1143,7 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { }, }, }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: false, // Should not proceed with failed test }, { @@ -1205,6 +1178,7 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { }, }, }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: false, // Should not proceed with cancelled test }, { @@ -1239,6 +1213,7 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { }, }, }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: false, // Should not proceed with terminated test }, { @@ -1275,6 +1250,7 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { }, }, }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: false, // Should not proceed with incomplete tests }, { @@ -1301,17 +1277,21 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { }, }, }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: false, // Should not proceed with no task queues }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - config := getVersionConfigDiff(logr.Discard(), tc.strategy, tc.status, []byte("token")) + config := &Config{ + RolloutStrategy: tc.strategy, + } + versionConfig := getVersionConfigDiff(logr.Discard(), tc.status, tc.spec, tc.state, config) if tc.expectConfig { - assert.NotNil(t, config, "expected version config") + assert.NotNil(t, versionConfig, "expected version config") } else { - assert.Nil(t, config, "expected no version config") + assert.Nil(t, versionConfig, "expected no version config") } }) } @@ -1361,6 +1341,9 @@ func TestComplexVersionStateScenarios(t *testing.T) { name string k8sState *k8s.DeploymentState config *Config + status *temporaliov1alpha1.TemporalWorkerDeploymentStatus + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec + state *temporal.TemporalWorkerState expectDeletes int expectScales int expectVersions []string // Expected version IDs for scaling @@ -1369,65 +1352,71 @@ func TestComplexVersionStateScenarios(t *testing.T) { name: "multiple deprecated versions in different states", k8sState: &k8s.DeploymentState{ Deployments: map[string]*appsv1.Deployment{ - "test/namespace.a": createDeploymentWithReplicas(3), + "test/namespace.a": createDeploymentWithReplicas(5), "test/namespace.b": createDeploymentWithReplicas(3), - "test/namespace.c": createDeploymentWithReplicas(1), - "test/namespace.d": createDeploymentWithReplicas(0), + "test/namespace.c": createDeploymentWithReplicas(3), + "test/namespace.d": createDeploymentWithReplicas(1), + "test/namespace.e": createDeploymentWithReplicas(0), }, }, config: &Config{ - TargetVersionID: "test/namespace.e", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.a", - Status: temporaliov1alpha1.VersionStatusInactive, - Deployment: &v1.ObjectReference{Name: "test-a"}, - }, + RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, + }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.a", + Status: temporaliov1alpha1.VersionStatusInactive, + Deployment: &v1.ObjectReference{Name: "test-a"}, + }, + }, + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.b", + Status: temporaliov1alpha1.VersionStatusInactive, + Deployment: &v1.ObjectReference{Name: "test-b"}, }, - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.b", - Status: temporaliov1alpha1.VersionStatusDraining, - Deployment: &v1.ObjectReference{Name: "test-b"}, - }, + }, + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.c", + Status: temporaliov1alpha1.VersionStatusDraining, + Deployment: &v1.ObjectReference{Name: "test-c"}, }, - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.c", - Status: temporaliov1alpha1.VersionStatusDrained, - Deployment: &v1.ObjectReference{Name: "test-c"}, - }, - DrainedSince: &metav1.Time{ - Time: time.Now().Add(-2 * time.Hour), // Recently drained - }, + }, + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.d", + Status: temporaliov1alpha1.VersionStatusDrained, + Deployment: &v1.ObjectReference{Name: "test-d"}, }, - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.d", - Status: temporaliov1alpha1.VersionStatusDrained, - Deployment: &v1.ObjectReference{Name: "test-d"}, - }, - DrainedSince: &metav1.Time{ - Time: time.Now().Add(-48 * time.Hour), // Long time drained - }, + DrainedSince: &metav1.Time{ + Time: time.Now().Add(-2 * time.Hour), // Recently drained }, }, - }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ - SunsetStrategy: temporaliov1alpha1.SunsetStrategy{ - ScaledownDelay: &metav1.Duration{Duration: 1 * time.Hour}, - DeleteDelay: &metav1.Duration{Duration: 24 * time.Hour}, + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.e", + Status: temporaliov1alpha1.VersionStatusDrained, + Deployment: &v1.ObjectReference{Name: "test-e"}, + }, + DrainedSince: &metav1.Time{ + Time: time.Now().Add(-48 * time.Hour), // Long time drained + }, }, }, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 5, - ConflictToken: []byte{}, }, - expectDeletes: 1, // Only d should be deleted (drained long enough and scaled to 0) - expectScales: 2, // a needs scaling up, c needs scaling down - expectVersions: []string{"test/namespace.a", "test/namespace.c"}, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + SunsetStrategy: temporaliov1alpha1.SunsetStrategy{ + ScaledownDelay: &metav1.Duration{Duration: 1 * time.Hour}, + DeleteDelay: &metav1.Duration{Duration: 24 * time.Hour}, + }, + Replicas: func() *int32 { r := int32(5); return &r }(), + }, + expectDeletes: 1, // Only e should be deleted (drained long enough and scaled to 0) + expectScales: 2, // b needs scaling up, d needs scaling down + expectVersions: []string{"test/namespace.b", "test/namespace.d"}, }, { name: "draining version not scaled down before delay", @@ -1437,29 +1426,34 @@ func TestComplexVersionStateScenarios(t *testing.T) { }, }, config: &Config{ - TargetVersionID: "test/namespace.b", - Status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ - { - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.a", - Status: temporaliov1alpha1.VersionStatusDrained, - Deployment: &v1.ObjectReference{Name: "test-a"}, - }, - DrainedSince: &metav1.Time{ - Time: time.Now().Add(-30 * time.Minute), // Not long enough - }, - }, + RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, + }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.a", + Status: temporaliov1alpha1.VersionStatusInactive, + Deployment: &v1.ObjectReference{Name: "test-a"}, }, }, - Spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ - SunsetStrategy: temporaliov1alpha1.SunsetStrategy{ - ScaledownDelay: &metav1.Duration{Duration: 2 * time.Hour}, + DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ + { + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + VersionID: "test/namespace.a", + Status: temporaliov1alpha1.VersionStatusDrained, + Deployment: &v1.ObjectReference{Name: "test-a"}, + }, + DrainedSince: &metav1.Time{ + Time: time.Now().Add(-30 * time.Minute), // Not long enough + }, }, }, - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, - Replicas: 3, - ConflictToken: []byte{}, + }, + spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + SunsetStrategy: temporaliov1alpha1.SunsetStrategy{ + ScaledownDelay: &metav1.Duration{Duration: 2 * time.Hour}, + }, + Replicas: func() *int32 { r := int32(3); return &r }(), }, expectDeletes: 0, expectScales: 0, // Should not scale down yet @@ -1468,7 +1462,7 @@ func TestComplexVersionStateScenarios(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - plan, err := GeneratePlan(logr.Discard(), tc.k8sState, tc.config) + plan, err := GeneratePlan(logr.Discard(), tc.k8sState, tc.status, tc.spec, tc.state, tc.config) require.NoError(t, err) assert.Equal(t, tc.expectDeletes, len(plan.DeleteDeployments), "unexpected number of deletes") @@ -1477,17 +1471,17 @@ func TestComplexVersionStateScenarios(t *testing.T) { if tc.expectVersions != nil { scaledVersions := make([]string, 0, len(plan.ScaleDeployments)) for ref := range plan.ScaleDeployments { - // Extract version ID by looking up in config - for _, v := range tc.config.Status.DeprecatedVersions { + // Extract version ID by looking up in status + for _, v := range tc.status.DeprecatedVersions { if v.Deployment != nil && v.Deployment.Name == ref.Name { scaledVersions = append(scaledVersions, v.VersionID) break } } - if tc.config.Status.CurrentVersion != nil && - tc.config.Status.CurrentVersion.Deployment != nil && - tc.config.Status.CurrentVersion.Deployment.Name == ref.Name { - scaledVersions = append(scaledVersions, tc.config.Status.CurrentVersion.VersionID) + if tc.status.CurrentVersion != nil && + tc.status.CurrentVersion.Deployment != nil && + tc.status.CurrentVersion.Deployment.Name == ref.Name { + scaledVersions = append(scaledVersions, tc.status.CurrentVersion.VersionID) } } // Sort for consistent comparison @@ -1500,34 +1494,24 @@ func TestComplexVersionStateScenarios(t *testing.T) { func TestGetTestWorkflowID(t *testing.T) { testCases := []struct { name string - config *Config taskQueue string versionID string expectID string }{ { - name: "basic workflow ID generation", - config: &Config{ - TargetVersionID: "test/namespace.123", - }, + name: "basic workflow ID generation", taskQueue: "my-queue", versionID: "test/namespace.123", expectID: "test-test/namespace.123-my-queue", }, { - name: "workflow ID with special characters in queue name", - config: &Config{ - TargetVersionID: "test/namespace.456", - }, + name: "workflow ID with special characters in queue name", taskQueue: "queue-with-dashes-and_underscores", versionID: "test/namespace.456", expectID: "test-test/namespace.456-queue-with-dashes-and_underscores", }, { - name: "workflow ID with dots in version", - config: &Config{ - TargetVersionID: "test/namespace.1.2.3", - }, + name: "workflow ID with dots in version", taskQueue: "queue", versionID: "test/namespace.1.2.3", expectID: "test-test/namespace.1.2.3-queue", @@ -1536,7 +1520,7 @@ func TestGetTestWorkflowID(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - id := getTestWorkflowID(tc.config, tc.taskQueue, tc.versionID) + id := getTestWorkflowID(tc.taskQueue, tc.versionID) assert.Equal(t, tc.expectID, id, "unexpected workflow ID") }) } From 39cc5354cc91a57a513d5b3c1c6efb7f94378851 Mon Sep 17 00:00:00 2001 From: Rob Holland Date: Wed, 2 Jul 2025 14:23:20 +0100 Subject: [PATCH 2/4] Remove some nil guards which should not be needed. Tidy up some tests, removing state that isn't relevant. --- internal/controller/state_mapper.go | 10 ++----- internal/planner/planner.go | 7 +++-- internal/planner/planner_test.go | 42 ++++------------------------- 3 files changed, 10 insertions(+), 49 deletions(-) diff --git a/internal/controller/state_mapper.go b/internal/controller/state_mapper.go index 4b00e95a..873b5e30 100644 --- a/internal/controller/state_mapper.go +++ b/internal/controller/state_mapper.go @@ -33,13 +33,11 @@ func (m *stateMapper) mapToStatus(targetVersionID string) *v1alpha1.TemporalWork // Set current version currentVersionID := m.temporalState.CurrentVersionID - if currentVersionID != "" { - status.CurrentVersion = m.mapCurrentWorkerDeploymentVersion(currentVersionID) - } + status.CurrentVersion = m.mapCurrentWorkerDeploymentVersion(currentVersionID) // Set target version (desired version) status.TargetVersion = m.mapTargetWorkerDeploymentVersion(targetVersionID) - if status.TargetVersion != nil && m.temporalState.RampingVersionID == targetVersionID { + if m.temporalState.RampingVersionID == targetVersionID { status.TargetVersion.RampingSince = m.temporalState.RampingSince status.TargetVersion.RampLastModifiedAt = m.temporalState.RampLastModifiedAt rampPercentage := m.temporalState.RampPercentage @@ -101,10 +99,6 @@ func (m *stateMapper) mapCurrentWorkerDeploymentVersion(versionID string) *v1alp // mapTargetWorkerDeploymentVersion creates a target version status from the states func (m *stateMapper) mapTargetWorkerDeploymentVersion(versionID string) *v1alpha1.TargetWorkerDeploymentVersion { - if versionID == "" { - return nil - } - version := &v1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: v1alpha1.BaseWorkerDeploymentVersion{ VersionID: versionID, diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 9622317b..0fa6cf18 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -95,7 +95,7 @@ func GeneratePlan( plan.TestWorkflows = getTestWorkflows(status, config) // Determine version config changes - plan.VersionConfig = getVersionConfigDiff(l, status, spec, temporalState, config) + plan.VersionConfig = getVersionConfigDiff(l, status, temporalState, config) // TODO(jlegrone): generate warnings/events on the TemporalWorkerDeployment resource when buildIDs are reachable // but have no corresponding Deployment. @@ -134,7 +134,7 @@ func getDeleteDeployments( case temporaliov1alpha1.VersionStatusNotRegistered: // NotRegistered versions are versions that the server doesn't know about. // Only delete if it's not the target version. - if status.TargetVersion == nil || status.TargetVersion.VersionID != version.VersionID { + if status.TargetVersion.VersionID != version.VersionID { deleteDeployments = append(deleteDeployments, d) } } @@ -259,7 +259,6 @@ func getTestWorkflowID(taskQueue, versionID string) string { func getVersionConfigDiff( l logr.Logger, status *temporaliov1alpha1.TemporalWorkerDeploymentStatus, - spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec, temporalState *temporal.TemporalWorkerState, config *Config, ) *VersionConfig { @@ -267,7 +266,7 @@ func getVersionConfigDiff( conflictToken := status.VersionConflictToken // Do nothing if target version's deployment is not healthy yet - if status == nil || status.TargetVersion.HealthySince == nil { + if status.TargetVersion.HealthySince == nil { return nil } diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index 8be147e1..20350f0a 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -275,17 +275,10 @@ func TestGetDeleteDeployments(t *testing.T) { name: "not yet drained long enough", k8sState: &k8s.DeploymentState{ Deployments: map[string]*appsv1.Deployment{ - "test/namespace.123": createDeploymentWithReplicas(0), + "test/namespace.456": createDeploymentWithReplicas(0), }, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.123", - Status: temporaliov1alpha1.VersionStatusCurrent, - Deployment: &v1.ObjectReference{Name: "test-123"}, - }, - }, DeprecatedVersions: []*temporaliov1alpha1.DeprecatedWorkerDeploymentVersion{ { BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ @@ -482,7 +475,7 @@ func TestGetScaleDeployments(t *testing.T) { expectScales: 1, }, { - name: "current version needs scaling up", + name: "target version needs scaling up", k8sState: &k8s.DeploymentState{ Deployments: map[string]*appsv1.Deployment{ "test/namespace.a": createDeploymentWithReplicas(0), @@ -662,12 +655,6 @@ func TestGetTestWorkflows(t *testing.T) { }, }, }, - CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.456", - Status: temporaliov1alpha1.VersionStatusCurrent, - }, - }, }, config: &Config{ RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, @@ -684,12 +671,6 @@ func TestGetTestWorkflows(t *testing.T) { TaskQueues: []temporaliov1alpha1.TaskQueue{}, // Empty }, }, - CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.456", - Status: temporaliov1alpha1.VersionStatusCurrent, - }, - }, }, config: &Config{ RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ @@ -723,12 +704,6 @@ func TestGetTestWorkflows(t *testing.T) { }, }, }, - CurrentVersion: &temporaliov1alpha1.CurrentWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - VersionID: "test/namespace.456", - Status: temporaliov1alpha1.VersionStatusCurrent, - }, - }, }, config: &Config{ RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ @@ -903,7 +878,7 @@ func TestGetVersionConfigDiff(t *testing.T) { config := &Config{ RolloutStrategy: tc.strategy, } - versionConfig := getVersionConfigDiff(logr.Discard(), tc.status, tc.spec, tc.state, config) + versionConfig := getVersionConfigDiff(logr.Discard(), tc.status, tc.state, config) assert.Equal(t, tc.expectConfig, versionConfig != nil, "unexpected version config presence") if tc.expectConfig { assert.NotNil(t, versionConfig, "expected version config") @@ -1098,8 +1073,7 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { config := &Config{ RolloutStrategy: tc.strategy, } - spec := &temporaliov1alpha1.TemporalWorkerDeploymentSpec{} - versionConfig := getVersionConfigDiff(testlogr.New(t), tc.status, spec, tc.state, config) + versionConfig := getVersionConfigDiff(testlogr.New(t), tc.status, tc.state, config) assert.Equal(t, tc.expectConfig, versionConfig != nil, "unexpected version config presence") if tc.expectConfig { assert.Equal(t, tc.expectSetCurrent, versionConfig.SetCurrent, "unexpected set default value") @@ -1237,7 +1211,6 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { name string strategy temporaliov1alpha1.RolloutStrategy status *temporaliov1alpha1.TemporalWorkerDeploymentStatus - spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec state *temporal.TemporalWorkerState expectConfig bool }{ @@ -1273,7 +1246,6 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { }, }, }, - spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: false, // Should not proceed with failed test }, { @@ -1308,7 +1280,6 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { }, }, }, - spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: false, // Should not proceed with cancelled test }, { @@ -1343,7 +1314,6 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { }, }, }, - spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: false, // Should not proceed with terminated test }, { @@ -1380,7 +1350,6 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { }, }, }, - spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: false, // Should not proceed with incomplete tests }, { @@ -1407,7 +1376,6 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { }, }, }, - spec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{}, expectConfig: false, // Should not proceed with no task queues }, } @@ -1417,7 +1385,7 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { config := &Config{ RolloutStrategy: tc.strategy, } - versionConfig := getVersionConfigDiff(logr.Discard(), tc.status, tc.spec, tc.state, config) + versionConfig := getVersionConfigDiff(logr.Discard(), tc.status, tc.state, config) if tc.expectConfig { assert.NotNil(t, versionConfig, "expected version config") } else { From c223c17a5a66f590c1138aca68d600cf58ecf225 Mon Sep 17 00:00:00 2001 From: Rob Holland Date: Tue, 15 Jul 2025 12:52:52 +0100 Subject: [PATCH 3/4] Make TargetVersion required. --- api/v1alpha1/worker_types.go | 1 + internal/controller/genstatus.go | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/api/v1alpha1/worker_types.go b/api/v1alpha1/worker_types.go index e79ae69e..e17e9d3e 100644 --- a/api/v1alpha1/worker_types.go +++ b/api/v1alpha1/worker_types.go @@ -106,6 +106,7 @@ type TemporalWorkerDeploymentStatus struct { // then the controller should create it. If not nil, the controller should // wait for it to become healthy and then move it to the CurrentVersion. // This must never be nil. + // +kubebuilder:validation:Required TargetVersion *TargetWorkerDeploymentVersion `json:"targetVersion"` // CurrentVersion is the version that is currently registered with diff --git a/internal/controller/genstatus.go b/internal/controller/genstatus.go index 0473ccc3..55200524 100644 --- a/internal/controller/genstatus.go +++ b/internal/controller/genstatus.go @@ -66,5 +66,10 @@ func (r *TemporalWorkerDeploymentReconciler) generateStatus( stateMapper := newStateMapper(k8sState, temporalState) status := stateMapper.mapToStatus(targetVersionID) + // Validate that TargetVersion is never nil (defensive programming) + if status.TargetVersion == nil { + return nil, fmt.Errorf("generated status has nil TargetVersion for target version %s, this should never happen", targetVersionID) + } + return status, nil } From 6375ce0fdc7fa2842645a992596a7996956b0dd6 Mon Sep 17 00:00:00 2001 From: Rob Holland Date: Fri, 18 Jul 2025 12:26:21 +0100 Subject: [PATCH 4/4] Adjust API so TargetVersion is required. This is clearer than leaving it as a pointer and validating. --- api/v1alpha1/worker_types.go | 4 +- api/v1alpha1/zz_generated.deepcopy.go | 6 +-- internal/controller/genstatus.go | 5 -- internal/controller/state_mapper.go | 4 +- internal/controller/state_mapper_test.go | 1 - internal/planner/planner_test.go | 66 ++++++++++++------------ 6 files changed, 37 insertions(+), 49 deletions(-) diff --git a/api/v1alpha1/worker_types.go b/api/v1alpha1/worker_types.go index e17e9d3e..a48f875b 100644 --- a/api/v1alpha1/worker_types.go +++ b/api/v1alpha1/worker_types.go @@ -105,9 +105,7 @@ type TemporalWorkerDeploymentStatus struct { // TargetVersion is the desired next version. If TargetVersion.Deployment is nil, // then the controller should create it. If not nil, the controller should // wait for it to become healthy and then move it to the CurrentVersion. - // This must never be nil. - // +kubebuilder:validation:Required - TargetVersion *TargetWorkerDeploymentVersion `json:"targetVersion"` + TargetVersion TargetWorkerDeploymentVersion `json:"targetVersion"` // CurrentVersion is the version that is currently registered with // Temporal as the current version of its worker deployment. This will be nil diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index d007a94d..8f4af3d9 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -436,11 +436,7 @@ func (in *TemporalWorkerDeploymentSpec) DeepCopy() *TemporalWorkerDeploymentSpec // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TemporalWorkerDeploymentStatus) DeepCopyInto(out *TemporalWorkerDeploymentStatus) { *out = *in - if in.TargetVersion != nil { - in, out := &in.TargetVersion, &out.TargetVersion - *out = new(TargetWorkerDeploymentVersion) - (*in).DeepCopyInto(*out) - } + in.TargetVersion.DeepCopyInto(&out.TargetVersion) if in.CurrentVersion != nil { in, out := &in.CurrentVersion, &out.CurrentVersion *out = new(CurrentWorkerDeploymentVersion) diff --git a/internal/controller/genstatus.go b/internal/controller/genstatus.go index 55200524..0473ccc3 100644 --- a/internal/controller/genstatus.go +++ b/internal/controller/genstatus.go @@ -66,10 +66,5 @@ func (r *TemporalWorkerDeploymentReconciler) generateStatus( stateMapper := newStateMapper(k8sState, temporalState) status := stateMapper.mapToStatus(targetVersionID) - // Validate that TargetVersion is never nil (defensive programming) - if status.TargetVersion == nil { - return nil, fmt.Errorf("generated status has nil TargetVersion for target version %s, this should never happen", targetVersionID) - } - return status, nil } diff --git a/internal/controller/state_mapper.go b/internal/controller/state_mapper.go index 873b5e30..980de67a 100644 --- a/internal/controller/state_mapper.go +++ b/internal/controller/state_mapper.go @@ -98,8 +98,8 @@ func (m *stateMapper) mapCurrentWorkerDeploymentVersion(versionID string) *v1alp } // mapTargetWorkerDeploymentVersion creates a target version status from the states -func (m *stateMapper) mapTargetWorkerDeploymentVersion(versionID string) *v1alpha1.TargetWorkerDeploymentVersion { - version := &v1alpha1.TargetWorkerDeploymentVersion{ +func (m *stateMapper) mapTargetWorkerDeploymentVersion(versionID string) v1alpha1.TargetWorkerDeploymentVersion { + version := v1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: v1alpha1.BaseWorkerDeploymentVersion{ VersionID: versionID, Status: v1alpha1.VersionStatusNotRegistered, diff --git a/internal/controller/state_mapper_test.go b/internal/controller/state_mapper_test.go index 5ea1a006..bab00761 100644 --- a/internal/controller/state_mapper_test.go +++ b/internal/controller/state_mapper_test.go @@ -235,7 +235,6 @@ func TestMapWorkerDeploymentVersion(t *testing.T) { // Test target version mapping targetVersion := mapper.mapTargetWorkerDeploymentVersion("worker.v1") - assert.NotNil(t, targetVersion) assert.Equal(t, "worker.v1", targetVersion.VersionID) assert.Equal(t, temporaliov1alpha1.VersionStatusCurrent, targetVersion.Status) assert.NotNil(t, targetVersion.HealthySince) diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index 20350f0a..f6debed7 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -41,7 +41,7 @@ func TestGeneratePlan(t *testing.T) { name: "empty state creates new deployment", k8sState: &k8s.DeploymentState{}, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusNotRegistered, @@ -70,7 +70,7 @@ func TestGeneratePlan(t *testing.T) { }, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusCurrent, @@ -117,7 +117,7 @@ func TestGeneratePlan(t *testing.T) { }, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusCurrent, @@ -168,7 +168,7 @@ func TestGeneratePlan(t *testing.T) { }, }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusCurrent, @@ -311,7 +311,7 @@ func TestGetDeleteDeployments(t *testing.T) { }, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusCurrent, @@ -360,7 +360,7 @@ func TestGetScaleDeployments(t *testing.T) { }, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusCurrent, @@ -389,7 +389,7 @@ func TestGetScaleDeployments(t *testing.T) { }, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusCurrent, @@ -425,7 +425,7 @@ func TestGetScaleDeployments(t *testing.T) { }, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusInactive, @@ -458,7 +458,7 @@ func TestGetScaleDeployments(t *testing.T) { }, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.b", Status: temporaliov1alpha1.VersionStatusRamping, @@ -485,7 +485,7 @@ func TestGetScaleDeployments(t *testing.T) { }, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.a", Status: temporaliov1alpha1.VersionStatusInactive, @@ -518,7 +518,7 @@ func TestGetScaleDeployments(t *testing.T) { }, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.a", Status: temporaliov1alpha1.VersionStatusInactive, @@ -567,7 +567,7 @@ func TestShouldCreateDeployment(t *testing.T) { { name: "existing deployment should not create", status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusInactive, @@ -580,7 +580,7 @@ func TestShouldCreateDeployment(t *testing.T) { { name: "target version without deployment should create", status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.b", Status: temporaliov1alpha1.VersionStatusInactive, @@ -610,7 +610,7 @@ func TestGetTestWorkflows(t *testing.T) { { name: "gate workflow needed", status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusInactive, @@ -645,7 +645,7 @@ func TestGetTestWorkflows(t *testing.T) { { name: "no gate workflow", status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusInactive, @@ -664,7 +664,7 @@ func TestGetTestWorkflows(t *testing.T) { { name: "gate workflow with empty task queues", status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusInactive, @@ -684,7 +684,7 @@ func TestGetTestWorkflows(t *testing.T) { { name: "all test workflows already running", status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusInactive, @@ -741,7 +741,7 @@ func TestGetVersionConfigDiff(t *testing.T) { Strategy: temporaliov1alpha1.UpdateAllAtOnce, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusInactive, @@ -773,7 +773,7 @@ func TestGetVersionConfigDiff(t *testing.T) { }, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusInactive, @@ -804,7 +804,7 @@ func TestGetVersionConfigDiff(t *testing.T) { Strategy: temporaliov1alpha1.UpdateAllAtOnce, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.123", Status: temporaliov1alpha1.VersionStatusCurrent, @@ -842,7 +842,7 @@ func TestGetVersionConfigDiff(t *testing.T) { Strategy: temporaliov1alpha1.UpdateAllAtOnce, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.789", Status: temporaliov1alpha1.VersionStatusInactive, @@ -916,7 +916,7 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusRamping, @@ -950,7 +950,7 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusInactive, @@ -979,7 +979,7 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusRamping, @@ -1014,7 +1014,7 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusRamping, @@ -1047,7 +1047,7 @@ func TestGetVersionConfig_ProgressiveRolloutEdgeCases(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusRamping, @@ -1229,7 +1229,7 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusInactive, @@ -1263,7 +1263,7 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusInactive, @@ -1297,7 +1297,7 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusInactive, @@ -1331,7 +1331,7 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusInactive, @@ -1367,7 +1367,7 @@ func TestGetVersionConfig_GateWorkflowValidation(t *testing.T) { Status: temporaliov1alpha1.VersionStatusCurrent, }, }, - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.456", Status: temporaliov1alpha1.VersionStatusInactive, @@ -1461,7 +1461,7 @@ func TestComplexVersionStateScenarios(t *testing.T) { RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.a", Status: temporaliov1alpha1.VersionStatusInactive, @@ -1527,7 +1527,7 @@ func TestComplexVersionStateScenarios(t *testing.T) { RolloutStrategy: temporaliov1alpha1.RolloutStrategy{}, }, status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: &temporaliov1alpha1.TargetWorkerDeploymentVersion{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ VersionID: "test/namespace.a", Status: temporaliov1alpha1.VersionStatusInactive,