From 310f048dd5eb29269ef810dd6d519c4ff33b6529 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Fri, 21 Apr 2023 14:42:35 +0000 Subject: [PATCH] backport of commit 7cc983b7f839add3700f16759e1863b4196feb52 --- .changelog/16962.txt | 3 ++ api/tasks.go | 9 +++- client/allocrunner/taskrunner/logmon_hook.go | 41 ++++++++++++--- .../taskrunner/logmon_hook_test.go | 52 ++++++++++++++++++- .../taskrunner/logmon_hook_unix_test.go | 4 +- .../taskrunner/task_runner_hooks.go | 2 +- command/agent/job_endpoint.go | 2 + command/agent/job_endpoint_test.go | 8 +++ drivers/docker/driver.go | 14 ++++- drivers/docker/driver_test.go | 33 +++++++++++- jobspec/parse_task.go | 1 + jobspec/parse_test.go | 1 + jobspec/test-fixtures/basic.hcl | 1 + nomad/structs/diff_test.go | 30 +++++++++++ nomad/structs/structs.go | 12 ++++- scheduler/annotate.go | 12 ++++- scheduler/annotate_test.go | 21 ++++++++ scheduler/util.go | 7 +++ website/content/api-docs/client.mdx | 4 +- website/content/api-docs/jobs.mdx | 4 ++ website/content/api-docs/json-jobs.mdx | 3 ++ .../content/docs/job-specification/logs.mdx | 24 ++++++--- 22 files changed, 261 insertions(+), 27 deletions(-) create mode 100644 .changelog/16962.txt diff --git a/.changelog/16962.txt b/.changelog/16962.txt new file mode 100644 index 00000000000..10310994bba --- /dev/null +++ b/.changelog/16962.txt @@ -0,0 +1,3 @@ +```release-note:improvement +jobspec: Added option for disabling task log collection in the `logs` block +``` diff --git a/api/tasks.go b/api/tasks.go index ecc14a8ea88..2008e3113f3 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -636,14 +636,16 @@ func (g *TaskGroup) AddSpread(s *Spread) *TaskGroup { // LogConfig provides configuration for log rotation type LogConfig struct { - MaxFiles *int `mapstructure:"max_files" hcl:"max_files,optional"` - MaxFileSizeMB *int `mapstructure:"max_file_size" hcl:"max_file_size,optional"` + MaxFiles *int `mapstructure:"max_files" hcl:"max_files,optional"` + MaxFileSizeMB *int `mapstructure:"max_file_size" hcl:"max_file_size,optional"` + Enabled *bool `mapstructure:"enabled" hcl:"enabled,optional"` } func DefaultLogConfig() *LogConfig { return &LogConfig{ MaxFiles: pointerOf(10), MaxFileSizeMB: pointerOf(10), + Enabled: pointerOf(true), } } @@ -654,6 +656,9 @@ func (l *LogConfig) Canonicalize() { if l.MaxFileSizeMB == nil { l.MaxFileSizeMB = pointerOf(10) } + if l.Enabled == nil { + l.Enabled = pointerOf(true) + } } // DispatchPayloadConfig configures how a task gets its input from a job dispatch diff --git a/client/allocrunner/taskrunner/logmon_hook.go b/client/allocrunner/taskrunner/logmon_hook.go index b983929599d..d27a6a63085 100644 --- a/client/allocrunner/taskrunner/logmon_hook.go +++ b/client/allocrunner/taskrunner/logmon_hook.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "os" "path/filepath" "runtime" "time" @@ -42,6 +43,7 @@ type logmonHook struct { type logmonHookConfig struct { logDir string + enabled bool stdoutFifo string stderrFifo string } @@ -56,10 +58,19 @@ func newLogMonHook(tr *TaskRunner, logger hclog.Logger) *logmonHook { return hook } -func newLogMonHookConfig(taskName, logDir string) *logmonHookConfig { +func newLogMonHookConfig(taskName string, logCfg *structs.LogConfig, logDir string) *logmonHookConfig { cfg := &logmonHookConfig{ - logDir: logDir, + logDir: logDir, + enabled: logCfg.Enabled, } + + // If logging is disabled configure task's stdout/err to point to devnull + if !logCfg.Enabled { + cfg.stdoutFifo = os.DevNull + cfg.stderrFifo = os.DevNull + return cfg + } + if runtime.GOOS == "windows" { id := uuid.Generate()[:8] cfg.stdoutFifo = fmt.Sprintf("//./pipe/%s-%s.stdout", taskName, id) @@ -102,9 +113,7 @@ func reattachConfigFromHookData(data map[string]string) (*plugin.ReattachConfig, func (h *logmonHook) Prestart(ctx context.Context, req *interfaces.TaskPrestartRequest, resp *interfaces.TaskPrestartResponse) error { - - if h.isLoggingDisabled() { - h.logger.Debug("logging is disabled by driver") + if !h.isLoggingEnabled() { return nil } @@ -139,14 +148,27 @@ func (h *logmonHook) Prestart(ctx context.Context, } } -func (h *logmonHook) isLoggingDisabled() bool { +func (h *logmonHook) isLoggingEnabled() bool { + if !h.config.enabled { + h.logger.Debug("log collection is disabled by task") + return false + } + + // internal plugins have access to a capability to disable logging and + // metrics via a private interface; this is an "experimental" interface and + // currently only the docker driver exposes this to users. ic, ok := h.runner.driver.(drivers.InternalCapabilitiesDriver) if !ok { - return false + return true } caps := ic.InternalCapabilities() - return caps.DisableLogCollection + if caps.DisableLogCollection { + h.logger.Debug("log collection is disabled by driver") + return false + } + + return true } func (h *logmonHook) prestartOneLoop(ctx context.Context, req *interfaces.TaskPrestartRequest) error { @@ -194,6 +216,9 @@ func (h *logmonHook) prestartOneLoop(ctx context.Context, req *interfaces.TaskPr } func (h *logmonHook) Stop(_ context.Context, req *interfaces.TaskStopRequest, _ *interfaces.TaskStopResponse) error { + if !h.isLoggingEnabled() { + return nil + } // It's possible that Stop was called without calling Prestart on agent // restarts. Attempt to reattach to an existing logmon. diff --git a/client/allocrunner/taskrunner/logmon_hook_test.go b/client/allocrunner/taskrunner/logmon_hook_test.go index bf5f9e7f078..88439bb3bf2 100644 --- a/client/allocrunner/taskrunner/logmon_hook_test.go +++ b/client/allocrunner/taskrunner/logmon_hook_test.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad/mock" pstructs "github.com/hashicorp/nomad/plugins/shared/structs" + "github.com/shoenig/test/must" "github.com/stretchr/testify/require" "golang.org/x/exp/maps" ) @@ -66,7 +67,7 @@ func TestTaskRunner_LogmonHook_StartStop(t *testing.T) { dir := t.TempDir() - hookConf := newLogMonHookConfig(task.Name, dir) + hookConf := newLogMonHookConfig(task.Name, task.LogConfig, dir) runner := &TaskRunner{logmonHookConfig: hookConf} hook := newLogMonHook(runner, testlog.HCLogger(t)) @@ -100,3 +101,52 @@ func TestTaskRunner_LogmonHook_StartStop(t *testing.T) { } require.NoError(t, hook.Stop(context.Background(), &stopReq, nil)) } + +// TestTaskRunner_LogmonHook_Disabled asserts that no logmon running or expected +// by any of the lifecycle hooks. +func TestTaskRunner_LogmonHook_Disabled(t *testing.T) { + ci.Parallel(t) + + alloc := mock.BatchAlloc() + task := alloc.Job.TaskGroups[0].Tasks[0] + task.LogConfig.Enabled = false + + dir := t.TempDir() + + hookConf := newLogMonHookConfig(task.Name, task.LogConfig, dir) + runner := &TaskRunner{logmonHookConfig: hookConf} + hook := newLogMonHook(runner, testlog.HCLogger(t)) + + req := interfaces.TaskPrestartRequest{Task: task} + resp := interfaces.TaskPrestartResponse{} + + // First prestart should not set reattach key and never be Done. + must.NoError(t, hook.Prestart(context.Background(), &req, &resp)) + t.Cleanup(func() { hook.Stop(context.Background(), nil, nil) }) + + must.False(t, resp.Done) + hookData, ok := resp.State[logmonReattachKey] + must.False(t, ok) + must.Eq(t, "", hookData) + + // Running prestart again should still be a noop + req.PreviousState = map[string]string{} + must.NoError(t, hook.Prestart(context.Background(), &req, &resp)) + + must.False(t, resp.Done) + hookData, ok = resp.State[logmonReattachKey] + must.False(t, ok) + must.Eq(t, "", hookData) + + // PreviousState should always be initialized by the caller, but just + // belt-and-suspenders for this test to ensure we can't panic on this code + // path + req.PreviousState = nil + must.NoError(t, hook.Prestart(context.Background(), &req, &resp)) + + // Running stop should not error even with no running logmon + stopReq := interfaces.TaskStopRequest{ + ExistingState: maps.Clone(resp.State), + } + must.NoError(t, hook.Stop(context.Background(), &stopReq, nil)) +} diff --git a/client/allocrunner/taskrunner/logmon_hook_unix_test.go b/client/allocrunner/taskrunner/logmon_hook_unix_test.go index f98c01b06cc..bb5e9e817bd 100644 --- a/client/allocrunner/taskrunner/logmon_hook_unix_test.go +++ b/client/allocrunner/taskrunner/logmon_hook_unix_test.go @@ -32,7 +32,7 @@ func TestTaskRunner_LogmonHook_StartCrashStop(t *testing.T) { dir := t.TempDir() - hookConf := newLogMonHookConfig(task.Name, dir) + hookConf := newLogMonHookConfig(task.Name, task.LogConfig, dir) runner := &TaskRunner{logmonHookConfig: hookConf} hook := newLogMonHook(runner, testlog.HCLogger(t)) @@ -97,7 +97,7 @@ func TestTaskRunner_LogmonHook_ShutdownMidStart(t *testing.T) { dir := t.TempDir() - hookConf := newLogMonHookConfig(task.Name, dir) + hookConf := newLogMonHookConfig(task.Name, task.LogConfig, dir) runner := &TaskRunner{logmonHookConfig: hookConf} hook := newLogMonHook(runner, testlog.HCLogger(t)) diff --git a/client/allocrunner/taskrunner/task_runner_hooks.go b/client/allocrunner/taskrunner/task_runner_hooks.go index 40ea084ba1a..6d71ba74e82 100644 --- a/client/allocrunner/taskrunner/task_runner_hooks.go +++ b/client/allocrunner/taskrunner/task_runner_hooks.go @@ -50,7 +50,7 @@ func (tr *TaskRunner) initHooks() { hookLogger := tr.logger.Named("task_hook") task := tr.Task() - tr.logmonHookConfig = newLogMonHookConfig(task.Name, tr.taskDir.LogDir) + tr.logmonHookConfig = newLogMonHookConfig(task.Name, task.LogConfig, tr.taskDir.LogDir) // Add the hook resources tr.hookResources = &hookResources{} diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index ec732240338..4565fe0a9a9 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1183,6 +1183,7 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup, structsTask.LogConfig = &structs.LogConfig{ MaxFiles: *apiTask.LogConfig.MaxFiles, MaxFileSizeMB: *apiTask.LogConfig.MaxFileSizeMB, + Enabled: *apiTask.LogConfig.Enabled, } if len(apiTask.Artifacts) > 0 { @@ -1747,6 +1748,7 @@ func apiLogConfigToStructs(in *api.LogConfig) *structs.LogConfig { return nil } return &structs.LogConfig{ + Enabled: *in.Enabled, MaxFiles: dereferenceInt(in.MaxFiles), MaxFileSizeMB: dereferenceInt(in.MaxFileSizeMB), } diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 9076391022e..b2f6240771f 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2707,6 +2707,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { KillTimeout: pointer.Of(10 * time.Second), KillSignal: "SIGQUIT", LogConfig: &api.LogConfig{ + Enabled: pointer.Of(true), MaxFiles: pointer.Of(10), MaxFileSizeMB: pointer.Of(100), }, @@ -3124,6 +3125,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { KillTimeout: 10 * time.Second, KillSignal: "SIGQUIT", LogConfig: &structs.LogConfig{ + Enabled: true, MaxFiles: 10, MaxFileSizeMB: 100, }, @@ -3280,6 +3282,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { KillTimeout: pointer.Of(10 * time.Second), KillSignal: "SIGQUIT", LogConfig: &api.LogConfig{ + Enabled: pointer.Of(true), MaxFiles: pointer.Of(10), MaxFileSizeMB: pointer.Of(100), }, @@ -3405,6 +3408,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { KillTimeout: 10 * time.Second, KillSignal: "SIGQUIT", LogConfig: &structs.LogConfig{ + Enabled: true, MaxFiles: 10, MaxFileSizeMB: 100, }, @@ -3577,9 +3581,11 @@ func TestConversion_apiLogConfigToStructs(t *testing.T) { ci.Parallel(t) require.Nil(t, apiLogConfigToStructs(nil)) require.Equal(t, &structs.LogConfig{ + Enabled: true, MaxFiles: 2, MaxFileSizeMB: 8, }, apiLogConfigToStructs(&api.LogConfig{ + Enabled: pointer.Of(true), MaxFiles: pointer.Of(2), MaxFileSizeMB: pointer.Of(8), })) @@ -3653,6 +3659,7 @@ func TestConversion_apiConnectSidecarTaskToStructs(t *testing.T) { Meta: meta, KillTimeout: &timeout, LogConfig: &structs.LogConfig{ + Enabled: true, MaxFiles: 2, MaxFileSizeMB: 8, }, @@ -3671,6 +3678,7 @@ func TestConversion_apiConnectSidecarTaskToStructs(t *testing.T) { Meta: meta, KillTimeout: &timeout, LogConfig: &api.LogConfig{ + Enabled: pointer.Of(true), MaxFiles: pointer.Of(2), MaxFileSizeMB: pointer.Of(8), }, diff --git a/drivers/docker/driver.go b/drivers/docker/driver.go index 144362102a4..64d93e9921c 100644 --- a/drivers/docker/driver.go +++ b/drivers/docker/driver.go @@ -251,7 +251,7 @@ func (d *Driver) RecoverTask(handle *drivers.TaskHandle) error { net: handleState.DriverNetwork, } - if !d.config.DisableLogCollection { + if loggingIsEnabled(d.config, handle.Config) { h.dlogger, h.dloggerPluginClient, err = d.reattachToDockerLogger(handleState.ReattachConfig) if err != nil { d.logger.Warn("failed to reattach to docker logger process", "error", err) @@ -282,6 +282,16 @@ func (d *Driver) RecoverTask(handle *drivers.TaskHandle) error { return nil } +func loggingIsEnabled(driverCfg *DriverConfig, taskCfg *drivers.TaskConfig) bool { + if driverCfg.DisableLogCollection { + return false + } + if taskCfg.StderrPath == os.DevNull && taskCfg.StdoutPath == os.DevNull { + return false + } + return true +} + func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drivers.DriverNetwork, error) { if _, ok := d.tasks.Get(cfg.ID); ok { return nil, nil, fmt.Errorf("task with ID %q already started", cfg.ID) @@ -397,7 +407,7 @@ CREATE: } } - collectingLogs := !d.config.DisableLogCollection + collectingLogs := loggingIsEnabled(d.config, cfg) var dlogger docklog.DockerLogger var pluginClient *plugin.Client diff --git a/drivers/docker/driver_test.go b/drivers/docker/driver_test.go index b70403da85d..5c363847110 100644 --- a/drivers/docker/driver_test.go +++ b/drivers/docker/driver_test.go @@ -130,7 +130,7 @@ func dockerTask(t *testing.T) (*drivers.TaskConfig, *TaskConfig, []int) { func dockerSetup(t *testing.T, task *drivers.TaskConfig, driverCfg map[string]interface{}) (*docker.Client, *dtestutil.DriverHarness, *taskHandle, func()) { client := newTestDockerClient(t) driver := dockerDriverHarness(t, driverCfg) - cleanup := driver.MkAllocDir(task, true) + cleanup := driver.MkAllocDir(task, loggingIsEnabled(&DriverConfig{}, task)) copyImage(t, task.TaskDir(), "busybox.tar") _, _, err := driver.StartTask(task) @@ -838,6 +838,37 @@ func TestDockerDriver_LoggingConfiguration(t *testing.T) { require.Equal(t, loggerConfig, container.HostConfig.LogConfig.Config) } +// TestDockerDriver_LogCollectionDisabled ensures that logmon isn't configured +// when log collection is disable, but out-of-band Docker log shipping still +// works as expected +func TestDockerDriver_LogCollectionDisabled(t *testing.T) { + ci.Parallel(t) + testutil.DockerCompatible(t) + + task, cfg, _ := dockerTask(t) + task.StdoutPath = os.DevNull + task.StderrPath = os.DevNull + + must.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + dockerClientConfig := make(map[string]interface{}) + loggerConfig := map[string]string{"gelf-address": "udp://1.2.3.4:12201", "tag": "gelf"} + + dockerClientConfig["logging"] = LoggingConfig{ + Type: "gelf", + Config: loggerConfig, + } + client, d, handle, cleanup := dockerSetup(t, task, dockerClientConfig) + t.Cleanup(cleanup) + must.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second)) + container, err := client.InspectContainer(handle.containerID) + must.NoError(t, err) + must.Nil(t, handle.dlogger) + + must.Eq(t, "gelf", container.HostConfig.LogConfig.Type) + must.Eq(t, loggerConfig, container.HostConfig.LogConfig.Config) +} + func TestDockerDriver_HealthchecksDisable(t *testing.T) { ci.Parallel(t) testutil.DockerCompatible(t) diff --git a/jobspec/parse_task.go b/jobspec/parse_task.go index b7315be74e5..1287af02fa3 100644 --- a/jobspec/parse_task.go +++ b/jobspec/parse_task.go @@ -259,6 +259,7 @@ func parseTask(item *ast.ObjectItem, keys []string) (*api.Task, error) { valid := []string{ "max_files", "max_file_size", + "enabled", } if err := checkHCLKeys(logsBlock.Val, valid); err != nil { return nil, multierror.Prefix(err, "logs ->") diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 173b338cacf..5ba608287b6 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -347,6 +347,7 @@ func TestParse(t *testing.T) { LogConfig: &api.LogConfig{ MaxFiles: intToPtr(14), MaxFileSizeMB: intToPtr(101), + Enabled: boolToPtr(true), }, Artifacts: []*api.TaskArtifact{ { diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index e2d86843edf..e418304f5db 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -191,6 +191,7 @@ job "binstore-storagelocker" { } logs { + enabled = true max_files = 14 max_file_size = 101 } diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 0d25622ac97..ef2e6c307d4 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -4417,6 +4417,7 @@ func TestTaskDiff(t *testing.T) { LogConfig: &LogConfig{ MaxFiles: 1, MaxFileSizeMB: 10, + Enabled: false, }, }, Expected: &TaskDiff{ @@ -4426,6 +4427,12 @@ func TestTaskDiff(t *testing.T) { Type: DiffTypeAdded, Name: "LogConfig", Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Enabled", + Old: "", + New: "false", + }, { Type: DiffTypeAdded, Name: "MaxFileSizeMB", @@ -4449,6 +4456,7 @@ func TestTaskDiff(t *testing.T) { LogConfig: &LogConfig{ MaxFiles: 1, MaxFileSizeMB: 10, + Enabled: false, }, }, New: &Task{}, @@ -4459,6 +4467,12 @@ func TestTaskDiff(t *testing.T) { Type: DiffTypeDeleted, Name: "LogConfig", Fields: []*FieldDiff{ + { + Type: DiffTypeDeleted, + Name: "Enabled", + Old: "false", + New: "", + }, { Type: DiffTypeDeleted, Name: "MaxFileSizeMB", @@ -4482,12 +4496,14 @@ func TestTaskDiff(t *testing.T) { LogConfig: &LogConfig{ MaxFiles: 1, MaxFileSizeMB: 10, + Enabled: false, }, }, New: &Task{ LogConfig: &LogConfig{ MaxFiles: 2, MaxFileSizeMB: 20, + Enabled: true, }, }, Expected: &TaskDiff{ @@ -4497,6 +4513,12 @@ func TestTaskDiff(t *testing.T) { Type: DiffTypeEdited, Name: "LogConfig", Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "Enabled", + Old: "false", + New: "true", + }, { Type: DiffTypeEdited, Name: "MaxFileSizeMB", @@ -4521,12 +4543,14 @@ func TestTaskDiff(t *testing.T) { LogConfig: &LogConfig{ MaxFiles: 1, MaxFileSizeMB: 10, + Enabled: false, }, }, New: &Task{ LogConfig: &LogConfig{ MaxFiles: 1, MaxFileSizeMB: 20, + Enabled: true, }, }, Expected: &TaskDiff{ @@ -4536,6 +4560,12 @@ func TestTaskDiff(t *testing.T) { Type: DiffTypeEdited, Name: "LogConfig", Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "Enabled", + Old: "false", + New: "true", + }, { Type: DiffTypeEdited, Name: "MaxFileSizeMB", diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index c5d5f6b618b..392ae2e9e92 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -7147,6 +7147,7 @@ const ( type LogConfig struct { MaxFiles int MaxFileSizeMB int + Enabled bool } func (l *LogConfig) Equal(o *LogConfig) bool { @@ -7162,6 +7163,10 @@ func (l *LogConfig) Equal(o *LogConfig) bool { return false } + if l.Enabled != o.Enabled { + return false + } + return true } @@ -7172,6 +7177,7 @@ func (l *LogConfig) Copy() *LogConfig { return &LogConfig{ MaxFiles: l.MaxFiles, MaxFileSizeMB: l.MaxFileSizeMB, + Enabled: l.Enabled, } } @@ -7180,11 +7186,13 @@ func DefaultLogConfig() *LogConfig { return &LogConfig{ MaxFiles: 10, MaxFileSizeMB: 10, + Enabled: true, } } -// Validate returns an error if the log config specified are less than -// the minimum allowed. +// Validate returns an error if the log config specified are less than the +// minimum allowed. Note that because we have a non-zero default MaxFiles and +// MaxFileSizeMB, we can't validate that they're unset if Enabled=false func (l *LogConfig) Validate() error { var mErr multierror.Error if l.MaxFiles < 1 { diff --git a/scheduler/annotate.go b/scheduler/annotate.go index 4ca41fad9eb..d1d5020a32f 100644 --- a/scheduler/annotate.go +++ b/scheduler/annotate.go @@ -185,7 +185,17 @@ FieldsLoop: ObjectsLoop: for _, oDiff := range diff.Objects { switch oDiff.Name { - case "LogConfig", "Service", "Constraint": + case "Service", "Constraint": + continue + case "LogConfig": + for _, fDiff := range oDiff.Fields { + switch fDiff.Name { + // force a destructive update if logger was enabled or disabled + case "Enabled": + destructive = true + break ObjectsLoop + } + } continue default: destructive = true diff --git a/scheduler/annotate_test.go b/scheduler/annotate_test.go index 9f651fc7092..42828fd634d 100644 --- a/scheduler/annotate_test.go +++ b/scheduler/annotate_test.go @@ -333,6 +333,27 @@ func TestAnnotateTask(t *testing.T) { Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, Desired: AnnotationForcesInplaceUpdate, }, + { + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { + Type: structs.DiffTypeAdded, + Name: "LogConfig", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "Enabled", + Old: "true", + New: "false", + }, + }, + }, + }, + }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesDestructiveUpdate, + }, { Diff: &structs.TaskDiff{ Type: structs.DiffTypeEdited, diff --git a/scheduler/util.go b/scheduler/util.go index d885a300dd4..3f7a482f77b 100644 --- a/scheduler/util.go +++ b/scheduler/util.go @@ -301,6 +301,13 @@ func tasksUpdated(jobA, jobB *structs.Job, taskGroup string) comparison { if !at.Identity.Equal(bt.Identity) { return difference("task identity", at.Identity, bt.Identity) } + + // Most LogConfig updates are in-place but if we change Enabled we need + // to recreate the task to stop/start log collection and change the + // stdout/stderr of the task + if at.LogConfig.Enabled != bt.LogConfig.Enabled { + return difference("task log enabled", at.LogConfig.Enabled, bt.LogConfig.Enabled) + } } // none of the fields that trigger a destructive update were modified, diff --git a/website/content/api-docs/client.mdx b/website/content/api-docs/client.mdx index 3f5c53222da..269c59a4ef5 100644 --- a/website/content/api-docs/client.mdx +++ b/website/content/api-docs/client.mdx @@ -603,7 +603,8 @@ fields: ## Stream Logs -This endpoint streams a task's stderr/stdout logs. +This endpoint streams a task's stderr/stdout logs. Note that if logging is set +to [enabled=false][] for the task, this endpoint will return a 404 error. | Method | Path | Produces | | ------ | ------------------------------ | ------------ | @@ -832,3 +833,4 @@ $ nomad operator api /v1/client/gc ``` [api-node-read]: /nomad/api-docs/nodes +[enabled=false]: /nomad/docs/job-specification/logs#enabled diff --git a/website/content/api-docs/jobs.mdx b/website/content/api-docs/jobs.mdx index e8bf00888f3..52110bdb52e 100644 --- a/website/content/api-docs/jobs.mdx +++ b/website/content/api-docs/jobs.mdx @@ -481,6 +481,7 @@ $ curl \ }, "KillTimeout": 5000000000, "LogConfig": { + "Enabled": true, "MaxFiles": 10, "MaxFileSizeMB": 10 }, @@ -695,6 +696,7 @@ $ curl \ "Leader": false, "Lifecycle": null, "LogConfig": { + "Enabled": true, "MaxFileSizeMB": 10, "MaxFiles": 10 }, @@ -913,6 +915,7 @@ $ curl \ "Leader": false, "Lifecycle": null, "LogConfig": { + "Enabled": true, "MaxFileSizeMB": 10, "MaxFiles": 10 }, @@ -1079,6 +1082,7 @@ $ curl \ "Leader": false, "Lifecycle": null, "LogConfig": { + "Enabled": true, "MaxFileSizeMB": 10, "MaxFiles": 10 }, diff --git a/website/content/api-docs/json-jobs.mdx b/website/content/api-docs/json-jobs.mdx index 36687139f34..06a7efb3d32 100644 --- a/website/content/api-docs/json-jobs.mdx +++ b/website/content/api-docs/json-jobs.mdx @@ -927,6 +927,8 @@ The `Affinity` object supports the following keys: The `LogConfig` object configures the log rotation policy for a task's `stdout` and `stderr`. The `LogConfig` object supports the following attributes: +- `Enabled` - Enables log collection. + - `MaxFiles` - The maximum number of rotated files Nomad will retain for `stdout` and `stderr`, each tracked individually. @@ -940,6 +942,7 @@ a validation error when a job is submitted. ```json { "LogConfig": { + "Enabled": true, "MaxFiles": 3, "MaxFileSizeMB": 10 } diff --git a/website/content/docs/job-specification/logs.mdx b/website/content/docs/job-specification/logs.mdx index f4424c72383..6ab62107d22 100644 --- a/website/content/docs/job-specification/logs.mdx +++ b/website/content/docs/job-specification/logs.mdx @@ -3,7 +3,7 @@ layout: docs page_title: logs Block - Job Specification description: |- The "logs" block configures the log rotation policy for a task's stdout and - stderr. Logging is enabled by default with sane defaults. The "logs" block + stderr. Logging is enabled by default with reasonable defaults. The "logs" block allows for finer-grained control over how Nomad handles log files. --- @@ -12,10 +12,9 @@ description: |- The `logs` block configures the log rotation policy for a task's `stdout` and -`stderr`. Logging is enabled by default with sane defaults (provided in the -parameters section below), and there is currently no way to disable logging for -tasks. The `logs` block allows for finer-grained control over how Nomad handles -log files. +`stderr`. Logging is enabled by default with reasonable defaults (provided in +the parameters section below). The `logs` block allows for finer-grained control +over how Nomad handles log files. Nomad's log rotation works by writing stdout/stderr output from tasks to a file inside the `alloc/logs/` directory with the following format: @@ -32,6 +31,7 @@ job "docs" { logs { max_files = 10 max_file_size = 10 + enabled = true } } } @@ -52,6 +52,17 @@ please see the [`nomad alloc logs`][logs-command] command. the total amount of disk space needed to retain the rotated set of files, Nomad will return a validation error when a job is submitted. +- `enabled` `(bool: true)` - Specifies that log collection should be enabled for + this task. If set to `false`, the task driver will attach stdout/stderr of the + task to `/dev/null` (or `NUL` on Windows). You should only disable log + collection if your application has some other way of emitting logs, such as + writing to a remote syslog server. Note that the `nomad alloc logs` command + and related APIs will return errors (404 "not found") if logging is disabled. + + Some task drivers such as `docker` support a [`disable_log_collection`][] + option. If the task driver's `disable_log_collection` option is set to `true`, + it will override `enabled=true` in the task's `logs` block. + ## `logs` Examples The following examples only show the `logs` blocks. Remember that the @@ -60,7 +71,7 @@ The following examples only show the `logs` blocks. Remember that the ### Configure Defaults This example shows a default logging configuration. Yes, it is empty on purpose. -Nomad automatically enables logging with sane defaults as described in the +Nomad automatically enables logging with reasonable defaults as described in the parameters section above. ```hcl @@ -81,3 +92,4 @@ logs { ``` [logs-command]: /nomad/docs/commands/alloc/logs 'Nomad logs command' +[`disable_log_collection`]: /nomad/docs/drivers/docker#disable_log_collection