diff --git a/util/src/main/java/io/kubernetes/client/informer/cache/DeltaFIFO.java b/util/src/main/java/io/kubernetes/client/informer/cache/DeltaFIFO.java index 8a27fa94d0..b379b177b2 100644 --- a/util/src/main/java/io/kubernetes/client/informer/cache/DeltaFIFO.java +++ b/util/src/main/java/io/kubernetes/client/informer/cache/DeltaFIFO.java @@ -158,47 +158,44 @@ public void replace(List list, String resourceVersion) { this.queueActionLocked(DeltaType.Sync, obj); } - if (this.knownObjects == null) { - for (Map.Entry>> entry : - this.items.entrySet()) { - if (keys.contains(entry.getKey())) { - continue; - } - - KubernetesObject deletedObj = null; - MutablePair delta = - entry.getValue().peekLast(); // get newest - if (delta != null) { - deletedObj = delta.getRight(); - } - this.queueActionLocked( - DeltaType.Deleted, new DeletedFinalStateUnknown(entry.getKey(), deletedObj)); + int queueDeletion = 0; + for (Map.Entry>> entry : + this.items.entrySet()) { + if (keys.contains(entry.getKey())) { + continue; } - if (!this.populated) { - this.populated = true; - this.initialPopulationCount = list.size(); + KubernetesObject deletedObj = null; + MutablePair delta = entry.getValue().peekLast(); // get newest + if (delta != null) { + deletedObj = delta.getRight(); + if (deletedObj instanceof DeletedFinalStateUnknown) { + deletedObj = ((DeletedFinalStateUnknown) deletedObj).getObj(); + } } - return; + queueDeletion++; + this.queueActionLocked( + DeltaType.Deleted, new DeletedFinalStateUnknown(entry.getKey(), deletedObj)); } // Detect deletions not already in the queue. - List knownKeys = this.knownObjects.listKeys(); - int queueDeletion = 0; - for (String knownKey : knownKeys) { - if (keys.contains(knownKey)) { - continue; - } + if (this.knownObjects != null) { + List knownKeys = this.knownObjects.listKeys(); + for (String knownKey : knownKeys) { + if (keys.contains(knownKey) || this.items.containsKey(knownKey)) { + continue; + } - KubernetesObject deletedObj = this.knownObjects.getByKey(knownKey); - if (deletedObj == null) { - log.warn( - "Key {} does not exist in known objects store, placing DeleteFinalStateUnknown marker without object", - knownKey); + KubernetesObject deletedObj = this.knownObjects.getByKey(knownKey); + if (deletedObj == null) { + log.warn( + "Key {} does not exist in known objects store, placing DeleteFinalStateUnknown marker without object", + knownKey); + } + queueDeletion++; + this.queueActionLocked( + DeltaType.Deleted, new DeletedFinalStateUnknown(knownKey, deletedObj)); } - queueDeletion++; - this.queueActionLocked( - DeltaType.Deleted, new DeletedFinalStateUnknown(knownKey, deletedObj)); } if (!this.populated) { diff --git a/util/src/test/java/io/kubernetes/client/informer/cache/DeltaFIFOTest.java b/util/src/test/java/io/kubernetes/client/informer/cache/DeltaFIFOTest.java index 1e54c9c109..ce55c93312 100644 --- a/util/src/test/java/io/kubernetes/client/informer/cache/DeltaFIFOTest.java +++ b/util/src/test/java/io/kubernetes/client/informer/cache/DeltaFIFOTest.java @@ -25,6 +25,8 @@ import java.util.LinkedList; import org.apache.commons.lang3.tuple.MutablePair; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; class DeltaFIFOTest { @Test @@ -156,9 +158,11 @@ void deltaFIFOReplaceWithDeleteDeltaIn() throws InterruptedException { deltaFIFO.delete(oldPod); deltaFIFO.replace(Arrays.asList(newPod), "0"); + deltaFIFO.replace(Arrays.asList(newPod), "0"); deltaFIFO.pop( (deltas) -> { + assertThat(deltas).hasSize(1); assertThat(deltas.getFirst().getLeft()).isEqualTo(DeltaFIFO.DeltaType.Deleted); assertThat(deltas.getFirst().getRight()).isEqualTo(oldPod); }); @@ -169,4 +173,116 @@ void deltaFIFOReplaceWithDeleteDeltaIn() throws InterruptedException { assertThat(deltas.getFirst().getRight()).isEqualTo(newPod); }); } + + @ParameterizedTest + @CsvSource({"false,false", "true,false", "true,true"}) + void replaceDeletesPendingObjectUsingNewestDelta(boolean withStore, boolean cached) + throws InterruptedException { + V1Pod oldPod = + new V1Pod() + .metadata(new V1ObjectMeta().namespace("default").name("foo").resourceVersion("1")); + V1Pod newPod = + new V1Pod() + .metadata(new V1ObjectMeta().namespace("default").name("foo").resourceVersion("2")); + Cache cache = new Cache<>(); + DeltaFIFO fifo = + new DeltaFIFO(Caches::deletionHandlingMetaNamespaceKeyFunc, withStore ? cache : null); + fifo.replace(Collections.emptyList(), "0"); + if (cached) { + cache.add(oldPod); + } + fifo.add(oldPod); + fifo.update(newPod); + + fifo.replace(Collections.emptyList(), "3"); + fifo.replace(Collections.emptyList(), "4"); + + assertThat(fifo.listKeys()).containsExactly("default/foo"); + fifo.pop( + deltas -> { + assertThat(deltas) + .extracting(MutablePair::getLeft) + .containsExactly( + DeltaFIFO.DeltaType.Added, + DeltaFIFO.DeltaType.Updated, + DeltaFIFO.DeltaType.Deleted); + DeltaFIFO.DeletedFinalStateUnknown tombstone = + (DeltaFIFO.DeletedFinalStateUnknown) deltas.peekLast().getRight(); + assertThat(tombstone.getKey()).isEqualTo("default/foo"); + assertThat(tombstone.getObj()).isSameAs(newPod); + }); + assertThat(fifo.listKeys()).isEmpty(); + assertThat(fifo.hasSynced()).isTrue(); + } + + @Test + void replaceDeletesRecreatedObjectUsingNewestDelta() throws InterruptedException { + V1Pod oldPod = + new V1Pod() + .metadata( + new V1ObjectMeta() + .namespace("default") + .name("foo") + .uid("old") + .resourceVersion("1")); + V1Pod newPod = + new V1Pod() + .metadata( + new V1ObjectMeta() + .namespace("default") + .name("foo") + .uid("new") + .resourceVersion("2")); + Cache cache = new Cache<>(); + cache.add(oldPod); + DeltaFIFO fifo = new DeltaFIFO(Caches::deletionHandlingMetaNamespaceKeyFunc, cache); + fifo.delete(oldPod); + fifo.add(newPod); + + fifo.replace(Collections.emptyList(), "3"); + + fifo.pop( + deltas -> { + assertThat(deltas) + .extracting(MutablePair::getLeft) + .containsExactly( + DeltaFIFO.DeltaType.Deleted, + DeltaFIFO.DeltaType.Added, + DeltaFIFO.DeltaType.Deleted); + assertThat(deltas.peekFirst().getRight()).isSameAs(oldPod); + DeltaFIFO.DeletedFinalStateUnknown tombstone = + (DeltaFIFO.DeletedFinalStateUnknown) deltas.peekLast().getRight(); + assertThat(tombstone.getObj()).isSameAs(newPod); + }); + assertThat(fifo.listKeys()).isEmpty(); + } + + @Test + void replaceCountsCachedDeletionsDuringInitialSync() throws InterruptedException { + V1Pod missing = new V1Pod().metadata(new V1ObjectMeta().namespace("default").name("missing")); + V1Pod present = new V1Pod().metadata(new V1ObjectMeta().namespace("default").name("present")); + Cache cache = new Cache<>(); + cache.add(missing); + DeltaFIFO fifo = new DeltaFIFO(Caches::deletionHandlingMetaNamespaceKeyFunc, cache); + + fifo.replace(Collections.singletonList(present), "1"); + assertThat(fifo.hasSynced()).isFalse(); + fifo.pop( + deltas -> { + assertThat(deltas).hasSize(1); + assertThat(deltas.peekFirst().getLeft()).isEqualTo(DeltaFIFO.DeltaType.Sync); + assertThat(deltas.peekFirst().getRight()).isSameAs(present); + }); + assertThat(fifo.hasSynced()).isFalse(); + fifo.pop( + deltas -> { + assertThat(deltas).hasSize(1); + assertThat(deltas.peekFirst().getLeft()).isEqualTo(DeltaFIFO.DeltaType.Deleted); + DeltaFIFO.DeletedFinalStateUnknown tombstone = + (DeltaFIFO.DeletedFinalStateUnknown) deltas.peekFirst().getRight(); + assertThat(tombstone.getObj()).isSameAs(missing); + }); + assertThat(fifo.hasSynced()).isTrue(); + assertThat(fifo.listKeys()).isEmpty(); + } } diff --git a/util/src/test/java/io/kubernetes/client/informer/impl/DefaultSharedIndexInformerTest.java b/util/src/test/java/io/kubernetes/client/informer/impl/DefaultSharedIndexInformerTest.java index 3b38a559ef..bdb98c0e78 100644 --- a/util/src/test/java/io/kubernetes/client/informer/impl/DefaultSharedIndexInformerTest.java +++ b/util/src/test/java/io/kubernetes/client/informer/impl/DefaultSharedIndexInformerTest.java @@ -12,25 +12,32 @@ */ package io.kubernetes.client.informer.impl; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + import io.kubernetes.client.informer.ListerWatcher; +import io.kubernetes.client.informer.cache.Cache; +import io.kubernetes.client.informer.cache.Caches; import io.kubernetes.client.informer.cache.DeltaFIFO; import io.kubernetes.client.informer.cache.Indexer; +import io.kubernetes.client.openapi.models.V1ObjectMeta; import io.kubernetes.client.openapi.models.V1Pod; import io.kubernetes.client.openapi.models.V1PodList; import io.kubernetes.client.util.Threads; +import java.util.Collections; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; import java.util.function.BiConsumer; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; - @ExtendWith(MockitoExtension.class) class DefaultSharedIndexInformerTest { @@ -80,4 +87,48 @@ void minimalConstructorExists() { new DefaultSharedIndexInformer<>(anyApiType, listerWatcher, anyResyncPeriod); } + + @ParameterizedTest + @CsvSource({ + "true,false,false", + "true,true,false", + "false,true,false", + "true,false,true", + "true,true,true", + "false,true,true" + }) + void relistReconcilesPendingAndCachedObjects(boolean queued, boolean cached, boolean present) + throws InterruptedException { + V1Pod pod = + new V1Pod() + .metadata(new V1ObjectMeta().namespace("default").name("foo").resourceVersion("2")); + V1Pod listedPod = + new V1Pod() + .metadata(new V1ObjectMeta().namespace("default").name("foo").resourceVersion("3")); + Cache cache = new Cache<>(); + DeltaFIFO fifo = new DeltaFIFO(Caches::deletionHandlingMetaNamespaceKeyFunc, cache); + DefaultSharedIndexInformer informer = + new DefaultSharedIndexInformer<>(anyApiType, listerWatcher, 0, fifo, cache); + fifo.replace(Collections.emptyList(), "1"); + if (cached) { + cache.add(pod); + } + if (queued) { + fifo.add(pod); + } + + fifo.replace(present ? Collections.singletonList(listedPod) : Collections.emptyList(), "3"); + assertThat(fifo.listKeys()).containsExactly("default/foo"); + fifo.pop(informer::handleDeltas); + + if (present) { + assertThat(cache.list()).containsExactly(listedPod); + assertThat(cache.getByKey("default/foo")).isSameAs(listedPod); + } else { + assertThat(cache.list()).isEmpty(); + assertThat(cache.getByKey("default/foo")).isNull(); + } + assertThat(fifo.listKeys()).isEmpty(); + assertThat(fifo.hasSynced()).isTrue(); + } }