diff --git a/api/v1alpha1/worker_types.go b/api/v1alpha1/worker_types.go index c557b9f5..3de47d60 100644 --- a/api/v1alpha1/worker_types.go +++ b/api/v1alpha1/worker_types.go @@ -27,6 +27,24 @@ type WorkerOptions struct { // The Temporal namespace for the worker to connect to. // +kubebuilder:validation:MinLength=1 TemporalNamespace string `json:"temporalNamespace"` + // BuildID optionally overrides the auto-generated build ID for this worker deployment. + // When set, the controller uses this value instead of computing a build ID from the + // pod template hash. This enables rolling updates for non-workflow code changes + // (bug fixes, config changes) while preserving the same build ID. + // + // WARNING: Using a custom build ID requires careful management. If workflow code changes + // but BuildID stays the same, pinned workflows may execute on workers running incompatible + // code. Only use this when you have a reliable way to detect changes in your workflow + // definitions (e.g., hashing workflow source files in CI/CD). + // + // When the BuildID is stable but pod template spec changes, the controller triggers + // a rolling update instead of creating a new deployment version. The controller uses + // a hash of the user-provided pod template spec to detect ANY changes, including + // container images, env vars, commands, volumes, resources, and all other fields. + // +optional + // +kubebuilder:validation:MaxLength=63 + // +kubebuilder:validation:Pattern=`^[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?$` + BuildID string `json:"buildID,omitempty"` } // TemporalWorkerDeploymentSpec defines the desired state of TemporalWorkerDeployment diff --git a/helm/temporal-worker-controller/crds/temporal.io_temporalworkerdeployments.yaml b/helm/temporal-worker-controller/crds/temporal.io_temporalworkerdeployments.yaml index db9e987c..a48712bc 100644 --- a/helm/temporal-worker-controller/crds/temporal.io_temporalworkerdeployments.yaml +++ b/helm/temporal-worker-controller/crds/temporal.io_temporalworkerdeployments.yaml @@ -64,7 +64,6 @@ spec: gate: properties: input: - type: object x-kubernetes-preserve-unknown-fields: true inputFrom: properties: @@ -73,25 +72,27 @@ spec: key: type: string name: + default: "" type: string optional: type: boolean required: - key - - name type: object + x-kubernetes-map-type: atomic secretKeyRef: properties: key: type: string name: + default: "" type: string optional: type: boolean required: - key - - name type: object + x-kubernetes-map-type: atomic type: object workflowType: type: string @@ -3941,6 +3942,10 @@ spec: type: object workerOptions: properties: + buildID: + maxLength: 63 + pattern: ^[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?$ + type: string connectionRef: properties: name: diff --git a/internal/k8s/deployments.go b/internal/k8s/deployments.go index 9e5d069f..b12c3c5d 100644 --- a/internal/k8s/deployments.go +++ b/internal/k8s/deployments.go @@ -11,7 +11,9 @@ import ( "fmt" "regexp" "sort" + "strings" + "github.com/davecgh/go-spew/spew" "github.com/distribution/reference" temporaliov1alpha1 "github.com/temporalio/temporal-worker-controller/api/v1alpha1" "github.com/temporalio/temporal-worker-controller/internal/controller/k8s.io/utils" @@ -30,8 +32,9 @@ const ( twdNameLabel = "temporal.io/deployment-name" WorkerDeploymentNameSeparator = "/" ResourceNameSeparator = "-" - MaxBuildIdLen = 63 - ConnectionSpecHashAnnotation = "temporal.io/connection-spec-hash" + MaxBuildIdLen = 63 + ConnectionSpecHashAnnotation = "temporal.io/connection-spec-hash" + PodTemplateSpecHashAnnotation = "temporal.io/pod-template-spec-hash" ) // DeploymentState represents the Kubernetes state of all deployments for a temporal worker deployment @@ -112,6 +115,15 @@ func NewObjectRef(obj client.Object) *corev1.ObjectReference { } func ComputeBuildID(w *temporaliov1alpha1.TemporalWorkerDeployment) string { + // Check for user-provided build ID in spec.workerOptions.buildID + if override := w.Spec.WorkerOptions.BuildID; override != "" { + cleaned := cleanBuildID(override) + if cleaned != "" { + return TruncateString(cleaned, MaxBuildIdLen) + } + // Fall through to default hash-based generation if buildID is invalid after cleaning + } + if containers := w.Spec.Template.Spec.Containers; len(containers) > 0 { if img := containers[0].Image; img != "" { shortHashSuffix := ResourceNameSeparator + utils.ComputeHash(&w.Spec.Template, nil, true) @@ -177,9 +189,12 @@ func CleanStringForDNS(s string) string { // // Temporal build IDs only need to be ASCII. func cleanBuildID(s string) string { - // Keep only letters, numbers, dashes, and dots. + // Keep only letters, numbers, dashes, underscores, and dots. re := regexp.MustCompile(`[^a-zA-Z0-9-._]+`) - return re.ReplaceAllString(s, ResourceNameSeparator) + s = re.ReplaceAllString(s, ResourceNameSeparator) + // Trim leading/trailing separators to comply with K8s label requirements + // (must begin and end with alphanumeric character) + return strings.Trim(s, "-._") } // NewDeploymentWithOwnerRef creates a new deployment resource, including owner references @@ -281,6 +296,9 @@ func NewDeploymentWithOwnerRef( podAnnotations[k] = v } podAnnotations[ConnectionSpecHashAnnotation] = ComputeConnectionSpecHash(connection) + // Store hash of user-provided pod template spec BEFORE controller modifications + // This enables drift detection when build ID is stable + podAnnotations[PodTemplateSpecHashAnnotation] = ComputePodTemplateSpecHash(spec.Template) blockOwnerDeletion := true return &appsv1.Deployment{ @@ -338,6 +356,29 @@ func ComputeConnectionSpecHash(connection temporaliov1alpha1.TemporalConnectionS return hex.EncodeToString(hasher.Sum(nil)) } +// ComputePodTemplateSpecHash computes a SHA256 hash of the user-provided pod template spec. +// This hash is used to detect drift when the build ID is stable but the pod spec has changed. +// The hash captures ALL user-controllable fields in the pod template spec. +func ComputePodTemplateSpecHash(template corev1.PodTemplateSpec) string { + hasher := sha256.New() + + // Use spew to get a deterministic string representation of the entire struct. + // This captures ALL fields including env vars, commands, volumes, etc. + // The config MUST NOT be changed because that could change the result of a hash operation. + printer := &spew.ConfigState{ + Indent: " ", + SortKeys: true, + DisableMethods: true, + SpewKeys: true, + DisablePointerAddresses: true, + DisableCapacities: true, + } + + _, _ = hasher.Write([]byte(printer.Sprintf("%#v", template))) + + return hex.EncodeToString(hasher.Sum(nil)) +} + func NewDeploymentWithControllerRef( w *temporaliov1alpha1.TemporalWorkerDeployment, buildID string, diff --git a/internal/k8s/deployments_test.go b/internal/k8s/deployments_test.go index ae1bafd9..68bdcf49 100644 --- a/internal/k8s/deployments_test.go +++ b/internal/k8s/deployments_test.go @@ -350,6 +350,81 @@ func TestGenerateBuildID(t *testing.T) { expectedHashLen: 4, expectEquality: false, }, + { + name: "spec buildID override", + generateInputs: func() (*temporaliov1alpha1.TemporalWorkerDeployment, *temporaliov1alpha1.TemporalWorkerDeployment) { + twd := testhelpers.MakeTWDWithImage("", "", "some-image") + twd.Spec.WorkerOptions.BuildID = "manual-override-v1" + return twd, nil + }, + expectedPrefix: "manual-override-v1", + expectedHashLen: 2, // "v1" is length 2. + // The override returns cleanBuildID(buildIDValue). + // If buildID is "manual-override-v1", cleanBuildID returns "manual-override-v1". + // split by "-" gives ["manual", "override", "v1"]. last element is "v1", len is 2. + expectEquality: false, + }, + { + name: "spec buildID override stability", + generateInputs: func() (*temporaliov1alpha1.TemporalWorkerDeployment, *temporaliov1alpha1.TemporalWorkerDeployment) { + // Two TWDs with DIFFERENT images but SAME buildID + twd1 := testhelpers.MakeTWDWithImage("", "", "image-v1") + twd1.Spec.WorkerOptions.BuildID = "stable-id" + + twd2 := testhelpers.MakeTWDWithImage("", "", "image-v2") + twd2.Spec.WorkerOptions.BuildID = "stable-id" + return twd1, twd2 + }, + expectedPrefix: "stable-id", + expectedHashLen: 2, // "id" has len 2 + expectEquality: true, + }, + { + name: "spec buildID override with long value is truncated", + generateInputs: func() (*temporaliov1alpha1.TemporalWorkerDeployment, *temporaliov1alpha1.TemporalWorkerDeployment) { + // 72 char buildID - should be truncated to 63 + longBuildID := "this-is-a-very-long-build-id-value-that-exceeds-63-characters-limit" + twd := testhelpers.MakeTWDWithImage("", "", "some-image") + twd.Spec.WorkerOptions.BuildID = longBuildID + return twd, nil + }, + expectedPrefix: "this-is-a-very-long-build-id-value-that-exceeds-63-characters-l", + expectedHashLen: 1, // "l" has len 1 + expectEquality: false, + }, + { + name: "spec buildID override with empty value falls back to hash", + generateInputs: func() (*temporaliov1alpha1.TemporalWorkerDeployment, *temporaliov1alpha1.TemporalWorkerDeployment) { + twd := testhelpers.MakeTWDWithImage("", "", "fallback-image") + twd.Spec.WorkerOptions.BuildID = "" // empty buildID + return twd, nil + }, + expectedPrefix: "fallback-image", // Falls back to image-based build ID + expectedHashLen: 4, + expectEquality: false, + }, + { + name: "spec buildID override with only invalid chars falls back to hash", + generateInputs: func() (*temporaliov1alpha1.TemporalWorkerDeployment, *temporaliov1alpha1.TemporalWorkerDeployment) { + twd := testhelpers.MakeTWDWithImage("", "", "fallback-image2") + twd.Spec.WorkerOptions.BuildID = "###$$$%%%" // all invalid chars + return twd, nil + }, + expectedPrefix: "fallback-image2", // Falls back to image-based build ID + expectedHashLen: 4, + expectEquality: false, + }, + { + name: "spec buildID override trims leading and trailing separators", + generateInputs: func() (*temporaliov1alpha1.TemporalWorkerDeployment, *temporaliov1alpha1.TemporalWorkerDeployment) { + twd := testhelpers.MakeTWDWithImage("", "", "some-image") + twd.Spec.WorkerOptions.BuildID = "---my-build-id---" // leading/trailing dashes + return twd, nil + }, + expectedPrefix: "my-build-id", // dashes trimmed + expectedHashLen: 2, // "id" has len 2 + expectEquality: false, + }, } for _, tt := range tests { @@ -623,6 +698,108 @@ func TestComputeConnectionSpecHash(t *testing.T) { } +func TestComputePodTemplateSpecHash(t *testing.T) { + t.Run("generates non-empty hash for valid template", func(t *testing.T) { + template := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "worker", Image: "test:v1"}, + }, + }, + } + result := k8s.ComputePodTemplateSpecHash(template) + assert.NotEmpty(t, result) + assert.Len(t, result, 64) // SHA256 hex encoded + }) + + t.Run("is deterministic - same input produces same hash", func(t *testing.T) { + template := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "worker", Image: "test:v1"}, + }, + }, + } + hash1 := k8s.ComputePodTemplateSpecHash(template) + hash2 := k8s.ComputePodTemplateSpecHash(template) + assert.Equal(t, hash1, hash2) + }) + + t.Run("different images produce different hashes", func(t *testing.T) { + template1 := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "worker", Image: "test:v1"}}, + }, + } + template2 := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "worker", Image: "test:v2"}}, + }, + } + hash1 := k8s.ComputePodTemplateSpecHash(template1) + hash2 := k8s.ComputePodTemplateSpecHash(template2) + assert.NotEqual(t, hash1, hash2) + }) + + t.Run("different env vars produce different hashes", func(t *testing.T) { + template1 := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "worker", Image: "test:v1", Env: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}}, + }, + }, + } + template2 := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "worker", Image: "test:v1", Env: []corev1.EnvVar{{Name: "FOO", Value: "baz"}}}, + }, + }, + } + hash1 := k8s.ComputePodTemplateSpecHash(template1) + hash2 := k8s.ComputePodTemplateSpecHash(template2) + assert.NotEqual(t, hash1, hash2) + }) + + t.Run("different commands produce different hashes", func(t *testing.T) { + template1 := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "worker", Image: "test:v1", Command: []string{"./old-cmd"}}, + }, + }, + } + template2 := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "worker", Image: "test:v1", Command: []string{"./new-cmd"}}, + }, + }, + } + hash1 := k8s.ComputePodTemplateSpecHash(template1) + hash2 := k8s.ComputePodTemplateSpecHash(template2) + assert.NotEqual(t, hash1, hash2) + }) + + t.Run("different volumes produce different hashes", func(t *testing.T) { + template1 := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "worker", Image: "test:v1"}}, + Volumes: []corev1.Volume{{Name: "vol1"}}, + }, + } + template2 := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "worker", Image: "test:v1"}}, + Volumes: []corev1.Volume{{Name: "vol2"}}, + }, + } + hash1 := k8s.ComputePodTemplateSpecHash(template1) + hash2 := k8s.ComputePodTemplateSpecHash(template2) + assert.NotEqual(t, hash1, hash2) + }) +} + func TestNewDeploymentWithOwnerRef_EnvironmentVariablesAndVolumes(t *testing.T) { tests := map[string]struct { connection temporaliov1alpha1.TemporalConnectionSpec diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 35b0ebcb..9e44291b 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -92,7 +92,7 @@ func GeneratePlan( plan.DeleteDeployments = getDeleteDeployments(k8sState, status, spec, foundDeploymentInTemporal) plan.ScaleDeployments = getScaleDeployments(k8sState, status, spec) plan.ShouldCreateDeployment = shouldCreateDeployment(status, maxVersionsIneligibleForDeletion) - plan.UpdateDeployments = getUpdateDeployments(k8sState, status, connection) + plan.UpdateDeployments = getUpdateDeployments(k8sState, status, spec, connection) // Determine if we need to start any test workflows plan.TestWorkflows = getTestWorkflows(status, config, workerDeploymentName, gateInput, isGateInputSecret) @@ -168,31 +168,225 @@ func updateDeploymentWithConnection(deployment *appsv1.Deployment, connection te } } +// checkAndUpdateDeploymentPodTemplateSpec determines whether the Deployment for the given buildID is +// out-of-date with respect to the user-provided pod template spec. This enables rolling updates when +// the build ID is stable (e.g., using spec.workerOptions.buildID) but the pod spec has changed. +// If an update is required, it rebuilds the deployment spec and returns a pointer to that Deployment. +// If no update is needed or the Deployment does not exist, it returns nil. +func checkAndUpdateDeploymentPodTemplateSpec( + buildID string, + k8sState *k8s.DeploymentState, + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec, + connection temporaliov1alpha1.TemporalConnectionSpec, +) *appsv1.Deployment { + existingDeployment, exists := k8sState.Deployments[buildID] + if !exists { + return nil + } + + // Only check for drift when buildID is explicitly set by the user. + // If buildID is auto-generated, any spec change would generate a new buildID anyway. + if spec.WorkerOptions.BuildID == "" { + return nil + } + + // Get the stored hash from the existing deployment's pod template annotations + storedHash := "" + if existingDeployment.Spec.Template.Annotations != nil { + storedHash = existingDeployment.Spec.Template.Annotations[k8s.PodTemplateSpecHashAnnotation] + } + + // Backwards compatibility: if no hash annotation exists (legacy deployment), + // don't trigger an update - the hash will be added on the next spec change + if storedHash == "" { + return nil + } + + // Compute the hash of the current user-provided pod template spec + currentHash := k8s.ComputePodTemplateSpecHash(spec.Template) + + // If hashes match, no drift detected + if storedHash == currentHash { + return nil + } + + // Pod template has changed - rebuild the pod spec from the TWD spec + // This applies all controller modifications (env vars, TLS mounts, etc.) + updateDeploymentWithPodTemplateSpec(existingDeployment, spec, connection) + + return existingDeployment +} + +// updateDeploymentWithPodTemplateSpec updates an existing deployment with a new pod template spec +// from the TWD spec. This applies all the controller modifications that NewDeploymentWithOwnerRef does. +func updateDeploymentWithPodTemplateSpec( + deployment *appsv1.Deployment, + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec, + connection temporaliov1alpha1.TemporalConnectionSpec, +) { + // Deep copy the user-provided pod spec to avoid mutating the original + podSpec := spec.Template.Spec.DeepCopy() + + // Extract the build ID from the deployment's labels (with nil safety) + var buildID string + if deployment.Labels != nil { + buildID = deployment.Labels[k8s.BuildIDLabel] + } + + // Extract the worker deployment name from existing env vars + var workerDeploymentName string + for _, container := range deployment.Spec.Template.Spec.Containers { + for _, env := range container.Env { + if env.Name == "TEMPORAL_DEPLOYMENT_NAME" { + workerDeploymentName = env.Value + break + } + } + if workerDeploymentName != "" { + break + } + } + + // Add environment variables to containers (same as NewDeploymentWithOwnerRef) + for i, container := range podSpec.Containers { + container.Env = append(container.Env, + corev1.EnvVar{ + Name: "TEMPORAL_ADDRESS", + Value: connection.HostPort, + }, + corev1.EnvVar{ + Name: "TEMPORAL_NAMESPACE", + Value: spec.WorkerOptions.TemporalNamespace, + }, + corev1.EnvVar{ + Name: "TEMPORAL_DEPLOYMENT_NAME", + Value: workerDeploymentName, + }, + corev1.EnvVar{ + Name: "TEMPORAL_WORKER_BUILD_ID", + Value: buildID, + }, + ) + podSpec.Containers[i] = container + } + + // Add TLS config if mTLS is enabled + if connection.MutualTLSSecretRef != nil { + for i, container := range podSpec.Containers { + container.Env = append(container.Env, + corev1.EnvVar{ + Name: "TEMPORAL_TLS", + Value: "true", + }, + corev1.EnvVar{ + Name: "TEMPORAL_TLS_CLIENT_KEY_PATH", + Value: "/etc/temporal/tls/tls.key", + }, + corev1.EnvVar{ + Name: "TEMPORAL_TLS_CLIENT_CERT_PATH", + Value: "/etc/temporal/tls/tls.crt", + }, + ) + container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ + Name: "temporal-tls", + MountPath: "/etc/temporal/tls", + }) + podSpec.Containers[i] = container + } + podSpec.Volumes = append(podSpec.Volumes, corev1.Volume{ + Name: "temporal-tls", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: connection.MutualTLSSecretRef.Name, + }, + }, + }) + } else if connection.APIKeySecretRef != nil { + for i, container := range podSpec.Containers { + container.Env = append(container.Env, + corev1.EnvVar{ + Name: "TEMPORAL_API_KEY", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: connection.APIKeySecretRef, + }, + }, + ) + podSpec.Containers[i] = container + } + } + + // Build new pod annotations + podAnnotations := make(map[string]string) + for k, v := range spec.Template.Annotations { + podAnnotations[k] = v + } + podAnnotations[k8s.ConnectionSpecHashAnnotation] = k8s.ComputeConnectionSpecHash(connection) + // Store the new pod template spec hash + podAnnotations[k8s.PodTemplateSpecHashAnnotation] = k8s.ComputePodTemplateSpecHash(spec.Template) + + // Preserve existing pod labels and add/update required labels + podLabels := make(map[string]string) + for k, v := range spec.Template.Labels { + podLabels[k] = v + } + // Copy selector labels from existing deployment + for k, v := range deployment.Spec.Selector.MatchLabels { + podLabels[k] = v + } + + // Update the deployment's pod template + deployment.Spec.Template.ObjectMeta.Labels = podLabels + deployment.Spec.Template.ObjectMeta.Annotations = podAnnotations + deployment.Spec.Template.Spec = *podSpec + + // Update replicas if changed + deployment.Spec.Replicas = spec.Replicas + deployment.Spec.MinReadySeconds = spec.MinReadySeconds +} + func getUpdateDeployments( k8sState *k8s.DeploymentState, status *temporaliov1alpha1.TemporalWorkerDeploymentStatus, + spec *temporaliov1alpha1.TemporalWorkerDeploymentSpec, connection temporaliov1alpha1.TemporalConnectionSpec, ) []*appsv1.Deployment { var updateDeployments []*appsv1.Deployment + // Track which deployments we've already added to avoid duplicates + updatedBuildIDs := make(map[string]bool) - // Check target version deployment if it has an expired connection spec hash + // Check target version deployment for pod template spec drift + // This enables rolling updates when the build ID is stable but spec changed if status.TargetVersion.BuildID != "" { + if deployment := checkAndUpdateDeploymentPodTemplateSpec(status.TargetVersion.BuildID, k8sState, spec, connection); deployment != nil { + updateDeployments = append(updateDeployments, deployment) + updatedBuildIDs[status.TargetVersion.BuildID] = true + } + } + + // Check target version deployment if it has an expired connection spec hash + // (only if not already updated by pod template check) + if status.TargetVersion.BuildID != "" && !updatedBuildIDs[status.TargetVersion.BuildID] { if deployment := checkAndUpdateDeploymentConnectionSpec(status.TargetVersion.BuildID, k8sState, connection); deployment != nil { updateDeployments = append(updateDeployments, deployment) + updatedBuildIDs[status.TargetVersion.BuildID] = true } } // Check current version deployment if it has an expired connection spec hash - if status.CurrentVersion != nil && status.CurrentVersion.BuildID != "" { + if status.CurrentVersion != nil && status.CurrentVersion.BuildID != "" && !updatedBuildIDs[status.CurrentVersion.BuildID] { if deployment := checkAndUpdateDeploymentConnectionSpec(status.CurrentVersion.BuildID, k8sState, connection); deployment != nil { updateDeployments = append(updateDeployments, deployment) + updatedBuildIDs[status.CurrentVersion.BuildID] = true } } // Check deprecated versions for expired connection spec hashes for _, version := range status.DeprecatedVersions { - if deployment := checkAndUpdateDeploymentConnectionSpec(version.BuildID, k8sState, connection); deployment != nil { - updateDeployments = append(updateDeployments, deployment) + if !updatedBuildIDs[version.BuildID] { + if deployment := checkAndUpdateDeploymentConnectionSpec(version.BuildID, k8sState, connection); deployment != nil { + updateDeployments = append(updateDeployments, deployment) + updatedBuildIDs[version.BuildID] = true + } } } diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index cff5678a..df3b8321 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -2176,6 +2176,206 @@ func TestCheckAndUpdateDeploymentConnectionSpec(t *testing.T) { } } +func TestCheckAndUpdateDeploymentPodTemplateSpec(t *testing.T) { + tests := []struct { + name string + buildID string + existingDeployment *appsv1.Deployment + newSpec *temporaliov1alpha1.TemporalWorkerDeploymentSpec + connection temporaliov1alpha1.TemporalConnectionSpec + expectUpdate bool + expectImage string + }{ + { + name: "non-existing deployment does not result in an update", + buildID: "non-existent", + existingDeployment: nil, + newSpec: createWorkerSpecWithBuildID("stable-build-id"), + connection: createDefaultConnectionSpec(), + expectUpdate: false, + }, + { + name: "no update when buildID is not explicitly set (auto-generated buildID)", + buildID: "v1", + existingDeployment: createDeploymentForDriftTest(1, "v1", "old-image:v1"), + newSpec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: int32Ptr(1), + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "worker", Image: "new-image:v2"}, + }, + }, + }, + WorkerOptions: temporaliov1alpha1.WorkerOptions{ + TemporalNamespace: "test-namespace", + // BuildID not set - means auto-generated + }, + }, + connection: createDefaultConnectionSpec(), + expectUpdate: false, // No update because BuildID is not explicitly set + }, + { + name: "same image does not trigger update when buildID is set", + buildID: "stable-build-id", + existingDeployment: createDeploymentForDriftTest(1, "stable-build-id", "my-image:v1"), + newSpec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: int32Ptr(1), + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "worker", Image: "my-image:v1"}, + }, + }, + }, + WorkerOptions: temporaliov1alpha1.WorkerOptions{ + TemporalNamespace: "test-namespace", + BuildID: "stable-build-id", + }, + }, + connection: createDefaultConnectionSpec(), + expectUpdate: false, // No update because image is the same + }, + { + name: "different image triggers update when buildID is set", + buildID: "stable-build-id", + existingDeployment: createDeploymentForDriftTest(1, "stable-build-id", "old-image:v1"), + newSpec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: int32Ptr(1), + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "worker", Image: "new-image:v2"}, + }, + }, + }, + WorkerOptions: temporaliov1alpha1.WorkerOptions{ + TemporalNamespace: "test-namespace", + BuildID: "stable-build-id", + }, + }, + connection: createDefaultConnectionSpec(), + expectUpdate: true, + expectImage: "new-image:v2", + }, + { + name: "replicas-only change does not trigger update (handled by scaling logic)", + buildID: "stable-build-id", + existingDeployment: createDeploymentForDriftTest(1, "stable-build-id", "my-image:v1"), + newSpec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: int32Ptr(3), // Changed from 1 to 3 + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + // Same image as stored - hash will match + {Name: "worker", Image: "my-image:v1"}, + }, + }, + }, + WorkerOptions: temporaliov1alpha1.WorkerOptions{ + TemporalNamespace: "test-namespace", + BuildID: "stable-build-id", + }, + }, + connection: createDefaultConnectionSpec(), + // Replicas are not part of PodTemplateSpec, so hash won't change. + // Replicas changes are handled by getScaleDeployments() instead. + expectUpdate: false, + }, + { + name: "env var change triggers update when buildID is set", + buildID: "stable-build-id", + existingDeployment: createDeploymentForDriftTestWithEnv(1, "stable-build-id", "my-image:v1", + []corev1.EnvVar{{Name: "MY_VAR", Value: "old-value"}}), + newSpec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: int32Ptr(1), + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "worker", + Image: "my-image:v1", + Env: []corev1.EnvVar{{Name: "MY_VAR", Value: "new-value"}}, + }, + }, + }, + }, + WorkerOptions: temporaliov1alpha1.WorkerOptions{ + TemporalNamespace: "test-namespace", + BuildID: "stable-build-id", + }, + }, + connection: createDefaultConnectionSpec(), + expectUpdate: true, + expectImage: "my-image:v1", + }, + { + name: "backwards compat: no hash annotation means no update", + buildID: "stable-build-id", + existingDeployment: createDeploymentWithoutHashAnnotation(1, "stable-build-id", "old-image:v1"), + newSpec: &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: int32Ptr(1), + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "worker", Image: "new-image:v2"}, + }, + }, + }, + WorkerOptions: temporaliov1alpha1.WorkerOptions{ + TemporalNamespace: "test-namespace", + BuildID: "stable-build-id", + }, + }, + connection: createDefaultConnectionSpec(), + // Legacy deployment without hash annotation should not trigger update + // to maintain backwards compatibility + expectUpdate: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k8sState := &k8s.DeploymentState{ + Deployments: map[string]*appsv1.Deployment{}, + } + + buildID := tt.buildID + if tt.existingDeployment != nil { + k8sState.Deployments[buildID] = tt.existingDeployment + } + + result := checkAndUpdateDeploymentPodTemplateSpec(buildID, k8sState, tt.newSpec, tt.connection) + + if !tt.expectUpdate { + assert.Nil(t, result, "Expected no update, but got deployment") + return + } + + require.NotNil(t, result, "Expected deployment update, but got nil") + + // Check container image was updated + if tt.expectImage != "" { + require.Len(t, result.Spec.Template.Spec.Containers, 1, "Should have one container") + assert.Equal(t, tt.expectImage, result.Spec.Template.Spec.Containers[0].Image, "Container image should be updated") + } + + // Check that controller-injected env vars are present + found := false + for _, container := range result.Spec.Template.Spec.Containers { + for _, env := range container.Env { + if env.Name == "TEMPORAL_WORKER_BUILD_ID" { + assert.Equal(t, buildID, env.Value, "TEMPORAL_WORKER_BUILD_ID should be set") + found = true + break + } + } + } + assert.True(t, found, "Should find TEMPORAL_WORKER_BUILD_ID environment variable") + }) + } +} + // Helper function to create a deployment with the specified replicas and the default connection spec hash func createDeploymentWithDefaultConnectionSpecHash(replicas int32) *appsv1.Deployment { return &appsv1.Deployment{ @@ -2214,6 +2414,149 @@ func createDeploymentWithExpiredConnectionSpecHash(replicas int32) *appsv1.Deplo } } +// Helper function to create a deployment for drift testing +func createDeploymentForDriftTest(replicas int32, buildID string, image string) *appsv1.Deployment { + r := replicas + // Create the user-provided pod template spec (without controller modifications) + userTemplate := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "worker", + Image: image, + }, + }, + }, + } + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment", + Labels: map[string]string{ + k8s.BuildIDLabel: buildID, + }, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: &r, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + k8s.BuildIDLabel: buildID, + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + k8s.ConnectionSpecHashAnnotation: k8s.ComputeConnectionSpecHash(createDefaultConnectionSpec()), + k8s.PodTemplateSpecHashAnnotation: k8s.ComputePodTemplateSpecHash(userTemplate), + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "worker", + Image: image, + Env: []corev1.EnvVar{ + {Name: "TEMPORAL_DEPLOYMENT_NAME", Value: "test/my-worker"}, + }, + }, + }, + }, + }, + }, + } +} + +// createDeploymentForDriftTestWithEnv creates a deployment for drift testing with custom env vars +func createDeploymentForDriftTestWithEnv(replicas int32, buildID string, image string, envVars []corev1.EnvVar) *appsv1.Deployment { + r := replicas + // Create the user-provided pod template spec (without controller modifications) + userTemplate := corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "worker", + Image: image, + Env: envVars, + }, + }, + }, + } + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment", + Labels: map[string]string{ + k8s.BuildIDLabel: buildID, + }, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: &r, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + k8s.BuildIDLabel: buildID, + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + k8s.ConnectionSpecHashAnnotation: k8s.ComputeConnectionSpecHash(createDefaultConnectionSpec()), + k8s.PodTemplateSpecHashAnnotation: k8s.ComputePodTemplateSpecHash(userTemplate), + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "worker", + Image: image, + Env: append(envVars, corev1.EnvVar{ + Name: "TEMPORAL_DEPLOYMENT_NAME", Value: "test/my-worker", + }), + }, + }, + }, + }, + }, + } +} + +// createDeploymentWithoutHashAnnotation creates a deployment without the pod template spec hash (for backwards compat testing) +func createDeploymentWithoutHashAnnotation(replicas int32, buildID string, image string) *appsv1.Deployment { + r := replicas + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment", + Labels: map[string]string{ + k8s.BuildIDLabel: buildID, + }, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: &r, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + k8s.BuildIDLabel: buildID, + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + // Only connection spec hash, no pod template spec hash + k8s.ConnectionSpecHashAnnotation: k8s.ComputeConnectionSpecHash(createDefaultConnectionSpec()), + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "worker", + Image: image, + Env: []corev1.EnvVar{ + {Name: "TEMPORAL_DEPLOYMENT_NAME", Value: "test/my-worker"}, + }, + }, + }, + }, + }, + }, + } +} + func int32Ptr(v int32) *int32 { return &v } @@ -2276,6 +2619,27 @@ func createDefaultWorkerSpec() *temporaliov1alpha1.TemporalWorkerDeploymentSpec } } +// createWorkerSpecWithBuildID creates a worker spec with an explicit buildID set +func createWorkerSpecWithBuildID(buildID string) *temporaliov1alpha1.TemporalWorkerDeploymentSpec { + return &temporaliov1alpha1.TemporalWorkerDeploymentSpec{ + Replicas: int32Ptr(1), + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "worker", + Image: "test-image:latest", + }, + }, + }, + }, + WorkerOptions: temporaliov1alpha1.WorkerOptions{ + TemporalNamespace: "test-namespace", + BuildID: buildID, + }, + } +} + // createTestDeploymentWithConnection creates a test deployment with the specified connection spec func createTestDeploymentWithConnection(deploymentName, buildID string, connection temporaliov1alpha1.TemporalConnectionSpec) *appsv1.Deployment { return k8s.NewDeploymentWithOwnerRef(