From 98df16b861aca124c14a06d138b440241d7d7114 Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Fri, 29 Sep 2023 12:51:40 +0200 Subject: [PATCH] Added a plugin logging configuration to specify defaults. This is similar to https://github.com/hashicorp/nomad/pull/10156/files and allows for a configuration like: ``` plugin "nomad-driver-podman" { config { extra_labels = ["job_name", "job_id", "task_group_name", "task_name", "namespace", "node_name", "node_id"] logging { driver = "journald" options = { tag = "{{index .Config.Labels \"com.hashicorp.nomad.alloc_id\" }}" } } } } ``` --- README.md | 13 ++++++++----- config.go | 15 +++++++++++++-- driver.go | 38 ++++++++++++++++++++++++++------------ driver_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 337cd367..f571e679 100644 --- a/README.md +++ b/README.md @@ -188,6 +188,11 @@ plugin "nomad-driver-podman" { } ``` +* logging stanza: + + * type - Defaults to `"nomad"`. See the task configuration for details. + * options - Defaults to `{}`. See the task configuration for details. + * client_http_timeout (string) Defaults to `60s` default timeout used by http.Client requests ```hcl @@ -342,11 +347,9 @@ Ensure you're running Podman 3.1.0 or higher because of bugs in older versions. config { logging = { driver = "journald" - options = [ - { - "tag" = "redis" - } - ] + options = { + "tag" = "redis" + } } } ``` diff --git a/config.go b/config.go index 0460e66c..a69309cd 100644 --- a/config.go +++ b/config.go @@ -44,6 +44,16 @@ var ( ), // optional extra_labels to append to all tasks for observability. Globs supported "extra_labels": hclspec.NewAttr("extra_labels", "list(string)", false), + + // logging options + "logging": hclspec.NewDefault(hclspec.NewBlock("logging", false, hclspec.NewObject(map[string]*hclspec.Spec{ + "driver": hclspec.NewAttr("driver", "string", false), + "options": hclspec.NewBlockAttrs("options", "string", false), + })), hclspec.NewLiteral(`{ + driver = "nomad" + options = {} + }`)), + // the path to the podman api socket "socket_path": hclspec.NewAttr("socket_path", "string", false), // disable_log_collection indicates whether nomad should collect logs of podman @@ -124,8 +134,8 @@ type GCConfig struct { // LoggingConfig is the tasks logging configuration type LoggingConfig struct { - Driver string `codec:"driver"` - Options hclutils.MapStrStr `codec:"options"` + Driver string `codec:"driver"` + Options map[string]string `codec:"options"` } // VolumeConfig is the drivers volume specific configuration @@ -150,6 +160,7 @@ type PluginConfig struct { ClientHttpTimeout string `codec:"client_http_timeout"` ExtraLabels []string `codec:"extra_labels"` DNSServers []string `codec:"dns_servers"` + Logging LoggingConfig `codec:"logging"` } // LogWarnings will emit logs about known problematic configurations diff --git a/driver.go b/driver.go index 70f76290..060bbdf2 100644 --- a/driver.go +++ b/driver.go @@ -502,20 +502,34 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive createOpts.ContainerBasicConfig.Labels = driverConfig.Labels // Logging - switch driverConfig.Logging.Driver { - case "", LOG_DRIVER_NOMAD: - // Only modify container logging path if LogCollection is not disabled - if !d.config.DisableLogCollection { - createOpts.LogConfiguration.Driver = "k8s-file" - - createOpts.ContainerBasicConfig.LogConfiguration.Path = cfg.StdoutPath + if driverConfig.Logging.Driver != "" { + switch driverConfig.Logging.Driver { + case LOG_DRIVER_NOMAD: + if !d.config.DisableLogCollection { + createOpts.LogConfiguration.Driver = "k8s-file" + createOpts.ContainerBasicConfig.LogConfiguration.Path = cfg.StdoutPath + } + case LOG_DRIVER_JOURNALD: + createOpts.LogConfiguration.Driver = "journald" + default: + return nil, nil, fmt.Errorf("Invalid task logging.driver option") } - case LOG_DRIVER_JOURNALD: - createOpts.LogConfiguration.Driver = "journald" - default: - return nil, nil, fmt.Errorf("Invalid logging.driver option") + createOpts.ContainerBasicConfig.LogConfiguration.Options = driverConfig.Logging.Options + } else { + d.logger.Trace("no podman log driver provided, defaulting to plugin config") + switch d.config.Logging.Driver { + case LOG_DRIVER_NOMAD: + if !d.config.DisableLogCollection { + createOpts.LogConfiguration.Driver = "k8s-file" + createOpts.ContainerBasicConfig.LogConfiguration.Path = cfg.StdoutPath + } + case LOG_DRIVER_JOURNALD: + createOpts.LogConfiguration.Driver = "journald" + default: + return nil, nil, fmt.Errorf("Invalid plugin logging.driver option") + } + createOpts.ContainerBasicConfig.LogConfiguration.Options = d.config.Logging.Options } - createOpts.ContainerBasicConfig.LogConfiguration.Options = driverConfig.Logging.Options // Storage config options createOpts.ContainerStorageConfig.Init = driverConfig.Init diff --git a/driver_test.go b/driver_test.go index 19040888..7b962e21 100644 --- a/driver_test.go +++ b/driver_test.go @@ -516,6 +516,55 @@ func TestPodmanDriver_logNomad(t *testing.T) { must.StrContains(t, stdoutLog, stderrMagic) } +// Check default plugin logger +// TODO: Can we make this nicer? +func TestPodmanDriver_logNomadDefault(t *testing.T) { + ci.Parallel(t) + + stdoutMagic := uuid.Generate() + stderrMagic := uuid.Generate() + + taskCfg := newTaskConfig("", []string{ + "sh", + "-c", + fmt.Sprintf("echo %s; 1>&2 echo %s", stdoutMagic, stderrMagic), + }) + task := &drivers.TaskConfig{ + ID: uuid.Generate(), + Name: "logNomad", + AllocID: uuid.Generate(), + Resources: createBasicResources(), + } + must.NoError(t, task.EncodeConcreteDriverConfig(&taskCfg)) + + d := podmanDriverHarness(t, nil) + cleanup := d.MkAllocDir(task, true) + defer cleanup() + + _, _, err := d.StartTask(task) + must.NoError(t, err) + + defer func() { + _ = d.DestroyTask(task.ID, true) + }() + + // Attempt to wait + waitCh, err := d.WaitTask(context.Background(), task.ID) + must.NoError(t, err) + + select { + case <-waitCh: + case <-time.After(10 * time.Second): + t.Fatalf("Container did not exit in time") + } + + // log_driver=nomad combines both streams into stdout, so we will find both + // magic values in the same stream + stdoutLog := readStdoutLog(t, task) + must.StrContains(t, stdoutLog, stdoutMagic) + must.StrContains(t, stdoutLog, stderrMagic) +} + // check if extra labels make it into logs func TestPodmanDriver_ExtraLabels(t *testing.T) { ci.Parallel(t)