diff --git a/pkg/controller/deployment/sync.go b/pkg/controller/deployment/sync.go index 0e06d3764d3..9f631dd02a7 100644 --- a/pkg/controller/deployment/sync.go +++ b/pkg/controller/deployment/sync.go @@ -27,6 +27,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/controller" @@ -35,6 +36,11 @@ import ( labelsutil "k8s.io/kubernetes/pkg/util/labels" ) +const ( + // replicaSetNameSeparator is the character used to separate deployment name from hash + replicaSetNameSeparator = "-" +) + // syncStatusOnly only updates Deployments Status and doesn't take any mutating actions. func (dc *DeploymentController) syncStatusOnly(ctx context.Context, d *apps.Deployment, rsList []*apps.ReplicaSet) error { newRS, oldRSs, err := dc.getAllReplicaSetsAndSyncRevision(ctx, d, rsList, false) @@ -198,7 +204,7 @@ func (dc *DeploymentController) getNewReplicaSet(ctx context.Context, d *apps.De newRS := apps.ReplicaSet{ ObjectMeta: metav1.ObjectMeta{ // Make the name deterministic, to ensure idempotence - Name: d.Name + "-" + podTemplateSpecHash, + Name: generateReplicaSetName(d.Name, podTemplateSpecHash), Namespace: d.Namespace, OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(d, controllerKind)}, Labels: newRSTemplate.Labels, @@ -549,3 +555,15 @@ func (dc *DeploymentController) isScalingEvent(ctx context.Context, d *apps.Depl } return false, nil } + +// generateReplicaSetName generates a ReplicaSet name by adding a pod template spec hash +// to a Deployment name, optionally truncating it to ensure the ReplicaSet name is within the limit. +func generateReplicaSetName(deploymentName, podTemplateSpecHash string) string { + maxDeploymentNameLength := validation.DNS1123SubdomainMaxLength - len(replicaSetNameSeparator) - len(podTemplateSpecHash) + + if len(deploymentName) > maxDeploymentNameLength && maxDeploymentNameLength > 0 { + return deploymentName[:maxDeploymentNameLength] + replicaSetNameSeparator + podTemplateSpecHash + } + + return deploymentName + replicaSetNameSeparator + podTemplateSpecHash +} diff --git a/pkg/controller/deployment/sync_test.go b/pkg/controller/deployment/sync_test.go index cda930933e1..76462d8038e 100644 --- a/pkg/controller/deployment/sync_test.go +++ b/pkg/controller/deployment/sync_test.go @@ -18,6 +18,7 @@ package deployment import ( "math" + "strings" "testing" "time" @@ -25,6 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" testclient "k8s.io/client-go/testing" @@ -592,3 +594,134 @@ func TestDeploymentController_cleanupDeploymentOrder(t *testing.T) { } } } + +func TestDeploymentController_generateReplicaSetName(t *testing.T) { + tests := []struct { + name string + deploymentName string + wantDeploymentPortion string + }{ + { + name: "short name", + deploymentName: "my-deployment", + wantDeploymentPortion: "my-deployment", + }, + { + name: "very long name truncated", + deploymentName: strings.Repeat("a", 250), + wantDeploymentPortion: strings.Repeat("a", 242), + }, + { + name: "very long name not truncated", + deploymentName: strings.Repeat("a", 242), + wantDeploymentPortion: strings.Repeat("a", 242), + }, + } + + for _, test := range tests { + _, ctx := ktesting.NewTestContext(t) + + fake := &fake.Clientset{} + informers := informers.NewSharedInformerFactory(fake, controller.NoResyncPeriodFunc()) + controller, err := NewDeploymentController(ctx, informers.Apps().V1().Deployments(), informers.Apps().V1().ReplicaSets(), informers.Core().V1().Pods(), fake) + if err != nil { + t.Fatalf("error creating Deployment controller: %v", err) + } + + controller.eventRecorder = &record.FakeRecorder{} + controller.dListerSynced = alwaysReady + controller.rsListerSynced = alwaysReady + controller.podListerSynced = alwaysReady + + stopCh := make(chan struct{}) + defer close(stopCh) + informers.Start(stopCh) + + d := newDeployment(test.deploymentName, 1, nil, nil, nil, map[string]string{"foo": "bar"}) + + if _, err := controller.getNewReplicaSet(ctx, d, []*apps.ReplicaSet{}, []*apps.ReplicaSet{}, true); err != nil { + t.Errorf("failed to create new ReplicaSet: %v", err) + return + } + + rsName := "" + for _, action := range fake.Actions() { + if createAction, ok := action.(testclient.CreateAction); ok { + if createdRS, ok := createAction.GetObject().(*apps.ReplicaSet); ok { + if createdRS.Name != "" { + rsName = createdRS.Name + break + } + } + } + } + + if len(rsName) > validation.DNS1123SubdomainMaxLength { + t.Errorf("ReplicaSet name length %d, want <= %d", len(rsName), validation.DNS1123SubdomainMaxLength) + } + + parts := strings.Split(rsName, "-") + if len(parts) < 2 { + t.Errorf("ReplicaSet name should contain at least one hyphen separator") + } + + deploymentPortion := strings.Join(parts[:len(parts)-1], "-") + if len(test.deploymentName) <= 242 { + if len(deploymentPortion) != len(test.deploymentName) { + t.Errorf("Deployment name portion should be %d chars, got %d", len(test.deploymentName), len(deploymentPortion)) + } + } else { + if len(deploymentPortion) != 242 { + t.Errorf("Truncated deployment name should be 242 chars, got %d", len(deploymentPortion)) + } + } + + if deploymentPortion != test.wantDeploymentPortion { + t.Errorf("Deployment name portion mismatch: got %q, want %q", deploymentPortion, test.wantDeploymentPortion) + } + } +} + +func TestGenerateReplicaSetName(t *testing.T) { + tests := []struct { + name string + deploymentName string + hash string + want string + }{ + { + name: "short name", + deploymentName: "my-deployment", + hash: "abcde12345", + want: "my-deployment-abcde12345", + }, + { + name: "maximum length without truncating", + deploymentName: strings.Repeat("a", 242), + hash: "abcde12345", + want: strings.Repeat("a", 242) + "-abcde12345", + }, + { + name: "very long deployment name is truncated", + deploymentName: strings.Repeat("b", 250), + hash: "abcde12345", + want: strings.Repeat("b", 242) + "-abcde12345", + }, + { + name: "very long hash is not truncated", + deploymentName: strings.Repeat("d", 252), + hash: strings.Repeat("h", 252), + want: strings.Repeat("d", 252) + "-" + strings.Repeat("h", 252), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := generateReplicaSetName(test.deploymentName, test.hash) + + if got != test.want { + t.Errorf("generateReplicaSetName(%q, %q) = %q, want %q", test.deploymentName, test.hash, got, test.want) + } + }) + } +}