Commit graph

176 commits

Author SHA1 Message Date
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
Michael Schurter ff639f9ccc docker: improve stats names and comments 2019-04-02 09:18:38 -07:00
Mahmood Ali e2723399f3
Update drivers/docker/stats.go comment
Co-Authored-By: schmichael <michael.schurter@gmail.com>
2019-04-02 09:09:17 -07:00
Michael Schurter df3467a6ce docker: fix send after close panic in stats
destCh was being written to by one goroutine and closed by another
goroutine. This panic occurred in Travis:

```
=== FAIL: drivers/docker TestDockerCoordinator_ConcurrentPulls (117.66s)
=== PAUSE TestDockerCoordinator_ConcurrentPulls
=== CONT  TestDockerCoordinator_ConcurrentPulls

panic: send on closed channel

goroutine 5358 [running]:
github.com/hashicorp/nomad/drivers/docker.dockerStatsCollector(0xc0003a4a20, 0xc0003a49c0, 0x3b9aca00)
	/home/travis/gopath/src/github.com/hashicorp/nomad/drivers/docker/stats.go:108 +0x167

created by
github.com/hashicorp/nomad/drivers/docker.TestDriver_DockerStatsCollector
	/home/travis/gopath/src/github.com/hashicorp/nomad/drivers/docker/stats_test.go:33 +0x1ab
```

The 2 ways to fix this kind of error are to either (1) add extra
coordination around multiple goroutines writing to a chan or (2) make it
so only one goroutines writes to a chan.

I implemented (2) first as it's simpler, but @notnoop pointed out since
the same destCh in reused in the stats loop there's now a double close
panic possible!

So this implements (1) by adding a *usageSender struct for handling
concurrent senders and closing.
2019-04-02 08:28:08 -07:00
Mahmood Ali 81f4f07ed7 rename fifo methods for clarity 2019-04-01 16:52:58 -04:00
Michael Schurter 294d405397 docker: restore pre-0.9 container names
As far as I can tell Nomad itself does not use the container name after
container creation, so this should be safe.

