From 155e39187c720ca2f592e1d31705e437c0fa0dc8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 31 Dec 2022 13:59:06 +0100 Subject: [PATCH] daemon/logger/gcplogs: remove ensureHomeIfIAmStatic workaround This function was added in b86e3bee5aea8e72b7f08e104ebb5d6cb18f8890 to work around an issue in os/user.Current(), which SEGFAULTS when compiling statically with cgo enabled (see golang/go#13470). We hit similar issues in other parts, and contributed a "osusergo" build- tag in https://go-review.googlesource.com/c/go/+/330753. The "osusergo" build tag must be set when compiling static binaries with cgo enabled. If that build-tag is set, the cgo implementation for user.Current() won't be used, and a pure-go implementation is used instead; https://github.com/golang/go/blob/go1.19.4/src/os/user/cgo_lookup_unix.go#L5 With the above in place, we no longer need this workaround, and can remove the ensureHomeIfIAmStatic() function. Signed-off-by: Sebastiaan van Stijn --- daemon/logger/gcplogs/gcplogging.go | 9 -------- daemon/logger/gcplogs/gcplogging_linux.go | 27 ---------------------- daemon/logger/gcplogs/gcplogging_others.go | 8 ------- 3 files changed, 44 deletions(-) delete mode 100644 daemon/logger/gcplogs/gcplogging_linux.go delete mode 100644 daemon/logger/gcplogs/gcplogging_others.go diff --git a/daemon/logger/gcplogs/gcplogging.go b/daemon/logger/gcplogs/gcplogging.go index 94ef107510..92178f189a 100644 --- a/daemon/logger/gcplogs/gcplogging.go +++ b/daemon/logger/gcplogs/gcplogging.go @@ -117,15 +117,6 @@ func New(info logger.Info) (logger.Logger, error) { return nil, fmt.Errorf("No project was specified and couldn't read project from the metadata server. Please specify a project") } - // Issue #29344: gcplogs segfaults (static binary) - // If HOME is not set, logging.NewClient() will call os/user.Current() via oauth2/google. - // However, in static binary, os/user.Current() leads to segfault due to a glibc issue that won't be fixed - // in a short term. (golang/go#13470, https://sourceware.org/bugzilla/show_bug.cgi?id=19341) - // So we forcibly set HOME so as to avoid call to os/user/Current() - if err := ensureHomeIfIAmStatic(); err != nil { - return nil, err - } - c, err := logging.NewClient(context.Background(), project) if err != nil { return nil, err diff --git a/daemon/logger/gcplogs/gcplogging_linux.go b/daemon/logger/gcplogs/gcplogging_linux.go deleted file mode 100644 index 1c2160c41c..0000000000 --- a/daemon/logger/gcplogs/gcplogging_linux.go +++ /dev/null @@ -1,27 +0,0 @@ -package gcplogs // import "github.com/docker/docker/daemon/logger/gcplogs" - -import ( - "os" - - "github.com/docker/docker/dockerversion" - "github.com/docker/docker/pkg/homedir" - "github.com/sirupsen/logrus" -) - -// ensureHomeIfIAmStatic ensure $HOME to be set if dockerversion.IAmStatic is "true". -// See issue #29344: gcplogs segfaults (static binary) -// If HOME is not set, logging.NewClient() will call os/user.Current() via oauth2/google. -// If compiling statically, make sure osusergo build tag is also used to prevent a segfault -// due to a glibc issue that won't be fixed in a short term -// (see golang/go#13470, https://sourceware.org/bugzilla/show_bug.cgi?id=19341). -// So we forcibly set HOME so as to avoid call to os/user/Current() -func ensureHomeIfIAmStatic() error { - // Note: dockerversion.IAmStatic is only available for linux. - // So we need to use them in this gcplogging_linux.go rather than in gcplogging.go - if dockerversion.IAmStatic == "true" && os.Getenv("HOME") == "" { - home := homedir.Get() - logrus.Warnf("gcplogs requires HOME to be set for static daemon binary. Forcibly setting HOME to %s.", home) - os.Setenv("HOME", home) - } - return nil -} diff --git a/daemon/logger/gcplogs/gcplogging_others.go b/daemon/logger/gcplogs/gcplogging_others.go deleted file mode 100644 index 55f43b0c0c..0000000000 --- a/daemon/logger/gcplogs/gcplogging_others.go +++ /dev/null @@ -1,8 +0,0 @@ -//go:build !linux -// +build !linux - -package gcplogs // import "github.com/docker/docker/daemon/logger/gcplogs" - -func ensureHomeIfIAmStatic() error { - return nil -}