cmd/buildlet: add revdial test
This replaces the broken test in internal/coordinator/pool.
For golang/go#48803
Change-Id: I7bd9265bba555562ffa7d59169a9c8792ed97d3c
Reviewed-on: https://go-review.googlesource.com/c/build/+/354629
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
diff --git a/cmd/buildlet/buildlet.go b/cmd/buildlet/buildlet.go
index 0215691..a1cf644 100644
--- a/cmd/buildlet/buildlet.go
+++ b/cmd/buildlet/buildlet.go
@@ -247,9 +247,13 @@
log.Printf("Error in serveReverseHealth: %v", err)
}
}()
- if err := dialCoordinator(); err != nil {
+ ln, err := dialCoordinator()
+ if err != nil {
log.Fatalf("Error dialing coordinator: %v", err)
}
+ srv := &http.Server{}
+ err = srv.Serve(ln)
+ log.Printf("http.Serve on reverse connection complete: %v", err)
log.Printf("buildlet reverse mode exiting.")
if *haltEntireOS {
// The coordinator disconnects before doHalt has time to
diff --git a/cmd/buildlet/reverse.go b/cmd/buildlet/reverse.go
index 3c3194d..9d33fe6 100644
--- a/cmd/buildlet/reverse.go
+++ b/cmd/buildlet/reverse.go
@@ -60,7 +60,10 @@
return !strings.HasPrefix(*coordinator, "farmer.golang.org")
}
-func dialCoordinator() error {
+// dialCoordinator dials the coordinator to establish a revdial connection
+// where the returned net.Listener can be used to accept connections from the
+// coordinator.
+func dialCoordinator() (net.Listener, error) {
devMode := isDevReverseMode()
if *hostname == "" {
@@ -108,7 +111,7 @@
}
conn, err := dial(context.Background())
if err != nil {
- return err
+ return nil, err
}
bufr := bufio.NewReader(conn)
@@ -125,28 +128,23 @@
req.Header.Set("X-Go-Builder-Version", strconv.Itoa(buildletVersion))
req.Header.Set("X-Revdial-Version", "2")
if err := req.Write(bufw); err != nil {
- return fmt.Errorf("coordinator /reverse request failed: %v", err)
+ return nil, fmt.Errorf("coordinator /reverse request failed: %v", err)
}
if err := bufw.Flush(); err != nil {
- return fmt.Errorf("coordinator /reverse request flush failed: %v", err)
+ return nil, fmt.Errorf("coordinator /reverse request flush failed: %v", err)
}
resp, err := http.ReadResponse(bufr, req)
if err != nil {
- return fmt.Errorf("coordinator /reverse response failed: %v", err)
+ return nil, fmt.Errorf("coordinator /reverse response failed: %v", err)
}
if resp.StatusCode != 101 {
msg, _ := ioutil.ReadAll(resp.Body)
- return fmt.Errorf("coordinator registration failed; want HTTP status 101; got %v:\n\t%s", resp.Status, msg)
+ return nil, fmt.Errorf("coordinator registration failed; want HTTP status 101; got %v:\n\t%s", resp.Status, msg)
}
log.Printf("Connected to coordinator; reverse dialing active")
- srv := &http.Server{}
ln := revdial.NewListener(conn, dial)
- err = srv.Serve(ln)
- if ln.Closed() { // TODO: this actually wants to know whether an error-free Close was called
- return nil
- }
- return fmt.Errorf("http.Serve on reverse connection complete: %v", err)
+ return ln, nil
}
var coordDialer = &net.Dialer{
@@ -206,8 +204,10 @@
return c, nil
}
+const devMasterKey = "gophers rule"
+
func devBuilderKey(builder string) string {
- h := hmac.New(md5.New, []byte("gophers rule"))
+ h := hmac.New(md5.New, []byte(devMasterKey))
io.WriteString(h, builder)
return fmt.Sprintf("%x", h.Sum(nil))
}
@@ -228,10 +228,3 @@
}
return "/"
}
-
-// TestDialCoordinator dials the coordinator. Exported for testing.
-func TestDialCoordinator() {
- // TODO(crawshaw): move some of this logic out of main to simplify testing hook.
- http.Handle("/status", http.HandlerFunc(handleStatus))
- dialCoordinator()
-}
diff --git a/cmd/buildlet/reverse_test.go b/cmd/buildlet/reverse_test.go
new file mode 100644
index 0000000..d503f6a
--- /dev/null
+++ b/cmd/buildlet/reverse_test.go
@@ -0,0 +1,130 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Same requirements as internal/coordinator/pool/reverse.go.
+//go:build go1.13 && (linux || darwin)
+// +build go1.13
+// +build linux darwin
+
+package main
+
+import (
+ "crypto/tls"
+ "fmt"
+ "net"
+ "net/http"
+ "testing"
+ "time"
+
+ "golang.org/x/build"
+ "golang.org/x/build/internal/coordinator/pool"
+ "golang.org/x/build/revdial/v2"
+)
+
+// coordinatorServer creates a server and listener for the coordinator side of
+// revdial. They should be closed when done.
+func coordinatorServer() (*http.Server, net.Listener, error) {
+ mux := http.NewServeMux()
+ mux.HandleFunc("/reverse", pool.HandleReverse)
+ mux.Handle("/revdial", revdial.ConnHandler())
+
+ ln, err := net.Listen("tcp", "")
+ if err != nil {
+ return nil, nil, fmt.Errorf(`net.Listen(":"): %v`, err)
+ }
+
+ cert, err := tls.X509KeyPair([]byte(build.DevCoordinatorCA), []byte(build.DevCoordinatorKey))
+ if err != nil {
+ return nil, nil, fmt.Errorf("error creating TLS cert: %v", err)
+ }
+
+ ln = tls.NewListener(ln, &tls.Config{
+ Certificates: []tls.Certificate{cert},
+ })
+
+ addr := ln.Addr().String()
+ srv := &http.Server{
+ Addr: addr,
+ Handler: mux,
+ }
+ return srv, ln, nil
+}
+
+// testReverseDial verfies that a revdial connection can be established and
+// registered in the coordinator reverse pool at coordAddr.
+func testReverseDial(t *testing.T, coordAddr, hostType string) {
+ t.Helper()
+
+ oldCoordinator := *coordinator
+ defer func() {
+ *coordinator = oldCoordinator
+ }()
+ *coordinator = coordAddr
+
+ // N.B. We don't need to set *hostname to anything in particular as it
+ // is only advisory in the coordinator. It is not used to connect back
+ // to reverse buildlets.
+
+ oldReverseType := *reverseType
+ defer func() {
+ *reverseType = oldReverseType
+ }()
+ *reverseType = hostType
+
+ ln, err := dialCoordinator()
+ if err != nil {
+ t.Fatalf("dialCoordinator got err %v want nil", err)
+ }
+
+ mux := http.NewServeMux()
+ mux.HandleFunc("/status", handleStatus)
+ srv := &http.Server{
+ Handler: mux,
+ }
+ c := make(chan error, 1)
+ go func() {
+ c <- srv.Serve(ln)
+ }()
+ defer func() {
+ srv.Close()
+ err := <-c
+ if err != http.ErrServerClosed {
+ t.Errorf("Server shutdown got err %v want ErrServerClosed", err)
+ }
+ }()
+
+ // Verify that we eventually get the "buildlet" registered with the pool.
+ tick := time.NewTicker(10 * time.Millisecond)
+ defer tick.Stop()
+ start := time.Now()
+ for range tick.C {
+ if time.Since(start) > 1*time.Second {
+ t.Fatalf("Buildlet failed to register within 1s.")
+ }
+
+ types := pool.ReversePool().HostTypes()
+ for _, typ := range types {
+ if typ == hostType {
+ // Success!
+ return
+ }
+ }
+ }
+}
+
+// TestReverseDial verfies that a revdial connection can be established and
+// registered in the coordinator reverse pool.
+func TestReverseDial(t *testing.T) {
+ pool.SetBuilderMasterKey([]byte(devMasterKey))
+
+ srv, ln, err := coordinatorServer()
+ if err != nil {
+ t.Fatalf("serveCoordinator got err %v want nil", err)
+ }
+ go srv.Serve(ln)
+ defer srv.Close()
+
+ const hostType = "test-reverse-dial"
+ testReverseDial(t, srv.Addr, hostType)
+}
diff --git a/internal/coordinator/pool/reverse_test.go b/internal/coordinator/pool/reverse_test.go
deleted file mode 100644
index 2ef9d1b..0000000
--- a/internal/coordinator/pool/reverse_test.go
+++ /dev/null
@@ -1,54 +0,0 @@
-// Copyright 2014 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-//go:build (linux || darwin) && BROKEN
-// +build linux darwin
-// +build BROKEN
-
-// I get:
-// $ go test -v
-// ./reverse_test.go:15: can't find import: "golang.org/x/build/cmd/buildlet"
-
-package main
-
-import (
- "flag"
- "net"
- "net/http"
- "strings"
- "testing"
- "time"
-
- buildletmain "golang.org/x/build/cmd/buildlet"
-)
-
-func TestReverseDial(t *testing.T) {
- *mode = "dev"
- http.HandleFunc("/reverse", handleReverse)
-
- ln, err := net.Listen("tcp", "")
- if err != nil {
- t.Fatalf(`net.Listen(":"): %v`, err)
- }
- t.Logf("listening on %s...", ln.Addr())
- go serveTLS(ln)
-
- wantModes := "goos-goarch-test1,goos-goarch-test2"
- flag.CommandLine.Set("coordinator", ln.Addr().String())
- flag.CommandLine.Set("reverse", wantModes)
-
- ch := make(chan []string)
- registerBuildlet = func(modes []string) { ch <- modes }
- go buildletmain.TestDialCoordinator()
-
- select {
- case modes := <-ch:
- gotModes := strings.Join(modes, ",")
- if gotModes != wantModes {
- t.Errorf("want buildlet registered with modes %q, got %q", wantModes, gotModes)
- }
- case <-time.After(2 * time.Second):
- t.Error("timeout waiting for buildlet registration")
- }
-}
