Canonicalize and Merge CheckRestart in api

api/tasks.go

@@ -87,6 +87,63 @@ type CheckRestart struct {
 	IgnoreWarnings bool           `mapstructure:"ignore_warnings"`
 }
 
+// Canonicalize CheckRestart fields if not nil.
+func (c *CheckRestart) Canonicalize() {
+	if c == nil {
+		return
+	}
+
+	if c.Grace == nil {
+		c.Grace = helper.TimeToPtr(1 * time.Second)
+	}
+}
+
+// Copy returns a copy of CheckRestart or nil if unset.
+func (c *CheckRestart) Copy() *CheckRestart {
+	if c == nil {
+		return nil
+	}
+
+	nc := new(CheckRestart)
+	nc.Limit = c.Limit
+	if c.Grace != nil {
+		g := *c.Grace
+		nc.Grace = &g
+	}
+	nc.IgnoreWarnings = c.IgnoreWarnings
+	return nc
+}
+
+// Merge values from other CheckRestart over default values on this
+// CheckRestart and return merged copy.
+func (c *CheckRestart) Merge(o *CheckRestart) *CheckRestart {
+	if c == nil {
+		// Just return other
+		return o
+	}
+
+	nc := c.Copy()
+
+	if o == nil {
+		// Nothing to merge
+		return nc
+	}
+
+	if nc.Limit == 0 {
+		nc.Limit = o.Limit
+	}
+
+	if nc.Grace == nil {
+		nc.Grace = o.Grace
+	}
+
+	if nc.IgnoreWarnings {
+		nc.IgnoreWarnings = o.IgnoreWarnings
+	}
+
+	return nc
+}
+
 // The ServiceCheck data model represents the consul health check that
 // Nomad registers for a Task
 type ServiceCheck struct {
@@ -107,14 +164,6 @@ type ServiceCheck struct {
 	CheckRestart  *CheckRestart `mapstructure:"check_restart"`
 }
 
-func (c *ServiceCheck) Canonicalize() {
-	if c.CheckRestart != nil {
-		if c.CheckRestart.Grace == nil {
-			c.CheckRestart.Grace = helper.TimeToPtr(1 * time.Second)
-		}
-	}
-}
-
 // The Service model represents a Consul service definition
 type Service struct {
 	Id           string
@@ -136,8 +185,11 @@ func (s *Service) Canonicalize(t *Task, tg *TaskGroup, job *Job) {
 		s.AddressMode = "auto"
 	}
 
+	s.CheckRestart.Canonicalize()
+
 	for _, c := range s.Checks {
-		c.Canonicalize()
+		c.CheckRestart.Canonicalize()
+		c.CheckRestart = c.CheckRestart.Merge(s.CheckRestart)
 	}
 }
 

command/agent/job_endpoint.go

@@ -685,13 +685,6 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
 				Tags:        service.Tags,
 				AddressMode: service.AddressMode,
 			}
-			if service.CheckRestart != nil {
-				structsTask.Services[i].CheckRestart = &structs.CheckRestart{
-					Limit:          service.CheckRestart.Limit,
-					Grace:          *service.CheckRestart.Grace,
-					IgnoreWarnings: service.CheckRestart.IgnoreWarnings,
-				}
-			}
 
 			if l := len(service.Checks); l != 0 {
 				structsTask.Services[i].Checks = make([]*structs.ServiceCheck, l)

nomad/structs/structs.go

@@ -2775,36 +2775,6 @@ func (c *CheckRestart) Copy() *CheckRestart {
 	return nc
 }
 
-// Merge non-zero values from other CheckRestart into a copy of this
-// CheckRestart. Returns nil iff both are nil.
-func (c *CheckRestart) Merge(o *CheckRestart) *CheckRestart {
-	if c == nil {
-		// Just return other
-		return o
-	}
-
-	nc := c.Copy()
-
-	if o == nil {
-		// Nothing to merge
-		return nc.Copy()
-	}
-
-	if nc.Limit == 0 {
-		nc.Limit = o.Limit
-	}
-
-	if nc.Grace == 0 {
-		nc.Grace = o.Grace
-	}
-
-	if nc.IgnoreWarnings {
-		nc.IgnoreWarnings = o.IgnoreWarnings
-	}
-
-	return nc
-}
-
 func (c *CheckRestart) Validate() error {
 	if c == nil {
 		return nil
@@ -3008,9 +2978,6 @@ type Service struct {
 
 	Tags   []string        // List of tags for the service
 	Checks []*ServiceCheck // List of checks associated with the service
-
-	// CheckRestart will be propagated to Checks if set.
-	CheckRestart *CheckRestart
 }
 
 func (s *Service) Copy() *Service {
@@ -3020,7 +2987,6 @@ func (s *Service) Copy() *Service {
 	ns := new(Service)
 	*ns = *s
 	ns.Tags = helper.CopySliceString(ns.Tags)
-	ns.CheckRestart = s.CheckRestart.Copy()
 
 	if s.Checks != nil {
 		checks := make([]*ServiceCheck, len(ns.Checks))
@@ -3056,14 +3022,6 @@ func (s *Service) Canonicalize(job string, taskGroup string, task string) {
 	for _, check := range s.Checks {
 		check.Canonicalize(s.Name)
 	}
-
-	// If CheckRestart is set propagate it to checks
-	if s.CheckRestart != nil {
-		for _, check := range s.Checks {
-			// Merge Service CheckRestart into Check's so Check's takes precedence
-			check.CheckRestart = check.CheckRestart.Merge(s.CheckRestart)
-		}
-	}
 }
 
 // Validate checks if the Check definition is valid
@@ -3098,10 +3056,6 @@ func (s *Service) Validate() error {
 		}
 	}
 
-	if s.CheckRestart != nil && len(s.Checks) == 0 {
-		mErr.Errors = append(mErr.Errors, fmt.Errorf("check_restart specified but no checks"))
-	}
-
 	return mErr.ErrorOrNil()
 }