From 1d531ff64f99e07ac8733894416de8212a6c7278 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Wed, 22 Aug 2018 02:37:32 +0000 Subject: [PATCH] builder: fix bridge networking when using buildkit Signed-off-by: Tibor Vass (cherry picked from commit dc7e472db986fa3c07806f56e82db756c47567fb) Signed-off-by: Tibor Vass --- builder/builder-next/builder.go | 1 + builder/builder-next/controller.go | 2 +- builder/builder-next/executor_unix.go | 71 +++++----- builder/builder-next/executor_windows.go | 2 +- cmd/dockerd/daemon.go | 1 + vendor.conf | 2 +- .../moby/buildkit/executor/oci/spec_unix.go | 10 +- .../executor/runcexecutor/executor.go | 134 ++++-------------- .../moby/buildkit/util/network/host.go | 28 ++++ .../moby/buildkit/util/network/network.go | 31 ++-- .../moby/buildkit/util/network/none.go | 26 ++++ 11 files changed, 146 insertions(+), 162 deletions(-) create mode 100644 vendor/github.com/moby/buildkit/util/network/host.go create mode 100644 vendor/github.com/moby/buildkit/util/network/none.go diff --git a/builder/builder-next/builder.go b/builder/builder-next/builder.go index 97cdfc62d2..6d3098aa63 100644 --- a/builder/builder-next/builder.go +++ b/builder/builder-next/builder.go @@ -37,6 +37,7 @@ func init() { 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 9794f5f5c7..48e74cb851 100644 --- a/builder/builder-next/controller.go +++ b/builder/builder-next/controller.go @@ -90,7 +90,7 @@ func newController(rt http.RoundTripper, opt Opt) (*control.Controller, error) { 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 44f2dfcd96..f72a27fb0a 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 @@ func (p *bridgeProvider) NewInterface() (network.Interface, error) { 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 @@ func (iface *lnInterface) init(c libnetwork.NetworkController, n libnetwork.Netw 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 + } + // 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()}, + }}, } - return iface.sbx.SetKey(fmt.Sprintf("/proc/%d/ns/net", pid)) } -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 e3b5aba829..f19bf18655 100644 --- a/builder/builder-next/executor_windows.go +++ b/builder/builder-next/executor_windows.go @@ -10,7 +10,7 @@ import ( "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 199bba0285..56f88c49be 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -288,6 +288,7 @@ func newRouterOptions(config *config.Config, daemon *daemon.Daemon) (routerOptio 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 4269c9f438..0156abbcd2 100644 --- a/vendor.conf +++ b/vendor.conf @@ -26,7 +26,7 @@ github.com/imdario/mergo v0.3.6 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 498f6c0de4..e529b96129 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 @@ import ( "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 @@ import ( // 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 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou 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 2874314198..4458a992a3 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 @@ import ( "path/filepath" "strconv" "strings" - "sync" "syscall" "github.com/containerd/containerd/contrib/seccomp" @@ -26,7 +25,6 @@ import ( "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 @@ type Opt struct { 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 @@ func New(opt Opt, networkProvider network.Provider) (executor.Executor, error) { 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 @@ func New(opt Opt, networkProvider network.Provider) (executor.Executor, error) { } 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 @@ func (w *runcExecutor) Exec(ctx context.Context, meta executor.Meta, root cache. 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 @@ func (w *runcExecutor) Exec(ctx context.Context, meta executor.Meta, root cache. } 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 newForwardIO(stdin io.ReadCloser, stdout, stderr io.WriteCloser) (f *forwar } 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 @@ func (s *forwardIO) Close() error { } // 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 @@ func (s *forwardIO) writeCloserToFile(wc io.WriteCloser) (*os.File, error) { 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 0000000000..dc58b1ce72 --- /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 2f66652dcf..055a52da8b 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 +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(), + } +} + // Provider interface for Network type Provider interface { - NewInterface() (Interface, error) - Release(Interface) error + New() (Namespace, error) } -// 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 +// 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 0000000000..ebf1ebda94 --- /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 +}