[Bugfix] Remove deadlock in cache (#690)

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

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()
+	for _, podDisruptionBudget := range i.PodDisruptionBudgets() {
-	defer i.lock.Unlock()
-
-	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()

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()
+	for _, pod := range i.Pods() {
-	defer i.lock.Unlock()
-
-	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()

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()
+	for _, pvc := range i.PersistentVolumeClaims() {
-	defer i.lock.Unlock()
-
-	for _, pvc := range i.pvcs {
 		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()

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()
+	for _, serviceAccount := range i.ServiceAccounts() {
-	defer i.lock.Unlock()
-
-	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()

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()
+	for _, secret := range i.Secrets() {
-	defer i.lock.Unlock()
-
-	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()

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()
+	for _, service := range i.Services() {
-	defer i.lock.Unlock()
-
-	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()

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()
+	for _, serviceMonitor := range i.ServiceMonitors() {
-	defer i.lock.Unlock()
-
-	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()