From 1036fafffa2a240456d0a39c75bad215892520ff Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Mon, 6 Jan 2020 00:39:30 +1100 Subject: [PATCH 1/6] Support npipe the same way we support Unix sockets The same function used to support Unix sockets automatically supports Named Pipes on Windows. This makes the default configuration option for the daemon address work correctly on Windows. Signed-off-by: Paul "TBBle" Hampson --- cmd/buildkitd/main.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/buildkitd/main.go b/cmd/buildkitd/main.go index 6a19eed36d07..0eb1a20a660d 100644 --- a/cmd/buildkitd/main.go +++ b/cmd/buildkitd/main.go @@ -495,7 +495,7 @@ func getListener(addr string, uid, gid int, tlsConfig *tls.Config) (net.Listener proto := addrSlice[0] listenAddr := addrSlice[1] switch proto { - case "unix": + case "unix", "npipe": if tlsConfig != nil { logrus.Warnf("TLS is disabled for %s", addr) } @@ -506,7 +506,6 @@ func getListener(addr string, uid, gid int, tlsConfig *tls.Config) (net.Listener } return sockets.NewTCPSocket(listenAddr, tlsConfig) default: - // TODO: support npipe (with TLS?) return nil, errors.Errorf("addr %s not supported", addr) } } From c67499de09b13a46a47ab1a739ae73455d9ea1f1 Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Mon, 6 Jan 2020 00:42:14 +1100 Subject: [PATCH 2/6] Create a new Error when there is no Error to wrap Wrapping a `nil` error produces `nil`, which causes the calling code to see success, and continue on with a default-created WorkerOpt, which causes segfaults later. Signed-off-by: Paul "TBBle" Hampson --- worker/containerd/containerd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/worker/containerd/containerd.go b/worker/containerd/containerd.go index 7d28982432a4..22c6dffcb5a2 100644 --- a/worker/containerd/containerd.go +++ b/worker/containerd/containerd.go @@ -74,7 +74,7 @@ func newContainerd(root string, client *containerd.Client, snapshotterName, ns s return base.WorkerOpt{}, errors.Wrap(err, "failed to list runtime plugin") } if len(resp.Plugins) == 0 { - return base.WorkerOpt{}, errors.Wrap(err, "failed to get runtime plugin") + return base.WorkerOpt{}, errors.New("failed to find any runtime plugins") } var platforms []specs.Platform From e801cc8238711f241ee813da8ec8345d9804400c Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Mon, 6 Jan 2020 00:44:20 +1100 Subject: [PATCH 3/6] Recognise Runtime V2 containerd plugins This makes this code successfully discover the Windows Runtime V2 (hcsshim-based) plugin now that the Windows Runtime V1 (runhcs-based) plugin has been removed upstream. Signed-off-by: Paul "TBBle" Hampson --- worker/containerd/containerd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/worker/containerd/containerd.go b/worker/containerd/containerd.go index 22c6dffcb5a2..da630dcab1d0 100644 --- a/worker/containerd/containerd.go +++ b/worker/containerd/containerd.go @@ -69,7 +69,7 @@ func newContainerd(root string, client *containerd.Client, snapshotterName, ns s cs := containerdsnapshot.NewContentStore(client.ContentStore(), ns) - resp, err := client.IntrospectionService().Plugins(context.TODO(), &introspection.PluginsRequest{Filters: []string{"type==io.containerd.runtime.v1"}}) + resp, err := client.IntrospectionService().Plugins(context.TODO(), &introspection.PluginsRequest{Filters: []string{"type==io.containerd.runtime.v1", "type==io.containerd.runtime.v2"}}) if err != nil { return base.WorkerOpt{}, errors.Wrap(err, "failed to list runtime plugin") } From 2bee17a65af98b8e1a2dd9ae65e2a768967bc10a Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Mon, 6 Jan 2020 00:56:16 +1100 Subject: [PATCH 4/6] Don't always fail euid check on Windows The check for running as a non-admin euid() doesn't work on Windows, always returning -1. For now, treat -1 as "Probably root", and let the failures happen later. Signed-off-by: Paul "TBBle" Hampson --- cmd/buildkitd/main.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/buildkitd/main.go b/cmd/buildkitd/main.go index 0eb1a20a660d..7c53484c0370 100644 --- a/cmd/buildkitd/main.go +++ b/cmd/buildkitd/main.go @@ -173,7 +173,9 @@ func main() { app.Flags = append(app.Flags, appFlags...) app.Action = func(c *cli.Context) error { - if os.Geteuid() != 0 { + // TODO: On Windows this always returns -1. The actual "are you admin" check is very Windows-specific. + // See https://github.com/golang/go/issues/28804#issuecomment-505326268 for the "short" version. + if os.Geteuid() > 0 { return errors.New("rootless mode requires to be executed as the mapped root in a user namespace; you may use RootlessKit for setting up the namespace") } ctx, cancel := context.WithCancel(appcontext.Context()) From e11b881c126b3fe2c31bdc1691ae6efa969062cd Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Mon, 6 Jan 2020 01:03:58 +1100 Subject: [PATCH 5/6] Set sensible defaults for Windows installations Non-packaged execution will need this to be overridden anyway, and it avoids a surprise "Drop state data into the current working directory" event. Signed-off-by: Paul "TBBle" Hampson --- util/appdefaults/appdefaults_windows.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/util/appdefaults/appdefaults_windows.go b/util/appdefaults/appdefaults_windows.go index 74f8389da656..d5d0ca1fb99f 100644 --- a/util/appdefaults/appdefaults_windows.go +++ b/util/appdefaults/appdefaults_windows.go @@ -1,9 +1,17 @@ package appdefaults +import ( + "os" + "path/filepath" +) + const ( - Address = "npipe:////./pipe/buildkitd" - Root = ".buildstate" - ConfigDir = "" + Address = "npipe:////./pipe/buildkitd" +) + +var ( + Root = filepath.Join(os.Getenv("ProgramData"), "buildkitd", ".buildstate") + ConfigDir = filepath.Join(os.Getenv("ProgramData"), "buildkitd") ) func UserAddress() string { From b9cf317850d50c57bda5266ff8480ab93eba0fd3 Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Mon, 6 Jan 2020 08:04:02 +1100 Subject: [PATCH 6/6] Distinguish containerd failure from process exit code Signed-off-by: Paul "TBBle" Hampson --- executor/containerdexecutor/executor.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/executor/containerdexecutor/executor.go b/executor/containerdexecutor/executor.go index 2ca4a5850f09..4919054acd27 100644 --- a/executor/containerdexecutor/executor.go +++ b/executor/containerdexecutor/executor.go @@ -193,7 +193,12 @@ func (w containerdExecutor) Exec(ctx context.Context, meta executor.Meta, root c cancel() } if status.ExitCode() != 0 { - err := errors.Errorf("process returned non-zero exit code: %d", status.ExitCode()) + var err error + if status.ExitCode() == containerd.UnknownExitStatus && status.Error() != nil { + err = errors.Wrap(status.Error(), "failure waiting for process") + } else { + err = errors.Errorf("process returned non-zero exit code: %d", status.ExitCode()) + } select { case <-ctx.Done(): err = errors.Wrap(ctx.Err(), err.Error())