From 314d7a0f41bb223dbcd9b45bbc2de26344b62f6e Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Thu, 28 Feb 2019 15:25:17 -0500 Subject: [PATCH 1/2] 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 --- drivers/docker/config.go | 2 ++ drivers/docker/config_test.go | 6 ++++-- drivers/docker/driver.go | 7 ++++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/docker/config.go b/drivers/docker/config.go index 7f673b583ddb..5039f0c5fab1 100644 --- a/drivers/docker/config.go +++ b/drivers/docker/config.go @@ -256,6 +256,7 @@ var ( "load": hclspec.NewAttr("load", "string", false), "logging": hclspec.NewBlock("logging", false, hclspec.NewObject(map[string]*hclspec.Spec{ "type": hclspec.NewAttr("type", "string", false), + "driver": hclspec.NewAttr("driver", "string", false), "config": hclspec.NewAttr("config", "list(map(string))", false), })), "mac_address": hclspec.NewAttr("mac_address", "string", false), @@ -397,6 +398,7 @@ func (d DockerDevice) toDockerDevice() (docker.Device, error) { type DockerLogging struct { Type string `codec:"type"` + Driver string `codec:"driver"` Config hclutils.MapStrStr `codec:"config"` } diff --git a/drivers/docker/config_test.go b/drivers/docker/config_test.go index 95ca5a4e25d0..47c71c55ffb9 100644 --- a/drivers/docker/config_test.go +++ b/drivers/docker/config_test.go @@ -181,7 +181,8 @@ config { } load = "/tmp/image.tar.gz" logging { - type = "json-file" + driver = "json-file-driver" + type = "json-file" config { "max-file" = "3" "max-size" = "10m" @@ -308,7 +309,8 @@ config { }, LoadImage: "/tmp/image.tar.gz", Logging: DockerLogging{ - Type: "json-file", + Driver: "json-file-driver", + Type: "json-file", Config: map[string]string{ "max-file": "3", "max-size": "10m", diff --git a/drivers/docker/driver.go b/drivers/docker/driver.go index 4263409a99ec..fb359887c9de 100644 --- a/drivers/docker/driver.go +++ b/drivers/docker/driver.go @@ -706,8 +706,13 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T hostConfig.MemorySwap = task.Resources.LinuxResources.MemoryLimitBytes // MemorySwap is memory + swap. } + loggingDriver := driverConfig.Logging.Driver + if loggingDriver == "" { + loggingDriver = driverConfig.Logging.Type + } + hostConfig.LogConfig = docker.LogConfig{ - Type: driverConfig.Logging.Type, + Type: loggingDriver, Config: driverConfig.Logging.Config, } From 011315ba4c228451a2089a82dd240d7389b36f92 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Thu, 28 Feb 2019 16:40:18 -0500 Subject: [PATCH 2/2] logging.Type over logging.Driver --- drivers/docker/driver.go | 4 +-- drivers/docker/driver_test.go | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/drivers/docker/driver.go b/drivers/docker/driver.go index fb359887c9de..a1718b5a7b66 100644 --- a/drivers/docker/driver.go +++ b/drivers/docker/driver.go @@ -706,9 +706,9 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T hostConfig.MemorySwap = task.Resources.LinuxResources.MemoryLimitBytes // MemorySwap is memory + swap. } - loggingDriver := driverConfig.Logging.Driver + loggingDriver := driverConfig.Logging.Type if loggingDriver == "" { - loggingDriver = driverConfig.Logging.Type + loggingDriver = driverConfig.Logging.Driver } hostConfig.LogConfig = docker.LogConfig{ diff --git a/drivers/docker/driver_test.go b/drivers/docker/driver_test.go index ea30027ec4ef..c0a9e8fbed38 100644 --- a/drivers/docker/driver_test.go +++ b/drivers/docker/driver_test.go @@ -930,6 +930,52 @@ func TestDockerDriver_CreateContainerConfig(t *testing.T) { require.EqualValues(t, opt, c.HostConfig.StorageOpt) } +func TestDockerDriver_CreateContainerConfig_Logging(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + loggingConfig DockerLogging + expectedDriver string + }{ + { + "simple type", + DockerLogging{Type: "fluentd"}, + "fluentd", + }, + { + "simple driver", + DockerLogging{Driver: "fluentd"}, + "fluentd", + }, + { + "type takes precedence", + DockerLogging{ + Type: "json-file", + Driver: "fluentd", + }, + "json-file", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + task, cfg, _ := dockerTask(t) + + cfg.Logging = c.loggingConfig + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + dh := dockerDriverHarness(t, nil) + driver := dh.Impl().(*Driver) + + cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") + require.NoError(t, err) + + require.Equal(t, c.expectedDriver, cc.HostConfig.LogConfig.Type) + }) + } +} + func TestDockerDriver_CreateContainerConfigWithRuntimes(t *testing.T) { if !tu.IsCI() { t.Parallel()