Codereorg: Part 5, state #1492

kradalby · 2023-06-21T09:31:01Z

This PR reworks the state management in headscale, instead of relying on timestamps and frequent checking, and sending the entire world every time, we now send incremental updates based on the change that has happened.

Closes #1390
Fixes #1461

kradalby · 2023-07-07T11:48:54Z

All tests pass locally, it is hard to know if Github actions is reliable or not.

kradalby · 2023-07-07T11:51:01Z

My observation so far is that tests including a larger amount of nodes are more unreliable, particularly with excess network requirements:

PingAll (the base of the typically failed ones)
Taildrop

Signed-off-by: Kristoffer Dalby <[email protected]>

This commit replaces the timestamp based state system with a new one that has update channels directly to the connected nodes. It will send an update to all listening clients via the polling mechanism. It introduces a new package notifier, which has a concurrency safe manager for all our channels to the connected nodes. Signed-off-by: Kristoffer Dalby <[email protected]>

This commits extends the mapper with functions for creating "delta" MapResponses for different purposes (peer changed, peer removed, derp). This wires up the new state management with a new StateUpdate struct letting the poll worker know what kind of update to send to the connected nodes. Signed-off-by: Kristoffer Dalby <[email protected]>

Co-Authored-By: Jason <[email protected]> Signed-off-by: Kristoffer Dalby <[email protected]>

Signed-off-by: Kristoffer Dalby <[email protected]>

Closes juanfont#1544 Co-Authored-By: Patrick Huang <[email protected]> Signed-off-by: Kristoffer Dalby <[email protected]>

Signed-off-by: Kristoffer Dalby <[email protected]>

This field is no longer used, it was used in our old state "algorithm" to determine if we should send an update. Signed-off-by: Kristoffer Dalby <[email protected]>

Signed-off-by: Kristoffer Dalby <[email protected]>

This commit rearranges the poll handler to immediatly accept updates and notify its peers and return, not travel down the function for a bit. This reduces the DB calls and other holdups that isnt necessary to send a "lite response", a map response without peers, or accepting an endpoint update. Signed-off-by: Kristoffer Dalby <[email protected]>

Signed-off-by: Kristoffer Dalby <[email protected]>

vsychov · 2023-09-15T21:22:49Z

Hello there,
Thank you for your hard work on headscale!

I'm start testing this changes, and found bug with "on-the-fly" state update:

Start headscale.
Connect client (macos): /Applications/Tailscale.app/Contents/MacOS/Tailscale up --login-server=https://headscale-test.example.com/
Connect headscale-test-1 (linux): tailscale login --login-server=https://headscale-test.example.com/ --advertise-exit-node
Connect headscale-test-ams3-1 and headscale-test-ams3-2 (linux): tailscale up --login-server=https://headscale-test.example.com/ --advertise-routes=10.0.0.0/8 --ssh
Approve the routes and verify:

ID | Machine               | Prefix     | Advertised | Enabled | Primary
1  | headscale-test-1      | 0.0.0.0/0  | true       | true    | -
2  | headscale-test-1      | ::/0       | true       | true    | -
4  | headscale-test-ams3-2 | 10.0.0.0/8 | true       | true    | true
3  | headscale-test-ams3-1 | 10.0.0.0/8 | true       | true    | false

Try to ping:

/Applications/Tailscale.app/Contents/MacOS/Tailscale ping 10.110.0.53
2023/09/15 23:06:47 no matching peer

➜  ~ /Applications/Tailscale.app/Contents/MacOS/Tailscale status
100.64.0.1      node1   user.example.com macOS   -
100.64.0.2      headscale-test-1     user.example.com linux   -
100.64.0.3      headscale-test-ams3-1..example.com user.example.com linux   -
100.64.0.4      headscale-test-ams3-2..example.com user.example.com linux   -

The exit-node on headscale-test-1 does not appear, and the ping to 10.110.0.53 does not work (though it should through headscale-test-ams3-1).
The problem is resolved by restarting headscale, but the status does not update until the restart (I wait ~10 min until restart headscale).

vsychov · 2023-09-15T21:48:35Z

It seems there is also an issue with updating the "last-seen"/"online" status. I wanted to test the subnet-router failover, but as soon as I executed tailscale down on headscale-test-ams3-2, I expected the traffic to switch through headscale-test-ams3-1. However, headscale-test-ams3-1 was unexpectedly marked as "offline":

root@headscale-f7489674b-4j5jf:/# date
Fri Sep 15 21:30:43 UTC 2023
root@headscale-f7489674b-4j5jf:/# headscale nodes list
ID | Hostname              | Name                  | MachineKey | NodeKey | User                 | IP addresses                  | Ephemeral | Last seen           | Expiration          | Online  | Expired
....
3  | headscale-test-ams3-1 | headscale-test-ams3-1 | [K4v5r]    | [LkHjz] | user.example.com | 100.64.0.3, fd7a:115c:a1e0::3 | false     | 2023-09-15 21:14:13 | 2023-09-16 16:58:33 | offline | no
....
root@headscale-f7489674b-4j5jf:/#

