From e596761c8e4e3c66c115a8a7f123ef4baa2bb5f7 Mon Sep 17 00:00:00 2001 From: Thomas Hallgren Date: Wed, 29 Mar 2023 17:12:26 +0200 Subject: [PATCH] Don't use named pipes in windows. Microsoft's Go implementation of named pipes sometimes causes the Telepresence daemons to hang indefinitely. This is the culprit: https://github.com/microsoft/go-winio/issues/281 This commit removes the use of named pipes (once implemented due to buggy Go implementation of unix sockets), in favor of using the same socket implementation (almost) as we do on other platforms. Signed-off-by: Thomas Hallgren --- integration_test/itest/cluster.go | 2 +- pkg/client/cli/cmd/quit.go | 2 +- pkg/client/cli/cmd/version.go | 2 +- pkg/client/cli/connect/connector.go | 8 +- pkg/client/cli/connect/daemon.go | 8 +- pkg/client/rootd/service.go | 2 +- pkg/client/rootd/session.go | 2 +- pkg/client/socket/sockets.go | 28 ++- .../{sockets_unix_test.go => sockets_test.go} | 15 +- pkg/client/socket/sockets_unix.go | 38 +--- pkg/client/socket/sockets_windows.go | 194 +++++++++++------- pkg/client/userd/daemon/grpc.go | 2 +- pkg/client/userd/daemon/service.go | 31 +-- pkg/client/userd/trafficmgr/session.go | 4 +- pkg/client/userd/trafficmgr/tracing.go | 4 +- 15 files changed, 196 insertions(+), 146 deletions(-) rename pkg/client/socket/{sockets_unix_test.go => sockets_test.go} (89%) diff --git a/integration_test/itest/cluster.go b/integration_test/itest/cluster.go index 083f2a24bb..45433fcab1 100644 --- a/integration_test/itest/cluster.go +++ b/integration_test/itest/cluster.go @@ -186,7 +186,7 @@ func (s *cluster) ensureQuit(ctx context.Context) { _, _, _ = Telepresence(ctx, "quit", "-s") //nolint:dogsled // don't care about any of the returns // Ensure that the daemon-socket is non-existent. - _ = rmAsRoot(socket.DaemonName) + _ = rmAsRoot(socket.RootDaemonPath(ctx)) } func (s *cluster) ensureExecutable(ctx context.Context, errs chan<- error, wg *sync.WaitGroup) { diff --git a/pkg/client/cli/cmd/quit.go b/pkg/client/cli/cmd/quit.go index dc76da59ff..f5b5f7df3e 100644 --- a/pkg/client/cli/cmd/quit.go +++ b/pkg/client/cli/cmd/quit.go @@ -40,7 +40,7 @@ func quit() *cobra.Command { if quitDaemons && daemon.GetUserClient(ctx) == nil { // User daemon isn't running. If the root daemon is running, we must // kill it from here. - if conn, err := socket.Dial(ctx, socket.DaemonName); err == nil { + if conn, err := socket.Dial(ctx, socket.RootDaemonPath(ctx)); err == nil { _, _ = daemon2.NewDaemonClient(conn).Quit(ctx, &emptypb.Empty{}) } } diff --git a/pkg/client/cli/cmd/version.go b/pkg/client/cli/cmd/version.go index 56e57eceaf..d34a58a622 100644 --- a/pkg/client/cli/cmd/version.go +++ b/pkg/client/cli/cmd/version.go @@ -86,7 +86,7 @@ func printVersion(cmd *cobra.Command, _ []string) error { } func daemonVersion(ctx context.Context) (*common.VersionInfo, error) { - if conn, err := socket.Dial(ctx, socket.DaemonName); err == nil { + if conn, err := socket.Dial(ctx, socket.RootDaemonPath(ctx)); err == nil { defer conn.Close() return daemonRpc.NewDaemonClient(conn).Version(ctx, &empty.Empty{}) } diff --git a/pkg/client/cli/connect/connector.go b/pkg/client/cli/connect/connector.go index 8168d2f3de..627299053c 100644 --- a/pkg/client/cli/connect/connector.go +++ b/pkg/client/cli/connect/connector.go @@ -57,7 +57,7 @@ func UserDaemonDisconnect(ctx context.Context, quitDaemons bool) (err error) { // Disconnect is not implemented so daemon predates 2.4.9. Force a quit } if _, err = ud.Quit(ctx, &emptypb.Empty{}); err == nil || status.Code(err) == codes.Unavailable { - err = socket.WaitUntilVanishes("user daemon", socket.ConnectorName, 5*time.Second) + err = socket.WaitUntilVanishes("user daemon", socket.UserDaemonPath(ctx), 5*time.Second) } if err != nil && status.Code(err) == codes.Unavailable { if quitDaemons { @@ -88,7 +88,7 @@ func RunConnect(cmd *cobra.Command, args []string) error { func launchConnectorDaemon(ctx context.Context, connectorDaemon string, required bool) (*daemon.UserClient, error) { cr := daemon.GetRequest(ctx) - conn, err := socket.Dial(ctx, socket.ConnectorName) + conn, err := socket.Dial(ctx, socket.UserDaemonPath(ctx)) if err == nil { if cr.Docker { return nil, errcat.User.New("option --docker cannot be used as long as a daemon is running on the host. Try telepresence quit -s") @@ -137,10 +137,10 @@ func launchConnectorDaemon(ctx context.Context, connectorDaemon string, required if err = proc.StartInBackground(false, args...); err != nil { return nil, errcat.NoDaemonLogs.Newf("failed to launch the connector service: %w", err) } - if err = socket.WaitUntilAppears("connector", socket.ConnectorName, 10*time.Second); err != nil { + if err = socket.WaitUntilAppears("connector", socket.UserDaemonPath(ctx), 10*time.Second); err != nil { return nil, errcat.NoDaemonLogs.Newf("connector service did not start: %w", err) } - conn, err = socket.Dial(ctx, socket.ConnectorName) + conn, err = socket.Dial(ctx, socket.UserDaemonPath(ctx)) if err != nil { return nil, err } diff --git a/pkg/client/cli/connect/daemon.go b/pkg/client/cli/connect/daemon.go index 59cca645e1..ae9d2ccb15 100644 --- a/pkg/client/cli/connect/daemon.go +++ b/pkg/client/cli/connect/daemon.go @@ -70,14 +70,14 @@ func ensureRootDaemonRunning(ctx context.Context) error { // Always assume that root daemon is running when a user daemon address is provided return nil } - running, err := socket.IsRunning(ctx, socket.DaemonName) + running, err := socket.IsRunning(ctx, socket.RootDaemonPath(ctx)) if err != nil || running { return err } if err = launchDaemon(ctx, cr); err != nil { return fmt.Errorf("failed to launch the daemon service: %w", err) } - if err = socket.WaitUntilRunning(ctx, "daemon", socket.DaemonName, 10*time.Second); err != nil { + if err = socket.WaitUntilRunning(ctx, "daemon", socket.RootDaemonPath(ctx), 10*time.Second); err != nil { return fmt.Errorf("daemon service did not start: %w", err) } return nil @@ -95,9 +95,9 @@ func Disconnect(ctx context.Context, quitDaemons bool) error { if quitDaemons { // User daemon is responsible for killing the root daemon, but we kill it here too to cater for // the fact that the user daemon might have been killed ungracefully. - if err = socket.WaitUntilVanishes("root daemon", socket.DaemonName, 5*time.Second); err != nil { + if err = socket.WaitUntilVanishes("root daemon", socket.RootDaemonPath(ctx), 5*time.Second); err != nil { var conn *grpc.ClientConn - if conn, err = socket.Dial(ctx, socket.DaemonName); err == nil { + if conn, err = socket.Dial(ctx, socket.RootDaemonPath(ctx)); err == nil { if _, err = rpc.NewDaemonClient(conn).Quit(ctx, &empty.Empty{}); err != nil { err = fmt.Errorf("error when quitting root daemon: %w", err) } diff --git a/pkg/client/rootd/service.go b/pkg/client/rootd/service.go index 5f9521a656..7300a343cc 100644 --- a/pkg/client/rootd/service.go +++ b/pkg/client/rootd/service.go @@ -429,7 +429,7 @@ func run(cmd *cobra.Command, args []string) error { // Listen on domain unix domain socket or windows named pipe. The listener must be opened // before other tasks because the CLI client will only wait for a short period of time for // the socket/pipe to appear before it gives up. - grpcListener, err := socket.Listen(c, ProcessName, socket.DaemonName) + grpcListener, err := socket.Listen(c, ProcessName, socket.RootDaemonPath(c)) if err != nil { return err } diff --git a/pkg/client/rootd/session.go b/pkg/client/rootd/session.go index 876f65170f..f2398d11da 100644 --- a/pkg/client/rootd/session.go +++ b/pkg/client/rootd/session.go @@ -174,7 +174,7 @@ func connectToUserDaemon(c context.Context) (*grpc.ClientConn, connector.Manager defer cancel() var conn *grpc.ClientConn - conn, err := socket.Dial(tc, socket.ConnectorName, + conn, err := socket.Dial(tc, socket.UserDaemonPath(c), grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor()), grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor()), ) diff --git a/pkg/client/socket/sockets.go b/pkg/client/socket/sockets.go index 8552c2abb0..886a8f76c7 100644 --- a/pkg/client/socket/sockets.go +++ b/pkg/client/socket/sockets.go @@ -11,6 +11,16 @@ import ( "google.golang.org/grpc" ) +// UserDaemonPath is the path used when communicating to the user daemon process. +func UserDaemonPath(ctx context.Context) string { + return userDaemonPath(ctx) +} + +// RootDaemonPath is the path used when communicating to the root daemon process. +func RootDaemonPath(ctx context.Context) string { + return rootDaemonPath(ctx) +} + // Dial dials the given socket and returns the resulting connection. func Dial(ctx context.Context, socketName string, opts ...grpc.DialOption) (*grpc.ClientConn, error) { return dial(ctx, socketName, opts...) @@ -18,15 +28,25 @@ func Dial(ctx context.Context, socketName string, opts ...grpc.DialOption) (*grp // Listen returns a listener for the given socket and returns the resulting connection. func Listen(ctx context.Context, processName, socketName string) (net.Listener, error) { - return listen(ctx, processName, socketName) + listener, err := net.Listen("unix", socketName) + if err != nil { + if err != nil { + err = fmt.Errorf("socket %q exists so the %s is either already running or terminated ungracefully: %T, %w", socketName, processName, err, err) + } + return nil, err + } + // Don't have dhttp.ServerConfig.Serve unlink the socket; defer unlinking the socket + // until the process exits. + listener.(*net.UnixListener).SetUnlinkOnClose(false) + return listener, nil } -// RemoveSocket removes any representation of the socket from the filesystem. +// Remove removes any representation of the socket from the filesystem. func Remove(listener net.Listener) error { - return remove(listener) + return os.Remove(listener.Addr().String()) } -// SocketExists returns true if a socket is found with the given name. +// Exists returns true if a socket is found with the given name. func Exists(name string) (bool, error) { return exists(name) } diff --git a/pkg/client/socket/sockets_unix_test.go b/pkg/client/socket/sockets_test.go similarity index 89% rename from pkg/client/socket/sockets_unix_test.go rename to pkg/client/socket/sockets_test.go index 4a20e3765d..18cdf0e5bd 100644 --- a/pkg/client/socket/sockets_unix_test.go +++ b/pkg/client/socket/sockets_test.go @@ -1,6 +1,3 @@ -//go:build !windows -// +build !windows - package socket_test import ( @@ -11,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "google.golang.org/grpc" "github.com/datawire/dlib/dgroup" @@ -20,9 +18,8 @@ import ( ) func TestDialSocket(t *testing.T) { - tmpdir := t.TempDir() t.Run("OK", func(t *testing.T) { - sockname := filepath.Join(tmpdir, "ok.sock") + sockname := filepath.Join(t.TempDir(), "ok.sock") listener, err := net.Listen("unix", sockname) if !assert.NoError(t, err) { return @@ -55,7 +52,7 @@ func TestDialSocket(t *testing.T) { assert.NoError(t, grp.Wait()) }) t.Run("Hang", func(t *testing.T) { - sockname := filepath.Join(tmpdir, "hang.sock") + sockname := filepath.Join(t.TempDir(), "hang.sock") listener, err := net.Listen("unix", sockname) if !assert.NoError(t, err) { return @@ -72,7 +69,7 @@ func TestDialSocket(t *testing.T) { assert.Contains(t, err.Error(), "this usually means that the process has locked up") }) t.Run("Orphan", func(t *testing.T) { - sockname := filepath.Join(tmpdir, "orphan.sock") + sockname := filepath.Join(t.TempDir(), "orphan.sock") listener, err := net.Listen("unix", sockname) if !assert.NoError(t, err) { return @@ -83,7 +80,7 @@ func TestDialSocket(t *testing.T) { ctx := dlog.NewTestContext(t, false) conn, err := socket.Dial(ctx, sockname) assert.Nil(t, conn) - assert.Error(t, err) + require.Error(t, err) t.Log(err) assert.ErrorIs(t, err, os.ErrNotExist) assert.Contains(t, err.Error(), "dial unix "+sockname) @@ -91,7 +88,7 @@ func TestDialSocket(t *testing.T) { }) t.Run("NotExist", func(t *testing.T) { ctx := dlog.NewTestContext(t, false) - sockname := filepath.Join(tmpdir, "not-exist.sock") + sockname := filepath.Join(t.TempDir(), "not-exist.sock") conn, err := socket.Dial(ctx, sockname) assert.Nil(t, conn) assert.Error(t, err) diff --git a/pkg/client/socket/sockets_unix.go b/pkg/client/socket/sockets_unix.go index a9b85a673d..5f98b99e49 100644 --- a/pkg/client/socket/sockets_unix.go +++ b/pkg/client/socket/sockets_unix.go @@ -14,17 +14,17 @@ import ( "golang.org/x/sys/unix" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" - - "github.com/telepresenceio/telepresence/v2/pkg/proc" ) -const ( - // ConnectorName is the path used when communicating to the connector process. - ConnectorName = "/tmp/telepresence-connector.socket" +// userDaemonPath is the path used when communicating to the user daemon process. +func userDaemonUnixSocket(ctx context.Context) string { + return "/tmp/telepresence-connector.socket" +} - // DaemonName is the path used when communicating to the daemon process. - DaemonName = "/var/run/telepresence-daemon.socket" -) +// rootDaemonPath is the path used when communicating to the root daemon process. +func rootDaemonUnixSocket(ctx context.Context) string { + return "/var/run/telepresence-daemon.socket" +} func dial(ctx context.Context, socketName string, opts ...grpc.DialOption) (*grpc.ClientConn, error) { ctx, cancel := context.WithTimeout(ctx, 5*time.Second) // FIXME(lukeshu): Make this configurable @@ -77,28 +77,6 @@ func dial(ctx context.Context, socketName string, opts ...grpc.DialOption) (*grp } } -func listen(_ context.Context, processName, socketName string) (net.Listener, error) { - if proc.IsAdmin() { - origUmask := unix.Umask(0) - defer unix.Umask(origUmask) - } - listener, err := net.Listen("unix", socketName) - if err != nil { - if errors.Is(err, unix.EADDRINUSE) { - err = fmt.Errorf("socket %q exists so the %s is either already running or terminated ungracefully", socketName, processName) - } - return nil, err - } - // Don't have dhttp.ServerConfig.Serve unlink the socket; defer unlinking the socket - // until the process exits. - listener.(*net.UnixListener).SetUnlinkOnClose(false) - return listener, nil -} - -func remove(listener net.Listener) error { - return os.Remove(listener.Addr().String()) -} - // exists returns true if a socket is found at the given path. func exists(path string) (bool, error) { s, err := os.Stat(path) diff --git a/pkg/client/socket/sockets_windows.go b/pkg/client/socket/sockets_windows.go index d5cabc1a8e..f6872d9873 100644 --- a/pkg/client/socket/sockets_windows.go +++ b/pkg/client/socket/sockets_windows.go @@ -2,98 +2,150 @@ package socket import ( "context" + "errors" "fmt" + "io/fs" "net" + "os" + "path/filepath" + "time" + "unsafe" - "github.com/Microsoft/go-winio" "golang.org/x/sys/windows" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" - "github.com/telepresenceio/telepresence/v2/pkg/proc" + "github.com/datawire/dlib/dlog" + "github.com/telepresenceio/telepresence/v2/pkg/filelocation" ) -// The Windows IPC between the CLI and the user and root daemons is based on named pipes rather than -// unix sockets. -// See https://docs.microsoft.com/en-us/windows/win32/ipc/pipe-names for more info -// about pipe names. -const ( - // ConnectorName is the name used when communicating to the connector process. - ConnectorName = `\\.\pipe\telepresence-connector` - - // DaemonName is the name used when communicating to the daemon process. - DaemonName = `\\.\pipe\telepresence-daemon` -) +// userDaemonPath is the path used when communicating to the user daemon process. +func userDaemonPath(ctx context.Context) string { + return filepath.Join(filelocation.AppUserCacheDir(ctx), "userd.socket") +} -// dial dials the given named pipe and returns the resulting connection. -func dial(c context.Context, socketName string, opts ...grpc.DialOption) (*grpc.ClientConn, error) { - conn, err := grpc.DialContext(c, socketName, append([]grpc.DialOption{ - grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithNoProxy(), - grpc.WithBlock(), - grpc.FailOnNonTempDialError(true), - grpc.WithContextDialer(func(c context.Context, s string) (net.Conn, error) { - conn, err := winio.DialPipeContext(c, socketName) - return conn, err - }), - }, opts...)...) - // The google.golang.org/grpc/internal/transport.ConnectionError does not have an - // Unwrap method. It does have a Origin method though. - // See: https://github.com/grpc/grpc-go/pull/5148 - if oe, ok := err.(interface{ Origin() error }); ok { - err = oe.Origin() - } - return conn, err +// rootDaemonPath is the path used when communicating to the root daemon process. +func rootDaemonPath(ctx context.Context) string { + return filepath.Join(filelocation.AppUserCacheDir(ctx), "rootd.socket") } -// allowEveryone is a security descriptor that allows everyone to perform the action. -// For more info about the syntax, sse: -// https://docs.microsoft.com/en-us/windows/win32/secauthz/security-descriptor-string-format -const allowEveryone = "S:(ML;;NW;;;LW)D:(A;;0x12019f;;;WD)" +func dial(ctx context.Context, socketName string, opts ...grpc.DialOption) (*grpc.ClientConn, error) { + ctx, cancel := context.WithTimeout(ctx, 3*time.Second) // FIXME(lukeshu): Make this configurable + defer cancel() + for firstTry := true; ; firstTry = false { + // Windows will give us a WSAECONNREFUSED if the socket does not exist. That's not + // what we want. + found, err := exists(socketName) + if err != nil { + return nil, err + } + if !found { + err = &net.OpError{ + Op: "dial", + Net: "unix", + Addr: &net.UnixAddr{ + Name: socketName, + Net: "unix", + }, + Err: fs.ErrNotExist, + } + } + if err == nil { + conn, dialErr := grpc.DialContext(ctx, "unix:"+socketName, append([]grpc.DialOption{ + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithNoProxy(), + grpc.WithBlock(), + grpc.FailOnNonTempDialError(true), + }, opts...)...) + if dialErr == nil { + return conn, nil + } + err = dialErr + } + + // Remove the gRPC internal transport.Connection error wrapper. It messes up the message by + // quoting it so that backslashes in the path get doubled. + var opErr *net.OpError + if errors.As(err, &opErr) { + err = opErr + } + + // Windows will give us a WSAECONNREFUSED if the socket does not exist. That's not + // what we want. + if errors.Is(err, windows.WSAECONNREFUSED) { + found, exErr := exists(socketName) + if exErr != nil { + return nil, exErr + } + if !found { + err = &net.OpError{ + Op: "dial", + Net: "unix", + Addr: &net.UnixAddr{ + Name: socketName, + Net: "unix", + }, + Err: fs.ErrNotExist, + } + } + } -// listen returns a listener for the given named pipe and returns the resulting connection. -func listen(_ context.Context, processName, socketName string) (net.Listener, error) { - var config *winio.PipeConfig - if proc.IsAdmin() { - config = &winio.PipeConfig{SecurityDescriptor: allowEveryone} + if firstTry && errors.Is(err, windows.WSAECONNREFUSED) { + // Socket exists but doesn't accept connections. This usually means that the process + // terminated ungracefully. To remedy this, we make an attempt to remove the socket + // and dial again. + dlog.Errorf(ctx, "Dial unix:%s failed: %v", socketName, err) + if rmErr := os.Remove(socketName); rmErr != nil && !errors.Is(err, os.ErrNotExist) { + err = fmt.Errorf("%w (socket rm failed with %v)", err, rmErr) + } else { + continue + } + } + + if err == context.DeadlineExceeded { + // grpc.DialContext doesn't wrap context.DeadlineExceeded with any useful + // information at all. Fix that. + err = &net.OpError{ + Op: "dial", + Net: "unix", + Addr: &net.UnixAddr{ + Name: socketName, + Net: "unix", + }, + Err: fmt.Errorf("socket exists but is not responding: %w", err), + } + } + + // Add some Telepresence-specific commentary on what specific common errors mean. + switch { + case errors.Is(err, context.DeadlineExceeded): + err = fmt.Errorf("%w; this usually means that the process has locked up", err) + case errors.Is(err, windows.WSAECONNREFUSED): + err = fmt.Errorf("%w; this usually means that the process has terminated ungracefully", err) + case errors.Is(err, os.ErrNotExist): + err = fmt.Errorf("%w; this usually means that the process is not running", err) + } + return nil, err } - return winio.ListenPipe(socketName, config) } -// remove does nothing because a named pipe has no representation in the file system that -// needs to be removed. -func remove(listener net.Listener) error { - return nil -} +// socketAttributes is the combination that Windows uses for Unix socket FileAttributes. +const socketAttributes = windows.FILE_ATTRIBUTE_REPARSE_POINT | windows.FILE_ATTRIBUTE_ARCHIVE -// exists returns true if a socket exists with the given name. -func exists(name string) (bool, error) { - uPath, err := windows.UTF16PtrFromString(name) +// exists returns true if a socket is found at the given path. +func exists(path string) (bool, error) { + namep, err := windows.UTF16PtrFromString(path) if err != nil { return false, err } - // Despite the name of the function, this is actually an attempt to open an existing socket. The - // OPEN_EXISTING disposition will make it fail unless it exists. - h, err := windows.CreateFile(uPath, windows.GENERIC_READ|windows.GENERIC_WRITE, 0, nil, windows.OPEN_EXISTING, windows.FILE_FLAG_OVERLAPPED, 0) - switch err { - case windows.ERROR_PIPE_BUSY: - // ERROR_PIPE_BUSY is an error that is issued somewhat sporadically, but it's a safe - // indication that the pipe exists. - return true, nil - case windows.ERROR_FILE_NOT_FOUND: + var fa windows.Win32FileAttributeData + err = windows.GetFileAttributesEx(namep, windows.GetFileExInfoStandard, (*byte)(unsafe.Pointer(&fa))) + if err != nil { return false, nil - case nil: - var ft uint32 - ft, err = windows.GetFileType(h) - if err != nil { - break - } - _ = windows.CloseHandle(h) - if ft|windows.FILE_TYPE_PIPE != 0 { - return true, nil - } - err = fmt.Errorf("%q is not a named pipe", name) } - return false, err + if fa.FileAttributes&socketAttributes != socketAttributes { + return false, fmt.Errorf("%q is not a socket", path) + } + return true, nil } diff --git a/pkg/client/userd/daemon/grpc.go b/pkg/client/userd/daemon/grpc.go index f51eba3e92..39fac3385d 100644 --- a/pkg/client/userd/daemon/grpc.go +++ b/pkg/client/userd/daemon/grpc.go @@ -641,7 +641,7 @@ func (s *Service) withRootDaemon(ctx context.Context, f func(ctx context.Context if s.rootSessionInProc { return status.Error(codes.Unavailable, "root daemon is embedded") } - conn, err := socket.Dial(ctx, socket.DaemonName) + conn, err := socket.Dial(ctx, socket.RootDaemonPath(ctx)) if err == nil { defer conn.Close() err = f(ctx, daemon.NewDaemonClient(conn)) diff --git a/pkg/client/userd/daemon/service.go b/pkg/client/userd/daemon/service.go index af1eed0bc0..54df5420b2 100644 --- a/pkg/client/userd/daemon/service.go +++ b/pkg/client/userd/daemon/service.go @@ -164,7 +164,7 @@ func Command() *cobra.Command { } flags := c.Flags() flags.String(nameFlag, userd.ProcessName, "Daemon name") - flags.String(addressFlag, "", "Address to listen to. Defaults to "+socket.ConnectorName) + flags.String(addressFlag, "", "Address to listen to. Defaults to "+socket.UserDaemonPath(context.Background())) flags.Bool(embedNetworkFlag, false, "Embed network functionality in the user daemon. Requires capability NET_ADMIN") flags.Uint16(pprofFlag, 0, "start pprof server on the given port") return c @@ -369,6 +369,18 @@ func run(cmd *cobra.Command, _ []string) error { } }() } + + name, _ := flags.GetString(nameFlag) + sessionName := "session" + if di := strings.IndexByte(name, '-'); di > 0 { + sessionName = name[di+1:] + name = name[:di] + } + c = dgroup.WithGoroutineName(c, "/"+name) + c, err = logging.InitContext(c, userd.ProcessName, logging.RotateDaily, true) + if err != nil { + return err + } rootSessionInProc, _ := flags.GetBool(embedNetworkFlag) var daemonAddress *net.TCPAddr if addr, _ := flags.GetString(addressFlag); addr != "" { @@ -381,25 +393,16 @@ func run(cmd *cobra.Command, _ []string) error { _ = grpcListener.Close() }() } else { - if grpcListener, err = socket.Listen(c, userd.ProcessName, socket.ConnectorName); err != nil { + socketPath := socket.UserDaemonPath(c) + dlog.Infof(c, "Starting socket listener for %s", socketPath) + if grpcListener, err = socket.Listen(c, userd.ProcessName, socketPath); err != nil { + dlog.Errorf(c, "socket listener for %s failed: %v", socketPath, err) return err } defer func() { _ = socket.Remove(grpcListener) }() } - - name, _ := flags.GetString(nameFlag) - sessionName := "session" - if di := strings.IndexByte(name, '-'); di > 0 { - sessionName = name[di+1:] - name = name[:di] - } - c = dgroup.WithGoroutineName(c, "/"+name) - c, err = logging.InitContext(c, userd.ProcessName, logging.RotateDaily, true) - if err != nil { - return err - } dlog.Debugf(c, "Listener opened on %s", grpcListener.Addr()) dlog.Info(c, "---") diff --git a/pkg/client/userd/trafficmgr/session.go b/pkg/client/userd/trafficmgr/session.go index 8be37af706..f5be736bd3 100644 --- a/pkg/client/userd/trafficmgr/session.go +++ b/pkg/client/userd/trafficmgr/session.go @@ -202,7 +202,7 @@ func NewSession( rdRunning := userd.GetService(ctx).RootSessionInProcess() if !rdRunning { // Connect to the root daemon if it is running. It's the CLI that starts it initially - rdRunning, err = socket.IsRunning(ctx, socket.DaemonName) + rdRunning, err = socket.IsRunning(ctx, socket.RootDaemonPath(ctx)) if err != nil { return ctx, nil, connectError(rpc.ConnectInfo_DAEMON_FAILED, err) } @@ -1183,7 +1183,7 @@ func (s *session) connectRootDaemon(ctx context.Context, oi *rootdRpc.OutboundIn rd = rootSession } else { var conn *grpc.ClientConn - conn, err = socket.Dial(ctx, socket.DaemonName, + conn, err = socket.Dial(ctx, socket.RootDaemonPath(ctx), grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor()), grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor()), ) diff --git a/pkg/client/userd/trafficmgr/tracing.go b/pkg/client/userd/trafficmgr/tracing.go index fc565c947b..df5dcaa9c9 100644 --- a/pkg/client/userd/trafficmgr/tracing.go +++ b/pkg/client/userd/trafficmgr/tracing.go @@ -109,7 +109,7 @@ func (*traceCollector) launchTraceWriter(ctx context.Context, destFile string) ( } func (c *traceCollector) userdTraces(ctx context.Context, tCh chan<- []byte) error { - userdConn, err := socket.Dial(ctx, socket.ConnectorName, grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor())) + userdConn, err := socket.Dial(ctx, socket.UserDaemonPath(ctx), grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor())) if err != nil { return err } @@ -119,7 +119,7 @@ func (c *traceCollector) userdTraces(ctx context.Context, tCh chan<- []byte) err } func (c *traceCollector) rootdTraces(ctx context.Context, tCh chan<- []byte) error { - dConn, err := socket.Dial(ctx, socket.DaemonName, grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor())) + dConn, err := socket.Dial(ctx, socket.RootDaemonPath(ctx), grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor())) if err != nil { return err }