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

tailscale.com/tstest/integration.TestControlTimeLogLine is flaky #11962

Closed
andrew-d opened this issue May 1, 2024 · 0 comments · Fixed by #12026
Closed

tailscale.com/tstest/integration.TestControlTimeLogLine is flaky #11962

andrew-d opened this issue May 1, 2024 · 0 comments · Fixed by #12026

Comments

@andrew-d
Copy link
Member

andrew-d commented May 1, 2024

Failed in https://github.com/tailscale/tailscale/actions/runs/8913641407/job/24479546700?pr=11961

=== RUN   TestControlTimeLogLine
=== PAUSE TestControlTimeLogLine
=== CONT  TestControlTimeLogLine
    integration_test.go:1006: DERP httpsrv listener: 127.0.0.1:44453
    integration_test.go:222: Status: NoState
    integration_test.go:223: Running /tmp/1365692733/tailscale --socket=/tmp/TestControlTimeLogLine3447055780/001/tailscale.sock up --login-server=http://127.0.0.1:40577/ --reset ...
2024/05/01 18:59:00 Error: unexpected HTTP proxy via proxy: CONNECT controlplane.tailscale.com:443 HTTP/1.1
Host: controlplane.tailscale.com:443
User-Agent: Go-http-client/1.1

2024/05/01 18:59:00 Error: unexpected HTTP proxy via proxy: CONNECT controlplane.tailscale.com:443 HTTP/1.1
Host: controlplane.tailscale.com:443
User-Agent: Go-http-client/1.1

2024/05/01 18:59:00 Error: unexpected HTTP proxy via proxy: CONNECT controlplane.tailscale.com:443 HTTP/1.1
Host: controlplane.tailscale.com:443
User-Agent: Go-http-client/1.1

FAIL	tailscale.com/tstest/integration
andrew-d added a commit that referenced this issue May 1, 2024
@andrew-d
tstest/integration: mark TestControlTimeLogLine as flaky
1882c5d 
Updates #11962

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I69c8bdfd39c76f33a6e1ed5848187d8003fb64bc
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
ipn/ipnlocal, tstest/integration: add panic to catch flaky test in th…
48ffa77 
…e act

Updates #11962

Change-Id: Ifa24b82f9c76639bfd83278a7c2fe9cf42897bbb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
@bradfitz bradfitz mentioned this issue May 6, 2024
Merged
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
ipn/ipnlocal, tstest/integration: add panic to catch flaky test in th…
f09381a 
…e act

Updates #11962

Change-Id: Ifa24b82f9c76639bfd83278a7c2fe9cf42897bbb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
ipn/ipnlocal, tstest/integration: add panic to catch flaky test in th…
30404cf 
…e act

Updates #11962

Change-Id: Ifa24b82f9c76639bfd83278a7c2fe9cf42897bbb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
net/portmapper: add envknob to disable portmapper in localhost integr…
1ea3c04 
…ation tests

Updates #11962

Change-Id: I8212cd814985b455d96986de0d4c45f119516cb3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
ipn/ipnlocal, tstest/integration: add panic to catch flaky test in th…
7e0dd61 
…e act

Updates #11962

Change-Id: Ifa24b82f9c76639bfd83278a7c2fe9cf42897bbb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
net/portmapper: add envknob to disable portmapper in localhost integr…
ce8969d 
…ation tests

Updates #11962

Change-Id: I8212cd814985b455d96986de0d4c45f119516cb3
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
tstest/integration: add more debugging, logs to catch flaky test
8bf773c 
Updates #11962

Change-Id: I1ab0db69bdf8d1d535aa2cef434c586311f0fe18
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
ipn/ipnlocal: don't send an empty BrowseToURL w/ WatchIPNBus NotifyIn…
84a3bbd 
…itialState

I noticed this while working on the following fix to #11962.

Updates #11962

Change-Id: I4c5894d8899d1ae8c42f54ecfd4d05a4a7ac598c
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
cmd/tailscale/cli: don't start WatchIPNBus until after up's initial S…
394da10 
…tart

The CLI "up" command is a historical mess, both on the CLI side and
the LocalBackend side. We're getting closer to cleaning it up, but in
the meantime it was again implicated in flaky tests.

In this case, the background goroutine running WatchIPNBus was very
occasionally running enough to get to its StartLoginInteractive call
before the original goroutine did its Start call. That meant
integration tests were very rarely but sometimes logging in with the
default control plane URL out on the internet
(controlplane.tailscale.com) instead of the localhost control server
for tests.