Although there are no network connectivity issues, tailscale is working, and headscale is accessible via http:

root@headscale-test-ams3-1:~# systemctl status tailscaled.service
● tailscaled.service - Tailscale node agent
     Loaded: loaded (/lib/systemd/system/tailscaled.service; enabled; vendor preset: enabled)
     Active: active (running) since Fri 2023-09-15 20:57:34 UTC; 32min ago
       Docs: https://tailscale.com/kb/
    Process: 8483 ExecStartPre=/usr/sbin/tailscaled --cleanup (code=exited, status=0/SUCCESS)
   Main PID: 8511 (tailscaled)
     Status: "Connected; user.example.com; 100.64.0.3 fd7a:115c:a1e0::3"
      Tasks: 8 (limit: 1116)
     Memory: 39.5M
        CPU: 1.760s
     CGroup: /system.slice/tailscaled.service
             └─8511 /usr/sbin/tailscaled --state=/var/lib/tailscale/tailscaled.state --socket=/run/tailscale/tailscaled.sock --port=41641
root@headscale-test-ams3-1:~# curl -I https://headscale-test.example.com/api/v1/machine
HTTP/2 401
date: Fri, 15 Sep 2023 21:30:33 GMT
content-type: text/plain; charset=utf-8
content-length: 12
strict-transport-security: max-age=15724800; includeSubDomains

root@headscale-test-ams3-1:~#

kradalby · 2023-09-18T15:05:42Z

@vsychov Thank you, I appreciate this a lot. I think what I will do since this PR is so messy already is to get it in, and then it would be great if you could create two issues for me on this (and other things you find), and then we make them blocking before alpha.

I would also appreciate your continued testing of this work.

juanfont · 2023-09-19T15:18:53Z

Approved!

juanfont · 2023-08-20T17:04:21Z

hscontrol/mapper/mapper.go

+	changed := make(types.Machines, len(machineKeys))
+	lastSeen := make(map[tailcfg.NodeID]bool)
+	for idx, machineKey := range machineKeys {
+		peer, err := m.db.GetMachineByID(machineKey)


Mixing MachineKey with IDs of the DB?

juanfont · 2023-08-20T17:04:50Z

hscontrol/notifier/notifier.go

 }

 func NewNotifier() *Notifier {
 	return &Notifier{}
 }

