Commit graph

303 commits

Author SHA1 Message Date
Mahmood Ali 54565e3836
Apply suggestions from code review
Co-authored-by: Tim Gross <tgross@hashicorp.com>
2020-05-12 10:56:47 -04:00
Mahmood Ali 06c672cbf2 more tests 2020-05-12 10:14:54 -04:00
Mahmood Ali 0d692f0931 Add a knob to restrict docker runtimes 2020-05-12 10:14:43 -04:00
Juan Larriba a0df437c62
Run Linux Images (LCOW) and Windows Containers side by side (#7850)
Makes it possible to run Linux Containers On Windows with Nomad alongside Windows Containers. Fingerprint prevents only to run Nomad in Windows 10 with Linux Containers
2020-05-04 13:08:47 -04:00
Mahmood Ali dff071c3b9 driver/docker: protect against nil container
Protect against a panic when we attempt to start a container with a name
that conflicts with an existing one.  If the existing one is being
deleted while nomad first attempts to create the container, the
createContainer will fail with `container already exists`, but we get
nil container reference from the `containerByName` lookup, and cause a
crash.

I'm not certain how we get into the state, except for being very
unlucky.  I suspect that this case may be the result of a concurrent
restart or the docker engine API not being fully consistent (e.g. an
earlier call purged the container, but docker didn't free up resources
yet to create a new container with the same name immediately yet).

If that's the case, then re-attempting creation will hopefully succeed,
or we'd at least fail enough times for the alloc to be rescheduled to
another node.
2020-04-19 15:34:45 -04:00
Ben Buzbee 769a3cd8b3 Rename OCIRuntime to Runtime; allow gpu conflicts is they are the same runtime; add conflict test 2020-04-03 12:15:11 -07:00
Ben Buzbee d4f26d1eee Support custom docker runtimes
This enables customers who want to use gvisor and have it configured on their clients.
2020-04-03 11:07:37 -07:00
Mahmood Ali db4c263180
Merge pull request #7554 from benbuzbee/benbuz/fix-seccomp-file
Parse security_opts before sending them to docker daemon
2020-03-31 11:54:17 -04:00
Ben Buzbee 4f6ea87ec4 Parse security_opts before sending them to docker daemon
Fixes #6720

Copy the parsing function from the docker CLI. Docker daemon expects to see JSON for seccomp file not a path.
2020-03-31 08:34:41 -07:00
Mahmood Ali 7225055e80
Merge pull request #7550 from hashicorp/vendor-fsouza-go-docker-client-20200330
Vendor fsouza/go-docker-client update
2020-03-31 08:46:30 -04:00
Mahmood Ali 452a057a8c driver/docker: fix memory swapping
MemorySwappiness can only be set in non-Windows options: https://ci.appveyor.com/project/hashicorp/nomad/builds/31832149

Also fixes https://github.com/hashicorp/nomad/issues/6085
2020-03-30 16:51:16 -04:00
Mahmood Ali 4b6aee24bd
Merge pull request #7508 from greut/docker-drain-timer
docker: drain fingerprint timer
2020-03-30 16:37:53 -04:00
Yoan Blanc c9f6cf385a
Update drivers/docker/fingerprint.go
Co-Authored-By: Mahmood Ali <mahmood@notnoop.com>
2020-03-30 22:11:42 +02:00
Mahmood Ali 8f57f78087 vendors: update fsouza/go-docker-client to v.1.6.3 2020-03-30 15:10:53 -04:00
Mahmood Ali 65d2fb5e32
Merge pull request #7531 from greut/docker-v19.03.8
Docker v19.03.8
2020-03-30 14:45:10 -04:00
Mahmood Ali 254fcd6c06 tests: attempt to deflake TestDockerDriver_PidsLimit
This is an attemp to deflake TestDockerDriver_PidsLimit by having one
more process and ensuring they run for longer.
2020-03-30 07:06:52 -04:00
Mahmood Ali 887292d757
Resolve docker types conflict
Looks like the latest `github.com/docker/docker/registry.ResolveAuthConfig` expect
`github.com/docker/docker/api/types.AuthConfig` rather than
`github.com/docker/cli/cli/config/types.AuthConfig`. The two types are
identical but live in different packages.

Here, we embed `registry.ResolveAuthConfig` from upstream repo, but with
the signature we need.
2020-03-28 17:29:06 +01:00
Yoan Blanc 1d92edbbbe
docker: v19.03.8
Signed-off-by: Yoan Blanc <yoan@dosimple.ch>
2020-03-28 17:29:04 +01:00
Mahmood Ali 6283a44870
Merge pull request #7257 from bbckr/avoid-resolving-dot-in-named-pipe
Avoid resolving dotted segments when host path for volume is named pipe
2020-03-26 16:59:29 -04:00
Yoan Blanc 139a0ae451
fixup! docker: drain fingerprint timer
Signed-off-by: Yoan Blanc <yoan@dosimple.ch>
2020-03-26 16:02:20 +01:00
Yoan Blanc 5f0b3234f0
docker: drain fingerprint timer
Signed-off-by: Yoan Blanc <yoan@dosimple.ch>
2020-03-26 16:00:53 +01:00
Mahmood Ali fd5d033e32
Revert "vendor: fsouza/go-docker-client v1.6.3" 2020-03-23 10:48:47 -04:00
Yoan Blanc ed8dcccb54
docker: disable swap in Windows only
Signed-off-by: Yoan Blanc <yoan@dosimple.ch>
2020-03-23 08:35:09 +01:00
Yoan Blanc d9ea68e807
fixup! fixup! vendor: fsouza/go-docker-client v1.6.3
Signed-off-by: Yoan Blanc <yoan@dosimple.ch>
2020-03-22 10:04:52 +01:00
Yoan Blanc 8e744d1877
vendor: fsouza/go-docker-client v1.6.3
Signed-off-by: Yoan Blanc <yoan@dosimple.ch>
2020-03-22 09:25:46 +01:00
Yoan Blanc c8e69a0427
docker: v18.09.9
Signed-off-by: Yoan Blanc <yoan@dosimple.ch>
2020-03-15 08:23:11 +01:00
bckr 977e7ac8b3 Remove argument passing runtime GOOS 2020-03-03 15:39:43 +01:00
bckr 86a5ff9cb9 Fix too many arguments 2020-03-03 15:38:38 +01:00
Mahmood Ali 88cfe504a0 update grpc
Upgrade grpc to v1.27.1 and protobuf plugins to v1.3.4.
2020-03-03 08:39:54 -05:00
bckr fe6da3df88 Avoid resolving dotted segments when host path for volume is named pipe 2020-03-03 14:00:19 +01:00
John Schlederer 8b35c75206 Making pull activity timeout configurable in Docker
* Making pull activity timeout configurable in Docker plugin config, first pass

* Fixing broken function call

* Fixing broken tests

* Fixing linter suggestion

* Adding documentation on new parameter in Docker plugin config

* Adding unit test

* Setting min value for pull_activity_timeout, making pull activity duration a private var
2019-12-18 12:58:53 +01:00
Mahmood Ali 4a1cc67f58
Merge pull request #6820 from hashicorp/f-skip-docker-logging-knob
driver: allow disabling log collection
2019-12-13 11:41:20 -05:00
Mahmood Ali 46bc3b57e6 address review comments 2019-12-13 11:21:00 -05:00
Seth Hoenig f0c3dca49c tests: swap lib/freeport for tweaked helper/freeport
Copy the updated version of freeport (sdk/freeport), and tweak it for use
in Nomad tests. This means staying below port 10000 to avoid conflicts with
the lib/freeport that is still transitively used by the old version of
consul that we vendor. Also provide implementations to find ephemeral ports
of macOS and Windows environments.

Ports acquired through freeport are supposed to be returned to freeport,
which this change now also introduces. Many tests are modified to include
calls to a cleanup function for Server objects.

This should help quite a bit with some flakey tests, but not all of them.
Our port problems will not go away completely until we upgrade our vendor
version of consul. With Go modules, we'll probably do a 'replace' to swap
out other copies of freeport with the one now in 'nomad/helper/freeport'.
2019-12-09 08:37:32 -06:00
Mahmood Ali 0b7085ba3a driver: allow disabling log collection
Operators commonly have docker logs aggregated using various tools and
don't need nomad to manage their docker logs.  Worse, Nomad uses a
somewhat heavy docker api call to collect them and it seems to cause
problems when a client runs hundreds of log collections.

Here we add a knob to disable log aggregation completely for nomad.
When log collection is disabled, we avoid running logmon and
docker_logger for the docker tasks in this implementation.

The downside here is once disabled, `nomad logs ...` commands and API
no longer return logs and operators must corrolate alloc-ids with their
aggregated log info.

This is meant as a stop gap measure.  Ideally, we'd follow up with at
least two changes:

First, we should optimize behavior when we can such that operators don't
need to disable docker log collection.  Potentially by reverting to
using pre-0.9 syslog aggregation in linux environments, though with
different trade-offs.

Second, when/if logs are disabled, nomad logs endpoints should lookup
docker logs api on demand.  This ensures that the cost of log collection
is paid sparingly.
2019-12-08 14:15:03 -05:00
Nick Ethier 729dd9018c
docker: set default cpu cfs period (#6737)
* docker: set default cpu cfs period

Co-Authored-By: Michael Schurter <mschurter@hashicorp.com>
2019-11-19 19:05:15 -05:00
Tim Gross b1b20cd479
remove misleading networking log line (#6588)
When a job has a task group network, this log line ends up being
misleading if you're trying to debug networking issues. We really only
care about this when there's no port map set, in which case we get the
error returned anyways.
2019-10-30 13:23:33 -04:00
Mahmood Ali fe14993582 docs: Docker driver supports task user option
Also, add a test case.
2019-10-24 14:00:37 -04:00
Mahmood Ali 977b86f924 driver/docker: ensure that defaults are populated
Looks like we may need to pass default literal at each layer to be able,
so defaults are set properly.
2019-10-18 18:27:28 -04:00
Mahmood Ali 1bdfcdcab7 add timeouts for docker reconciler docker calls 2019-10-18 15:31:13 -04:00
Mahmood Ali 414e01b6a6 only set a single label for now
Other labels aren't strictly necessary here, and we may follow up with a
better way to customize.
2019-10-18 15:31:13 -04:00
Mahmood Ali 3aec7b56ea Only start reconciler once in main driver
driver.SetConfig is not appropriate for starting up reconciler
goroutine.  Some ephemeral driver instances are created for validating
config and we ought not to side-effecting goroutines for those.

We currently lack a lifecycle hook to inject these, so I picked the
`Fingerprinter` function for now, and reconciler should only run after
fingerprinter started.

Use `sync.Once` to ensure that we only start reconciler loop once.
2019-10-18 14:43:23 -04:00
Mahmood Ali ac3b555cc8 docker label refactoring and additional tests 2019-10-17 10:45:13 -04:00
Mahmood Ali e24c3fac56 add docker labels 2019-10-17 10:45:12 -04:00
Mahmood Ali 8739cc2a62 refactor reconciler code and address comments 2019-10-17 09:42:23 -04:00
Mahmood Ali c01c6de481 address code review comments 2019-10-17 08:36:02 -04:00
Mahmood Ali 2a63caafba docker: explicit grace period for initial container reconcilation
Ensure we wait for some grace period before killing docker containers
that may have launched in earlier nomad restore.
2019-10-17 08:36:02 -04:00
Mahmood Ali aa59280edc docker: periodically reconcile containers
When running at scale, it's possible that Docker Engine starts
containers successfully but gets wedged in a way where API call fails.
The Docker Engine may remain unavailable for arbitrary long time.

Here, we introduce a periodic reconcilation process that ensures that any
container started by nomad is tracked, and killed if is running
unexpectedly.

Basically, the periodic job inspects any container that isn't tracked in
its handlers.  A creation grace period is used to prevent killing newly
created containers that aren't registered yet.

Also, we aim to avoid killing unrelated containters started by host or
through raw_exec drivers.  The logic is to pattern against containers
environment variables and mounts to infer if they are an alloc docker
container.

Lastly, the periodic job can be disabled to avoid any interference if
need be.
2019-10-17 08:36:01 -04:00
Danielle Lancashire 4fbcc668d0
volumes: Add support for mount propagation
This commit introduces support for configuring mount propagation when
mounting volumes with the `volume_mount` stanza on Linux targets.

Similar to Kubernetes, we expose 3 options for configuring mount
propagation:

- private, which is equivalent to `rprivate` on Linux, which does not allow the
           container to see any new nested mounts after the chroot was created.

- host-to-task, which is equivalent to `rslave` on Linux, which allows new mounts
                that have been created _outside of the container_ to be visible
                inside the container after the chroot is created.

- bidirectional, which is equivalent to `rshared` on Linux, which allows both
                 the container to see new mounts created on the host, but
                 importantly _allows the container to create mounts that are
                 visible in other containers an don the host_

private and host-to-task are safe, but bidirectional mounts can be
dangerous, as if the code inside a container creates a mount, and does
not clean it up before tearing down the container, it can cause bad
things to happen inside the kernel.

To add a layer of safety here, we require that the user has ReadWrite
permissions on the volume before allowing bidirectional mounts, as a
defense in depth / validation case, although creating mounts should also require
a priviliged execution environment inside the container.
2019-10-14 14:09:58 +02:00
Tim Gross d965a15490 driver/networking: don't recreate existing network namespaces 2019-09-25 14:58:17 -04:00
rpramodd 0d09b564fa utils: add missing error info in case of cmd failure (#6355) 2019-09-24 09:33:27 -04:00
Mahmood Ali 1d945994d0 docker: remove containers on creation failures
The docker creation API calls may fail with http errors (e.g. timeout)
even if container was successfully created.

Here, we force remove container if we got unexpected failure.  We
already do this in some error handlers, and this commit updates all
paths.

I stopped short from a more aggressive refactoring, as the code is ripe
for refactoring and would rather do that in another PR.
2019-09-18 08:45:59 -04:00
Mahmood Ali 75ede5a685 add exponential backoff for docker api calls 2019-09-18 08:12:54 -04:00
Mahmood Ali ac329a5e07 retry transient docker errors within function 2019-09-13 15:25:31 -04:00
Mahmood Ali e8d73e3d72 docker: defensive against failed starts
This handles a bug where we may start a container successfully, yet we
fail due to retries and startContainer not being idempotent call.

Here, we ensure that when starting a container fails with 500 error,
the retry succeeds if container was started successfully.
2019-09-13 13:02:35 -04:00
Mahmood Ali 87f0457973 fix qemu and update docker with tests 2019-09-04 11:27:51 -04:00
Jasmine Dahilig 5b6e39b37c fix portmap envvars in docker driver 2019-09-04 11:26:13 -04:00
Michael Schurter 8fe42fccb0
Merge pull request #6000 from Iqoqo/docker-convert-host-paths-to-host-native
driver/docker: convert host bind path to os native
2019-09-03 09:34:56 -07:00
Danielle Lancashire 724586ba1d
docker: Fix driver spec
hclspec.NewLiteral does not quote its values, which caused `3m` to be
parsed as a nonsensical literal which broke the plugin loader during
initialization. By quoting the value here, it starts correctly.
2019-09-03 08:53:37 +02:00
Zhiguang Wang 832df1091b Add default value "3m" to image_delay, making it consistent with docs. 2019-09-02 16:40:00 +08:00
Danielle Lancashire fb63259921
docker: Fix issue where an exec may never timeout 2019-08-16 15:40:03 +02:00
Michael Schurter 83dbac65b2 docker: reword FromSlash(hostPath) comment 2019-08-12 14:38:31 -07:00
ilya guterman 92ce8a0a49 Update utils.go 2019-08-12 19:31:34 +03:00
Ilya Guterman c4b4d7fa43 add comment 2019-08-12 19:31:33 +03:00
Ilya Guterman 52aab40fb3 driver/docker: convert host bind path to os native
relative mounting can be specified using backslashes or forward slashes.
so no prior knowledge of host OS is needed for relative volumes mounting
2019-08-12 19:31:33 +03:00
Michael Schurter aeeec126f5
Merge pull request #5999 from Iqoqo/use-default-network-for-docker
driver/docker: use default network mode
2019-08-01 09:58:12 -07:00
Ilya Guterman a4931ba25b driver/docker: support unix destination mount path in windows
This reverts commit a6c96eade56f0b8880edbec3c4392934492f09bf.
2019-08-01 19:54:08 +03:00
Ilya Guterman 1e6ea0af8c driver/docker: use default network mode
fallback to docker default network mode instead of explicit bridge for linux
or nat for windows
2019-07-31 21:07:46 +03:00
Nick Ethier 1dae42ab81
docker: allow configuration of infra image 2019-07-31 01:04:07 -04:00
Nick Ethier 0e40063092
docker: add nil check on network isolation spec 2019-07-31 01:03:21 -04:00
Nick Ethier f50fa7ef08
docker: fix driver test from changed func args 2019-07-31 01:03:20 -04:00
Nick Ethier d752734719
docker: add additional commens 2019-07-31 01:03:20 -04:00
Nick Ethier 1fc5f86a7c
docker: support shared network namespaces 2019-07-31 01:03:20 -04:00
Nick Ethier 2d60ef64d9
plugins/driver: make DriverNetworkManager interface optional 2019-07-31 01:03:19 -04:00
Nick Ethier 548f78ef15
ar: initial driver based network management 2019-07-31 01:03:17 -04:00
Michael Schurter ea7fac7bcf
Revert "driver/docker: support unix destination mount path in windows" 2019-07-29 09:09:54 -07:00
Ilya Guterman cb2284fc3a driver/docker: support unix destination mount path in windows 2019-07-24 11:57:28 +03:00
Jasmine Dahilig 1c1e81b294
Merge pull request #5846 from hashicorp/f-docker-log-constraints
add log rotation to docker driver log defaults
2019-07-03 10:17:19 -07:00
Jasmine Dahilig cece83dd9c default to json-file log rotation for docker driver 2019-07-03 09:04:45 -07:00
Mahmood Ali 6c245c9b6a
Merge pull request #5811 from cloudbuy/b-win32-volume-split
lift code from docker/volume/mounts for splitting windows volumes
2019-06-18 21:19:15 -04:00
Mahmood Ali ac64509c59 comment on use of init() for plugin handlers 2019-06-18 20:54:55 -04:00
Damien Churchill 9ee17f32c0 run new file through goimports 2019-06-18 08:35:25 +01:00
Damien Churchill dba5bd96cd run gofmt over the new file 2019-06-18 08:35:25 +01:00
Damien Churchill 4d7d352d7e drivers/docker: move lifted code out to separate file and link the source & license 2019-06-18 08:35:25 +01:00
Damien Churchill 70daca3395 lift code from docker/volume/mounts for splitting windows volumes
Using the API as provided from the `mounts` package imposes validation
on the `src:dest` which shouldn't be performed at this time. To workaround
that lift the internal code from that library required to only perform
the split.
2019-06-18 08:35:25 +01:00
Mahmood Ali 962921f86c Use init to handle plugin invocation
Currently, nomad "plugin" processes (e.g. executor, logmon, docker_logger) are started as CLI
commands to be handled by command CLI framework.  Plugin launchers use
`discover.NomadBinary()` to identify the binary and start it.

This has few downsides: The trivial one is that when running tests, one
must re-compile the nomad binary as the tests need to invoke the nomad
executable to start plugin.  This is frequently overlooked, resulting in
puzzlement.

The more significant issue with `executor` in particular is in relation
to external driver:

* Plugin must identify the path of invoking nomad binary, which is not
trivial; `discvoer.NomadBinary()` now returns the path to the plugin
rather than to nomad, preventing external drivers from launching
executors.

* The external driver may get a different version of executor than it
expects (specially if we make a binary incompatible change in future).

This commit addresses both downside by having the plugin invocation
handling through an `init()` call, similar to how libcontainer init
handler is done in [1] and recommened by libcontainer [2].  `init()`
will be invoked and handled properly in tests and external drivers.

For external drivers, this change will cause external drivers to launch
the executor that's compiled against.

There a are a couple of downsides to this approach:
* These specific packages (i.e executor, logmon, and dockerlog) need to
be careful in use of `init()`, package initializers.  Must avoid having
command execution rely on any other init in the package.  I prefixed
files with `z_` (golang processes files in lexical order), but ensured
we don't depend on order.
* The command handling is spread in multiple packages making it a bit
less obvious how plugin starts are handled.

[1] drivers/shared/executor/libcontainer_nsenter_linux.go
[2] eb4aeed24f/libcontainer (using-libcontainer)
2019-06-13 16:48:01 -04:00
Chris Baker 3ca97d52db docker/driver: downgraded log level for error in DestroyTask 2019-06-03 21:21:32 +00:00
Chris Baker 2af897c76f drivers/docker: modify container/image cleanup to be robust to containers removed out of band 2019-06-03 19:52:28 +00:00
Chris Baker be6c6e8ce1 docker/tests:
- modified tests to cleanup now that RemoveContainer isn't in StartTask
- fix some broken tests by removing docker images/containers before test
2019-06-03 19:05:08 +00:00
Chris Baker 9442c26cff docker: DestroyTask was not cleaning up Docker images because it was erroring early due to an attempt to inspect an image that had already been removed 2019-06-03 19:04:27 +00:00
Mahmood Ali 13c83ee38e drivers/docker: implement streaming exec 2019-05-09 16:49:08 -04:00
Mahmood Ali 0ee771b020 driver/docker: Support volumes field in Windows
Support Docker `volumes` field in Windows.  Previously, volumes parser
assumed some Unix-ism (e.g. didn't expect `:` in mount paths).
Here, we use the Docker parser to identify host and container paths.

Docker parsers use different validation logic from our previous unix
implementation: Docker parser accepts single path as a volume entry
(parsing it as a container path with auto-created volume) and enforces
additional checks (e.g. validity of mode).  Thereforce, I opted to use
Docker parser only for Windows, and keep Nomad's linux parser to
preserve current behavior.
2019-04-25 09:02:44 -04:00
Mahmood Ali df2b579c6b driver/docker: collect tty container logs
Fixes https://github.com/hashicorp/nomad/issues/5475

When container is a tty container, we need to get raw terminal output
without any additional processing.
2019-04-24 22:01:51 -04:00
Danielle Lancashire a096a7f112 Switch to pre-0.9 behaviour for handling volumes
In Nomad 0.9, we made volume driver handling the same for `""`, and
`"local"` volumes. Prior to Nomad 0.9 however these had slightly different
behaviour for relative paths and named volumes.

Prior to 0.9 the empty string would expand relative paths within the task
dir, and `"local"` volumes that are not absolute paths would be treated
as docker named volumes.

This commit reverts to the previous behaviour as follows:

| Nomad Version | Driver  |   Volume Spec    | Behaviour                 |
|-------------------------------------------------------------------------
| all           | ""      | testing:/testing | allocdir/testing          |
| 0.8.7         | "local" | testing:/testing | "testing" as named volume |
| 0.9.0         | "local" | testing:/testing | allocdir/testing          |
| 0.9.1         | "local" | testing:/testing | "testing" as named volume |
2019-04-18 14:28:45 +02:00
Mahmood Ali 01a13a0947 locking and opening streams in goroutine comment 2019-04-16 11:02:19 -04:00
Mahmood Ali 357b86adc3 open fifo on background goroutine 2019-04-15 21:20:09 -04:00
Mahmood Ali 9e48fab225
Merge pull request #5537 from hashicorp/b-nonvidia-flag
Allow compiling without nvidia integration
2019-04-10 13:40:55 -04:00
Mahmood Ali b4d84fd6a9 Allow compiling without nvidia integration
nvidia library use of dynamic library seems to conflict with alpine and
musl based OSes.  This adds a `nonvidia` tag to allow compiling nomad
for alpine images.

The nomad releases currently only support glibc based OS environments,
so we default to compiling with nvidia.
2019-04-10 09:19:12 -04:00
Mahmood Ali f7d39d6186 ci: move docker unix-y test
Fix AppVeyor failing builds, by moving docker image url test to run on unix
systems only.  The used paused image is a linux image only, not
available on Windows.
2019-04-09 19:59:58 -04:00
Nick Ethier 4bbdb80b73
drivers/docker: fix image name handleing when prefixed with https:// 2019-04-04 22:10:18 -04:00