OP: https://groups.google.com/d/topic/nomad-tool/kYkyERfVRXE/discussion
v0.8.7 code: https://github.com/hashicorp/nomad/blob/v0.8.7/client/driver/docker.go#L1530-L1531
2019-03-29 13:55:43 -07:00
Mahmood Ali 8ec49fc133
Handle when cannot fetch docker logs (#5420)
Fix #5418

When using a docker logger that doesn't support log streaming through
API, currently docker logger runs a tight loop of Docker API calls
unexpectedly. This change ensures we stop fetching logs early.

Also, this adds some basic backoff strategy when Docker API logging
fails unexpectedly, to avoid accidentally DoSing the docker daemon.
2019-03-14 16:23:11 -04:00
Mahmood Ali fb55717b0c
Regenerate Proto files (#5421)
Noticed that the protobuf files are out of sync with ones generated by 1.2.0 protoc go plugin.

The cause for these files seem to be related to release processes, e.g. [0.9.0-beta1 preperation](ecec3d38de (diff-da4da188ee496377d456025c2eab4e87)), and [0.9.0-beta3 preperation](b849d84f2f).

This restores the changes to that of the pinned protoc version and fails build if protobuf files are out of sync.  Sample failing Travis job is that of the first commit change: https://travis-ci.org/hashicorp/nomad/jobs/506285085
2019-03-14 10:56:27 -04:00
Preetha Appan 7f0d9e0c8e
minor review feedback 2019-03-13 13:27:28 -05:00
Preetha Appan 273f1e993d
Validate all auth fields being empty rather than just email
This fixes a regression in 0.9 beta3 compared to 0.8.7 in validating
docker auth config
2019-03-13 11:47:37 -05:00
Preetha Appan 549ae657f0
Dont require email address for docker auth 2019-03-13 11:08:56 -05:00
Mahmood Ali bb32ba8784
Support driver config fields being set to nil (#5391)
To pick up https://github.com/hashicorp/hcl2/pull/90
2019-03-05 21:47:06 -05:00
Mahmood Ali 4726cb2207 logging.Type over logging.Driver 2019-02-28 16:40:18 -05:00
Mahmood Ali 104869c0e1 drivers/docker: rename logging type to driver
Docker uses the term logging `driver` in its public documentations: in
`docker` daemon config[1], `docker run` arguments [2] and in docker compose file[3].
Interestingly, docker used `type` in its API [4] instead of everywhere
else.

It's unfortunate that Nomad used `type` modeling after the Docker API
rather than the user facing documents.  Nomad using `type` feels very
non-user friendly as it's disconnected from how Docker markets the flag
and shows internal representation instead.

Here, we rectify the situation by introducing `driver` field and
prefering it over `type` in logging.

[1] https://docs.docker.com/config/containers/logging/configure/
[2] https://docs.docker.com/engine/reference/run/#logging-drivers---log-driver
[3] https://docs.docker.com/compose/compose-file/#logging
[4] https://docs.docker.com/engine/api/v1.39/#operation/ContainerCreate
2019-02-28 16:04:03 -05:00
Mahmood Ali 67e2a0ac05
docker: report unhealthy in unsupported Windows (#5356)
On Windows, Nomad only supports Windows containers, so report as
unhealthy otherwise.
2019-02-27 08:10:23 -05:00
Danielle Tomlinson e250aad31b
Merge pull request #5355 from hashicorp/dani/windows-dockerstats
docker: Support Stats on Windows
2019-02-26 16:39:48 +01:00
Danielle Tomlinson e3dc80bea3 docker: Return undetected before first detection
This commit causes the docker driver to return undetected before it
first establishes a connection to the docker daemon.

This fixes a bug where hosts without docker installed would return as
unhealthy, rather than undetected.
2019-02-25 11:02:42 +01:00
Danielle Tomlinson 8aff115fca docker: Support stats on Windows 2019-02-22 14:19:58 +01:00
Mahmood Ali 6d30284ec9
Merge pull request #5341 from hashicorp/ci-windows-docker
Run Docker tests in Windows AppVeyor CI
2019-02-21 13:17:33 -05:00
Danielle Tomlinson 2610e2d9ef docker: Avoid leaking containers during Reattach
Currently if a docker_logger cannot be reattached to, we will leak the
container that was being used. This is problematic if e.g using static
ports as it means you can never recover your task, or if a service is
expensive to run and will then be running without supervision.
2019-02-20 17:47:06 +01:00
Danielle Tomlinson 953755ce24
Merge pull request #5335 from hashicorp/dani/docker-logger-spawn
Increase resiliency of docker driver logging
2019-02-20 17:16:05 +01:00
Michael Schurter a1645edb0b Update drivers/docker/docklog/docker_logger.go
Co-Authored-By: dantoml <dani@tomlinson.io>
2019-02-20 17:12:56 +01:00
Danielle Tomlinson 2f18441a47 docker: Respawn docker logger during recovery
Sometimes the nomad docker_logger may be killed by a service manager
when restarting the client for upgrades or reliability reasons.

Currently if this happens, we leak the users container and try to
reschedule over it.

This commit adds a new step to the recovery process that will spawn a
new docker logger process that will fetch logs from _the current
timestamp_. This is to avoid restarting users tasks because our logging
sidecar has failed.
2019-02-20 17:12:56 +01:00
Mahmood Ali 8c82c19831 tests: IsTravis() -> IsCI()
Replace IsTravis() references that is intended for more CI environments
rather than for Travis environment specifically.
2019-02-20 08:21:03 -05:00
Mahmood Ali fedab3d7b0 driver/docker: Skip failing Windows tests
Skip currently Docker tests that fail on Windows for further
investigation.
2019-02-20 07:48:02 -05:00
Mahmood Ali 4def8529db driver/docker: use BlockAttrs for storage_opts
storage_opts is a new field in 0.9 cycle and doesn't have backward
compatibility constraints.
2019-02-19 20:35:28 -05:00
Danielle Tomlinson 3cf3ac7eac dlogger: Increase resilience to docker api failure
This commit adds some extra resiliency to the docker logger in the case
of API failure from the docker daemon, by restarting the stream from the
current point in time if the stream returns and the container is still
running.
2019-02-19 15:17:54 +01:00
Mahmood Ali 46cd3c3f55 drivers: restore port_map old json support
This ensures that `port_map` along with other block like attribute
declarations (e.g. ulimit, labels, etc) can handle various hcl and json
syntax that was supported in 0.8.

In 0.8.7, the following declarations are effectively equivalent:

```
// hcl block
port_map {
  http = 80
  https = 443
}

// hcl assignment
port_map = {
  http  = 80
  https = 443
}

// json single element array of map (default in API response)
{"port_map": [{"http": 80, "https": 443}]}

// json array of individual maps (supported accidentally iiuc)
{"port_map: [{"http": 80}, {"https": 443}]}
```

We achieve compatbility by using `NewAttr("...", "list(map(string))",
false)` to be serialized to a `map[string]string` wrapper, instead of using
`BlockAttrs` declaration.  The wrapper merges the list of maps
automatically, to ease driver development.

This approach is closer to how v0.8.7 implemented the fields [1][2], and
despite its verbosity, seems to perserve 0.8.7 behavior in hcl2.

This is only required for built-in types that have backward
compatibility constraints.  External drivers should use `BlockAttrs`
instead, as they see fit.

[1] https://github.com/hashicorp/nomad/blob/v0.8.7/client/driver/docker.go#L216
[2] https://github.com/hashicorp/nomad/blob/v0.8.7/client/driver/docker.go#L698-L700
2019-02-16 11:37:33 -05:00