Repository: buchgr/bazel-remote
Pull request #502 (2021-12-12 23:04:38)

Add --profile_address for consistency with --http_address and --grpc_address


Patch:
diff --git a/README.md b/README.md
index ab46c6b..4884a35 100644
--- a/README.md
+++ b/README.md
@@ -147,30 +147,36 @@ OPTIONS:
    --storage_mode value Which format to store CAS blobs in. Must be one of
       "zstd" or "uncompressed". (default: "zstd") [$BAZEL_REMOTE_STORAGE_MODE]
 
+   --http_address value Address specification for the HTTP server listener,
+      formatted either as [host]:port for TCP or unix://path.sock for Unix
+      domain sockets. [$BAZEL_REMOTE_HTTP_ADDRESS]
+
    --host value DEPRECATED. Use --http_address to specify the HTTP server
       listener. [$BAZEL_REMOTE_HOST]
 
    --port value DEPRECATED. Use --http_address to specify the HTTP server
       listener. (default: 8080) [$BAZEL_REMOTE_PORT]
 
-   --http_address value Address specification for the HTTP server listener,
+   --grpc_address value Address specification for the gRPC server listener,
       formatted either as [host]:port for TCP or unix://path.sock for Unix
-      domain sockets. [$BAZEL_REMOTE_HTTP_ADDRESS]
+      domain sockets. Set to 'none' to disable. [$BAZEL_REMOTE_GRPC_ADDRESS]
 
    --grpc_port value DEPRECATED. Use --grpc_address to specify the gRPC
       server listener. Set to 0 to disable. (default: 9092)
       [$BAZEL_REMOTE_GRPC_PORT]
 
-   --grpc_address value Address specification for the gRPC server listener,
-      formatted either as [host]:port for TCP or unix://path.sock for Unix
-      domain sockets. Set to 'none' to disable.
-      [$BAZEL_REMOTE_GRPC_ADDRESS]
+   --profile_address value Address specification for a http server to listen
+      on for profiling, formatted either as [host]:port for TCP or
+      unix://path.sock for Unix domain sockets. Off by default, but can also be
+      set to 'none' to disable explicitly. (default: "", ie profiling disabled)
+      [$BAZEL_REMOTE_PROFILE_ADDRESS]
 
-   --profile_host value A host address to listen on for profiling, if enabled
-      by a valid --profile_port setting. (default: "127.0.0.1")
-      [$BAZEL_REMOTE_PROFILE_HOST]
+   --profile_host value DEPRECATED. Use --profile_address instead. A host
+      address to listen on for profiling, if enabled by a valid --profile_port
+      setting. (default: "127.0.0.1") [$BAZEL_REMOTE_PROFILE_HOST]
 
