add optional task field to group service checks

api/services.go

@@ -91,6 +91,7 @@ type ServiceCheck struct {
 	CheckRestart  *CheckRestart `mapstructure:"check_restart"`
 	GRPCService   string        `mapstructure:"grpc_service"`
 	GRPCUseTLS    bool          `mapstructure:"grpc_use_tls"`
+	TaskName      string        `mapstructure:"task"`
 }
 
 // Service represents a Consul service definition.

api/services_test.go

@@ -0,0 +1,56 @@
+package api
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestService_CheckRestart asserts Service.CheckRestart settings are properly
+// inherited by Checks.
+func TestService_CheckRestart(t *testing.T) {
+	job := &Job{Name: stringToPtr("job")}
+	tg := &TaskGroup{Name: stringToPtr("group")}
+	task := &Task{Name: "task"}
+	service := &Service{
+		CheckRestart: &CheckRestart{
+			Limit:          11,
+			Grace:          timeToPtr(11 * time.Second),
+			IgnoreWarnings: true,
+		},
+		Checks: []ServiceCheck{
+			{
+				Name: "all-set",
+				CheckRestart: &CheckRestart{
+					Limit:          22,
+					Grace:          timeToPtr(22 * time.Second),
+					IgnoreWarnings: true,
+				},
+			},
+			{
+				Name: "some-set",
+				CheckRestart: &CheckRestart{
+					Limit: 33,
+					Grace: timeToPtr(33 * time.Second),
+				},
+			},
+			{
+				Name: "unset",
+			},
+		},
+	}
+
+	service.Canonicalize(task, tg, job)
+	assert.Equal(t, service.Checks[0].CheckRestart.Limit, 22)
+	assert.Equal(t, *service.Checks[0].CheckRestart.Grace, 22*time.Second)
+	assert.True(t, service.Checks[0].CheckRestart.IgnoreWarnings)
+
+	assert.Equal(t, service.Checks[1].CheckRestart.Limit, 33)
+	assert.Equal(t, *service.Checks[1].CheckRestart.Grace, 33*time.Second)
+	assert.True(t, service.Checks[1].CheckRestart.IgnoreWarnings)
+
+	assert.Equal(t, service.Checks[2].CheckRestart.Limit, 11)
+	assert.Equal(t, *service.Checks[2].CheckRestart.Grace, 11*time.Second)
+	assert.True(t, service.Checks[2].CheckRestart.IgnoreWarnings)
+}

api/tasks_test.go

@@ -577,54 +577,6 @@ func TestTaskGroup_Canonicalize_MigrateStrategy(t *testing.T) {
 	}
 }
 
-// TestService_CheckRestart asserts Service.CheckRestart settings are properly
-// inherited by Checks.
-func TestService_CheckRestart(t *testing.T) {
-	job := &Job{Name: stringToPtr("job")}
-	tg := &TaskGroup{Name: stringToPtr("group")}
-	task := &Task{Name: "task"}
-	service := &Service{
-		CheckRestart: &CheckRestart{
-			Limit:          11,
-			Grace:          timeToPtr(11 * time.Second),
-			IgnoreWarnings: true,
-		},
-		Checks: []ServiceCheck{
-			{
-				Name: "all-set",
-				CheckRestart: &CheckRestart{
-					Limit:          22,
-					Grace:          timeToPtr(22 * time.Second),
-					IgnoreWarnings: true,
-				},
-			},
-			{
-				Name: "some-set",
-				CheckRestart: &CheckRestart{
-					Limit: 33,
-					Grace: timeToPtr(33 * time.Second),
-				},
-			},
-			{
-				Name: "unset",
-			},
-		},
-	}
-
-	service.Canonicalize(task, tg, job)
-	assert.Equal(t, service.Checks[0].CheckRestart.Limit, 22)
-	assert.Equal(t, *service.Checks[0].CheckRestart.Grace, 22*time.Second)
-	assert.True(t, service.Checks[0].CheckRestart.IgnoreWarnings)
-
-	assert.Equal(t, service.Checks[1].CheckRestart.Limit, 33)
-	assert.Equal(t, *service.Checks[1].CheckRestart.Grace, 33*time.Second)
-	assert.True(t, service.Checks[1].CheckRestart.IgnoreWarnings)
-
-	assert.Equal(t, service.Checks[2].CheckRestart.Limit, 11)
-	assert.Equal(t, *service.Checks[2].CheckRestart.Grace, 11*time.Second)
-	assert.True(t, service.Checks[2].CheckRestart.IgnoreWarnings)
-}
-
 // TestSpread_Canonicalize asserts that the spread stanza is canonicalized correctly
 func TestSpread_Canonicalize(t *testing.T) {
 	job := &Job{

command/agent/job_endpoint.go

@@ -1028,6 +1028,7 @@ func ApiServicesToStructs(in []*api.Service) []*structs.Service {
 					Method:        check.Method,
 					GRPCService:   check.GRPCService,
 					GRPCUseTLS:    check.GRPCUseTLS,
+					TaskName:      check.TaskName,
 				}
 				if check.CheckRestart != nil {
 					out[i].Checks[j].CheckRestart = &structs.CheckRestart{

command/agent/job_endpoint_test.go

@@ -1531,6 +1531,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 									Limit:          3,
 									IgnoreWarnings: true,
 								},
+								TaskName: "task1",
 							},
 						},
 					},
