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 configuration of Docker hostnames in bridge mode #11173

Merged
merged 8 commits into from
Sep 16, 2021
Merged
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions api/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ type NetworkResource struct {
DNS *DNSConfig `hcl:"dns,block"`
ReservedPorts []Port `hcl:"reserved_ports,block"`
DynamicPorts []Port `hcl:"port,block"`
Hostname string `hcl:"hostname,optional"`

// COMPAT(0.13)
// XXX Deprecated. Please do not use. The field will be removed in Nomad
Expand Down
9 changes: 7 additions & 2 deletions client/allocrunner/alloc_runner_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ func (ar *allocRunner) initRunnerHooks(config *clientconfig.Config) error {
return fmt.Errorf("failed to initialize network configurator: %v", err)
}

// Create a new taskenv.TaskEnv which is used by networkHook and
// groupServiceHook for interpolation.
envBuilder := taskenv.NewBuilder(
config.Node, ar.Alloc(), nil, config.Region).SetAllocDir(ar.allocDir.AllocDir)

// Create the alloc directory hook. This is run first to ensure the
// directory path exists for other hooks.
alloc := ar.Alloc()
Expand All @@ -142,13 +147,13 @@ func (ar *allocRunner) initRunnerHooks(config *clientconfig.Config) error {
newUpstreamAllocsHook(hookLogger, ar.prevAllocWatcher),
newDiskMigrationHook(hookLogger, ar.prevAllocMigrator, ar.allocDir),
newAllocHealthWatcherHook(hookLogger, alloc, hs, ar.Listener(), ar.consulClient),
newNetworkHook(hookLogger, ns, alloc, nm, nc, ar),
newNetworkHook(hookLogger, ns, alloc, nm, nc, ar, envBuilder),
newGroupServiceHook(groupServiceHookConfig{
alloc: alloc,
consul: ar.consulClient,
consulNamespace: alloc.ConsulNamespace(),
restarter: ar,
taskEnvBuilder: taskenv.NewBuilder(config.Node, ar.Alloc(), nil, config.Region).SetAllocDir(ar.allocDir.AllocDir),
taskEnvBuilder: envBuilder,
networkStatusGetter: ar,
logger: hookLogger,
}),
Expand Down
68 changes: 58 additions & 10 deletions client/allocrunner/network_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,26 @@ import (
"context"
"fmt"

"github.com/asaskevich/govalidator"
hclog "github.com/hashicorp/go-hclog"
"github.com/hashicorp/nomad/client/taskenv"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/plugins/drivers"
)

// We create a pause container to own the network namespace, and the
// NetworkIsolationSpec we get back from CreateNetwork has this label set as
// the container ID. We'll use this to generate a hostname for the task.
const dockerNetSpecLabelKey = "docker_sandbox_container_id"
const (
// dockerNetSpecLabelKey is the label added when we create a pause
// container to own the network namespace, and the NetworkIsolationSpec we
// get back from CreateNetwork has this label set as the container ID.
// We'll use this to generate a hostname for the task in the event the user
// did not specify a custom one. Please see dockerNetSpecHostnameKey.
dockerNetSpecLabelKey = "docker_sandbox_container_id"

// dockerNetSpecHostnameKey is the label added when we create a pause
// container and the task group network include a user supplied hostname
// parameter.
dockerNetSpecHostnameKey = "docker_sandbox_hostname"
)

type networkIsolationSetter interface {
SetNetworkIsolation(*drivers.NetworkIsolationSpec)
Expand Down Expand Up @@ -61,6 +72,10 @@ type networkHook struct {
// the alloc network has been created
networkConfigurator NetworkConfigurator

// taskEnvBuilder is used to perform interpolation within the network
// blocks.
taskEnvBuilder *taskenv.Builder

logger hclog.Logger
}

Expand All @@ -69,13 +84,16 @@ func newNetworkHook(logger hclog.Logger,
alloc *structs.Allocation,
netManager drivers.DriverNetworkManager,
netConfigurator NetworkConfigurator,
networkStatusSetter networkStatusSetter) *networkHook {
networkStatusSetter networkStatusSetter,
envBuilder *taskenv.Builder,
) *networkHook {
return &networkHook{
isolationSetter: ns,
networkStatusSetter: networkStatusSetter,
alloc: alloc,
manager: netManager,
networkConfigurator: netConfigurator,
taskEnvBuilder: envBuilder,
logger: logger,
}
}
Expand All @@ -95,8 +113,25 @@ func (h *networkHook) Prerun() error {
return nil
}

spec, created, err := h.manager.CreateNetwork(h.alloc.ID)
// Perform our networks block interpolation.
interpolatedNetworks := taskenv.InterpolateNetworks(h.taskEnvBuilder.Build(), tg.Networks)
jrasell marked this conversation as resolved.
Show resolved Hide resolved

// Interpolated values need to be validated. It is also possible a user
// supplied hostname avoids the validation on job registrations because it
// looks like it includes interpolation, when it doesn't.
if interpolatedNetworks[0].Hostname != "" {
if !govalidator.IsDNSName(interpolatedNetworks[0].Hostname) {
return fmt.Errorf("network hostname %q is not a valid DNS name", interpolatedNetworks[0].Hostname)
}
}

// Our network create request.
networkCreateReq := drivers.NetworkCreateRequest{
AllocID: h.alloc.ID,
Hostname: interpolatedNetworks[0].Hostname,
}

spec, created, err := h.manager.CreateNetwork(&networkCreateReq)
if err != nil {
return fmt.Errorf("failed to create network for alloc: %v", err)
}
Expand All @@ -111,18 +146,31 @@ func (h *networkHook) Prerun() error {
if err != nil {
return fmt.Errorf("failed to configure networking for alloc: %v", err)
}
if hostname, ok := spec.Labels[dockerNetSpecLabelKey]; ok {

// If the driver set the sandbox hostname label, then we will use that
// to set the HostsConfig.Hostname. Otherwise, identify the sandbox
// container ID which will have been used to set the network namespace
// hostname.
if hostname, ok := spec.Labels[dockerNetSpecHostnameKey]; ok {
h.spec.HostsConfig = &drivers.HostsConfig{
Address: status.Address,
Hostname: hostname,
}
} else if hostname, ok := spec.Labels[dockerNetSpecLabelKey]; ok {

// the docker_sandbox_container_id is the full ID of the pause
// container, whereas we want the shortened name that dockerd sets
// as the pause container's hostname.
if len(hostname) > 12 {
// the docker_sandbox_container_id is the full ID of the pause
// container, whereas we want the shortened name that dockerd
// sets as the pause container's hostname
hostname = hostname[:12]
}

h.spec.HostsConfig = &drivers.HostsConfig{
Address: status.Address,
Hostname: hostname,
}
}

h.networkStatusSetter.SetNetworkStatus(status)
}
return nil
Expand Down
12 changes: 7 additions & 5 deletions client/allocrunner/network_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"github.com/hashicorp/nomad/client/allocrunner/interfaces"
"github.com/hashicorp/nomad/client/taskenv"
"github.com/hashicorp/nomad/helper/testlog"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
Expand Down Expand Up @@ -56,8 +57,8 @@ func TestNetworkHook_Prerun_Postrun(t *testing.T) {
destroyCalled := false
nm := &testutils.MockDriver{
MockNetworkManager: testutils.MockNetworkManager{
CreateNetworkF: func(allocID string) (*drivers.NetworkIsolationSpec, bool, error) {
require.Equal(t, alloc.ID, allocID)
CreateNetworkF: func(req *drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error) {
require.Equal(t, alloc.ID, req.AllocID)
return spec, false, nil
},

Expand All @@ -79,8 +80,10 @@ func TestNetworkHook_Prerun_Postrun(t *testing.T) {
}
require := require.New(t)

envBuilder := taskenv.NewBuilder(mock.Node(), alloc, nil, alloc.Job.Region)

logger := testlog.HCLogger(t)
hook := newNetworkHook(logger, setter, alloc, nm, &hostNetworkConfigurator{}, statusSetter)
hook := newNetworkHook(logger, setter, alloc, nm, &hostNetworkConfigurator{}, statusSetter, envBuilder)
require.NoError(hook.Prerun())
require.True(setter.called)
require.False(destroyCalled)
Expand All @@ -91,11 +94,10 @@ func TestNetworkHook_Prerun_Postrun(t *testing.T) {
setter.called = false
destroyCalled = false
alloc.Job.TaskGroups[0].Networks[0].Mode = "host"
hook = newNetworkHook(logger, setter, alloc, nm, &hostNetworkConfigurator{}, statusSetter)
hook = newNetworkHook(logger, setter, alloc, nm, &hostNetworkConfigurator{}, statusSetter, envBuilder)
require.NoError(hook.Prerun())
require.False(setter.called)
require.False(destroyCalled)
require.NoError(hook.Postrun())
require.False(destroyCalled)

}
28 changes: 24 additions & 4 deletions client/allocrunner/network_manager_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,18 @@ func newNetworkManager(alloc *structs.Allocation, driverManager drivermanager.Ma
tgNetMode = tg.Networks[0].Mode
}

groupIsolationMode := netModeToIsolationMode(tgNetMode)

// Setting the hostname is only possible where the task groups networking
// mode is group; meaning bridge or none.
if len(tg.Networks) > 0 &&
(groupIsolationMode != drivers.NetIsolationModeGroup && tg.Networks[0].Hostname != "") {
return nil, fmt.Errorf("hostname cannot be set on task group using %q networking mode",
groupIsolationMode)
}

// networkInitiator tracks the task driver which needs to create the network
// to check for multiple drivers needing the create the network
// to check for multiple drivers needing to create the network.
var networkInitiator string

// driverCaps tracks which drivers we've checked capabilities for so as not
Expand Down Expand Up @@ -80,6 +90,13 @@ func newNetworkManager(alloc *structs.Allocation, driverManager drivermanager.Ma

nm = netManager
networkInitiator = task.Name
} else if tg.Networks[0].Hostname != "" {
// TODO jrasell: remove once the default linux network manager
// supports setting the hostname in bridged mode. This currently
// indicates only Docker supports this, which is true unless a
// custom driver can which means this check still holds as true as
// we can tell.
return nil, fmt.Errorf("hostname is not currently supported on driver %s", task.Driver)
jrasell marked this conversation as resolved.
Show resolved Hide resolved
}

// mark this driver's capabilities as checked
Expand All @@ -92,13 +109,16 @@ func newNetworkManager(alloc *structs.Allocation, driverManager drivermanager.Ma
// defaultNetworkManager creates a network namespace for the alloc
type defaultNetworkManager struct{}

func (*defaultNetworkManager) CreateNetwork(allocID string) (*drivers.NetworkIsolationSpec, bool, error) {
netns, err := nsutil.NewNS(allocID)
// CreateNetwork is the CreateNetwork implementation of the
// drivers.DriverNetworkManager interface function. It does not currently
// support setting the hostname of the network namespace.
func (*defaultNetworkManager) CreateNetwork(createSpec *drivers.NetworkCreateRequest) (*drivers.NetworkIsolationSpec, bool, error) {
netns, err := nsutil.NewNS(createSpec.AllocID)
if err != nil {
// when a client restarts, the namespace will already exist and
// there will be a namespace file in use by the task process
if e, ok := err.(*os.PathError); ok && e.Err == syscall.EPERM {
nsPath := path.Join(nsutil.NetNSRunDir, allocID)
nsPath := path.Join(nsutil.NetNSRunDir, createSpec.AllocID)
_, err := os.Stat(nsPath)
if err == nil {
return nil, false, nil
Expand Down
86 changes: 86 additions & 0 deletions client/allocrunner/network_manager_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,92 @@ func TestNewNetworkManager(t *testing.T) {
err: true,
errContains: "want to initiate networking but only one",
},
{
name: "hostname set in bridged mode",
alloc: &structs.Allocation{
TaskGroup: "group",
Job: &structs.Job{
TaskGroups: []*structs.TaskGroup{
{
Name: "group",
Networks: []*structs.NetworkResource{
{
Mode: "bridge",
Hostname: "foobar",
},
},
Tasks: []*structs.Task{
{
Name: "task1",
Driver: "mustinit1",
Resources: &structs.Resources{},
},
},
},
},
},
},
mustInit: true,
err: false,
},
{
name: "hostname set in host mode",
alloc: &structs.Allocation{
TaskGroup: "group",
Job: &structs.Job{
TaskGroups: []*structs.TaskGroup{
{
Name: "group",
Networks: []*structs.NetworkResource{
{
Mode: "host",
Hostname: "foobar",
},
},
Tasks: []*structs.Task{
{
Name: "task1",
Driver: "group1",
Resources: &structs.Resources{},
},
},
},
},
},
},
mustInit: false,
err: true,
errContains: `hostname cannot be set on task group using "host" networking mode`,
},
{
name: "hostname set using exec driver",
alloc: &structs.Allocation{
TaskGroup: "group",
Job: &structs.Job{
TaskGroups: []*structs.TaskGroup{
{
Name: "group",
Networks: []*structs.NetworkResource{
{
Mode: "bridge",
Hostname: "foobar",
},
},
Tasks: []*structs.Task{
{
Name: "task1",
Driver: "group1",
Resources: &structs.Resources{},
},
},
},
},
},
},
mustInit: false,
err: true,
errContains: "hostname is not currently supported on driver group1",
},
} {
t.Run(tc.name, func(t *testing.T) {
require := require.New(t)
Expand Down
29 changes: 29 additions & 0 deletions client/taskenv/network.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package taskenv

import (
"github.com/hashicorp/nomad/nomad/structs"
)

// InterpolateNetworks returns an interpolated copy of the task group networks
// with values from the task's environment.
//
// Current interoperable fields:
// - Hostname
func InterpolateNetworks(taskEnv *TaskEnv, networks structs.Networks) structs.Networks {

// Guard against not having a valid taskEnv. This can be the case if the
// PreKilling or Exited hook is run before Poststart.
if taskEnv == nil || networks == nil {
return nil
}

// Create a copy of the networks array, so we can manipulate the copy.
interpolated := networks.Copy()

// Iterate the copy and perform the interpolation.
for i := range interpolated {
interpolated[i].Hostname = taskEnv.ReplaceEnv(interpolated[i].Hostname)
}

return interpolated
}
Loading