From 32d3918c373aa245b64358cc43f6e53a42899a07 Mon Sep 17 00:00:00 2001 From: Donnie Adams Date: Mon, 5 Feb 2024 10:06:02 -0500 Subject: [PATCH] fix: avoid concurrent map operations for valid owner changes For controllers that themselves start other controllers, concurrent maps operations can happen when using the "valid owner change" feature. This change will get rid of these concurrent map operations by using a sync.Map instead. Signed-off-by: Donnie Adams --- pkg/apply/apply.go | 5 +++-- pkg/apply/desiredset_process.go | 14 ++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/apply/apply.go b/pkg/apply/apply.go index c08bd6a..60fc504 100644 --- a/pkg/apply/apply.go +++ b/pkg/apply/apply.go @@ -3,6 +3,7 @@ package apply import ( "context" "fmt" + "sync" "k8s.io/apimachinery/pkg/runtime/schema" kclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -17,11 +18,11 @@ var ( // values that are allowed to change. The key is in the format of // "oldValue => newValue" // subcontext and the value is the old subcontext name. - validOwnerChange = map[string]bool{} + validOwnerChange = sync.Map{} ) func AddValidOwnerChange(oldSubcontext, newSubContext string) { - validOwnerChange[fmt.Sprintf("%s => %s", oldSubcontext, newSubContext)] = true + validOwnerChange.Store(fmt.Sprintf("%s => %s", oldSubcontext, newSubContext), struct{}{}) } type Apply interface { diff --git a/pkg/apply/desiredset_process.go b/pkg/apply/desiredset_process.go index 0bc6c55..80625d0 100644 --- a/pkg/apply/desiredset_process.go +++ b/pkg/apply/desiredset_process.go @@ -277,12 +277,14 @@ func (a *apply) process(debugID string, set labels.Selector, gvk schema.GroupVer func isAllowOwnerTransition(existingObj, newObj kclient.Object) bool { existingAnno := existingObj.GetAnnotations() newAnno := newObj.GetAnnotations() - return newAnno[LabelSubContext] != "" && - (existingAnno[LabelGVK] == "" || existingAnno[LabelGVK] == newAnno[LabelGVK]) && - (existingAnno[LabelNamespace] == "" || existingAnno[LabelNamespace] == newAnno[LabelNamespace]) && - (existingAnno[LabelName] == "" || existingAnno[LabelName] == newAnno[LabelName]) && - validOwnerChange[fmt.Sprintf("%s => %s", existingAnno[LabelSubContext], newAnno[LabelSubContext])] - + if newAnno[LabelSubContext] == "" || + (existingAnno[LabelGVK] != "" && existingAnno[LabelGVK] != newAnno[LabelGVK]) || + (existingAnno[LabelNamespace] != "" && existingAnno[LabelNamespace] != newAnno[LabelNamespace]) || + (existingAnno[LabelName] != "" && existingAnno[LabelName] != newAnno[LabelName]) { + return false + } + _, ok := validOwnerChange.Load(fmt.Sprintf("%s => %s", existingAnno[LabelSubContext], newAnno[LabelSubContext])) + return ok } // isAssigningSubContext is checking to see if an existing managed object