Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libcontainerd: split client and daemon supervision #37149

Merged
merged 1 commit into from Aug 16, 2018
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+338 −474
Diff settings

Always

Just for now

@@ -36,7 +36,7 @@ import (
"github.com/docker/docker/daemon/config"
"github.com/docker/docker/daemon/listeners"
"github.com/docker/docker/dockerversion"
"github.com/docker/docker/libcontainerd"
"github.com/docker/docker/libcontainerd/supervisor"
dopts "github.com/docker/docker/opts"
"github.com/docker/docker/pkg/authorization"
"github.com/docker/docker/pkg/jsonmessage"
@@ -45,7 +45,6 @@ import (
"github.com/docker/docker/pkg/signal"
"github.com/docker/docker/pkg/system"
"github.com/docker/docker/plugin"
"github.com/docker/docker/registry"
"github.com/docker/docker/runconfig"
"github.com/docker/go-connections/tlsconfig"
swarmapi "github.com/docker/swarmkit/api"
@@ -112,6 +111,10 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
return err
}

if err := system.MkdirAll(cli.Config.ExecRoot, 0700, ""); err != nil {

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Jul 10, 2018

Member

This looks new; where/when was this directory created in the past?

I also see some validations steps below that may fail; is it possible to move this further down, so that we don't end up with this directory if the configuration happened to be invalid?

Finally; not sure if this directory needs special treatment (as the "daemon" root gets); also taking into account that this code is both Linux and Windows (and Windows doesn't handle permissions similar to Linux)

moby/daemon/daemon_unix.go

Lines 1136 to 1187 in 8e2f920

func setupDaemonRoot(config *config.Config, rootDir string, rootIDs idtools.IDPair) error {
config.Root = rootDir
// the docker root metadata directory needs to have execute permissions for all users (g+x,o+x)
// so that syscalls executing as non-root, operating on subdirectories of the graph root
// (e.g. mounted layers of a container) can traverse this path.
// The user namespace support will create subdirectories for the remapped root host uid:gid
// pair owned by that same uid:gid pair for proper write access to those needed metadata and
// layer content subtrees.
if _, err := os.Stat(rootDir); err == nil {
// root current exists; verify the access bits are correct by setting them
if err = os.Chmod(rootDir, 0711); err != nil {
return err
}
} else if os.IsNotExist(err) {
// no root exists yet, create it 0711 with root:root ownership
if err := os.MkdirAll(rootDir, 0711); err != nil {
return err
}
}
// if user namespaces are enabled we will create a subtree underneath the specified root
// with any/all specified remapped root uid/gid options on the daemon creating
// a new subdirectory with ownership set to the remapped uid/gid (so as to allow
// `chdir()` to work for containers namespaced to that uid/gid)
if config.RemappedRoot != "" {
config.Root = filepath.Join(rootDir, fmt.Sprintf("%d.%d", rootIDs.UID, rootIDs.GID))
logrus.Debugf("Creating user namespaced daemon root: %s", config.Root)
// Create the root directory if it doesn't exist
if err := idtools.MkdirAllAndChown(config.Root, 0700, rootIDs); err != nil {
return fmt.Errorf("Cannot create daemon root: %s: %v", config.Root, err)
}
// we also need to verify that any pre-existing directories in the path to
// the graphroot won't block access to remapped root--if any pre-existing directory
// has strict permissions that don't allow "x", container start will fail, so
// better to warn and fail now
dirPath := config.Root
for {
dirPath = filepath.Dir(dirPath)
if dirPath == "/" {
break
}
if !idtools.CanAccess(dirPath, rootIDs) {
return fmt.Errorf("a subdirectory in your graphroot path (%s) restricts access to the remapped root uid/gid; please fix by allowing 'o+x' permissions on existing directories", config.Root)
}
}
}
if err := setupDaemonRootPropagation(config); err != nil {
logrus.WithError(err).WithField("dir", config.Root).Warn("Error while setting daemon root propagation, this is not generally critical but may cause some functionality to not work or fallback to less desirable behavior")
}
return nil
}

func setupDaemonRoot(config *config.Config, rootDir string, rootIDs idtools.IDPair) error {
config.Root = rootDir
// Create the root directory if it doesn't exists
if err := system.MkdirAllWithACL(config.Root, 0, system.SddlAdministratorsLocalSystem); err != nil {
return err
}
return nil
}

This comment has been minimized.

Copy link
@dmcgowan

dmcgowan Jul 18, 2018

Author Member

Previously this directory was getting created as a side effect of getting a containerd client. This seemed to be an inappropriate way to create a shared directory so I just moved it here. It is still getting created with same permissions at roughly the same time though.

return err
}

if cli.Pidfile != "" {
pf, err := pidfile.New(cli.Pidfile)
if err != nil {
@@ -135,19 +138,27 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
return fmt.Errorf("Failed to load listeners: %v", err)
}

registryService, err := registry.NewService(cli.Config.ServiceOptions)
if err != nil {
return err
}
ctx, cancel := context.WithCancel(context.Background())
if cli.Config.ContainerdAddr == "" && runtime.GOOS != "windows" {
opts, err := cli.getContainerdDaemonOpts()
if err != nil {
cancel()

This comment has been minimized.

Copy link
@kolyshkin

kolyshkin Aug 16, 2018

Contributor

It looks like if you move defer cancel() to right after context.WithCancel you won't have to call it explicitly, i.e.

	ctx, cancel := context.WithCancel(context.Background())
+ 	defer cancel()
 	if cli.Config.ContainerdAddr == "" && runtime.GOOS != "windows" {
 		opts, err := cli.getContainerdDaemonOpts()
 		if err != nil {
- 			cancel()
 			return fmt.Errorf("Failed to generate containerd options: %v", err)
 		}

This comment has been minimized.

Copy link
@dmcgowan

dmcgowan Aug 16, 2018

Author Member

This is actually like this on purpose, the defer ordering is important because the cancel() should run before r.WaitTimeout(10 * time.Second). I should probably note this in a comment

return fmt.Errorf("Failed to generate containerd options: %v", err)
}

rOpts, err := cli.getRemoteOptions()
if err != nil {
return fmt.Errorf("Failed to generate containerd options: %v", err)
}
containerdRemote, err := libcontainerd.New(filepath.Join(cli.Config.Root, "containerd"), filepath.Join(cli.Config.ExecRoot, "containerd"), rOpts...)
if err != nil {
return err
r, err := supervisor.Start(ctx, filepath.Join(cli.Config.Root, "containerd"), filepath.Join(cli.Config.ExecRoot, "containerd"), opts...)
if err != nil {
cancel()

This comment has been minimized.

Copy link
@kolyshkin

kolyshkin Aug 16, 2018

Contributor

ditto

return fmt.Errorf("Failed to start containerd: %v", err)
}

cli.Config.ContainerdAddr = r.Address()

// Try to wait for containerd to shutdown
defer r.WaitTimeout(10 * time.Second)
}
defer cancel()

signal.Trap(func() {
cli.stop()
<-stopc // wait for daemonCli.start() to return
@@ -162,7 +173,7 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
logrus.Fatalf("Error creating middlewares: %v", err)
}

d, err := daemon.NewDaemon(cli.Config, registryService, containerdRemote, pluginStore)
d, err := daemon.NewDaemon(ctx, cli.Config, pluginStore)
if err != nil {
return fmt.Errorf("Error starting daemon: %v", err)
}
@@ -207,10 +218,7 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {

initRouter(routerOptions)

// process cluster change notifications
watchCtx, cancel := context.WithCancel(context.Background())
defer cancel()
go d.ProcessClusterNotifications(watchCtx, c.GetWatchStream())
go d.ProcessClusterNotifications(ctx, c.GetWatchStream())

cli.setupConfigReloadTrap()

@@ -227,8 +235,12 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
// Wait for serve API to complete
errAPI := <-serveAPIWait
c.Cleanup()

shutdownDaemon(d)
containerdRemote.Cleanup()

// Stop notification processing and any background processes
cancel()

if errAPI != nil {
return fmt.Errorf("Shutting down due to ServeAPI error: %v", errAPI)
}
@@ -511,14 +523,22 @@ func (cli *DaemonCli) initMiddlewares(s *apiserver.Server, cfg *apiserver.Config
return nil
}

func (cli *DaemonCli) getRemoteOptions() ([]libcontainerd.RemoteOption, error) {
opts := []libcontainerd.RemoteOption{}

pOpts, err := cli.getPlatformRemoteOptions()
func (cli *DaemonCli) getContainerdDaemonOpts() ([]supervisor.DaemonOpt, error) {
opts, err := cli.getPlatformContainerdDaemonOpts()
if err != nil {
return nil, err
}
opts = append(opts, pOpts...)

if cli.Config.Debug {
opts = append(opts, supervisor.WithLogLevel("debug"))

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Jul 10, 2018

Member

Should we use cli.Config.LogLevel for this? (That's set to debug if debug is enabled, but otherwise uses what's configured by the user) see #37419

This comment has been minimized.

Copy link
@dmcgowan

dmcgowan Jul 20, 2018

Author Member

Looks like you addressed this based on the last rebase

} else if cli.Config.LogLevel != "" {
opts = append(opts, supervisor.WithLogLevel(cli.Config.LogLevel))
}

if !cli.Config.CriContainerd {
opts = append(opts, supervisor.WithPlugin("cri", nil))
}

return opts, nil
}

@@ -13,7 +13,7 @@ import (
"github.com/containerd/containerd/runtime/linux"
"github.com/docker/docker/cmd/dockerd/hack"
"github.com/docker/docker/daemon"
"github.com/docker/docker/libcontainerd"
"github.com/docker/docker/libcontainerd/supervisor"
"github.com/docker/libnetwork/portallocator"
"golang.org/x/sys/unix"
)
@@ -36,29 +36,16 @@ func getDaemonConfDir(_ string) string {
return "/etc/docker"
}

func (cli *DaemonCli) getPlatformRemoteOptions() ([]libcontainerd.RemoteOption, error) {
opts := []libcontainerd.RemoteOption{
libcontainerd.WithOOMScore(cli.Config.OOMScoreAdjust),
libcontainerd.WithPlugin("linux", &linux.Config{
func (cli *DaemonCli) getPlatformContainerdDaemonOpts() ([]supervisor.DaemonOpt, error) {
opts := []supervisor.DaemonOpt{
supervisor.WithOOMScore(cli.Config.OOMScoreAdjust),
supervisor.WithPlugin("linux", &linux.Config{
Shim: daemon.DefaultShimBinary,
Runtime: daemon.DefaultRuntimeBinary,
RuntimeRoot: filepath.Join(cli.Config.Root, "runc"),
ShimDebug: cli.Config.Debug,
}),
}
if cli.Config.Debug {
opts = append(opts, libcontainerd.WithLogLevel("debug"))
} else if cli.Config.LogLevel != "" {
opts = append(opts, libcontainerd.WithLogLevel(cli.Config.LogLevel))
}
if cli.Config.ContainerdAddr != "" {
opts = append(opts, libcontainerd.WithRemoteAddr(cli.Config.ContainerdAddr))
} else {
opts = append(opts, libcontainerd.WithStartDaemon(true))
}
if !cli.Config.CriContainerd {
opts = append(opts, libcontainerd.WithPlugin("cri", nil))
}

return opts, nil
}
@@ -6,7 +6,7 @@ import (
"os"
"path/filepath"

"github.com/docker/docker/libcontainerd"
"github.com/docker/docker/libcontainerd/supervisor"
"github.com/sirupsen/logrus"
"golang.org/x/sys/windows"
)
@@ -48,7 +48,7 @@ func notifyShutdown(err error) {
}
}

func (cli *DaemonCli) getPlatformRemoteOptions() ([]libcontainerd.RemoteOption, error) {
func (cli *DaemonCli) getPlatformContainerdDaemonOpts() ([]supervisor.DaemonOpt, error) {
return nil, nil
}

@@ -18,6 +18,11 @@ import (
"sync"
"time"

"google.golang.org/grpc"

"github.com/containerd/containerd"
"github.com/containerd/containerd/defaults"
"github.com/containerd/containerd/pkg/dialer"
"github.com/docker/docker/api/types"
containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/swarm"
@@ -94,6 +99,7 @@ type Daemon struct {
PluginStore *plugin.Store // todo: remove
pluginManager *plugin.Manager
linkIndex *linkIndex
containerdCli *containerd.Client
containerd libcontainerd.Client
defaultIsolation containertypes.Isolation // Default isolation mode on Windows
clusterProvider cluster.Provider
@@ -565,9 +571,14 @@ func (daemon *Daemon) IsSwarmCompatible() error {

// NewDaemon sets up everything for the daemon to be able to service
// requests from the webserver.
func NewDaemon(config *config.Config, registryService registry.Service, containerdRemote libcontainerd.Remote, pluginStore *plugin.Store) (daemon *Daemon, err error) {
func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.Store) (daemon *Daemon, err error) {
setDefaultMtu(config)

registryService, err := registry.NewService(config.ServiceOptions)
if err != nil {
return nil, err
}

// Ensure that we have a correct root key limit for launching containers.
if err := ModifyRootKeyLimit(); err != nil {
logrus.Warnf("unable to modify root key limit, number of containers could be limited by this quota: %v", err)
@@ -720,8 +731,35 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe
}
registerMetricsPluginCallback(d.PluginStore, metricsSockPath)

gopts := []grpc.DialOption{
grpc.WithInsecure(),
grpc.WithBackoffMaxDelay(3 * time.Second),
grpc.WithDialer(dialer.Dialer),

// TODO(stevvooe): We may need to allow configuration of this on the client.
grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(defaults.DefaultMaxRecvMsgSize)),
grpc.WithDefaultCallOptions(grpc.MaxCallSendMsgSize(defaults.DefaultMaxSendMsgSize)),
}
if config.ContainerdAddr != "" {
d.containerdCli, err = containerd.New(config.ContainerdAddr, containerd.WithDefaultNamespace(ContainersNamespace), containerd.WithDialOpts(gopts))
if err != nil {
return nil, errors.Wrapf(err, "failed to dial %q", config.ContainerdAddr)
}
}

createPluginExec := func(m *plugin.Manager) (plugin.Executor, error) {
return pluginexec.New(getPluginExecRoot(config.Root), containerdRemote, m)
var pluginCli *containerd.Client

// Windows is not currently using containerd, keep the
// client as nil
if config.ContainerdAddr != "" {
pluginCli, err = containerd.New(config.ContainerdAddr, containerd.WithDefaultNamespace(pluginexec.PluginNamespace), containerd.WithDialOpts(gopts))
if err != nil {
return nil, errors.Wrapf(err, "failed to dial %q", config.ContainerdAddr)
}
}

return pluginexec.New(ctx, getPluginExecRoot(config.Root), pluginCli, m)
}

// Plugin system initialization should happen before restore. Do not change order.
@@ -880,7 +918,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe

go d.execCommandGC()

d.containerd, err = containerdRemote.NewClient(ContainersNamespace, d)
d.containerd, err = libcontainerd.NewClient(ctx, d.containerdCli, filepath.Join(config.ExecRoot, "containerd"), ContainersNamespace, d)
if err != nil {
return nil, err
}
@@ -1037,6 +1075,10 @@ func (daemon *Daemon) Shutdown() error {
daemon.netController.Stop()
}

if daemon.containerdCli != nil {
daemon.containerdCli.Close()
}

return daemon.cleanupMounts()
}

Oops, something went wrong.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.