From 092057a32b413591e2e8350c183c97ae3ebdd84d Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Wed, 13 Sep 2017 17:27:08 -0700 Subject: [PATCH] Canonicalize and Merge CheckRestart in api --- api/tasks.go | 70 ++++++++++++++++++++++++++++++----- command/agent/job_endpoint.go | 7 ---- nomad/structs/structs.go | 46 ----------------------- 3 files changed, 61 insertions(+), 62 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 291971b71..9b7886384 100644 --- a/api/tasks.go +++ b/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) } } diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 7c7b0089b..5fcf65161 100644 --- a/command/agent/job_endpoint.go +++ b/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) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 5335a7105..728e3b145 100644 --- a/nomad/structs/structs.go +++ b/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() }