Skip to content

Commit

Permalink
Fix and make todo for expired nodes
Browse files Browse the repository at this point in the history
Signed-off-by: Kristoffer Dalby <[email protected]>
  • Loading branch information
kradalby committed Dec 6, 2023
1 parent 3ca953a commit 8b3721b
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 13 deletions.
2 changes: 1 addition & 1 deletion hscontrol/db/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ func (hsdb *HSDatabase) nodeSetExpiry(node *types.Node, expiry time.Time) error
},
}
if stateUpdate.Valid() {
hsdb.notifier.NotifyWithIgnore(stateUpdate, node.MachineKey.String())
hsdb.notifier.NotifyAll(stateUpdate)
}

return nil
Expand Down
1 change: 1 addition & 0 deletions hscontrol/mapper/tail.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ func tailNode(
PrimaryRoutes: primaryPrefixes,

MachineAuthorized: !node.IsExpired(),
Expired: node.IsExpired(),
}

// - 74: 2023-09-18: Client understands NodeCapMap
Expand Down
39 changes: 27 additions & 12 deletions integration/general_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/samber/lo"
"github.com/stretchr/testify/assert"
"tailscale.com/client/tailscale/apitype"
"tailscale.com/types/key"
)

func TestPingAllByIP(t *testing.T) {
Expand Down Expand Up @@ -553,25 +554,39 @@ func TestExpireNode(t *testing.T) {
err = json.Unmarshal([]byte(result), &node)
assertNoErr(t, err)

var expiredNodeKey key.NodePublic
err = expiredNodeKey.UnmarshalText([]byte(node.GetNodeKey()))
assertNoErr(t, err)

t.Logf("Node %s with node_key %s has been expired", node.GetName(), expiredNodeKey.String())

time.Sleep(30 * time.Second)

// Verify that the expired not is no longer present in the Peer list
// of connected nodes.
now := time.Now()

// Verify that the expired node has been marked in all peers list.
for _, client := range allClients {
status, err := client.Status()
assertNoErr(t, err)

for _, peerKey := range status.Peers() {
peerStatus := status.Peer[peerKey]

peerPublicKey := strings.TrimPrefix(peerStatus.PublicKey.String(), "nodekey:")

assert.NotEqual(t, node.GetNodeKey(), peerPublicKey)
}

if client.Hostname() != node.GetName() {
// Assert that we have the original count - self - expired node
assert.Len(t, status.Peers(), len(MustTestVersions)-2)
t.Logf("available peers of %s: %v", client.Hostname(), status.Peers())

// In addition to marking nodes expired, we filter them out during the map response
// this check ensures that the node is either not present, or that it is expired
// if it is in the map response.
if peerStatus, ok := status.Peer[expiredNodeKey]; ok {
assertNotNil(t, peerStatus.Expired)
assert.Truef(t, peerStatus.KeyExpiry.Before(now), "node %s should have a key expire before %s, was %s", peerStatus.HostName, now.String(), peerStatus.KeyExpiry)
assert.Truef(t, peerStatus.Expired, "node %s should be expired, expired is %v", peerStatus.HostName, peerStatus.Expired)
}

// TODO(kradalby): We do not propogate expiry correctly, nodes should be aware
// of their status, and this should be sent directly to the node when its
// expired. This needs a notifier that goes directly to the node (currently we only do peers)
// so fix this in a follow up PR.
// } else {
// assert.True(t, status.Self.Expired)
}
}
}
Expand Down

0 comments on commit 8b3721b

Please sign in to comment.