diff --git a/cmd/buildkitd/main_containerd_worker.go b/cmd/buildkitd/main_containerd_worker.go index 161fb2bad151..f77ee95d226b 100644 --- a/cmd/buildkitd/main_containerd_worker.go +++ b/cmd/buildkitd/main_containerd_worker.go @@ -354,7 +354,7 @@ func validContainerdSocket(cfg config.ContainerdConfig) bool { // FIXME(AkihiroSuda): prohibit tcp? return true } - socketPath := strings.TrimPrefix(socket, "unix://") + socketPath := strings.TrimPrefix(socket, socketScheme) if _, err := os.Stat(socketPath); errors.Is(err, os.ErrNotExist) { // FIXME(AkihiroSuda): add more conditions bklog.L.Warnf("skipping containerd worker, as %q does not exist", socketPath) diff --git a/cmd/buildkitd/main_unix.go b/cmd/buildkitd/main_unix.go index f9373356dbc8..d819d1187f59 100644 --- a/cmd/buildkitd/main_unix.go +++ b/cmd/buildkitd/main_unix.go @@ -14,6 +14,8 @@ import ( "github.com/pkg/errors" ) +const socketScheme = "unix://" + func init() { syscall.Umask(0) } diff --git a/cmd/buildkitd/main_windows.go b/cmd/buildkitd/main_windows.go index e273760b53f8..5ca20a689534 100644 --- a/cmd/buildkitd/main_windows.go +++ b/cmd/buildkitd/main_windows.go @@ -13,6 +13,8 @@ import ( "github.com/pkg/errors" ) +const socketScheme = "npipe://" + func listenFD(addr string, tlsConfig *tls.Config) (net.Listener, error) { return nil, errors.New("listening server on fd not supported on windows") } diff --git a/util/testutil/integration/sandbox.go b/util/testutil/integration/sandbox.go index d7f1dfff2734..585a8baa189b 100644 --- a/util/testutil/integration/sandbox.go +++ b/util/testutil/integration/sandbox.go @@ -99,7 +99,7 @@ func newSandbox(ctx context.Context, w Worker, mirror string, mv matrixValue) (s b, closer, err := w.New(ctx, cfg) if err != nil { - return nil, nil, err + return nil, nil, errors.Wrap(err, "creating worker") } deferF.Append(closer) diff --git a/util/testutil/integration/util.go b/util/testutil/integration/util.go index 6a7979e4ae16..85b52b572ce8 100644 --- a/util/testutil/integration/util.go +++ b/util/testutil/integration/util.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "io" - "net" "os" "os/exec" "strings" @@ -100,13 +99,11 @@ func StartCmd(cmd *exec.Cmd, logs map[string]*bytes.Buffer) (func() error, error }, nil } -func WaitUnix(address string, d time.Duration, cmd *exec.Cmd) error { - address = strings.TrimPrefix(address, "unix://") - addr, err := net.ResolveUnixAddr("unix", address) - if err != nil { - return errors.Wrapf(err, "failed resolving unix addr: %s", address) - } - +// WaitSocket will dial a socket opened by a command passed in as cmd. +// On Linux this socket is typically a Unix socket, +// while on Windows this will be a named pipe. +func WaitSocket(address string, d time.Duration, cmd *exec.Cmd) error { + address = strings.TrimPrefix(address, socketScheme) step := 50 * time.Millisecond i := 0 for { @@ -114,7 +111,7 @@ func WaitUnix(address string, d time.Duration, cmd *exec.Cmd) error { return errors.Errorf("process exited: %s", cmd.String()) } - if conn, err := net.DialUnix("unix", nil, addr); err == nil { + if conn, err := dialPipe(address); err == nil { conn.Close() break } diff --git a/util/testutil/integration/util_unix.go b/util/testutil/integration/util_unix.go new file mode 100644 index 000000000000..f096f954779e --- /dev/null +++ b/util/testutil/integration/util_unix.go @@ -0,0 +1,23 @@ +//go:build !windows +// +build !windows + +package integration + +import ( + "net" + + "github.com/pkg/errors" +) + +var socketScheme = "unix://" + +// abstracted function to handle pipe dialing on unix. +// some simplification has been made to discard +// laddr for unix -- left as nil. +func dialPipe(address string) (net.Conn, error) { + addr, err := net.ResolveUnixAddr("unix", address) + if err != nil { + return nil, errors.Wrapf(err, "failed resolving unix addr: %s", address) + } + return net.DialUnix("unix", nil, addr) +} diff --git a/util/testutil/integration/util_windows.go b/util/testutil/integration/util_windows.go new file mode 100644 index 000000000000..eef0e37f85e8 --- /dev/null +++ b/util/testutil/integration/util_windows.go @@ -0,0 +1,15 @@ +package integration + +import ( + "net" + + "github.com/Microsoft/go-winio" +) + +var socketScheme = "npipe://" + +// abstracted function to handle pipe dialing on windows. +// some simplification has been made to discard timeout param. +func dialPipe(address string) (net.Conn, error) { + return winio.DialPipe(address, nil) +} diff --git a/util/testutil/workers/containerd.go b/util/testutil/workers/containerd.go index 6f590e604600..37501c676e6c 100644 --- a/util/testutil/workers/containerd.go +++ b/util/testutil/workers/containerd.go @@ -88,9 +88,11 @@ func (c *Containerd) New(ctx context.Context, cfg *integration.BackendConfig) (b if err := integration.LookupBinary(c.Containerd); err != nil { return nil, nil, err } + if err := integration.LookupBinary("buildkitd"); err != nil { return nil, nil, err } + if err := requireRoot(); err != nil { return nil, nil, err } @@ -117,6 +119,7 @@ func (c *Containerd) New(ctx context.Context, cfg *integration.BackendConfig) (b if err != nil { return nil, nil, err } + if rootless { if err := os.Chown(tmpdir, c.UID, c.GID); err != nil { return nil, nil, err @@ -125,7 +128,7 @@ func (c *Containerd) New(ctx context.Context, cfg *integration.BackendConfig) (b deferF.Append(func() error { return os.RemoveAll(tmpdir) }) - address := filepath.Join(tmpdir, "containerd.sock") + address := getContainerdSock(tmpdir) config := fmt.Sprintf(`root = %q state = %q # CRI plugins listens on 10010/tcp for stream server. @@ -137,8 +140,11 @@ disabled_plugins = ["cri"] [debug] level = "debug" - address = %q -`, filepath.Join(tmpdir, "root"), filepath.Join(tmpdir, "state"), address, filepath.Join(tmpdir, "debug.sock")) + address = %q`, + filepath.Join(tmpdir, "root"), + filepath.Join(tmpdir, "state"), + address, getContainerdDebugSock(tmpdir), + ) var snBuildkitdArgs []string if c.Snapshotter != "" { @@ -185,19 +191,23 @@ disabled_plugins = ["cri"] if err != nil { return nil, nil, err } - if err := integration.WaitUnix(address, 10*time.Second, cmd); err != nil { + if err := integration.WaitSocket(address, 10*time.Second, cmd); err != nil { ctdStop() return nil, nil, errors.Wrapf(err, "containerd did not start up: %s", integration.FormatLogs(cfg.Logs)) } deferF.Append(ctdStop) - buildkitdArgs := append([]string{"buildkitd", - "--oci-worker=false", + // handles only windows case, no effect on unix + address = normalizeAddress(address) + buildkitdArgs := []string{ + "buildkitd", "--containerd-worker-gc=false", "--containerd-worker=true", "--containerd-worker-addr", address, "--containerd-worker-labels=org.mobyproject.buildkit.worker.sandbox=true", // Include use of --containerd-worker-labels to trigger https://github.com/moby/buildkit/pull/603 - }, snBuildkitdArgs...) + } + buildkitdArgs = applyBuildkitdPlatformFlags(buildkitdArgs) + buildkitdArgs = append(buildkitdArgs, snBuildkitdArgs...) if runtime.GOOS != "windows" && c.Snapshotter != "native" { c.ExtraEnv = append(c.ExtraEnv, "BUILDKIT_DEBUG_FORCE_OVERLAY_DIFF=true") @@ -266,7 +276,7 @@ func runStargzSnapshotter(cfg *integration.BackendConfig) (address string, cl fu if err != nil { return "", nil, err } - if err = integration.WaitUnix(address, 10*time.Second, cmd); err != nil { + if err = integration.WaitSocket(address, 10*time.Second, cmd); err != nil { snStop() return "", nil, errors.Wrapf(err, "containerd-stargz-grpc did not start up: %s", integration.FormatLogs(cfg.Logs)) } diff --git a/util/testutil/workers/dockerd.go b/util/testutil/workers/dockerd.go index 41622d84d8f6..8443ca9041af 100644 --- a/util/testutil/workers/dockerd.go +++ b/util/testutil/workers/dockerd.go @@ -159,7 +159,7 @@ func (c Moby) New(ctx context.Context, cfg *integration.BackendConfig) (b integr } deferF.Append(d.StopWithError) - if err := integration.WaitUnix(d.Sock(), 5*time.Second, nil); err != nil { + if err := integration.WaitSocket(d.Sock(), 5*time.Second, nil); err != nil { return nil, nil, errors.Errorf("dockerd did not start up: %q, %s", err, integration.FormatLogs(cfg.Logs)) } diff --git a/util/testutil/workers/sysprocattr_unix.go b/util/testutil/workers/sysprocattr_unix.go deleted file mode 100644 index 7f4db76ea5c7..000000000000 --- a/util/testutil/workers/sysprocattr_unix.go +++ /dev/null @@ -1,23 +0,0 @@ -//go:build !windows -// +build !windows - -package workers - -import ( - "path/filepath" - "syscall" -) - -func getSysProcAttr() *syscall.SysProcAttr { - return &syscall.SysProcAttr{ - Setsid: true, // stretch sudo needs this for sigterm - } -} - -func getBuildkitdAddr(tmpdir string) string { - return "unix://" + filepath.Join(tmpdir, "buildkitd.sock") -} - -func getTraceSocketPath(tmpdir string) string { - return filepath.Join(tmpdir, "otel-grpc.sock") -} diff --git a/util/testutil/workers/sysprocattr_windows.go b/util/testutil/workers/sysprocattr_windows.go deleted file mode 100644 index 9d67037fbf8a..000000000000 --- a/util/testutil/workers/sysprocattr_windows.go +++ /dev/null @@ -1,21 +0,0 @@ -//go:build windows -// +build windows - -package workers - -import ( - "path/filepath" - "syscall" -) - -func getSysProcAttr() *syscall.SysProcAttr { - return &syscall.SysProcAttr{} -} - -func getBuildkitdAddr(tmpdir string) string { - return "//./pipe/buildkitd-" + filepath.Base(tmpdir) -} - -func getTraceSocketPath(tmpdir string) string { - return `\\.\pipe\buildkit-otel-grpc-` + filepath.Base(tmpdir) -} diff --git a/util/testutil/workers/util.go b/util/testutil/workers/util.go index f6861bbf32dc..4d9d7dccc045 100644 --- a/util/testutil/workers/util.go +++ b/util/testutil/workers/util.go @@ -1,28 +1,39 @@ package workers import ( - "bufio" "bytes" "context" "fmt" "os" "os/exec" "path/filepath" - "strings" "time" "github.com/moby/buildkit/util/testutil/integration" - "github.com/pkg/errors" ) -func requireRoot() error { - if os.Getuid() != 0 { - return errors.Wrap(integration.ErrRequirements, "requires root") - } - return nil +func withOTELSocketPath(socketPath string) integration.ConfigUpdater { + return otelSocketPath(socketPath) +} + +type otelSocketPath string + +func (osp otelSocketPath) UpdateConfigFile(in string) string { + return fmt.Sprintf(`%s + +[otel] + socketPath = %q +`, in, osp) } -func runBuildkitd(ctx context.Context, conf *integration.BackendConfig, args []string, logs map[string]*bytes.Buffer, uid, gid int, extraEnv []string) (address string, cl func() error, err error) { +func runBuildkitd( + ctx context.Context, + conf *integration.BackendConfig, + args []string, + logs map[string]*bytes.Buffer, + uid, gid int, + extraEnv []string, +) (address string, cl func() error, err error) { deferF := &integration.MultiCloser{} cl = deferF.F() @@ -37,31 +48,39 @@ func runBuildkitd(ctx context.Context, conf *integration.BackendConfig, args []s if err != nil { return "", nil, err } - if err := os.Chown(tmpdir, uid, gid); err != nil { + + if err := chown(tmpdir, uid, gid); err != nil { return "", nil, err } + if err := os.MkdirAll(filepath.Join(tmpdir, "tmp"), 0711); err != nil { return "", nil, err } - if err := os.Chown(filepath.Join(tmpdir, "tmp"), uid, gid); err != nil { + + if err := chown(filepath.Join(tmpdir, "tmp"), uid, gid); err != nil { return "", nil, err } deferF.Append(func() error { return os.RemoveAll(tmpdir) }) - cfgfile, err := integration.WriteConfig(append(conf.DaemonConfig, withOTELSocketPath(getTraceSocketPath(tmpdir)))) + cfgfile, err := integration.WriteConfig( + append(conf.DaemonConfig, withOTELSocketPath(getTraceSocketPath(tmpdir)))) if err != nil { return "", nil, err } deferF.Append(func() error { return os.RemoveAll(filepath.Dir(cfgfile)) }) - args = append(args, "--config="+cfgfile) + args = append(args, "--config="+cfgfile) address = getBuildkitdAddr(tmpdir) args = append(args, "--root", tmpdir, "--addr", address, "--debug") cmd := exec.Command(args[0], args[1:]...) //nolint:gosec // test utility - cmd.Env = append(os.Environ(), "BUILDKIT_DEBUG_EXEC_OUTPUT=1", "BUILDKIT_DEBUG_PANIC_ON_ERROR=1", "TMPDIR="+filepath.Join(tmpdir, "tmp")) + cmd.Env = append( + os.Environ(), + "BUILDKIT_DEBUG_EXEC_OUTPUT=1", + "BUILDKIT_DEBUG_PANIC_ON_ERROR=1", + "TMPDIR="+filepath.Join(tmpdir, "tmp")) cmd.Env = append(cmd.Env, extraEnv...) cmd.SysProcAttr = getSysProcAttr() @@ -71,38 +90,14 @@ func runBuildkitd(ctx context.Context, conf *integration.BackendConfig, args []s } deferF.Append(stop) - if err := integration.WaitUnix(address, 15*time.Second, cmd); err != nil { + if err := integration.WaitSocket(address, 15*time.Second, cmd); err != nil { return "", nil, err } + // separated out since it's not required in windows deferF.Append(func() error { - f, err := os.Open("/proc/self/mountinfo") - if err != nil { - return errors.Wrap(err, "failed to open mountinfo") - } - defer f.Close() - s := bufio.NewScanner(f) - for s.Scan() { - if strings.Contains(s.Text(), tmpdir) { - return errors.Errorf("leaked mountpoint for %s", tmpdir) - } - } - return s.Err() + return mountInfo(tmpdir) }) return address, cl, err } - -func withOTELSocketPath(socketPath string) integration.ConfigUpdater { - return otelSocketPath(socketPath) -} - -type otelSocketPath string - -func (osp otelSocketPath) UpdateConfigFile(in string) string { - return fmt.Sprintf(`%s - -[otel] - socketPath = %q -`, in, osp) -} diff --git a/util/testutil/workers/util_unix.go b/util/testutil/workers/util_unix.go new file mode 100644 index 000000000000..76d5c8b516b5 --- /dev/null +++ b/util/testutil/workers/util_unix.go @@ -0,0 +1,74 @@ +//go:build !windows +// +build !windows + +package workers + +import ( + "bufio" + "os" + "path/filepath" + "strings" + "syscall" + + "github.com/moby/buildkit/util/testutil/integration" + "github.com/pkg/errors" +) + +func applyBuildkitdPlatformFlags(args []string) []string { + return append(args, "--oci-worker=false") +} + +func requireRoot() error { + if os.Getuid() != 0 { + return errors.Wrap(integration.ErrRequirements, "requires root") + } + return nil +} + +func getSysProcAttr() *syscall.SysProcAttr { + return &syscall.SysProcAttr{ + Setsid: true, // stretch sudo needs this for sigterm + } +} + +func getBuildkitdAddr(tmpdir string) string { + return "unix://" + filepath.Join(tmpdir, "buildkitd.sock") +} + +func getTraceSocketPath(tmpdir string) string { + return filepath.Join(tmpdir, "otel-grpc.sock") +} + +func getContainerdSock(tmpdir string) string { + return filepath.Join(tmpdir, "containerd.sock") +} + +func getContainerdDebugSock(tmpdir string) string { + return filepath.Join(tmpdir, "debug.sock") +} + +func mountInfo(tmpdir string) error { + f, err := os.Open("/proc/self/mountinfo") + if err != nil { + return errors.Wrap(err, "failed to open mountinfo") + } + defer f.Close() + s := bufio.NewScanner(f) + for s.Scan() { + if strings.Contains(s.Text(), tmpdir) { + return errors.Errorf("leaked mountpoint for %s", tmpdir) + } + } + return s.Err() +} + +// moved here since os.Chown is not supported on Windows. +// see no-op counterpart in util_windows.go +func chown(name string, uid, gid int) error { + return os.Chown(name, uid, gid) +} + +func normalizeAddress(address string) string { + // for parity with windows, no effect for unix + return address +} diff --git a/util/testutil/workers/util_windows.go b/util/testutil/workers/util_windows.go new file mode 100644 index 000000000000..8d95f04ef5fa --- /dev/null +++ b/util/testutil/workers/util_windows.go @@ -0,0 +1,53 @@ +package workers + +import ( + "path/filepath" + "strings" + "syscall" +) + +func applyBuildkitdPlatformFlags(args []string) []string { + return args +} + +func requireRoot() error { + return nil +} + +func getSysProcAttr() *syscall.SysProcAttr { + return &syscall.SysProcAttr{} +} + +func getBuildkitdAddr(tmpdir string) string { + return "npipe:////./pipe/buildkitd-" + filepath.Base(tmpdir) +} + +func getTraceSocketPath(tmpdir string) string { + return `\\.\pipe\buildkit-otel-grpc-` + filepath.Base(tmpdir) +} + +func getContainerdSock(tmpdir string) string { + return `\\.\pipe\containerd-` + filepath.Base(tmpdir) +} + +func getContainerdDebugSock(tmpdir string) string { + return `\\.\pipe\containerd-` + filepath.Base(tmpdir) + `debug` +} + +// no-op for parity with unix +func mountInfo(tmpdir string) error { + return nil +} + +func chown(name string, uid, gid int) error { + // Chown not supported on Windows + return nil +} + +func normalizeAddress(address string) string { + address = filepath.ToSlash(address) + if !strings.HasPrefix(address, "npipe://") { + address = "npipe://" + address + } + return address +} diff --git a/worker/containerd/containerd.go b/worker/containerd/containerd.go index 7d3d28fe5728..42987b727b21 100644 --- a/worker/containerd/containerd.go +++ b/worker/containerd/containerd.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + goRuntime "runtime" "strconv" "strings" @@ -30,14 +31,47 @@ import ( type RuntimeInfo = containerdexecutor.RuntimeInfo // NewWorkerOpt creates a WorkerOpt. -func NewWorkerOpt(root string, address, snapshotterName, ns string, rootless bool, labels map[string]string, dns *oci.DNSConfig, nopt netproviders.Opt, apparmorProfile string, selinux bool, parallelismSem *semaphore.Weighted, traceSocket string, runtime *RuntimeInfo, opts ...containerd.ClientOpt) (base.WorkerOpt, error) { +func NewWorkerOpt( + root string, + address, snapshotterName, ns string, + rootless bool, + labels map[string]string, + dns *oci.DNSConfig, + nopt netproviders.Opt, + apparmorProfile string, + selinux bool, + parallelismSem *semaphore.Weighted, + traceSocket string, + runtime *RuntimeInfo, + opts ...containerd.ClientOpt, +) (base.WorkerOpt, error) { opts = append(opts, containerd.WithDefaultNamespace(ns)) + if goRuntime.GOOS == "windows" { + // TODO(profnandaa): once the upstream PR[1] is merged and + // vendored in buildkit, we will remove this block. + // [1] https://github.com/containerd/containerd/pull/9412 + address = strings.TrimPrefix(address, "npipe://") + } client, err := containerd.New(address, opts...) if err != nil { return base.WorkerOpt{}, errors.Wrapf(err, "failed to connect client to %q . make sure containerd is running", address) } - return newContainerd(root, client, snapshotterName, ns, rootless, labels, dns, nopt, apparmorProfile, selinux, parallelismSem, traceSocket, runtime) + return newContainerd( + root, + client, + snapshotterName, + ns, + rootless, + labels, + dns, + nopt, + apparmorProfile, + selinux, + parallelismSem, + traceSocket, + runtime, + ) } func newContainerd(root string, client *containerd.Client, snapshotterName, ns string, rootless bool, labels map[string]string, dns *oci.DNSConfig, nopt netproviders.Opt, apparmorProfile string, selinux bool, parallelismSem *semaphore.Weighted, traceSocket string, runtime *RuntimeInfo) (base.WorkerOpt, error) { diff --git a/worker/containerd/containerd_test.go b/worker/containerd/containerd_test.go index ceebd12d0392..82e804a4fcbc 100644 --- a/worker/containerd/containerd_test.go +++ b/worker/containerd/containerd_test.go @@ -1,11 +1,10 @@ -//go:build linux -// +build linux +//go:build !windows +// +build !windows package containerd import ( "context" - "os" "testing" "github.com/moby/buildkit/util/network/netproviders" @@ -37,12 +36,6 @@ func newWorkerOpt(t *testing.T, addr string) base.WorkerOpt { return workerOpt } -func checkRequirement(t *testing.T) { - if os.Getuid() != 0 { - t.Skip("requires root") - } -} - func testContainerdWorkerExec(t *testing.T, sb integration.Sandbox) { if sb.Rootless() { t.Skip("requires root") diff --git a/worker/containerd/containerd_test_unix.go b/worker/containerd/containerd_test_unix.go new file mode 100644 index 000000000000..0e08990c2017 --- /dev/null +++ b/worker/containerd/containerd_test_unix.go @@ -0,0 +1,15 @@ +//go:build !windows +// +build !windows + +package containerd + +import ( + "os" + "testing" +) + +func checkRequirement(t *testing.T) { + if os.Getuid() != 0 { + t.Skip("requires root") + } +} diff --git a/worker/containerd/containerd_test_windows.go b/worker/containerd/containerd_test_windows.go new file mode 100644 index 000000000000..b3b412684e71 --- /dev/null +++ b/worker/containerd/containerd_test_windows.go @@ -0,0 +1,8 @@ +package containerd + +import "testing" + +//lint:ignore U1000 for parity with unix +func checkRequirement(t *testing.T) { + // no special requirements needed for Windows +}