runsc metric-server: Add flag to tolerate absence of --root directory.

PiperOrigin-RevId: 508122696
This commit is contained in:
Etienne Perot
2023-02-08 10:47:55 -08:00
committed by gVisor bot
parent 6d4f68ec4f
commit f08082fbd7
6 changed files with 75 additions and 30 deletions
+22 -10
View File
@@ -230,11 +230,12 @@ func queryMetrics(ctx context.Context, sand *sandbox.Sandbox, verifier *promethe
// MetricServer implements subcommands.Command for the "metric-server" command.
type MetricServer struct {
rootDir string
address string
exporterPrefix string
startTime time.Time
srv http.Server
rootDir string
allowUnknownRoot bool
address string
exporterPrefix string
startTime time.Time
srv http.Server
// Size of the map of written metrics during the last /metrics export. Initially zero.
// Used to efficiently reallocate a map of the right size during the next export.
@@ -329,7 +330,9 @@ func (m *MetricServer) refreshSandboxesLocked() {
}
sandboxIDs, err := container.ListSandboxes(m.rootDir)
if err != nil {
log.Warningf("Cannot list containers in root directory %s, it has likely gone away: %v.", m.rootDir, err)
if !m.allowUnknownRoot {
log.Warningf("Cannot list containers in root directory %s, it has likely gone away: %v.", m.rootDir, err)
}
return
}
for sandboxID, sandbox := range m.sandboxes {
@@ -752,8 +755,10 @@ func (m *MetricServer) verify(ctx context.Context) {
m.mu.Lock()
defer m.mu.Unlock()
if err != nil {
log.Warningf("Cannot list sandboxes in root directory %s, it has likely gone away: %v. Server shutting down.", m.rootDir, err)
m.shutdownLocked(ctx)
if !m.allowUnknownRoot {
log.Warningf("Cannot list sandboxes in root directory %s, it has likely gone away: %v. Server shutting down.", m.rootDir, err)
m.shutdownLocked(ctx)
}
return
}
m.refreshSandboxesLocked()
@@ -797,15 +802,22 @@ func (m *MetricServer) Execute(ctx context.Context, f *flag.FlagSet, args ...any
return util.Errorf("Metric server address contains '%%ID%%': %v. This should have been replaced by the parent process.", conf.MetricServer)
}
if _, err := container.ListSandboxes(conf.RootDir); err != nil {
return util.Errorf("Invalid root directory %q: tried to list sandboxes within it and got: %v", conf.RootDir, err)
if !conf.MetricServerAllowUnknownRoot {
return util.Errorf("Invalid root directory %q: tried to list sandboxes within it and got: %v", conf.RootDir, err)
}
log.Warningf("Invalid root directory %q: tried to list sandboxes within it and got: %v. Continuing anyway, as the server is configured to tolerate this.", conf.RootDir, err)
}
// container.ListSandboxes uses a glob pattern, which doesn't error out on
// permission errors. Double-check by actually listing the directory.
if _, err := ioutil.ReadDir(conf.RootDir); err != nil {
return util.Errorf("Invalid root directory %q: tried to list all entries within it and got: %v", conf.RootDir, err)
if !conf.MetricServerAllowUnknownRoot {
return util.Errorf("Invalid root directory %q: tried to list all entries within it and got: %v", conf.RootDir, err)
}
log.Warningf("Invalid root directory %q: tried to list all entries within it and got: %v. Continuing anyway, as the server is configured to tolerate this.", conf.RootDir, err)
}
m.startTime = time.Now()
m.rootDir = conf.RootDir
m.allowUnknownRoot = conf.MetricServerAllowUnknownRoot
m.exporterPrefix = conf.MetricExporterPrefix
if strings.Contains(conf.MetricServer, "%RUNTIME_ROOT%") {
newAddr := strings.ReplaceAll(conf.MetricServer, "%RUNTIME_ROOT%", m.rootDir)
+6
View File
@@ -152,6 +152,12 @@ type Config struct {
// The value of this flag must also match across the two command lines.
MetricServer string `flag:"metric-server"`
// MetricServerAllowUnknownRoot, if set, makes the metric server tolerate a non-existent or bad
// --root directory, and will remain running regardless of its validity.
// This is useful if the existence of the --root directory depends on the state of the machine,
// e.g. it is only created after the first pod that uses runsc has been created.
MetricServerAllowUnknownRoot bool `flag:"metric-server-allow-unknown-root"`
// MetricExporterPrefix is added as prefix to all metric names.
// It is used to follow Prometheus's exporter convention, whereby all metric names should be
// prefixed by a name meaningfully identifying the software exporting the metric.
+1
View File
@@ -53,6 +53,7 @@ func RegisterFlags(flagSet *flag.FlagSet) {
// Metrics flags.
flagSet.String("metric-server", "", "if set, export metrics on this address. This may either be 1) 'addr:port' to export metrics on a specific network interface address, 2) ':port' for exporting metrics on all interfaces, or 3) an absolute path to a Unix Domain Socket. The substring '%ID%' will be replaced by the container ID, and '%RUNTIME_ROOT%' by the root. This flag must be specified in both `runsc metric-server` and `runsc create`, and their values must match.")
flagSet.Bool("metric-server-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.")
flagSet.String("metric-exporter-prefix", "runsc_", "prefix for all metric names, following Prometheus exporter convention")
// Debugging flags: strace related
+39 -20
View File
@@ -54,7 +54,7 @@ type metricsTest struct {
// setupMetrics sets up a container configuration with metrics enabled, and returns it all.
// Also returns a cleanup function.
func setupMetrics(t *testing.T) (*metricsTest, func()) {
func setupMetrics(t *testing.T, forceTempUDS bool) (*metricsTest, func()) {
// Start the child reaper.
childReaper := &testutil.Reaper{}
childReaper.Start()
@@ -73,21 +73,19 @@ func setupMetrics(t *testing.T) (*metricsTest, func()) {
t.Fatalf("error setting up container: %v", err)
}
cu.Add(cleanup)
udsPath := filepath.Join(rootDir, "metrics.sock")
if len(udsPath) >= 100 {
// This is longer than the max UDS path length allowed by Linux. Try somewhere else in /tmp.
tmpDir, err := os.MkdirTemp("/tmp", "metrics-")
if err != nil {
t.Fatalf("Runtime root is %s which means the metrics UDS %s (%d bytes) is longer than the maximum length allowed for a UDS path. The test could also not create a temporary directory in /tmp as a fallback (%v).", rootDir, udsPath, len(udsPath), err)
}
cu.Add(func() { os.RemoveAll(tmpDir) })
udsPathTmp := filepath.Join(tmpDir, "metrics.sock")
if len(udsPathTmp) >= 100 {
t.Fatalf("Runtime root is %s which means the metrics UDS %s (%d bytes) is longer than the maximum length allowed for a UDS path. The test tried to create a fallback in /tmp but it was too long too (%q is %d characters).", rootDir, udsPath, len(udsPath), udsPathTmp, len(udsPathTmp))
}
udsPath = udsPathTmp
conf.MetricServer = udsPathTmp
tmpDir, err := os.MkdirTemp("/tmp", "metrics-")
if err != nil {
t.Fatalf("Cannot create temporary directory in /tmp: %v", err)
}
cu.Add(func() { os.RemoveAll(tmpDir) })
udsPath := filepath.Join(rootDir, "metrics.sock")
if forceTempUDS || len(udsPath) >= 100 {
udsPath = filepath.Join(tmpDir, "metrics.sock")
}
if len(udsPath) >= 100 {
t.Fatalf("Cannot come up with a UDS path shorter than the maximum length allowed by Linux (tried to use %q)", udsPath)
}
conf.MetricServer = udsPath
// The UDS should be deleted by the metrics server itself, but we clean it up here anyway just in case:
cu.Add(func() { os.Remove(udsPath) })
@@ -112,7 +110,7 @@ func setupMetrics(t *testing.T) (*metricsTest, func()) {
func TestContainerMetrics(t *testing.T) {
targetOpens := 200
te, cleanup := setupMetrics(t)
te, cleanup := setupMetrics(t /* forceTempUDS= */, false)
defer cleanup()
if _, err := te.client.GetMetrics(te.testCtx); err != nil {
@@ -202,7 +200,7 @@ func TestContainerMetrics(t *testing.T) {
// TestContainerMetricsIterationID verifies that two successive containers with the same ID
// do not have the same iteration ID.
func TestContainerMetricsIterationID(t *testing.T) {
te, cleanup := setupMetrics(t)
te, cleanup := setupMetrics(t /* forceTempUDS= */, false)
defer cleanup()
args := Args{
@@ -264,7 +262,7 @@ func TestContainerMetricsIterationID(t *testing.T) {
// unavailability or restarts.
func TestContainerMetricsRobustAgainstRestarts(t *testing.T) {
targetOpens := 200
te, cleanup := setupMetrics(t)
te, cleanup := setupMetrics(t /* forceTempUDS= */, false)
defer cleanup()
// First, start a container which will kick off the metric server as normal.
@@ -414,7 +412,7 @@ func TestContainerMetricsRobustAgainstRestarts(t *testing.T) {
func TestContainerMetricsMultiple(t *testing.T) {
numConcurrentContainers := 5
te, cleanup := setupMetrics(t)
te, cleanup := setupMetrics(t /* forceTempUDS= */, false)
defer cleanup()
var containers []*Container
needCleanup := map[*Container]struct{}{}
@@ -517,7 +515,7 @@ func TestContainerMetricsMultiple(t *testing.T) {
}
func TestMetricServerChecksRootDirectoryAccess(t *testing.T) {
te, cleanup := setupMetrics(t)
te, cleanup := setupMetrics(t /* forceTempUDS= */, false)
defer cleanup()
if err := te.client.ShutdownServer(te.testCtx); err != nil {
t.Fatalf("Cannot stop metric server: %v", err)
@@ -540,3 +538,24 @@ func TestMetricServerChecksRootDirectoryAccess(t *testing.T) {
t.Error("Metric server was successfully able to be spawned despite not having access to the root directory")
}
}
func TestMetricServerToleratesNoRootDirectory(t *testing.T) {
te, cleanup := setupMetrics(t /* forceTempUDS= */, true)
defer cleanup()
if err := te.client.ShutdownServer(te.testCtx); err != nil {
t.Fatalf("Cannot stop metric server: %v", err)
}
if err := os.RemoveAll(te.sleepConf.RootDir); err != nil {
t.Fatalf("cannot remove root directory %q: %v", te.sleepConf.RootDir, err)
}
te.sleepConf.MetricServerAllowUnknownRoot = false
shortCtx, shortCtxCancel := context.WithTimeout(te.testCtx, time.Second)
defer shortCtxCancel()
if err := te.client.SpawnServer(shortCtx, te.sleepConf); err == nil {
t.Fatalf("Metric server was successfully able to be spawned despite a non-existent root directory")
}
te.sleepConf.MetricServerAllowUnknownRoot = true
if err := te.client.SpawnServer(te.testCtx, te.sleepConf); err != nil {
t.Errorf("Metric server was not able to be spawned despite being configured to tolerate a non-existent root directory: %v", err)
}
}
+1
View File
@@ -12,6 +12,7 @@ go_library(
"//runsc:__subpackages__",
],
deps = [
"//pkg/cleanup",
"//pkg/prometheus",
"//pkg/sync",
"//runsc/config",
+6
View File
@@ -34,6 +34,7 @@ import (
"github.com/cenkalti/backoff"
"github.com/prometheus/common/expfmt"
"golang.org/x/sys/unix"
"gvisor.dev/gvisor/pkg/cleanup"
"gvisor.dev/gvisor/pkg/prometheus"
"gvisor.dev/gvisor/pkg/sync"
"gvisor.dev/gvisor/runsc/config"
@@ -187,6 +188,10 @@ func (c *MetricClient) SpawnServer(ctx context.Context, baseConf *config.Config)
overriddenConf.MetricServer = c.addr
overriddenConf.RootDir = c.rootDir
c.server = exec.Command(specutils.ExePath, overriddenConf.ToFlags()...)
cu := cleanup.Make(func() {
c.server = nil
})
defer cu.Clean()
c.server.SysProcAttr = &unix.SysProcAttr{
// Detach from this session, otherwise cmd will get SIGHUP and SIGCONT
// when re-parented.
@@ -221,6 +226,7 @@ func (c *MetricClient) SpawnServer(ctx context.Context, baseConf *config.Config)
if bindCtx.Err() != nil {
return fmt.Errorf("metrics server did not bind to %s in time: %w", c.addr, bindCtx.Err())
}
cu.Release()
return nil
}