From ee996e6ad58c5af92467e38c8f9dbc214ab5f424 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 4 May 2023 14:56:24 +0000 Subject: [PATCH 1/2] backport of commit 4ef8b96b82525b637ab9dabbab0fe989e7240f91 --- .changelog/17087.txt | 3 ++ api/tasks.go | 17 +++++--- client/allocrunner/taskrunner/logmon_hook.go | 24 +++++------ .../taskrunner/logmon_hook_test.go | 2 +- command/agent/job_endpoint.go | 22 ++++++---- command/agent/job_endpoint_test.go | 41 ++++++++++++++----- jobspec/parse_task.go | 1 + jobspec/parse_test.go | 2 +- jobspec/test-fixtures/basic.hcl | 2 +- nomad/structs/diff_test.go | 24 +++++------ nomad/structs/structs.go | 10 ++--- scheduler/annotate.go | 2 +- scheduler/annotate_test.go | 2 +- scheduler/util.go | 6 +-- website/content/api-docs/client.mdx | 4 +- website/content/api-docs/jobs.mdx | 8 ++-- website/content/api-docs/json-jobs.mdx | 4 +- .../content/docs/job-specification/logs.mdx | 8 ++-- 18 files changed, 110 insertions(+), 72 deletions(-) create mode 100644 .changelog/17087.txt diff --git a/.changelog/17087.txt b/.changelog/17087.txt new file mode 100644 index 00000000000..bc6ee01b723 --- /dev/null +++ b/.changelog/17087.txt @@ -0,0 +1,3 @@ +```release-note:bug +logging: Fixed a bug where alloc logs would not be collected after an upgrade to 1.5.4 +``` diff --git a/api/tasks.go b/api/tasks.go index 2008e3113f3..77fb66046f4 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -636,16 +636,21 @@ 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"` - Enabled *bool `mapstructure:"enabled" hcl:"enabled,optional"` + MaxFiles *int `mapstructure:"max_files" hcl:"max_files,optional"` + MaxFileSizeMB *int `mapstructure:"max_file_size" hcl:"max_file_size,optional"` + + // COMPAT(1.7.0): Enabled had to be swapped for Disabled to fix a backwards + // compatibility bug when restoring pre-1.5.4 jobs. Remove in 1.7.0 + Enabled *bool `mapstructure:"enabled" hcl:"enabled,optional"` + + Disabled *bool `mapstructure:"disabled" hcl:"disabled,optional"` } func DefaultLogConfig() *LogConfig { return &LogConfig{ MaxFiles: pointerOf(10), MaxFileSizeMB: pointerOf(10), - Enabled: pointerOf(true), + Disabled: pointerOf(false), } } @@ -656,8 +661,8 @@ func (l *LogConfig) Canonicalize() { if l.MaxFileSizeMB == nil { l.MaxFileSizeMB = pointerOf(10) } - if l.Enabled == nil { - l.Enabled = pointerOf(true) + if l.Disabled == nil { + l.Disabled = pointerOf(false) } } diff --git a/client/allocrunner/taskrunner/logmon_hook.go b/client/allocrunner/taskrunner/logmon_hook.go index d27a6a63085..73bc5bcd27c 100644 --- a/client/allocrunner/taskrunner/logmon_hook.go +++ b/client/allocrunner/taskrunner/logmon_hook.go @@ -43,7 +43,7 @@ type logmonHook struct { type logmonHookConfig struct { logDir string - enabled bool + disabled bool stdoutFifo string stderrFifo string } @@ -60,12 +60,12 @@ func newLogMonHook(tr *TaskRunner, logger hclog.Logger) *logmonHook { func newLogMonHookConfig(taskName string, logCfg *structs.LogConfig, logDir string) *logmonHookConfig { cfg := &logmonHookConfig{ - logDir: logDir, - enabled: logCfg.Enabled, + logDir: logDir, + disabled: logCfg.Disabled, } // If logging is disabled configure task's stdout/err to point to devnull - if !logCfg.Enabled { + if logCfg.Disabled { cfg.stdoutFifo = os.DevNull cfg.stderrFifo = os.DevNull return cfg @@ -113,7 +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.isLoggingEnabled() { + if h.isLoggingDisabled() { return nil } @@ -148,10 +148,10 @@ func (h *logmonHook) Prestart(ctx context.Context, } } -func (h *logmonHook) isLoggingEnabled() bool { - if !h.config.enabled { +func (h *logmonHook) isLoggingDisabled() bool { + if h.config.disabled { h.logger.Debug("log collection is disabled by task") - return false + return true } // internal plugins have access to a capability to disable logging and @@ -159,16 +159,16 @@ func (h *logmonHook) isLoggingEnabled() bool { // currently only the docker driver exposes this to users. ic, ok := h.runner.driver.(drivers.InternalCapabilitiesDriver) if !ok { - return true + return false } caps := ic.InternalCapabilities() if caps.DisableLogCollection { h.logger.Debug("log collection is disabled by driver") - return false + return true } - return true + return false } func (h *logmonHook) prestartOneLoop(ctx context.Context, req *interfaces.TaskPrestartRequest) error { @@ -216,7 +216,7 @@ 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() { + if h.isLoggingDisabled() { return nil } diff --git a/client/allocrunner/taskrunner/logmon_hook_test.go b/client/allocrunner/taskrunner/logmon_hook_test.go index 88439bb3bf2..2588d862e32 100644 --- a/client/allocrunner/taskrunner/logmon_hook_test.go +++ b/client/allocrunner/taskrunner/logmon_hook_test.go @@ -109,7 +109,7 @@ func TestTaskRunner_LogmonHook_Disabled(t *testing.T) { alloc := mock.BatchAlloc() task := alloc.Job.TaskGroups[0].Tasks[0] - task.LogConfig.Enabled = false + task.LogConfig.Disabled = true dir := t.TempDir() diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 4565fe0a9a9..209ef94dd99 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1180,11 +1180,7 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup, structsTask.Resources = ApiResourcesToStructs(apiTask.Resources) - structsTask.LogConfig = &structs.LogConfig{ - MaxFiles: *apiTask.LogConfig.MaxFiles, - MaxFileSizeMB: *apiTask.LogConfig.MaxFileSizeMB, - Enabled: *apiTask.LogConfig.Enabled, - } + structsTask.LogConfig = apiLogConfigToStructs(apiTask.LogConfig) if len(apiTask.Artifacts) > 0 { structsTask.Artifacts = []*structs.TaskArtifact{} @@ -1747,11 +1743,23 @@ func apiLogConfigToStructs(in *api.LogConfig) *structs.LogConfig { if in == nil { return nil } - return &structs.LogConfig{ - Enabled: *in.Enabled, + out := &structs.LogConfig{ + Disabled: dereferenceBool(in.Disabled), MaxFiles: dereferenceInt(in.MaxFiles), MaxFileSizeMB: dereferenceInt(in.MaxFileSizeMB), } + if in.Disabled == nil { + // COMPAT(1.7.0): fix for backwards compatibility + out.Disabled = !dereferenceBool(in.Enabled) + } + return out +} + +func dereferenceBool(in *bool) bool { + if in == nil { + return false + } + return *in } func dereferenceInt(in *int) int { diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index b2f6240771f..3d23a500119 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2707,7 +2707,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { KillTimeout: pointer.Of(10 * time.Second), KillSignal: "SIGQUIT", LogConfig: &api.LogConfig{ - Enabled: pointer.Of(true), + Disabled: pointer.Of(true), MaxFiles: pointer.Of(10), MaxFileSizeMB: pointer.Of(100), }, @@ -3125,7 +3125,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { KillTimeout: 10 * time.Second, KillSignal: "SIGQUIT", LogConfig: &structs.LogConfig{ - Enabled: true, + Disabled: true, MaxFiles: 10, MaxFileSizeMB: 100, }, @@ -3282,7 +3282,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { KillTimeout: pointer.Of(10 * time.Second), KillSignal: "SIGQUIT", LogConfig: &api.LogConfig{ - Enabled: pointer.Of(true), + Disabled: pointer.Of(true), MaxFiles: pointer.Of(10), MaxFileSizeMB: pointer.Of(100), }, @@ -3408,7 +3408,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { KillTimeout: 10 * time.Second, KillSignal: "SIGQUIT", LogConfig: &structs.LogConfig{ - Enabled: true, + Disabled: true, MaxFiles: 10, MaxFileSizeMB: 100, }, @@ -3579,16 +3579,37 @@ func TestConversion_dereferenceInt(t *testing.T) { func TestConversion_apiLogConfigToStructs(t *testing.T) { ci.Parallel(t) - require.Nil(t, apiLogConfigToStructs(nil)) - require.Equal(t, &structs.LogConfig{ - Enabled: true, + must.Nil(t, apiLogConfigToStructs(nil)) + must.Eq(t, &structs.LogConfig{ + Disabled: true, MaxFiles: 2, MaxFileSizeMB: 8, }, apiLogConfigToStructs(&api.LogConfig{ - Enabled: pointer.Of(true), + Disabled: pointer.Of(true), MaxFiles: pointer.Of(2), MaxFileSizeMB: pointer.Of(8), })) + + // COMPAT(1.7.0): verify backwards compatibility fixes + must.Eq(t, &structs.LogConfig{Disabled: true}, + apiLogConfigToStructs(&api.LogConfig{ + Enabled: pointer.Of(false), + })) + must.Eq(t, &structs.LogConfig{Disabled: false}, + apiLogConfigToStructs(&api.LogConfig{ + Enabled: pointer.Of(true), + })) + + must.Eq(t, &structs.LogConfig{Disabled: false}, + apiLogConfigToStructs(&api.LogConfig{ + Disabled: pointer.Of(false), + })) + must.Eq(t, &structs.LogConfig{Disabled: false}, + apiLogConfigToStructs(&api.LogConfig{ + Enabled: pointer.Of(false), + Disabled: pointer.Of(false), + })) + } func TestConversion_apiResourcesToStructs(t *testing.T) { @@ -3659,7 +3680,7 @@ func TestConversion_apiConnectSidecarTaskToStructs(t *testing.T) { Meta: meta, KillTimeout: &timeout, LogConfig: &structs.LogConfig{ - Enabled: true, + Disabled: true, MaxFiles: 2, MaxFileSizeMB: 8, }, @@ -3678,7 +3699,7 @@ func TestConversion_apiConnectSidecarTaskToStructs(t *testing.T) { Meta: meta, KillTimeout: &timeout, LogConfig: &api.LogConfig{ - Enabled: pointer.Of(true), + Disabled: pointer.Of(true), MaxFiles: pointer.Of(2), MaxFileSizeMB: pointer.Of(8), }, diff --git a/jobspec/parse_task.go b/jobspec/parse_task.go index 1287af02fa3..aa33cb5fa2b 100644 --- a/jobspec/parse_task.go +++ b/jobspec/parse_task.go @@ -260,6 +260,7 @@ func parseTask(item *ast.ObjectItem, keys []string) (*api.Task, error) { "max_files", "max_file_size", "enabled", + "disabled", } 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 5ba608287b6..1d2f0974775 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -347,7 +347,7 @@ func TestParse(t *testing.T) { LogConfig: &api.LogConfig{ MaxFiles: intToPtr(14), MaxFileSizeMB: intToPtr(101), - Enabled: boolToPtr(true), + Disabled: boolToPtr(false), }, Artifacts: []*api.TaskArtifact{ { diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index e418304f5db..1a713ffd723 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -191,7 +191,7 @@ job "binstore-storagelocker" { } logs { - enabled = true + disabled = false max_files = 14 max_file_size = 101 } diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index ef2e6c307d4..5991ee6a9bf 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -4417,7 +4417,7 @@ func TestTaskDiff(t *testing.T) { LogConfig: &LogConfig{ MaxFiles: 1, MaxFileSizeMB: 10, - Enabled: false, + Disabled: true, }, }, Expected: &TaskDiff{ @@ -4429,9 +4429,9 @@ func TestTaskDiff(t *testing.T) { Fields: []*FieldDiff{ { Type: DiffTypeAdded, - Name: "Enabled", + Name: "Disabled", Old: "", - New: "false", + New: "true", }, { Type: DiffTypeAdded, @@ -4456,7 +4456,7 @@ func TestTaskDiff(t *testing.T) { LogConfig: &LogConfig{ MaxFiles: 1, MaxFileSizeMB: 10, - Enabled: false, + Disabled: true, }, }, New: &Task{}, @@ -4469,8 +4469,8 @@ func TestTaskDiff(t *testing.T) { Fields: []*FieldDiff{ { Type: DiffTypeDeleted, - Name: "Enabled", - Old: "false", + Name: "Disabled", + Old: "true", New: "", }, { @@ -4496,14 +4496,14 @@ func TestTaskDiff(t *testing.T) { LogConfig: &LogConfig{ MaxFiles: 1, MaxFileSizeMB: 10, - Enabled: false, + Disabled: false, }, }, New: &Task{ LogConfig: &LogConfig{ MaxFiles: 2, MaxFileSizeMB: 20, - Enabled: true, + Disabled: true, }, }, Expected: &TaskDiff{ @@ -4515,7 +4515,7 @@ func TestTaskDiff(t *testing.T) { Fields: []*FieldDiff{ { Type: DiffTypeEdited, - Name: "Enabled", + Name: "Disabled", Old: "false", New: "true", }, @@ -4543,14 +4543,14 @@ func TestTaskDiff(t *testing.T) { LogConfig: &LogConfig{ MaxFiles: 1, MaxFileSizeMB: 10, - Enabled: false, + Disabled: false, }, }, New: &Task{ LogConfig: &LogConfig{ MaxFiles: 1, MaxFileSizeMB: 20, - Enabled: true, + Disabled: true, }, }, Expected: &TaskDiff{ @@ -4562,7 +4562,7 @@ func TestTaskDiff(t *testing.T) { Fields: []*FieldDiff{ { Type: DiffTypeEdited, - Name: "Enabled", + Name: "Disabled", Old: "false", New: "true", }, diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 95341afc34e..fd4699626b0 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -7147,7 +7147,7 @@ const ( type LogConfig struct { MaxFiles int MaxFileSizeMB int - Enabled bool + Disabled bool } func (l *LogConfig) Equal(o *LogConfig) bool { @@ -7163,7 +7163,7 @@ func (l *LogConfig) Equal(o *LogConfig) bool { return false } - if l.Enabled != o.Enabled { + if l.Disabled != o.Disabled { return false } @@ -7177,7 +7177,7 @@ func (l *LogConfig) Copy() *LogConfig { return &LogConfig{ MaxFiles: l.MaxFiles, MaxFileSizeMB: l.MaxFileSizeMB, - Enabled: l.Enabled, + Disabled: l.Disabled, } } @@ -7186,13 +7186,13 @@ func DefaultLogConfig() *LogConfig { return &LogConfig{ MaxFiles: 10, MaxFileSizeMB: 10, - Enabled: true, + Disabled: false, } } // 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 +// MaxFileSizeMB, we can't validate that they're unset if Disabled=true func (l *LogConfig) Validate() error { var mErr multierror.Error if l.MaxFiles < 1 { diff --git a/scheduler/annotate.go b/scheduler/annotate.go index d1d5020a32f..fe887b8fa34 100644 --- a/scheduler/annotate.go +++ b/scheduler/annotate.go @@ -191,7 +191,7 @@ FieldsLoop: for _, fDiff := range oDiff.Fields { switch fDiff.Name { // force a destructive update if logger was enabled or disabled - case "Enabled": + case "Disabled": destructive = true break ObjectsLoop } diff --git a/scheduler/annotate_test.go b/scheduler/annotate_test.go index 42828fd634d..4fce3b0d447 100644 --- a/scheduler/annotate_test.go +++ b/scheduler/annotate_test.go @@ -343,7 +343,7 @@ func TestAnnotateTask(t *testing.T) { Fields: []*structs.FieldDiff{ { Type: structs.DiffTypeAdded, - Name: "Enabled", + Name: "Disabled", Old: "true", New: "false", }, diff --git a/scheduler/util.go b/scheduler/util.go index 3f7a482f77b..b75f36659d5 100644 --- a/scheduler/util.go +++ b/scheduler/util.go @@ -302,11 +302,11 @@ func tasksUpdated(jobA, jobB *structs.Job, taskGroup string) comparison { return difference("task identity", at.Identity, bt.Identity) } - // Most LogConfig updates are in-place but if we change Enabled we need + // Most LogConfig updates are in-place but if we change Disabled 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) + if at.LogConfig.Disabled != bt.LogConfig.Disabled { + return difference("task log disabled", at.LogConfig.Disabled, bt.LogConfig.Disabled) } } diff --git a/website/content/api-docs/client.mdx b/website/content/api-docs/client.mdx index 269c59a4ef5..208250eaf7d 100644 --- a/website/content/api-docs/client.mdx +++ b/website/content/api-docs/client.mdx @@ -604,7 +604,7 @@ fields: ## Stream 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. +to [disabled=true][] for the task, this endpoint will return a 404 error. | Method | Path | Produces | | ------ | ------------------------------ | ------------ | @@ -833,4 +833,4 @@ $ nomad operator api /v1/client/gc ``` [api-node-read]: /nomad/api-docs/nodes -[enabled=false]: /nomad/docs/job-specification/logs#enabled +[disabled=true]: /nomad/docs/job-specification/logs#disabled diff --git a/website/content/api-docs/jobs.mdx b/website/content/api-docs/jobs.mdx index 52110bdb52e..df71cfa6c0e 100644 --- a/website/content/api-docs/jobs.mdx +++ b/website/content/api-docs/jobs.mdx @@ -481,7 +481,7 @@ $ curl \ }, "KillTimeout": 5000000000, "LogConfig": { - "Enabled": true, + "Disabled": false, "MaxFiles": 10, "MaxFileSizeMB": 10 }, @@ -696,7 +696,7 @@ $ curl \ "Leader": false, "Lifecycle": null, "LogConfig": { - "Enabled": true, + "Disabled": false, "MaxFileSizeMB": 10, "MaxFiles": 10 }, @@ -915,7 +915,7 @@ $ curl \ "Leader": false, "Lifecycle": null, "LogConfig": { - "Enabled": true, + "Disabled": false, "MaxFileSizeMB": 10, "MaxFiles": 10 }, @@ -1082,7 +1082,7 @@ $ curl \ "Leader": false, "Lifecycle": null, "LogConfig": { - "Enabled": true, + "Disabled": false, "MaxFileSizeMB": 10, "MaxFiles": 10 }, diff --git a/website/content/api-docs/json-jobs.mdx b/website/content/api-docs/json-jobs.mdx index 06a7efb3d32..20766dd3a56 100644 --- a/website/content/api-docs/json-jobs.mdx +++ b/website/content/api-docs/json-jobs.mdx @@ -927,7 +927,7 @@ 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. +- `Disabled` - Disables log collection. - `MaxFiles` - The maximum number of rotated files Nomad will retain for `stdout` and `stderr`, each tracked individually. @@ -942,7 +942,7 @@ a validation error when a job is submitted. ```json { "LogConfig": { - "Enabled": true, + "Disabled": false, "MaxFiles": 3, "MaxFileSizeMB": 10 } diff --git a/website/content/docs/job-specification/logs.mdx b/website/content/docs/job-specification/logs.mdx index 6ab62107d22..67984f07ffa 100644 --- a/website/content/docs/job-specification/logs.mdx +++ b/website/content/docs/job-specification/logs.mdx @@ -31,7 +31,7 @@ job "docs" { logs { max_files = 10 max_file_size = 10 - enabled = true + disabled = false } } } @@ -52,8 +52,8 @@ 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 +- `disabled` `(bool: false)` - Specifies that log collection should be enabled for + this task. If set to `true`, 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 @@ -61,7 +61,7 @@ please see the [`nomad alloc logs`][logs-command] command. 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. + it will override `disabled=false` in the task's `logs` block. ## `logs` Examples From 178f7a08b57393f89c6784d155a1dc1793fdf69a Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 4 May 2023 19:00:26 +0000 Subject: [PATCH 2/2] backport of commit 7aaef0ebbe7c3ed1cc1b741f3ed78574ceba66c0 --- api/tasks.go | 4 ++-- command/agent/job_endpoint.go | 8 ++------ command/agent/job_endpoint_test.go | 8 +++++--- jobspec/parse_task.go | 2 +- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 77fb66046f4..3f5a4eaaa93 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -639,8 +639,8 @@ type LogConfig struct { MaxFiles *int `mapstructure:"max_files" hcl:"max_files,optional"` MaxFileSizeMB *int `mapstructure:"max_file_size" hcl:"max_file_size,optional"` - // COMPAT(1.7.0): Enabled had to be swapped for Disabled to fix a backwards - // compatibility bug when restoring pre-1.5.4 jobs. Remove in 1.7.0 + // COMPAT(1.6.0): Enabled had to be swapped for Disabled to fix a backwards + // compatibility bug when restoring pre-1.5.4 jobs. Remove in 1.6.0 Enabled *bool `mapstructure:"enabled" hcl:"enabled,optional"` Disabled *bool `mapstructure:"disabled" hcl:"disabled,optional"` diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 209ef94dd99..fe6f32251e1 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1743,16 +1743,12 @@ func apiLogConfigToStructs(in *api.LogConfig) *structs.LogConfig { if in == nil { return nil } - out := &structs.LogConfig{ + + return &structs.LogConfig{ Disabled: dereferenceBool(in.Disabled), MaxFiles: dereferenceInt(in.MaxFiles), MaxFileSizeMB: dereferenceInt(in.MaxFileSizeMB), } - if in.Disabled == nil { - // COMPAT(1.7.0): fix for backwards compatibility - out.Disabled = !dereferenceBool(in.Enabled) - } - return out } func dereferenceBool(in *bool) bool { diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 3d23a500119..ad65cc9c46b 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -3590,8 +3590,9 @@ func TestConversion_apiLogConfigToStructs(t *testing.T) { MaxFileSizeMB: pointer.Of(8), })) - // COMPAT(1.7.0): verify backwards compatibility fixes - must.Eq(t, &structs.LogConfig{Disabled: true}, + // COMPAT(1.6.0): verify backwards compatibility fixes + // Note: we're intentionally ignoring the Enabled: false case + must.Eq(t, &structs.LogConfig{Disabled: false}, apiLogConfigToStructs(&api.LogConfig{ Enabled: pointer.Of(false), })) @@ -3599,7 +3600,8 @@ func TestConversion_apiLogConfigToStructs(t *testing.T) { apiLogConfigToStructs(&api.LogConfig{ Enabled: pointer.Of(true), })) - + must.Eq(t, &structs.LogConfig{Disabled: false}, + apiLogConfigToStructs(&api.LogConfig{})) must.Eq(t, &structs.LogConfig{Disabled: false}, apiLogConfigToStructs(&api.LogConfig{ Disabled: pointer.Of(false), diff --git a/jobspec/parse_task.go b/jobspec/parse_task.go index aa33cb5fa2b..d9359aa08c9 100644 --- a/jobspec/parse_task.go +++ b/jobspec/parse_task.go @@ -259,7 +259,7 @@ func parseTask(item *ast.ObjectItem, keys []string) (*api.Task, error) { valid := []string{ "max_files", "max_file_size", - "enabled", + "enabled", // COMPAT(1.6.0): remove in favor of disabled "disabled", } if err := checkHCLKeys(logsBlock.Val, valid); err != nil {