Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up subprocess handling and make shell use optional #3509

Merged
merged 20 commits into from
Oct 4, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 49 additions & 13 deletions agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -535,16 +535,40 @@ func (a *Agent) reloadWatches(cfg *config.RuntimeConfig) error {
var watchPlans []*watch.Plan
for _, params := range cfg.Watches {
// Parse the watches, excluding the handler
wp, err := watch.ParseExempt(params, []string{"handler"})
wp, err := watch.ParseExempt(params, []string{"handler", "args"})
if err != nil {
return fmt.Errorf("Failed to parse watch (%#v): %v", params, err)
}

// Get the handler
h := wp.Exempt["handler"]
if _, ok := h.(string); h == nil || !ok {
// Get the handler and subprocess arguments
handler, hasHandler := wp.Exempt["handler"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about issuing a deprecation warning for handler here and in health checks?

args, hasArgs := wp.Exempt["args"]
if hasHandler {
a.logger.Printf("[WARN] agent: The 'handler' field in watches has been deprecated " +
"and replaced with the 'args' field. See https://www.consul.io/docs/agent/watches.html")
}
if _, ok := handler.(string); hasHandler && !ok {
return fmt.Errorf("Watch handler must be a string")
}
if raw, ok := args.([]interface{}); hasArgs && ok {
var parsed []string
for _, arg := range raw {
if v, ok := arg.(string); !ok {
return fmt.Errorf("Watch args must be a list of strings")
} else {
parsed = append(parsed, v)
}
}
wp.Exempt["args"] = parsed
} else if hasArgs && !ok {
return fmt.Errorf("Watch args must be a list of strings")
}
if hasHandler && hasArgs {
return fmt.Errorf("Cannot define both watch handler and args")
}
if !hasHandler && !hasArgs {
return fmt.Errorf("Must define either watch handler or args")
}

// Store the watch plan
watchPlans = append(watchPlans, wp)
Expand All @@ -566,7 +590,13 @@ func (a *Agent) reloadWatches(cfg *config.RuntimeConfig) error {
for _, wp := range watchPlans {
a.watchPlans = append(a.watchPlans, wp)
go func(wp *watch.Plan) {
wp.Handler = makeWatchHandler(a.LogOutput, wp.Exempt["handler"])
var handler interface{}
if h, ok := wp.Exempt["handler"]; ok {
handler = h
} else {
handler = wp.Exempt["args"]
}
wp.Handler = makeWatchHandler(a.LogOutput, handler)
wp.LogOutput = a.LogOutput
if err := wp.Run(addr); err != nil {
a.logger.Printf("[ERR] Failed to run watch: %v", err)
Expand Down Expand Up @@ -1694,18 +1724,24 @@ func (a *Agent) AddCheck(check *structs.HealthCheck, chkType *structs.CheckType,
delete(a.checkMonitors, check.CheckID)
}
if chkType.Interval < MinInterval {
a.logger.Println(fmt.Sprintf("[WARN] agent: check '%s' has interval below minimum of %v",
check.CheckID, MinInterval))
a.logger.Printf("[WARN] agent: check '%s' has interval below minimum of %v",
check.CheckID, MinInterval)
chkType.Interval = MinInterval
}
if chkType.Script != "" {
a.logger.Printf("[WARN] agent: check %q has the 'script' field, which has been deprecated "+
"and replaced with the 'args' field. See https://www.consul.io/docs/agent/checks.html",
check.CheckID)
}

monitor := &CheckMonitor{
Notify: a.state,
CheckID: check.CheckID,
Script: chkType.Script,
Interval: chkType.Interval,
Timeout: chkType.Timeout,
Logger: a.logger,
Notify: a.state,
CheckID: check.CheckID,
Script: chkType.Script,
ScriptArgs: chkType.ScriptArgs,
Interval: chkType.Interval,
Timeout: chkType.Timeout,
Logger: a.logger,
}
monitor.Start()
a.checkMonitors[check.CheckID] = monitor
Expand Down
23 changes: 15 additions & 8 deletions agent/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,13 @@ type CheckNotifier interface {
// determine the health of a given check. It is compatible with
// nagios plugins and expects the output in the same format.
type CheckMonitor struct {
Notify CheckNotifier
CheckID types.CheckID
Script string
Interval time.Duration
Timeout time.Duration
Logger *log.Logger
Notify CheckNotifier
CheckID types.CheckID
Script string
ScriptArgs []string
Interval time.Duration
Timeout time.Duration
Logger *log.Logger

stop bool
stopCh chan struct{}
Expand Down Expand Up @@ -101,7 +102,13 @@ func (c *CheckMonitor) run() {
// check is invoked periodically to perform the script check
func (c *CheckMonitor) check() {
// Create the command
cmd, err := ExecScript(c.Script)
var cmd *exec.Cmd
var err error
if len(c.ScriptArgs) > 0 {
cmd, err = ExecSubprocess(c.ScriptArgs)
} else {
cmd, err = ExecScript(c.Script)
}
if err != nil {
c.Logger.Printf("[ERR] agent: failed to setup invoke '%s': %s", c.Script, err)
c.Notify.UpdateCheck(c.CheckID, api.HealthCritical, err.Error())
Expand All @@ -114,7 +121,7 @@ func (c *CheckMonitor) check() {
cmd.Stderr = output

// Start the check
if err := cmd.Start(); err != nil {
if err := StartSubprocess(cmd, false, c.Logger); err != nil {
c.Logger.Printf("[ERR] agent: failed to invoke '%s': %s", c.Script, err)
c.Notify.UpdateCheck(c.CheckID, api.HealthCritical, err.Error())
return
Expand Down
1 change: 1 addition & 0 deletions agent/config/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,7 @@ func (b *Builder) checkVal(v *CheckDefinition) *structs.CheckDefinition {
Token: b.stringVal(v.Token),
Status: b.stringVal(v.Status),
Script: b.stringVal(v.Script),
ScriptArgs: v.ScriptArgs,
HTTP: b.stringVal(v.HTTP),
Header: v.Header,
Method: b.stringVal(v.Method),
Expand Down
1 change: 1 addition & 0 deletions agent/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ type CheckDefinition struct {
Token *string `json:"token,omitempty" hcl:"token" mapstructure:"token"`
Status *string `json:"status,omitempty" hcl:"status" mapstructure:"status"`
Script *string `json:"script,omitempty" hcl:"script" mapstructure:"script"`
ScriptArgs []string `json:"args,omitempty" hcl:"args" mapstructure:"args"`
HTTP *string `json:"http,omitempty" hcl:"http" mapstructure:"http"`
Header map[string][]string `json:"header,omitempty" hcl:"header" mapstructure:"header"`
Method *string `json:"method,omitempty" hcl:"method" mapstructure:"method"`
Expand Down
Loading