This also might've affected new Headscale etc users on initial "up".

Fixes #11962

Change-Id: I36f8817b69267a99271b5ee78cb7dbf0fcc0bd34
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
@bradfitz bradfitz mentioned this issue May 6, 2024
Merged
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
cmd/tailscale/cli: don't start WatchIPNBus until after up's initial S…
7e1b0d8 
…tart

The CLI "up" command is a historical mess, both on the CLI side and
the LocalBackend side. We're getting closer to cleaning it up, but in
the meantime it was again implicated in flaky tests.

In this case, the background goroutine running WatchIPNBus was very
occasionally running enough to get to its StartLoginInteractive call
before the original goroutine did its Start call. That meant
integration tests were very rarely but sometimes logging in with the
default control plane URL out on the internet
(controlplane.tailscale.com) instead of the localhost control server
for tests.

This also might've affected new Headscale etc users on initial "up".

Fixes #11960
Fixes #11962

Change-Id: I36f8817b69267a99271b5ee78cb7dbf0fcc0bd34
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
ipn/ipnlocal: don't send an empty BrowseToURL w/ WatchIPNBus NotifyIn…
3831337 
…itialState

I noticed this while working on the following fix to #11962.

Updates #11962

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I4c5894d8899d1ae8c42f54ecfd4d05a4a7ac598c
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
cmd/tailscale/cli: don't start WatchIPNBus until after up's initial S…
acf4768 
…tart

The CLI "up" command is a historical mess, both on the CLI side and
the LocalBackend side. We're getting closer to cleaning it up, but in
the meantime it was again implicated in flaky tests.

In this case, the background goroutine running WatchIPNBus was very
occasionally running enough to get to its StartLoginInteractive call
before the original goroutine did its Start call. That meant
integration tests were very rarely but sometimes logging in with the
default control plane URL out on the internet
(controlplane.tailscale.com) instead of the localhost control server
for tests.

This also might've affected new Headscale etc users on initial "up".

Fixes #11960
Fixes #11962

Change-Id: I36f8817b69267a99271b5ee78cb7dbf0fcc0bd34
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
cmd/tailscale/cli: don't start WatchIPNBus until after up's initial S…
0d267e5 
…tart

The CLI "up" command is a historical mess, both on the CLI side and
the LocalBackend side. We're getting closer to cleaning it up, but in
the meantime it was again implicated in flaky tests.

In this case, the background goroutine running WatchIPNBus was very
occasionally running enough to get to its StartLoginInteractive call
before the original goroutine did its Start call. That meant
integration tests were very rarely but sometimes logging in with the
default control plane URL out on the internet
(controlplane.tailscale.com) instead of the localhost control server
for tests.

This also might've affected new Headscale etc users on initial "up".

Fixes #11960
Fixes #11962

Change-Id: I36f8817b69267a99271b5ee78cb7dbf0fcc0bd34
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
tstest/integration: add more debugging, logs to catch flaky test
e26f76a 
Updates #11962

Change-Id: I1ab0db69bdf8d1d535aa2cef434c586311f0fe18
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
ipn/ipnlocal: don't send an empty BrowseToURL w/ WatchIPNBus NotifyIn…
aadb8d9 
…itialState

I noticed this while working on the following fix to #11962.

Updates #11962

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I4c5894d8899d1ae8c42f54ecfd4d05a4a7ac598c
bradfitz added a commit that referenced this issue May 6, 2024
@bradfitz
cmd/tailscale/cli: don't start WatchIPNBus until after up's initial S…
f3d2fd2 
…tart

The CLI "up" command is a historical mess, both on the CLI side and
the LocalBackend side. We're getting closer to cleaning it up, but in
the meantime it was again implicated in flaky tests.

In this case, the background goroutine running WatchIPNBus was very
occasionally running enough to get to its StartLoginInteractive call
before the original goroutine did its Start call. That meant
integration tests were very rarely but sometimes logging in with the
default control plane URL out on the internet
(controlplane.tailscale.com) instead of the localhost control server
for tests.

This also might've affected new Headscale etc users on initial "up".

Fixes #11960
Fixes #11962

Change-Id: I36f8817b69267a99271b5ee78cb7dbf0fcc0bd34
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cmd/tailscale/cli: don't start WatchIPNBus until after up's initial Start tailscale/tailscale
1 participant
@andrew-d