New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test(promtail): Fix flaky test with promtail bind port. #6859
Conversation
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need this upgrade.
weaveworks/common#249
c43e6b1
to
d9e168e
Compare
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>
d9e168e
to
4a031c8
Compare
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
- logql -0.4%
+ loki 0% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* 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>
Signed-off-by: Kaviraj kavirajkanagaraj@gmail.com
What this PR does / why we need it:
You cannot run promtail tests if there is already running promtail in your local.
e.g:
It gives you error
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
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'snet
package to bind some unused random ports. Making eachtest run
choose random portsenabling easy running theses tests in parallel.
NOTE: This PR also contains upgrade in
common/server
package that made this fix possible.weaveworks/common#249
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist
CHANGELOG.md
.docs/sources/upgrading/_index.md