Fix ip allocation bug, make integration tests faster

.github/workflows/test-integration.yml

@@ -29,4 +29,4 @@ jobs:
 
       - name: Run Integration tests
         if: steps.changed-files.outputs.any_changed == 'true'
-        run: go test -tags integration -timeout 30m
+        run: make test_integration

CHANGELOG.md

@@ -2,7 +2,13 @@
 
 **TBD (TBD):**
 
-**0.14.0 (2022-xx-xx):**
+**0.15.0 (2022-xx-xx):**
+
+**Changes**:
+
+- Fix a bug were the same IP could be assigned to multiple hosts if joined in quick succession [#346](https://github.com/juanfont/headscale/pull/346)
+
+**0.14.0 (2022-02-25):**
 
 **UPCOMING BREAKING**:
 From the **next** version (`0.15.0`), all machines will be able to communicate regardless of

Makefile

@@ -18,7 +18,7 @@ test:
 	@go test -coverprofile=coverage.out ./...
 
 test_integration:
-	go test -tags integration -timeout 30m -count=1 ./...
+	go test -failfast -tags integration -timeout 30m -count=1 ./...
 
 test_integration_cli:
 	go test -tags integration -v integration_cli_test.go integration_common_test.go

api.go

@@ -574,6 +574,9 @@ func (h *Headscale) handleAuthKey(
 			Str("func", "handleAuthKey").
 			Str("machine", machine.Name).
 			Msg("Authentication key was valid, proceeding to acquire IP addresses")
+
+		h.ipAllocationMutex.Lock()
+
 		ips, err := h.getAvailableIPs()
 		if err != nil {
 			log.Error().
@@ -602,6 +605,8 @@ func (h *Headscale) handleAuthKey(
 		machine.Registered = true
 		machine.RegisterMethod = RegisterMethodAuthKey
 		h.db.Save(&machine)
+
+		h.ipAllocationMutex.Unlock()
 	}
 
 	pak.Used = true

app.go

@@ -153,6 +153,8 @@ type Headscale struct {
 	oidcStateCache *cache.Cache
 
 	requestedExpiryCache *cache.Cache
+
+	ipAllocationMutex sync.Mutex
 }
 
 // Look up the TLS constant relative to user-supplied TLS client

integration_test.go

@@ -15,6 +15,7 @@ import (
 	"os"
 	"path"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -44,6 +45,8 @@ type IntegrationTestSuite struct {
 	headscale dockertest.Resource
 
 	namespaces map[string]TestNamespace
+
+	joinWaitGroup sync.WaitGroup
 }
 
 func TestIntegrationTestSuite(t *testing.T) {
@@ -118,7 +121,7 @@ func (s *IntegrationTestSuite) saveLog(
 		return err
 	}
 
-	fmt.Printf("Saving logs for %s to %s\n", resource.Container.Name, basePath)
+	log.Printf("Saving logs for %s to %s\n", resource.Container.Name, basePath)
 
 	err = ioutil.WriteFile(
 		path.Join(basePath, resource.Container.Name+".stdout.log"),
@@ -141,6 +144,34 @@ func (s *IntegrationTestSuite) saveLog(
 	return nil
 }
 
+func (s *IntegrationTestSuite) Join(
+	endpoint, key, hostname string,
+	tailscale dockertest.Resource,
+) {
+	defer s.joinWaitGroup.Done()
+
+	command := []string{
+		"tailscale",
+		"up",
+		"-login-server",
+		endpoint,
+		"--authkey",
+		key,
+		"--hostname",
+		hostname,
+	}
+
+	log.Println("Join command:", command)
+	log.Printf("Running join command for %s\n", hostname)
+	_, err := ExecuteCommand(
+		&tailscale,
+		command,
+		[]string{},
+	)
+	assert.Nil(s.T(), err)
+	log.Printf("%s joined\n", hostname)
+}
+
 func (s *IntegrationTestSuite) tailscaleContainer(
 	namespace, identifier, version string,
 ) (string, *dockertest.Resource) {
@@ -178,7 +209,7 @@ func (s *IntegrationTestSuite) tailscaleContainer(
 	if err != nil {
 		log.Fatalf("Could not start resource: %s", err)
 	}
-	fmt.Printf("Created %s container\n", hostname)
+	log.Printf("Created %s container\n", hostname)
 
 	return hostname, pts
 }
@@ -221,15 +252,15 @@ func (s *IntegrationTestSuite) SetupSuite() {
 		Cmd:      []string{"headscale", "serve"},
 	}
 
-	fmt.Println("Creating headscale container")
+	log.Println("Creating headscale container")
 	if pheadscale, err := s.pool.BuildAndRunWithBuildOptions(headscaleBuildOptions, headscaleOptions, DockerRestartPolicy); err == nil {
 		s.headscale = *pheadscale
 	} else {
 		log.Fatalf("Could not start resource: %s", err)
 	}
-	fmt.Println("Created headscale container")
+	log.Println("Created headscale container")
 
-	fmt.Println("Creating tailscale containers")
+	log.Println("Creating tailscale containers")
 	for namespace, scales := range s.namespaces {
 		for i := 0; i < scales.count; i++ {
 			version := tailscaleVersions[i%len(tailscaleVersions)]
@@ -243,7 +274,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
 		}
 	}
 
-	fmt.Println("Waiting for headscale to be ready")
+	log.Println("Waiting for headscale to be ready")
 	hostEndpoint := fmt.Sprintf("localhost:%s", s.headscale.GetPort("8080/tcp"))
 
 	if err := s.pool.Retry(func() error {
@@ -266,19 +297,19 @@ func (s *IntegrationTestSuite) SetupSuite() {
 		// https://github.com/stretchr/testify/issues/849
 		return // fmt.Errorf("Could not connect to headscale: %s", err)
 	}
-	fmt.Println("headscale container is ready")
+	log.Println("headscale container is ready")
 
 	for namespace, scales := range s.namespaces {
-		fmt.Printf("Creating headscale namespace: %s\n", namespace)
+		log.Printf("Creating headscale namespace: %s\n", namespace)
 		result, err := ExecuteCommand(
 			&s.headscale,
 			[]string{"headscale", "namespaces", "create", namespace},
 			[]string{},
 		)
-		fmt.Println("headscale create namespace result: ", result)
+		log.Println("headscale create namespace result: ", result)
 		assert.Nil(s.T(), err)
 
-		fmt.Printf("Creating pre auth key for %s\n", namespace)
+		log.Printf("Creating pre auth key for %s\n", namespace)
 		preAuthResult, err := ExecuteCommand(
 			&s.headscale,
 			[]string{
@@ -304,33 +335,16 @@ func (s *IntegrationTestSuite) SetupSuite() {
 
 		headscaleEndpoint := "http://headscale:8080"
 
-		fmt.Printf(
+		log.Printf(
 			"Joining tailscale containers to headscale at %s\n",
 			headscaleEndpoint,
 		)
 		for hostname, tailscale := range scales.tailscales {
-			command := []string{
-				"tailscale",
-				"up",
-				"-login-server",
-				headscaleEndpoint,
-				"--authkey",
-				preAuthKey.Key,
-				"--hostname",
-				hostname,
-			}
-
-			fmt.Println("Join command:", command)
-			fmt.Printf("Running join command for %s\n", hostname)
-			result, err := ExecuteCommand(
-				&tailscale,
-				command,
-				[]string{},
-			)
-			fmt.Println("tailscale result: ", result)
-			assert.Nil(s.T(), err)
-			fmt.Printf("%s joined\n", hostname)
+			s.joinWaitGroup.Add(1)
+			go s.Join(headscaleEndpoint, preAuthKey.Key, hostname, tailscale)
 		}
+
+		s.joinWaitGroup.Wait()
 	}
 
 	// The nodes need a bit of time to get their updated maps from headscale
@@ -350,15 +364,15 @@ func (s *IntegrationTestSuite) HandleStats(
 
 func (s *IntegrationTestSuite) TestListNodes() {
 	for namespace, scales := range s.namespaces {
-		fmt.Println("Listing nodes")
+		log.Println("Listing nodes")
 		result, err := ExecuteCommand(
 			&s.headscale,
 			[]string{"headscale", "--namespace", namespace, "nodes", "list"},
 			[]string{},
 		)
 		assert.Nil(s.T(), err)
 
-		fmt.Printf("List nodes: \n%s\n", result)
+		log.Printf("List nodes: \n%s\n", result)
 
 		// Chck that the correct count of host is present in node list
 		lines := strings.Split(result, "\n")
@@ -381,7 +395,7 @@ func (s *IntegrationTestSuite) TestGetIpAddresses() {
 				s.T().Run(hostname, func(t *testing.T) {
 					assert.NotNil(t, ip)
 
-					fmt.Printf("IP for %s: %s\n", hostname, ip)
+					log.Printf("IP for %s: %s\n", hostname, ip)
 
 					// c.Assert(ip.Valid(), check.IsTrue)
 					assert.True(t, ip.Is4() || ip.Is6())
@@ -410,7 +424,7 @@ func (s *IntegrationTestSuite) TestGetIpAddresses() {
 //			s.T().Run(hostname, func(t *testing.T) {
 //				command := []string{"tailscale", "status", "--json"}
 //
-//				fmt.Printf("Getting status for %s\n", hostname)
+//				log.Printf("Getting status for %s\n", hostname)
 //				result, err := ExecuteCommand(
 //					&tailscale,
 //					command,
@@ -477,7 +491,7 @@ func (s *IntegrationTestSuite) TestPingAllPeersByAddress() {
 								ip.String(),
 							}
 
-							fmt.Printf(
+							log.Printf(
 								"Pinging from %s to %s (%s)\n",
 								hostname,
 								peername,
@@ -489,7 +503,7 @@ func (s *IntegrationTestSuite) TestPingAllPeersByAddress() {
 								[]string{},
 							)
 							assert.Nil(t, err)
-							fmt.Printf("Result for %s: %s\n", hostname, result)
+							log.Printf("Result for %s: %s\n", hostname, result)
 							assert.Contains(t, result, "pong")
 						})
 				}
@@ -512,6 +526,7 @@ func (s *IntegrationTestSuite) TestTailDrop() {
 				}
 				time.Sleep(sleepInverval)
 			}
+
 			return
 		}
 
@@ -534,7 +549,7 @@ func (s *IntegrationTestSuite) TestTailDrop() {
 						fmt.Sprintf("%s:", peername),
 					}
 					retry(10, 1*time.Second, func() error {
-						fmt.Printf(
+						log.Printf(
 							"Sending file from %s to %s\n",
 							hostname,
 							peername,
@@ -573,7 +588,7 @@ func (s *IntegrationTestSuite) TestTailDrop() {
 						"ls",
 						fmt.Sprintf("/tmp/file_from_%s", peername),
 					}
-					fmt.Printf(
+					log.Printf(
 						"Checking file in %s (%s) from %s (%s)\n",
 						hostname,
 						ips[hostname],
@@ -586,7 +601,7 @@ func (s *IntegrationTestSuite) TestTailDrop() {
 						[]string{},
 					)
 					assert.Nil(t, err)
-					fmt.Printf("Result for %s: %s\n", peername, result)
+					log.Printf("Result for %s: %s\n", peername, result)
 					assert.Equal(
 						t,
 						fmt.Sprintf("/tmp/file_from_%s\n", peername),
@@ -616,7 +631,7 @@ func (s *IntegrationTestSuite) TestPingAllPeersByHostname() {
 						fmt.Sprintf("%s.%s.headscale.net", peername, namespace),
 					}
 
-					fmt.Printf(
+					log.Printf(
 						"Pinging using hostname from %s to %s\n",
 						hostname,
 						peername,
@@ -627,7 +642,7 @@ func (s *IntegrationTestSuite) TestPingAllPeersByHostname() {
 						[]string{},
 					)
 					assert.Nil(t, err)
-					fmt.Printf("Result for %s: %s\n", hostname, result)
+					log.Printf("Result for %s: %s\n", hostname, result)
 					assert.Contains(t, result, "pong")
 				})
 			}
@@ -650,7 +665,7 @@ func (s *IntegrationTestSuite) TestMagicDNS() {
 						fmt.Sprintf("%s.%s.headscale.net", peername, namespace),
 					}
 
-					fmt.Printf(
+					log.Printf(
 						"Resolving name %s from %s\n",
 						peername,
 						hostname,
@@ -661,7 +676,7 @@ func (s *IntegrationTestSuite) TestMagicDNS() {
 						[]string{},
 					)
 					assert.Nil(t, err)
-					fmt.Printf("Result for %s: %s\n", hostname, result)
+					log.Printf("Result for %s: %s\n", hostname, result)
 
 					for _, ip := range ips {
 						assert.Contains(t, result, ip.String())

machine.go

@@ -755,6 +755,9 @@ func (h *Headscale) RegisterMachine(
 		return nil, err
 	}
 
+	h.ipAllocationMutex.Lock()
+	defer h.ipAllocationMutex.Unlock()
+
 	ips, err := h.getAvailableIPs()
 	if err != nil {
 		log.Error().

oidc.go

@@ -317,6 +317,8 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) {
 				return
 			}
 
+			h.ipAllocationMutex.Lock()
+
 			ips, err := h.getAvailableIPs()
 			if err != nil {
 				log.Error().
@@ -338,6 +340,8 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) {
 			machine.LastSuccessfulUpdate = &now
 			machine.Expiry = &requestedTime
 			h.db.Save(&machine)
+
+			h.ipAllocationMutex.Unlock()
 		}
 
 		var content bytes.Buffer