diff --git a/internal/k8s/deployments.go b/internal/k8s/deployments.go index 0232ad39..53db385c 100644 --- a/internal/k8s/deployments.go +++ b/internal/k8s/deployments.go @@ -26,12 +26,11 @@ import ( const ( DeployOwnerKey = ".metadata.controller" // BuildIDLabel is the label that identifies the build ID for a deployment - BuildIDLabel = "temporal.io/build-id" - DeploymentNameSeparator = "/" // TODO(carlydf): change this to "." once the server accepts `.` in deployment names - VersionIDSeparator = "." // TODO(carlydf): change this to ":" - K8sResourceNameSeparator = "-" - MaxBuildIdLen = 63 - ConnectionSpecHashAnnotation = "temporal.io/connection-spec-hash" + BuildIDLabel = "temporal.io/build-id" + WorkerDeploymentNameSeparator = "/" + K8sResourceNameSeparator = "-" + MaxBuildIdLen = 63 + ConnectionSpecHashAnnotation = "temporal.io/connection-spec-hash" ) // DeploymentState represents the Kubernetes state of all deployments for a temporal worker deployment @@ -111,11 +110,6 @@ func NewObjectRef(obj client.Object) *corev1.ObjectReference { } } -// ComputeVersionID generates a version ID from the worker deployment spec -func ComputeVersionID(w *temporaliov1alpha1.TemporalWorkerDeployment) string { - return ComputeWorkerDeploymentName(w) + VersionIDSeparator + ComputeBuildID(w) -} - func ComputeBuildID(w *temporaliov1alpha1.TemporalWorkerDeployment) string { if containers := w.Spec.Template.Spec.Containers; len(containers) > 0 { if img := containers[0].Image; img != "" { @@ -131,7 +125,7 @@ func ComputeBuildID(w *temporaliov1alpha1.TemporalWorkerDeployment) string { // ComputeWorkerDeploymentName generates the base worker deployment name func ComputeWorkerDeploymentName(w *temporaliov1alpha1.TemporalWorkerDeployment) string { // Use the name and namespace to form the worker deployment name - return w.GetName() + DeploymentNameSeparator + w.GetNamespace() + return w.GetNamespace() + WorkerDeploymentNameSeparator + w.GetName() } // ComputeVersionedDeploymentName generates a name for a versioned deployment @@ -162,8 +156,8 @@ func CleanAndTruncateString(s string, n int) string { if len(s) > n && n > 0 { s = s[:n] } - // Keep only letters, numbers, and dashes - re := regexp.MustCompile(`[^a-zA-Z0-9-]+`) + // Keep only letters, numbers, dashes, and dots + re := regexp.MustCompile(`[^a-zA-Z0-9-.]+`) return re.ReplaceAllString(s, K8sResourceNameSeparator) } diff --git a/internal/k8s/deployments_test.go b/internal/k8s/deployments_test.go index 5dba54b2..f2314cb9 100644 --- a/internal/k8s/deployments_test.go +++ b/internal/k8s/deployments_test.go @@ -235,7 +235,7 @@ func TestGenerateBuildID(t *testing.T) { twd2 := testhelpers.MakeTWD("", "", 1, pod2, nil, nil, nil) return twd1, twd2 }, - expectedPrefix: "my-test-image", + expectedPrefix: "my.test-image", expectedHashLen: 4, expectEquality: false, // should be different }, @@ -248,7 +248,7 @@ func TestGenerateBuildID(t *testing.T) { twd2 := testhelpers.MakeTWD("", "", 2, pod, nil, nil, nil) return twd1, twd2 }, - expectedPrefix: "my-test-image", + expectedPrefix: "my.test-image", expectedHashLen: 4, expectEquality: true, // should be the same }, @@ -334,7 +334,7 @@ func TestGenerateBuildID(t *testing.T) { twd := testhelpers.MakeTWDWithImage("", "", illegalCharsImg) return twd, nil // only check 1 result, no need to compare }, - expectedPrefix: "this-is-my-weird-image", + expectedPrefix: "this.is.my-weird-image", expectedHashLen: 4, expectEquality: false, }, @@ -450,15 +450,9 @@ func TestComputeWorkerDeploymentName_Integration_WithVersionedName(t *testing.T) versionedName := k8s.ComputeVersionedDeploymentName(workerDeploymentName, buildID) // Verify the expected formats - assert.Equal(t, "hello-world"+k8s.DeploymentNameSeparator+"demo", workerDeploymentName) - assert.True(t, strings.HasPrefix(versionedName, "hello-world"+k8s.DeploymentNameSeparator+"demo-")) - assert.True(t, strings.Contains(versionedName, "v1-0-0"), "versioned name should contain cleaned image tag") - - // Verify the version ID combines worker deployment name and build ID - versionID := k8s.ComputeVersionID(twd) - expectedVersionID := workerDeploymentName + k8s.VersionIDSeparator + buildID - assert.Equal(t, expectedVersionID, versionID) - assert.Equal(t, "hello-world"+k8s.DeploymentNameSeparator+"demo"+k8s.VersionIDSeparator+"v1-0-0-dd84", versionID) + assert.Equal(t, "demo"+k8s.WorkerDeploymentNameSeparator+"hello-world", workerDeploymentName) + assert.True(t, strings.HasPrefix(versionedName, "demo"+k8s.WorkerDeploymentNameSeparator+"hello-world-")) + assert.True(t, strings.Contains(versionedName, "v1.0.0"), "versioned name should contain cleaned image tag") } // TestNewDeploymentWithPodAnnotations tests that every new pod created has a connection spec hash annotation diff --git a/internal/tests/internal/integration_test.go b/internal/tests/internal/integration_test.go index 925d20cc..c1a4c236 100644 --- a/internal/tests/internal/integration_test.go +++ b/internal/tests/internal/integration_test.go @@ -47,53 +47,53 @@ func TestIntegration(t *testing.T) { WithInput( testhelpers.NewTemporalWorkerDeploymentBuilder(). WithManualStrategy(). - WithTargetTemplate("v1"), + WithTargetTemplate("v1.0"), ). WithWaitTime(5 * time.Second). // wait before checking to confirm no change WithExpectedStatus( testhelpers.NewStatusBuilder(). - WithTargetVersion("v1", temporaliov1alpha1.VersionStatusInactive, -1, true, false), + WithTargetVersion("v1.0", temporaliov1alpha1.VersionStatusInactive, -1, true, false), ), "all-at-once-rollout-2-replicas": testhelpers.NewTestCase(). WithInput( testhelpers.NewTemporalWorkerDeploymentBuilder(). WithAllAtOnceStrategy(). WithReplicas(2). - WithTargetTemplate("v1"), + WithTargetTemplate("v1.0"), ). WithExpectedStatus( testhelpers.NewStatusBuilder(). - WithTargetVersion("v1", temporaliov1alpha1.VersionStatusCurrent, -1, true, false). - WithCurrentVersion("v1", true, false), + WithTargetVersion("v1.0", temporaliov1alpha1.VersionStatusCurrent, -1, true, false). + WithCurrentVersion("v1.0", true, false), ), "progressive-rollout-no-unversioned-pollers-expect-all-at-once": testhelpers.NewTestCase(). WithInput( testhelpers.NewTemporalWorkerDeploymentBuilder(). WithProgressiveStrategy(testhelpers.ProgressiveStep(5, time.Hour)). - WithTargetTemplate("v1"), + WithTargetTemplate("v1.0"), ). WithExpectedStatus( testhelpers.NewStatusBuilder(). - WithTargetVersion("v1", temporaliov1alpha1.VersionStatusCurrent, -1, true, false). - WithCurrentVersion("v1", true, false), + WithTargetVersion("v1.0", temporaliov1alpha1.VersionStatusCurrent, -1, true, false). + WithCurrentVersion("v1.0", true, false), ), //// TODO(carlydf): this won't work until the controller detects unversioned pollers // "progressive-rollout-yes-unversioned-pollers-expect-first-step": testhelpers.NewTestCase(). // WithInput( // testhelpers.NewTemporalWorkerDeploymentBuilder(). // WithProgressiveStrategy(testhelpers.ProgressiveStep(5, time.Hour)). - // WithTargetTemplate("v1"), + // WithTargetTemplate("v1.0"), // ). // WithSetupFunction(setupUnversionedPoller). // WithExpectedStatus( // testhelpers.NewStatusBuilder(). - // WithTargetVersion("v1", temporaliov1alpha1.VersionStatusRamping, 5, true, false), + // WithTargetVersion("v1.0", temporaliov1alpha1.VersionStatusRamping, 5, true, false), // ), "nth-progressive-rollout-expect-first-step": testhelpers.NewTestCase(). WithInput( testhelpers.NewTemporalWorkerDeploymentBuilder(). WithProgressiveStrategy(testhelpers.ProgressiveStep(5, time.Hour)). - WithTargetTemplate("v1"). + WithTargetTemplate("v1.0"). WithStatus( testhelpers.NewStatusBuilder(). WithTargetVersion("v0", temporaliov1alpha1.VersionStatusCurrent, -1, true, true). @@ -105,14 +105,14 @@ func TestIntegration(t *testing.T) { ). WithExpectedStatus( testhelpers.NewStatusBuilder(). - WithTargetVersion("v1", temporaliov1alpha1.VersionStatusRamping, 5, true, false), + WithTargetVersion("v1.0", temporaliov1alpha1.VersionStatusRamping, 5, true, false), ), "nth-progressive-rollout-with-success-gate": testhelpers.NewTestCase(). WithInput( testhelpers.NewTemporalWorkerDeploymentBuilder(). WithProgressiveStrategy(testhelpers.ProgressiveStep(5, time.Hour)). WithGate(true). - WithTargetTemplate("v1"). + WithTargetTemplate("v1.0"). WithStatus( testhelpers.NewStatusBuilder(). WithTargetVersion("v0", temporaliov1alpha1.VersionStatusCurrent, -1, true, true). @@ -124,14 +124,14 @@ func TestIntegration(t *testing.T) { ). WithExpectedStatus( testhelpers.NewStatusBuilder(). - WithTargetVersion("v1", temporaliov1alpha1.VersionStatusRamping, 5, true, false), + WithTargetVersion("v1.0", temporaliov1alpha1.VersionStatusRamping, 5, true, false), ), "nth-progressive-rollout-with-failed-gate": testhelpers.NewTestCase(). WithInput( testhelpers.NewTemporalWorkerDeploymentBuilder(). WithProgressiveStrategy(testhelpers.ProgressiveStep(5, time.Hour)). WithGate(false). - WithTargetTemplate("v1"). + WithTargetTemplate("v1.0"). WithStatus( testhelpers.NewStatusBuilder(). WithTargetVersion("v0", temporaliov1alpha1.VersionStatusCurrent, -1, true, true). @@ -144,7 +144,7 @@ func TestIntegration(t *testing.T) { WithWaitTime(5 * time.Second). WithExpectedStatus( testhelpers.NewStatusBuilder(). - WithTargetVersion("v1", temporaliov1alpha1.VersionStatusInactive, -1, true, false). + WithTargetVersion("v1.0", temporaliov1alpha1.VersionStatusInactive, -1, true, false). WithCurrentVersion("v0", true, true), ), "failed-gate-is-not-scaled-down-while-target": testhelpers.NewTestCase(). @@ -152,21 +152,21 @@ func TestIntegration(t *testing.T) { testhelpers.NewTemporalWorkerDeploymentBuilder(). WithAllAtOnceStrategy(). WithGate(false). - WithTargetTemplate("v1"), + WithTargetTemplate("v1.0"), ). WithWaitTime(5 * time.Second). WithExpectedStatus( testhelpers.NewStatusBuilder(). - WithTargetVersion("v1", temporaliov1alpha1.VersionStatusInactive, -1, true, false), + WithTargetVersion("v1.0", temporaliov1alpha1.VersionStatusInactive, -1, true, false), ), "failed-gate-is-scaled-down-when-deprecated": testhelpers.NewTestCase(). WithInput( testhelpers.NewTemporalWorkerDeploymentBuilder(). WithAllAtOnceStrategy(). - WithTargetTemplate("v2"). + WithTargetTemplate("v2.0"). WithStatus( testhelpers.NewStatusBuilder(). - WithTargetVersion("v1", temporaliov1alpha1.VersionStatusInactive, -1, true, true). + WithTargetVersion("v1.0", temporaliov1alpha1.VersionStatusInactive, -1, true, true). WithDeprecatedVersions( testhelpers.NewDeprecatedVersionInfo("v0", temporaliov1alpha1.VersionStatusDrained, true, true, true), ), @@ -174,22 +174,22 @@ func TestIntegration(t *testing.T) { ). WithExistingDeployments( testhelpers.NewDeploymentInfo("v0", 1), - testhelpers.NewDeploymentInfo("v1", 1), + testhelpers.NewDeploymentInfo("v1.0", 1), ). WithWaitTime(5*time.Second). WithExpectedStatus( testhelpers.NewStatusBuilder(). - WithTargetVersion("v2", temporaliov1alpha1.VersionStatusCurrent, -1, true, false). - WithCurrentVersion("v2", true, false). + WithTargetVersion("v2.0", temporaliov1alpha1.VersionStatusCurrent, -1, true, false). + WithCurrentVersion("v2.0", true, false). WithDeprecatedVersions( testhelpers.NewDeprecatedVersionInfo("v0", temporaliov1alpha1.VersionStatusDrained, true, false, true), - testhelpers.NewDeprecatedVersionInfo("v1", temporaliov1alpha1.VersionStatusInactive, true, false, true), + testhelpers.NewDeprecatedVersionInfo("v1.0", temporaliov1alpha1.VersionStatusInactive, true, false, true), ), ). WithExpectedDeployments( // note: right now this is only checked for deprecated versions, TODO(carlydf) add for non-deprecated too testhelpers.NewDeploymentInfo("v0", 1), - testhelpers.NewDeploymentInfo("v1", 0), - testhelpers.NewDeploymentInfo("v2", 1), + testhelpers.NewDeploymentInfo("v1.0", 0), + testhelpers.NewDeploymentInfo("v2.0", 1), ), } // TODO(carlydf): Add additional test case where multiple ramping steps are done