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.

Sign up for GitHub

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

Jump to bottom

TestIntegrations flakiness #47156

Closed
GavinFrazar opened this issue Oct 3, 2024 · 6 comments · Fixed by #49850
Closed

TestIntegrations flakiness #47156

GavinFrazar opened this issue Oct 3, 2024 · 6 comments · Fixed by #49850
Assignees
@rosstimothy
Labels
flaky tests

Comments

@GavinFrazar
Copy link
Contributor

GavinFrazar commented Oct 3, 2024 •
edited by rosstimothy
Loading

Failure

Link(s) to logs

Relevant snippet

    integration_test.go:4867: 
        	Error Trace:	/__w/teleport/teleport/integration/integration_test.go:4867
        	Error:      	Received unexpected error:
        	            	failed connecting to host localhost:24990: failed to receive cluster details response
        	            		failed to dial target host
        	            		direct dialing to nodes not found in inventory is not supported
        	Test:       	TestIntegrations/ProxyHostKeyCheck/Disabled

I'm making this issue to cover multiple subtests of TestIntegrations that are all flaky for the same inventory error reason:
@GavinFrazar
Copy link
Contributor Author

@zmb3 who would be best to assign this since Alex is no longer with Teleport?

@GavinFrazar
Copy link
Contributor Author

another subtest: https://github.com/gravitational/teleport/actions/runs/11169042992/job/31048876887?pr=46747

        	Error Trace:	/__w/teleport/teleport/integration/integration_test.go:3237
        	            				/__w/teleport/teleport/integration/integration_test.go:2956
        	            				/__w/teleport/teleport/integration/integration_test.go:130
        	Error:      	Received unexpected error:
        	            	failed connecting to host 127.0.0.1:34095: failed to receive cluster details response
        	            		failed to dial target host
        	            		direct dialing to nodes not found in inventory is not supported
        	Test:       	TestIntegrations/TrustedClusters
    --- FAIL: TestIntegrations/TrustedClusters (30.99s)

@greedy52
Copy link
Contributor

greedy52 commented Oct 4, 2024 •
edited
Loading

https://github.com/gravitational/teleport/actions/runs/11184689664/job/31096020045
https://github.com/gravitational/teleport/actions/runs/11188058592/job/31106193681?pr=47239

@ravicious
Copy link
Member

https://github.com/gravitational/teleport/actions/runs/11480984785/job/31950631855#step:6:1976

     integration_test.go:4867: 
        	Error Trace:	/__w/teleport/teleport/integration/integration_test.go:4867
        	Error:      	Received unexpected error:
        	            	failed connecting to host localhost:24990: failed to receive cluster details response
        	            		failed to dial target host
        	            		direct dialing to nodes not found in inventory is not supported
        	Test:       	TestIntegrations/ProxyHostKeyCheck/Disabled

@rosstimothy rosstimothy self-assigned this Oct 24, 2024
@hugoShaka
Copy link
Contributor

TestIntegrations/TrustedClustersWithLabels failed with

failed connecting to host 127.0.0.1:40111: failed to receive cluster details response
    failed to dial target host
    connection error: desc = "transport: Error while dialing: failed to dial: cluster cluster-aux is offline"

Source: https://github.com/gravitational/teleport/actions/runs/11686293079/job/32541747209

The root cause seems to be the same as the TestIntegrations/TrustedClusters failure described above. I can file a new one if you think this is useful.

@rosstimothy
Copy link
Contributor

The crux of these issues seems to stem from the fact that we are waiting for the nodes to be visible by auth, but not waiting for them to be propagated to the proxy cache. As a result the check to wait for nodes passes, but when the dial request gets processed by the proxy it still doesn't know about the target node.

