Skip to content

Commit

Permalink
fix/pr: fixes from pr review
Browse files Browse the repository at this point in the history
NOTE: all these follow-up commits will be squashed after the review
is done.

What changed in this commit:

- refactor the code to make the dial retry logic be the same across
  platforms.
- a number of code duplication fixes and refactors, raised by @jedevc
  • Loading branch information
profnandaa committed Dec 2, 2023
1 parent 414edfa commit 4cdae01
Show file tree
Hide file tree
Showing 13 changed files with 299 additions and 292 deletions.
116 changes: 97 additions & 19 deletions client/client_test.go

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions util/testutil/integration/sanbox_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package integration

func RootlessSupported(uid int) bool {
// not supported on Windows
return false
}
11 changes: 0 additions & 11 deletions util/testutil/integration/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"testing"

"github.com/google/shlex"
"github.com/moby/buildkit/util/bklog"
"github.com/pkg/errors"
)

Expand Down Expand Up @@ -113,16 +112,6 @@ func newSandbox(ctx context.Context, w Worker, mirror string, mv matrixValue) (s
}, cl, nil
}

func RootlessSupported(uid int) bool {
cmd := exec.Command("sudo", "-u", fmt.Sprintf("#%d", uid), "-i", "--", "exec", "unshare", "-U", "true") //nolint:gosec // test utility
b, err := cmd.CombinedOutput()
if err != nil {
bklog.L.Warnf("rootless mode is not supported on this host: %v (%s)", err, string(b))
return false
}
return true
}

func PrintLogs(logs map[string]*bytes.Buffer, f func(args ...interface{})) {
for name, l := range logs {
f(name)
Expand Down
21 changes: 21 additions & 0 deletions util/testutil/integration/sandbox_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//go:build !windows
// +build !windows

package integration

import (
"fmt"
"os/exec"

"github.com/moby/buildkit/util/bklog"
)

func RootlessSupported(uid int) bool {
cmd := exec.Command("sudo", "-u", fmt.Sprintf("#%d", uid), "-i", "--", "exec", "unshare", "-U", "true") //nolint:gosec // test utility
b, err := cmd.CombinedOutput()
if err != nil {
bklog.L.Warnf("rootless mode is not supported on this host: %v (%s)", err, string(b))
return false
}
return true
}
21 changes: 20 additions & 1 deletion util/testutil/integration/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io"
"os"
"os/exec"
"strings"
"sync"
"syscall"
"testing"
Expand Down Expand Up @@ -102,7 +103,25 @@ func StartCmd(cmd *exec.Cmd, logs map[string]*bytes.Buffer) (func() error, error
// 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 {
return waitSocket(address, d, cmd)
address = strings.TrimPrefix(address, socketScheme)
step := 50 * time.Millisecond
i := 0
for {
if cmd != nil && cmd.ProcessState != nil {
return errors.Errorf("process exited: %s", cmd.String())
}

if conn, err := dialPipe(address); err == nil {
conn.Close()
break
}
i++
if time.Duration(i)*step > d {
return errors.Errorf("failed dialing: %s", address)
}
time.Sleep(step)
}
return nil
}

func LookupBinary(name string) error {
Expand Down
33 changes: 8 additions & 25 deletions util/testutil/integration/util_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,19 @@ package integration

import (
"net"
"os/exec"
"strings"
"time"

"github.com/pkg/errors"
)

func waitSocket(address string, d time.Duration, cmd *exec.Cmd) error {
address = strings.TrimPrefix(address, "unix://")
var socketScheme = "unix://"

// abstracted function to handle pipe dialing on
// 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 errors.Wrapf(err, "failed resolving unix addr: %s", address)
}

step := 50 * time.Millisecond
i := 0
for {
if cmd != nil && cmd.ProcessState != nil {
return errors.Errorf("process exited: %s", cmd.String())
}

if conn, err := net.DialUnix("unix", nil, addr); err == nil {
conn.Close()
break
}
i++
if time.Duration(i)*step > d {
return errors.Errorf("failed dialing: %s", address)
}
time.Sleep(step)
return nil, errors.Wrapf(err, "failed resolving unix addr: %s", address)
}
return nil
return net.DialUnix("unix", nil, addr)
}
30 changes: 6 additions & 24 deletions util/testutil/integration/util_windows.go
Original file line number Diff line number Diff line change
@@ -1,33 +1,15 @@
package integration

import (
"os/exec"
"strings"
"time"
"net"

"github.com/Microsoft/go-winio"
"github.com/pkg/errors"
)

func waitSocket(address string, d time.Duration, cmd *exec.Cmd) error {
address = strings.TrimPrefix(address, "npipe://")
step := 50 * time.Millisecond
i := 0
var socketScheme = "npipe://"

for {
if cmd != nil && cmd.ProcessState != nil {
return errors.Errorf("process exited: %s", cmd.String())
}

if conn, err := winio.DialPipe(address, nil); err == nil {
conn.Close()
break
}
i++
if time.Duration(i)*step > d {
return errors.Errorf("failed dialing: %s", address)
}
time.Sleep(step)
}
return nil
// abstracted function to handle pipe dialing on
// Some simplification has been made to discard timeout param
func dialPipe(address string) (net.Conn, error) {
return winio.DialPipe(address, nil)
}
2 changes: 1 addition & 1 deletion util/testutil/workers/containerd.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (c *Containerd) New(ctx context.Context, cfg *integration.BackendConfig) (b
}()

rootless := false
if runtime.GOOS != "windows" && c.UID != 0 {
if c.UID != 0 {
if c.GID == 0 {
return nil, nil, errors.Errorf("unsupported id pair: uid=%d, gid=%d", c.UID, c.GID)
}
Expand Down
41 changes: 0 additions & 41 deletions util/testutil/workers/sysprocattr_unix.go

This file was deleted.

43 changes: 0 additions & 43 deletions util/testutil/workers/sysprocattr_windows.go

This file was deleted.

94 changes: 94 additions & 0 deletions util/testutil/workers/util.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
package workers

import (
"bytes"
"context"
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
"time"

"github.com/moby/buildkit/util/testutil/integration"
)
Expand All @@ -19,3 +26,90 @@ func (osp otelSocketPath) UpdateConfigFile(in string) string {
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) {
deferF := &integration.MultiCloser{}
cl = deferF.F()

defer func() {
if err != nil {
deferF.F()()
cl = nil
}
}()

tmpdir, err := os.MkdirTemp("", "bktest_buildkitd")
if err != nil {
return "", nil, err
}

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 := 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))))
if err != nil {
return "", nil, err
}
deferF.Append(func() error {
return os.RemoveAll(filepath.Dir(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(cmd.Env, extraEnv...)
cmd.SysProcAttr = getSysProcAttr()

stop, err := integration.StartCmd(cmd, logs)
if err != nil {
return "", nil, err
}
deferF.Append(stop)

if err := integration.WaitSocket(address, 15*time.Second, cmd); err != nil {
return "", nil, err
}

mountInfo(deferF)

return address, cl, err
}

func getBuildkitdArgs(address string) []string {
// handles case for windows, no effect on unix.
address = filepath.ToSlash(address)
if !strings.HasPrefix(address, "npipe://") {
address = "npipe://" + address
}
return []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
}
}
Loading

0 comments on commit 4cdae01

Please sign in to comment.