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

Support Docker Volumes and Logging #1767

Merged
merged 8 commits into from
Oct 7, 2016
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 81 additions & 23 deletions client/driver/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,18 @@ const (
// driver
dockerDriverAttr = "driver.docker"

// dockerSELinuxLabelConfigOption is the key for configuring the
// SELinux label for binds.
dockerSELinuxLabelConfigOption = "docker.volumes.selinuxlabel"

// dockerVolumesConfigOption is the key for enabling the use of custom
// bind volumes.
dockerVolumesConfigOption = "docker.volumes.enabled"

// dockerPrivilegedConfigOption is the key for running containers in
// Docker's privileged mode.
dockerPrivilegedConfigOption = "docker.privileged.enabled"

// dockerTimeout is the length of time a request can be outstanding before
// it is timed out.
dockerTimeout = 1 * time.Minute
Expand All @@ -74,6 +86,12 @@ type DockerDriverAuth struct {
ServerAddress string `mapstructure:"server_address"` // server address of the registry
}

type DockerLoggingOpts struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments on fields

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

Type string `mapstructure:"type"`
ConfigRaw []map[string]string `mapstructure:"config"`
Config map[string]string `mapstructure:"-"`
}

type DockerDriverConfig struct {
ImageName string `mapstructure:"image"` // Container's Image Name
LoadImages []string `mapstructure:"load"` // LoadImage is array of paths to image archive files
Expand All @@ -97,6 +115,8 @@ type DockerDriverConfig struct {
Interactive bool `mapstructure:"interactive"` // Keep STDIN open even if not attached
ShmSize int64 `mapstructure:"shm_size"` // Size of /dev/shm of the container in bytes
WorkDir string `mapstructure:"work_dir"` // Working directory inside the container
Logging []DockerLoggingOpts `mapstructure:"logging"` // Logging options for syslog server
Volumes []string `mapstructure:"volumes"` // Host-Volumes to mount in, syntax: /path/to/host/directory:/destination/path/in/container
}

// Validate validates a docker driver config
Expand All @@ -107,7 +127,9 @@ func (c *DockerDriverConfig) Validate() error {

c.PortMap = mapMergeStrInt(c.PortMapRaw...)
c.Labels = mapMergeStrStr(c.LabelsRaw...)

if len(c.Logging) > 0 {
c.Logging[0].Config = mapMergeStrStr(c.Logging[0].ConfigRaw...)
}
return nil
}

Expand Down Expand Up @@ -227,6 +249,12 @@ func (d *DockerDriver) Validate(config map[string]interface{}) error {
"work_dir": &fields.FieldSchema{
Type: fields.TypeString,
},
"logging": &fields.FieldSchema{
Type: fields.TypeArray,
},
"volumes": &fields.FieldSchema{
Type: fields.TypeArray,
},
},
}

Expand Down Expand Up @@ -320,9 +348,9 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool
return false, nil
}

privileged := d.config.ReadBoolDefault("docker.privileged.enabled", false)
privileged := d.config.ReadBoolDefault(dockerPrivilegedConfigOption, false)
if privileged {
node.Attributes["docker.privileged.enabled"] = "1"
node.Attributes[dockerPrivilegedConfigOption] = "1"
}

// This is the first operation taken on the client so we'll try to
Expand All @@ -339,10 +367,18 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool

node.Attributes[dockerDriverAttr] = "1"
node.Attributes["driver.docker.version"] = env.Get("Version")

// Advertise if this node supports Docker volumes (by default we do not)
if d.config.ReadBoolDefault(dockerVolumesConfigOption, false) {
node.Attributes["driver."+dockerVolumesConfigOption] = "1"
}

return true, nil
}

