From e8c1fff214d0ecf02cfe5aa9c62d11174130c339 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Tue, 22 Aug 2023 14:16:19 -0700 Subject: [PATCH] `runsc metric-server`: Move metric server to its own binary. This creates a separate `go_binary` target for the metric server, and changes the `runsc metric-server` subcommand to call it instead of directly executing metric server code. This helps avoid Go library dependencies used only by the metric server into the `runsc` binary. PiperOrigin-RevId: 559221695 --- runsc/cmd/BUILD | 2 +- runsc/cmd/metric_server.go | 42 +++++++++----- runsc/cmd/metricserver/BUILD | 29 ++++++++++ runsc/cmd/metricserver/metricserver_main.go | 61 +++++++++++++++++++++ test/metricclient/metricclient.go | 2 +- 5 files changed, 120 insertions(+), 16 deletions(-) create mode 100644 runsc/cmd/metricserver/BUILD create mode 100644 runsc/cmd/metricserver/metricserver_main.go diff --git a/runsc/cmd/BUILD b/runsc/cmd/BUILD index e6417a64b..f251f260a 100644 --- a/runsc/cmd/BUILD +++ b/runsc/cmd/BUILD @@ -70,6 +70,7 @@ go_library( "//pkg/unet", "//pkg/urpc", "//runsc/boot", + "//runsc/cmd/metricserver", "//runsc/cmd/util", "//runsc/config", "//runsc/console", @@ -77,7 +78,6 @@ go_library( "//runsc/flag", "//runsc/fsgofer", "//runsc/fsgofer/filter", - "//runsc/metricserver", "//runsc/metricserver/containermetrics", "//runsc/mitigate", "//runsc/profile", diff --git a/runsc/cmd/metric_server.go b/runsc/cmd/metric_server.go index a3a308371..fb55d1f65 100644 --- a/runsc/cmd/metric_server.go +++ b/runsc/cmd/metric_server.go @@ -16,17 +16,22 @@ package cmd import ( "context" + "fmt" + "os" "github.com/google/subcommands" + "gvisor.dev/gvisor/runsc/cmd/metricserver" "gvisor.dev/gvisor/runsc/cmd/util" "gvisor.dev/gvisor/runsc/config" "gvisor.dev/gvisor/runsc/flag" - "gvisor.dev/gvisor/runsc/metricserver" ) // MetricServer implements subcommands.Command for the "metric-server" command. type MetricServer struct { - Server metricserver.Server + ExporterPrefix string + PIDFile string + ExposeProfileEndpoints bool + AllowUnknownRoot bool } // Name implements subcommands.Command.Name. @@ -47,10 +52,10 @@ func (*MetricServer) Usage() string { // SetFlags implements subcommands.Command.SetFlags. func (m *MetricServer) SetFlags(f *flag.FlagSet) { - f.StringVar(&m.Server.ExporterPrefix, "exporter-prefix", "runsc_", "Prefix for all metric names, following Prometheus exporter convention") - f.StringVar(&m.Server.PIDFile, "pid-file", "", "If set, write the metric server's own PID to this file after binding to the --metric-server address. The parent directory of this file must already exist.") - f.BoolVar(&m.Server.ExposeProfileEndpoints, "allow-profiling", false, "If true, expose /runsc-metrics/profile-cpu and /runsc-metrics/profile-heap to get profiling data about the metric server") - f.BoolVar(&m.Server.AllowUnknownRoot, "allow-unknown-root", false, "if set, the metric server will keep running regardless of the existence of --root or the metric server's ability to access it.") + f.StringVar(&m.ExporterPrefix, "exporter-prefix", "runsc_", "Prefix for all metric names, following Prometheus exporter convention") + f.StringVar(&m.PIDFile, "pid-file", "", "If set, write the metric server's own PID to this file after binding to the --metric-server address. The parent directory of this file must already exist.") + f.BoolVar(&m.ExposeProfileEndpoints, "allow-profiling", false, "If true, expose /runsc-metrics/profile-cpu and /runsc-metrics/profile-heap to get profiling data about the metric server") + f.BoolVar(&m.AllowUnknownRoot, "allow-unknown-root", false, "if set, the metric server will keep running regardless of the existence of --root or the metric server's ability to access it.") } // Execute implements subcommands.Command.Execute. @@ -59,13 +64,22 @@ func (m *MetricServer) Execute(ctx context.Context, f *flag.FlagSet, args ...any f.Usage() return subcommands.ExitUsageError } - m.Server.Config = args[0].(*config.Config) - if m.Server.Config.MetricServer == "" || m.Server.Config.RootDir == "" { - f.Usage() - return subcommands.ExitUsageError + var newArgs []string + newArgs = append(newArgs, metricserver.BinaryName) + newArgs = append(newArgs, args[0].(*config.Config).ToFlags()...) + newArgs = append(newArgs, + fmt.Sprintf("--metricserver-exporter-prefix=%s", m.ExporterPrefix), + fmt.Sprintf("--metricserver-pid-file=%s", m.PIDFile), + fmt.Sprintf("--metricserver-allow-profiling=%t", m.ExposeProfileEndpoints), + fmt.Sprintf("--metricserver-allow-unknown-root=%t", m.AllowUnknownRoot), + ) + err := metricserver.Exec(metricserver.Options{ + Argv: newArgs, + Envv: os.Environ(), + }) + if err != nil { + util.Fatalf("metric server: %v", err) } - if err := m.Server.Run(ctx); err != nil { - return util.Errorf("%v", err) - } - return subcommands.ExitSuccess + util.Fatalf("unreachable") + return subcommands.ExitFailure } diff --git a/runsc/cmd/metricserver/BUILD b/runsc/cmd/metricserver/BUILD new file mode 100644 index 000000000..c9edf95e9 --- /dev/null +++ b/runsc/cmd/metricserver/BUILD @@ -0,0 +1,29 @@ +load("//tools:defs.bzl", "go_binary") +load("//tools/embeddedbinary:defs.bzl", "embedded_binary_go_library") + +package( + default_applicable_licenses = ["//:license"], + licenses = ["notice"], +) + +go_binary( + name = "metricserver_bin", + srcs = [ + "metricserver_main.go", + ], + deps = [ + "//runsc/cmd/util", + "//runsc/config", + "//runsc/flag", + "//runsc/metricserver", + "@com_github_google_subcommands//:go_default_library", + ], +) + +embedded_binary_go_library( + name = "metricserver", + binary = ":metricserver_bin", + visibility = [ + "//runsc:__subpackages__", + ], +) diff --git a/runsc/cmd/metricserver/metricserver_main.go b/runsc/cmd/metricserver/metricserver_main.go new file mode 100644 index 000000000..d301570ab --- /dev/null +++ b/runsc/cmd/metricserver/metricserver_main.go @@ -0,0 +1,61 @@ +// Copyright 2023 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// The metricserver binary is a separate binary that implements the +// 'runsc metric-server' subcommand. +package main + +import ( + "context" + "os" + + "github.com/google/subcommands" + "gvisor.dev/gvisor/runsc/cmd/util" + "gvisor.dev/gvisor/runsc/config" + "gvisor.dev/gvisor/runsc/flag" + "gvisor.dev/gvisor/runsc/metricserver" +) + +// Main returns the status code of the metric server. +func Main() subcommands.ExitStatus { + ctx := context.Background() + config.RegisterFlags(flag.CommandLine) + server := metricserver.Server{} + flag.CommandLine.StringVar(&server.ExporterPrefix, "metricserver-exporter-prefix", "runsc_", "Prefix for all metric names, following Prometheus exporter convention") + flag.CommandLine.StringVar(&server.PIDFile, "metricserver-pid-file", "", "If set, write the metric server's own PID to this file after binding to the --metric-server address. The parent directory of this file must already exist.") + flag.CommandLine.BoolVar(&server.ExposeProfileEndpoints, "metricserver-allow-profiling", false, "If true, expose /runsc-metrics/profile-cpu and /runsc-metrics/profile-heap to get profiling data about the metric server") + flag.CommandLine.BoolVar(&server.AllowUnknownRoot, "metricserver-allow-unknown-root", false, "if set, the metric server will keep running regardless of the existence of --root or the metric server's ability to access it.") + flag.Parse() + if flag.CommandLine.NArg() != 0 { + flag.CommandLine.Usage() + return subcommands.ExitUsageError + } + conf, err := config.NewFromFlags(flag.CommandLine) + if err != nil { + util.Fatalf(err.Error()) + } + if conf.MetricServer == "" || conf.RootDir == "" { + flag.CommandLine.Usage() + return subcommands.ExitUsageError + } + server.Config = conf + if err := server.Run(ctx); err != nil { + return util.Errorf("%v", err) + } + return subcommands.ExitSuccess +} + +func main() { + os.Exit(int(Main())) +} diff --git a/test/metricclient/metricclient.go b/test/metricclient/metricclient.go index 7a3cd450a..873e593a4 100644 --- a/test/metricclient/metricclient.go +++ b/test/metricclient/metricclient.go @@ -184,7 +184,7 @@ func (c *MetricClient) SpawnServer(ctx context.Context, baseConf *config.Config, if c.server != nil { return errors.New("this metric client already has a server associated with it") } - bindCtx, bindCancel := context.WithTimeout(ctx, 5*time.Second) + bindCtx, bindCancel := context.WithTimeout(ctx, 20*time.Second) defer bindCancel() launchBackoff := backoff.WithContext(&backoff.ExponentialBackOff{ InitialInterval: time.Millisecond,