From fa8cd96108d4e71e2c499469046f5fb22a3cdda2 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Tue, 12 Oct 2021 17:20:14 +0200 Subject: [PATCH 1/6] Get peers from namespaces where shared nodes are shared to This is rather shameful. Shared nodes should have never worked without this. --- machine.go | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/machine.go b/machine.go index 2fa4d9e0..f97169b2 100644 --- a/machine.go +++ b/machine.go @@ -78,13 +78,13 @@ func (h *Headscale) getDirectPeers(m *Machine) (Machines, error) { return machines, nil } +// getShared fetches machines that are shared to the `Namespace` of the machine we are getting peers for func (h *Headscale) getShared(m *Machine) (Machines, error) { log.Trace(). Str("func", "getShared"). Str("machine", m.Name). Msg("Finding shared peers") - // We fetch here machines that are shared to the `Namespace` of the machine we are getting peers for sharedMachines := []SharedMachine{} if err := h.db.Preload("Namespace").Preload("Machine").Where("namespace_id = ?", m.NamespaceID).Find(&sharedMachines).Error; err != nil { @@ -105,6 +105,37 @@ func (h *Headscale) getShared(m *Machine) (Machines, error) { return peers, nil } +// getSharedTo fetches the machines of the namespaces this machine is shared in +func (h *Headscale) getSharedTo(m *Machine) (Machines, error) { + log.Trace(). + Str("func", "getSharedTo"). + Str("machine", m.Name). + Msg("Finding peers in namespaces this machine is shared with") + + sharedMachines := []SharedMachine{} + if err := h.db.Preload("Namespace").Preload("Machine").Where("machine_id = ?", + m.ID).Find(&sharedMachines).Error; err != nil { + return Machines{}, err + } + + peers := make(Machines, 0) + for _, sharedMachine := range sharedMachines { + namespaceMachines, err := h.ListMachinesInNamespace(sharedMachine.Namespace.Name) + if err != nil { + return Machines{}, err + } + peers = append(peers, *namespaceMachines...) + } + + sort.Slice(peers, func(i, j int) bool { return peers[i].ID < peers[j].ID }) + + log.Trace(). + Str("func", "getSharedTo"). + Str("machine", m.Name). + Msgf("Found peers we are shared with: %s", peers.String()) + return peers, nil +} + func (h *Headscale) getPeers(m *Machine) (Machines, error) { direct, err := h.getDirectPeers(m) if err != nil { @@ -118,13 +149,24 @@ func (h *Headscale) getPeers(m *Machine) (Machines, error) { shared, err := h.getShared(m) if err != nil { log.Error(). - Str("func", "getDirectPeers"). + Str("func", "getShared"). + Err(err). + Msg("Cannot fetch peers") + return Machines{}, err + } + + sharedTo, err := h.getSharedTo(m) + if err != nil { + log.Error(). + Str("func", "sharedTo"). Err(err). Msg("Cannot fetch peers") return Machines{}, err } peers := append(direct, shared...) + peers = append(peers, sharedTo...) + sort.Slice(peers, func(i, j int) bool { return peers[i].ID < peers[j].ID }) log.Trace(). From ebfb8c8c5ee512fec956b325aeca03d93b4619dd Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Wed, 13 Oct 2021 20:48:50 +0200 Subject: [PATCH 2/6] Fix tests, as IDs of Machines where wrongly starting in 0 --- sharing_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/sharing_test.go b/sharing_test.go index a39160bd..45ba40d8 100644 --- a/sharing_test.go +++ b/sharing_test.go @@ -274,7 +274,7 @@ func (s *Suite) TestComplexSharingAcrossNamespaces(c *check.C) { c.Assert(err, check.NotNil) m1 := &Machine{ - ID: 0, + ID: 1, MachineKey: "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", NodeKey: "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", DiscoKey: "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", @@ -291,7 +291,7 @@ func (s *Suite) TestComplexSharingAcrossNamespaces(c *check.C) { c.Assert(err, check.IsNil) m2 := &Machine{ - ID: 1, + ID: 2, MachineKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", NodeKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", DiscoKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", @@ -308,7 +308,7 @@ func (s *Suite) TestComplexSharingAcrossNamespaces(c *check.C) { c.Assert(err, check.IsNil) m3 := &Machine{ - ID: 2, + ID: 3, MachineKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", NodeKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", DiscoKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", @@ -325,7 +325,7 @@ func (s *Suite) TestComplexSharingAcrossNamespaces(c *check.C) { c.Assert(err, check.IsNil) m4 := &Machine{ - ID: 3, + ID: 4, MachineKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", NodeKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", DiscoKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", @@ -391,7 +391,7 @@ func (s *Suite) TestDeleteSharedMachine(c *check.C) { c.Assert(err, check.NotNil) m1 := &Machine{ - ID: 0, + ID: 1, MachineKey: "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", NodeKey: "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", DiscoKey: "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", @@ -408,7 +408,7 @@ func (s *Suite) TestDeleteSharedMachine(c *check.C) { c.Assert(err, check.IsNil) m2 := &Machine{ - ID: 1, + ID: 2, MachineKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", NodeKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", DiscoKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", @@ -425,7 +425,7 @@ func (s *Suite) TestDeleteSharedMachine(c *check.C) { c.Assert(err, check.IsNil) m3 := &Machine{ - ID: 2, + ID: 3, MachineKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", NodeKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", DiscoKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", @@ -442,7 +442,7 @@ func (s *Suite) TestDeleteSharedMachine(c *check.C) { c.Assert(err, check.IsNil) m4 := &Machine{ - ID: 3, + ID: 4, MachineKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", NodeKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", DiscoKey: "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", From 6aa763a1ae3e9063c901385d5a405b50ce79c9e5 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Wed, 13 Oct 2021 20:56:32 +0200 Subject: [PATCH 3/6] Expanded unit tests to better cover sharing nodes --- sharing_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/sharing_test.go b/sharing_test.go index 45ba40d8..d8cd8029 100644 --- a/sharing_test.go +++ b/sharing_test.go @@ -343,7 +343,7 @@ func (s *Suite) TestComplexSharingAcrossNamespaces(c *check.C) { p1s, err := h.getPeers(m1) c.Assert(err, check.IsNil) - c.Assert(len(p1s), check.Equals, 1) // nodes 1 and 4 + c.Assert(len(p1s), check.Equals, 1) // node1 can see node4 c.Assert(p1s[0].Name, check.Equals, "test_get_shared_nodes_4") err = h.AddSharedMachineToNamespace(m2, n1) @@ -351,18 +351,24 @@ func (s *Suite) TestComplexSharingAcrossNamespaces(c *check.C) { p1sAfter, err := h.getPeers(m1) c.Assert(err, check.IsNil) - c.Assert(len(p1sAfter), check.Equals, 2) // nodes 1, 2, 4 + c.Assert(len(p1sAfter), check.Equals, 2) // node1 can see node2 (shared) and node4 (same namespace) c.Assert(p1sAfter[0].Name, check.Equals, "test_get_shared_nodes_2") c.Assert(p1sAfter[1].Name, check.Equals, "test_get_shared_nodes_4") node1shared, err := h.getShared(m1) c.Assert(err, check.IsNil) - c.Assert(len(node1shared), check.Equals, 1) // nodes 1, 2, 4 + c.Assert(len(node1shared), check.Equals, 1) // node1 can see node2 as shared c.Assert(node1shared[0].Name, check.Equals, "test_get_shared_nodes_2") pAlone, err := h.getPeers(m3) c.Assert(err, check.IsNil) - c.Assert(len(pAlone), check.Equals, 0) // node 3 is alone + c.Assert(len(pAlone), check.Equals, 0) // node3 is alone + + pSharedTo, err := h.getPeers(m2) + c.Assert(err, check.IsNil) + c.Assert(len(pSharedTo), check.Equals, 2) // node2 should see node1 (sharedTo) and node4 (sharedTo), as is shared in namespace1 + c.Assert(pSharedTo[0].Name, check.Equals, "test_get_shared_nodes_1") + c.Assert(pSharedTo[1].Name, check.Equals, "test_get_shared_nodes_4") } func (s *Suite) TestDeleteSharedMachine(c *check.C) { From 3e1e07e8c153517155beb727b5be4cec2c1e9903 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Thu, 14 Oct 2021 22:37:44 +0200 Subject: [PATCH 4/6] Fixed integration tests for shared nodes --- integration_test.go | 70 ++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/integration_test.go b/integration_test.go index 9bc51f99..80410361 100644 --- a/integration_test.go +++ b/integration_test.go @@ -58,11 +58,11 @@ func TestIntegrationTestSuite(t *testing.T) { s.namespaces = map[string]TestNamespace{ "main": { - count: 20, + count: 5, tailscales: make(map[string]dockertest.Resource), }, "shared": { - count: 5, + count: 2, tailscales: make(map[string]dockertest.Resource), }, } @@ -503,44 +503,42 @@ func (s *IntegrationTestSuite) TestSharedNodes() { for hostname := range shared.tailscales { assert.Contains(s.T(), result, hostname) } + time.Sleep(100 * time.Second) // Wait for the nodes to receive updates - // TODO(kradalby): Figure out why these connections are not set up - // // TODO: See if we can have a more deterministic wait here. - // time.Sleep(100 * time.Second) + mainIps, err := getIPs(main.tailscales) + assert.Nil(s.T(), err) - // mainIps, err := getIPs(main.tailscales) - // assert.Nil(s.T(), err) + sharedIps, err := getIPs(shared.tailscales) + assert.Nil(s.T(), err) - // sharedIps, err := getIPs(shared.tailscales) - // assert.Nil(s.T(), err) + for hostname, tailscale := range main.tailscales { + for peername, ip := range sharedIps { + s.T().Run(fmt.Sprintf("%s-%s", hostname, peername), func(t *testing.T) { + // We currently cant ping ourselves, so skip that. + if peername != hostname { + // We are only interested in "direct ping" which means what we + // might need a couple of more attempts before reaching the node. + command := []string{ + "tailscale", "ping", + "--timeout=15s", + "--c=20", + "--until-direct=true", + ip.String(), + } - // for hostname, tailscale := range main.tailscales { - // for peername, ip := range sharedIps { - // s.T().Run(fmt.Sprintf("%s-%s", hostname, peername), func(t *testing.T) { - // // We currently cant ping ourselves, so skip that. - // if peername != hostname { - // // We are only interested in "direct ping" which means what we - // // might need a couple of more attempts before reaching the node. - // command := []string{ - // "tailscale", "ping", - // "--timeout=1s", - // "--c=20", - // "--until-direct=true", - // ip.String(), - // } - - // fmt.Printf("Pinging from %s (%s) to %s (%s)\n", hostname, mainIps[hostname], peername, ip) - // result, err := executeCommand( - // &tailscale, - // command, - // ) - // assert.Nil(t, err) - // fmt.Printf("Result for %s: %s\n", hostname, result) - // assert.Contains(t, result, "pong") - // } - // }) - // } - // } + fmt.Printf("Pinging from %s (%s) to %s (%s)\n", hostname, mainIps[hostname], peername, ip) + result, err := executeCommand( + &tailscale, + command, + []string{}, + ) + assert.Nil(t, err) + fmt.Printf("Result for %s: %s\n", hostname, result) + assert.Contains(t, result, "pong") + } + }) + } + } } func (s *IntegrationTestSuite) TestTailDrop() { From be36480a64770ff508aafe1f72a267fa2c8d3dc9 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sat, 16 Oct 2021 11:06:33 +0200 Subject: [PATCH 5/6] Reverted back values in integration tests --- integration_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration_test.go b/integration_test.go index 80410361..aed805e8 100644 --- a/integration_test.go +++ b/integration_test.go @@ -58,11 +58,11 @@ func TestIntegrationTestSuite(t *testing.T) { s.namespaces = map[string]TestNamespace{ "main": { - count: 5, + count: 20, tailscales: make(map[string]dockertest.Resource), }, "shared": { - count: 2, + count: 5, tailscales: make(map[string]dockertest.Resource), }, } From d0daff180e82db621189d6916595535d2ebc06f1 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sat, 16 Oct 2021 11:36:16 +0200 Subject: [PATCH 6/6] Added TODO in waiting --- integration_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/integration_test.go b/integration_test.go index aed805e8..3c51215d 100644 --- a/integration_test.go +++ b/integration_test.go @@ -503,6 +503,8 @@ func (s *IntegrationTestSuite) TestSharedNodes() { for hostname := range shared.tailscales { assert.Contains(s.T(), result, hostname) } + + // TODO(juanfont): We have to find out why do we need to wait time.Sleep(100 * time.Second) // Wait for the nodes to receive updates mainIps, err := getIPs(main.tailscales)