Merge pull request #63606 from liggitt/dynamic-discovery-tuning

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Optimize admission plugin API requests

xref https://github.com/kubernetes/kubernetes/issues/63030#issuecomment-387774934

* resource changes are typically slow-moving, and full discovery can be slightly expensive, so this reduces the refresh to a 30 second interval and bumps QPS on the admission client (which wasn't done when the client started getting used for discovery in #62659)
* a large consumer of API requests in scale tests was the node restriction plugin pod lookups during pod deletion. this switches to use the same informer that is feeding the node authorizer graph to avoid those lookups entirely.

```release-note
NONE
```

cmd/kube-apiserver/app/server.go

@@ -579,7 +579,7 @@ func BuildAdmissionPluginInitializers(
 
 	admissionPostStartHook := func(context genericapiserver.PostStartHookContext) error {
 		discoveryRESTMapper.Reset()
-		go utilwait.Until(discoveryRESTMapper.Reset, 10*time.Second, context.StopCh)
+		go utilwait.Until(discoveryRESTMapper.Reset, 30*time.Second, context.StopCh)
 		return nil
 	}
 

plugin/pkg/admission/noderestriction/BUILD

@@ -16,13 +16,12 @@ go_library(
         "//pkg/apis/core:go_default_library",
         "//pkg/apis/policy:go_default_library",
         "//pkg/auth/nodeidentifier:go_default_library",
-        "//pkg/client/clientset_generated/internalclientset:go_default_library",
+        "//pkg/client/informers/informers_generated/internalversion:go_default_library",
-        "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library",
+        "//pkg/client/listers/core/internalversion:go_default_library",
         "//pkg/features:go_default_library",
         "//pkg/kubeapiserver/admission:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
@@ -38,14 +37,14 @@ go_test(
         "//pkg/apis/core:go_default_library",
         "//pkg/apis/policy:go_default_library",
         "//pkg/auth/nodeidentifier:go_default_library",
-        "//pkg/client/clientset_generated/internalclientset/fake:go_default_library",
+        "//pkg/client/listers/core/internalversion:go_default_library",
-        "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library",
         "//pkg/features:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
+        "//vendor/k8s.io/client-go/tools/cache:go_default_library",
     ],
 )
 

plugin/pkg/admission/noderestriction/admission.go

@@ -22,7 +22,6 @@ import (
 
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apiserver/pkg/admission"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
@@ -31,8 +30,8 @@ import (
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/policy"
 	"k8s.io/kubernetes/pkg/auth/nodeidentifier"
-	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
+	informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
-	coreinternalversion "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
+	internalversion "k8s.io/kubernetes/pkg/client/listers/core/internalversion"
 	"k8s.io/kubernetes/pkg/features"
 	kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
 )
@@ -62,18 +61,18 @@ func NewPlugin(nodeIdentifier nodeidentifier.NodeIdentifier) *nodePlugin {
 type nodePlugin struct {
 	*admission.Handler
 	nodeIdentifier nodeidentifier.NodeIdentifier
-	podsGetter     coreinternalversion.PodsGetter
+	podsGetter     internalversion.PodLister
 	// allows overriding for testing
 	features utilfeature.FeatureGate
 }
 
 var (
 	_ = admission.Interface(&nodePlugin{})
-	_ = kubeapiserveradmission.WantsInternalKubeClientSet(&nodePlugin{})
+	_ = kubeapiserveradmission.WantsInternalKubeInformerFactory(&nodePlugin{})
 )
 
-func (p *nodePlugin) SetInternalKubeClientSet(f internalclientset.Interface) {
+func (p *nodePlugin) SetInternalKubeInformerFactory(f informers.SharedInformerFactory) {
-	p.podsGetter = f.Core()
+	p.podsGetter = f.Core().InternalVersion().Pods().Lister()
 }
 
 func (p *nodePlugin) ValidateInitialization() error {
@@ -183,15 +182,11 @@ func (c *nodePlugin) admitPod(nodeName string, a admission.Attributes) error {
 		return nil
 
 	case admission.Delete:
-		// get the existing pod from the server cache
+		// get the existing pod
-		existingPod, err := c.podsGetter.Pods(a.GetNamespace()).Get(a.GetName(), v1.GetOptions{ResourceVersion: "0"})
+		existingPod, err := c.podsGetter.Pods(a.GetNamespace()).Get(a.GetName())
-		if errors.IsNotFound(err) {
-			// wasn't found in the server cache, do a live lookup before forbidding
-			existingPod, err = c.podsGetter.Pods(a.GetNamespace()).Get(a.GetName(), v1.GetOptions{})
 		if errors.IsNotFound(err) {
 			return err
 		}
-		}
 		if err != nil {
 			return admission.NewForbidden(a, err)
 		}
@@ -241,15 +236,11 @@ func (c *nodePlugin) admitPodEviction(nodeName string, a admission.Attributes) e
 			}
 			podName = eviction.Name
 		}
-		// get the existing pod from the server cache
+		// get the existing pod
-		existingPod, err := c.podsGetter.Pods(a.GetNamespace()).Get(podName, v1.GetOptions{ResourceVersion: "0"})
+		existingPod, err := c.podsGetter.Pods(a.GetNamespace()).Get(podName)
-		if errors.IsNotFound(err) {
-			// wasn't found in the server cache, do a live lookup before forbidding
-			existingPod, err = c.podsGetter.Pods(a.GetNamespace()).Get(podName, v1.GetOptions{})
 		if errors.IsNotFound(err) {
 			return err
 		}
-		}
 		if err != nil {
 			return admission.NewForbidden(a, err)
 		}
@@ -376,7 +367,7 @@ func (c *nodePlugin) admitServiceAccount(nodeName string, a admission.Attributes
 	if ref.UID == "" {
 		return admission.NewForbidden(a, fmt.Errorf("node requested token with a pod binding without a uid"))
 	}
-	pod, err := c.podsGetter.Pods(a.GetNamespace()).Get(ref.Name, v1.GetOptions{})
+	pod, err := c.podsGetter.Pods(a.GetNamespace()).Get(ref.Name)
 	if errors.IsNotFound(err) {
 		return err
 	}

plugin/pkg/admission/noderestriction/admission_test.go

@@ -25,12 +25,12 @@ import (
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/authentication/user"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	"k8s.io/client-go/tools/cache"
 	authenticationapi "k8s.io/kubernetes/pkg/apis/authentication"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/policy"
 	"k8s.io/kubernetes/pkg/auth/nodeidentifier"
-	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
+	"k8s.io/kubernetes/pkg/client/listers/core/internalversion"
-	coreinternalversion "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
 	"k8s.io/kubernetes/pkg/features"
 )
 
@@ -63,6 +63,7 @@ func makeTestPod(namespace, name, node string, mirror bool) *api.Pod {
 func makeTestPodEviction(name string) *policy.Eviction {
 	eviction := &policy.Eviction{}
 	eviction.Name = name
+	eviction.Namespace = "ns"
 	return eviction
 }
 
@@ -135,10 +136,20 @@ func Test_nodePlugin_Admit(t *testing.T) {
 		svcacctResource  = api.Resource("serviceaccounts").WithVersion("v1")
 		tokenrequestKind = api.Kind("TokenRequest").WithVersion("v1")
 
-		noExistingPods = fake.NewSimpleClientset().Core()
+		noExistingPodsIndex = cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)
-		existingPods   = fake.NewSimpleClientset(mymirrorpod, othermirrorpod, unboundmirrorpod, mypod, otherpod, unboundpod).Core()
+		noExistingPods      = internalversion.NewPodLister(noExistingPodsIndex)
+
+		existingPodsIndex = cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)
+		existingPods      = internalversion.NewPodLister(existingPodsIndex)
 	)
 
+	existingPodsIndex.Add(mymirrorpod)
+	existingPodsIndex.Add(othermirrorpod)
+	existingPodsIndex.Add(unboundmirrorpod)
+	existingPodsIndex.Add(mypod)
+	existingPodsIndex.Add(otherpod)
+	existingPodsIndex.Add(unboundpod)
+
 	sapod := makeTestPod("ns", "mysapod", "mynode", true)
 	sapod.Spec.ServiceAccountName = "foo"
 
@@ -153,7 +164,7 @@ func Test_nodePlugin_Admit(t *testing.T) {
 
 	tests := []struct {
 		name       string
-		podsGetter coreinternalversion.PodsGetter
+		podsGetter internalversion.PodLister
 		attributes admission.Attributes
 		features   utilfeature.FeatureGate
 		err        string
@@ -456,7 +467,7 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			err:        "forbidden: unexpected operation",
 		},
 		{
-			name:       "forbid create of eviction for normal pod bound to another",
+			name:       "forbid create of unnamed eviction for normal pod bound to another",
 			podsGetter: existingPods,
 			attributes: admission.NewAttributesRecord(unnamedEviction, nil, evictionKind, otherpod.Namespace, otherpod.Name, podResource, "eviction", admission.Create, mynode),
 			err:        "spec.nodeName set to itself",

test/integration/auth/node_test.go

@@ -97,7 +97,7 @@ func TestNodeAuthorizer(t *testing.T) {
 
 	// Set up NodeRestriction admission
 	nodeRestrictionAdmission := noderestriction.NewPlugin(nodeidentifier.NewDefaultNodeIdentifier())
-	nodeRestrictionAdmission.SetInternalKubeClientSet(superuserClient)
+	nodeRestrictionAdmission.SetInternalKubeInformerFactory(informerFactory)
 	if err := nodeRestrictionAdmission.ValidateInitialization(); err != nil {
 		t.Fatal(err)
 	}