-   --profile_port value If a positive integer, serve /debug/pprof/* URLs from
+   --profile_port value DEPRECATED. Use --profile_address instead. If a
+      positive integer, serve /debug/pprof/* URLs from
       http://profile_host:profile_port. (default: 0, ie profiling disabled)
       [$BAZEL_REMOTE_PROFILE_PORT]
 
@@ -324,12 +330,15 @@ max_size: 100
 # use unix:///path/to/socket.sock, where /path/to/socket.sock can be
 # either an absolute or relative path to a socket path.
 http_address: localhost:8080
-# The server listener address for gRPC:
+
+# The server listener address for gRPC (unix sockets are also supported
+# as described above):
 #grpc_address: localhost:9092
 
-# If profile_port is specified, then serve /debug/pprof/* URLs here:
-#profile_host: 127.0.0.1
-#profile_port: 7070
+# If profile_address (or the deprecated profile_port and/or profile_host)
+# is specified, then serve /debug/pprof/* URLs here (unix sockets are also
+# supported as described above):
+#profile_address: 127.0.0.1:7070
 
 # HTTP read/write timeouts. Note that these do not apply to the proxy
 # backends or the profiling endpoint. Reasonable values might be twice
diff --git a/config/config.go b/config/config.go
index 15435f5..a651590 100644
--- a/config/config.go
+++ b/config/config.go
@@ -37,8 +37,7 @@ type HTTPBackendConfig struct {
 type Config struct {
 	HTTPAddress                 string                    `yaml:"http_address"`
 	GRPCAddress                 string                    `yaml:"grpc_address"`
-	ProfileHost                 string                    `yaml:"profile_host"`
-	ProfilePort                 int                       `yaml:"profile_port"`
+	ProfileAddress              string                    `yaml:"profile_address"`
 	Dir                         string                    `yaml:"dir"`
 	MaxSize                     int                       `yaml:"max_size"`
 	StorageMode                 string                    `yaml:"storage_mode"`
@@ -71,9 +70,11 @@ type Config struct {
 	ErrorLogger  *log.Logger
 
 	// Deprecated fields. Retained for backwards compatibility.
-	Host     string `yaml:"host"`
-	Port     int    `yaml:"port"`
-	GRPCPort int    `yaml:"grpc_port"`
+	Host        string `yaml:"host"`
+	Port        int    `yaml:"port"`
+	GRPCPort    int    `yaml:"grpc_port"`
+	ProfileHost string `yaml:"profile_host"`
+	ProfilePort int    `yaml:"profile_port"`
 }
 
 const disabledGRPCListener = "none"
@@ -84,7 +85,7 @@ var defaultDurationBuckets = []float64{.5, 1, 2.5, 5, 10, 20, 40, 80, 160, 320}
 // an error if there were any problems with the validation.
 func newFromArgs(dir string, maxSize int, storageMode string,
 	httpAddress string, grpcAddress string,
-	profileHost string, profilePort int,
+	profileAddress string,
 	htpasswdFile string,
 	maxQueuedUploads int,
 	numUploaders int,
@@ -109,8 +110,7 @@ func newFromArgs(dir string, maxSize int, storageMode string,
 	c := Config{
 		HTTPAddress:                 httpAddress,
 		GRPCAddress:                 grpcAddress,
-		ProfileHost:                 profileHost,
-		ProfilePort:                 profilePort,
+		ProfileAddress:              profileAddress,
 		Dir:                         dir,
 		MaxSize:                     maxSize,
 		StorageMode:                 storageMode,
@@ -185,6 +185,10 @@ func newFromYaml(data []byte) (*Config, error) {
 		c.GRPCAddress = net.JoinHostPort(c.Host, strconv.Itoa(c.GRPCPort))
 	}
 
+	if c.ProfileAddress == "" && c.ProfilePort > 0 {
+		c.ProfileAddress = net.JoinHostPort(c.ProfileHost, strconv.Itoa(c.ProfilePort))
+	}
+
 	if c.MetricsDurationBuckets != nil {
 		sort.Float64s(c.MetricsDurationBuckets)
 	}
@@ -255,6 +259,14 @@ func validateConfig(c *Config) error {
 		}
 	}
 
+	if c.ProfileAddress != "" && c.ProfileAddress != disabledGRPCListener {
+		if strings.HasPrefix(c.ProfileAddress, "unix://") {
+			if c.ProfileAddress[len("unix://"):] == "" {
+				return errors.New("'profile_address' Unix socket specification is missing a socket path")
+			}
+		}
+	}
+
 	if c.GRPCAddress == disabledGRPCListener && c.ExperimentalRemoteAssetAPI {
 		return errors.New("Remote Asset API support depends on gRPC being enabled")
 	}
@@ -368,6 +380,13 @@ func get(ctx *cli.Context) (*Config, error) {
 		grpcAddress = net.JoinHostPort(ctx.String("host"), strconv.Itoa(ctx.Int("grpc_port")))
 	}
 
+	profileAddress := ctx.String("profile_address")
+	if profileAddress == "" && ctx.Int("profile_port") > 0 {
+		profileAddress = net.JoinHostPort(ctx.String("profile_host"), strconv.Itoa(ctx.Int("profile_port")))
+	} else if profileAddress == "none" {
+		profileAddress = ""
+	}
+
 	var s3 *S3CloudStorageConfig
 	if ctx.String("s3.bucket") != "" {
 		s3 = &S3CloudStorageConfig{
@@ -407,8 +426,7 @@ func get(ctx *cli.Context) (*Config, error) {
 		ctx.String("storage_mode"),
 		httpAddress,
 		grpcAddress,
-		ctx.String("profile_host"),
-		ctx.Int("profile_port"),
+		profileAddress,
 		ctx.String("htpasswd_file"),
 		ctx.Int("max_queued_uploads"),
 		ctx.Int("num_uploaders"),
diff --git a/main.go b/main.go
index 305586e..b04b122 100644
--- a/main.go
+++ b/main.go
@@ -9,7 +9,6 @@ import (
 	_ "net/http/pprof" // Register pprof handlers with DefaultServeMux.
 	"os"
 	"runtime"
-	"strconv"
 	"strings"
 
 	auth "github.com/abbot/go-http-auth"
@@ -237,14 +236,12 @@ func run(ctx *cli.Context) error {
 		}()
 	}
 
-	if c.ProfilePort > 0 {
+	if c.ProfileAddress != "" {
 		go func() {
 			// Allow access to /debug/pprof/ URLs.
-			profileAddr := c.ProfileHost + ":" +
-				strconv.Itoa(c.ProfilePort)
 			log.Printf("Starting HTTP server for profiling on address %s",
-				profileAddr)
-			log.Fatal(http.ListenAndServe(profileAddr, nil))
+				c.ProfileAddress)
+			log.Fatal(`Failed to listen on address: "`, c.ProfileAddress, `": `, http.ListenAndServe(c.ProfileAddress, nil))
 		}()
 	}
 
@@ -260,7 +257,7 @@ func run(ctx *cli.Context) error {
 		ln, err = net.Listen("tcp", c.HTTPAddress)
 	}
 	if err != nil {
-		log.Fatal(err)
+		log.Fatal(`Failed to listen on address: "`, c.HTTPAddress, `": `, err)
 	}
 
 	if len(c.TLSCertFile) > 0 && len(c.TLSKeyFile) > 0 {
diff --git a/utils/flags/flags.go b/utils/flags/flags.go
index 28069cf..6395df9 100644
--- a/utils/flags/flags.go
+++ b/utils/flags/flags.go
@@ -42,6 +42,11 @@ func GetCliFlags() []cli.Flag {
 			Usage:   "Which format to store CAS blobs in. Must be one of \"zstd\" or \"uncompressed\".",
 			EnvVars: []string{"BAZEL_REMOTE_STORAGE_MODE"},
 		},
+		&cli.StringFlag{
+			Name:    "http_address",
+			Usage:   "Address specification for the HTTP server listener, formatted either as [host]:port for TCP or unix://path.sock for Unix domain sockets.",
+			EnvVars: []string{"BAZEL_REMOTE_HTTP_ADDRESS"},
+		},
 		&cli.StringFlag{
 			Name:    "host",
 			Value:   "",
@@ -55,9 +60,10 @@ func GetCliFlags() []cli.Flag {
 			EnvVars: []string{"BAZEL_REMOTE_PORT"},
 		},
 		&cli.StringFlag{
-			Name:    "http_address",
-			Usage:   "Address specification for the HTTP server listener, formatted either as [host]:port for TCP or unix://path.sock for Unix domain sockets.",
-			EnvVars: []string{"BAZEL_REMOTE_HTTP_ADDRESS"},
+			Name: "grpc_address",
+			Usage: "Address specification for the gRPC server listener, formatted either as [host]:port for TCP or unix://path.sock for Unix domain sockets. " +
+				"Set to 'none' to disable.",
+			EnvVars: []string{"BAZEL_REMOTE_GRPC_ADDRESS"},
 		},
 		&cli.IntFlag{
 			Name:    "grpc_port",
@@ -66,21 +72,24 @@ func GetCliFlags() []cli.Flag {
 			EnvVars: []string{"BAZEL_REMOTE_GRPC_PORT"},
 		},
 		&cli.StringFlag{
-			Name: "grpc_address",
-			Usage: "Address specification for the gRPC server listener, formatted either as [host]:port for TCP or unix://path.sock for Unix domain sockets. " +
-				"Set to 'none' to disable.",
-			EnvVars: []string{"BAZEL_REMOTE_GRPC_ADDRESS"},
+			Name: "profile_address",
+			Usage: "Address specification for a http server to listen on for profiling, formatted either as [host]:port for TCP or " +
+				"unix://path.sock for Unix domain sockets. Off by default, but can also be set to 'none' to disable explicitly.",
+			DefaultText: "\"\", ie profiling disabled",
+			EnvVars:     []string{"BAZEL_REMOTE_PROFILE_ADDRESS"},
 		},
 		&cli.StringFlag{
-			Name:    "profile_host",
-			Value:   "127.0.0.1",
-			Usage:   "A host address to listen on for profiling, if enabled by a valid --profile_port setting.",
+			Name:  "profile_host",
+			Value: "127.0.0.1",
+			Usage: "DEPRECATED. Use --profile_address instead. A host address to listen on for profiling, if enabled by " +
+				"a valid --profile_port setting.",
 			EnvVars: []string{"BAZEL_REMOTE_PROFILE_HOST"},
 		},
 		&cli.IntFlag{
-			Name:        "profile_port",
-			Value:       0,
-			Usage:       "If a positive integer, serve /debug/pprof/* URLs from http://profile_host:profile_port.",
+			Name:  "profile_port",
+			Value: 0,
+			Usage: "DEPRECATED. Use --profile_address instead. If a positive integer, serve /debug/pprof/* URLs " +
+				"from http://profile_host:profile_port.",
 			DefaultText: "0, ie profiling disabled",
 			EnvVars:     []string{"BAZEL_REMOTE_PROFILE_PORT"},
 		},


Test patch:
diff --git a/config/config_test.go b/config/config_test.go
index 4a966c0..348144b 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -224,7 +224,7 @@ func TestValidProfiling(t *testing.T) {
 port: 1234
 dir: /opt/cache-dir
 max_size: 42
-profile_port: 7070
+profile_address: :7070
 `
 	config, err := newFromYaml([]byte(yaml))
 	if err != nil {
@@ -238,8 +238,7 @@ profile_port: 7070
 		Dir:                    "/opt/cache-dir",
 		MaxSize:                42,
 		StorageMode:            "zstd",
-		ProfilePort:            7070,
-		ProfileHost:            "",
+		ProfileAddress:         ":7070",
 		NumUploaders:           100,
 		MaxQueuedUploads:       1000000,
 		MaxBlobSize:            math.MaxInt64,
@@ -252,9 +251,9 @@ profile_port: 7070
 	}
 
 	yaml += `
-profile_host: 192.168.1.1`
+profile_address: 192.168.1.1:7070`
 
-	expectedConfig.ProfileHost = "192.168.1.1"
+	expectedConfig.ProfileAddress = "192.168.1.1:7070"
 
 	config, err = newFromYaml([]byte(yaml))
 	if err != nil {