Merge pull request #56478 from hzxuzhonghu/fix-admissionregistration

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>.

admissionregistration use shared informer instead of poll

**What this PR does / why we need it**:

poll with 1s interval influence apiserver's performance

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #56357 

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

staging/src/k8s.io/apiserver/Godeps/Godeps.json

@@ -1790,6 +1790,10 @@
 			"ImportPath": "k8s.io/client-go/informers",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 		},
+		{
+			"ImportPath": "k8s.io/client-go/informers/admissionregistration/v1beta1",
+			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+		},
 		{
 			"ImportPath": "k8s.io/client-go/kubernetes",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
@@ -1814,6 +1818,10 @@
 			"ImportPath": "k8s.io/client-go/kubernetes/typed/core/v1",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 		},
+		{
+			"ImportPath": "k8s.io/client-go/listers/admissionregistration/v1beta1",
+			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+		},
 		{
 			"ImportPath": "k8s.io/client-go/listers/core/v1",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"

staging/src/k8s.io/apiserver/pkg/admission/configuration/BUILD

@@ -21,9 +21,12 @@ go_test(
         "//vendor/k8s.io/api/admissionregistration/v1beta1: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/labels:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
+        "//vendor/k8s.io/client-go/listers/admissionregistration/v1beta1:go_default_library",
+        "//vendor/k8s.io/client-go/tools/cache:go_default_library",
     ],
 )
 
@@ -42,8 +45,13 @@ go_library(
         "//vendor/k8s.io/api/admissionregistration/v1beta1: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/labels:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
+        "//vendor/k8s.io/client-go/informers/admissionregistration/v1beta1:go_default_library",
+        "//vendor/k8s.io/client-go/listers/admissionregistration/v1beta1:go_default_library",
+        "//vendor/k8s.io/client-go/tools/cache:go_default_library",
     ],
 )
 

staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go

@@ -18,84 +18,70 @@ package configuration
 
 import (
 	"fmt"
-	"reflect"
 	"sort"
-
-	"github.com/golang/glog"
+	"sync/atomic"
 
 	"k8s.io/api/admissionregistration/v1beta1"
-	"k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/labels"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	admissionregistrationinformers "k8s.io/client-go/informers/admissionregistration/v1beta1"
+	admissionregistrationlisters "k8s.io/client-go/listers/admissionregistration/v1beta1"
+	"k8s.io/client-go/tools/cache"
 )
 
-type MutatingWebhookConfigurationLister interface {
-	List(opts metav1.ListOptions) (*v1beta1.MutatingWebhookConfigurationList, error)
-}
-
 // MutatingWebhookConfigurationManager collects the mutating webhook objects so that they can be called.
 type MutatingWebhookConfigurationManager struct {
-	*poller
+	configuration *atomic.Value
+	lister        admissionregistrationlisters.MutatingWebhookConfigurationLister
 }
 
-func NewMutatingWebhookConfigurationManager(c MutatingWebhookConfigurationLister) *MutatingWebhookConfigurationManager {
-	getFn := func() (runtime.Object, error) {
-		list, err := c.List(metav1.ListOptions{})
-		if err != nil {
-			if errors.IsNotFound(err) || errors.IsForbidden(err) {
-				glog.V(5).Infof("MutatingWebhookConfiguration are disabled due to an error: %v", err)
-				return nil, ErrDisabled
-			}
-			return nil, err
-		}
-		return mergeMutatingWebhookConfigurations(list), nil
+func NewMutatingWebhookConfigurationManager(informer admissionregistrationinformers.MutatingWebhookConfigurationInformer) *MutatingWebhookConfigurationManager {
+	manager := &MutatingWebhookConfigurationManager{
+		configuration: &atomic.Value{},
+		lister:        informer.Lister(),
 	}
 
-	return &MutatingWebhookConfigurationManager{
-		newPoller(getFn),
-	}
+	// Start with an empty list
+	manager.configuration.Store(&v1beta1.MutatingWebhookConfiguration{})
+
+	// On any change, rebuild the config
+	informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc:    func(_ interface{}) { manager.updateConfiguration() },
+		UpdateFunc: func(_, _ interface{}) { manager.updateConfiguration() },
+		DeleteFunc: func(_ interface{}) { manager.updateConfiguration() },
+	})
+
+	return manager
 }
 
 // Webhooks returns the merged MutatingWebhookConfiguration.
