Browse Source

cmd/k8s-operator: truncate long StatefulSet name prefixes (#10343)

Kubernetes can generate StatefulSet names that are too long and result in invalid Pod revision hash label values.
Calculate whether a StatefulSet name generated for a Service or Ingress
will be too long and if so, truncate it.

Updates tailscale/tailscale#10284

Signed-off-by: Irbe Krumina <[email protected]>
Irbe Krumina 2 years ago
parent
commit
66471710f8
4 changed files with 84 additions and 1 deletions
  1. 31 1
      cmd/k8s-operator/sts.go
  2. 50 0
      cmd/k8s-operator/sts_test.go
  3. 1 0
      go.mod
  4. 2 0
      go.sum

+ 31 - 1
cmd/k8s-operator/sts.go

@@ -21,6 +21,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apiserver/pkg/storage/names"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/yaml"
 	"tailscale.com/client/tailscale"
@@ -176,10 +177,39 @@ func (a *tailscaleSTSReconciler) Cleanup(ctx context.Context, logger *zap.Sugare
 	return true, nil
 }
 
+// maxStatefulSetNameLength is maximum length the StatefulSet name can
+// have to NOT result in a too long value for controller-revision-hash
+// label value (see https://github.com/kubernetes/kubernetes/issues/64023).
+// controller-revision-hash label value consists of StatefulSet's name + hyphen + revision hash.
+// Maximum label value length is 63 chars. Length of revision hash is 10 chars.
+// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
+// https://github.com/kubernetes/kubernetes/blob/v1.28.4/pkg/controller/history/controller_history.go#L90-L104
+const maxStatefulSetNameLength = 63 - 10 - 1
+
+// statefulSetNameBase accepts name of parent resource and returns a string in
+// form ts-<portion-of-parentname>- that, when passed to Kubernetes name
+// generation will NOT result in a StatefulSet name longer than 52 chars.
+// This is done because of https://github.com/kubernetes/kubernetes/issues/64023.
+func statefulSetNameBase(parent string) string {
+
+	base := fmt.Sprintf("ts-%s-", parent)
+
+	// Calculate what length name GenerateName returns for this base.
+	generator := names.SimpleNameGenerator
+	generatedName := generator.GenerateName(base)
+
+	if excess := len(generatedName) - maxStatefulSetNameLength; excess > 0 {
+		base = base[:len(base)-excess-1] // take extra char off to make space for hyphen
+		base = base + "-"                // re-instate hyphen
+	}
+	return base
+}
+
 func (a *tailscaleSTSReconciler) reconcileHeadlessService(ctx context.Context, logger *zap.SugaredLogger, sts *tailscaleSTSConfig) (*corev1.Service, error) {
+	nameBase := statefulSetNameBase(sts.ParentResourceName)
 	hsvc := &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: "ts-" + sts.ParentResourceName + "-",
+			GenerateName: nameBase,
 			Namespace:    a.operatorNamespace,
 			Labels:       sts.ChildResourceLabels,
 		},

+ 50 - 0
cmd/k8s-operator/sts_test.go

@@ -0,0 +1,50 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !plan9
+
+package main
+
+import (
+	"testing"
+)
+
+// Test_statefulSetNameBase tests that parent name portion in a StatefulSet name
+// base will be truncated if the parent name is longer than 43 chars to ensure
+// that the total does not exceed 52 chars.
+// How many chars need to be cut off parent name depends on an internal var in
+// kube name generation code that can change at which point this test will break
+// and need to be changed. This is okay as we do not rely on that value in
+// code whilst being aware when it changes might still be useful.
+// https://github.com/kubernetes/kubernetes/blob/v1.28.4/staging/src/k8s.io/apiserver/pkg/storage/names/generate.go#L45.
+// https://github.com/kubernetes/kubernetes/pull/116430
+func Test_statefulSetNameBase(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		out  string
+	}{
+		{
+			name: "43 chars",
+			in:   "oidhexl9o832hcbhyg4uz6o0s7u9uae54h5k8ofs9xb",
+			out:  "ts-oidhexl9o832hcbhyg4uz6o0s7u9uae54h5k8ofs9xb-",
+		},
+		{
+			name: "44 chars",
+			in:   "oidhexl9o832hcbhyg4uz6o0s7u9uae54h5k8ofs9xbo",
+			out:  "ts-oidhexl9o832hcbhyg4uz6o0s7u9uae54h5k8ofs9xb-",
+		},
+		{
+			name: "42 chars",
+			in:   "oidhexl9o832hcbhyg4uz6o0s7u9uae54h5k8ofs9x",
+			out:  "ts-oidhexl9o832hcbhyg4uz6o0s7u9uae54h5k8ofs9x-",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := statefulSetNameBase(tt.in); got != tt.out {
+				t.Errorf("stsNamePrefix(%s) = %q, want %s", tt.in, got, tt.out)
+			}
+		})
+	}
+}

+ 1 - 0
go.mod

@@ -97,6 +97,7 @@ require (
 	inet.af/wf v0.0.0-20221017222439-36129f591884
 	k8s.io/api v0.28.2
 	k8s.io/apimachinery v0.28.2
+	k8s.io/apiserver v0.28.2
 	k8s.io/client-go v0.28.2
 	nhooyr.io/websocket v1.8.7
 	sigs.k8s.io/controller-runtime v0.16.2

+ 2 - 0
go.sum

@@ -1449,6 +1449,8 @@ k8s.io/apiextensions-apiserver v0.28.2 h1:J6/QRWIKV2/HwBhHRVITMLYoypCoPY1ftigDM0
 k8s.io/apiextensions-apiserver v0.28.2/go.mod h1:5tnkxLGa9nefefYzWuAlWZ7RZYuN/765Au8cWLA6SRg=
 k8s.io/apimachinery v0.28.2 h1:KCOJLrc6gu+wV1BYgwik4AF4vXOlVJPdiqn0yAWWwXQ=
 k8s.io/apimachinery v0.28.2/go.mod h1:RdzF87y/ngqk9H4z3EL2Rppv5jj95vGS/HaFXrLDApU=
+k8s.io/apiserver v0.28.2 h1:rBeYkLvF94Nku9XfXyUIirsVzCzJBs6jMn3NWeHieyI=
+k8s.io/apiserver v0.28.2/go.mod h1:f7D5e8wH8MWcKD7azq6Csw9UN+CjdtXIVQUyUhrtb+E=
 k8s.io/client-go v0.28.2 h1:DNoYI1vGq0slMBN/SWKMZMw0Rq+0EQW6/AK4v9+3VeY=
 k8s.io/client-go v0.28.2/go.mod h1:sMkApowspLuc7omj1FOSUxSoqjr+d5Q0Yc0LOFnYFJY=
 k8s.io/component-base v0.28.2 h1:Yc1yU+6AQSlpJZyvehm/NkJBII72rzlEsd6MkBQ+G0E=