[buildkite] Fix "Prometheus compatibility (:docker:)" test in buildkite pipeline #4295
Conversation
kentzeng12
force-pushed
the
zengk/prometheus
branch
2 times, most recently
from
efbcff3 to
a7f198a
Compare
kentzeng12
force-pushed
the
zengk/prometheus
branch
from
a7f198a to
0f1b873
Compare
kentzeng12
force-pushed
the
zengk/prometheus
branch
from
0f1b873 to
31f1846
Compare
|
This lgtm, though I'm not sure it will run on local developer machines after this (tried it a bit, ran into some issues). Since it's better to have regression test than not though, and since I don't think this is a very actively developed code path, I'm okay with sacrificing that here. cc @arnikola since it's your test originally :) @kentzeng12 can you add a followup issue and link it in the README.md just to track the fact that this won't run locally? |
| @@ -371,7 +371,7 @@ func newEvalCmd(expr string, start time.Time, line int) *evalCmd { | |||
|
|
|||
| metrics: map[uint64]labels.Labels{}, | |||
| expected: map[uint64]entry{}, | |||
| m3query: newM3QueryClient("localhost", 7201), | |||
| m3query: newM3QueryClient("host.docker.internal", 7201), | |||
There was a problem hiding this comment.
@kentzeng12 is this only called from a docker context? (afaict answer is yes, so this is okay).
There was a problem hiding this comment.
Since it's being run in the buildkite, which runs the test in a docker container, yes it's called in a docker context, but like you mentioned if you run this locally we run into issues.
There was a problem hiding this comment.
Stamping on following idea:
This lgtm, though I'm not sure it will run on local developer machines after this (tried it a bit, ran into some issues). Since it's better to have regression test than not though, and since I don't think this is a very actively developed code path, I'm okay with sacrificing that here.
|
What this PR does / why we need it:
This PR fixes "Prometheus compatibility (:docker:)" test in buildkite pipeline.
Note about scripts/comparator/compare.go:
I changed
pPromAddressto have the same url aspQueryAddress. This is because from scripts/comparator/README.md, the prometheus node connects to M3Query instance as a remote_read endpoint.Fixes #4274
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
No
Does this PR require updating code package or user-facing documentation?:
No