-func (im *MutatingWebhookConfigurationManager) Webhooks() (*v1beta1.MutatingWebhookConfiguration, error) {
-	configuration, err := im.poller.configuration()
+func (m *MutatingWebhookConfigurationManager) Webhooks() *v1beta1.MutatingWebhookConfiguration {
+	return m.configuration.Load().(*v1beta1.MutatingWebhookConfiguration)
+}
+
+func (m *MutatingWebhookConfigurationManager) updateConfiguration() {
+	configurations, err := m.lister.List(labels.Everything())
 	if err != nil {
-		return nil, err
+		utilruntime.HandleError(fmt.Errorf("error updating configuration: %v", err))
+		return
 	}
-	mutatingWebhookConfiguration, ok := configuration.(*v1beta1.MutatingWebhookConfiguration)
-	if !ok {
-		return nil, fmt.Errorf("expected type %v, got type %v", reflect.TypeOf(mutatingWebhookConfiguration), reflect.TypeOf(configuration))
-	}
-	return mutatingWebhookConfiguration, nil
+	m.configuration.Store(mergeMutatingWebhookConfigurations(configurations))
 }
 
-func (im *MutatingWebhookConfigurationManager) Run(stopCh <-chan struct{}) {
-	im.poller.Run(stopCh)
-}
-
-func mergeMutatingWebhookConfigurations(
-	list *v1beta1.MutatingWebhookConfigurationList,
-) *v1beta1.MutatingWebhookConfiguration {
-	configurations := append([]v1beta1.MutatingWebhookConfiguration{}, list.Items...)
+func mergeMutatingWebhookConfigurations(configurations []*v1beta1.MutatingWebhookConfiguration) *v1beta1.MutatingWebhookConfiguration {
 	var ret v1beta1.MutatingWebhookConfiguration
 	// The internal order of webhooks for each configuration is provided by the user
 	// but configurations themselves can be in any order. As we are going to run these
 	// webhooks in serial, they are sorted here to have a deterministic order.
-	sort.Sort(byName(configurations))
+	sort.SliceStable(configurations, MutatingWebhookConfigurationSorter(configurations).ByName)
 	for _, c := range configurations {
 		ret.Webhooks = append(ret.Webhooks, c.Webhooks...)
 	}
 	return &ret
 }
 
-// byName sorts MutatingWebhookConfiguration by name. These objects are all in
-// cluster namespace (aka no namespace) thus they all have unique names.
-type byName []v1beta1.MutatingWebhookConfiguration
+type MutatingWebhookConfigurationSorter []*v1beta1.MutatingWebhookConfiguration
 
-func (x byName) Len() int { return len(x) }
-
-func (x byName) Swap(i, j int) { x[i], x[j] = x[j], x[i] }
-
-func (x byName) Less(i, j int) bool {
-	return x[i].ObjectMeta.Name < x[j].ObjectMeta.Name
+func (a MutatingWebhookConfigurationSorter) ByName(i, j int) bool {
+	return a[i].Name < a[j].Name
 }

staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager_test.go

@@ -17,24 +17,107 @@ limitations under the License.
 package configuration
 
 import (
+	"fmt"
+	"reflect"
 	"testing"
+	"time"
 
 	"k8s.io/api/admissionregistration/v1beta1"
-	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/labels"
+	admissionregistrationlisters "k8s.io/client-go/listers/admissionregistration/v1beta1"
+	"k8s.io/client-go/tools/cache"
 )
 
-type disabledMutatingWebhookConfigLister struct{}
-
-func (l *disabledMutatingWebhookConfigLister) List(options metav1.ListOptions) (*v1beta1.MutatingWebhookConfigurationList, error) {
-	return nil, errors.NewNotFound(schema.GroupResource{Group: "admissionregistration", Resource: "MutatingWebhookConfigurations"}, "")
+type fakeMutatingWebhookConfigSharedInformer struct {
+	informer *fakeMutatingWebhookConfigInformer
+	lister   *fakeMutatingWebhookConfigLister
 }
-func TestMutatingWebhookConfigDisabled(t *testing.T) {
-	manager := NewMutatingWebhookConfigurationManager(&disabledMutatingWebhookConfigLister{})
-	manager.sync()
-	_, err := manager.Webhooks()
-	if err.Error() != ErrDisabled.Error() {
-		t.Errorf("expected %v, got %v", ErrDisabled, err)
+
+func (f *fakeMutatingWebhookConfigSharedInformer) Informer() cache.SharedIndexInformer {
+	return f.informer
+}
+func (f *fakeMutatingWebhookConfigSharedInformer) Lister() admissionregistrationlisters.MutatingWebhookConfigurationLister {
+	return f.lister
+}
+
+type fakeMutatingWebhookConfigInformer struct {
+	eventHandler cache.ResourceEventHandler
+}
+
+func (f *fakeMutatingWebhookConfigInformer) AddEventHandler(handler cache.ResourceEventHandler) {
+	fmt.Println("added handler")
+	f.eventHandler = handler
+}
+func (f *fakeMutatingWebhookConfigInformer) AddEventHandlerWithResyncPeriod(handler cache.ResourceEventHandler, resyncPeriod time.Duration) {
+	panic("unsupported")
+}
+func (f *fakeMutatingWebhookConfigInformer) GetStore() cache.Store {
+	panic("unsupported")
+}
+func (f *fakeMutatingWebhookConfigInformer) GetController() cache.Controller {
+	panic("unsupported")
+}
+func (f *fakeMutatingWebhookConfigInformer) Run(stopCh <-chan struct{}) {
+	panic("unsupported")
+}
+func (f *fakeMutatingWebhookConfigInformer) HasSynced() bool {
+	panic("unsupported")
+}
+func (f *fakeMutatingWebhookConfigInformer) LastSyncResourceVersion() string {
+	panic("unsupported")
+}
+func (f *fakeMutatingWebhookConfigInformer) AddIndexers(indexers cache.Indexers) error {
+	panic("unsupported")
+}
+func (f *fakeMutatingWebhookConfigInformer) GetIndexer() cache.Indexer { panic("unsupported") }
+
+type fakeMutatingWebhookConfigLister struct {
+	list []*v1beta1.MutatingWebhookConfiguration
+	err  error
+}
+
+func (f *fakeMutatingWebhookConfigLister) List(selector labels.Selector) (ret []*v1beta1.MutatingWebhookConfiguration, err error) {
+	return f.list, f.err
+}
+
+func (f *fakeMutatingWebhookConfigLister) Get(name string) (*v1beta1.MutatingWebhookConfiguration, error) {
+	panic("unsupported")
+}
+
+func TestGetMutatingWebhookConfig(t *testing.T) {
+	informer := &fakeMutatingWebhookConfigSharedInformer{
+		informer: &fakeMutatingWebhookConfigInformer{},
+		lister:   &fakeMutatingWebhookConfigLister{},
+	}
+
+	// no configurations
+	informer.lister.list = nil
+	manager := NewMutatingWebhookConfigurationManager(informer)
+	if configurations := manager.Webhooks(); len(configurations.Webhooks) != 0 {
+		t.Errorf("expected empty webhooks, but got %v", configurations.Webhooks)
+	}
+
+	// list err
+	webhookConfiguration := &v1beta1.MutatingWebhookConfiguration{
+		ObjectMeta: metav1.ObjectMeta{Name: "webhook1"},
+		Webhooks:   []v1beta1.Webhook{{Name: "webhook1.1"}},
+	}
+	informer.lister.list = []*v1beta1.MutatingWebhookConfiguration{webhookConfiguration.DeepCopy()}
+	informer.lister.err = fmt.Errorf("mutating webhook configuration list error")
+	informer.informer.eventHandler.OnAdd(webhookConfiguration.DeepCopy())
+	if configurations := manager.Webhooks(); len(configurations.Webhooks) != 0 {
+		t.Errorf("expected empty webhooks, but got %v", configurations.Webhooks)
+	}
+
+	// configuration populated
+	informer.lister.err = nil
+	informer.informer.eventHandler.OnAdd(webhookConfiguration.DeepCopy())
+	configurations := manager.Webhooks()
+	if len(configurations.Webhooks) == 0 {
+		t.Errorf("expected non empty webhooks")
+	}
+	if !reflect.DeepEqual(configurations.Webhooks, webhookConfiguration.Webhooks) {
+		t.Errorf("Expected\n%#v\ngot\n%#v", webhookConfiguration.Webhooks, configurations.Webhooks)
 	}
 }

staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go

@@ -18,67 +18,69 @@ package configuration
 
 import (
 	"fmt"
-	"reflect"
-
-	"github.com/golang/glog"
+	"sort"
+	"sync/atomic"
 
 	"k8s.io/api/admissionregistration/v1beta1"
-	"k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/labels"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	admissionregistrationinformers "k8s.io/client-go/informers/admissionregistration/v1beta1"
+	admissionregistrationlisters "k8s.io/client-go/listers/admissionregistration/v1beta1"
+	"k8s.io/client-go/tools/cache"
 )
 
-type ValidatingWebhookConfigurationLister interface {
-	List(opts metav1.ListOptions) (*v1beta1.ValidatingWebhookConfigurationList, error)
-}
-
 // ValidatingWebhookConfigurationManager collects the validating webhook objects so that they can be called.
 type ValidatingWebhookConfigurationManager struct {
-	*poller
+	configuration *atomic.Value
+	lister        admissionregistrationlisters.ValidatingWebhookConfigurationLister
 }
 
-func NewValidatingWebhookConfigurationManager(c ValidatingWebhookConfigurationLister) *ValidatingWebhookConfigurationManager {
-	getFn := func() (runtime.Object, error) {
-		list, err := c.List(metav1.ListOptions{})
-		if err != nil {
-			if errors.IsNotFound(err) || errors.IsForbidden(err) {
-				glog.V(5).Infof("ValidatingWebhookConfiguration are disabled due to an error: %v", err)
-				return nil, ErrDisabled
-			}
-			return nil, err
-		}
-		return mergeValidatingWebhookConfigurations(list), nil
+func NewValidatingWebhookConfigurationManager(informer admissionregistrationinformers.ValidatingWebhookConfigurationInformer) *ValidatingWebhookConfigurationManager {
+	manager := &ValidatingWebhookConfigurationManager{
+		configuration: &atomic.Value{},
+		lister:        informer.Lister(),
 	}
 
-	return &ValidatingWebhookConfigurationManager{
-		newPoller(getFn),
-	}
+	// Start with an empty list
+	manager.configuration.Store(&v1beta1.ValidatingWebhookConfiguration{})
+
+	// On any change, rebuild the config
+	informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc:    func(_ interface{}) { manager.updateConfiguration() },
+		UpdateFunc: func(_, _ interface{}) { manager.updateConfiguration() },
+		DeleteFunc: func(_ interface{}) { manager.updateConfiguration() },
+	})
+
+	return manager
 }
 
 // Webhooks returns the merged ValidatingWebhookConfiguration.
-func (im *ValidatingWebhookConfigurationManager) Webhooks() (*v1beta1.ValidatingWebhookConfiguration, error) {
-	configuration, err := im.poller.configuration()
-	if err != nil {
-		return nil, err
-	}
-	validatingWebhookConfiguration, ok := configuration.(*v1beta1.ValidatingWebhookConfiguration)
-	if !ok {
-		return nil, fmt.Errorf("expected type %v, got type %v", reflect.TypeOf(validatingWebhookConfiguration), reflect.TypeOf(configuration))
-	}
-	return validatingWebhookConfiguration, nil
+func (v *ValidatingWebhookConfigurationManager) Webhooks() *v1beta1.ValidatingWebhookConfiguration {
+	return v.configuration.Load().(*v1beta1.ValidatingWebhookConfiguration)
 }
 
-func (im *ValidatingWebhookConfigurationManager) Run(stopCh <-chan struct{}) {
-	im.poller.Run(stopCh)
+func (v *ValidatingWebhookConfigurationManager) updateConfiguration() {
+	configurations, err := v.lister.List(labels.Everything())
+	if err != nil {
+		utilruntime.HandleError(fmt.Errorf("error updating configuration: %v", err))
+		return
+	}
+	v.configuration.Store(mergeValidatingWebhookConfigurations(configurations))
 }
 
 func mergeValidatingWebhookConfigurations(
-	list *v1beta1.ValidatingWebhookConfigurationList,
+	configurations []*v1beta1.ValidatingWebhookConfiguration,
 ) *v1beta1.ValidatingWebhookConfiguration {
-	configurations := list.Items
+	sort.SliceStable(configurations, ValidatingWebhookConfigurationSorter(configurations).ByName)
 	var ret v1beta1.ValidatingWebhookConfiguration
 	for _, c := range configurations {
 		ret.Webhooks = append(ret.Webhooks, c.Webhooks...)
 	}
 	return &ret
 }
+
+type ValidatingWebhookConfigurationSorter []*v1beta1.ValidatingWebhookConfiguration
+
+func (a ValidatingWebhookConfigurationSorter) ByName(i, j int) bool {
+	return a[i].Name < a[j].Name
+}

staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go

@@ -17,24 +17,107 @@ limitations under the License.
 package configuration
 
 import (
+	"fmt"
+	"reflect"
 	"testing"
+	"time"
 
 	"k8s.io/api/admissionregistration/v1beta1"
-	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/labels"
+	admissionregistrationlisters "k8s.io/client-go/listers/admissionregistration/v1beta1"
+	"k8s.io/client-go/tools/cache"
 )
 
-type disabledValidatingWebhookConfigLister struct{}
-
-func (l *disabledValidatingWebhookConfigLister) List(options metav1.ListOptions) (*v1beta1.ValidatingWebhookConfigurationList, error) {
-	return nil, errors.NewNotFound(schema.GroupResource{Group: "admissionregistration", Resource: "ValidatingWebhookConfigurations"}, "")
+type fakeValidatingWebhookConfigSharedInformer struct {
+	informer *fakeValidatingWebhookConfigInformer
+	lister   *fakeValidatingWebhookConfigLister
 }
-func TestWebhookConfigDisabled(t *testing.T) {
-	manager := NewValidatingWebhookConfigurationManager(&disabledValidatingWebhookConfigLister{})
-	manager.sync()
-	_, err := manager.Webhooks()
-	if err.Error() != ErrDisabled.Error() {
-		t.Errorf("expected %v, got %v", ErrDisabled, err)
+
+func (f *fakeValidatingWebhookConfigSharedInformer) Informer() cache.SharedIndexInformer {
+	return f.informer
+}
+func (f *fakeValidatingWebhookConfigSharedInformer) Lister() admissionregistrationlisters.ValidatingWebhookConfigurationLister {
+	return f.lister
+}
+
+type fakeValidatingWebhookConfigInformer struct {
+	eventHandler cache.ResourceEventHandler
+}
+
+func (f *fakeValidatingWebhookConfigInformer) AddEventHandler(handler cache.ResourceEventHandler) {
+	fmt.Println("added handler")
+	f.eventHandler = handler
+}
+func (f *fakeValidatingWebhookConfigInformer) AddEventHandlerWithResyncPeriod(handler cache.ResourceEventHandler, resyncPeriod time.Duration) {
+	panic("unsupported")
+}
+func (f *fakeValidatingWebhookConfigInformer) GetStore() cache.Store {
+	panic("unsupported")
+}
+func (f *fakeValidatingWebhookConfigInformer) GetController() cache.Controller {
+	panic("unsupported")
+}
+func (f *fakeValidatingWebhookConfigInformer) Run(stopCh <-chan struct{}) {
+	panic("unsupported")
+}
+func (f *fakeValidatingWebhookConfigInformer) HasSynced() bool {
+	panic("unsupported")
+}
+func (f *fakeValidatingWebhookConfigInformer) LastSyncResourceVersion() string {
+	panic("unsupported")
+}
+func (f *fakeValidatingWebhookConfigInformer) AddIndexers(indexers cache.Indexers) error {
+	panic("unsupported")
+}
+func (f *fakeValidatingWebhookConfigInformer) GetIndexer() cache.Indexer { panic("unsupported") }
+
+type fakeValidatingWebhookConfigLister struct {
+	list []*v1beta1.ValidatingWebhookConfiguration
+	err  error
+}
+
+func (f *fakeValidatingWebhookConfigLister) List(selector labels.Selector) (ret []*v1beta1.ValidatingWebhookConfiguration, err error) {
+	return f.list, f.err
+}
+
+func (f *fakeValidatingWebhookConfigLister) Get(name string) (*v1beta1.ValidatingWebhookConfiguration, error) {
+	panic("unsupported")
+}
+
+func TestGettValidatingWebhookConfig(t *testing.T) {
+	informer := &fakeValidatingWebhookConfigSharedInformer{
+		informer: &fakeValidatingWebhookConfigInformer{},
+		lister:   &fakeValidatingWebhookConfigLister{},
+	}
+
+	// no configurations
+	informer.lister.list = nil
+	manager := NewValidatingWebhookConfigurationManager(informer)
+	if configurations := manager.Webhooks(); len(configurations.Webhooks) != 0 {
+		t.Errorf("expected empty webhooks, but got %v", configurations.Webhooks)
+	}
+
+	// list error
+	webhookConfiguration := &v1beta1.ValidatingWebhookConfiguration{
+		ObjectMeta: metav1.ObjectMeta{Name: "webhook1"},
+		Webhooks:   []v1beta1.Webhook{{Name: "webhook1.1"}},
+	}
+	informer.lister.list = []*v1beta1.ValidatingWebhookConfiguration{webhookConfiguration.DeepCopy()}
+	informer.lister.err = fmt.Errorf("validating webhook configuration list error")
+	informer.informer.eventHandler.OnAdd(webhookConfiguration.DeepCopy())
+	if configurations := manager.Webhooks(); len(configurations.Webhooks) != 0 {
+		t.Errorf("expected empty webhooks, but got %v", configurations.Webhooks)
+	}
+
+	// configuration populated
+	informer.lister.err = nil
+	informer.informer.eventHandler.OnAdd(webhookConfiguration.DeepCopy())
+	configurations := manager.Webhooks()
+	if len(configurations.Webhooks) == 0 {
+		t.Errorf("expected non empty webhooks")
+	}
+	if !reflect.DeepEqual(configurations.Webhooks, webhookConfiguration.Webhooks) {
+		t.Errorf("Expected\n%#v\ngot\n%#v", webhookConfiguration.Webhooks, configurations.Webhooks)
 	}
 }

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD

@@ -14,12 +14,10 @@ go_library(
         "//vendor/k8s.io/api/admission/v1beta1:go_default_library",
         "//vendor/k8s.io/api/admissionregistration/v1beta1: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/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/configuration:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library",

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go

@@ -30,12 +30,10 @@ import (
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	"k8s.io/api/admissionregistration/v1beta1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/runtime/serializer/json"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/configuration"
 	genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer"
@@ -69,8 +67,7 @@ func Register(plugins *admission.Plugins) {
 
 // WebhookSource can list dynamic webhook plugins.
 type WebhookSource interface {
-	Run(stopCh <-chan struct{})
-	Webhooks() (*v1beta1.MutatingWebhookConfiguration, error)
+	Webhooks() *v1beta1.MutatingWebhookConfiguration
 }
 
 // NewMutatingWebhook returns a generic admission webhook plugin.
@@ -146,14 +143,17 @@ func (a *MutatingWebhook) SetScheme(scheme *runtime.Scheme) {
 // WantsExternalKubeClientSet defines a function which sets external ClientSet for admission plugins that need it
 func (a *MutatingWebhook) SetExternalKubeClientSet(client clientset.Interface) {
 	a.namespaceMatcher.Client = client
-	a.hookSource = configuration.NewMutatingWebhookConfigurationManager(client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations())
 }
 
 // SetExternalKubeInformerFactory implements the WantsExternalKubeInformerFactory interface.
 func (a *MutatingWebhook) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) {
 	namespaceInformer := f.Core().V1().Namespaces()
 	a.namespaceMatcher.NamespaceLister = namespaceInformer.Lister()
-	a.SetReadyFunc(namespaceInformer.Informer().HasSynced)
+	mutatingWebhookConfigurationsInformer := f.Admissionregistration().V1beta1().MutatingWebhookConfigurations()
+	a.hookSource = configuration.NewMutatingWebhookConfigurationManager(mutatingWebhookConfigurationsInformer)
+	a.SetReadyFunc(func() bool {
+		return namespaceInformer.Informer().HasSynced() && mutatingWebhookConfigurationsInformer.Informer().HasSynced()
+	})
 }
 
 // ValidateInitialization implements the InitializationValidator interface.
@@ -176,35 +176,21 @@ func (a *MutatingWebhook) ValidateInitialization() error {
 	if a.defaulter == nil {
 		return fmt.Errorf("MutatingWebhook.defaulter is not properly setup")
 	}
-	go a.hookSource.Run(wait.NeverStop)
 	return nil
 }
 
-func (a *MutatingWebhook) loadConfiguration(attr admission.Attributes) (*v1beta1.MutatingWebhookConfiguration, error) {
-	hookConfig, err := a.hookSource.Webhooks()
-	// if Webhook configuration is disabled, fail open
-	if err == configuration.ErrDisabled {
-		return &v1beta1.MutatingWebhookConfiguration{}, nil
-	}
-	if err != nil {
-		e := apierrors.NewServerTimeout(attr.GetResource().GroupResource(), string(attr.GetOperation()), 1)
-		e.ErrStatus.Message = fmt.Sprintf("Unable to refresh the Webhook configuration: %v", err)
-		e.ErrStatus.Reason = "LoadingConfiguration"
-		e.ErrStatus.Details.Causes = append(e.ErrStatus.Details.Causes, metav1.StatusCause{
-			Type:    "MutatingWebhookConfigurationFailure",
-			Message: "An error has occurred while refreshing the MutatingWebhook configuration, no resources can be created/updated/deleted/connected until a refresh succeeds.",
-		})
-		return nil, e
-	}
-	return hookConfig, nil
+func (a *MutatingWebhook) loadConfiguration(attr admission.Attributes) *v1beta1.MutatingWebhookConfiguration {
+	hookConfig := a.hookSource.Webhooks()
+	return hookConfig
 }
 
 // Admit makes an admission decision based on the request attributes.
 func (a *MutatingWebhook) Admit(attr admission.Attributes) error {
-	hookConfig, err := a.loadConfiguration(attr)
-	if err != nil {
-		return err
+	if !a.WaitForReady() {
+		return admission.NewForbidden(attr, fmt.Errorf("not yet ready to handle request"))
 	}
+
+	hookConfig := a.loadConfiguration(attr)
 	hooks := hookConfig.Webhooks
 	ctx := context.TODO()
 

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission_test.go

@@ -47,16 +47,16 @@ type fakeHookSource struct {
 	err   error
 }
 
-func (f *fakeHookSource) Webhooks() (*registrationv1beta1.MutatingWebhookConfiguration, error) {
+func (f *fakeHookSource) Webhooks() *registrationv1beta1.MutatingWebhookConfiguration {
 	if f.err != nil {
-		return nil, f.err
+		return nil
 	}
 	for i, h := range f.hooks {
 		if h.NamespaceSelector == nil {
 			f.hooks[i].NamespaceSelector = &metav1.LabelSelector{}
 		}
 	}
-	return &registrationv1beta1.MutatingWebhookConfiguration{Webhooks: f.hooks}, nil
+	return &registrationv1beta1.MutatingWebhookConfiguration{Webhooks: f.hooks}
 }
 
 func (f *fakeHookSource) Run(stopCh <-chan struct{}) {}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD

@@ -13,11 +13,9 @@ go_library(
         "//vendor/k8s.io/api/admission/v1beta1:go_default_library",
         "//vendor/k8s.io/api/admissionregistration/v1beta1: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/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/configuration:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library",

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go

@@ -30,11 +30,9 @@ import (
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	"k8s.io/api/admissionregistration/v1beta1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/configuration"
 	genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer"
@@ -68,8 +66,7 @@ func Register(plugins *admission.Plugins) {
 
 // WebhookSource can list dynamic webhook plugins.
 type WebhookSource interface {
-	Run(stopCh <-chan struct{})
-	Webhooks() (*v1beta1.ValidatingWebhookConfiguration, error)
+	Webhooks() *v1beta1.ValidatingWebhookConfiguration
 }
 
 // NewValidatingAdmissionWebhook returns a generic admission webhook plugin.
@@ -141,20 +138,23 @@ func (a *ValidatingAdmissionWebhook) SetScheme(scheme *runtime.Scheme) {
 // WantsExternalKubeClientSet defines a function which sets external ClientSet for admission plugins that need it
 func (a *ValidatingAdmissionWebhook) SetExternalKubeClientSet(client clientset.Interface) {
 	a.namespaceMatcher.Client = client
-	a.hookSource = configuration.NewValidatingWebhookConfigurationManager(client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations())
 }
 
 // SetExternalKubeInformerFactory implements the WantsExternalKubeInformerFactory interface.
 func (a *ValidatingAdmissionWebhook) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) {
 	namespaceInformer := f.Core().V1().Namespaces()
 	a.namespaceMatcher.NamespaceLister = namespaceInformer.Lister()
-	a.SetReadyFunc(namespaceInformer.Informer().HasSynced)
+	validatingWebhookConfigurationsInformer := f.Admissionregistration().V1beta1().ValidatingWebhookConfigurations()
+	a.hookSource = configuration.NewValidatingWebhookConfigurationManager(validatingWebhookConfigurationsInformer)
+	a.SetReadyFunc(func() bool {
+		return namespaceInformer.Informer().HasSynced() && validatingWebhookConfigurationsInformer.Informer().HasSynced()
+	})
 }
 
 // ValidateInitialization implements the InitializationValidator interface.
 func (a *ValidatingAdmissionWebhook) ValidateInitialization() error {
 	if a.hookSource == nil {
-		return fmt.Errorf("ValidatingAdmissionWebhook admission plugin requires a Kubernetes client to be provided")
+		return fmt.Errorf("ValidatingAdmissionWebhook admission plugin requires a Kubernetes informer to be provided")
 	}
 	if err := a.namespaceMatcher.Validate(); err != nil {
 		return fmt.Errorf("ValidatingAdmissionWebhook.namespaceMatcher is not properly setup: %v", err)
@@ -165,35 +165,19 @@ func (a *ValidatingAdmissionWebhook) ValidateInitialization() error {
 	if err := a.convertor.Validate(); err != nil {
 		return fmt.Errorf("ValidatingAdmissionWebhook.convertor is not properly setup: %v", err)
 	}
-	go a.hookSource.Run(wait.NeverStop)
 	return nil
 }
 
-func (a *ValidatingAdmissionWebhook) loadConfiguration(attr admission.Attributes) (*v1beta1.ValidatingWebhookConfiguration, error) {
-	hookConfig, err := a.hookSource.Webhooks()
-	// if Webhook configuration is disabled, fail open
-	if err == configuration.ErrDisabled {
-		return &v1beta1.ValidatingWebhookConfiguration{}, nil
-	}
-	if err != nil {
-		e := apierrors.NewServerTimeout(attr.GetResource().GroupResource(), string(attr.GetOperation()), 1)
-		e.ErrStatus.Message = fmt.Sprintf("Unable to refresh the Webhook configuration: %v", err)
-		e.ErrStatus.Reason = "LoadingConfiguration"
-		e.ErrStatus.Details.Causes = append(e.ErrStatus.Details.Causes, metav1.StatusCause{
-			Type:    "ValidatingWebhookConfigurationFailure",
-			Message: "An error has occurred while refreshing the ValidatingWebhook configuration, no resources can be created/updated/deleted/connected until a refresh succeeds.",
-		})
-		return nil, e
-	}
-	return hookConfig, nil
+func (a *ValidatingAdmissionWebhook) loadConfiguration(attr admission.Attributes) *v1beta1.ValidatingWebhookConfiguration {
+	return a.hookSource.Webhooks()
 }
 
 // Validate makes an admission decision based on the request attributes.
 func (a *ValidatingAdmissionWebhook) Validate(attr admission.Attributes) error {
-	hookConfig, err := a.loadConfiguration(attr)
-	if err != nil {
-		return err
+	if !a.WaitForReady() {
+		return admission.NewForbidden(attr, fmt.Errorf("not yet ready to handle request"))
 	}
+	hookConfig := a.loadConfiguration(attr)
 	hooks := hookConfig.Webhooks
 	ctx := context.TODO()
 

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission_test.go

@@ -47,16 +47,16 @@ type fakeHookSource struct {
 	err   error
 }
 
-func (f *fakeHookSource) Webhooks() (*registrationv1beta1.ValidatingWebhookConfiguration, error) {
+func (f *fakeHookSource) Webhooks() *registrationv1beta1.ValidatingWebhookConfiguration {
 	if f.err != nil {
-		return nil, f.err
+		return nil
 	}
 	for i, h := range f.hooks {
 		if h.NamespaceSelector == nil {
 			f.hooks[i].NamespaceSelector = &metav1.LabelSelector{}
 		}
 	}
-	return &registrationv1beta1.ValidatingWebhookConfiguration{Webhooks: f.hooks}, nil
+	return &registrationv1beta1.ValidatingWebhookConfiguration{Webhooks: f.hooks}
 }
 
 func (f *fakeHookSource) Run(stopCh <-chan struct{}) {}