starred/headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale.git synced 2026-01-11 20:00:28 +01:00
Code Issues 98 Packages Projects Releases 10 Wiki Activity

[PR #83] [MERGED] Improve reliability of PollMapHandler, more integration tests #1247

New Issue
Closed
opened 2025-12-29 02:29:27 +01:00 by adam · 0 comments
adam commented 2025-12-29 02:29:27 +01:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/juanfont/headscale/pull/83
Author: @kradalby
Created: 8/13/2021
Status: ✅ Merged
Merged: 8/13/2021
Merged by: @kradalby

Base: main ← Head: more-integration-tests

📝 Commits (3)

  • 9698abb Resolve merge conflict
  • 700382c Split stream part of pollhandlermap into its own func
  • a8d9fdc Uncomment ping test

📊 Changes

11 files changed (+278 additions, -139 deletions)

View changed files

📝 .dockerignore (+1 -0)
📝 api.go (+30 -95)
📝 app.go (+1 -4)
📝 go.mod (+1 -0)
📝 go.sum (+1 -0)
📝 integration_test.go (+117 -28)
📝 machine.go (+10 -2)
📝 namespaces.go (+1 -4)
➕ poll.go (+98 -0)
📝 routes.go (+1 -6)
📝 utils.go (+17 -0)

📄 Description

This PR adds several more integration tests with Tailscale, and tries to resolve some of the issues that arise from this in PollNetMapHandler:

  • Integration test ensuring all Nodes are present in each note tailscale status output
  • Integration test pinging all nodes from all nodes (cross-ping)

These test will mostly pass, however, there is still some timing and/or race conditions preventing them from being completely reliable. This will be work for the future now that we have a reference to compare against.

The integration tests has switched from gocheck to testify, as they provide a much richer test setup (and most importantly allow for sub-tests in for loops)

Current state

When setting up five tailscale nodes via the integration tests and running 20 tests we get the following result:

------------------------------------------
Full successful runs: 18
Failed runs: 2
------------------------------------------

Which means that most of the runs, all nodes got the appropriate updates and was able to see all nodes in the mesh.

When trying to run test for cross-ping, the tests were slightly more unreliable:

------------------------------------------
Full successful runs: 13
Failed runs: 7
------------------------------------------

Which seem to be a product that there is not two failure modes:

  • Not having the correct maps, preventing us from pinging a host we dont know about
  • tailscale ping fails for other reasons. e.g. connection not set up.

These results are after some work on breaking up PollMapHandler and ensuring that most nodes can enter the "stream" mode correctly (see comment in PR).

Apologise for the massive commit, there was a bit of trying and failing + a merge.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/juanfont/headscale/pull/83 **Author:** [@kradalby](https://github.com/kradalby) **Created:** 8/13/2021 **Status:** ✅ Merged **Merged:** 8/13/2021 **Merged by:** [@kradalby](https://github.com/kradalby) **Base:** `main` ← **Head:** `more-integration-tests` --- ### 📝 Commits (3) - [`9698abb`](https://github.com/juanfont/headscale/commit/9698abbfd5ae19d3a49bb876b8cffa0a7a746790) Resolve merge conflict - [`700382c`](https://github.com/juanfont/headscale/commit/700382cba492fef8e1d923e4ae4343589463b26f) Split stream part of pollhandlermap into its own func - [`a8d9fdc`](https://github.com/juanfont/headscale/commit/a8d9fdce3c6e2bf4966841f46c9fa8fe04d1e0e8) Uncomment ping test ### 📊 Changes **11 files changed** (+278 additions, -139 deletions) <details> <summary>View changed files</summary> 📝 `.dockerignore` (+1 -0) 📝 `api.go` (+30 -95) 📝 `app.go` (+1 -4) 📝 `go.mod` (+1 -0) 📝 `go.sum` (+1 -0) 📝 `integration_test.go` (+117 -28) 📝 `machine.go` (+10 -2) 📝 `namespaces.go` (+1 -4) ➕ `poll.go` (+98 -0) 📝 `routes.go` (+1 -6) 📝 `utils.go` (+17 -0) </details> ### 📄 Description This PR adds several more integration tests with `Tailscale`, and tries to resolve some of the issues that arise from this in `PollNetMapHandler`: - Integration test ensuring all Nodes are present in each note `tailscale status` output - Integration test pinging all nodes from all nodes (cross-ping) These test will mostly pass, however, there is still some timing and/or race conditions preventing them from being completely reliable. This will be work for the future now that we have a reference to compare against. The integration tests has switched from gocheck to [testify](https://pkg.go.dev/github.com/stretchr/testify/suite?utm_source=godoc#pkg-overview), as they provide a much richer test setup (and most importantly allow for sub-tests in for loops) ### Current state When setting up five tailscale nodes via the integration tests and running 20 tests we get the following result: ``` ------------------------------------------ Full successful runs: 18 Failed runs: 2 ------------------------------------------ ``` Which means that most of the runs, all nodes got the appropriate updates and was able to see all nodes in the mesh. When trying to run test for cross-ping, the tests were slightly more unreliable: ``` ------------------------------------------ Full successful runs: 13 Failed runs: 7 ------------------------------------------ ``` Which seem to be a product that there is not two failure modes: - Not having the correct maps, preventing us from pinging a host we dont know about - `tailscale ping` fails for other reasons. e.g. connection not set up. These results are after some work on breaking up `PollMapHandler` and ensuring that most nodes can enter the "stream" mode correctly (see comment in PR). **Apologise** for the massive commit, there was a bit of trying and failing + a merge. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2025-12-29 02:29:27 +01:00
adam closed this issue 2025-12-29 02:29:27 +01:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
CLI
DERP
DNS
Nix
OIDC
SSH
bug
database
documentation
duplicate
enhancement
faq
good first issue
grants
help wanted
might-come
needs design doc
needs investigation
no-stale-bot
out of scope
performance
policy 📝
pull-request
Mirrored from GitHub Pull Request
 question
regression
routes
stale
tags
tailscale-feature-gap
well described ❤️
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/headscale#1247
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?