Skip to content

Commit

Permalink
Fix PR comments
Browse files Browse the repository at this point in the history
Signed-off-by: Will Beason <willbeason@google.com>
  • Loading branch information
Will Beason committed Sep 24, 2021
1 parent 428eb04 commit 82f40fa
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 12 deletions.
10 changes: 10 additions & 0 deletions pkg/controller/mutators/core/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,16 @@ func TestReconcile(t *testing.T) {

g.Expect(c.Create(ctx, mFoo.DeepCopy())).NotTo(gomega.HaveOccurred())
g.Expect(c.Create(ctx, mBar1.DeepCopy())).NotTo(gomega.HaveOccurred())

g.Eventually(func() error {
err := podStatusMatches(ctx, c, pod, mFooID, hasStatusErrors(nil))
if err != nil {
return err
}

return podStatusMatches(ctx, c, pod, mBar1ID, hasStatusErrors(nil))
})

g.Expect(c.Create(ctx, mBar2.DeepCopy())).NotTo(gomega.HaveOccurred())

g.Eventually(func() error {
Expand Down
3 changes: 1 addition & 2 deletions pkg/controller/mutators/core/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
// previousConflicts records the conflicts this Mutator has with other mutators
// before making any changes.
previousConflicts := r.system.GetConflicts(id)
delete(previousConflicts, id)

if deleted {
// Either the mutator was deleted before we were able to process this request, or it has been marked for
Expand All @@ -134,12 +133,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
}

newConflicts := r.system.GetConflicts(id)
delete(newConflicts, id)

// diff is the set of mutators which either:
// 1) previously conflicted with mutationObj but do not after this change, or
// 2) now conflict with mutationObj but did not before this change.
diff := symmetricDifference(previousConflicts, newConflicts)
delete(diff, id)

// Now that we've made changes to the recorded Mutator schemas, we can re-check
// for conflicts.
Expand Down
2 changes: 1 addition & 1 deletion pkg/mutation/schema/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type node struct {
// of which is the passed id.
func (n *node) Add(id types.ID, path []parser.Node, terminalType parser.NodeType) IDSet {
if n.ReferencedBy == nil {
n.ReferencedBy = make(map[types.ID]bool)
n.ReferencedBy = make(IDSet)
}
// This node is referenced by the passed ID.
n.ReferencedBy[id] = true
Expand Down
12 changes: 6 additions & 6 deletions pkg/mutation/schema/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func TestNode_Add(t *testing.T) {
ip("object", "spec.name"),
},
add: ip("list", "spec[list: foo]"),
want: map[types.ID]bool{
want: IDSet{
id("object"): true,
id("list"): true,
},
Expand Down Expand Up @@ -103,7 +103,7 @@ func TestNode_Add(t *testing.T) {
ip("object", "spec.name"),
},
add: ip("list", "spec[name: foo]"),
want: map[types.ID]bool{
want: IDSet{
id("object"): true,
id("list"): true,
},
Expand All @@ -114,7 +114,7 @@ func TestNode_Add(t *testing.T) {
ip("list", "spec.containers[name: foo]"),
},
add: ipt("set", "spec.containers", Set),
want: map[types.ID]bool{
want: IDSet{
id("list"): true,
id("set"): true,
},
Expand All @@ -125,7 +125,7 @@ func TestNode_Add(t *testing.T) {
ip("object", "spec.containers.name"),
},
add: ipt("set", "spec.containers", Set),
want: map[types.ID]bool{
want: IDSet{
id("object"): true,
id("set"): true,
},
Expand All @@ -136,7 +136,7 @@ func TestNode_Add(t *testing.T) {
ip("list image", "spec[image: bar]"),
},
add: ip("list name", "spec[name: foo]"),
want: map[types.ID]bool{
want: IDSet{
id("list image"): true,
id("list name"): true,
},
Expand All @@ -148,7 +148,7 @@ func TestNode_Add(t *testing.T) {
ip("object-list", "spec.container[name: foo]"),
},
add: ip("list-object", "spec[container: foo].name"),
want: map[types.ID]bool{
want: IDSet{
id("object-object"): true,
id("object-list"): true,
id("list-object"): true,
Expand Down
6 changes: 3 additions & 3 deletions pkg/mutation/schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func New() *DB {
return &DB{
cachedMutators: make(map[types.ID]MutatorWithSchema),
schemas: make(map[schema.GroupVersionKind]*node),
conflicts: make(map[types.ID]bool),
conflicts: make(IDSet),
}
}

Expand Down Expand Up @@ -59,7 +59,7 @@ type DB struct {
// schemas are the per-GVK implicit schemas.
schemas map[schema.GroupVersionKind]*node

conflicts map[types.ID]bool
conflicts IDSet
}

// Upsert inserts or updates the given mutator.
Expand Down Expand Up @@ -186,7 +186,7 @@ func (db *DB) HasConflicts(id types.ID) bool {
return db.conflicts[id]
}

func (db *DB) GetConflicts(id types.ID) map[types.ID]bool {
func (db *DB) GetConflicts(id types.ID) IDSet {
db.mutex.RLock()
mutator, ok := db.cachedMutators[id]
db.mutex.RUnlock()
Expand Down

0 comments on commit 82f40fa

Please sign in to comment.