diff --git a/docs/limits.md b/docs/limits.md index 2da5067c..5089128a 100644 --- a/docs/limits.md +++ b/docs/limits.md @@ -1,4 +1,16 @@ # Limits -- Max `TemporalWorkerDeployment.Name` length: 63 characters -- Max `BuildID` length: 63 characters \ No newline at end of file +- Max `TemporalWorkerDeployment.Name` length: 63 characters, since Kubernetes label values are limited to 63 characters +- Max `BuildID` length: 63 characters, since Kubernetes label values are limited to 63 characters + +### Note on naming of controller-generated resources: +The controller generates a Kubernetes Deployment resource for each Version in your Worker Deployment. + +Kubernetes operational best practice is to limit Deployment names to 47 characters to ensure that full Pod names are always less than 63 characters. + +The controller will first generate the Deployment name for each Version as follows: `-`. +If that name exceeds 47 characters, the controller will generate a unique less-than-47-character name for the Deployment. +You can find the names of all the Deployment resources owned by a certain `TemporalWorkerDeployment` in the `Status` field of the `TemporalWorkerDeployment` resource. +Note that the original untruncated deployment name and build ID can always be found in the `temporal.io/deployment-name` and `temporal.io/build-id` labels. + +For transparency, know that the controller will generate these unique short names as follows, but don't depend on this name format because it may change: `trunc()-trunc()-hash(-)` \ No newline at end of file diff --git a/internal/demo/scripts/loadtest/loadtest b/internal/demo/scripts/loadtest/loadtest new file mode 100755 index 00000000..9d211aa3 Binary files /dev/null and b/internal/demo/scripts/loadtest/loadtest differ diff --git a/internal/k8s/deployments.go b/internal/k8s/deployments.go index f6e64b8c..3a9ef045 100644 --- a/internal/k8s/deployments.go +++ b/internal/k8s/deployments.go @@ -33,6 +33,7 @@ const ( WorkerDeploymentNameSeparator = "/" ResourceNameSeparator = "-" MaxBuildIDLen = 63 + MaxDeploymentNameLen = 47 ConnectionSpecHashAnnotation = "temporal.io/connection-spec-hash" PodTemplateSpecHashAnnotation = "temporal.io/pod-template-spec-hash" ) @@ -142,8 +143,20 @@ func ComputeWorkerDeploymentName(w *temporaliov1alpha1.TemporalWorkerDeployment) } // ComputeVersionedDeploymentName generates a name for a versioned deployment +// Name will be <=47 characters and unique for that Worker Deployment Version within the namespace. func ComputeVersionedDeploymentName(baseName, buildID string) string { - return CleanStringForDNS(baseName + ResourceNameSeparator + buildID) + fullName := baseName + ResourceNameSeparator + buildID + if len(fullName) > MaxDeploymentNameLen { + hashName := HashString(fullName)[:10] + fullName = TruncateString(baseName, 10) + ResourceNameSeparator + TruncateString(buildID, 10) + ResourceNameSeparator + hashName + } + return CleanStringForDNS(fullName) +} + +func HashString(s string) string { + h := sha256.New() + _, _ = h.Write([]byte(s)) + return hex.EncodeToString(h.Sum(nil)) } func computeImagePrefix(s string, maxLen int) string { diff --git a/internal/k8s/deployments_test.go b/internal/k8s/deployments_test.go index d3fbf5a9..169053f3 100644 --- a/internal/k8s/deployments_test.go +++ b/internal/k8s/deployments_test.go @@ -12,6 +12,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "fmt" "math/big" "os" "path/filepath" @@ -478,27 +479,44 @@ func TestComputeVersionedDeploymentName(t *testing.T) { buildID: "image-v2.1.0-a1b2c3d4", expectedName: "worker-name-image-v2-1-0-a1b2c3d4", }, + { + name: "exceed max length", + baseName: "worker-name-0123456789-0123456789", + buildID: "image-v2.1.0-0123456789-0123456789", + expectedName: "worker-nam-image-v2-1-9c4f60cc97", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := k8s.ComputeVersionedDeploymentName(tt.baseName, tt.buildID) assert.Equal(t, tt.expectedName, result) + assert.LessOrEqual(t, len(result), k8s.MaxDeploymentNameLen) + assert.Equal(t, result, k8s.CleanStringForDNS(result)) - // Verify the format is always baseName-buildID - expected := tt.baseName + k8s.ResourceNameSeparator + k8s.CleanStringForDNS(tt.buildID) - assert.Equal(t, expected, result) - - // Verify it ends with the build ID - assert.True(t, strings.HasSuffix(result, k8s.ResourceNameSeparator+k8s.CleanStringForDNS(tt.buildID)), - "versioned deployment name should end with '-cleaned(buildID)'") - - // Verify it starts with the base name - assert.True(t, strings.HasPrefix(result, tt.baseName), - "versioned deployment name should start with baseName") + if name := tt.baseName + k8s.ResourceNameSeparator + tt.buildID; len(name) <= k8s.MaxDeploymentNameLen { + // Verify it ends with the build ID + assert.True(t, strings.HasSuffix(result, k8s.ResourceNameSeparator+k8s.CleanStringForDNS(tt.buildID)), + "versioned deployment name should end with '-cleaned(buildID)'") + + // Verify it starts with the base name + assert.True(t, strings.HasPrefix(result, tt.baseName), + "versioned deployment name should start with baseName") + } else { // we had to truncate the baseName + // Verify it contains the build ID + assert.True(t, strings.Contains(result, k8s.ResourceNameSeparator+k8s.CleanStringForDNS(tt.buildID[:10])), + "versioned deployment name should end with '-cleaned(buildID)'") + + // Verify it starts with the truncated base name + assert.True(t, strings.HasPrefix(result, k8s.CleanStringForDNS(tt.baseName[:10])), + "versioned deployment name should start with baseName") + + // Verify it ends with the hash suffix + assert.True(t, strings.HasSuffix(result, k8s.ResourceNameSeparator+k8s.HashString(name)[:10]), + fmt.Sprintf("versioned deployment name '%s' should end with hash suffix '%s'", + result, k8s.HashString(name))) + } - // Verify it is cleaned for DNS - assert.Equal(t, result, k8s.CleanStringForDNS(result)) }) } } diff --git a/internal/tests/internal/env_helpers.go b/internal/tests/internal/env_helpers.go index 9c6d399c..ff3d3deb 100644 --- a/internal/tests/internal/env_helpers.go +++ b/internal/tests/internal/env_helpers.go @@ -166,7 +166,9 @@ func setupTestEnvironment(t *testing.T) (*rest.Config, client.Client, manager.Ma // Start manager ctx, cancel := context.WithCancel(context.Background()) + managerStopped := make(chan struct{}) go func() { + defer close(managerStopped) if err := mgr.Start(ctx); err != nil { t.Errorf("failed to start manager: %v", err) } @@ -175,6 +177,7 @@ func setupTestEnvironment(t *testing.T) (*rest.Config, client.Client, manager.Ma // Return cleanup function cleanup := func() { cancel() + <-managerStopped if err := testEnv.Stop(); err != nil { t.Errorf("failed to stop test environment: %v", err) }