From 7d6bee26270c6ebb4f234746bbadd7e7b909285b Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Sat, 28 Jan 2023 17:04:43 -0800 Subject: [PATCH 1/3] Fix tracing listener on Windows Signed-off-by: Gabriel Adrian Samfira --- cmd/buildkitd/main.go | 11 +++-------- cmd/buildkitd/main_unix.go | 20 ++++++++++++++++++++ cmd/buildkitd/main_windows.go | 26 ++++++++++++++++++++++++++ executor/oci/spec.go | 23 ++++++++++++++++------- executor/oci/spec_unix.go | 6 ++++++ executor/oci/spec_windows.go | 6 ++++++ 6 files changed, 77 insertions(+), 15 deletions(-) diff --git a/cmd/buildkitd/main.go b/cmd/buildkitd/main.go index 350129902680..1744abb4c291 100644 --- a/cmd/buildkitd/main.go +++ b/cmd/buildkitd/main.go @@ -630,7 +630,7 @@ func newController(c *cli.Context, cfg *config.Config) (*control.Controller, err var traceSocket string if tc != nil { - traceSocket = filepath.Join(cfg.Root, "otel-grpc.sock") + traceSocket = traceSocketPath(cfg.Root) if err := runTraceController(traceSocket, tc); err != nil { return nil, err } @@ -813,14 +813,9 @@ func parseBoolOrAuto(s string) (*bool, error) { func runTraceController(p string, exp sdktrace.SpanExporter) error { server := grpc.NewServer() tracev1.RegisterTraceServiceServer(server, &traceCollector{exporter: exp}) - uid := os.Getuid() - l, err := sys.GetLocalListener(p, uid, uid) + l, err := getLocalListener(p) if err != nil { - return err - } - if err := os.Chmod(p, 0666); err != nil { - l.Close() - return err + return errors.Wrap(err, "creating trace controller listener") } go server.Serve(l) return nil diff --git a/cmd/buildkitd/main_unix.go b/cmd/buildkitd/main_unix.go index 5a4d21d7099a..b50187719612 100644 --- a/cmd/buildkitd/main_unix.go +++ b/cmd/buildkitd/main_unix.go @@ -6,8 +6,11 @@ package main import ( "crypto/tls" "net" + "os" + "path/filepath" "syscall" + "github.com/containerd/containerd/sys" "github.com/coreos/go-systemd/v22/activation" "github.com/pkg/errors" ) @@ -43,3 +46,20 @@ func listenFD(addr string, tlsConfig *tls.Config) (net.Listener, error) { //TODO: systemd fd selection (default is 3) return nil, errors.New("not supported yet") } + +func traceSocketPath(root string) string { + return filepath.Join(root, "otel-grpc.sock") +} + +func getLocalListener(listenerPath string) (net.Listener, error) { + uid := os.Getuid() + l, err := sys.GetLocalListener(listenerPath, uid, uid) + if err != nil { + return nil, err + } + if err := os.Chmod(listenerPath, 0666); err != nil { + l.Close() + return nil, err + } + return l, nil +} diff --git a/cmd/buildkitd/main_windows.go b/cmd/buildkitd/main_windows.go index 196e4c6f7526..e49a4d5c7749 100644 --- a/cmd/buildkitd/main_windows.go +++ b/cmd/buildkitd/main_windows.go @@ -7,10 +7,36 @@ import ( "crypto/tls" "net" + "github.com/Microsoft/go-winio" _ "github.com/moby/buildkit/solver/llbsolver/ops" "github.com/pkg/errors" ) +const ( + defaultTraceSocketPath = `\\.\pipe\otel-grpc` +) + func listenFD(addr string, tlsConfig *tls.Config) (net.Listener, error) { return nil, errors.New("listening server on fd not supported on windows") } + +func traceSocketPath(root string) string { + return defaultTraceSocketPath +} + +func getLocalListener(listenerPath string) (net.Listener, error) { + pc := &winio.PipeConfig{ + // Allow generic read and generic write access to authenticated users + // and system users. On Linux, this pipe seems to be given rw access to + // user, group and others (666). + // TODO(gabriel-samfira): should we restrict access to this pipe to just + // authenticated users? Or Administrators group? + SecurityDescriptor: "D:P(A;;GRGW;;;AU)(A;;GRGW;;;SY)", + } + + listener, err := winio.ListenPipe(listenerPath, pc) + if err != nil { + return nil, errors.Wrap(err, "creating listener") + } + return listener, nil +} diff --git a/executor/oci/spec.go b/executor/oci/spec.go index 94b48a7aa9ff..d313c3f70634 100644 --- a/executor/oci/spec.go +++ b/executor/oci/spec.go @@ -4,6 +4,7 @@ import ( "context" "path" "path/filepath" + "runtime" "strings" "sync" @@ -112,7 +113,7 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou if tracingSocket != "" { // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md - meta.Env = append(meta.Env, "OTEL_TRACES_EXPORTER=otlp", "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=unix:///dev/otel-grpc.sock", "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=grpc") + meta.Env = append(meta.Env, tracingEnvVars...) meta.Env = append(meta.Env, traceexec.Environ(ctx)...) } @@ -183,12 +184,20 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou } if tracingSocket != "" { - s.Mounts = append(s.Mounts, specs.Mount{ - Destination: "/dev/otel-grpc.sock", - Type: "bind", - Source: tracingSocket, - Options: []string{"ro", "rbind"}, - }) + if runtime.GOOS == "windows" { + s.Mounts = append(s.Mounts, specs.Mount{ + Destination: `\\.\pipe\otel-grpc`, + Source: tracingSocket, + Options: []string{"ro"}, + }) + } else { + s.Mounts = append(s.Mounts, specs.Mount{ + Destination: "/dev/otel-grpc.sock", + Type: "bind", + Source: tracingSocket, + Options: []string{"ro", "rbind"}, + }) + } } s.Mounts = dedupMounts(s.Mounts) diff --git a/executor/oci/spec_unix.go b/executor/oci/spec_unix.go index f906f79b6bac..3933764c8502 100644 --- a/executor/oci/spec_unix.go +++ b/executor/oci/spec_unix.go @@ -21,6 +21,12 @@ import ( "github.com/pkg/errors" ) +var tracingEnvVars = []string{ + "OTEL_TRACES_EXPORTER=otlp", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=unix:///dev/otel-grpc.sock", + "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=grpc", +} + func generateMountOpts(resolvConf, hostsFile string) ([]oci.SpecOpts, error) { return []oci.SpecOpts{ // https://github.com/moby/buildkit/issues/429 diff --git a/executor/oci/spec_windows.go b/executor/oci/spec_windows.go index 48b0969e3922..69827931cf47 100644 --- a/executor/oci/spec_windows.go +++ b/executor/oci/spec_windows.go @@ -10,6 +10,12 @@ import ( "github.com/pkg/errors" ) +var tracingEnvVars = []string{ + "OTEL_TRACES_EXPORTER=otlp", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=npipe:////./pipe/otel-grpc", + "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=grpc", +} + func generateMountOpts(resolvConf, hostsFile string) ([]oci.SpecOpts, error) { return nil, nil } From b133b13652e50efd88c24eb1d34735a3f0e6f97f Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Thu, 9 Feb 2023 10:54:10 -0800 Subject: [PATCH 2/3] Add platform tracing socket paths and mounts Signed-off-by: Gabriel Adrian Samfira --- executor/oci/spec.go | 22 +++++++--------------- executor/oci/spec_unix.go | 21 ++++++++++++++++----- executor/oci/spec_windows.go | 24 +++++++++++++++++++----- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/executor/oci/spec.go b/executor/oci/spec.go index d313c3f70634..c99a23ec40b1 100644 --- a/executor/oci/spec.go +++ b/executor/oci/spec.go @@ -4,7 +4,6 @@ import ( "context" "path" "path/filepath" - "runtime" "strings" "sync" @@ -36,6 +35,12 @@ const ( NoProcessSandbox ) +var tracingEnvVars = []string{ + "OTEL_TRACES_EXPORTER=otlp", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=" + getTracingSocket(), + "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=grpc", +} + func (pm ProcessMode) String() string { switch pm { case ProcessSandbox: @@ -184,20 +189,7 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou } if tracingSocket != "" { - if runtime.GOOS == "windows" { - s.Mounts = append(s.Mounts, specs.Mount{ - Destination: `\\.\pipe\otel-grpc`, - Source: tracingSocket, - Options: []string{"ro"}, - }) - } else { - s.Mounts = append(s.Mounts, specs.Mount{ - Destination: "/dev/otel-grpc.sock", - Type: "bind", - Source: tracingSocket, - Options: []string{"ro", "rbind"}, - }) - } + s.Mounts = append(s.Mounts, getTracingSocketMount(tracingSocket)) } s.Mounts = dedupMounts(s.Mounts) diff --git a/executor/oci/spec_unix.go b/executor/oci/spec_unix.go index 3933764c8502..3c809e7ff9f3 100644 --- a/executor/oci/spec_unix.go +++ b/executor/oci/spec_unix.go @@ -21,11 +21,9 @@ import ( "github.com/pkg/errors" ) -var tracingEnvVars = []string{ - "OTEL_TRACES_EXPORTER=otlp", - "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=unix:///dev/otel-grpc.sock", - "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=grpc", -} +const ( + tracingSocketPath = "/dev/otel-grpc.sock" +) func generateMountOpts(resolvConf, hostsFile string) ([]oci.SpecOpts, error) { return []oci.SpecOpts{ @@ -128,3 +126,16 @@ func withDefaultProfile() oci.SpecOpts { return err } } + +func getTracingSocketMount(socket string) specs.Mount { + return specs.Mount{ + Destination: tracingSocketPath, + Type: "bind", + Source: socket, + Options: []string{"ro", "rbind"}, + } +} + +func getTracingSocket() string { + return fmt.Sprintf("unix://%s", tracingSocketPath) +} diff --git a/executor/oci/spec_windows.go b/executor/oci/spec_windows.go index 69827931cf47..faa9baafa1d0 100644 --- a/executor/oci/spec_windows.go +++ b/executor/oci/spec_windows.go @@ -4,17 +4,19 @@ package oci import ( + "fmt" + "path/filepath" + "github.com/containerd/containerd/oci" "github.com/docker/docker/pkg/idtools" "github.com/moby/buildkit/solver/pb" + specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" ) -var tracingEnvVars = []string{ - "OTEL_TRACES_EXPORTER=otlp", - "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=npipe:////./pipe/otel-grpc", - "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=grpc", -} +const ( + tracingSocketPath = "//./pipe/otel-grpc" +) func generateMountOpts(resolvConf, hostsFile string) ([]oci.SpecOpts, error) { return nil, nil @@ -49,3 +51,15 @@ func generateRlimitOpts(ulimits []*pb.Ulimit) ([]oci.SpecOpts, error) { } return nil, errors.New("no support for POSIXRlimit on Windows") } + +func getTracingSocketMount(socket string) specs.Mount { + return specs.Mount{ + Destination: filepath.FromSlash(tracingSocketPath), + Source: socket, + Options: []string{"ro"}, + } +} + +func getTracingSocket() string { + return fmt.Sprintf("npipe://%s", filepath.ToSlash(tracingSocketPath)) +} From dc8d71edf544886f04c955e0cce56cb8f1dbbf9e Mon Sep 17 00:00:00 2001 From: Gabriel Date: Thu, 23 Feb 2023 13:51:15 +0200 Subject: [PATCH 3/3] Update cmd/buildkitd/main_windows.go Co-authored-by: Akihiro Suda Signed-off-by: Gabriel --- cmd/buildkitd/main_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/buildkitd/main_windows.go b/cmd/buildkitd/main_windows.go index e49a4d5c7749..7dccc16383c0 100644 --- a/cmd/buildkitd/main_windows.go +++ b/cmd/buildkitd/main_windows.go @@ -13,7 +13,7 @@ import ( ) const ( - defaultTraceSocketPath = `\\.\pipe\otel-grpc` + defaultTraceSocketPath = `\\.\pipe\buildkit-otel-grpc` ) func listenFD(addr string, tlsConfig *tls.Config) (net.Listener, error) {