Skip to content

Commit

Permalink
<Promtail>: Fix flaky docker test (grafana#11777)
Browse files Browse the repository at this point in the history
**What this PR does / why we need it**:

We ported this fix a few days ago to the Agent via this
[PR](grafana/agent#6201) but the added test is
often failing in our drone pipeline.

I believe that there are two reasons why this test is flaky:
- the test expects that the 5 last lines of logs are the expected ones
but it seems that other lines might be logged as well
(https://drone.grafana.net/grafana/agent/16045/4/2)
- we saw that the simulated container did not always have enough time to
stop before calling the tgt.startIfNotRunning() to restart it.
(https://drone.grafana.net/grafana/agent/16077/4/2)

Fix:
- the test now uses "assert.EventuallyWithT" and checks that within 5
seconds the expected log lines will be amongst the logs without
duplicate.
- the test now stops the simulated container before restarting it

**Checklist**
- [x] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)

---------

Co-authored-by: Michel Hollands <[email protected]>
  • Loading branch information
2 people authored and Gordejj committed Jan 29, 2024
1 parent 37ebda0 commit 2febba6
Showing 1 changed file with 36 additions and 29 deletions.
65 changes: 36 additions & 29 deletions clients/pkg/promtail/targets/docker/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"net/http"
"net/http/httptest"
"os"
"sort"
"strings"
"testing"
"time"
Expand All @@ -17,6 +16,7 @@ import (
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/common/model"
"github.com/prometheus/prometheus/model/relabel"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/grafana/loki/clients/pkg/promtail/client/fake"
Expand Down Expand Up @@ -77,49 +77,56 @@ func Test_DockerTarget(t *testing.T) {
)
require.NoError(t, err)

require.Eventually(t, func() bool {
return len(entryHandler.Received()) >= 5
}, 5*time.Second, 100*time.Millisecond)

received := entryHandler.Received()
sort.Slice(received, func(i, j int) bool {
return received[i].Timestamp.Before(received[j].Timestamp)
})

expectedLines := []string{
"5.3.69.55 - - [09/Dec/2021:09:15:02 +0000] \"HEAD /brand/users/clicks-and-mortar/front-end HTTP/2.0\" 503 27087",
"101.54.183.185 - - [09/Dec/2021:09:15:03 +0000] \"POST /next-generation HTTP/1.0\" 416 11468",
"69.27.137.160 - runolfsdottir2670 [09/Dec/2021:09:15:03 +0000] \"HEAD /content/visionary/engineer/cultivate HTTP/1.1\" 302 2975",
"28.104.242.74 - - [09/Dec/2021:09:15:03 +0000] \"PATCH /value-added/cultivate/systems HTTP/2.0\" 405 11843",
"150.187.51.54 - satterfield1852 [09/Dec/2021:09:15:03 +0000] \"GET /incentivize/deliver/innovative/cross-platform HTTP/1.1\" 301 13032",
}
actualLines := make([]string, 0, 5)
for _, entry := range received[:5] {
actualLines = append(actualLines, entry.Line)
}
require.ElementsMatch(t, actualLines, expectedLines)

assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertExpectedLog(c, entryHandler, expectedLines)
}, 5*time.Second, 100*time.Millisecond, "Expected log lines were not found within the time limit.")

target.Stop()
entryHandler.Clear()
// restart target to simulate container restart
target.startIfNotRunning()
entryHandler.Clear()
require.Eventually(t, func() bool {
return len(entryHandler.Received()) >= 5
}, 5*time.Second, 100*time.Millisecond)

receivedAfterRestart := entryHandler.Received()
sort.Slice(receivedAfterRestart, func(i, j int) bool {
return receivedAfterRestart[i].Timestamp.Before(receivedAfterRestart[j].Timestamp)
})
actualLinesAfterRestart := make([]string, 0, 5)
for _, entry := range receivedAfterRestart[:5] {
actualLinesAfterRestart = append(actualLinesAfterRestart, entry.Line)
}
expectedLinesAfterRestart := []string{
"243.115.12.215 - - [09/Dec/2023:09:16:57 +0000] \"DELETE /morph/exploit/granular HTTP/1.0\" 500 26468",
"221.41.123.237 - - [09/Dec/2023:09:16:57 +0000] \"DELETE /user-centric/whiteboard HTTP/2.0\" 205 22487",
"89.111.144.144 - - [09/Dec/2023:09:16:57 +0000] \"DELETE /open-source/e-commerce HTTP/1.0\" 401 11092",
"62.180.191.187 - - [09/Dec/2023:09:16:57 +0000] \"DELETE /cultivate/integrate/technologies HTTP/2.0\" 302 12979",
"156.249.2.192 - - [09/Dec/2023:09:16:57 +0000] \"POST /revolutionize/mesh/metrics HTTP/2.0\" 401 5297",
}
require.ElementsMatch(t, actualLinesAfterRestart, expectedLinesAfterRestart)
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertExpectedLog(c, entryHandler, expectedLinesAfterRestart)
}, 5*time.Second, 100*time.Millisecond, "Expected log lines after restart were not found within the time limit.")
}

// assertExpectedLog will verify that all expectedLines were received, in any order, without duplicates.
func assertExpectedLog(c *assert.CollectT, entryHandler *fake.Client, expectedLines []string) {
logLines := entryHandler.Received()
testLogLines := make(map[string]int)
for _, l := range logLines {
if containsString(expectedLines, l.Line) {
testLogLines[l.Line]++
}
}
// assert that all log lines were received
assert.Len(c, testLogLines, len(expectedLines))
// assert that there are no duplicated log lines
for _, v := range testLogLines {
assert.Equal(c, v, 1)
}
}

func containsString(slice []string, str string) bool {
for _, item := range slice {
if item == str {
return true
}
}
return false
}

0 comments on commit 2febba6

Please sign in to comment.