rosstimothy added a commit that referenced this issue Dec 5, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests
67351cc 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy cache.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `helpers.WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`.
rosstimothy added a commit that referenced this issue Dec 5, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests
d1eed15 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy cache.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `helpers.WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`.
@rosstimothy rosstimothy mentioned this issue Dec 5, 2024
Merged
rosstimothy added a commit that referenced this issue Dec 5, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests
8779d70 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `helpers.WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`. Additionally, `helpers.WaitForNodeCount` was updated
to validate that the node watcher used for routing in the Proxy
also contained the expected number of nodes.
rosstimothy added a commit that referenced this issue Dec 5, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests
ae13c10 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `helpers.WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`. Additionally, `helpers.WaitForNodeCount` was updated
to validate that the node watcher used for routing in the Proxy
also contained the expected number of nodes.
rosstimothy added a commit that referenced this issue Dec 6, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests
f8d626b 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `helpers.WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`. Additionally, `helpers.WaitForNodeCount` was updated
to validate that the node watcher used for routing in the Proxy
also contained the expected number of nodes.
rosstimothy added a commit that referenced this issue Dec 6, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests
2683dcb 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `(TeleInstance) WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`. Additionally, `helpers.WaitForNodeCount` was updated
to validate that the node watcher used for routing in the Proxy
also contained the expected number of nodes.
github-merge-queue bot pushed a commit that referenced this issue Dec 6, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests (#49850)
da9ed65 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `(TeleInstance) WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`. Additionally, `helpers.WaitForNodeCount` was updated
to validate that the node watcher used for routing in the Proxy
also contained the expected number of nodes.
github-actions bot pushed a commit that referenced this issue Dec 6, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests
69ed211 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `(TeleInstance) WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`. Additionally, `helpers.WaitForNodeCount` was updated
to validate that the node watcher used for routing in the Proxy
also contained the expected number of nodes.
rosstimothy added a commit that referenced this issue Dec 6, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests (#49850)
913e3f1 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `(TeleInstance) WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`. Additionally, `helpers.WaitForNodeCount` was updated
to validate that the node watcher used for routing in the Proxy
also contained the expected number of nodes.
rosstimothy added a commit that referenced this issue Dec 6, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests (#49850)
830b466 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `(TeleInstance) WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`. Additionally, `helpers.WaitForNodeCount` was updated
to validate that the node watcher used for routing in the Proxy
also contained the expected number of nodes.
github-merge-queue bot pushed a commit that referenced this issue Dec 6, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests (#49888)
a56bc4e 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `(TeleInstance) WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`. Additionally, `helpers.WaitForNodeCount` was updated
to validate that the node watcher used for routing in the Proxy
also contained the expected number of nodes.
github-merge-queue bot pushed a commit that referenced this issue Dec 6, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests (#49850) (#49893)
42a4c11 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `(TeleInstance) WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`. Additionally, `helpers.WaitForNodeCount` was updated
to validate that the node watcher used for routing in the Proxy
also contained the expected number of nodes.
github-merge-queue bot pushed a commit that referenced this issue Dec 6, 2024
@rosstimothy
Attempt to reduce flakiness of integration tests (#49850) (#49894)
74e33c8 
Closes #47156.

All of the tests suffering from issues dialing hosts, and failing
with a `failed to dial target host` error were incorrectly waiting
for nodes to become visible before establishing connections. The
main culprit for most of the failures was `waitForNodesToRegister`,
though a few tests had a very similar hand rolled variant, which
incorrectly returned when the nodes appeard in Auth. However, since
the Proxy is the one performing dialing, they should have waited
for the nodes to appear in the Proxy.

To resolve, `waitForNodesToRegister` and all hand rolled equivalents
have been removed in favor of `(TeleInstance) WaitForNodeCount` which
correctly uses the `CachingAccessPoint` of the RemoteSite instead
of `GetClient`. Additionally, `helpers.WaitForNodeCount` was updated
to validate that the node watcher used for routing in the Proxy
also contained the expected number of nodes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rosstimothy rosstimothy

Labels
flaky tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Attempt to reduce flakiness of integration tests gravitational/teleport
5 participants
@ravicious @greedy52 @hugoShaka @GavinFrazar @rosstimothy