Skip to content

Commit

Permalink
Create metrics socket dir
Browse files Browse the repository at this point in the history
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 <walster@amazon.com>
  • Loading branch information
Kern-- committed Dec 20, 2024
1 parent ac157d8 commit aa306e4
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
8 changes: 7 additions & 1 deletion cmd/soci-snapshotter-grpc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
12 changes: 11 additions & 1 deletion integration/startup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit aa306e4

Please sign in to comment.