Skip to content

Commit 58df597

Browse files
carlydfclaude
andauthored
Shorten deployment names when over 47 characters (#204)
<!--- Note to EXTERNAL Contributors --> <!-- Thanks for opening a PR! If it is a significant code change, please **make sure there is an open issue** for this. We work best with you when we have accepted the idea first before you code. --> <!--- For ALL Contributors 👇 --> ## What was changed Shorten deployment names (while keeping them unique) if over 47 characters. ## Why? Deployment names in k8s should be <=47 characters so that Pod names can be short enough for DNS limits. ## Checklist <!--- add/delete as needed ---> 1. Closes <!-- add issue number here --> 2. How was this tested: New unit tests, and existing functional tests. 3. Any docs updates needed? <!--- update README if applicable or point out where to update docs.temporal.io --> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent f79003c commit 58df597

5 files changed

Lines changed: 62 additions & 16 deletions

File tree

docs/limits.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,16 @@
11
# Limits
22

3-
- Max `TemporalWorkerDeployment.Name` length: 63 characters
4-
- Max `BuildID` length: 63 characters
3+
- Max `TemporalWorkerDeployment.Name` length: 63 characters, since Kubernetes label values are limited to 63 characters
4+
- Max `BuildID` length: 63 characters, since Kubernetes label values are limited to 63 characters
5+
6+
### Note on naming of controller-generated resources:
7+
The controller generates a Kubernetes Deployment resource for each Version in your Worker Deployment.
8+
9+
Kubernetes operational best practice is to limit Deployment names to 47 characters to ensure that full Pod names are always less than 63 characters.
10+
11+
The controller will first generate the Deployment name for each Version as follows: `<TemporalWorkerDeployment.Name>-<BuildID>`.
12+
If that name exceeds 47 characters, the controller will generate a unique less-than-47-character name for the Deployment.
13+
You can find the names of all the Deployment resources owned by a certain `TemporalWorkerDeployment` in the `Status` field of the `TemporalWorkerDeployment` resource.
14+
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.
15+
16+
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>)`
26.2 MB
Binary file not shown.

internal/k8s/deployments.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ const (
3333
WorkerDeploymentNameSeparator = "/"
3434
ResourceNameSeparator = "-"
3535
MaxBuildIDLen = 63
36+
MaxDeploymentNameLen = 47
3637
ConnectionSpecHashAnnotation = "temporal.io/connection-spec-hash"
3738
PodTemplateSpecHashAnnotation = "temporal.io/pod-template-spec-hash"
3839
)
@@ -142,8 +143,20 @@ func ComputeWorkerDeploymentName(w *temporaliov1alpha1.TemporalWorkerDeployment)
142143
}
143144

144145
// ComputeVersionedDeploymentName generates a name for a versioned deployment
146+
// Name will be <=47 characters and unique for that Worker Deployment Version within the namespace.
145147
func ComputeVersionedDeploymentName(baseName, buildID string) string {
146-
return CleanStringForDNS(baseName + ResourceNameSeparator + buildID)
148+
fullName := baseName + ResourceNameSeparator + buildID
149+
if len(fullName) > MaxDeploymentNameLen {
150+
hashName := HashString(fullName)[:10]
151+
fullName = TruncateString(baseName, 10) + ResourceNameSeparator + TruncateString(buildID, 10) + ResourceNameSeparator + hashName
152+
}
153+
return CleanStringForDNS(fullName)
154+
}
155+
156+
func HashString(s string) string {
157+
h := sha256.New()
158+
_, _ = h.Write([]byte(s))
159+
return hex.EncodeToString(h.Sum(nil))
147160
}
148161

149162
func computeImagePrefix(s string, maxLen int) string {

internal/k8s/deployments_test.go

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"crypto/x509"
1313
"crypto/x509/pkix"
1414
"encoding/pem"
15+
"fmt"
1516
"math/big"
1617
"os"
1718
"path/filepath"
@@ -478,27 +479,44 @@ func TestComputeVersionedDeploymentName(t *testing.T) {
478479
buildID: "image-v2.1.0-a1b2c3d4",
479480
expectedName: "worker-name-image-v2-1-0-a1b2c3d4",
480481
},
482+
{
483+
name: "exceed max length",
484+
baseName: "worker-name-0123456789-0123456789",
485+
buildID: "image-v2.1.0-0123456789-0123456789",
486+
expectedName: "worker-nam-image-v2-1-9c4f60cc97",
487+
},
481488
}
482489

483490
for _, tt := range tests {
484491
t.Run(tt.name, func(t *testing.T) {
485492
result := k8s.ComputeVersionedDeploymentName(tt.baseName, tt.buildID)
486493
assert.Equal(t, tt.expectedName, result)
494+
assert.LessOrEqual(t, len(result), k8s.MaxDeploymentNameLen)
495+
assert.Equal(t, result, k8s.CleanStringForDNS(result))
487496

488-
// Verify the format is always baseName-buildID
489-
expected := tt.baseName + k8s.ResourceNameSeparator + k8s.CleanStringForDNS(tt.buildID)
490-
assert.Equal(t, expected, result)
491-
492-
// Verify it ends with the build ID
493-
assert.True(t, strings.HasSuffix(result, k8s.ResourceNameSeparator+k8s.CleanStringForDNS(tt.buildID)),
494-
"versioned deployment name should end with '-cleaned(buildID)'")
495-
496-
// Verify it starts with the base name
497-
assert.True(t, strings.HasPrefix(result, tt.baseName),
498-
"versioned deployment name should start with baseName")
497+
if name := tt.baseName + k8s.ResourceNameSeparator + tt.buildID; len(name) <= k8s.MaxDeploymentNameLen {
498+
// Verify it ends with the build ID
499+
assert.True(t, strings.HasSuffix(result, k8s.ResourceNameSeparator+k8s.CleanStringForDNS(tt.buildID)),
500+
"versioned deployment name should end with '-cleaned(buildID)'")
501+
502+
// Verify it starts with the base name
503+
assert.True(t, strings.HasPrefix(result, tt.baseName),
504+
"versioned deployment name should start with baseName")
505+
} else { // we had to truncate the baseName
506+
// Verify it contains the build ID
507+
assert.True(t, strings.Contains(result, k8s.ResourceNameSeparator+k8s.CleanStringForDNS(tt.buildID[:10])),
508+
"versioned deployment name should end with '-cleaned(buildID)'")
509+
510+
// Verify it starts with the truncated base name
511+
assert.True(t, strings.HasPrefix(result, k8s.CleanStringForDNS(tt.baseName[:10])),
512+
"versioned deployment name should start with baseName")
513+
514+
// Verify it ends with the hash suffix
515+
assert.True(t, strings.HasSuffix(result, k8s.ResourceNameSeparator+k8s.HashString(name)[:10]),
516+
fmt.Sprintf("versioned deployment name '%s' should end with hash suffix '%s'",
517+
result, k8s.HashString(name)))
518+
}
499519

500-
// Verify it is cleaned for DNS
501-
assert.Equal(t, result, k8s.CleanStringForDNS(result))
502520
})
503521
}
504522
}

internal/tests/internal/env_helpers.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,9 @@ func setupTestEnvironment(t *testing.T) (*rest.Config, client.Client, manager.Ma
166166

167167
// Start manager
168168
ctx, cancel := context.WithCancel(context.Background())
169+
managerStopped := make(chan struct{})
169170
go func() {
171+
defer close(managerStopped)
170172
if err := mgr.Start(ctx); err != nil {
171173
t.Errorf("failed to start manager: %v", err)
172174
}
@@ -175,6 +177,7 @@ func setupTestEnvironment(t *testing.T) (*rest.Config, client.Client, manager.Ma
175177
// Return cleanup function
176178
cleanup := func() {
177179
cancel()
180+
<-managerStopped
178181
if err := testEnv.Stop(); err != nil {
179182
t.Errorf("failed to stop test environment: %v", err)
180183
}

0 commit comments

Comments
 (0)