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 3 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 38 additions & 11 deletions agent/agent.go
Expand Up @@ -535,16 +535,36 @@ 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 _, 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 +586,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 @@ -1700,12 +1726,13 @@ func (a *Agent) AddCheck(check *structs.HealthCheck, chkType *structs.CheckType,
}

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
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); 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
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
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