Add sync errorgroup handling in integration tests

juanfont · May 11, 2023 · 3047fe4 · 3047fe4
1 parent 252342a
commit 3047fe4
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 89 deletions.
diff --git a/integration/README.md b/integration/README.md
@@ -11,7 +11,7 @@ Tests are located in files ending with `_test.go` and the framework are located
 
 ## Running integration tests locally
 
-The easiest way to run tests locally is to use `[act](INSERT LINK)`, a local GitHub Actions runner:
+The easiest way to run tests locally is to use `[act](https://github.com/nektos/act)`, a local GitHub Actions runner:
 
 ```
 act pull_request -W .github/workflows/test-integration-v2-TestPingAllByIP.yaml

diff --git a/integration/auth_oidc_test.go b/integration/auth_oidc_test.go
@@ -170,7 +170,9 @@ func TestOIDCExpireNodesBasedOnTokenExpiry(t *testing.T) {
 	t.Logf("%d successful pings out of %d (before expiry)", success, len(allClients)*len(allIps))
 
 	// await all nodes being logged out after OIDC token expiry
-	scenario.WaitForTailscaleLogout()
+	if err = scenario.WaitForTailscaleLogout(); err != nil {
+		t.Errorf("failed to logout tailscale nodes: %s", err)
+	}
 
 	err = scenario.Shutdown()
 	if err != nil {
@@ -310,15 +312,11 @@ func (s *AuthOIDCScenario) runTailscaleUp(
 	log.Printf("running tailscale up for user %s", userStr)
 	if user, ok := s.users[userStr]; ok {
 		for _, client := range user.Clients {
-			user.joinWaitGroup.Add(1)
-
-			go func(c TailscaleClient) {
-				defer user.joinWaitGroup.Done()
-
-				// TODO(juanfont): error handle this
+			c := client
+			user.joinWaitGroup.Go(func() error {
 				loginURL, err := c.UpWithLoginURL(loginServer)
 				if err != nil {
-					log.Printf("failed to run tailscale up: %s", err)
+					return fmt.Errorf("failed to run tailscale up: %w", err)
 				}
 
 				loginURL.Host = fmt.Sprintf("%s:8080", headscale.GetIP())
@@ -335,22 +333,19 @@ func (s *AuthOIDCScenario) runTailscaleUp(
 				req, _ := http.NewRequestWithContext(ctx, http.MethodGet, loginURL.String(), nil)
 				resp, err := httpClient.Do(req)
 				if err != nil {
-					log.Printf("%s failed to get login url %s: %s", c.Hostname(), loginURL, err)
-
-					return
+					return fmt.Errorf("%s failed to get login url %s: %w", c.Hostname(), loginURL, err)
 				}
 
 				defer resp.Body.Close()
 
 				_, err = io.ReadAll(resp.Body)
 				if err != nil {
-					log.Printf("%s failed to read response body: %s", c.Hostname(), err)
-
-					return
+					return fmt.Errorf("%s failed to read response body: %w", c.Hostname(), err)
 				}
-
 				log.Printf("Finished request for %s to join tailnet", c.Hostname())
-			}(client)
+
+				return nil
+			})
 
 			err = client.WaitForReady()
 			if err != nil {
@@ -360,7 +355,9 @@ func (s *AuthOIDCScenario) runTailscaleUp(
 			log.Printf("client %s is ready", client.Hostname())
 		}
 
-		user.joinWaitGroup.Wait()
+		if err := user.joinWaitGroup.Wait(); err != nil {
+			return fmt.Errorf("failed to bring up tailscale nodes: %w", err)
+		}
 
 		for _, client := range user.Clients {
 			err := client.WaitForReady()

diff --git a/integration/auth_web_flow_test.go b/integration/auth_web_flow_test.go
@@ -134,7 +134,9 @@ func TestAuthWebFlowLogoutAndRelogin(t *testing.T) {
 		}
 	}
 
-	scenario.WaitForTailscaleLogout()
+	if err = scenario.WaitForTailscaleLogout(); err != nil {
+		t.Errorf("failed to logout tailscale nodes: %s", err)
+	}
 
 	t.Logf("all clients logged out")
 
@@ -250,29 +252,29 @@ func (s *AuthWebFlowScenario) runTailscaleUp(
 	log.Printf("running tailscale up for user %s", userStr)
 	if user, ok := s.users[userStr]; ok {
 		for _, client := range user.Clients {
-			user.joinWaitGroup.Add(1)
-
-			go func(c TailscaleClient) {
-				defer user.joinWaitGroup.Done()
-
-				// TODO(juanfont): error handle this
+			c := client
+			user.joinWaitGroup.Go(func() error {
 				loginURL, err := c.UpWithLoginURL(loginServer)
 				if err != nil {
-					log.Printf("failed to run tailscale up: %s", err)
+					return fmt.Errorf("failed to run tailscale up: %w", err)
 				}
 
 				err = s.runHeadscaleRegister(userStr, loginURL)
 				if err != nil {
-					log.Printf("failed to register client: %s", err)
+					return fmt.Errorf("failed to register client: %w", err)
 				}
-			}(client)
+
+				return nil
+			})
 
 			err := client.WaitForReady()
 			if err != nil {
 				log.Printf("error waiting for client %s to be ready: %s", client.Hostname(), err)
 			}
 		}
-		user.joinWaitGroup.Wait()
+		if err := user.joinWaitGroup.Wait(); err != nil {
+			return fmt.Errorf("failed to up tailscale nodes: %w", err)
+		}
 
 		for _, client := range user.Clients {
 			err := client.WaitForReady()

diff --git a/integration/embedded_derp_test.go b/integration/embedded_derp_test.go
@@ -2,7 +2,6 @@ package integration
 
 import (
 	"fmt"
-	"log"
 	"net/url"
 	"testing"
 
@@ -186,39 +185,33 @@ func (s *EmbeddedDERPServerScenario) CreateTailscaleIsolatedNodesInUser(
 
 			cert := hsServer.GetCert()
 
-			user.createWaitGroup.Add(1)
-
 			opts = append(opts,
 				tsic.WithHeadscaleTLS(cert),
 			)
 
-			go func() {
-				defer user.createWaitGroup.Done()
-
-				// TODO(kradalby): error handle this
+			user.createWaitGroup.Go(func() error {
 				tsClient, err := tsic.New(
 					s.pool,
 					version,
 					network,
 					opts...,
 				)
 				if err != nil {
-					// return fmt.Errorf("failed to add tailscale node: %w", err)
-					log.Printf("failed to create tailscale node: %s", err)
+					return fmt.Errorf("failed to create tailscale node: %w", err)
 				}
 
 				err = tsClient.WaitForReady()
 				if err != nil {
-					// return fmt.Errorf("failed to add tailscale node: %w", err)
-					log.Printf("failed to wait for tailscaled: %s", err)
+					return fmt.Errorf("failed to wait for tailscaled: %w", err)
 				}
 
 				user.Clients[tsClient.Hostname()] = tsClient
-			}()
+
+				return nil
+			})
 		}
-		user.createWaitGroup.Wait()
 
-		return nil
+		return user.createWaitGroup.Wait()
 	}
 
 	return fmt.Errorf("failed to add tailscale node: %w", errNoUserAvailable)

diff --git a/integration/general_test.go b/integration/general_test.go
@@ -108,7 +108,9 @@ func TestAuthKeyLogoutAndRelogin(t *testing.T) {
 		}
 	}
 
-	scenario.WaitForTailscaleLogout()
+	if err = scenario.WaitForTailscaleLogout(); err != nil {
+		t.Errorf("failed to logout tailscale nodes: %s", err)
+	}
 
 	t.Logf("all clients logged out")
 
@@ -261,7 +263,9 @@ func TestEphemeral(t *testing.T) {
 		}
 	}
 
-	scenario.WaitForTailscaleLogout()
+	if err = scenario.WaitForTailscaleLogout(); err != nil {
+		t.Errorf("failed to logout tailscale nodes: %s", err)
+	}
 
 	t.Logf("all clients logged out")
 

diff --git a/integration/scenario.go b/integration/scenario.go
@@ -16,6 +16,7 @@ import (
 	"github.com/juanfont/headscale/integration/tsic"
 	"github.com/ory/dockertest/v3"
 	"github.com/puzpuzpuz/xsync/v2"
+	"golang.org/x/sync/errgroup"
 )
 
 const (
@@ -79,9 +80,9 @@ var (
 type User struct {
 	Clients map[string]TailscaleClient
 
-	createWaitGroup sync.WaitGroup
-	joinWaitGroup   sync.WaitGroup
-	syncWaitGroup   sync.WaitGroup
+	createWaitGroup errgroup.Group
+	joinWaitGroup   errgroup.Group
+	syncWaitGroup   errgroup.Group
 }
 
 // Scenario is a representation of an environment with one ControlServer and
@@ -286,40 +287,34 @@ func (s *Scenario) CreateTailscaleNodesInUser(
 			cert := headscale.GetCert()
 			hostname := headscale.GetHostname()
 
-			user.createWaitGroup.Add(1)
-
 			opts = append(opts,
 				tsic.WithHeadscaleTLS(cert),
 				tsic.WithHeadscaleName(hostname),
 			)
 
-			go func() {
-				defer user.createWaitGroup.Done()
-
-				// TODO(kradalby): error handle this
+			user.createWaitGroup.Go(func() error {
 				tsClient, err := tsic.New(
 					s.pool,
 					version,
 					s.network,
 					opts...,
 				)
 				if err != nil {
-					// return fmt.Errorf("failed to add tailscale node: %w", err)
-					log.Printf("failed to create tailscale node: %s", err)
+					return fmt.Errorf("failed to add tailscale node: %w", err)
 				}
 
 				err = tsClient.WaitForReady()
 				if err != nil {
-					// return fmt.Errorf("failed to add tailscale node: %w", err)
-					log.Printf("failed to wait for tailscaled: %s", err)
+					return fmt.Errorf("failed to add tailscale node: %w", err)
 				}
 
 				user.Clients[tsClient.Hostname()] = tsClient
-			}()
+
+				return nil
+			})
 		}
-		user.createWaitGroup.Wait()
 
-		return nil
+		return user.createWaitGroup.Wait()
 	}
 
 	return fmt.Errorf("failed to add tailscale node: %w", errNoUserAvailable)
@@ -332,22 +327,20 @@ func (s *Scenario) RunTailscaleUp(
 ) error {
 	if user, ok := s.users[userStr]; ok {
 		for _, client := range user.Clients {
-			user.joinWaitGroup.Add(1)
-
-			go func(c TailscaleClient) {
-				defer user.joinWaitGroup.Done()
-
-				// TODO(kradalby): error handle this
-				_ = c.Up(loginServer, authKey)
-			}(client)
+			c := client
+			user.joinWaitGroup.Go(func() error {
+				return c.Up(loginServer, authKey)
+			})
 
 			err := client.WaitForReady()
 			if err != nil {
 				log.Printf("error waiting for client %s to be ready: %s", client.Hostname(), err)
 			}
 		}
 
-		user.joinWaitGroup.Wait()
+		if err := user.joinWaitGroup.Wait(); err != nil {
+			return fmt.Errorf("failed to up tailscale nodes: %w", err)
+		}
 
 		for _, client := range user.Clients {
 			err := client.WaitForReady()
@@ -383,16 +376,14 @@ func (s *Scenario) WaitForTailscaleSync() error {
 
 	for _, user := range s.users {
 		for _, client := range user.Clients {
-			user.syncWaitGroup.Add(1)
-
-			go func(c TailscaleClient) {
-				defer user.syncWaitGroup.Done()
-
-				// TODO(kradalby): error handle this
-				_ = c.WaitForPeers(tsCount)
-			}(client)
+			c := client
+			user.syncWaitGroup.Go(func() error {
+				return c.WaitForPeers(tsCount)
+			})
+		}
+		if err := user.syncWaitGroup.Wait(); err != nil {
+			return err
 		}
-		user.syncWaitGroup.Wait()
 	}
 
 	return nil
@@ -555,18 +546,18 @@ func (s *Scenario) ListTailscaleClientsFQDNs(users ...string) ([]string, error)
 
 // WaitForTailscaleLogout blocks execution until all TailscaleClients have
 // logged out of the ControlServer.
-func (s *Scenario) WaitForTailscaleLogout() {
+func (s *Scenario) WaitForTailscaleLogout() error {
 	for _, user := range s.users {
 		for _, client := range user.Clients {
-			user.syncWaitGroup.Add(1)
-
-			go func(c TailscaleClient) {
-				defer user.syncWaitGroup.Done()
-
-				// TODO(kradalby): error handle this
-				_ = c.WaitForLogout()
-			}(client)
+			c := client
+			user.syncWaitGroup.Go(func() error {
+				return c.WaitForLogout()
+			})
+		}
+		if err := user.syncWaitGroup.Wait(); err != nil {
+			return err
 		}
-		user.syncWaitGroup.Wait()
 	}
+
+	return nil
 }