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

Allow switching Windows runtimes. #42089

Merged
merged 1 commit into from Sep 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/dockerd/config.go
Expand Up @@ -99,6 +99,8 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
flags.StringVar(&conf.ContainerdNamespace, "containerd-namespace", daemon.ContainersNamespace, "Containerd namespace to use")
flags.StringVar(&conf.ContainerdPluginNamespace, "containerd-plugins-namespace", containerd.PluginNamespace, "Containerd namespace to use for plugins")

flags.StringVar(&conf.DefaultRuntime, "default-runtime", config.StockRuntimeName, "Default OCI runtime for containers")

return nil
}

Expand Down
2 changes: 0 additions & 2 deletions cmd/dockerd/config_common_unix.go
Expand Up @@ -60,6 +60,4 @@ func installUnixConfigFlags(conf *config.Config, flags *pflag.FlagSet) {
flags.BoolVar(&conf.BridgeConfig.InterContainerCommunication, "icc", true, "Enable inter-container communication")
flags.Var(opts.NewIPOpt(&conf.BridgeConfig.DefaultIP, "0.0.0.0"), "ip", "Default IP when binding container ports")
flags.Var(opts.NewNamedRuntimeOpt("runtimes", &conf.Runtimes, config.StockRuntimeName), "add-runtime", "Register an additional OCI compatible runtime")
flags.StringVar(&conf.DefaultRuntime, "default-runtime", config.StockRuntimeName, "Default OCI runtime for containers")

}
15 changes: 12 additions & 3 deletions daemon/config/config.go
Expand Up @@ -48,9 +48,7 @@ const (
// DefaultRuntimeBinary is the default runtime to be used by
// containerd if none is specified
DefaultRuntimeBinary = "runc"
// StockRuntimeName is the reserved name/alias used to represent the
// OCI runtime being shipped with the docker daemon package.
StockRuntimeName = "runc"

// LinuxV1RuntimeName is the runtime used to specify the containerd v1 shim with the runc binary
// Note this is different than io.containerd.runc.v1 which would be the v1 shim using the v2 shim API.
// This is specifically for the v1 shim using the v1 shim API.
Expand Down Expand Up @@ -273,6 +271,8 @@ type CommonConfig struct {

ContainerdNamespace string `json:"containerd-namespace,omitempty"`
ContainerdPluginNamespace string `json:"containerd-plugin-namespace,omitempty"`

DefaultRuntime string `json:"default-runtime,omitempty"`
}

// IsValueSet returns true if a configuration value
Expand Down Expand Up @@ -636,3 +636,12 @@ func ModifiedDiscoverySettings(config *Config, backendType, advertise string, cl

return !reflect.DeepEqual(config.ClusterOpts, clusterOpts)
}

// GetDefaultRuntimeName returns the current default runtime
func (conf *Config) GetDefaultRuntimeName() string {
conf.Lock()
rt := conf.DefaultRuntime
conf.Unlock()

return rt
}
14 changes: 4 additions & 10 deletions daemon/config/config_linux.go
Expand Up @@ -19,6 +19,10 @@ const (

// DefaultCgroupV1NamespaceMode is the default mode for containers cgroup namespace when using cgroups v1.
DefaultCgroupV1NamespaceMode = containertypes.CgroupnsModeHost

// StockRuntimeName is the reserved name/alias used to represent the
// OCI runtime being shipped with the docker daemon package.
StockRuntimeName = "runc"
)

// BridgeConfig stores all the bridge driver specific
Expand Down Expand Up @@ -51,7 +55,6 @@ type Config struct {

// Fields below here are platform specific.
Runtimes map[string]types.Runtime `json:"runtimes,omitempty"`
DefaultRuntime string `json:"default-runtime,omitempty"`
DefaultInitBinary string `json:"default-init,omitempty"`
CgroupParent string `json:"cgroup-parent,omitempty"`
EnableSelinuxSupport bool `json:"selinux-enabled,omitempty"`
Expand Down Expand Up @@ -83,15 +86,6 @@ func (conf *Config) GetRuntime(name string) *types.Runtime {
return nil
}

// GetDefaultRuntimeName returns the current default runtime
func (conf *Config) GetDefaultRuntimeName() string {
conf.Lock()
rt := conf.DefaultRuntime
conf.Unlock()

return rt
}

// GetAllRuntimes returns a copy of the runtimes map
func (conf *Config) GetAllRuntimes() map[string]types.Runtime {
conf.Lock()
Expand Down
4 changes: 3 additions & 1 deletion daemon/config/config_linux_test.go
Expand Up @@ -150,7 +150,9 @@ func TestUnixValidateConfigurationErrors(t *testing.T) {
Runtimes: map[string]types.Runtime{
"foo": {},
},
DefaultRuntime: "bar",
CommonConfig: CommonConfig{
DefaultRuntime: "bar",
},
},
},
}
Expand Down
11 changes: 6 additions & 5 deletions daemon/config/config_windows.go
Expand Up @@ -4,6 +4,12 @@ import (
"github.com/docker/docker/api/types"
)

const (
// This is used by the `default-runtime` flag in dockerd as the default value.
// On windows we'd prefer to keep this empty so the value is auto-detected based on other options.
StockRuntimeName = ""
Comment on lines +8 to +10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole StockRuntimeName vs DefaultRuntimeName is a bit confusing.

Wondering, will setting this to an empty string cause some odd behavior if somehow the option is set to an empty string?

moby/opts/runtime.go

Lines 44 to 46 in 7b9275c

if parts[0] == o.stockRuntimeName {
return fmt.Errorf("runtime name '%s' is reserved", o.stockRuntimeName)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, never mind; looks like we first check if it's not empty;

moby/opts/runtime.go

Lines 39 to 41 in 7b9275c

if parts[0] == "" || parts[1] == "" {
return fmt.Errorf("invalid runtime argument: %s", val)
}

)

// BridgeConfig stores all the bridge driver specific
// configuration.
type BridgeConfig struct {
Expand All @@ -26,11 +32,6 @@ func (conf *Config) GetRuntime(name string) *types.Runtime {
return nil
}

// GetDefaultRuntimeName returns the current default runtime
func (conf *Config) GetDefaultRuntimeName() string {
return StockRuntimeName
}

// GetAllRuntimes returns a copy of the runtimes map
func (conf *Config) GetAllRuntimes() map[string]types.Runtime {
return map[string]types.Runtime{}
Expand Down
7 changes: 2 additions & 5 deletions daemon/daemon.go
Expand Up @@ -42,7 +42,6 @@ import (
"github.com/docker/docker/errdefs"
"github.com/docker/docker/image"
"github.com/docker/docker/layer"
"github.com/docker/docker/libcontainerd"
libcontainerdtypes "github.com/docker/docker/libcontainerd/types"
"github.com/docker/docker/libnetwork"
"github.com/docker/docker/libnetwork/cluster"
Expand Down Expand Up @@ -922,6 +921,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(defaults.DefaultMaxRecvMsgSize)),
grpc.WithDefaultCallOptions(grpc.MaxCallSendMsgSize(defaults.DefaultMaxSendMsgSize)),
}

if config.ContainerdAddr != "" {
d.containerdCli, err = containerd.New(config.ContainerdAddr, containerd.WithDefaultNamespace(config.ContainerdNamespace), containerd.WithDialOpts(gopts), containerd.WithTimeout(60*time.Second))
if err != nil {
Expand All @@ -932,8 +932,6 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
createPluginExec := func(m *plugin.Manager) (plugin.Executor, error) {
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(config.ContainerdPluginNamespace), containerd.WithDialOpts(gopts), containerd.WithTimeout(60*time.Second))
if err != nil {
Expand Down Expand Up @@ -1113,8 +1111,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S

go d.execCommandGC()

d.containerd, err = libcontainerd.NewClient(ctx, d.containerdCli, filepath.Join(config.ExecRoot, "containerd"), config.ContainerdNamespace, d)
if err != nil {
if err := d.initLibcontainerd(ctx); err != nil {
return nil, err
}

Expand Down
13 changes: 13 additions & 0 deletions daemon/daemon_unix.go
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/docker/docker/daemon/config"
"github.com/docker/docker/daemon/initlayer"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/libcontainerd/remote"
"github.com/docker/docker/libnetwork"
nwconfig "github.com/docker/docker/libnetwork/config"
"github.com/docker/docker/libnetwork/drivers/bridge"
Expand Down Expand Up @@ -1736,3 +1737,15 @@ func (daemon *Daemon) RawSysInfo() *sysinfo.SysInfo {
func recursiveUnmount(target string) error {
return mount.RecursiveUnmount(target)
}

func (daemon *Daemon) initLibcontainerd(ctx context.Context) error {
var err error
daemon.containerd, err = remote.NewClient(
ctx,
daemon.containerdCli,
filepath.Join(daemon.configStore.ExecRoot, "containerd"),
daemon.configStore.ContainerdNamespace,
daemon,
)
return err
}
44 changes: 44 additions & 0 deletions daemon/daemon_windows.go
Expand Up @@ -14,6 +14,8 @@ import (
containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/container"
"github.com/docker/docker/daemon/config"
"github.com/docker/docker/libcontainerd/local"
"github.com/docker/docker/libcontainerd/remote"
"github.com/docker/docker/libnetwork"
nwconfig "github.com/docker/docker/libnetwork/config"
"github.com/docker/docker/libnetwork/datastore"
Expand All @@ -40,6 +42,9 @@ const (
windowsMaxCPUShares = 10000
windowsMinCPUPercent = 1
windowsMaxCPUPercent = 100

windowsV1RuntimeName = "com.docker.hcsshim.v1"
windowsV2RuntimeName = "io.containerd.runhcs.v1"
)

// Windows containers are much larger than Linux containers and each of them
Expand Down Expand Up @@ -649,3 +654,42 @@ func setupResolvConf(config *config.Config) {
func (daemon *Daemon) RawSysInfo() *sysinfo.SysInfo {
return sysinfo.New()
}

func (daemon *Daemon) initLibcontainerd(ctx context.Context) error {
var err error

rt := daemon.configStore.GetDefaultRuntimeName()
if rt == "" {
if daemon.configStore.ContainerdAddr == "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No cases where we use a default value for the containerd address or anything like that? ISTR some talk about trying to default to V2 on 2022+? (I guess that ship sailed?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So my plan is to follow-up with a PR to embed containerd and use that if no address is specified on both linux and windows.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, even on Linux we don't default a socket here.
On Windows we just don't start a containerd process like we do on Linux.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, got it; thanks for the added detail.

rt = windowsV1RuntimeName
} else {
rt = windowsV2RuntimeName
}
}

switch rt {
case windowsV1RuntimeName:
daemon.containerd, err = local.NewClient(
ctx,
daemon.containerdCli,
filepath.Join(daemon.configStore.ExecRoot, "containerd"),
daemon.configStore.ContainerdNamespace,
daemon,
)
case windowsV2RuntimeName:
if daemon.configStore.ContainerdAddr == "" {
return fmt.Errorf("cannot use the specified runtime %q without containerd", rt)
}
daemon.containerd, err = remote.NewClient(
ctx,
daemon.containerdCli,
filepath.Join(daemon.configStore.ExecRoot, "containerd"),
daemon.configStore.ContainerdNamespace,
daemon,
)
default:
return fmt.Errorf("unknown windows runtime %s", rt)
}

return err
}