-
Notifications
You must be signed in to change notification settings - Fork 808
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
Add integration test using older image #2064
Conversation
Here we test pushing from the new distributor to an older ingester, and hand-over from the old ingester to a new ingester. The backwards-compatibility version is pinned at v0.6.0. Signed-off-by: Bryan Boreham <bryan@weave.works>
docker stop i1 | ||
|
||
I2_ADDR=$(container_ip i2) | ||
wait_for "curl -s -f -m 3 $I2_ADDR/ready" "ingester ready" |
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.
How does this guarantee that the hand-over succeeded? Shouldn't we query back the second ingester and ensure it has the same series pushed to the first one?
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.
Some failures, e.g. continuous crashing, will be detected, but you are right that we could do a lot better in checking that the test was fully successful.
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.
I've extended the test to query back; it's still quite rudimentary.
I see in #1839 you did your own Push()
rather than using Avalanche. That would remove the problem that we can't check the random samples.
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.
Thanks for adding the querier. I understand the struggle of doing all this stuff with bash. The assertion you've introduced here LGTM (but please be aware tests are failing in CI).
This week I will revive the PR #1839 starting from your feedback (remove docker-compose, simplify it) and solve all TODOs: if we do a conjunct effort, me on the implementation and you on the review, we may get something solid soon, and start pushing people writing integration tests too when we do code reviews.
Signed-off-by: Bryan Boreham <bryan@weave.works>
858af08
to
1ac49b0
Compare
Note it's only checking the count of series, because the samples are generated randomly by Avalanche. Backwards-compatibility test checks both current and older querier against current ingester. Signed-off-by: Bryan Boreham <bryan@weave.works>
1ac49b0
to
7028f64
Compare
Superseded by #1839. |
I'm closing this PR because superseeded by #1839. |
An example of what I proposed at #2055
Here we test pushing from the new distributor to an older ingester, and hand-over from the old ingester to a new ingester.
The backwards-compatibility version is pinned at v0.6.0.