cmd/buildlet, cmd/coordinator: delete old --reverse mode
All the buildlets have been updated to use --reverse-type with a host
type instead of a builder type.
Fixes golang/go#21260
Change-Id: I1264261f099c3686fe01455949486f523b94c6de
Reviewed-on: https://go-review.googlesource.com/c/build/+/205608
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
diff --git a/cmd/buildlet/buildlet.go b/cmd/buildlet/buildlet.go
index d1d1a7d..222b5c8 100644
--- a/cmd/buildlet/buildlet.go
+++ b/cmd/buildlet/buildlet.go
@@ -50,7 +50,6 @@
rebootOnHalt = flag.Bool("reboot", false, "reboot system in /halt handler.")
workDir = flag.String("workdir", "", "Temporary directory to use. The contents of this directory may be deleted at any time. If empty, TempDir is used to create one.")
listenAddr = flag.String("listen", "AUTO", "address to listen on. Unused in reverse mode. Warning: this service is inherently insecure and offers no protection of its own. Do not expose this port to the world.")
- reverse = flag.String("reverse", "", "[deprecated; use --reverse-type instead] if non-empty, go into reverse mode where the buildlet dials the coordinator instead of listening for connections. The value is a comma-separated list of modes, e.g. 'darwin-arm,darwin-amd64-race'")
reverseType = flag.String("reverse-type", "", "if non-empty, go into reverse mode where the buildlet dials the coordinator instead of listening for connections. The value is the dashboard/builders.go Hosts map key, naming a HostConfig. This buildlet will receive work for any BuildConfig specifying this named HostConfig.")
coordinator = flag.String("coordinator", "localhost:8119", "address of coordinator, in production use farmer.golang.org. Only used in reverse mode.")
hostname = flag.String("hostname", "", "hostname to advertise to coordinator for reverse mode; default is actual hostname")
@@ -157,10 +156,7 @@
log.Printf("set OS rlimits.")
}
- if *reverse != "" && *reverseType != "" {
- log.Fatalf("can't specify both --reverse and --reverse-type")
- }
- isReverse := *reverse != "" || *reverseType != ""
+ isReverse := *reverseType != ""
if *listenAddr == "AUTO" && !isReverse {
v := defaultListenAddr()
diff --git a/cmd/buildlet/reverse.go b/cmd/buildlet/reverse.go
index aae12a8..f849366 100644
--- a/cmd/buildlet/reverse.go
+++ b/cmd/buildlet/reverse.go
@@ -49,13 +49,6 @@
os.Remove(keyPath)
}
if err != nil {
- if os.IsNotExist(err) && *reverse != "" && !strings.Contains(*reverse, ",") {
- globalKeyPath := filepath.Join(homedir(), ".gobuildkey")
- key, err = ioutil.ReadFile(globalKeyPath)
- if err != nil {
- return "", fmt.Errorf("cannot read either key file %q or %q: %v", keyPath, globalKeyPath, err)
- }
- }
if len(key) == 0 || err != nil {
return "", fmt.Errorf("cannot read key file %q: %v", keyPath, err)
}
@@ -77,24 +70,9 @@
}
}
- var modes, keys []string
- if *reverse != "" {
- // Old way.
- modes = strings.Split(*reverse, ",")
- for _, m := range modes {
- key, err := keyForMode(m)
- if err != nil {
- log.Fatalf("failed to find key for %s: %v", m, err)
- }
- keys = append(keys, key)
- }
- } else {
- // New way.
- key, err := keyForMode(*reverseType)
- if err != nil {
- log.Fatalf("failed to find key for %s: %v", *reverseType, err)
- }
- keys = append(keys, key)
+ key, err := keyForMode(*reverseType)
+ if err != nil {
+ log.Fatalf("failed to find key for %s: %v", *reverseType, err)
}
addr := *coordinator
@@ -138,13 +116,8 @@
if err != nil {
log.Fatal(err)
}
- if *reverse != "" {
- // Old way.
- req.Header["X-Go-Builder-Type"] = modes
- } else {
- req.Header.Set("X-Go-Host-Type", *reverseType)
- }
- req.Header["X-Go-Builder-Key"] = keys
+ req.Header.Set("X-Go-Host-Type", *reverseType)
+ req.Header.Set("X-Go-Builder-Key", key)
req.Header.Set("X-Go-Builder-Hostname", *hostname)
req.Header.Set("X-Go-Builder-Version", strconv.Itoa(buildletVersion))
req.Header.Set("X-Revdial-Version", "2")
diff --git a/cmd/coordinator/reverse.go b/cmd/coordinator/reverse.go
index 1b7b264..d0709f8 100644
--- a/cmd/coordinator/reverse.go
+++ b/cmd/coordinator/reverse.go
@@ -541,14 +541,14 @@
http.Error(w, "buildlet registration requires SSL", http.StatusInternalServerError)
return
}
- // Check build keys.
- // modes can be either 1 buildlet type (new way) or builder mode(s) (the old way)
- hostType := r.Header.Get("X-Go-Host-Type")
- modes := r.Header["X-Go-Builder-Type"] // old way
- gobuildkeys := r.Header["X-Go-Builder-Key"]
- buildletVersion := r.Header.Get("X-Go-Builder-Version")
- revDialVersion := r.Header.Get("X-Revdial-Version")
+ var (
+ hostType = r.Header.Get("X-Go-Host-Type")
+ buildKey = r.Header.Get("X-Go-Builder-Key")
+ buildletVersion = r.Header.Get("X-Go-Builder-Version")
+ revDialVersion = r.Header.Get("X-Revdial-Version")
+ hostname = r.Header.Get("X-Go-Builder-Hostname")
+ )
switch revDialVersion {
case "":
@@ -561,34 +561,14 @@
return
}
- // Convert the new argument style (X-Go-Host-Type) into the
- // old way, to minimize changes in the rest of this code.
+ // Check build keys.
if hostType != "" {
- if len(modes) > 0 {
- http.Error(w, "invalid mix of X-Go-Host-Type and X-Go-Builder-Type", http.StatusBadRequest)
- return
- }
- modes = []string{hostType}
- }
- if len(modes) == 0 || len(modes) != len(gobuildkeys) {
- http.Error(w, fmt.Sprintf("need at least one mode and matching key, got %d/%d", len(modes), len(gobuildkeys)), http.StatusPreconditionFailed)
+ http.Error(w, "missing X-Go-Host-Type; old buildlet binary?", http.StatusBadRequest)
return
}
- hostname := r.Header.Get("X-Go-Builder-Hostname")
-
- for i, m := range modes {
- if gobuildkeys[i] != builderKey(m) {
- http.Error(w, fmt.Sprintf("bad key for mode %q", m), http.StatusPreconditionFailed)
- return
- }
- }
-
- // For older builders using the buildlet's -reverse flag only,
- // collapse their builder modes down into a singular hostType.
- legacyNote := ""
- if hostType == "" {
- hostType = mapBuilderToHostType(modes)
- legacyNote = fmt.Sprintf(" (mapped from legacy modes %q)", modes)
+ if buildKey != builderKey(hostType) {
+ http.Error(w, "invalid build key", http.StatusPreconditionFailed)
+ return
}
conn, bufrw, err := w.(http.Hijacker).Hijack()
@@ -603,8 +583,8 @@
return
}
- log.Printf("Registering reverse buildlet %q (%s) for host type %v %s; buildletVersion=%v; revDialVersion=%v",
- hostname, r.RemoteAddr, hostType, legacyNote, buildletVersion, revDialVersion)
+ log.Printf("Registering reverse buildlet %q (%s) for host type %v; buildletVersion=%v; revDialVersion=%v",
+ hostname, r.RemoteAddr, hostType, buildletVersion, revDialVersion)
var dialer func(context.Context) (net.Conn, error)
var revDialerDone <-chan struct{}
@@ -659,8 +639,8 @@
tstatus := time.Now()
status, err := client.Status()
if err != nil {
- log.Printf("Reverse connection %s/%s for modes %v did not answer status after %v: %v",
- hostname, r.RemoteAddr, modes, time.Since(tstatus), err)
+ log.Printf("Reverse connection %s/%s for %s did not answer status after %v: %v",
+ hostname, r.RemoteAddr, hostType, time.Since(tstatus), err)
conn.Close()
return
}
@@ -669,7 +649,7 @@
conn.Close()
return
}
- log.Printf("Buildlet %s/%s: %+v for %s", hostname, r.RemoteAddr, status, modes)
+ log.Printf("Buildlet %s/%s: %+v for %s", hostname, r.RemoteAddr, status, hostType)
now := time.Now()
b := &reverseBuildlet{
@@ -683,11 +663,8 @@
regTime: now,
}
reversePool.addBuildlet(b)
- registerBuildlet(modes) // testing only
}
-var registerBuildlet = func(modes []string) {} // test hook
-
type byTypeThenHostname []*reverseBuildlet
func (s byTypeThenHostname) Len() int { return len(s) }
@@ -700,29 +677,3 @@
}
return ti < tj
}
-
-// mapBuilderToHostType maps from the user's Request.Header["X-Go-Builder-Type"]
-// mode list down into a single host type, or the empty string if unknown.
-func mapBuilderToHostType(modes []string) string {
- // First, see if any of the provided modes are a host type.
- // If so, this is an updated client.
- for _, v := range modes {
- if _, ok := dashboard.Hosts[v]; ok {
- return v
- }
- }
-
- // Else, it's an old client, still speaking in terms of
- // builder names. See if any are registered aliases. First
- // one wins. (There are no ambiguities in the wild.)
- for hostType, hconf := range dashboard.Hosts {
- for _, alias := range hconf.ReverseAliases {
- for _, v := range modes {
- if v == alias {
- return hostType
- }
- }
- }
- }
- return ""
-}
diff --git a/dashboard/builders.go b/dashboard/builders.go
index 8adb9ca..cd5914d 100644
--- a/dashboard/builders.go
+++ b/dashboard/builders.go
@@ -200,15 +200,13 @@
HermeticReverse: true,
ExpectNum: 50,
env: []string{"GOROOT_BOOTSTRAP=/usr/local/go"},
- ReverseAliases: []string{"linux-arm", "linux-arm-arm5"},
SSHUsername: "root",
},
"host-linux-arm5spacemonkey": &HostConfig{
- IsReverse: true,
- ExpectNum: 3,
- env: []string{"GOROOT_BOOTSTRAP=/usr/local/go"},
- ReverseAliases: []string{"linux-arm-arm5spacemonkey"},
- OwnerGithub: "esnolte", // https://github.com/golang/go/issues/34973#issuecomment-543836871
+ IsReverse: true,
+ ExpectNum: 3,
+ env: []string{"GOROOT_BOOTSTRAP=/usr/local/go"},
+ OwnerGithub: "esnolte", // https://github.com/golang/go/issues/34973#issuecomment-543836871
},
"host-openbsd-amd64-60": &HostConfig{
VMImage: "openbsd-amd64-60",
@@ -357,12 +355,11 @@
OwnerGithub: "tuxillo",
},
"host-freebsd-arm-paulzhol": &HostConfig{
- IsReverse: true,
- ExpectNum: 1,
- Notes: "Cubiboard2 1Gb RAM dual-core Cortex-A7 (Allwinner A20), FreeBSD 11.1-RELEASE",
- env: []string{"GOROOT_BOOTSTRAP=/usr/home/paulzhol/go1.4"},
- ReverseAliases: []string{"freebsd-arm-paulzhol"},
- OwnerGithub: "paulzhol",
+ IsReverse: true,
+ ExpectNum: 1,
+ Notes: "Cubiboard2 1Gb RAM dual-core Cortex-A7 (Allwinner A20), FreeBSD 11.1-RELEASE",
+ env: []string{"GOROOT_BOOTSTRAP=/usr/home/paulzhol/go1.4"},
+ OwnerGithub: "paulzhol",
},
"host-freebsd-arm64-dmgk": &HostConfig{
IsReverse: true,
@@ -452,7 +449,6 @@
env: []string{
"GOROOT_BOOTSTRAP=/Users/gopher/go1.4",
},
- ReverseAliases: []string{"darwin-amd64-10_8"},
SSHUsername: "gopher",
HermeticReverse: false, // TODO: make it so, like 10.12
},
@@ -463,7 +459,6 @@
env: []string{
"GOROOT_BOOTSTRAP=/Users/gopher/go1.4",
},
- ReverseAliases: []string{"darwin-amd64-10_10"},
SSHUsername: "gopher",
HermeticReverse: false, // TODO: make it so, like 10.12
},
@@ -474,7 +469,6 @@
env: []string{
"GOROOT_BOOTSTRAP=/Users/gopher/go1.4",
},
- ReverseAliases: []string{"darwin-amd64-10_11"},
SSHUsername: "gopher",
HermeticReverse: false, // TODO: make it so, like 10.12
},
@@ -485,7 +479,6 @@
env: []string{
"GOROOT_BOOTSTRAP=/Users/gopher/go1.4",
},
- ReverseAliases: []string{"darwin-amd64-10_12"},
SSHUsername: "gopher",
HermeticReverse: true, // we destroy the VM when done & let cmd/makemac recreate
},
@@ -500,18 +493,16 @@
HermeticReverse: true, // we destroy the VM when done & let cmd/makemac recreate
},
"host-linux-s390x": &HostConfig{
- Notes: "run by IBM",
- OwnerGithub: "mundaym",
- IsReverse: true,
- env: []string{"GOROOT_BOOTSTRAP=/var/buildlet/go-linux-s390x-bootstrap"},
- ReverseAliases: []string{"linux-s390x-ibm"},
+ Notes: "run by IBM",
+ OwnerGithub: "mundaym",
+ IsReverse: true,
+ env: []string{"GOROOT_BOOTSTRAP=/var/buildlet/go-linux-s390x-bootstrap"},
},
"host-linux-ppc64-osu": &HostConfig{
Notes: "Debian jessie; run by Go team on osuosl.org",
IsReverse: true,
ExpectNum: 5,
env: []string{"GOROOT_BOOTSTRAP=/usr/local/go-bootstrap"},
- ReverseAliases: []string{"linux-ppc64-buildlet"},
SSHUsername: "root",
HermeticReverse: false, // TODO: run in chroots with overlayfs? https://github.com/golang/go/issues/34830#issuecomment-543386764
},
@@ -723,14 +714,6 @@
Notes string // notes for humans
SSHUsername string // username to ssh as, empty means not supported
-
- // ReverseAliases lists alternate names for this buildlet
- // config, for older clients doing a reverse dial into the
- // coordinator from outside. This prevents us from updating
- // 75+ dedicated machines/VMs atomically, switching them to
- // the new "host-*" names.
- // This is only applicable if IsReverse.
- ReverseAliases []string
}
// A BuildConfig describes how to run a builder.