-func (n *Notifier) AddNode(machineKey string, c chan<- struct{}) {
+func (n *Notifier) AddNode(machineKey string, c chan<- types.StateUpdate) {


Here seems to be the MachineKey of the Node.

We might have nodes with the same machinekey but different nodekeys, right?

kradalby · 2023-09-24T11:21:52Z

@vsychov did you try the tests with the route/exit node issue including the last commit? Would you mind opening an issue for that one?

I'll have a look at the online status now.

vsychov · 2023-09-26T09:24:07Z

@kradalby , I'll try my tests on current main revision, and if that still reproducible, I'll create issues for that.

vsychov · 2023-09-26T11:35:16Z

Problems still appear, I created issue: #1561

kradalby force-pushed the codereorg-p5-state branch 2 times, most recently from 631d1a8 to e4b0e82 Compare June 24, 2023 08:05

kradalby marked this pull request as ready for review June 25, 2023 11:31

kradalby requested a review from juanfont as a code owner June 25, 2023 11:31

kradalby marked this pull request as draft June 25, 2023 12:14

kradalby force-pushed the codereorg-p5-state branch 4 times, most recently from 32a0567 to b5be2ef Compare July 5, 2023 17:13

This was referenced Jul 7, 2023

Fix hangs on SIGTERM. #1480

Closed

Strange shutdown/restart behavior on 0.22.2 #1461

Closed

kradalby mentioned this pull request Jul 7, 2023

Frequent "offline" status causing subnet router re-election and connection disruptions #1501

Closed

kradalby marked this pull request as ready for review July 7, 2023 20:02

kradalby force-pushed the codereorg-p5-state branch 2 times, most recently from 260c535 to 1b344ac Compare July 14, 2023 14:50

kradalby marked this pull request as draft July 14, 2023 15:29

kradalby force-pushed the codereorg-p5-state branch from 55aca05 to 5b8053f Compare July 17, 2023 11:35

kradalby force-pushed the codereorg-p5-state branch from 7071644 to 0b4dc82 Compare July 26, 2023 05:24

vsychov mentioned this pull request Aug 15, 2023

Enhancement: Enable Account Switching for Non-Ephemeral Devices on Logout #1528

Closed

kradalby force-pushed the codereorg-p5-state branch 3 times, most recently from 546f1f0 to 0864811 Compare August 29, 2023 14:19

kradalby force-pushed the codereorg-p5-state branch from 0864811 to 4bdfde3 Compare September 5, 2023 07:37

kradalby and others added 5 commits September 11, 2023 05:08

format with prettier 3.0

a885e1f

Signed-off-by: Kristoffer Dalby <[email protected]>

Add missing return in shutdown

a52db30

Co-Authored-By: Jason <[email protected]> Signed-off-by: Kristoffer Dalby <[email protected]>

add script to run integration tests

8e973f4

Signed-off-by: Kristoffer Dalby <[email protected]>

kradalby force-pushed the codereorg-p5-state branch 2 times, most recently from 93788d5 to 1d7b736 Compare September 11, 2023 10:59

kradalby and others added 5 commits September 11, 2023 06:02

give ci more tollerance for timeouts

db5864d

Signed-off-by: Kristoffer Dalby <[email protected]>

Upgrade go and debian in headscale docker

df1f4b3

Signed-off-by: Kristoffer Dalby <[email protected]>

add lock around saving ts clients

b6d64e1

Closes juanfont#1544 Co-Authored-By: Patrick Huang <[email protected]> Signed-off-by: Kristoffer Dalby <[email protected]>

gitignore infolder tailscale

8882825

Signed-off-by: Kristoffer Dalby <[email protected]>

order path

4b4b032

Signed-off-by: Kristoffer Dalby <[email protected]>

kradalby force-pushed the codereorg-p5-state branch from 1d7b736 to 86937b9 Compare September 11, 2023 11:03

kradalby added 4 commits September 11, 2023 06:04

add pprof endpoint

db8761f

Signed-off-by: Kristoffer Dalby <[email protected]>

Remove LastSuccessfulUpdate from Machine

ec39145

This field is no longer used, it was used in our old state "algorithm" to determine if we should send an update. Signed-off-by: Kristoffer Dalby <[email protected]>

improve debug logging, rw lock for notifier

decbeb7

Signed-off-by: Kristoffer Dalby <[email protected]>

kradalby force-pushed the codereorg-p5-state branch from 86937b9 to 1f4d51d Compare September 11, 2023 11:46

handle route updates correctly

bc9fc67

Signed-off-by: Kristoffer Dalby <[email protected]>

kradalby marked this pull request as ready for review September 12, 2023 03:19

juanfont approved these changes Sep 19, 2023

View reviewed changes

kradalby merged commit 096ac31 into juanfont:main Sep 19, 2023
44 checks passed

kradalby deleted the codereorg-p5-state branch September 19, 2023 15:20

tomfitzhenry mentioned this pull request Sep 21, 2023

headscale: fix reacting to SIGTERM NixOS/nixpkgs#254934

Merged

12 tasks

This was referenced Sep 24, 2023

[Code Freeze] Restructuring the code #1473

Closed

FR: Send Peers delta in MapResponse #1383

Closed

vsychov mentioned this pull request Sep 26, 2023

Bugs related to state change in #1492 #1561

Closed

2 tasks

wrenix mentioned this pull request Apr 18, 2024

Missing Metrics for last machine contact #1896

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Codereorg: Part 5, state #1492

Codereorg: Part 5, state #1492

kradalby commented Jun 21, 2023 •

edited

Loading

kradalby commented Jul 7, 2023

kradalby commented Jul 7, 2023

vsychov commented Sep 15, 2023

vsychov commented Sep 15, 2023

kradalby commented Sep 18, 2023

juanfont commented Sep 19, 2023

juanfont Aug 20, 2023

juanfont Aug 20, 2023

juanfont Aug 20, 2023

kradalby commented Sep 24, 2023

vsychov commented Sep 26, 2023

vsychov commented Sep 26, 2023

Codereorg: Part 5, state #1492

Codereorg: Part 5, state #1492

Conversation

kradalby commented Jun 21, 2023 • edited Loading

kradalby commented Jul 7, 2023

kradalby commented Jul 7, 2023

vsychov commented Sep 15, 2023

vsychov commented Sep 15, 2023

kradalby commented Sep 18, 2023

juanfont commented Sep 19, 2023

juanfont Aug 20, 2023

Choose a reason for hiding this comment

juanfont Aug 20, 2023

Choose a reason for hiding this comment

juanfont Aug 20, 2023

Choose a reason for hiding this comment

kradalby commented Sep 24, 2023

vsychov commented Sep 26, 2023

vsychov commented Sep 26, 2023

kradalby commented Jun 21, 2023 •

edited

Loading