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

driver/docker: enable setting hard/soft memory limits #8087

Merged
merged 7 commits into from
Jun 1, 2020
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ IMPROVEMENTS:
* csi: Return better error messages [[GH-7984](https://github.com/hashicorp/nomad/issues/7984)] [[GH-8030](https://github.com/hashicorp/nomad/issues/8030)]
* csi: Move volume claim releases out of evaluation workers [[GH-8021](https://github.com/hashicorp/nomad/issues/8021)]
* csi: Added support for `VolumeContext` and `VolumeParameters` [[GH-7957](https://github.com/hashicorp/nomad/issues/7957)]
* driver/docker: Added support for `memory_hard_limit` configuration in docker task driver [[GH-2093](https://github.com/hashicorp/nomad/issues/2093)]
* logging: Remove spurious error log on task shutdown [[GH-8028](https://github.com/hashicorp/nomad/issues/8028)]

BUG FIXES:
Expand Down
4 changes: 3 additions & 1 deletion drivers/docker/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,8 @@ var (
"driver": hclspec.NewAttr("driver", "string", false),
"config": hclspec.NewAttr("config", "list(map(string))", false),
})),
"mac_address": hclspec.NewAttr("mac_address", "string", false),
"mac_address": hclspec.NewAttr("mac_address", "string", false),
"memory_hard_limit": hclspec.NewAttr("memory_hard_limit", "number", false),
"mounts": hclspec.NewBlockList("mounts", hclspec.NewObject(map[string]*hclspec.Spec{
"type": hclspec.NewDefault(
hclspec.NewAttr("type", "string", false),
Expand Down Expand Up @@ -408,6 +409,7 @@ type TaskConfig struct {
LoadImage string `codec:"load"`
Logging DockerLogging `codec:"logging"`
MacAddress string `codec:"mac_address"`
MemoryHardLimit int64 `codec:"memory_hard_limit"`
Mounts []DockerMount `codec:"mounts"`
NetworkAliases []string `codec:"network_aliases"`
NetworkMode string `codec:"network_mode"`
Expand Down
6 changes: 3 additions & 3 deletions drivers/docker/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ config {
}
}
mac_address = "02:42:ac:11:00:02"
memory_hard_limit = 512
mounts = [
{
type = "bind"
Expand Down Expand Up @@ -349,7 +350,8 @@ config {
"max-file": "3",
"max-size": "10m",
}},
MacAddress: "02:42:ac:11:00:02",
MacAddress: "02:42:ac:11:00:02",
MemoryHardLimit: 512,
Mounts: []DockerMount{
{
Type: "bind",
Expand Down Expand Up @@ -524,7 +526,6 @@ func TestConfig_InternalCapabilities(t *testing.T) {
require.Equal(t, c.expected, d.InternalCapabilities())
})
}

}

func TestConfig_DriverConfig_PullActivityTimeout(t *testing.T) {
Expand Down Expand Up @@ -582,5 +583,4 @@ func TestConfig_DriverConfig_AllowRuntimes(t *testing.T) {
require.Equal(t, c.expected, d.config.allowRuntimes)
})
}

}
33 changes: 31 additions & 2 deletions drivers/docker/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,30 @@ func parseSecurityOpts(securityOpts []string) ([]string, error) {
return securityOpts, nil
}

// memoryLimits computes the memory and memory_reservation values passed along to
// the docker host config. These fields represent hard and soft memory limits from
// docker's perspective, respectively.
//
// The memory field on the task configuration can be interpreted as a hard or soft
// limit. Before Nomad v0.11.3, it was always a hard limit. Now, it is interpreted
// as a soft limit if the memory_hard_limit value is configured on the docker
// task driver configuration. When memory_hard_limit is set, the docker host
// config is configured such that the memory field is equal to memory_hard_limit
// value, and the memory_reservation field is set to the task driver memory value.
//
// If memory_hard_limit is not set (i.e. zero value), then the memory field of
// the task resource config is interpreted as a hard limit. In this case both the
// memory is set to the task resource memory value and memory_reservation is left
// unset.
//
// Returns (memory (hard), memory_reservation (soft)) values in bytes.
func (_ *Driver) memoryLimits(driverHardLimitMB, taskMemoryLimitBytes int64) (int64, int64) {
if driverHardLimitMB <= 0 {
return taskMemoryLimitBytes, 0
}
return driverHardLimitMB * 1024 * 1024, taskMemoryLimitBytes
}

func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *TaskConfig,
imageID string) (docker.CreateContainerOptions, error) {

Expand Down Expand Up @@ -772,8 +796,12 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T
return c, fmt.Errorf("requested runtime %q is not allowed", containerRuntime)
}

memory, memoryReservation := d.memoryLimits(driverConfig.MemoryHardLimit, task.Resources.LinuxResources.MemoryLimitBytes)

hostConfig := &docker.HostConfig{
Memory: task.Resources.LinuxResources.MemoryLimitBytes,
Memory: memory, // hard limit
MemoryReservation: memoryReservation, // soft limit

CPUShares: task.Resources.LinuxResources.CPUShares,

// Binds are used to mount a host volume into the container. We mount a
Expand Down Expand Up @@ -837,7 +865,8 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T
}
}

logger.Debug("configured resources", "memory", hostConfig.Memory,
logger.Debug("configured resources",
"memory", hostConfig.Memory, "memory_reservation", hostConfig.MemoryReservation,
"cpu_shares", hostConfig.CPUShares, "cpu_quota", hostConfig.CPUQuota,
"cpu_period", hostConfig.CPUPeriod)

Expand Down
42 changes: 42 additions & 0 deletions drivers/docker/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1484,6 +1484,32 @@ func TestDockerDriver_DNS(t *testing.T) {
require.Exactly(t, cfg.DNSOptions, container.HostConfig.DNSOptions)
}

func TestDockerDriver_MemoryHardLimit(t *testing.T) {
if !tu.IsCI() {
t.Parallel()
}
testutil.DockerCompatible(t)
if runtime.GOOS == "windows" {
t.Skip("Windows does not support MemoryReservation")
}

task, cfg, ports := dockerTask(t)
defer freeport.Return(ports)

cfg.MemoryHardLimit = 300
require.NoError(t, task.EncodeConcreteDriverConfig(cfg))

client, d, handle, cleanup := dockerSetup(t, task, nil)
defer cleanup()
require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))

container, err := client.InspectContainer(handle.containerID)
require.NoError(t, err)

require.Equal(t, task.Resources.LinuxResources.MemoryLimitBytes, container.HostConfig.MemoryReservation)
require.Equal(t, cfg.MemoryHardLimit*1024*1024, container.HostConfig.Memory)
}

func TestDockerDriver_MACAddress(t *testing.T) {
if !tu.IsCI() {
t.Parallel()
Expand Down Expand Up @@ -2681,3 +2707,19 @@ func TestDockerDriver_CreateContainerConfig_CPUHardLimit(t *testing.T) {
require.NotZero(t, c.HostConfig.CPUQuota)
require.NotZero(t, c.HostConfig.CPUPeriod)
}

func TestDockerDriver_memoryLimits(t *testing.T) {
t.Parallel()

t.Run("driver hard limit not set", func(t *testing.T) {
memory, memoryReservation := new(Driver).memoryLimits(0, 256*1024*1024)
require.Equal(t, int64(256*1024*1024), memory)
require.Equal(t, int64(0), memoryReservation)
})

t.Run("driver hard limit is set", func(t *testing.T) {
memory, memoryReservation := new(Driver).memoryLimits(512, 256*1024*1024)
require.Equal(t, int64(512*1024*1024), memory)
require.Equal(t, int64(256*1024*1024), memoryReservation)
})
}
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there's an integration test we could piggy back on to ensure that Docker is interpreting our limits as we expect. Similar to this: https://github.com/hashicorp/nomad/blob/v0.11.2/drivers/docker/driver_test.go#L1406-L1454

Not a big deal or blocker.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh, I was looking for something like these and missed them. Added!

8 changes: 8 additions & 0 deletions website/pages/docs/drivers/docker.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,14 @@ The `docker` driver supports the following configuration in the job spec. Only
- `mac_address` - (Optional) The MAC address for the container to use (e.g.
"02:68:b3:29:da:98").

- `memory_hard_limit` - (Optional) The maximum allowable amount of memory used
(megabytes) by the container. If set, the [`memory`](/docs/job-specification/resources#memory)
parameter of the task resource configuration becomes a soft limit passed to the
docker driver as [`--memory_reservation`](https://docs.docker.com/config/containers/resource_constraints/#limit-a-containers-access-to-memory),
and `memory_hard_limit` is passed as the [`--memory`](https://docs.docker.com/config/containers/resource_constraints/#limit-a-containers-access-to-memory)
hard limit. When the host is under memory pressure, the behavior of soft limit
activation is governed by the [Kernel](https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt).

- `network_aliases` - (Optional) A list of network-scoped aliases, provide a way for a
container to be discovered by an alternate name by any other container within
the scope of a particular network. Network-scoped alias is supported only for
Expand Down