From ab18f362c81c8c009fb0e56d92f47412d0f4811e Mon Sep 17 00:00:00 2001 From: Aniket Date: Wed, 30 Oct 2019 16:20:56 -0400 Subject: [PATCH 1/2] UPSTREAM 83911: Fix DeltaFIFO Replace method Fix DeltaFIFO Replace method to prevent SharedIndexInformers from missing notifications. --- .../client-go/tools/cache/delta_fifo.go | 14 ---------- .../client-go/tools/cache/delta_fifo_test.go | 27 +++++++++++++++++++ 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/vendor/k8s.io/client-go/tools/cache/delta_fifo.go b/vendor/k8s.io/client-go/tools/cache/delta_fifo.go index 1cd53b382..335c6ae10 100644 --- a/vendor/k8s.io/client-go/tools/cache/delta_fifo.go +++ b/vendor/k8s.io/client-go/tools/cache/delta_fifo.go @@ -348,13 +348,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 { @@ -363,13 +356,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) diff --git a/vendor/k8s.io/client-go/tools/cache/delta_fifo_test.go b/vendor/k8s.io/client-go/tools/cache/delta_fifo_test.go index a530eb890..2c2e2f7c0 100644 --- a/vendor/k8s.io/client-go/tools/cache/delta_fifo_test.go +++ b/vendor/k8s.io/client-go/tools/cache/delta_fifo_test.go @@ -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) From 5c20922e085b8c6560187d4b204117f5c03d7a57 Mon Sep 17 00:00:00 2001 From: Aniket Date: Mon, 9 Mar 2020 15:39:19 -0400 Subject: [PATCH 2/2] Update glide.diff --- glide.diff | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/glide.diff b/glide.diff index 72b96723b..815ed4ad8 100644 --- a/glide.diff +++ b/glide.diff @@ -3,6 +3,52 @@ diff --no-dereference -N -r current/vendor/github.com/vishvananda/netlink/link_l < gre.FlowBased = true --- > gre.FlowBased = int8(datum.Value[0]) != 0 +diff --no-dereference -N -r current/vendor/k8s.io/client-go/tools/cache/delta_fifo.go updated/vendor/k8s.io/client-go/tools/cache/delta_fifo.go +350a351,357 +> // 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 +> } +> +356a364,370 +> } +> +> // 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 +diff --no-dereference -N -r current/vendor/k8s.io/client-go/tools/cache/delta_fifo_test.go updated/vendor/k8s.io/client-go/tools/cache/delta_fifo_test.go +88,114d87 +< // 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) +< } +< } +< diff --no-dereference -N -r current/vendor/k8s.io/kubernetes/pkg/proxy/iptables/proxier.go updated/vendor/k8s.io/kubernetes/pkg/proxy/iptables/proxier.go 37c37 < v1 "k8s.io/api/core/v1"