From b9bfb84b530d4a4707caa63668021f147549dad9 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Fri, 9 Jun 2017 10:29:41 -0700 Subject: [PATCH] Implement DriverNetwork and Service.AddressMode Ideally DriverNetwork would be fully populated in Driver.Prestart, but Docker doesn't assign the container's IP until you start the container. However, it's important to setup the port env vars before calling Driver.Start, so Prestart should populate that. --- api/tasks.go | 11 ++-- client/consul.go | 3 +- client/driver/docker.go | 60 ++++++++++++++++-- client/driver/driver.go | 27 ++++++--- client/driver/env/env.go | 108 +++++++++++++++++++++++++-------- client/driver/exec.go | 4 +- client/driver/java.go | 4 +- client/driver/lxc.go | 8 +-- client/driver/mock_driver.go | 4 +- client/driver/qemu.go | 6 +- client/driver/raw_exec.go | 4 +- client/driver/rkt.go | 5 +- client/structs/structs.go | 37 +++++++++++ client/task_runner.go | 29 +++++---- command/agent/consul/client.go | 46 +++++++++++--- command/agent/job_endpoint.go | 7 ++- jobspec/parse.go | 1 + nomad/structs/structs.go | 86 ++++++++++++++++++-------- 18 files changed, 338 insertions(+), 112 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index b6f89c04d..67c4a92ce 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -95,11 +95,12 @@ type ServiceCheck struct { // The Service model represents a Consul service definition type Service struct { - Id string - Name string - Tags []string - PortLabel string `mapstructure:"port"` - Checks []ServiceCheck + Id string + Name string + Tags []string + PortLabel string `mapstructure:"port"` + AddressMode string `mapstructure:"address_mode"` + Checks []ServiceCheck } func (s *Service) Canonicalize(t *Task, tg *TaskGroup, job *Job) { diff --git a/client/consul.go b/client/consul.go index 043a17bdb..25f61882f 100644 --- a/client/consul.go +++ b/client/consul.go @@ -2,13 +2,14 @@ package client import ( "github.com/hashicorp/nomad/client/driver" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) // ConsulServiceAPI is the interface the Nomad Client uses to register and // remove services and checks from Consul. type ConsulServiceAPI interface { - RegisterTask(allocID string, task *structs.Task, exec driver.ScriptExecutor) error + RegisterTask(allocID string, task *structs.Task, exec driver.ScriptExecutor, net *cstructs.DriverNetwork) error RemoveTask(allocID string, task *structs.Task) UpdateTask(allocID string, existing, newTask *structs.Task, exec driver.ScriptExecutor) error } diff --git a/client/driver/docker.go b/client/driver/docker.go index b28c1a606..0885a7271 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -471,7 +471,7 @@ func (d *DockerDriver) Prestart(ctx *ExecContext, task *structs.Task) (*Prestart return nil, err } - // Set state needed by Start() + // Set state needed by Start d.driverConfig = driverConfig // Initialize docker API clients @@ -488,12 +488,11 @@ func (d *DockerDriver) Prestart(ctx *ExecContext, task *structs.Task) (*Prestart resp := NewPrestartResponse() resp.CreatedResources.Add(dockerImageResKey, id) - resp.PortMap = d.driverConfig.PortMap d.imageID = id return resp, nil } -func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, error) { +func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse, error) { pluginLogFile := filepath.Join(ctx.TaskDir.Dir, "executor.out") executorConfig := &dstructs.ExecutorConfig{ @@ -560,6 +559,15 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle pluginClient.Kill() return nil, fmt.Errorf("Failed to start container %s: %s", container.ID, err) } + // InspectContainer to get all of the container metadata as + // much of the metadata (eg networking) isn't populated until + // the container is started + if container, err = client.InspectContainer(container.ID); err != nil { + err = fmt.Errorf("failed to inspect started container %s: %s", container.ID, err) + d.logger.Printf("[ERR] driver.docker: %v", err) + pluginClient.Kill() + return nil, structs.NewRecoverableError(err, true) + } d.logger.Printf("[INFO] driver.docker: started container %s", container.ID) } else { d.logger.Printf("[DEBUG] driver.docker: re-attaching to container %s with status %q", @@ -585,7 +593,51 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle } go h.collectStats() go h.run() - return h, nil + + // Detect container address + ip, autoUse := d.detectIP(container) + + // Create a response with the driver handle and container network metadata + resp := &StartResponse{ + Handle: h, + Network: &cstructs.DriverNetwork{ + PortMap: d.driverConfig.PortMap, + IP: ip, + AutoUseIP: autoUse, + }, + } + return resp, nil +} + +func (d *DockerDriver) detectIP(c *docker.Container) (string, bool) { + if c.NetworkSettings == nil { + // This should only happen if there's been a coding error (such + // as not calling InspetContainer after CreateContainer). Code + // defensively in case the Docker API changes subtly. + d.logger.Printf("[WARN] driver.docker: no network settings for container %s", c.ID) + return "", false + } + ip, ipName := "", "" + n := 0 + auto := false + for name, net := range c.NetworkSettings.Networks { + if net.IPAddress == "" { + // Ignore networks without an IP address + continue + } + n++ + ip = net.IPAddress + ipName = name + + // Don't auto-advertise bridge IPs + if name != "bridge" { + auto = true + } + } + if n > 1 { + d.logger.Printf("[WARN] driver.docker: multiple (%d) Docker networks for container %q but Nomad only supports 1: choosing %q", n, c.ID, ipName) + } + return ip, auto } func (d *DockerDriver) Cleanup(_ *ExecContext, res *CreatedResources) error { diff --git a/client/driver/driver.go b/client/driver/driver.go index a04a798d2..1dae7e051 100644 --- a/client/driver/driver.go +++ b/client/driver/driver.go @@ -57,10 +57,6 @@ type Factory func(*DriverContext) Driver type PrestartResponse struct { // CreatedResources by the driver. CreatedResources *CreatedResources - - // PortMap can be set by drivers to replace ports in environment - // variables with driver-specific mappings. - PortMap map[string]int } // NewPrestartResponse creates a new PrestartResponse with CreatedResources @@ -183,6 +179,20 @@ func (r *CreatedResources) Hash() []byte { return h.Sum(nil) } +// StartResponse is returned by Driver.Start. +type StartResponse struct { + // Handle to the driver's task executor for controlling the lifecycle + // of the task. + Handle DriverHandle + + // DriverNetwork contains driver-specific network parameters such as + // the port map between the host and a container. + // + // Network may be nil as not all drivers or configurations create + // networks. + Network *cstructs.DriverNetwork +} + // Driver is used for execution of tasks. This allows Nomad // to support many pluggable implementations of task drivers. // Examples could include LXC, Docker, Qemu, etc. @@ -196,8 +206,11 @@ type Driver interface { // CreatedResources may be non-nil even when an error occurs. Prestart(*ExecContext, *structs.Task) (*PrestartResponse, error) - // Start is used to being task execution - Start(ctx *ExecContext, task *structs.Task) (DriverHandle, error) + // Start is used to begin task execution. If error is nil, + // StartResponse.Handle will be the handle to the task's executor. + // StartResponse.Network may be nil if the task doesn't configure a + // network. + Start(ctx *ExecContext, task *structs.Task) (*StartResponse, error) // Open is used to re-open a handle to a task Open(ctx *ExecContext, handleID string) (DriverHandle, error) @@ -208,7 +221,7 @@ type Driver interface { // // If Cleanup returns a recoverable error it may be retried. On retry // it will be passed the same CreatedResources, so all successfully - // cleaned up resources should be removed. + // cleaned up resources should be removed or handled idempotently. Cleanup(*ExecContext, *CreatedResources) error // Drivers must validate their configuration diff --git a/client/driver/env/env.go b/client/driver/env/env.go index 0abc4fd35..f8d006c9f 100644 --- a/client/driver/env/env.go +++ b/client/driver/env/env.go @@ -8,11 +8,16 @@ import ( "strings" "sync" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/helper" hargs "github.com/hashicorp/nomad/helper/args" "github.com/hashicorp/nomad/nomad/structs" ) +// Network env vars +/* + */ + // A set of environment variables that are exported by each driver. const ( // AllocDir is the environment variable with the path to the alloc directory @@ -60,12 +65,23 @@ const ( // E.g $NOMAD_ADDR_http=127.0.0.1:80 AddrPrefix = "NOMAD_ADDR_" - // IpPrefix is the prefix for passing the IP of a port allocation to a task. + // HostAddrPrefix is the prefix for passing both dynamic and static + // port allocations to tasks with the host's IP address for cases where + // the task advertises a different address. + HostAddrPrefix = "NOMAD_HOST_ADDR_" + + // IpPrefix is the prefix for passing the IP of a port allocation to a + // task. This may not be the host's address depending on task + // configuration. IpPrefix = "NOMAD_IP_" // PortPrefix is the prefix for passing the port allocation to a task. PortPrefix = "NOMAD_PORT_" + // HostIPPrefix is the prefix for passing the host's IP to a task for + // cases where the task advertises a different address. + HostIPPrefix = "NOMAD_HOST_IP_" + // HostPortPrefix is the prefix for passing the host port when a portmap is // specified. HostPortPrefix = "NOMAD_HOST_PORT_" @@ -202,7 +218,6 @@ type Builder struct { region string allocId string allocName string - portMap map[string]string vaultToken string injectVaultToken bool jobName string @@ -210,9 +225,13 @@ type Builder struct { // otherPorts for tasks in the same alloc otherPorts map[string]string + // driverNetwork is the network defined by the driver (or nil if none + // was defined). + driverNetwork *cstructs.DriverNetwork + // network resources from the task; must be lazily turned into env vars - // because portMaps can change after builder creation and affect - // network env vars. + // because portMaps and advertiseIP can change after builder creation + // and affect network env vars. networks []*structs.NetworkResource mu *sync.RWMutex @@ -287,21 +306,8 @@ func (b *Builder) Build() *TaskEnv { nodeAttrs[nodeRegionKey] = b.region } - // Build the addrs for this task - for _, network := range b.networks { - for label, intVal := range network.MapLabelToValues(nil) { - value := strconv.Itoa(intVal) - envMap[fmt.Sprintf("%s%s", IpPrefix, label)] = network.IP - envMap[fmt.Sprintf("%s%s", HostPortPrefix, label)] = value - if forwardedPort, ok := b.portMap[label]; ok { - value = forwardedPort - } - envMap[fmt.Sprintf("%s%s", PortPrefix, label)] = value - ipPort := net.JoinHostPort(network.IP, value) - envMap[fmt.Sprintf("%s%s", AddrPrefix, label)] = ipPort - - } - } + // Build the network related env vars + buildNetworkEnv(envMap, b.networks, b.driverNetwork) // Build the addr of the other tasks for k, v := range b.otherPorts { @@ -455,17 +461,69 @@ func (b *Builder) SetSecretsDir(dir string) *Builder { return b } -func (b *Builder) SetPortMap(portMap map[string]int) *Builder { - newPortMap := make(map[string]string, len(portMap)) - for k, v := range portMap { - newPortMap[k] = strconv.Itoa(v) - } +// SetDriverNetwork defined by the driver. +func (b *Builder) SetDriverNetwork(n *cstructs.DriverNetwork) *Builder { + ncopy := n.Copy() b.mu.Lock() - b.portMap = newPortMap + b.driverNetwork = ncopy b.mu.Unlock() return b } +// buildNetworkEnv env vars in the given map. +// +// Auto: NOMAD_{IP,PORT,ADDR}_