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 327c5a7
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 6 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
10 changes: 6 additions & 4 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] != "" &&
if 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])]

(existingAnno[LabelName] == "" || existingAnno[LabelName] == newAnno[LabelName]) {
return true
}
_, 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 327c5a7

Please sign in to comment.