Added support to specify default docker log driver in plugin options.

[apollo13]

Fixes #6563

[apollo13]

Is there a way to access the circleci results without giving it access to my private repos?

[tgross]

Is there a way to access the circleci results without giving it access to my private repos?

Honestly I'm not totally sure how that works with Circle's permissions. Some of the errors are some transient infrastructure issue, so I'll kick off a "restart from failed" to fix that. but this one looks real and relevant. I think "local" is specific to a recent version of Docker IIRC?

=== RUN   TestDockerDriver_LoggingConfiguration
2021-03-10T15:30:16.791Z [TRACE] eventer/eventer.go:68: docker: task event loop shutdown
    docker.go:36: Successfully connected to docker daemon running version 18.06.0-ce
    docker.go:36: Successfully connected to docker daemon running version 18.06.0-ce
2021-03-10T15:30:16.794Z [WARN]  loader/init.go:224: plugin_loader: skipping external plugins since plugin_dir doesn't exist: plugin_dir=./plugins
2021-03-10T15:30:16.798Z [INFO]  logmon/logmon.go:198: driver_harness.logmon: opening fifo: path=/tmp/nomad_driver_harness-691772282/alloc/logs/.redis-demo.stdout.fifo
2021-03-10T15:30:16.798Z [INFO]  logmon/logmon.go:198: driver_harness.logmon: opening fifo: path=/tmp/nomad_driver_harness-691772282/alloc/logs/.redis-demo.stderr.fifo
2021-03-10T15:30:16.812Z [DEBUG] docker/coordinator.go:250: docker: image reference count incremented: image_name=busybox:1.29.3 image_id=sha256:59788edf1f3e78cd0ebe6ce1446e9d10788225db3dedcfd1a59f764bad2b2690 references=1
2021-03-10T15:30:16.812Z [TRACE] docker/driver.go:782: docker: binding volumes: task_name=redis-demo volumes=[/tmp/nomad_driver_harness-691772282/alloc:/alloc, /tmp/nomad_driver_harness-691772282/redis-demo/local:/local, /tmp/nomad_driver_harness-691772282/redis-demo/secrets:/secrets]
2021-03-10T15:30:16.812Z [TRACE] docker/driver.go:879: docker: no docker log driver provided, defaulting to plugin config: task_name=redis-demo
2021-03-10T15:30:16.812Z [DEBUG] docker/driver.go:884: docker: configured resources: task_name=redis-demo memory=268435456 memory_reservation=0 cpu_shares=512 cpu_quota=0 cpu_period=0
2021-03-10T15:30:16.812Z [DEBUG] docker/driver.go:889: docker: binding directories: task_name=redis-demo binds="[]string{"/tmp/nomad_driver_harness-691772282/alloc:/alloc", "/tmp/nomad_driver_harness-691772282/redis-demo/local:/local", "/tmp/nomad_driver_harness-691772282/redis-demo/secrets:/secrets"}"
2021-03-10T15:30:16.812Z [DEBUG] docker/driver.go:1054: docker: networking mode not specified; using default: task_name=redis-demo
2021-03-10T15:30:16.812Z [DEBUG] docker/ports.go:61: docker: allocated static port: task_name=redis-demo ip=127.0.0.1 port=9619 label=main
2021-03-10T15:30:16.812Z [DEBUG] docker/ports.go:65: docker: exposed port: task_name=redis-demo port=9619 label=main
2021-03-10T15:30:16.812Z [DEBUG] docker/ports.go:61: docker: allocated static port: task_name=redis-demo ip=127.0.0.1 port=9620 label=REDIS
2021-03-10T15:30:16.812Z [DEBUG] docker/ports.go:65: docker: exposed port: task_name=redis-demo port=9620 label=REDIS
2021-03-10T15:30:16.812Z [DEBUG] docker/driver.go:1108: docker: setting container startup command: task_name=redis-demo command="nc -l -p 3000 127.0.0.1"
2021-03-10T15:30:16.812Z [DEBUG] docker/driver.go:1151: docker: applied labels on the container: task_name=redis-demo labels=map[com.hashicorp.nomad.alloc_id:616591df-1e62-a296-f1f8-b1ee74977e49]
2021-03-10T15:30:16.812Z [DEBUG] docker/driver.go:1156: docker: setting container name: task_name=redis-demo container_name=redis-demo-616591df-1e62-a296-f1f8-b1ee74977e49
2021-03-10T15:30:31.820Z [DEBUG] docker/driver.go:427: docker: failed to create container: container_name=redis-demo-616591df-1e62-a296-f1f8-b1ee74977e49 image_name=busybox:1.29.3 image_id=busybox:1.29.3 attempt=1 error="API error (400): logger: no log driver named 'local' is registered"
2021-03-10T15:30:31.820Z [ERROR] docker/driver.go:296: docker: failed to create container: error="API error (400): logger: no log driver named 'local' is registered"
    driver_test.go:137: 
        	Error Trace:	driver_test.go:137
        	            				driver_test.go:863
        	Error:      	Received unexpected error:
        	            	rpc error: code = FailedPrecondition desc = failed to create container: API error (400): logger: no log driver named 'local' is registered
        	Test:       	TestDockerDriver_LoggingConfiguration
--- FAIL: TestDockerDriver_LoggingConfiguration (15.03s)

[apollo13]

I think "local" is specific to a recent version of Docker IIRC?

As recent as 18.09, how "old", or which version do you have on circle ci?

[tgross]

As recent as 18.09, how "old", or which version do you have on circle ci?

From the logs, looks like 18.06-ce, which is what's shipping for Ubuntu 16.04 LTS (which we need to be able to support).

docker.go:36: Successfully connected to docker daemon running version 18.06.0-ce

[apollo13]

Ok, switched the logger to gelf which did already exist in 18.06

[tgross]

This mostly LGTM. I have some conceptual/UX questions that I've left below.

[tgross]

Especially after reading the docs updates, I'm realizing there's a bit of a conceptual gap here we need to solve. The log driver configuration with max-file and max-size is not the same as the Nomad logs block; one is Docker's own logs and the other is the Nomad allocation logs.

I feel like the original issue #6563 that @powellchristoph opened was ambiguous as to whether it was understood that these are different values. If so, we should probably clarify that further in the docs. I'm also wondering whether there's value in being able to set the Nomad defaults in the client config (either as well or instead of).

[apollo13]

ha, I never knew that nomad has a logs block. For me the main goal is to send the task logs somewhere else. For docker the easiest way to do this is via a logdriver. I think in an ideal world nomad would be able to send those logs away (with metadata attached) -- for all task drivers without relying on some hacks like docker logging drivers.

As for the logs block, no idea -- I'll leave it up to you to decide what to do with it. Personally I think it would belong in another PR to keep to scope small.

[tgross]

I think in an ideal world nomad would be able to send those logs away (with metadata attached) -- for all task drivers without relying on some hacks like docker logging drivers.

Agreed. We have an issue open for some kind of log driver plugin concept (#9211) but that hasn't quite made its way onto the near-term roadmap yet.

As for the logs block, no idea -- I'll leave it up to you to decide what to do with it. Personally I think it would belong in another PR to keep to scope small.

I'm inclined to agree, but lemme run that by a couple of other folks on the team just to get a little consensus.

[tgross]

Ok, had a quick discussion about that internally and I think we're just going to keep the code here as is. I'm going to push a commit onto this PR just to add a few choice warnings about the difference between the two configuration values and then we'll get this merged.

[tgross]

Actually, I'm happy with what you've got here. Let's merge this.

[tgross]

LGTM

[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.