Shorten deployment names when over 47 characters

docs/limits.md

@@ -1,4 +1,16 @@
 # Limits
 
-- Max `TemporalWorkerDeployment.Name` length: 63 characters
-- Max `BuildID` length: 63 characters
+- 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: `<TemporalWorkerDeployment.Name>-<BuildID>`.
+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(<TemporalWorkerDeployment.Name>)-trunc(<BuildID>)-hash(<TemporalWorkerDeployment.Name>-<BuildID>)`

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 {

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))
 		})
 	}
 }

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)
 		}