@@ -1864,6 +1865,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 									Limit:          3,
 									IgnoreWarnings: true,
 								},
+								TaskName: "task1",
 							},
 						},
 					},

jobspec/parse_service.go

@@ -406,6 +406,7 @@ func parseChecks(service *api.Service, checkObjs *ast.ObjectList) error {
 			"address_mode",
 			"grpc_service",
 			"grpc_use_tls",
+			"task",
 		}
 		if err := helper.CheckHCLKeys(co.Val, valid); err != nil {
 			return multierror.Prefix(err, "check ->")

jobspec/parse_test.go

@@ -960,6 +960,51 @@ func TestParse(t *testing.T) {
 			},
 			false,
 		},
+
+		{
+			"tg-service-check.hcl",
+			&api.Job{
+				ID:   helper.StringToPtr("group_service_check_script"),
+				Name: helper.StringToPtr("group_service_check_script"),
+				TaskGroups: []*api.TaskGroup{
+					{
+						Name:  helper.StringToPtr("group"),
+						Count: helper.IntToPtr(1),
+						Networks: []*api.NetworkResource{
+							{
+								Mode: "bridge",
+								ReservedPorts: []api.Port{
+									{
+										Label: "http",
+										Value: 80,
+										To:    8080,
+									},
+								},
+							},
+						},
+						Services: []*api.Service{
+							{
+								Name:      "foo-service",
+								PortLabel: "http",
+								Checks: []api.ServiceCheck{
+									{
+										Name:          "check-name",
+										Type:          "script",
+										Command:       "/bin/true",
+										Interval:      time.Duration(10 * time.Second),
+										Timeout:       time.Duration(2 * time.Second),
+										InitialStatus: "passing",
+										TaskName:      "foo",
+									},
+								},
+							},
+						},
+						Tasks: []*api.Task{{Name: "foo"}},
+					},
+				},
+			},
+			false,
+		},
 	}
 
 	for _, tc := range cases {

jobspec/test-fixtures/tg-service-check.hcl

@@ -0,0 +1,32 @@
+job "group_service_check_script" {
+
+  group "group" {
+    count = 1
+
+    network {
+      mode = "bridge"
+
+      port "http" {
+	static = 80
+	to = 8080
+      }
+    }
+
+    service {
+      name = "foo-service"
+      port = "http"
+
+      check {
+        name           = "check-name"
+        type           = "script"
+        command        = "/bin/true"
+        interval       = "10s"
+        timeout        = "2s"
+        initial_status = "passing"
+        task           = "foo"
+      }
+    }
+
+    task "foo" {}
+  }
+}

nomad/structs/diff_test.go

@@ -4569,6 +4569,12 @@ func TestTaskDiff(t *testing.T) {
 										Old:  "false",
 										New:  "false",
 									},
+									{
+										Type: DiffTypeNone,
+										Name: "TaskName",
+										Old:  "",
+										New:  "",
+									},
 									{
 										Type: DiffTypeNone,
 										Name: "Timeout",

nomad/structs/services.go

@@ -53,6 +53,7 @@ type ServiceCheck struct {
 	CheckRestart  *CheckRestart       // If and when a task should be restarted based on checks
 	GRPCService   string              // Service for GRPC checks
 	GRPCUseTLS    bool                // Whether or not to use TLS for GRPC checks
+	TaskName      string              // What task to execute this check in
 }
 
 // Copy the stanza recursively. Returns nil if nil.
@@ -90,6 +91,10 @@ func (sc *ServiceCheck) Equals(o *ServiceCheck) bool {
 		return false
 	}
 
+	if sc.TaskName != o.TaskName {
+		return false
+	}
+
 	if sc.Command != o.Command {
 		return false
 	}
@@ -386,24 +391,24 @@ func (s *Service) Validate() error {
 	serviceNameStripped := args.ReplaceEnvWithPlaceHolder(s.Name, "ENV-VAR")
 
 	if err := s.ValidateName(serviceNameStripped); err != nil {
-		mErr.Errors = append(mErr.Errors, fmt.Errorf("service name must be valid per RFC 1123 and can contain only alphanumeric characters or dashes: %q", s.Name))
+		mErr.Errors = append(mErr.Errors, fmt.Errorf("Service name must be valid per RFC 1123 and can contain only alphanumeric characters or dashes: %q", s.Name))
 	}
 
 	switch s.AddressMode {
 	case "", AddressModeAuto, AddressModeHost, AddressModeDriver:
 		// OK
 	default:
-		mErr.Errors = append(mErr.Errors, fmt.Errorf("service address_mode must be %q, %q, or %q; not %q", AddressModeAuto, AddressModeHost, AddressModeDriver, s.AddressMode))
+		mErr.Errors = append(mErr.Errors, fmt.Errorf("Service address_mode must be %q, %q, or %q; not %q", AddressModeAuto, AddressModeHost, AddressModeDriver, s.AddressMode))
 	}
 
 	for _, c := range s.Checks {
 		if s.PortLabel == "" && c.PortLabel == "" && c.RequiresPort() {
-			mErr.Errors = append(mErr.Errors, fmt.Errorf("check %s invalid: check requires a port but neither check nor service %+q have a port", c.Name, s.Name))
+			mErr.Errors = append(mErr.Errors, fmt.Errorf("Check %s invalid: check requires a port but neither check nor service %+q have a port", c.Name, s.Name))
 			continue
 		}
 
 		if err := c.validate(); err != nil {
-			mErr.Errors = append(mErr.Errors, fmt.Errorf("check %s invalid: %v", c.Name, err))
+			mErr.Errors = append(mErr.Errors, fmt.Errorf("Check %s invalid: %v", c.Name, err))
 		}
 	}
 
@@ -425,7 +430,7 @@ func (s *Service) ValidateName(name string) error {
 	// (https://tools.ietf.org/html/rfc2782).
 	re := regexp.MustCompile(`^(?i:[a-z0-9]|[a-z0-9][a-z0-9\-]{0,61}[a-z0-9])$`)
 	if !re.MatchString(name) {
-		return fmt.Errorf("service name must be valid per RFC 1123 and can contain only alphanumeric characters or dashes and must be no longer than 63 characters: %q", name)
+		return fmt.Errorf("Service name must be valid per RFC 1123 and can contain only alphanumeric characters or dashes and must be no longer than 63 characters: %q", name)
 	}
 	return nil
 }

nomad/structs/structs.go

@@ -4908,6 +4908,12 @@ func (tg *TaskGroup) Validate(j *Job) error {
 		mErr.Errors = append(mErr.Errors, outer)
 	}
 
+	// Validate task group and task services
+	if err := tg.validateServices(); err != nil {
+		outer := fmt.Errorf("Task group service validation failed: %v", err)
+		mErr.Errors = append(mErr.Errors, outer)
+	}
+
 	// Validate the tasks
 	for _, task := range tg.Tasks {
 		// Validate the task does not reference undefined volume mounts
@@ -5000,6 +5006,62 @@ func (tg *TaskGroup) validateNetworks() error {
 	return mErr.ErrorOrNil()
 }
 
+// validateServices runs Service.Validate() on group-level services,
+// checks that group services do not conflict with task services and that
+// group service checks that refer to tasks only refer to tasks that exist.
+func (tg *TaskGroup) validateServices() error {
+	var mErr multierror.Error
+	knownTasks := make(map[string]struct{})
+	knownServices := make(map[string]struct{})
+
+	// Create a map of known tasks and their services so we can compare
+	// vs the group-level services and checks
+	for _, task := range tg.Tasks {
+		knownTasks[task.Name] = struct{}{}
+		if task.Services == nil {
+			continue
+		}
+		for _, service := range task.Services {
+			if _, ok := knownServices[service.Name+service.PortLabel]; ok {
+				mErr.Errors = append(mErr.Errors, fmt.Errorf("Service %s is duplicate", service.Name))
+			}
+			for _, check := range service.Checks {
+				if check.TaskName != "" {
+					mErr.Errors = append(mErr.Errors, fmt.Errorf("Check %s is invalid: only task group service checks can be assigned tasks", check.Name))
+				}
+			}
+			knownServices[service.Name+service.PortLabel] = struct{}{}
+		}
+	}
+	for i, service := range tg.Services {
+		if err := service.Validate(); err != nil {
+			outer := fmt.Errorf("Service[%d] %s validation failed: %s", i, service.Name, err)
+			mErr.Errors = append(mErr.Errors, outer)
+			// we break here to avoid the risk of crashing on null-pointer
+			// access in a later step, accepting that we might miss out on
+			// error messages to provide the user.
+			continue
+		}
+		if _, ok := knownServices[service.Name+service.PortLabel]; ok {
+			mErr.Errors = append(mErr.Errors, fmt.Errorf("Service %s is duplicate", service.Name))
+		}
+		knownServices[service.Name+service.PortLabel] = struct{}{}
+		for _, check := range service.Checks {
+			if check.TaskName != "" {
+				if check.Type != ServiceCheckScript && check.Type != ServiceCheckGRPC {
+					mErr.Errors = append(mErr.Errors,
+						fmt.Errorf("Check %s invalid: only script and gRPC checks should have tasks", check.Name))
+				}
+				if _, ok := knownTasks[check.TaskName]; !ok {
+					mErr.Errors = append(mErr.Errors,
+						fmt.Errorf("Check %s invalid: refers to non-existent task %s", check.Name, check.TaskName))
+				}
+			}
+		}
+	}
+	return mErr.ErrorOrNil()
+}
+
 // Warnings returns a list of warnings that may be from dubious settings or
 // deprecation warnings.
 func (tg *TaskGroup) Warnings(j *Job) error {