Fix: Docker container are using swap

[fho]

        docker: fix: disallow docker-containers to use swap
        
        According to the nomad documentation docker containers cannot use swap.
        This was not the case. Container were swapping.
        
        The swappiness value for docker containers was unset. This had the
        effect that the swappiness value is inherited from the parent cgroup,
        where swappiness is enabled by default.
        
        Setting memory.memsw.limit_in_bytes (MemorySwap) to the same value then
        memory.limit_in_bytes (task.Resources.LinuxResources.MemoryLimitBytes)
        is not sufficient to disable swapping of a container.
        If the host system runs out of memory, the container will start swapping
        to make more RAM available for other processes if swappiness is not set
        to 0.
        
-------------------------------------------------------------------------------

        drivers: adapt to changes in go-dockerclient API
        
        The API of go-dockerclient changed in the new version.
        PidsLimit and MemorySwappiness are now pointers.
        
        Because MemorySwappiness is a pointer and the default is nil, we do not
        have to assign it anymore to deactivate setting swappiness.
        
-------------------------------------------------------------------------------

        vendor: update go-dockerclient to 1.4.1
        
        The go-dockerclient library that was used did not support to disable
        swapping for a docker-container.
        When MemorySwappiness was set to 0, the value was not passed to the
        docker-client. Therefore the swappiness setting of the parent cgroup was
        used.
        
        Version 1.4.1 contains the change https://github.com/fsouza/go-dockerclient/pull/776.
        MemorySwappiness is changed to a pointer type. If MemorySwappiness is
        null, the value will not be passed. If it's 0, swappiness is set to 0.
        
        This will allow to disable swapping for docker containers.

        

This fixes the issue: #6085

To reproduce that a docker container swap when memory.memsw.limit_in_byte = memory.limit_in_bytes, do the following:

• Start a docker container with the same limit for --memory-swap and --memory, set swappiness to the max. value to make swapping happen faster:
  docker run -i -t --memory-swap=1G -m 1G --memory-swappiness=100 debian:buster bash
• Install the stress utility in the container: apt-get update && apt-get install stress -y
• Run the memory stress test to allocate 1G of memory: stress -m 1 --vm-bytes 1048576000
• On the host system, allocate memory until it gets scarce by e.g. running also stress on the host with --vm-bytes set to all or little bit then the available RAM
• monitor the memory.stat file of the docker container cgroup, the swap value will start to show a value != 0

[hashicorp-cla]

All committers have signed the CLA.

[shantanugadgil]

How about disabling SWAP as an option, and let the user choose if they want to let the task swap.

I am all for letting docker containers to use swap.

At the most, make one the default behavior but still let the user choose what to do!

[fho]

@shantanugadgil

How about disabling SWAP as an option, and let the user choose if they want to let the task swap.

I am all for letting docker containers to use swap.

At the most, make one the default behavior but still let the user choose what to do!

I totally agree and also favor the approach to have an option to enable/disable swapping per job.
This is a bigger change though which I can not implement quickly. It could be implemented in a following PR and would also need to be implemented for other drivers like exec.

With this PR I want to fix the discrepancy that according to the nomad documentation containers do not swap (https://www.nomadproject.io/docs/drivers/docker.html#memory) but they do.
The behavior that is documented seem to me the one that nomad should follow.

[fho]

ci/circleci: lint-go failed because:

#!/bin/bash -eo pipefail
make check

==> Linting source code...
make: *** [GNUmakefile:170: check] Terminated
Too long with no output (exceeded 10m0s)

Could somebody rerun the CI check?

[nickethier]

@fho could you run make vendorfmt to format the vendor.json file. This makes it much easier to inspect the diff.

[fho]

@nickethier yes, done

[fho]

I force pushed to improve the commit messages

[fho]

@nickethier could you provide feedback for the PR? thanks! :-)
CI is currently failing because the lint checks timed out and tests failed because some testcases where skipped because of test should be run as non-root user. Both seems not related to the changes.

[zonnie]

This is really hurting us with 0.9.x and 0.10.x - can we somehow move this forward ? 😄
Thanks !

[notnoop]

Superceeded by #7550 . Thanks @sirkjohannsen - the PR was very descriptive and helped me understand the situation more!

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.