Merge pull request #35 from tiborvass/18.09-fix-network-buildkit
[18.09] builder: fix bridge networking when using buildkit
diff --git a/builder/builder-next/builder.go b/builder/builder-next/builder.go
index 97cdfc6..6d3098a 100644
--- a/builder/builder-next/builder.go
+++ b/builder/builder-next/builder.go
@@ -37,6 +37,7 @@
type Opt struct {
SessionManager *session.Manager
Root string
+ NetnsRoot string
Dist images.DistributionServices
NetworkController libnetwork.NetworkController
}
diff --git a/builder/builder-next/controller.go b/builder/builder-next/controller.go
index 9794f5f..48e74cb 100644
--- a/builder/builder-next/controller.go
+++ b/builder/builder-next/controller.go
@@ -90,7 +90,7 @@
return nil, err
}
- exec, err := newExecutor(root, opt.NetworkController)
+ exec, err := newExecutor(root, opt.NetnsRoot, opt.NetworkController)
if err != nil {
return nil, err
}
diff --git a/builder/builder-next/executor_unix.go b/builder/builder-next/executor_unix.go
index 44f2dfc..f72a27f 100644
--- a/builder/builder-next/executor_unix.go
+++ b/builder/builder-next/executor_unix.go
@@ -3,41 +3,46 @@
package buildkit
import (
- "fmt"
+ "os"
"path/filepath"
+ "strconv"
"sync"
"github.com/docker/libnetwork"
"github.com/moby/buildkit/executor"
"github.com/moby/buildkit/executor/runcexecutor"
"github.com/moby/buildkit/identity"
+ "github.com/moby/buildkit/solver/pb"
"github.com/moby/buildkit/util/network"
- "github.com/pkg/errors"
- "github.com/sirupsen/logrus"
+ specs "github.com/opencontainers/runtime-spec/specs-go"
)
const networkName = "bridge"
-func newExecutor(root string, net libnetwork.NetworkController) (executor.Executor, error) {
- // FIXME: fix bridge networking
- _ = bridgeProvider{}
+func newExecutor(root, netnsRoot string, net libnetwork.NetworkController) (executor.Executor, error) {
+ networkProviders := map[pb.NetMode]network.Provider{
+ pb.NetMode_UNSET: &bridgeProvider{NetworkController: net, netnsRoot: netnsRoot},
+ pb.NetMode_HOST: network.NewHostProvider(),
+ pb.NetMode_NONE: network.NewNoneProvider(),
+ }
return runcexecutor.New(runcexecutor.Opt{
Root: filepath.Join(root, "executor"),
CommandCandidates: []string{"docker-runc", "runc"},
- }, nil)
+ }, networkProviders)
}
type bridgeProvider struct {
libnetwork.NetworkController
+ netnsRoot string
}
-func (p *bridgeProvider) NewInterface() (network.Interface, error) {
+func (p *bridgeProvider) New() (network.Namespace, error) {
n, err := p.NetworkByName(networkName)
if err != nil {
return nil, err
}
- iface := &lnInterface{ready: make(chan struct{})}
+ iface := &lnInterface{ready: make(chan struct{}), provider: p}
iface.Once.Do(func() {
go iface.init(p.NetworkController, n)
})
@@ -45,33 +50,13 @@
return iface, nil
}
-func (p *bridgeProvider) Release(iface network.Interface) error {
- go func() {
- if err := p.release(iface); err != nil {
- logrus.Errorf("%s", err)
- }
- }()
- return nil
-}
-
-func (p *bridgeProvider) release(iface network.Interface) error {
- li, ok := iface.(*lnInterface)
- if !ok {
- return errors.Errorf("invalid interface %T", iface)
- }
- err := li.sbx.Delete()
- if err1 := li.ep.Delete(true); err1 != nil && err == nil {
- err = err1
- }
- return err
-}
-
type lnInterface struct {
ep libnetwork.Endpoint
sbx libnetwork.Sandbox
sync.Once
- err error
- ready chan struct{}
+ err error
+ ready chan struct{}
+ provider *bridgeProvider
}
func (iface *lnInterface) init(c libnetwork.NetworkController, n libnetwork.Network) {
@@ -99,14 +84,26 @@
iface.ep = ep
}
-func (iface *lnInterface) Set(pid int) error {
+func (iface *lnInterface) Set(s *specs.Spec) {
<-iface.ready
if iface.err != nil {
- return iface.err
+ return
}
- return iface.sbx.SetKey(fmt.Sprintf("/proc/%d/ns/net", pid))
+ // attach netns to bridge within the container namespace, using reexec in a prestart hook
+ s.Hooks = &specs.Hooks{
+ Prestart: []specs.Hook{{
+ Path: filepath.Join("/proc", strconv.Itoa(os.Getpid()), "exe"),
+ Args: []string{"libnetwork-setkey", iface.sbx.ContainerID(), iface.provider.NetworkController.ID()},
+ }},
+ }
}
-func (iface *lnInterface) Remove(pid int) error {
- return nil
+func (iface *lnInterface) Close() error {
+ <-iface.ready
+ err := iface.sbx.Delete()
+ if iface.err != nil {
+ // iface.err takes precedence over cleanup errors
+ return iface.err
+ }
+ return err
}
diff --git a/builder/builder-next/executor_windows.go b/builder/builder-next/executor_windows.go
index e3b5aba..f19bf18 100644
--- a/builder/builder-next/executor_windows.go
+++ b/builder/builder-next/executor_windows.go
@@ -10,7 +10,7 @@
"github.com/moby/buildkit/executor"
)
-func newExecutor(_ string, _ libnetwork.NetworkController) (executor.Executor, error) {
+func newExecutor(_, _ string, _ libnetwork.NetworkController) (executor.Executor, error) {
return &winExecutor{}, nil
}
diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go
index 199bba0..56f88c4 100644
--- a/cmd/dockerd/daemon.go
+++ b/cmd/dockerd/daemon.go
@@ -288,6 +288,7 @@
bk, err := buildkit.New(buildkit.Opt{
SessionManager: sm,
Root: filepath.Join(config.Root, "buildkit"),
+ NetnsRoot: filepath.Join(config.ExecRoot, "netns"),
Dist: daemon.DistributionServices(),
NetworkController: daemon.NetworkController(),
})
diff --git a/vendor.conf b/vendor.conf
index 4269c9f..0156abb 100644
--- a/vendor.conf
+++ b/vendor.conf
@@ -26,7 +26,7 @@
golang.org/x/sync 1d60e4601c6fd243af51cc01ddf169918a5407ca
# buildkit
-github.com/moby/buildkit 49906c62925ed429ec9174a0b6869982967f1a39
+github.com/moby/buildkit e1cd06ad6b74e4b747306c4408c451b3b6d87a89
github.com/tonistiigi/fsutil b19464cd1b6a00773b4f2eb7acf9c30426f9df42
github.com/grpc-ecosystem/grpc-opentracing 8e809c8a86450a29b90dcc9efbf062d0fe6d9746
github.com/opentracing/opentracing-go 1361b9cd60be79c4c3a7fa9841b3c132e40066a7
diff --git a/vendor/github.com/moby/buildkit/executor/oci/spec_unix.go b/vendor/github.com/moby/buildkit/executor/oci/spec_unix.go
index 498f6c0..e529b96 100644
--- a/vendor/github.com/moby/buildkit/executor/oci/spec_unix.go
+++ b/vendor/github.com/moby/buildkit/executor/oci/spec_unix.go
@@ -14,6 +14,7 @@
"github.com/mitchellh/hashstructure"
"github.com/moby/buildkit/executor"
"github.com/moby/buildkit/snapshot"
+ "github.com/moby/buildkit/util/network"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
)
@@ -21,7 +22,7 @@
// Ideally we don't have to import whole containerd just for the default spec
// GenerateSpec generates spec using containerd functionality.
-func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mount, id, resolvConf, hostsFile string, hostNetwork bool, opts ...oci.SpecOpts) (*specs.Spec, func(), error) {
+func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mount, id, resolvConf, hostsFile string, namespace network.Namespace, opts ...oci.SpecOpts) (*specs.Spec, func(), error) {
c := &containers.Container{
ID: id,
}
@@ -30,16 +31,15 @@
ctx = namespaces.WithNamespace(ctx, "buildkit")
}
- if hostNetwork {
- opts = append(opts, oci.WithHostNamespace(specs.NetworkNamespace))
- }
-
// Note that containerd.GenerateSpec is namespaced so as to make
// specs.Linux.CgroupsPath namespaced
s, err := oci.GenerateSpec(ctx, nil, c, opts...)
if err != nil {
return nil, nil, err
}
+ // set the networking information on the spec
+ namespace.Set(s)
+
s.Process.Args = meta.Args
s.Process.Env = meta.Env
s.Process.Cwd = meta.Cwd
diff --git a/vendor/github.com/moby/buildkit/executor/runcexecutor/executor.go b/vendor/github.com/moby/buildkit/executor/runcexecutor/executor.go
index 2874314..4458a99 100644
--- a/vendor/github.com/moby/buildkit/executor/runcexecutor/executor.go
+++ b/vendor/github.com/moby/buildkit/executor/runcexecutor/executor.go
@@ -10,7 +10,6 @@
"path/filepath"
"strconv"
"strings"
- "sync"
"syscall"
"github.com/containerd/containerd/contrib/seccomp"
@@ -26,7 +25,6 @@
"github.com/moby/buildkit/util/network"
rootlessspecconv "github.com/moby/buildkit/util/rootless/specconv"
"github.com/moby/buildkit/util/system"
- runcsystem "github.com/opencontainers/runc/libcontainer/system"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
@@ -43,14 +41,14 @@
var defaultCommandCandidates = []string{"buildkit-runc", "runc"}
type runcExecutor struct {
- runc *runc.Runc
- root string
- cmd string
- rootless bool
- networkProvider network.Provider
+ runc *runc.Runc
+ root string
+ cmd string
+ rootless bool
+ networkProviders map[pb.NetMode]network.Provider
}
-func New(opt Opt, networkProvider network.Provider) (executor.Executor, error) {
+func New(opt Opt, networkProviders map[pb.NetMode]network.Provider) (executor.Executor, error) {
cmds := opt.CommandCandidates
if cmds == nil {
cmds = defaultCommandCandidates
@@ -70,10 +68,6 @@
root := opt.Root
- if err := setSubReaper(); err != nil {
- return nil, err
- }
-
if err := os.MkdirAll(root, 0700); err != nil {
return nil, errors.Wrapf(err, "failed to create %s", root)
}
@@ -98,36 +92,28 @@
}
w := &runcExecutor{
- runc: runtime,
- root: root,
- rootless: opt.Rootless,
- networkProvider: networkProvider,
+ runc: runtime,
+ root: root,
+ rootless: opt.Rootless,
+ networkProviders: networkProviders,
}
return w, nil
}
func (w *runcExecutor) Exec(ctx context.Context, meta executor.Meta, root cache.Mountable, mounts []executor.Mount, stdin io.ReadCloser, stdout, stderr io.WriteCloser) error {
- var iface network.Interface
- // FIXME: still uses host if no provider configured
- if meta.NetMode == pb.NetMode_UNSET {
- if w.networkProvider != nil {
- var err error
- iface, err = w.networkProvider.NewInterface()
- if err != nil || iface == nil {
- meta.NetMode = pb.NetMode_HOST
- }
- } else {
- meta.NetMode = pb.NetMode_HOST
- }
+ provider, ok := w.networkProviders[meta.NetMode]
+ if !ok {
+ return errors.Errorf("unknown network mode %s", meta.NetMode)
}
+ namespace, err := provider.New()
+ if err != nil {
+ return err
+ }
+ defer namespace.Close()
+
if meta.NetMode == pb.NetMode_HOST {
logrus.Info("enabling HostNetworking")
}
- defer func() {
- if iface != nil {
- w.networkProvider.Release(iface)
- }
- }()
resolvConf, err := oci.GetResolvConf(ctx, w.root)
if err != nil {
@@ -187,7 +173,7 @@
if meta.ReadonlyRootFS {
opts = append(opts, containerdoci.WithRootFSReadonly())
}
- spec, cleanup, err := oci.GenerateSpec(ctx, meta, mounts, id, resolvConf, hostsFile, meta.NetMode == pb.NetMode_HOST, opts...)
+ spec, cleanup, err := oci.GenerateSpec(ctx, meta, mounts, id, resolvConf, hostsFile, namespace, opts...)
if err != nil {
return err
}
@@ -225,75 +211,14 @@
}
defer forwardIO.Close()
- pidFilePath := filepath.Join(w.root, "runc_pid_"+identity.NewID())
- defer os.RemoveAll(pidFilePath)
-
logrus.Debugf("> creating %s %v", id, meta.Args)
- err = w.runc.Create(ctx, id, bundle, &runc.CreateOpts{
- PidFile: pidFilePath,
- IO: forwardIO,
+ status, err := w.runc.Run(ctx, id, bundle, &runc.CreateOpts{
+ IO: forwardIO,
})
if err != nil {
return err
}
- forwardIO.release()
- defer func() {
- go func() {
- if err := w.runc.Delete(context.TODO(), id, &runc.DeleteOpts{}); err != nil {
- logrus.Errorf("failed to delete %s: %+v", id, err)
- }
- }()
- }()
-
- dt, err := ioutil.ReadFile(pidFilePath)
- if err != nil {
- return err
- }
- pid, err := strconv.Atoi(string(dt))
- if err != nil {
- return err
- }
-
- done := make(chan struct{})
- defer close(done)
-
- go func() {
- select {
- case <-done:
- case <-ctx.Done():
- syscall.Kill(-pid, syscall.SIGKILL)
- }
- }()
-
- if iface != nil {
- if err := iface.Set(pid); err != nil {
- return errors.Wrap(err, "could not set the network")
- }
- defer func() {
- iface.Remove(pid)
- }()
- }
-
- err = w.runc.Start(ctx, id)
- if err != nil {
- return err
- }
-
- p, err := os.FindProcess(pid)
- if err != nil {
- return err
- }
-
- status := 0
- ps, err := p.Wait()
- if err != nil {
- status = 255
- }
-
- if ws, ok := ps.Sys().(syscall.WaitStatus); ok {
- status = ws.ExitStatus()
- }
if status != 0 {
return errors.Errorf("exit code: %d", status)
}
@@ -336,7 +261,7 @@
}
func (s *forwardIO) Close() error {
- s.release()
+ s.CloseAfterStart()
var err error
for _, cl := range s.toClose {
if err1 := cl.Close(); err == nil {
@@ -348,11 +273,12 @@
}
// release releases active FDs if the process doesn't need them any more
-func (s *forwardIO) release() {
+func (s *forwardIO) CloseAfterStart() error {
for _, cl := range s.toRelease {
cl.Close()
}
s.toRelease = nil
+ return nil
}
func (s *forwardIO) Set(cmd *exec.Cmd) {
@@ -401,16 +327,6 @@
return pw, nil
}
-var subReaperOnce sync.Once
-var subReaperError error
-
-func setSubReaper() error {
- subReaperOnce.Do(func() {
- subReaperError = runcsystem.SetSubreaper(1)
- })
- return subReaperError
-}
-
func (s *forwardIO) Stdin() io.WriteCloser {
return nil
}
diff --git a/vendor/github.com/moby/buildkit/util/network/host.go b/vendor/github.com/moby/buildkit/util/network/host.go
new file mode 100644
index 0000000..dc58b1c
--- /dev/null
+++ b/vendor/github.com/moby/buildkit/util/network/host.go
@@ -0,0 +1,28 @@
+package network
+
+import (
+ "github.com/containerd/containerd/oci"
+ specs "github.com/opencontainers/runtime-spec/specs-go"
+)
+
+func NewHostProvider() Provider {
+ return &host{}
+}
+
+type host struct {
+}
+
+func (h *host) New() (Namespace, error) {
+ return &hostNS{}, nil
+}
+
+type hostNS struct {
+}
+
+func (h *hostNS) Set(s *specs.Spec) {
+ oci.WithHostNamespace(specs.NetworkNamespace)(nil, nil, nil, s)
+}
+
+func (h *hostNS) Close() error {
+ return nil
+}
diff --git a/vendor/github.com/moby/buildkit/util/network/network.go b/vendor/github.com/moby/buildkit/util/network/network.go
index 2f66652..055a52d 100644
--- a/vendor/github.com/moby/buildkit/util/network/network.go
+++ b/vendor/github.com/moby/buildkit/util/network/network.go
@@ -1,17 +1,32 @@
package network
-// Provider interface for Network
-type Provider interface {
- NewInterface() (Interface, error)
- Release(Interface) error
+import (
+ "io"
+
+ "github.com/moby/buildkit/solver/pb"
+ specs "github.com/opencontainers/runtime-spec/specs-go"
+)
+
+// Default returns the default network provider set
+func Default() map[pb.NetMode]Provider {
+ return map[pb.NetMode]Provider{
+ // FIXME: still uses host if no provider configured
+ pb.NetMode_UNSET: NewHostProvider(),
+ pb.NetMode_HOST: NewHostProvider(),
+ pb.NetMode_NONE: NewNoneProvider(),
+ }
}
-// Interface of network for workers
-type Interface interface {
- // Set the pid with network interace namespace
- Set(int) error
- // Removes the network interface
- Remove(int) error
+// Provider interface for Network
+type Provider interface {
+ New() (Namespace, error)
+}
+
+// Namespace of network for workers
+type Namespace interface {
+ io.Closer
+ // Set the namespace on the spec
+ Set(*specs.Spec)
}
// NetworkOpts hold network options
diff --git a/vendor/github.com/moby/buildkit/util/network/none.go b/vendor/github.com/moby/buildkit/util/network/none.go
new file mode 100644
index 0000000..ebf1ebd
--- /dev/null
+++ b/vendor/github.com/moby/buildkit/util/network/none.go
@@ -0,0 +1,26 @@
+package network
+
+import (
+ specs "github.com/opencontainers/runtime-spec/specs-go"
+)
+
+func NewNoneProvider() Provider {
+ return &none{}
+}
+
+type none struct {
+}
+
+func (h *none) New() (Namespace, error) {
+ return &noneNS{}, nil
+}
+
+type noneNS struct {
+}
+
+func (h *noneNS) Set(s *specs.Spec) {
+}
+
+func (h *noneNS) Close() error {
+ return nil
+}
