Remove check causing informers to miss notifications

Fix DeltaFIFO bug that caused the sync delta created by a relist
for object ID X to be dropped if the DeltaFIFO already stored a
Delete delta for X. This caused SharedIndexInformer to miss create
notifications. Also, add unit test to expose the bug.

Kubernetes-commit: 9e4be54895e8c0655408b4766cd78de17b6d5b7c
This commit is contained in:
matte21 2019-10-14 18:19:05 +02:00 committed by Kubernetes Publisher
parent bd505ee787
commit cc50c9b333
2 changed files with 27 additions and 14 deletions

View File

@ -295,13 +295,6 @@ func isDeletionDup(a, b *Delta) *Delta {
return b
}
// willObjectBeDeletedLocked returns true only if the last delta for the
// given object is Delete. Caller must lock first.
func (f *DeltaFIFO) willObjectBeDeletedLocked(id string) bool {
deltas := f.items[id]
return len(deltas) > 0 && deltas[len(deltas)-1].Type == Deleted
}
// queueActionLocked appends to the delta list for the object.
// Caller must lock first.
func (f *DeltaFIFO) queueActionLocked(actionType DeltaType, obj interface{}) error {
@ -310,13 +303,6 @@ func (f *DeltaFIFO) queueActionLocked(actionType DeltaType, obj interface{}) err
return KeyError{obj, err}
}
// If object is supposed to be deleted (last event is Deleted),
// then we should ignore Sync events, because it would result in
// recreation of this object.
if actionType == Sync && f.willObjectBeDeletedLocked(id) {
return nil
}
newDeltas := append(f.items[id], Delta{actionType, obj})
newDeltas = dedupDeltas(newDeltas)

View File

@ -85,6 +85,33 @@ func TestDeltaFIFO_basic(t *testing.T) {
}
}
// TestDeltaFIFO_replaceWithDeleteDeltaIn tests that a `Sync` delta for an
// object `O` with ID `X` is added when .Replace is called and `O` is among the
// replacement objects even if the DeltaFIFO already stores in terminal position
// a delta of type `Delete` for ID `X`. Not adding the `Sync` delta causes
// SharedIndexInformers to miss `O`'s create notification, see https://github.com/kubernetes/kubernetes/issues/83810
// for more details.
func TestDeltaFIFO_replaceWithDeleteDeltaIn(t *testing.T) {
oldObj := mkFifoObj("foo", 1)
newObj := mkFifoObj("foo", 2)
f := NewDeltaFIFO(testFifoObjectKeyFunc, keyLookupFunc(func() []testFifoObject {
return []testFifoObject{oldObj}
}))
f.Delete(oldObj)
f.Replace([]interface{}{newObj}, "")
actualDeltas := Pop(f)
expectedDeltas := Deltas{
Delta{Type: Deleted, Object: oldObj},
Delta{Type: Sync, Object: newObj},
}
if !reflect.DeepEqual(expectedDeltas, actualDeltas) {
t.Errorf("expected %#v, got %#v", expectedDeltas, actualDeltas)
}
}
func TestDeltaFIFO_requeueOnPop(t *testing.T) {
f := NewDeltaFIFO(testFifoObjectKeyFunc, nil)