From aa306e4bf5657ce4a0aca4e0c64ff6828523d362 Mon Sep 17 00:00:00 2001 From: Kern Walster Date: Thu, 19 Dec 2024 23:33:18 +0000 Subject: [PATCH] Create metrics socket dir Before this change, if the metrics socket was a unix socket, SOCI would not create the parent dir before trying to bind. This exposed an implicit dependency on bind order if you tried to put the metrics address next to the main socket which we broke when we introduced systemd socket activation. This change ensures that we create the parent dir before binding the metrics socket. Signed-off-by: Kern Walster --- cmd/soci-snapshotter-grpc/main.go | 8 +++++++- integration/startup_test.go | 12 +++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/cmd/soci-snapshotter-grpc/main.go b/cmd/soci-snapshotter-grpc/main.go index 10f4969aa..24cdffb9c 100644 --- a/cmd/soci-snapshotter-grpc/main.go +++ b/cmd/soci-snapshotter-grpc/main.go @@ -221,7 +221,13 @@ func serve(ctx context.Context, rpc *grpc.Server, addr string, rs snapshots.Snap // We need to consider both the existence of MetricsAddress as well as NoPrometheus flag not set if cfg.MetricsAddress != "" && !cfg.NoPrometheus { - l, err := net.Listen(cfg.MetricsNetwork, cfg.MetricsAddress) + var l net.Listener + var err error + if cfg.MetricsNetwork == "unix" { + l, err = listenUnix(cfg.MetricsAddress) + } else { + l, err = net.Listen(cfg.MetricsNetwork, cfg.MetricsAddress) + } if err != nil { return false, fmt.Errorf("failed to get listener for metrics endpoint: %w", err) } diff --git a/integration/startup_test.go b/integration/startup_test.go index 2c8beab8e..8f43f0a9e 100644 --- a/integration/startup_test.go +++ b/integration/startup_test.go @@ -24,13 +24,23 @@ import ( shell "github.com/awslabs/soci-snapshotter/util/dockershell" ) +// Use a custom metrics config to test that the snapshotter +// correctly starts up when the metrics address is next to the socket address. +// This tests a regression with the first implementation of systemd socket activation +// where we moved creation of the directory later which caused the metrics address +// bind to fail. This verifies that the directory gets created before binding the metrics socket. +const metricsConfig = ` +metrics_address="/run/soci-snapshotter-grpc/metrics.sock" +metrics_network="unix" +` + // TestSnapshotterStartup tests to run containerd + snapshotter and check plugin is // recognized by containerd func TestSnapshotterStartup(t *testing.T) { t.Parallel() sh, done := newSnapshotterBaseShell(t) defer done() - rebootContainerd(t, sh, "", "") + rebootContainerd(t, sh, "", getSnapshotterConfigToml(t, false, metricsConfig)) found := false err := sh.ForEach(shell.C("ctr", "plugin", "ls"), func(l string) bool { info := strings.Fields(l)