github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-09 12:07:47 +00:00
Code Issues Projects Releases Wiki Activity

Fix deadlock issue

Browse Source 
This patch fixes the deadlock issue by using a map to cache already
initiated Webhooks instead of using `needRefresh` map.
This commit is contained in:
Amine 2023-05-10 15:09:15 -05:00
parent 99875b3fb7
commit c6f36e8702
1 changed files with 15 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -34,10 +34,10 @@ import (
// validatingWebhookConfigurationManager collects the validating webhook objects so that they can be called. // validatingWebhookConfigurationManager collects the validating webhook objects so that they can be called.
type validatingWebhookConfigurationManager struct { type validatingWebhookConfigurationManager struct {
lister admissionregistrationlisters.ValidatingWebhookConfigurationLister lister admissionregistrationlisters.ValidatingWebhookConfigurationLister
hasSynced func() bool hasSynced func() bool
lazy synctrack.Lazy[[]webhook.WebhookAccessor] lazy synctrack.Lazy[[]webhook.WebhookAccessor]
needRefresh sync.Map // NOTE maybe use simple map and a sync.Mutex configurationsCache sync.Map // NOTE maybe use simple map and a sync.Mutex
} }
var _ generic.Source = &validatingWebhookConfigurationManager{} var _ generic.Source = &validatingWebhookConfigurationManager{}
@ -57,12 +57,12 @@ func NewValidatingWebhookConfigurationManager(f informers.SharedInformerFactory)
// etc... thinking about using reflect.. // etc... thinking about using reflect..
obj := new.(*v1.ValidatingWebhookConfiguration) obj := new.(*v1.ValidatingWebhookConfiguration)
// lock R+W // lock R+W
manager.needRefresh.Store(obj.GetName(), true) manager.configurationsCache.Delete(obj.GetName())
manager.lazy.Notify() manager.lazy.Notify()
}, },
DeleteFunc: func(vwc interface{}) { DeleteFunc: func(vwc interface{}) {
obj := vwc.(*v1.ValidatingWebhookConfiguration) obj := vwc.(*v1.ValidatingWebhookConfiguration)
manager.needRefresh.Delete(obj.Name) manager.configurationsCache.Delete(obj.Name)
manager.lazy.Notify() manager.lazy.Notify()
}, },
}) })
@ -71,11 +71,6 @@ func NewValidatingWebhookConfigurationManager(f informers.SharedInformerFactory)
return manager return manager
} }
func (v *validatingWebhookConfigurationManager) configurationNeedRefresh(namespacedName string) bool {
_, ok := v.needRefresh.Load(namespacedName)
return ok
}
// Webhooks returns the merged ValidatingWebhookConfiguration. // Webhooks returns the merged ValidatingWebhookConfiguration.
func (v *validatingWebhookConfigurationManager) Webhooks() []webhook.WebhookAccessor { func (v *validatingWebhookConfigurationManager) Webhooks() []webhook.WebhookAccessor {
out, err := v.lazy.Get() out, err := v.lazy.Get()
@ -100,18 +95,13 @@ func (v *validatingWebhookConfigurationManager) getConfiguration() ([]webhook.We
func (v *validatingWebhookConfigurationManager) smartReloadValidatingWebhookConfigurations(configurations []*v1.ValidatingWebhookConfiguration) []webhook.WebhookAccessor { func (v *validatingWebhookConfigurationManager) smartReloadValidatingWebhookConfigurations(configurations []*v1.ValidatingWebhookConfiguration) []webhook.WebhookAccessor {
sort.SliceStable(configurations, ValidatingWebhookConfigurationSorter(configurations).ByName) sort.SliceStable(configurations, ValidatingWebhookConfigurationSorter(configurations).ByName)
accessors := []webhook.WebhookAccessor{} accessors := []webhook.WebhookAccessor{}
cachedAccessors, _ := v.lazy.Get()
cfgLoop: cfgLoop:
for _, c := range configurations { for _, c := range configurations {
if !v.configurationNeedRefresh(c.Name) { cachedConfigurationAccessors, ok := v.configurationsCache.Load(c.Name)
if ok {
// Pick an already cached webhookAccessor // Pick an already cached webhookAccessor
for _, ca := range cachedAccessors { accessors = append(accessors, cachedConfigurationAccessors.([]webhook.WebhookAccessor)...)
if ca.GetName() == c.Name { continue cfgLoop
accessors = append(accessors, ca)
v.needRefresh.Delete(c.Name)
continue cfgLoop
}
}
} }
// webhook names are not validated for uniqueness, so we check for duplicates and // webhook names are not validated for uniqueness, so we check for duplicates and
@ -119,12 +109,16 @@ cfgLoop:
// //
// NOTE: In `pkg/apis/admissionregistration/validation.go` webhook names are checked // NOTE: In `pkg/apis/admissionregistration/validation.go` webhook names are checked
// for uniqueness now. Is it safe to remove this? // for uniqueness now. Is it safe to remove this?
configurationAccessors := []webhook.WebhookAccessor{}
names := map[string]int{} names := map[string]int{}
for i := range c.Webhooks { for i := range c.Webhooks {
n := c.Webhooks[i].Name n := c.Webhooks[i].Name
uid := fmt.Sprintf("%s/%s/%d", c.Name, n, names[n]) uid := fmt.Sprintf("%s/%s/%d", c.Name, n, names[n])
names[n]++ names[n]++
accessors = append(accessors, webhook.NewValidatingWebhookAccessor(uid, c.Name, &c.Webhooks[i])) configurationAccessors = append(configurationAccessors, webhook.NewValidatingWebhookAccessor(uid, c.Name, &c.Webhooks[i]))
// Cache the new accessors
v.configurationsCache.Store(c.Name, configurationAccessors)
accessors = append(accessors, configurationAccessors...)
} }
} }
return accessors return accessors