From 7b1c0d6b6684398ce2fee1bbf2deac2b6f96dc5e Mon Sep 17 00:00:00 2001 From: Frederic Branczyk Date: Thu, 11 Jun 2020 16:02:09 +0200 Subject: [PATCH] discovery/kubernetes: Fix incorrect premature break of reading results Previously `max` results stopped reading from results in tests prematurely, as it stopped when `max` number of items were received from the channel instead of `max` number of unique target groups received. This caused flaky tests where the same target group was received multiple times, as Kubernetes informers may emit the same event multiple times. Before this patch, running this test repeatedly failed eventually. After this patch I have run the test many thousand times without failure. ```bash go test -run TestEndpointsDiscoveryNamespaces -count 1000 -test.v ``` Signed-off-by: Frederic Branczyk --- discovery/kubernetes/kubernetes_test.go | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/discovery/kubernetes/kubernetes_test.go b/discovery/kubernetes/kubernetes_test.go index 803002e06..eb1f97fa2 100644 --- a/discovery/kubernetes/kubernetes_test.go +++ b/discovery/kubernetes/kubernetes_test.go @@ -107,14 +107,18 @@ func (d k8sDiscoveryTest) Run(t *testing.T) { // readResultWithTimeout reads all targegroups from channel with timeout. // It merges targegroups by source and sends the result to result channel. func readResultWithTimeout(t *testing.T, ch <-chan []*targetgroup.Group, max int, timeout time.Duration, resChan chan<- map[string]*targetgroup.Group) { - allTgs := make([][]*targetgroup.Group, 0) - + res := make(map[string]*targetgroup.Group) Loop: for { select { case tgs := <-ch: - allTgs = append(allTgs, tgs) - if len(allTgs) == max { + for _, tg := range tgs { + if tg == nil { + continue + } + res[tg.Source] = tg + } + if len(res) == max { // Reached max target groups we may get, break fast. break Loop } @@ -122,21 +126,11 @@ Loop: // Because we use queue, an object that is created then // deleted or updated may be processed only once. // So possibly we may skip events, timed out here. - t.Logf("timed out, got %d (max: %d) items, some events are skipped", len(allTgs), max) + t.Logf("timed out, got %d (max: %d) items, some events are skipped", len(res), max) break Loop } } - // Merge by source and sent it to channel. - res := make(map[string]*targetgroup.Group) - for _, tgs := range allTgs { - for _, tg := range tgs { - if tg == nil { - continue - } - res[tg.Source] = tg - } - } resChan <- res }