Skip to content

Commit

Permalink
test(promtail): Fix flaky test with promtail bind port. (#6859)
Browse files Browse the repository at this point in the history
* test(promtail): Fix flaky test with promtail bind port.

You cannot run promtail tests if there is already running promtail in your local.
e.g:
```
$ go test -run TestPromtail ./clients/pkg/promtail/
```

It gives you error
```
--- FAIL: TestPromtail (0.02s)
    promtail_test.go:112: error creating promtail listen tcp 127.0.0.1:9080: bind: address already in use
FAIL
FAIL	github.com/grafana/loki/clients/pkg/promtail	0.043s
```
Cause for the error is, test uses static default port (the same we use for promtail normally) in tests.

This is not **too** annonying if seen in isolation. But mainly annoying in two cases particularly
1. Hard to run tests parallely in general with these kind of static ports
2. These are causing some real problems in some integeration tests on GEL which starts docker-compose to run some test suites and
even stopping containers won't release ports soon enough making CI fail randomly.

This PR takes advantage of Go idiom of passing `port=0` to http (and also grpc) servers enforce Go's `net` package to bind some unused random ports. Making each test run choose random ports
enabling easy running theses tests in parallel.

NOTE: This PR also contains upgrade in `common/server` package that made this fix possible.
weaveworks/common#249

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>

* go mod tidy

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
  • Loading branch information
kavirajk committed Aug 10, 2022
1 parent e372383 commit e8b4417
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 24 deletions.
20 changes: 14 additions & 6 deletions clients/pkg/promtail/promtail_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io/ioutil"
"math"
"math/rand"
"net"
"net/http"
"net/url"
"os"
Expand Down Expand Up @@ -36,15 +37,14 @@ import (
"github.com/grafana/loki/clients/pkg/promtail/positions"
"github.com/grafana/loki/clients/pkg/promtail/scrapeconfig"
"github.com/grafana/loki/clients/pkg/promtail/server"
pserver "github.com/grafana/loki/clients/pkg/promtail/server"
file2 "github.com/grafana/loki/clients/pkg/promtail/targets/file"

"github.com/grafana/loki/pkg/logproto"
"github.com/grafana/loki/pkg/util"
util_log "github.com/grafana/loki/pkg/util/log"
)

const httpTestPort = 9080

var clientMetrics = client.NewMetrics(prometheus.DefaultRegisterer, nil)

func TestPromtail(t *testing.T) {
Expand Down Expand Up @@ -122,6 +122,10 @@ func TestPromtail(t *testing.T) {
}()
defer p.Shutdown() // In case the test fails before the call to Shutdown below.

svr := p.server.(*pserver.PromtailServer)

httpListenAddr := svr.Server.HTTPListenAddr()

expectedCounts := map[string]int{}

startupMarkerFile := testDir + "/startupMarker.log"
Expand Down Expand Up @@ -202,7 +206,7 @@ func TestPromtail(t *testing.T) {
<-time.After(500 * time.Millisecond)

// Pull out some prometheus metrics before shutting down
metricsBytes, contentType := getPromMetrics(t)
metricsBytes, contentType := getPromMetrics(t, httpListenAddr)

p.Shutdown()

Expand Down Expand Up @@ -496,8 +500,8 @@ func (h *testServerHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
h.recMtx.Unlock()
}

func getPromMetrics(t *testing.T) ([]byte, string) {
resp, err := http.Get(fmt.Sprintf("http://localhost:%d/metrics", httpTestPort))
func getPromMetrics(t *testing.T, httpListenAddr net.Addr) ([]byte, string) {
resp, err := http.Get(fmt.Sprintf("http://%s/metrics", httpListenAddr))
if err != nil {
t.Fatal("Could not query metrics endpoint", err)
}
Expand Down Expand Up @@ -561,7 +565,11 @@ func buildTestConfig(t *testing.T, positionsFileName string, logDirName string)
cfg.ServerConfig.HTTPListenAddress = hostname
cfg.ServerConfig.ExternalURL = hostname
cfg.ServerConfig.GRPCListenAddress = hostname
cfg.ServerConfig.HTTPListenPort = httpTestPort

// NOTE: setting port to `0` makes it bind to some unused random port.
// enabling tests run more self contained and easy to run tests in parallel.
cfg.ServerConfig.HTTPListenPort = 0
cfg.ServerConfig.GRPCListenPort = 0

// Override some of those defaults
cfg.ClientConfig.URL = clientURL
Expand Down
12 changes: 6 additions & 6 deletions clients/pkg/promtail/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type Server interface {
}

// Server embed weaveworks server with static file and templating capability
type server struct {
type PromtailServer struct {
*serverww.Server
log log.Logger
tms *targets.TargetManagers
Expand Down Expand Up @@ -88,7 +88,7 @@ func New(cfg Config, log log.Logger, tms *targets.TargetManagers, promtailCfg st
healthCheckTargetFlag = *cfg.HealthCheckTarget
}

serv := &server{
serv := &PromtailServer{
Server: wws,
log: log,
tms: tms,
Expand All @@ -108,7 +108,7 @@ func New(cfg Config, log log.Logger, tms *targets.TargetManagers, promtailCfg st
}

// serviceDiscovery serves the service discovery page.
func (s *server) serviceDiscovery(rw http.ResponseWriter, req *http.Request) {
func (s *PromtailServer) serviceDiscovery(rw http.ResponseWriter, req *http.Request) {
var index []string
allTarget := s.tms.AllTargets()
for job := range allTarget {
Expand Down Expand Up @@ -175,7 +175,7 @@ func (s *server) serviceDiscovery(rw http.ResponseWriter, req *http.Request) {
})
}

func (s *server) config(rw http.ResponseWriter, req *http.Request) {
func (s *PromtailServer) config(rw http.ResponseWriter, req *http.Request) {
executeTemplate(req.Context(), rw, templateOptions{
Data: s.promtailCfg,
BuildVersion: version.Info(),
Expand All @@ -186,7 +186,7 @@ func (s *server) config(rw http.ResponseWriter, req *http.Request) {
}

// targets serves the targets page.
func (s *server) targets(rw http.ResponseWriter, req *http.Request) {
func (s *PromtailServer) targets(rw http.ResponseWriter, req *http.Request) {
executeTemplate(req.Context(), rw, templateOptions{
Data: struct {
TargetPools map[string][]target.Target
Expand Down Expand Up @@ -219,7 +219,7 @@ func (s *server) targets(rw http.ResponseWriter, req *http.Request) {
}

// ready serves the ready endpoint
func (s *server) ready(rw http.ResponseWriter, _ *http.Request) {
func (s *PromtailServer) ready(rw http.ResponseWriter, _ *http.Request) {
if s.healthCheckTarget && !s.tms.Ready() {
http.Error(rw, readinessProbeFailure, http.StatusInternalServerError)
return
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ require (
github.com/thanos-io/thanos v0.22.0
github.com/tonistiigi/fifo v0.0.0-20190226154929-a9fb20d87448
github.com/uber/jaeger-client-go v2.30.0+incompatible
github.com/weaveworks/common v0.0.0-20220706100410-67d27ed40fae
github.com/weaveworks/common v0.0.0-20220809154356-72ba250fe659
github.com/xdg-go/scram v1.0.2
go.etcd.io/bbolt v1.3.6
go.uber.org/atomic v1.9.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1581,8 +1581,8 @@ github.com/vultr/govultr/v2 v2.17.1 h1:UBmotwA0mkGtyJMakUF9jhLH/W3mN5wfGRn543i/B
github.com/vultr/govultr/v2 v2.17.1/go.mod h1:ZFOKGWmgjytfyjeyAdhQlSWwTjh2ig+X49cAp50dzXI=
github.com/wavefronthq/wavefront-sdk-go v0.9.2/go.mod h1:hQI6y8M9OtTCtc0xdwh+dCER4osxXdEAeCpacjpDZEU=
github.com/weaveworks/common v0.0.0-20210913144402-035033b78a78/go.mod h1:YU9FvnS7kUnRt6HY10G+2qHkwzP3n3Vb1XsXDsJTSp8=
github.com/weaveworks/common v0.0.0-20220706100410-67d27ed40fae h1:Z8YibUpdBEdCq8nwrYXJQ8vYooevbmEBIdFpseXK3/8=
github.com/weaveworks/common v0.0.0-20220706100410-67d27ed40fae/go.mod h1:YfOOLoW1Q/jIIu0WLeSwgStmrKjuJEZSKTAUc+0KFvE=
github.com/weaveworks/common v0.0.0-20220809154356-72ba250fe659 h1:McPAXT5tVW1NVs7SSGOCU2EzLyiUdygrGKZJZMTYZgA=
github.com/weaveworks/common v0.0.0-20220809154356-72ba250fe659/go.mod h1:YfOOLoW1Q/jIIu0WLeSwgStmrKjuJEZSKTAUc+0KFvE=
github.com/weaveworks/promrus v1.2.0 h1:jOLf6pe6/vss4qGHjXmGz4oDJQA+AOCqEL3FvvZGz7M=
github.com/weaveworks/promrus v1.2.0/go.mod h1:SaE82+OJ91yqjrE1rsvBWVzNZKcHYFtMUyS1+Ogs/KA=
github.com/willf/bitset v1.1.11 h1:N7Z7E9UvjW+sGsEl7k/SJrvY2reP1A07MrGuCjIOjRE=
Expand Down
9 changes: 1 addition & 8 deletions vendor/github.com/weaveworks/common/instrument/instrument.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions vendor/github.com/weaveworks/common/server/server.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,7 @@ github.com/uber/jaeger-lib/metrics/prometheus
# github.com/ugorji/go/codec v1.1.7
## explicit
github.com/ugorji/go/codec
# github.com/weaveworks/common v0.0.0-20220706100410-67d27ed40fae
# github.com/weaveworks/common v0.0.0-20220809154356-72ba250fe659
## explicit; go 1.14
github.com/weaveworks/common/aws
github.com/weaveworks/common/errors
Expand Down

0 comments on commit e8b4417

Please sign in to comment.