From 5378f759dd21cf7063080c7599c4f48748913311 Mon Sep 17 00:00:00 2001 From: ajanikow <12255597+ajanikow@users.noreply.github.com> Date: Fri, 19 Feb 2021 14:03:09 +0000 Subject: [PATCH] Remove deadlock in cache --- CHANGELOG.md | 1 + pkg/deployment/resources/inspector/pdbs.go | 17 +++++++++++++---- pkg/deployment/resources/inspector/pods.go | 17 +++++++++++++---- pkg/deployment/resources/inspector/pvcs.go | 17 +++++++++++++---- pkg/deployment/resources/inspector/sa.go | 17 +++++++++++++---- pkg/deployment/resources/inspector/secrets.go | 17 +++++++++++++---- pkg/deployment/resources/inspector/services.go | 17 +++++++++++++---- pkg/deployment/resources/inspector/sms.go | 18 +++++++++++++----- 8 files changed, 92 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42c9200e7..2babeade2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Fix AKS Volume Resize mode - Use cached status in member client creation - Remove failed DBServers +- Remove deadlock in internal cache ## [1.1.4](https://github.com/arangodb/kube-arangodb/tree/1.1.4) (2021-02-15) - Add support for spec.ClusterDomain to be able to use FQDN in ArangoDB cluster communication diff --git a/pkg/deployment/resources/inspector/pdbs.go b/pkg/deployment/resources/inspector/pdbs.go index bc078277a..7b93093b6 100644 --- a/pkg/deployment/resources/inspector/pdbs.go +++ b/pkg/deployment/resources/inspector/pdbs.go @@ -33,10 +33,7 @@ type PodDisruptionBudgetFilter func(podDisruptionBudget *policy.PodDisruptionBud type PodDisruptionBudgetAction func(podDisruptionBudget *policy.PodDisruptionBudget) error func (i *inspector) IteratePodDisruptionBudgets(action PodDisruptionBudgetAction, filters ...PodDisruptionBudgetFilter) error { - i.lock.Lock() - defer i.lock.Unlock() - - for _, podDisruptionBudget := range i.podDisruptionBudgets { + for _, podDisruptionBudget := range i.PodDisruptionBudgets() { if err := i.iteratePodDisruptionBudget(podDisruptionBudget, action, filters...); err != nil { return err } @@ -54,6 +51,18 @@ func (i *inspector) iteratePodDisruptionBudget(podDisruptionBudget *policy.PodDi return action(podDisruptionBudget) } +func (i *inspector) PodDisruptionBudgets() []*policy.PodDisruptionBudget { + i.lock.Lock() + defer i.lock.Unlock() + + var r []*policy.PodDisruptionBudget + for _, podDisruptionBudget := range i.podDisruptionBudgets { + r = append(r, podDisruptionBudget) + } + + return r +} + func (i *inspector) PodDisruptionBudget(name string) (*policy.PodDisruptionBudget, bool) { i.lock.Lock() defer i.lock.Unlock() diff --git a/pkg/deployment/resources/inspector/pods.go b/pkg/deployment/resources/inspector/pods.go index 518fd582b..4af869bb2 100644 --- a/pkg/deployment/resources/inspector/pods.go +++ b/pkg/deployment/resources/inspector/pods.go @@ -33,10 +33,7 @@ type PodFilter func(pod *core.Pod) bool type PodAction func(pod *core.Pod) error func (i *inspector) IteratePods(action PodAction, filters ...PodFilter) error { - i.lock.Lock() - defer i.lock.Unlock() - - for _, pod := range i.pods { + for _, pod := range i.Pods() { if err := i.iteratePod(pod, action, filters...); err != nil { return err } @@ -54,6 +51,18 @@ func (i *inspector) iteratePod(pod *core.Pod, action PodAction, filters ...PodFi return action(pod) } +func (i *inspector) Pods() []*core.Pod { + i.lock.Lock() + defer i.lock.Unlock() + + var r []*core.Pod + for _, pod := range i.pods { + r = append(r, pod) + } + + return r +} + func (i *inspector) Pod(name string) (*core.Pod, bool) { i.lock.Lock() defer i.lock.Unlock() diff --git a/pkg/deployment/resources/inspector/pvcs.go b/pkg/deployment/resources/inspector/pvcs.go index 511d4f196..5b14127f5 100644 --- a/pkg/deployment/resources/inspector/pvcs.go +++ b/pkg/deployment/resources/inspector/pvcs.go @@ -33,10 +33,7 @@ type PersistentVolumeClaimFilter func(pvc *core.PersistentVolumeClaim) bool type PersistentVolumeClaimAction func(pvc *core.PersistentVolumeClaim) error func (i *inspector) IteratePersistentVolumeClaims(action PersistentVolumeClaimAction, filters ...PersistentVolumeClaimFilter) error { - i.lock.Lock() - defer i.lock.Unlock() - - for _, pvc := range i.pvcs { + for _, pvc := range i.PersistentVolumeClaims() { if err := i.iteratePersistentVolumeClaim(pvc, action, filters...); err != nil { return err } @@ -54,6 +51,18 @@ func (i *inspector) iteratePersistentVolumeClaim(pvc *core.PersistentVolumeClaim return action(pvc) } +func (i *inspector) PersistentVolumeClaims() []*core.PersistentVolumeClaim { + i.lock.Lock() + defer i.lock.Unlock() + + var r []*core.PersistentVolumeClaim + for _, persistentVolumeClaim := range i.pvcs { + r = append(r, persistentVolumeClaim) + } + + return r +} + func (i *inspector) PersistentVolumeClaim(name string) (*core.PersistentVolumeClaim, bool) { i.lock.Lock() defer i.lock.Unlock() diff --git a/pkg/deployment/resources/inspector/sa.go b/pkg/deployment/resources/inspector/sa.go index 6cb01e2aa..495febfdf 100644 --- a/pkg/deployment/resources/inspector/sa.go +++ b/pkg/deployment/resources/inspector/sa.go @@ -33,10 +33,7 @@ type ServiceAccountFilter func(serviceAccount *core.ServiceAccount) bool type ServiceAccountAction func(serviceAccount *core.ServiceAccount) error func (i *inspector) IterateServiceAccounts(action ServiceAccountAction, filters ...ServiceAccountFilter) error { - i.lock.Lock() - defer i.lock.Unlock() - - for _, serviceAccount := range i.serviceAccounts { + for _, serviceAccount := range i.ServiceAccounts() { if err := i.iterateServiceAccount(serviceAccount, action, filters...); err != nil { return err } @@ -54,6 +51,18 @@ func (i *inspector) iterateServiceAccount(serviceAccount *core.ServiceAccount, a return action(serviceAccount) } +func (i *inspector) ServiceAccounts() []*core.ServiceAccount { + i.lock.Lock() + defer i.lock.Unlock() + + var r []*core.ServiceAccount + for _, serviceAccount := range i.serviceAccounts { + r = append(r, serviceAccount) + } + + return r +} + func (i *inspector) ServiceAccount(name string) (*core.ServiceAccount, bool) { i.lock.Lock() defer i.lock.Unlock() diff --git a/pkg/deployment/resources/inspector/secrets.go b/pkg/deployment/resources/inspector/secrets.go index e8f8c2c6e..d29582ec1 100644 --- a/pkg/deployment/resources/inspector/secrets.go +++ b/pkg/deployment/resources/inspector/secrets.go @@ -35,10 +35,7 @@ type SecretFilter func(pod *core.Secret) bool type SecretAction func(pod *core.Secret) error func (i *inspector) IterateSecrets(action SecretAction, filters ...SecretFilter) error { - i.lock.Lock() - defer i.lock.Unlock() - - for _, secret := range i.secrets { + for _, secret := range i.Secrets() { if err := i.iterateSecrets(secret, action, filters...); err != nil { return err } @@ -56,6 +53,18 @@ func (i *inspector) iterateSecrets(secret *core.Secret, action SecretAction, fil return action(secret) } +func (i *inspector) Secrets() []*core.Secret { + i.lock.Lock() + defer i.lock.Unlock() + + var r []*core.Secret + for _, secret := range i.secrets { + r = append(r, secret) + } + + return r +} + func (i *inspector) Secret(name string) (*core.Secret, bool) { i.lock.Lock() defer i.lock.Unlock() diff --git a/pkg/deployment/resources/inspector/services.go b/pkg/deployment/resources/inspector/services.go index 4a198932a..86067be3d 100644 --- a/pkg/deployment/resources/inspector/services.go +++ b/pkg/deployment/resources/inspector/services.go @@ -33,10 +33,7 @@ type ServiceFilter func(pod *core.Service) bool type ServiceAction func(pod *core.Service) error func (i *inspector) IterateServices(action ServiceAction, filters ...ServiceFilter) error { - i.lock.Lock() - defer i.lock.Unlock() - - for _, service := range i.services { + for _, service := range i.Services() { if err := i.iterateServices(service, action, filters...); err != nil { return err } @@ -54,6 +51,18 @@ func (i *inspector) iterateServices(service *core.Service, action ServiceAction, return action(service) } +func (i *inspector) Services() []*core.Service { + i.lock.Lock() + defer i.lock.Unlock() + + var r []*core.Service + for _, service := range i.services { + r = append(r, service) + } + + return r +} + func (i *inspector) Service(name string) (*core.Service, bool) { i.lock.Lock() defer i.lock.Unlock() diff --git a/pkg/deployment/resources/inspector/sms.go b/pkg/deployment/resources/inspector/sms.go index 3b0054332..5f9ac3327 100644 --- a/pkg/deployment/resources/inspector/sms.go +++ b/pkg/deployment/resources/inspector/sms.go @@ -26,7 +26,6 @@ import ( "github.com/arangodb/kube-arangodb/pkg/util/errors" monitoring "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1" monitoringClient "github.com/coreos/prometheus-operator/pkg/client/versioned/typed/monitoring/v1" - meta "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -34,10 +33,7 @@ type ServiceMonitorFilter func(serviceMonitor *monitoring.ServiceMonitor) bool type ServiceMonitorAction func(serviceMonitor *monitoring.ServiceMonitor) error func (i *inspector) IterateServiceMonitors(action ServiceMonitorAction, filters ...ServiceMonitorFilter) error { - i.lock.Lock() - defer i.lock.Unlock() - - for _, serviceMonitor := range i.serviceMonitors { + for _, serviceMonitor := range i.ServiceMonitors() { if err := i.iterateServiceMonitor(serviceMonitor, action, filters...); err != nil { return err } @@ -55,6 +51,18 @@ func (i *inspector) iterateServiceMonitor(serviceMonitor *monitoring.ServiceMoni return action(serviceMonitor) } +func (i *inspector) ServiceMonitors() []*monitoring.ServiceMonitor { + i.lock.Lock() + defer i.lock.Unlock() + + var r []*monitoring.ServiceMonitor + for _, sms := range i.serviceMonitors { + r = append(r, sms) + } + + return r +} + func (i *inspector) ServiceMonitor(name string) (*monitoring.ServiceMonitor, bool) { i.lock.Lock() defer i.lock.Unlock()