From 8baba629ca17359692aa689c523f6e125f81dafd Mon Sep 17 00:00:00 2001 From: Oscar Bonilla Date: Tue, 19 Feb 2019 09:16:37 -0800 Subject: [PATCH 1/5] Add directory ownership and enable ephemeral ports --- BUILD.bazel | 1 + WORKSPACE | 32 +++++++++---- config/config.go | 2 +- go.mod | 11 +++-- go.sum | 5 ++ main.go | 121 +++++++++++++++++++++++++++++++++++++++++++++-- 6 files changed, 151 insertions(+), 21 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index d9285f0d4..e290026e8 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -21,6 +21,7 @@ go_library( "//config:go_default_library", "//server:go_default_library", "@com_github_abbot_go_http_auth//:go_default_library", + "@com_github_nightlyone_lockfile//:go_default_library", "@com_github_urfave_cli//:go_default_library", ], ) diff --git a/WORKSPACE b/WORKSPACE index 6f284245b..c905f8671 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -39,8 +39,8 @@ gazelle_dependencies() go_repository( name = "com_github_abbot_go_http_auth", - commit = "0ddd408d5d60ea76e320503cc7dd091992dee608", importpath = "github.com/abbot/go-http-auth", + tag = "v0.4.0", ) go_repository( @@ -51,49 +51,49 @@ go_repository( go_repository( name = "com_github_djherbis_atime", - commit = "8e47e0e01d08df8b9f840d74299c8ab70a024a30", importpath = "github.com/djherbis/atime", + tag = "v1.0.0", ) go_repository( name = "org_golang_x_crypto", - commit = "ab813273cd59e1333f7ae7bff5d027d4aadf528c", + commit = "ab813273cd59", importpath = "golang.org/x/crypto", ) go_repository( name = "org_golang_x_net", - commit = "1e491301e022f8f977054da4c2d852decd59571f", + commit = "1e491301e022", importpath = "golang.org/x/net", ) go_repository( name = "com_github_golang_protobuf", - commit = "b4deda0973fb4c70b50d226b1af49f3da59f5265", importpath = "github.com/golang/protobuf", + tag = "v1.1.0", ) go_repository( name = "com_google_cloud_go", - commit = "0fd7230b2a7505833d5f69b75cbd6c9582401479", importpath = "cloud.google.com/go", + tag = "v0.23.0", ) go_repository( name = "in_gopkg_yaml_v2", - commit = "5420a8b6744d3b0345ab293f6fcba19c978f1183", importpath = "gopkg.in/yaml.v2", + tag = "v2.2.1", ) go_repository( name = "org_golang_google_appengine", - commit = "150dc57a1b433e64154302bdc40b6bb8aefa313a", importpath = "google.golang.org/appengine", + tag = "v1.0.0", ) go_repository( name = "org_golang_x_oauth2", - commit = "ec22f46f877b4505e0117eeaab541714644fdd28", + commit = "ec22f46f877b", importpath = "golang.org/x/oauth2", ) @@ -105,6 +105,18 @@ go_repository( go_repository( name = "com_github_google_go_cmp", - commit = "3af367b6b30c263d47e8895973edcca9a49cf029", importpath = "github.com/google/go-cmp", + tag = "v0.2.0", +) + +go_repository( + name = "com_github_nightlyone_lockfile", + commit = "0ad87eef1443", + importpath = "github.com/nightlyone/lockfile", +) + +go_repository( + name = "in_gopkg_check_v1", + commit = "20d25e280405", + importpath = "gopkg.in/check.v1", ) diff --git a/config/config.go b/config/config.go index ca8d5f2a6..949f0cd02 100644 --- a/config/config.go +++ b/config/config.go @@ -96,7 +96,7 @@ func validateConfig(c *Config) error { return errors.New("The 'max_size' flag/key must be set to a value > 0") } - if c.Port == 0 { + if c.Port < 0 { return errors.New("A valid 'port' flag/key must be specified") } diff --git a/go.mod b/go.mod index 8afb160b4..694b15641 100644 --- a/go.mod +++ b/go.mod @@ -1,15 +1,16 @@ module github.com/buchgr/bazel-remote require ( - cloud.google.com/go v0.23.0 + cloud.google.com/go v0.23.0 // indirect github.com/abbot/go-http-auth v0.4.0 github.com/djherbis/atime v1.0.0 - github.com/golang/protobuf v1.1.0 + github.com/golang/protobuf v1.1.0 // indirect github.com/google/go-cmp v0.2.0 + github.com/nightlyone/lockfile v0.0.0-20180618180623-0ad87eef1443 github.com/urfave/cli v1.20.0 - golang.org/x/crypto v0.0.0-20180527072434-ab813273cd59 - golang.org/x/net v0.0.0-20180530234432-1e491301e022 + golang.org/x/crypto v0.0.0-20180527072434-ab813273cd59 // indirect + golang.org/x/net v0.0.0-20180530234432-1e491301e022 // indirect golang.org/x/oauth2 v0.0.0-20180529203656-ec22f46f877b - google.golang.org/appengine v1.0.0 + google.golang.org/appengine v1.0.0 // indirect gopkg.in/yaml.v2 v2.2.1 ) diff --git a/go.sum b/go.sum index 755166bdf..20a7f50d3 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,12 @@ github.com/abbot/go-http-auth v0.4.0 h1:QjmvZ5gSC7jm3Zg54DqWE/T5m1t2AfDu6QlXJT0E github.com/abbot/go-http-auth v0.4.0/go.mod h1:Cz6ARTIzApMJDzh5bRMSUou6UMSp0IEXg9km/ci7TJM= github.com/djherbis/atime v1.0.0 h1:ySLvBAM0EvOGaX7TI4dAM5lWj+RdJUCKtGSEHN8SGBg= github.com/djherbis/atime v1.0.0/go.mod h1:5W+KBIuTwVGcqjIfaTwt+KSYX1o6uep8dtevevQP/f8= +github.com/golang/protobuf v1.1.0 h1:0iH4Ffd/meGoXqF2lSAhZHt8X+cPgkfn/cb6Cce5Vpc= github.com/golang/protobuf v1.1.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= +github.com/nightlyone/lockfile v0.0.0-20180618180623-0ad87eef1443 h1:+2OJrU8cmOstEoh0uQvYemRGVH1O6xtO2oANUWHFnP0= +github.com/nightlyone/lockfile v0.0.0-20180618180623-0ad87eef1443/go.mod h1:JbxfV1Iifij2yhRjXai0oFrbpxszXHRx1E5RuM26o4Y= github.com/urfave/cli v1.20.0 h1:fDqGv3UG/4jbVl/QkFwEdddtEDjh/5Ov6X+0B/3bPaw= github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= golang.org/x/crypto v0.0.0-20180527072434-ab813273cd59 h1:hk3yo72LXLapY9EXVttc3Z1rLOxT9IuAPPX3GpY2+jo= @@ -14,6 +18,7 @@ golang.org/x/net v0.0.0-20180530234432-1e491301e022 h1:MVYFTUmVD3/+ERcvRRI+P/C2+ golang.org/x/net v0.0.0-20180530234432-1e491301e022/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/oauth2 v0.0.0-20180529203656-ec22f46f877b h1:nCwwlzLoBQhkY/S3CJ2CGAU4pYfR8+5/TPGEHT+p5Nk= golang.org/x/oauth2 v0.0.0-20180529203656-ec22f46f877b/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= +google.golang.org/appengine v1.0.0 h1:dN4LljjBKVChsv0XCSI+zbyzdqrkEwX5LQFUMRSGqOc= google.golang.org/appengine v1.0.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.1 h1:mUhvW9EsL+naU5Q3cakzfE91YhliOondGd6ZrsDBHQE= diff --git a/main.go b/main.go index 6ad28b2cc..12237e6b7 100644 --- a/main.go +++ b/main.go @@ -3,12 +3,18 @@ package main import ( "context" "fmt" + "io/ioutil" "log" + "net" "net/http" "net/url" "os" + "os/signal" + "path/filepath" "strconv" + "strings" "sync" + "syscall" "time" auth "github.com/abbot/go-http-auth" @@ -20,9 +26,49 @@ import ( "github.com/buchgr/bazel-remote/config" "github.com/buchgr/bazel-remote/server" + "github.com/nightlyone/lockfile" "github.com/urfave/cli" ) +const ( + bazelRemotePortFile = "bazel-remote.port" + bazelRemotePidFile = "bazel-remote.pid" +) + +var signalHandlers []func(os.Signal) +var signalHandlersMutex sync.Mutex + +//http server.go doesn't export tcpKeepAliveListener so we have to do the same here +type tcpKeepAliveListener struct { + *net.TCPListener +} + +func (ln tcpKeepAliveListener) Accept() (net.Conn, error) { + tc, err := ln.AcceptTCP() + if err != nil { + return nil, err + } + tc.SetKeepAlive(true) + tc.SetKeepAlivePeriod(3 * time.Minute) + return tc, nil +} + +func init() { + // set up a signal handler to clean up if we are interrupted + c := make(chan os.Signal, 1) + signal.Notify(c, syscall.SIGINT, syscall.SIGTERM) + go func() { + select { + case sig := <-c: + signalHandlersMutex.Lock() + defer signalHandlersMutex.Unlock() + for _, fn := range signalHandlers { + fn(sig) + } + } + }() +} + func main() { app := cli.NewApp() app.Description = "A remote build cache for Bazel." @@ -118,6 +164,45 @@ func main() { accessLogger := log.New(os.Stdout, "", log.Ldate|log.Ltime|log.LUTC) errorLogger := log.New(os.Stderr, "", log.Ldate|log.Ltime|log.LUTC) + if err := os.MkdirAll(c.Dir, 0755); err != nil { + return err + } + lockAbsPath, err := filepath.Abs(filepath.Join(c.Dir, bazelRemotePidFile)) + if err != nil { + return err + } + pidFileLock, err := lockfile.New(lockAbsPath) + if err != nil { + return err + } + portFile := filepath.Join(c.Dir, bazelRemotePortFile) + err = pidFileLock.TryLock() + if err != nil { + if err == lockfile.ErrBusy { + pid, _ := ioutil.ReadFile(lockAbsPath) + port, err := ioutil.ReadFile(portFile) + if err != nil { + // the other process just started, give it time to write the port file + retry := 3 + for err != nil && retry > 0 { + time.Sleep(time.Second) + port, err = ioutil.ReadFile(portFile) + retry-- + } + } + return fmt.Errorf( + "Already locked by pid %v, listening on %v", + strings.Trim(string(pid), "\n"), + strings.Trim(string(port), "\n"), + ) + } + return fmt.Errorf("Could not lock %v: %v", c.Dir, err) + } + defer func() { + os.Remove(portFile) + pidFileLock.Unlock() + }() + diskCache := disk.New(c.Dir, int64(c.MaxSize)*1024*1024*1024) var proxyCache cache.Cache @@ -142,9 +227,24 @@ func main() { mux := http.NewServeMux() httpServer := &http.Server{ - Addr: c.Host + ":" + strconv.Itoa(c.Port), Handler: mux, } + + // graceful shutdown on signal + func() { + signalHandlersMutex.Lock() + defer signalHandlersMutex.Unlock() + signalHandlers = append(signalHandlers, func(sig os.Signal) { + errorLogger.Printf("Shutting down server due to signal: %v", sig) + if error := os.Remove(filepath.Join(c.Dir, bazelRemotePortFile)); error != nil { + errorLogger.Printf("Failed to remove %v", c.Dir) + } + if err := httpServer.Shutdown(context.Background()); err != nil { + errorLogger.Printf("Failed to shutdown http server: %v", err) + } + }) + }() + h := server.NewHTTPCache(proxyCache, accessLogger, errorLogger) mux.HandleFunc("/status", h.StatusPageHandler) @@ -152,17 +252,28 @@ func main() { if c.HtpasswdFile != "" { cacheHandler = wrapAuthHandler(cacheHandler, c.HtpasswdFile, c.Host) } + if c.IdleTimeout > 0 { cacheHandler = wrapIdleHandler(cacheHandler, c.IdleTimeout, accessLogger, httpServer) } - mux.HandleFunc("/", cacheHandler) + mux.HandleFunc("/", cacheHandler) + ln, err := net.Listen("tcp", c.Host+":"+strconv.Itoa(c.Port)) + if err != nil { + return err + } + defer ln.Close() + // now it's safe to write the port file since we're already listening + s := fmt.Sprintf("%s\n", ln.Addr().String()) + err = ioutil.WriteFile(filepath.Join(c.Dir, bazelRemotePortFile), []byte(s), 0644) + if err != nil { + return err + } if len(c.TLSCertFile) > 0 && len(c.TLSKeyFile) > 0 { - return httpServer.ListenAndServeTLS(c.TLSCertFile, c.TLSKeyFile) + return httpServer.ServeTLS(tcpKeepAliveListener{ln.(*net.TCPListener)}, c.TLSCertFile, c.TLSKeyFile) } - return httpServer.ListenAndServe() + return httpServer.Serve(tcpKeepAliveListener{ln.(*net.TCPListener)}) } - serverErr := app.Run(os.Args) if serverErr != nil { log.Fatal("bazel-remote terminated: ", serverErr) From 5911413913d027b413df06867888b037b40bb960 Mon Sep 17 00:00:00 2001 From: Oscar Bonilla Date: Tue, 26 Feb 2019 13:42:18 -0800 Subject: [PATCH 2/5] No need to remove port file on signal handler Since we are callling http.shutdown, just let the normal cleanup code deal with removing the file. --- main.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/main.go b/main.go index 12237e6b7..efa41e757 100644 --- a/main.go +++ b/main.go @@ -236,9 +236,6 @@ func main() { defer signalHandlersMutex.Unlock() signalHandlers = append(signalHandlers, func(sig os.Signal) { errorLogger.Printf("Shutting down server due to signal: %v", sig) - if error := os.Remove(filepath.Join(c.Dir, bazelRemotePortFile)); error != nil { - errorLogger.Printf("Failed to remove %v", c.Dir) - } if err := httpServer.Shutdown(context.Background()); err != nil { errorLogger.Printf("Failed to shutdown http server: %v", err) } From 93c2b65b1f9aeb1840517c0d18b59c540fd2b81e Mon Sep 17 00:00:00 2001 From: Oscar Bonilla Date: Tue, 26 Feb 2019 14:35:32 -0800 Subject: [PATCH 3/5] Move the os.Remove() to a clearer place --- main.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/main.go b/main.go index efa41e757..2e6f8732f 100644 --- a/main.go +++ b/main.go @@ -198,10 +198,7 @@ func main() { } return fmt.Errorf("Could not lock %v: %v", c.Dir, err) } - defer func() { - os.Remove(portFile) - pidFileLock.Unlock() - }() + defer pidFileLock.Unlock() diskCache := disk.New(c.Dir, int64(c.MaxSize)*1024*1024*1024) @@ -262,10 +259,12 @@ func main() { defer ln.Close() // now it's safe to write the port file since we're already listening s := fmt.Sprintf("%s\n", ln.Addr().String()) - err = ioutil.WriteFile(filepath.Join(c.Dir, bazelRemotePortFile), []byte(s), 0644) + err = ioutil.WriteFile(portFile, []byte(s), 0666) if err != nil { return err } + defer os.Remove(portFile) + if len(c.TLSCertFile) > 0 && len(c.TLSKeyFile) > 0 { return httpServer.ServeTLS(tcpKeepAliveListener{ln.(*net.TCPListener)}, c.TLSCertFile, c.TLSKeyFile) } From 0bc00839d3845c7709769ac1c00cdaa4ba860103 Mon Sep 17 00:00:00 2001 From: Oscar Bonilla Date: Thu, 7 Mar 2019 10:22:12 -0800 Subject: [PATCH 4/5] Simplify handoff by using domain socket --- main.go | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/main.go b/main.go index 2e6f8732f..6628cc71b 100644 --- a/main.go +++ b/main.go @@ -30,10 +30,7 @@ import ( "github.com/urfave/cli" ) -const ( - bazelRemotePortFile = "bazel-remote.port" - bazelRemotePidFile = "bazel-remote.pid" -) +const bazelRemotePidFile = "bazel-remote.pid" var signalHandlers []func(os.Signal) var signalHandlersMutex sync.Mutex @@ -175,25 +172,13 @@ func main() { if err != nil { return err } - portFile := filepath.Join(c.Dir, bazelRemotePortFile) err = pidFileLock.TryLock() if err != nil { if err == lockfile.ErrBusy { pid, _ := ioutil.ReadFile(lockAbsPath) - port, err := ioutil.ReadFile(portFile) - if err != nil { - // the other process just started, give it time to write the port file - retry := 3 - for err != nil && retry > 0 { - time.Sleep(time.Second) - port, err = ioutil.ReadFile(portFile) - retry-- - } - } return fmt.Errorf( - "Already locked by pid %v, listening on %v", + "Already locked by pid %v", strings.Trim(string(pid), "\n"), - strings.Trim(string(port), "\n"), ) } return fmt.Errorf("Could not lock %v: %v", c.Dir, err) @@ -257,13 +242,14 @@ func main() { return err } defer ln.Close() - // now it's safe to write the port file since we're already listening - s := fmt.Sprintf("%s\n", ln.Addr().String()) - err = ioutil.WriteFile(portFile, []byte(s), 0666) + + // create a unix domain socket and respond with the port when asked + sock, err := net.Listen("unix", lockAbsPath+".sock") if err != nil { return err } - defer os.Remove(portFile) + defer os.Remove(lockAbsPath + ".sock") + go handlePortRequest(sock, ln.Addr(), errorLogger) if len(c.TLSCertFile) > 0 && len(c.TLSKeyFile) > 0 { return httpServer.ServeTLS(tcpKeepAliveListener{ln.(*net.TCPListener)}, c.TLSCertFile, c.TLSKeyFile) @@ -276,6 +262,22 @@ func main() { } } +func handlePortRequest(sock net.Listener, addr net.Addr, errorLogger *log.Logger) { + for { + fd, err := sock.Accept() + if err != nil { + errorLogger.Printf("sock: %v", err) + continue + } + if _, err := fd.Write([]byte(addr.String() + "\n")); err != nil { + errorLogger.Printf("sock write: %v", err) + } + if err := fd.Close(); err != nil { + errorLogger.Printf("sock close: %v", err) + } + } +} + func wrapIdleHandler(handler http.HandlerFunc, idleTimeout time.Duration, accessLogger cache.Logger, httpServer *http.Server) http.HandlerFunc { lastRequest := time.Now() ticker := time.NewTicker(time.Second) From 677b28ca9d3620c66d7a5129719433296f43d5db Mon Sep 17 00:00:00 2001 From: Oscar Bonilla Date: Thu, 7 Mar 2019 13:37:38 -0800 Subject: [PATCH 5/5] Fix domain socket location --- main.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index 6628cc71b..896af266e 100644 --- a/main.go +++ b/main.go @@ -244,11 +244,12 @@ func main() { defer ln.Close() // create a unix domain socket and respond with the port when asked - sock, err := net.Listen("unix", lockAbsPath+".sock") + bazelRemoteSocketPath := filepath.Join(c.Dir, fmt.Sprintf("bazel-remote.%d.sock", os.Getpid())) + sock, err := net.Listen("unix", bazelRemoteSocketPath) if err != nil { return err } - defer os.Remove(lockAbsPath + ".sock") + defer os.Remove(bazelRemoteSocketPath) go handlePortRequest(sock, ln.Addr(), errorLogger) if len(c.TLSCertFile) > 0 && len(c.TLSKeyFile) > 0 {