Merge pull request #114586 from andrewsykim/apiserver-lease-rename

Rename apiserver identity lease labels to apiserver.kubernetes.io/identity

pkg/controller/storageversiongc/gc_controller_test.go

@@ -48,7 +48,7 @@ func newKubeApiserverLease(name, holderIdentity string) *coordinationv1.Lease {
 			Name:      name,
 			Namespace: metav1.NamespaceSystem,
 			Labels: map[string]string{
-				"k8s.io/component": "kube-apiserver",
+				"apiserver.kubernetes.io/identity": "kube-apiserver",
 			},
 		},
 		Spec: coordinationv1.LeaseSpec{

pkg/controlplane/controller/apiserverleasegc/gc_controller_test.go

@@ -44,7 +44,7 @@ func Test_Controller(t *testing.T) {
 					Name:      "kube-apiserver-12345",
 					Namespace: metav1.NamespaceSystem,
 					Labels: map[string]string{
-						"k8s.io/component": "kube-apiserver",
+						"apiserver.kubernetes.io/identity": "kube-apiserver",
 					},
 				},
 				Spec: coordinationv1.LeaseSpec{
@@ -62,7 +62,7 @@ func Test_Controller(t *testing.T) {
 					Name:      "kube-apiserver-12345",
 					Namespace: metav1.NamespaceSystem,
 					Labels: map[string]string{
-						"k8s.io/component": "kube-controller-manager",
+						"apiserver.kubernetes.io/identity": "kube-controller-manager",
 					},
 				},
 				Spec: coordinationv1.LeaseSpec{
@@ -80,7 +80,7 @@ func Test_Controller(t *testing.T) {
 					Name:      "kube-apiserver-12345",
 					Namespace: metav1.NamespaceSystem,
 					Labels: map[string]string{
-						"k8s.io/component": "kube-apiserver",
+						"apiserver.kubernetes.io/identity": "kube-apiserver",
 					},
 				},
 				Spec: coordinationv1.LeaseSpec{
@@ -98,7 +98,7 @@ func Test_Controller(t *testing.T) {
 					Name:      "kube-apiserver-12345",
 					Namespace: metav1.NamespaceSystem,
 					Labels: map[string]string{
-						"k8s.io/component": "kube-apiserver",
+						"apiserver.kubernetes.io/identity": "kube-apiserver",
 					},
 				},
 				Spec: coordinationv1.LeaseSpec{
@@ -116,7 +116,7 @@ func Test_Controller(t *testing.T) {
 					Name:      "kube-apiserver-12345",
 					Namespace: metav1.NamespaceSystem,
 					Labels: map[string]string{
-						"k8s.io/component": "kube-apiserver",
+						"apiserver.kubernetes.io/identity": "kube-apiserver",
 					},
 				},
 				Spec: coordinationv1.LeaseSpec{
@@ -132,7 +132,7 @@ func Test_Controller(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			clientset := fake.NewSimpleClientset(test.lease)
-			controller := NewAPIServerLeaseGC(clientset, 100*time.Millisecond, metav1.NamespaceSystem, "k8s.io/component=kube-apiserver")
+			controller := NewAPIServerLeaseGC(clientset, 100*time.Millisecond, metav1.NamespaceSystem, "apiserver.kubernetes.io/identity=kube-apiserver")
 			go controller.Run(nil)
 
 			time.Sleep(time.Second)

pkg/controlplane/instance.go

@@ -123,9 +123,11 @@ const (
 	// IdentityLeaseComponentLabelKey is used to apply a component label to identity lease objects, indicating:
 	//   1. the lease is an identity lease (different from leader election leases)
 	//   2. which component owns this lease
-	IdentityLeaseComponentLabelKey = "k8s.io/component"
+	IdentityLeaseComponentLabelKey = "apiserver.kubernetes.io/identity"
 	// KubeAPIServer defines variable used internally when referring to kube-apiserver component
 	KubeAPIServer = "kube-apiserver"
+	// DeprecatedKubeAPIServerIdentityLeaseLabelSelector selects kube-apiserver identity leases
+	DeprecatedKubeAPIServerIdentityLeaseLabelSelector = "k8s.io/component=kube-apiserver"
 	// KubeAPIServerIdentityLeaseLabelSelector selects kube-apiserver identity leases
 	KubeAPIServerIdentityLeaseLabelSelector = IdentityLeaseComponentLabelKey + "=" + KubeAPIServer
 	// repairLoopInterval defines the interval used to run the Services ClusterIP and NodePort repair loops
@@ -505,10 +507,28 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget)
 				IdentityLeaseRenewIntervalPeriod,
 				leaseName,
 				metav1.NamespaceSystem,
-				labelAPIServerHeartbeat)
+				// TODO: receive identity label value as a parameter when post start hook is moved to generic apiserver.
+				labelAPIServerHeartbeatFunc(KubeAPIServer))
 			go controller.Run(hookContext.StopCh)
 			return nil
 		})
+		// Labels for apiserver idenitiy leases switched from k8s.io/component=kube-apiserver to apiserver.kubernetes.io/identity=kube-apiserver.
+		// For compatibility, garbage collect leases with both labels for at least 1 release
+		// TODO: remove in Kubernetes 1.28
+		m.GenericAPIServer.AddPostStartHookOrDie("start-deprecated-kube-apiserver-identity-lease-garbage-collector", func(hookContext genericapiserver.PostStartHookContext) error {
+			kubeClient, err := kubernetes.NewForConfig(hookContext.LoopbackClientConfig)
+			if err != nil {
+				return err
+			}
+			go apiserverleasegc.NewAPIServerLeaseGC(
+				kubeClient,
+				IdentityLeaseGCPeriod,
+				metav1.NamespaceSystem,
+				DeprecatedKubeAPIServerIdentityLeaseLabelSelector,
+			).Run(hookContext.StopCh)
+			return nil
+		})
+		// TODO: move this into generic apiserver and make the lease identity value configurable
 		m.GenericAPIServer.AddPostStartHookOrDie("start-kube-apiserver-identity-lease-garbage-collector", func(hookContext genericapiserver.PostStartHookContext) error {
 			kubeClient, err := kubernetes.NewForConfig(hookContext.LoopbackClientConfig)
 			if err != nil {
@@ -536,21 +556,24 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget)
 	return m, nil
 }
 
-func labelAPIServerHeartbeat(lease *coordinationapiv1.Lease) error {
+func labelAPIServerHeartbeatFunc(identity string) lease.ProcessLeaseFunc {
-	if lease.Labels == nil {
+	return func(lease *coordinationapiv1.Lease) error {
-		lease.Labels = map[string]string{}
+		if lease.Labels == nil {
-	}
+			lease.Labels = map[string]string{}
-	// This label indicates that kube-apiserver owns this identity lease object
+		}
-	lease.Labels[IdentityLeaseComponentLabelKey] = KubeAPIServer
 
-	hostname, err := os.Hostname()
+		// This label indiciates the identity of the lease object.
-	if err != nil {
+		lease.Labels[IdentityLeaseComponentLabelKey] = identity
-		return err
-	}
 
-	// convenience label to easily map a lease object to a specific apiserver
+		hostname, err := os.Hostname()
-	lease.Labels[apiv1.LabelHostname] = hostname
+		if err != nil {
-	return nil
+			return err
+		}
+
+		// convenience label to easily map a lease object to a specific apiserver
+		lease.Labels[apiv1.LabelHostname] = hostname
+		return nil
+	}
 }
 
 // InstallLegacyAPI will install the legacy APIs for the restStorageProviders if they are enabled.

staging/src/k8s.io/apiserver/pkg/server/config.go

@@ -34,6 +34,7 @@ import (
 
 	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/google/uuid"
+	"golang.org/x/crypto/cryptobyte"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -345,8 +346,24 @@ func NewConfig(codecs serializer.CodecFactory) *Config {
 			klog.Fatalf("error getting hostname for apiserver identity: %v", err)
 		}
 
-		hash := sha256.Sum256([]byte(hostname))
+		// Since the hash needs to be unique across each kube-apiserver and aggregated apiservers,
-		id = "kube-apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16]))
+		// the hash used for the identity should include both the hostname and the identity value.
+		// TODO: receive the identity value as a parameter once the apiserver identity lease controller
+		// post start hook is moved to generic apiserver.
+		b := cryptobyte.NewBuilder(nil)
+		b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+			b.AddBytes([]byte(hostname))
+		})
+		b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+			b.AddBytes([]byte("kube-apiserver"))
+		})
+		hashData, err := b.Bytes()
+		if err != nil {
+			klog.Fatalf("error building hash data for apiserver identity: %v", err)
+		}
+
+		hash := sha256.Sum256(hashData)
+		id = "apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16]))
 	}
 	lifecycleSignals := newLifecycleSignals()
 

staging/src/k8s.io/controller-manager/go.mod

@@ -79,6 +79,7 @@ require (
 	go.uber.org/atomic v1.7.0 // indirect
 	go.uber.org/multierr v1.6.0 // indirect
 	go.uber.org/zap v1.19.0 // indirect
+	golang.org/x/crypto v0.1.0 // indirect
 	golang.org/x/net v0.4.0 // indirect
 	golang.org/x/sync v0.1.0 // indirect
 	golang.org/x/sys v0.3.0 // indirect

staging/src/k8s.io/controller-manager/go.sum

@@ -381,6 +381,7 @@ golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8U
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.1.0 h1:MDRAIl0xIo9Io2xV565hzXHw3zVseKrJKodhohM5CjU=
+golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=

test/e2e/apimachinery/apiserver_identity.go

@@ -27,6 +27,8 @@ import (
 	"time"
 
 	"github.com/onsi/ginkgo/v2"
+	"golang.org/x/crypto/cryptobyte"
+
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -115,7 +117,7 @@ var _ = SIGDescribe("kube-apiserver identity [Feature:APIServerIdentity]", func(
 		}
 
 		leases, err := client.CoordinationV1().Leases(metav1.NamespaceSystem).List(context.TODO(), metav1.ListOptions{
-			LabelSelector: "k8s.io/component=kube-apiserver",
+			LabelSelector: "apiserver.kubernetes.io/identity=kube-apiserver",
 		})
 		framework.ExpectNoError(err)
 		framework.ExpectEqual(len(leases.Items), len(controlPlaneNodes), "unexpected number of leases")
@@ -124,8 +126,18 @@ var _ = SIGDescribe("kube-apiserver identity [Feature:APIServerIdentity]", func(
 			hostname, err := getControlPlaneHostname(ctx, &node)
 			framework.ExpectNoError(err)
 
-			hash := sha256.Sum256([]byte(hostname))
+			b := cryptobyte.NewBuilder(nil)
-			leaseName := "kube-apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16]))
+			b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+				b.AddBytes([]byte(hostname))
+			})
+			b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+				b.AddBytes([]byte("kube-apiserver"))
+			})
+
+			hashData, err := b.Bytes()
+			framework.ExpectNoError(err)
+			hash := sha256.Sum256(hashData)
+			leaseName := "apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16]))
 
 			lease, err := client.CoordinationV1().Leases(metav1.NamespaceSystem).Get(context.TODO(), leaseName, metav1.GetOptions{})
 			framework.ExpectNoError(err)
@@ -161,7 +173,7 @@ var _ = SIGDescribe("kube-apiserver identity [Feature:APIServerIdentity]", func(
 		// As long as the hostname of kube-apiserver is unchanged, a restart should not result in new Lease objects.
 		// Check that the number of lease objects remains the same after restarting kube-apiserver.
 		leases, err = client.CoordinationV1().Leases(metav1.NamespaceSystem).List(context.TODO(), metav1.ListOptions{
-			LabelSelector: "k8s.io/component=kube-apiserver",
+			LabelSelector: "apiserver.kubernetes.io/identity=kube-apiserver",
 		})
 		framework.ExpectNoError(err)
 		framework.ExpectEqual(len(leases.Items), len(controlPlaneNodes), "unexpected number of leases")

test/integration/controlplane/apiserver_identity_test.go

@@ -26,6 +26,8 @@ import (
 	"testing"
 	"time"
 
+	"golang.org/x/crypto/cryptobyte"
+
 	coordinationv1 "k8s.io/api/coordination/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -45,9 +47,21 @@ const (
 	testLeaseName = "apiserver-lease-test"
 )
 
-func expectedAPIServerIdentity(hostname string) string {
+func expectedAPIServerIdentity(t *testing.T, hostname string) string {
-	hash := sha256.Sum256([]byte(hostname))
+	b := cryptobyte.NewBuilder(nil)
-	return "kube-apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16]))
+	b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+		b.AddBytes([]byte(hostname))
+	})
+	b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+		b.AddBytes([]byte("kube-apiserver"))
+	})
+	hashData, err := b.Bytes()
+	if err != nil {
+		t.Fatalf("error building hash data for apiserver identity: %v", err)
+	}
+
+	hash := sha256.Sum256(hashData)
+	return "apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16]))
 }
 
 func TestCreateLeaseOnStart(t *testing.T) {
@@ -84,8 +98,8 @@ func TestCreateLeaseOnStart(t *testing.T) {
 		}
 
 		lease := leases.Items[0]
-		if lease.Name != expectedAPIServerIdentity(hostname) {
+		if lease.Name != expectedAPIServerIdentity(t, hostname) {
-			return false, fmt.Errorf("unexpected apiserver identity, got: %v, expected: %v", lease.Name, expectedAPIServerIdentity(hostname))
+			return false, fmt.Errorf("unexpected apiserver identity, got: %v, expected: %v", lease.Name, expectedAPIServerIdentity(t, hostname))
 		}
 
 		if lease.Labels[corev1.LabelHostname] != hostname {
@@ -134,12 +148,54 @@ func TestLeaseGarbageCollection(t *testing.T) {
 	t.Run("expired non-identity lease should not be garbage collected",
 		testLeaseNotGarbageCollected(t, kubeclient, expiredLease))
 
-	// identity leases (with k8s.io/component label) created in user namespaces should not be GC'ed
+	// identity leases (with apiserver.kubernetes.io/identity label) created in user namespaces should not be GC'ed
 	expiredNonKubeSystemLease := newTestLease(time.Now().Add(-2*time.Hour), metav1.NamespaceDefault)
 	t.Run("expired non-system identity lease should not be garbage collected",
 		testLeaseNotGarbageCollected(t, kubeclient, expiredNonKubeSystemLease))
 }
 
+func TestLeaseGarbageCollectionWithDeprecatedLabels(t *testing.T) {
+	oldIdentityLeaseDurationSeconds := controlplane.IdentityLeaseDurationSeconds
+	oldIdentityLeaseGCPeriod := controlplane.IdentityLeaseGCPeriod
+	oldIdentityLeaseRenewIntervalPeriod := controlplane.IdentityLeaseRenewIntervalPeriod
+	defer func() {
+		// reset the default values for leases after this test
+		controlplane.IdentityLeaseDurationSeconds = oldIdentityLeaseDurationSeconds
+		controlplane.IdentityLeaseGCPeriod = oldIdentityLeaseGCPeriod
+		controlplane.IdentityLeaseRenewIntervalPeriod = oldIdentityLeaseRenewIntervalPeriod
+	}()
+
+	// Shorten lease parameters so GC behavior can be exercised in integration tests
+	controlplane.IdentityLeaseDurationSeconds = 1
+	controlplane.IdentityLeaseGCPeriod = time.Second
+	controlplane.IdentityLeaseRenewIntervalPeriod = time.Second
+
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.APIServerIdentity, true)()
+	result := kubeapiservertesting.StartTestServerOrDie(t, nil, nil, framework.SharedEtcd())
+	defer result.TearDownFn()
+
+	kubeclient, err := kubernetes.NewForConfig(result.ClientConfig)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	expiredLease := newTestLeaseWithDeprecatedLabels(time.Now().Add(-2*time.Hour), metav1.NamespaceSystem)
+	t.Run("expired apiserver lease should be garbage collected",
+		testLeaseGarbageCollected(t, kubeclient, expiredLease))
+
+	freshLease := newTestLeaseWithDeprecatedLabels(time.Now().Add(-2*time.Minute), metav1.NamespaceSystem)
+	t.Run("fresh apiserver lease should not be garbage collected",
+		testLeaseNotGarbageCollected(t, kubeclient, freshLease))
+
+	expiredLease.Labels = nil
+	t.Run("expired non-identity lease should not be garbage collected",
+		testLeaseNotGarbageCollected(t, kubeclient, expiredLease))
+
+	// identity leases (with k8s.io/component label) created in user namespaces should not be GC'ed
+	expiredNonKubeSystemLease := newTestLeaseWithDeprecatedLabels(time.Now().Add(-2*time.Hour), metav1.NamespaceDefault)
+	t.Run("expired non-system identity lease should not be garbage collected",
+		testLeaseNotGarbageCollected(t, kubeclient, expiredNonKubeSystemLease))
+}
+
 func testLeaseGarbageCollected(t *testing.T, client kubernetes.Interface, lease *coordinationv1.Lease) func(t *testing.T) {
 	return func(t *testing.T) {
 		ns := lease.Namespace
@@ -203,3 +259,21 @@ func newTestLease(acquireTime time.Time, namespace string) *coordinationv1.Lease
 		},
 	}
 }
+
+func newTestLeaseWithDeprecatedLabels(acquireTime time.Time, namespace string) *coordinationv1.Lease {
+	return &coordinationv1.Lease{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      testLeaseName,
+			Namespace: namespace,
+			Labels: map[string]string{
+				"k8s.io/component": "kube-apiserver",
+			},
+		},
+		Spec: coordinationv1.LeaseSpec{
+			HolderIdentity:       pointer.StringPtr(testLeaseName),
+			LeaseDurationSeconds: pointer.Int32Ptr(3600),
+			AcquireTime:          &metav1.MicroTime{Time: acquireTime},
+			RenewTime:            &metav1.MicroTime{Time: acquireTime},
+		},
+	}
+}