Skip to content
This repository has been archived by the owner on Mar 16, 2024. It is now read-only.

Commit

Permalink
fix: avoid concurrent map operations for valid owner changes
Browse files Browse the repository at this point in the history
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 <donnie@acorn.io>
  • Loading branch information
thedadams committed Feb 5, 2024
1 parent 2a58ee7 commit 32d3918
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 8 deletions.
5 changes: 3 additions & 2 deletions pkg/apply/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package apply
import (
"context"
"fmt"
"sync"

"k8s.io/apimachinery/pkg/runtime/schema"
kclient "sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -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 {
Expand Down
14 changes: 8 additions & 6 deletions pkg/apply/desiredset_process.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 32d3918

Please sign in to comment.