Skip to content
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

Fix flaky TestMultipleClientsWithMixedLabelsAndExpectFailure #599

Conversation

julienduchesne
Copy link
Member

This failed in a bunch of runs in #598

When it fails, it has 3 updates, but they're all from the same member. I believe what we want to check is that the cluster never gets joined by members with labels

@julienduchesne
Copy link
Member Author

julienduchesne commented Oct 8, 2024

/find-flaky-tests

Run: https://github.com/grafana/dskit/actions/runs/11237399709

@julienduchesne julienduchesne force-pushed the julienduchesne/flaky-test-TestMultipleClientsWithMixedLabelsAndExpectFailure branch from 3b58d5f to f76ff6b Compare October 8, 2024 14:16
Comment on lines -11 to +12
fail-fast: false
strategy:
fail-fast: false
Copy link
Member Author

@julienduchesne julienduchesne Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll get it right sometime... I managed to start a run anyways since "workflow dispatch" seems to use the workflow config from the branch, while a workflow triggered from issue comments takes the workflow config from main (while checking out the branch to run tests)

@julienduchesne julienduchesne marked this pull request as ready for review October 8, 2024 14:26
This failed in a bunch of runs in #598

When it fails, it has 3 updates, but they're all from the same member. I believe what we want to check is that the cluster never gets joined by members with labels
@julienduchesne julienduchesne force-pushed the julienduchesne/flaky-test-TestMultipleClientsWithMixedLabelsAndExpectFailure branch from f76ff6b to 7464c1e Compare October 8, 2024 14:29
@julienduchesne julienduchesne requested a review from a team October 8, 2024 14:49
@julienduchesne julienduchesne merged commit f77c1dd into main Oct 8, 2024
5 checks passed
@julienduchesne julienduchesne deleted the julienduchesne/flaky-test-TestMultipleClientsWithMixedLabelsAndExpectFailure branch October 8, 2024 17:40
julienduchesne added a commit that referenced this pull request Oct 9, 2024
#599 went in a wrong direction, so I revert it here

Instead, to fix the `TestMultipleClientsWithMixedLabelsAndExpectFailure` test, I add a couple more labeled members, that way, even if we reach 3 updates (which happened some times), we'll never get to 5
julienduchesne added a commit that referenced this pull request Oct 9, 2024
#599 went in a wrong direction, so I revert it here: it makes all `TestMultipleClients` tests flaky since not all members are necessarily joined after the updates

Instead, to fix the `TestMultipleClientsWithMixedLabelsAndExpectFailure` test, I add a couple more labeled members, that way, even if we reach 3 updates (which happened some times), we'll never get to 5 with a single member

Also, try to fix the `TestTLSServerWithLocalhostCertWithClientCertificateEnforcementUsingClientCA1` test by closing GRPC connections. Maybe this (golang/go#36700) is the issue?
julienduchesne added a commit that referenced this pull request Oct 9, 2024
#599 went in a wrong direction, so I revert it here: it makes all `TestMultipleClients` tests flaky since not all members are necessarily joined after the updates

Instead, to fix the `TestMultipleClientsWithMixedLabelsAndExpectFailure` test, I add a couple more labeled members, that way, even if we reach 3 updates (which happened some times), we'll never get to 5 with a single member

Also, try to fix the `TestTLSServerWithLocalhostCertWithClientCertificateEnforcementUsingClientCA1` test by closing GRPC connections. Maybe this (golang/go#36700) is the issue?
julienduchesne added a commit that referenced this pull request Oct 10, 2024
* Really fix flaky `TestMultipleClients` tests
#599 went in a wrong direction, so I revert it here: it makes all `TestMultipleClients` tests flaky since not all members are necessarily joined after the updates

Instead, to fix the `TestMultipleClientsWithMixedLabelsAndExpectFailure` test, I add a couple more labeled members, that way, even if we reach 3 updates (which happened some times), we'll never get to 5 with a single member

Also, try to fix the `TestTLSServerWithLocalhostCertWithClientCertificateEnforcementUsingClientCA1` test by closing GRPC connections. Maybe this (golang/go#36700) is the issue?

* Set `MaxIdleConnsPerHost`

* New attempt to fix this test. Could it be aborted connections?

* Retry RSTs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants