From 729931aced36defd83495e4ea1d130d79218f33c Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Thu, 9 Apr 2020 21:01:16 -0600 Subject: [PATCH 1/2] connect: enable configuring sidecar_task.name Before, the submitted jobspec for sidecar_task would pass through 2 key validation steps - once for the subset specific to connect sidecar task definitions, and once again for the set of normal task definition where the task would actually get unmarshalled. The valid keys for the normal task definition did not include "name", which is supposed to be configurable for the sidecar task. To fix this, just eliminate the double validation step, and instead pass-in the correct set of keys to validate against to the one generic task parser. Fixes #7680 --- jobspec/parse_service.go | 27 +------ jobspec/parse_task.go | 73 ++++++++++++------- jobspec/parse_test.go | 22 ++++++ .../service-connect-sidecar_task-name.hcl | 15 ++++ 4 files changed, 83 insertions(+), 54 deletions(-) create mode 100644 jobspec/test-fixtures/service-connect-sidecar_task-name.hcl diff --git a/jobspec/parse_service.go b/jobspec/parse_service.go index 00963b327f9..24c35176df7 100644 --- a/jobspec/parse_service.go +++ b/jobspec/parse_service.go @@ -263,32 +263,7 @@ func parseSidecarService(o *ast.ObjectItem) (*api.ConsulSidecarService, error) { } func parseSidecarTask(item *ast.ObjectItem) (*api.SidecarTask, error) { - // We need this later - var listVal *ast.ObjectList - if ot, ok := item.Val.(*ast.ObjectType); ok { - listVal = ot.List - } else { - return nil, fmt.Errorf("should be an object") - } - - // Check for invalid keys - valid := []string{ - "config", - "driver", - "env", - "kill_timeout", - "logs", - "meta", - "resources", - "shutdown_delay", - "user", - "kill_signal", - } - if err := helper.CheckHCLKeys(listVal, valid); err != nil { - return nil, err - } - - task, err := parseTask(item) + task, err := parseTask(item, sidecarTaskKeys) if err != nil { return nil, err } diff --git a/jobspec/parse_task.go b/jobspec/parse_task.go index 230ef32a820..1e6ace7ca18 100644 --- a/jobspec/parse_task.go +++ b/jobspec/parse_task.go @@ -12,6 +12,48 @@ import ( "github.com/mitchellh/mapstructure" ) +var ( + normalTaskKeys = []string{ + "artifact", + "config", + "constraint", + "affinity", + "dispatch_payload", + "lifecycle", + "driver", + "env", + "kill_timeout", + "leader", + "logs", + "meta", + "resources", + "restart", + "service", + "shutdown_delay", + "template", + "user", + "vault", + "kill_signal", + "kind", + "volume_mount", + "csi_plugin", + } + + sidecarTaskKeys = []string{ + "name", + "driver", + "user", + "config", + "env", + "resources", + "meta", + "logs", + "kill_timeout", + "shutdown_delay", + "kill_signal", + } +) + func parseTasks(result *[]*api.Task, list *ast.ObjectList) error { list = list.Children() if len(list.Items) == 0 { @@ -29,7 +71,7 @@ func parseTasks(result *[]*api.Task, list *ast.ObjectList) error { } seen[n] = struct{}{} - t, err := parseTask(item) + t, err := parseTask(item, normalTaskKeys) if err != nil { return multierror.Prefix(err, fmt.Sprintf("'%s',", n)) } @@ -42,7 +84,7 @@ func parseTasks(result *[]*api.Task, list *ast.ObjectList) error { return nil } -func parseTask(item *ast.ObjectItem) (*api.Task, error) { +func parseTask(item *ast.ObjectItem, keys []string) (*api.Task, error) { // We need this later var listVal *ast.ObjectList if ot, ok := item.Val.(*ast.ObjectType); ok { @@ -52,32 +94,7 @@ func parseTask(item *ast.ObjectItem) (*api.Task, error) { } // Check for invalid keys - valid := []string{ - "artifact", - "config", - "constraint", - "affinity", - "dispatch_payload", - "lifecycle", - "driver", - "env", - "kill_timeout", - "leader", - "logs", - "meta", - "resources", - "restart", - "service", - "shutdown_delay", - "template", - "user", - "vault", - "kill_signal", - "kind", - "volume_mount", - "csi_plugin", - } - if err := helper.CheckHCLKeys(listVal, valid); err != nil { + if err := helper.CheckHCLKeys(listVal, keys); err != nil { return nil, err } diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index c445a681829..c171f9acf47 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -894,6 +894,28 @@ func TestParse(t *testing.T) { }, false, }, + { + "service-connect-sidecar_task-name.hcl", + &api.Job{ + ID: helper.StringToPtr("sidecar_task_name"), + Name: helper.StringToPtr("sidecar_task_name"), + Type: helper.StringToPtr("service"), + TaskGroups: []*api.TaskGroup{{ + Name: helper.StringToPtr("group"), + Services: []*api.Service{{ + Name: "example", + Connect: &api.ConsulConnect{ + Native: false, + SidecarService: &api.ConsulSidecarService{}, + SidecarTask: &api.SidecarTask{ + Name: "my-sidecar", + }, + }, + }}, + }}, + }, + false, + }, { "reschedule-job.hcl", &api.Job{ diff --git a/jobspec/test-fixtures/service-connect-sidecar_task-name.hcl b/jobspec/test-fixtures/service-connect-sidecar_task-name.hcl new file mode 100644 index 00000000000..e46d8ce3d5f --- /dev/null +++ b/jobspec/test-fixtures/service-connect-sidecar_task-name.hcl @@ -0,0 +1,15 @@ +job "sidecar_task_name" { + type = "service" + + group "group" { + service { + name = "example" + connect { + sidecar_service {} + sidecar_task { + name = "my-sidecar" + } + } + } + } +} \ No newline at end of file From 0eb2844900d66c89c11754f54e1b7e91ad1b9a1a Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Fri, 10 Apr 2020 09:49:19 -0600 Subject: [PATCH 2/2] connect: extract common task keys --- jobspec/parse_task.go | 41 +++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/jobspec/parse_task.go b/jobspec/parse_task.go index 1e6ace7ca18..2a1ebc60b76 100644 --- a/jobspec/parse_task.go +++ b/jobspec/parse_task.go @@ -13,45 +13,38 @@ import ( ) var ( - normalTaskKeys = []string{ - "artifact", + commonTaskKeys = []string{ + "driver", + "user", "config", + "env", + "resources", + "meta", + "logs", + "kill_timeout", + "shutdown_delay", + "kill_signal", + } + + normalTaskKeys = append(commonTaskKeys, + "artifact", "constraint", "affinity", "dispatch_payload", "lifecycle", - "driver", - "env", - "kill_timeout", "leader", - "logs", - "meta", - "resources", "restart", "service", - "shutdown_delay", "template", - "user", "vault", - "kill_signal", "kind", "volume_mount", "csi_plugin", - } + ) - sidecarTaskKeys = []string{ + sidecarTaskKeys = append(commonTaskKeys, "name", - "driver", - "user", - "config", - "env", - "resources", - "meta", - "logs", - "kill_timeout", - "shutdown_delay", - "kill_signal", - } + ) ) func parseTasks(result *[]*api.Task, list *ast.ObjectList) error {