From 3047fe41ce539550060eebb0475f71d456a8aab4 Mon Sep 17 00:00:00 2001 From: Kevin Allen Date: Thu, 11 May 2023 13:47:33 -0400 Subject: [PATCH] Add sync errorgroup handling in integration tests --- integration/README.md | 2 +- integration/auth_oidc_test.go | 31 ++++++------ integration/auth_web_flow_test.go | 24 +++++----- integration/embedded_derp_test.go | 21 +++----- integration/general_test.go | 8 +++- integration/scenario.go | 79 ++++++++++++++----------------- 6 files changed, 76 insertions(+), 89 deletions(-) diff --git a/integration/README.md b/integration/README.md index e5676a44c6a..ccf6b1e4251 100644 --- 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 index 8ad8f329c57..3b5203bc15b 100644 --- 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 index 7e497f68075..7c3a14cda65 100644 --- 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 index be128087d58..93734e785fa 100644 --- 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,16 +185,11 @@ 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, @@ -203,22 +197,21 @@ func (s *EmbeddedDERPServerScenario) CreateTailscaleIsolatedNodesInUser( 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 index f3187e3a937..5828676899c 100644 --- 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 index 58005482a60..2a6435a3bb5 100644 --- 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,17 +287,12 @@ 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, @@ -304,22 +300,21 @@ func (s *Scenario) CreateTailscaleNodesInUser( 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,14 +327,10 @@ 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 { @@ -347,7 +338,9 @@ func (s *Scenario) RunTailscaleUp( } } - 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 }