func (d *DockerDriver) containerBinds(alloc *allocdir.AllocDir, task *structs.Task) ([]string, error) {
func (d *DockerDriver) containerBinds(driverConfig *DockerDriverConfig, alloc *allocdir.AllocDir,
task *structs.Task) ([]string, error) {

shared := alloc.SharedDir
local, ok := alloc.TaskDirs[task.Name]
if !ok {
Expand All @@ -356,17 +392,25 @@ func (d *DockerDriver) containerBinds(alloc *allocdir.AllocDir, task *structs.Ta
allocDirBind := fmt.Sprintf("%s:%s", shared, allocdir.SharedAllocContainerPath)
taskLocalBind := fmt.Sprintf("%s:%s", local, allocdir.TaskLocalContainerPath)
secretDirBind := fmt.Sprintf("%s:%s", secret, allocdir.TaskSecretsContainerPath)
binds := []string{allocDirBind, taskLocalBind, secretDirBind}

if selinuxLabel := d.config.Read("docker.volumes.selinuxlabel"); selinuxLabel != "" {
allocDirBind = fmt.Sprintf("%s:%s", allocDirBind, selinuxLabel)
taskLocalBind = fmt.Sprintf("%s:%s", taskLocalBind, selinuxLabel)
secretDirBind = fmt.Sprintf("%s:%s", secretDirBind, selinuxLabel)
volumesEnabled := d.config.ReadBoolDefault(dockerVolumesConfigOption, false)
if len(driverConfig.Volumes) > 0 && !volumesEnabled {
return nil, fmt.Errorf("%s is false; cannot use Docker Volumes: %+q", dockerVolumesConfigOption, driverConfig.Volumes)
}
return []string{
allocDirBind,
taskLocalBind,
secretDirBind,
}, nil

if len(driverConfig.Volumes) > 0 {
binds = append(binds, driverConfig.Volumes...)
}

if selinuxLabel := d.config.Read(dockerSELinuxLabelConfigOption); selinuxLabel != "" {
// Apply SELinux Label to each volume
for i := range binds {
binds[i] = fmt.Sprintf("%s:%s", binds[i], selinuxLabel)
}
}

return binds, nil
}

// createContainer initializes a struct needed to call docker.client.CreateContainer()
Expand All @@ -380,7 +424,7 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task,
return c, fmt.Errorf("task.Resources is empty")
}

binds, err := d.containerBinds(ctx.AllocDir, task)
binds, err := d.containerBinds(driverConfig, ctx.AllocDir, task)
if err != nil {
return c, err
}
Expand All @@ -403,6 +447,16 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task,
}

memLimit := int64(task.Resources.MemoryMB) * 1024 * 1024

if len(driverConfig.Logging) == 0 {
d.logger.Printf("[DEBUG] driver.docker: Setting default logging options to syslog and %s", syslogAddr)
driverConfig.Logging = []DockerLoggingOpts{
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this to a factory method NewDockerLoggingOpts

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

{Type: "syslog", Config: map[string]string{"syslog-address": syslogAddr}},
}
}

d.logger.Printf("[DEBUG] driver.docker: Using config for logging: %+v", driverConfig.Logging[0])

hostConfig := &docker.HostConfig{
// Convert MB to bytes. This is an absolute value.
Memory: memLimit,
Expand All @@ -415,10 +469,8 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task,
// used to share data between different tasks in the same task group.
Binds: binds,
LogConfig: docker.LogConfig{
Type: "syslog",
Config: map[string]string{
"syslog-address": syslogAddr,
},
Type: driverConfig.Logging[0].Type,
Config: driverConfig.Logging[0].Config,
},
}

Expand All @@ -427,7 +479,7 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task,
d.logger.Printf("[DEBUG] driver.docker: binding directories %#v for %s", hostConfig.Binds, task.Name)

// set privileged mode
hostPrivileged := d.config.ReadBoolDefault("docker.privileged.enabled", false)
hostPrivileged := d.config.ReadBoolDefault(dockerPrivilegedConfigOption, false)
if driverConfig.Privileged && !hostPrivileged {
return c, fmt.Errorf(`Docker privileged mode is disabled on this Nomad agent`)
}
Expand Down Expand Up @@ -731,12 +783,18 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle
PortLowerBound: d.config.ClientMinPort,
PortUpperBound: d.config.ClientMaxPort,
}
ss, err := exec.LaunchSyslogServer(executorCtx)
if err != nil {
return nil, fmt.Errorf("failed to start syslog collector: %v", err)

// Only launch syslog server if we're going to use it!
syslogAddr := ""
if len(driverConfig.Logging) == 0 || driverConfig.Logging[0].Type == "syslog" {
Copy link
Contributor

Choose a reason for hiding this comment

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

The first condition will never be true right?

Copy link
Member Author

Choose a reason for hiding this comment

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

At this point it can be. In the createContainer call a few lines down it adds the default logging option if this slice is empty.

ss, err := exec.LaunchSyslogServer(executorCtx)
if err != nil {
return nil, fmt.Errorf("failed to start syslog collector: %v", err)
}
syslogAddr = ss.Addr
}

config, err := d.createContainer(ctx, task, driverConfig, ss.Addr)
config, err := d.createContainer(ctx, task, driverConfig, syslogAddr)
if err != nil {
d.logger.Printf("[ERR] driver.docker: failed to create container configuration for image %s: %s", image, err)
pluginClient.Kill()
Expand Down
94 changes: 94 additions & 0 deletions client/driver/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"io/ioutil"
"math/rand"
"os"
"path"
"path/filepath"
"reflect"
"runtime/debug"
Expand All @@ -17,6 +19,7 @@ import (
"github.com/hashicorp/nomad/client/config"
"github.com/hashicorp/nomad/client/driver/env"
"github.com/hashicorp/nomad/client/testutil"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
tu "github.com/hashicorp/nomad/testutil"
)
Expand Down Expand Up @@ -861,6 +864,97 @@ func TestDockerDriver_Stats(t *testing.T) {

}

func setupDockerVolumes(t *testing.T, cfg *config.Config) (*structs.Task, Driver, *ExecContext, string, func()) {
if !testutil.DockerIsConnected(t) {
t.SkipNow()
}

tmpvol, err := ioutil.TempDir("", "nomadtest_dockerdriver_volumes")
if err != nil {
t.Fatalf("error creating temporary dir: %v", err)
}

randfn := fmt.Sprintf("test-%d", rand.Int())
hostpath := path.Join(tmpvol, randfn)
contpath := path.Join("/mnt/vol", randfn)

task := &structs.Task{
Name: "ls",
Config: map[string]interface{}{
"image": "busybox",
"load": []string{"busybox.tar"},
"command": "touch",
"args": []string{contpath},
"volumes": []string{fmt.Sprintf("%s:/mnt/vol", tmpvol)},
},
LogConfig: &structs.LogConfig{
MaxFiles: 10,
MaxFileSizeMB: 10,
},
Resources: basicResources,
}

allocDir := allocdir.NewAllocDir(filepath.Join(cfg.AllocDir, structs.GenerateUUID()), task.Resources.DiskMB)
allocDir.Build([]*structs.Task{task})
alloc := mock.Alloc()
execCtx := NewExecContext(allocDir, alloc.ID)
cleanup := func() {
execCtx.AllocDir.Destroy()
os.RemoveAll(tmpvol)
}

taskEnv, err := GetTaskEnv(allocDir, cfg.Node, task, alloc, "")
if err != nil {
cleanup()
t.Fatalf("Failed to get task env: %v", err)
}

driverCtx := NewDriverContext(task.Name, cfg, cfg.Node, testLogger(), taskEnv)
driver := NewDockerDriver(driverCtx)
copyImage(execCtx, task, "busybox.tar", t)

return task, driver, execCtx, hostpath, cleanup
}

func TestDockerDriver_VolumesDisabled(t *testing.T) {
cfg := testConfig()

task, driver, execCtx, _, cleanup := setupDockerVolumes(t, cfg)
defer cleanup()

_, err := driver.Start(execCtx, task)
if err == nil {
t.Fatalf("Started driver successfully when volumes should have been disabled.")
}
}

func TestDockerDriver_VolumesEnabled(t *testing.T) {
cfg := testConfig()
cfg.Options = map[string]string{dockerVolumesConfigOption: "true"}

task, driver, execCtx, hostpath, cleanup := setupDockerVolumes(t, cfg)
defer cleanup()

handle, err := driver.Start(execCtx, task)
if err != nil {
t.Fatalf("Failed to start docker driver: %v", err)
}
defer handle.Kill()

select {
case res := <-handle.WaitCh():
if !res.Successful() {
t.Fatalf("unexpected err: %v", res)
}
case <-time.After(time.Duration(tu.TestMultiplier()*10) * time.Second):
t.Fatalf("timeout")
}

if _, err := ioutil.ReadFile(hostpath); err != nil {
t.Fatalf("unexpected error reading %s: %v", hostpath, err)
}
}

func copyImage(execCtx *ExecContext, task *structs.Task, image string, t *testing.T) {
taskDir, _ := execCtx.AllocDir.TaskDirs[task.Name]
dst := filepath.Join(taskDir, allocdir.TaskLocal, image)
Expand Down
3 changes: 3 additions & 0 deletions command/agent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,9 @@ func DevConfig() *Config {
conf.Client.Options = map[string]string{
"driver.raw_exec.enable": "true",
}
conf.Client.Options = map[string]string{
"driver.docker.volumes": "true",
}

return conf
}
Expand Down
16 changes: 15 additions & 1 deletion website/source/docs/drivers/docker.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,14 @@ The `docker` driver supports the following configuration in the job spec:

* `shm_size` - (Optional) The size (bytes) of /dev/shm for the container.

* `volumes` - (Optional) A list of `host_path:container_path` strings to bind
host paths to container paths. Can only be run on clients with the
`docker.volumes.enabled` option set to true.

* `volumes_from` - (Optional) A list of volumes to inherit from another
container. Can only be run on clients with the `docker.volumes.enabled`
option set to true.

* `work_dir` - (Optional) The working directory inside the container.

### Container Name
Expand Down Expand Up @@ -329,8 +337,14 @@ options](/docs/agent/config.html#options):
* `docker.cleanup.image` Defaults to `true`. Changing this to `false` will
prevent Nomad from removing images from stopped tasks.

* `docker.volumes.enabled`: Defaults to `false`. Allows tasks to bind host
paths (`volumes`) or other containers (`volums_from`) inside their container.
Disabled by default as it removes the isolation between containers' data.

* `docker.volumes.selinuxlabel`: Allows the operator to set a SELinux
label to the allocation and task local bind-mounts to containers.
label to the allocation and task local bind-mounts to containers. If used
with `docker.volumes.enabled` set to false, the labels will still be applied
to the standard binds in the container.

* `docker.privileged.enabled` Defaults to `false`. Changing this to `true` will
allow containers to use `privileged` mode, which gives the containers full
Expand Down