Merge "Cleanup RBE logs directory" into tm-dev

4 files changed, 63 insertions, 47 deletions

diff --git a/ui/build/build.go b/ui/build/build.go
index aadf4af4e..ec42b7004 100644
--- a/ui/build/build.go
+++ b/ui/build/build.go
@@ -266,6 +266,7 @@ func Build(ctx Context, config Config) {
 	}
 
 	if config.StartRBE() {
+		cleanupRBELogsDir(ctx, config)
 		startRBE(ctx, config)
 		defer DumpRBEMetrics(ctx, config, filepath.Join(config.LogsDir(), "rbe_metrics.pb"))
 	}
diff --git a/ui/build/config.go b/ui/build/config.go
index e271bfca2..5765f2123 100644
--- a/ui/build/config.go
+++ b/ui/build/config.go
@@ -19,12 +19,14 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"math/rand"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
 	"strconv"
 	"strings"
+	"syscall"
 	"time"
 
 	"android/soong/shared"
@@ -42,6 +44,15 @@ const (
 	envConfigFetchTimeout = 10 * time.Second
 )
 
+var (
+	rbeRandPrefix int
+)
+
+func init() {
+	rand.Seed(time.Now().UnixNano())
+	rbeRandPrefix = rand.Intn(1000)
+}
+
 type Config struct{ *configImpl }
 
 type configImpl struct {
@@ -1144,34 +1155,25 @@ func (c *configImpl) StartRBE() bool {
 	return true
 }
 
-func (c *configImpl) rbeLogDir() string {
-	for _, f := range []string{"RBE_log_dir", "FLAG_log_dir"} {
-		if v, ok := c.environ.Get(f); ok {
-			return v
-		}
-	}
-	if c.Dist() {
-		return c.LogsDir()
-	}
-	return c.OutDir()
-}
-
-func (c *configImpl) rbeStatsOutputDir() string {
-	for _, f := range []string{"RBE_output_dir", "FLAG_output_dir"} {
+func (c *configImpl) rbeProxyLogsDir() string {
+	for _, f := range []string{"RBE_proxy_log_dir", "FLAG_output_dir"} {
 		if v, ok := c.environ.Get(f); ok {
 			return v
 		}
 	}
-	return c.rbeLogDir()
+	buildTmpDir := shared.TempDirForOutDir(c.SoongOutDir())
+	return filepath.Join(buildTmpDir, "rbe")
 }
 
-func (c *configImpl) rbeLogPath() string {
-	for _, f := range []string{"RBE_log_path", "FLAG_log_path"} {
-		if v, ok := c.environ.Get(f); ok {
-			return v
+func (c *configImpl) shouldCleanupRBELogsDir() bool {
+	// Perform a log directory cleanup only when the log directory
+	// is auto created by the build rather than user-specified.
+	for _, f := range []string{"RBE_proxy_log_dir", "FLAG_output_dir"} {
+		if _, ok := c.environ.Get(f); ok {
+			return false
 		}
 	}
-	return fmt.Sprintf("text://%v/reproxy_log.txt", c.rbeLogDir())
+	return true
 }
 
 func (c *configImpl) rbeExecRoot() string {
@@ -1223,6 +1225,23 @@ func (c *configImpl) rbeAuth() (string, string) {
 	return "RBE_use_application_default_credentials", "true"
 }
 
+func (c *configImpl) rbeSockAddr(dir string) (string, error) {
+	maxNameLen := len(syscall.RawSockaddrUnix{}.Path)
+	base := fmt.Sprintf("reproxy_%v.sock", rbeRandPrefix)
+
+	name := filepath.Join(dir, base)
+	if len(name) < maxNameLen {
+		return name, nil
+	}
+
+	name = filepath.Join("/tmp", base)
+	if len(name) < maxNameLen {
+		return name, nil
+	}
+
+	return "", fmt.Errorf("cannot generate a proxy socket address shorter than the limit of %v", maxNameLen)
+}
+
 func (c *configImpl) UseRemoteBuild() bool {
 	return c.UseGoma() || c.UseRBE()
 }
diff --git a/ui/build/rbe.go b/ui/build/rbe.go
index 8f9a69991..3e558f738 100644
--- a/ui/build/rbe.go
+++ b/ui/build/rbe.go
@@ -16,12 +16,9 @@ package build
 
 import (
 	"fmt"
-	"math/rand"
 	"os"
 	"path/filepath"
 	"runtime"
-	"syscall"
-	"time"
 
 	"android/soong/ui/metrics"
 )
@@ -54,34 +51,16 @@ func rbeCommand(ctx Context, config Config, rbeCmd string) string {
 	return cmdPath
 }
 
-func sockAddr(dir string) (string, error) {
-	maxNameLen := len(syscall.RawSockaddrUnix{}.Path)
-	rand.Seed(time.Now().UnixNano())
-	base := fmt.Sprintf("reproxy_%v.sock", rand.Intn(1000))
-
-	name := filepath.Join(dir, base)
-	if len(name) < maxNameLen {
-		return name, nil
-	}
-
-	name = filepath.Join("/tmp", base)
-	if len(name) < maxNameLen {
-		return name, nil
-	}
-
-	return "", fmt.Errorf("cannot generate a proxy socket address shorter than the limit of %v", maxNameLen)
-}
-
 func getRBEVars(ctx Context, config Config) map[string]string {
 	vars := map[string]string{
-		"RBE_log_path":   config.rbeLogPath(),
-		"RBE_log_dir":    config.rbeLogDir(),
+		"RBE_log_dir":    config.rbeProxyLogsDir(),
 		"RBE_re_proxy":   config.rbeReproxy(),
 		"RBE_exec_root":  config.rbeExecRoot(),
-		"RBE_output_dir": config.rbeStatsOutputDir(),
+		"RBE_output_dir": config.rbeProxyLogsDir(),
+		"RBE_proxy_log_dir": config.rbeProxyLogsDir(),
 	}
 	if config.StartRBE() {
-		name, err := sockAddr(absPath(ctx, config.TempDir()))
+		name, err := config.rbeSockAddr(absPath(ctx, config.TempDir()))
 		if err != nil {
 			ctx.Fatalf("Error retrieving socket address: %v", err)
 			return nil
@@ -100,6 +79,17 @@ func getRBEVars(ctx Context, config Config) map[string]string {
 	return vars
 }
 
+func cleanupRBELogsDir(ctx Context, config Config) {
+	if !config.shouldCleanupRBELogsDir() {
+		return
+	}
+
+	rbeTmpDir := config.rbeProxyLogsDir()
+	if err := os.RemoveAll(rbeTmpDir); err != nil {
+		fmt.Fprintln(ctx.Writer, "\033[33mUnable to remove RBE log directory: ", err, "\033[0m")
+	}
+}
+
 func startRBE(ctx Context, config Config) {
 	ctx.BeginTrace(metrics.RunSetupTool, "rbe_bootstrap")
 	defer ctx.EndTrace()
@@ -110,6 +100,11 @@ func startRBE(ctx Context, config Config) {
 	if n := ulimitOrFatal(ctx, config, "-n"); n < rbeLeastNFiles {
 		ctx.Fatalf("max open files is insufficient: %d; want >= %d.\n", n, rbeLeastNFiles)
 	}
+	if _, err := os.Stat(config.rbeProxyLogsDir()); os.IsNotExist(err) {
+		if err := os.MkdirAll(config.rbeProxyLogsDir(), 0744); err != nil {
+			ctx.Fatalf("Unable to create logs dir (%v) for RBE: %v", config.rbeProxyLogsDir, err)
+		}
+	}
 
 	cmd := Command(ctx, config, "startRBE bootstrap", rbeCommand(ctx, config, bootstrapCmd))
 
@@ -151,7 +146,7 @@ func DumpRBEMetrics(ctx Context, config Config, filename string) {
 		return
 	}
 
-	outputDir := config.rbeStatsOutputDir()
+	outputDir := config.rbeProxyLogsDir()
 	if outputDir == "" {
 		ctx.Fatal("RBE output dir variable not defined. Aborting metrics dumping.")
 	}
diff --git a/ui/build/rbe_test.go b/ui/build/rbe_test.go
index 8ff96bcbb..266f76b35 100644
--- a/ui/build/rbe_test.go
+++ b/ui/build/rbe_test.go
@@ -56,7 +56,8 @@ func TestDumpRBEMetrics(t *testing.T) {
 			env := Environment(tt.env)
 			env.Set("OUT_DIR", tmpDir)
 			env.Set("RBE_DIR", tmpDir)
-			env.Set("RBE_output_dir", t.TempDir())
+			env.Set("RBE_output_dir", tmpDir)
+			env.Set("RBE_proxy_log_dir", tmpDir)
 			config := Config{&configImpl{
 				environ: &env,
 			}}