diff --git a/agent/agent.go b/agent/agent.go index c6d3a1a62b..a58c8a940d 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -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"] + 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) @@ -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) @@ -1684,6 +1714,7 @@ func (a *Agent) AddCheck(check *structs.HealthCheck, chkType *structs.CheckType, DockerContainerID: chkType.DockerContainerID, Shell: chkType.Shell, Script: chkType.Script, + ScriptArgs: chkType.ScriptArgs, Interval: chkType.Interval, Logger: a.logger, client: a.dockerClient, @@ -1697,18 +1728,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 diff --git a/agent/agent_test.go b/agent/agent_test.go index 8a649f87df..19acf63d6d 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1928,9 +1928,9 @@ func TestAgent_reloadWatches(t *testing.T) { newConf := *a.config newConf.Watches = []map[string]interface{}{ { - "type": "key", - "key": "asdf", - "handler": "ls", + "type": "key", + "key": "asdf", + "args": []interface{}{"ls"}, }, } if err := a.reloadWatches(&newConf); err != nil { @@ -1942,9 +1942,9 @@ func TestAgent_reloadWatches(t *testing.T) { newConf.HTTPAddrs = make([]net.Addr, 0) newConf.Watches = []map[string]interface{}{ { - "type": "key", - "key": "asdf", - "handler": "ls", + "type": "key", + "key": "asdf", + "args": []interface{}{"ls"}, }, } if err := a.reloadWatches(&newConf); err != nil { @@ -1955,9 +1955,9 @@ func TestAgent_reloadWatches(t *testing.T) { newConf.HTTPSAddrs = make([]net.Addr, 0) newConf.Watches = []map[string]interface{}{ { - "type": "key", - "key": "asdf", - "handler": "ls", + "type": "key", + "key": "asdf", + "args": []interface{}{"ls"}, }, } if err := a.reloadWatches(&newConf); err == nil || !strings.Contains(err.Error(), "watch plans require an HTTP or HTTPS endpoint") { diff --git a/agent/check.go b/agent/check.go index d99dcc33e8..3d86579f94 100644 --- a/agent/check.go +++ b/agent/check.go @@ -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{} @@ -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()) @@ -524,6 +531,7 @@ type CheckDocker struct { Notify CheckNotifier CheckID types.CheckID Script string + ScriptArgs []string DockerContainerID string Shell string Interval time.Duration @@ -599,7 +607,13 @@ func (c *CheckDocker) check() { } func (c *CheckDocker) doCheck() (string, *circbuf.Buffer, error) { - cmd := []string{c.Shell, "-c", c.Script} + var cmd []string + if len(c.ScriptArgs) > 0 { + cmd = c.ScriptArgs + } else { + cmd = []string{c.Shell, "-c", c.Script} + } + execID, err := c.client.CreateExec(c.DockerContainerID, cmd) if err != nil { return api.HealthCritical, nil, err diff --git a/agent/check_test.go b/agent/check_test.go index d89302c520..eb544d9775 100644 --- a/agent/check_test.go +++ b/agent/check_test.go @@ -20,7 +20,7 @@ import ( "github.com/hashicorp/consul/types" ) -func TestCheckMonitor(t *testing.T) { +func TestCheckMonitor_Script(t *testing.T) { tests := []struct { script, status string }{ @@ -54,16 +54,51 @@ func TestCheckMonitor(t *testing.T) { } } +func TestCheckMonitor_Args(t *testing.T) { + tests := []struct { + args []string + status string + }{ + {[]string{"sh", "-c", "exit 0"}, "passing"}, + {[]string{"sh", "-c", "exit 1"}, "warning"}, + {[]string{"sh", "-c", "exit 2"}, "critical"}, + {[]string{"foobarbaz"}, "critical"}, + } + + for _, tt := range tests { + t.Run(tt.status, func(t *testing.T) { + notif := mock.NewNotify() + check := &CheckMonitor{ + Notify: notif, + CheckID: types.CheckID("foo"), + ScriptArgs: tt.args, + Interval: 25 * time.Millisecond, + Logger: log.New(ioutil.Discard, UniqueID(), log.LstdFlags), + } + check.Start() + defer check.Stop() + retry.Run(t, func(r *retry.R) { + if got, want := notif.Updates("foo"), 2; got < want { + r.Fatalf("got %d updates want at least %d", got, want) + } + if got, want := notif.State("foo"), tt.status; got != want { + r.Fatalf("got state %q want %q", got, want) + } + }) + }) + } +} + func TestCheckMonitor_Timeout(t *testing.T) { // t.Parallel() // timing test. no parallel notif := mock.NewNotify() check := &CheckMonitor{ - Notify: notif, - CheckID: types.CheckID("foo"), - Script: "sleep 1 && exit 0", - Interval: 50 * time.Millisecond, - Timeout: 25 * time.Millisecond, - Logger: log.New(ioutil.Discard, UniqueID(), log.LstdFlags), + Notify: notif, + CheckID: types.CheckID("foo"), + ScriptArgs: []string{"sh", "-c", "sleep 1 && exit 0"}, + Interval: 50 * time.Millisecond, + Timeout: 25 * time.Millisecond, + Logger: log.New(ioutil.Discard, UniqueID(), log.LstdFlags), } check.Start() defer check.Stop() @@ -83,11 +118,11 @@ func TestCheckMonitor_RandomStagger(t *testing.T) { // t.Parallel() // timing test. no parallel notif := mock.NewNotify() check := &CheckMonitor{ - Notify: notif, - CheckID: types.CheckID("foo"), - Script: "exit 0", - Interval: 25 * time.Millisecond, - Logger: log.New(ioutil.Discard, UniqueID(), log.LstdFlags), + Notify: notif, + CheckID: types.CheckID("foo"), + ScriptArgs: []string{"sh", "-c", "exit 0"}, + Interval: 25 * time.Millisecond, + Logger: log.New(ioutil.Discard, UniqueID(), log.LstdFlags), } check.Start() defer check.Stop() @@ -108,11 +143,11 @@ func TestCheckMonitor_LimitOutput(t *testing.T) { t.Parallel() notif := mock.NewNotify() check := &CheckMonitor{ - Notify: notif, - CheckID: types.CheckID("foo"), - Script: "od -N 81920 /dev/urandom", - Interval: 25 * time.Millisecond, - Logger: log.New(ioutil.Discard, UniqueID(), log.LstdFlags), + Notify: notif, + CheckID: types.CheckID("foo"), + ScriptArgs: []string{"od", "-N", "81920", "/dev/urandom"}, + Interval: 25 * time.Millisecond, + Logger: log.New(ioutil.Discard, UniqueID(), log.LstdFlags), } check.Start() defer check.Stop() @@ -775,7 +810,7 @@ func TestCheck_Docker(t *testing.T) { check := &CheckDocker{ Notify: notif, CheckID: id, - Script: "/health.sh", + ScriptArgs: []string{"/health.sh"}, DockerContainerID: "123", Interval: 25 * time.Millisecond, client: c, diff --git a/agent/config/builder.go b/agent/config/builder.go index d8c33cef33..b9dcdee359 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -871,6 +871,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), diff --git a/agent/config/config.go b/agent/config/config.go index 20a291db97..4db585bd38 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -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"` diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 9caf9a216a..6c713da9eb 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -1967,6 +1967,7 @@ func TestFullConfig(t *testing.T) { "token": "oo4BCTgJ", "status": "qLykAl5u", "script": "dhGfIF8n", + "args": ["f3BemRjy", "e5zgpef7"], "http": "29B93haH", "header": { "hBq0zn1q": [ "2a9o9ZKP", "vKwA5lR6" ], @@ -1991,6 +1992,7 @@ func TestFullConfig(t *testing.T) { "token": "toO59sh8", "status": "9RlWsXMV", "script": "8qbd8tWw", + "args": ["4BAJttck", "4D2NPtTQ"], "http": "dohLcyQ2", "header": { "ZBfTin3L": [ "1sDbEqYG", "lJGASsWK" ], @@ -2014,6 +2016,7 @@ func TestFullConfig(t *testing.T) { "token": "a3nQzHuy", "status": "irj26nf3", "script": "FJsI1oXt", + "args": ["9s526ogY", "gSlOHj1w"], "http": "yzhgsQ7Y", "header": { "zcqwA8dO": [ "qb1zx0DL", "sXCxPFsD" ], @@ -2139,6 +2142,7 @@ func TestFullConfig(t *testing.T) { "status": "rCvn53TH", "notes": "fti5lfF3", "script": "rtj34nfd", + "args": ["16WRUmwS", "QWk7j7ae"], "http": "dl3Fgme3", "header": { "rjm4DEd3": ["2m3m2Fls"], @@ -2161,6 +2165,7 @@ func TestFullConfig(t *testing.T) { "notes": "yP5nKbW0", "status": "7oLMEyfu", "script": "NlUQ3nTE", + "args": ["5wEZtZpv", "0Ihyk8cS"], "http": "KyDjGY9H", "header": { "gv5qefTz": [ "5Olo2pMG", "PvvKWQU5" ], @@ -2182,6 +2187,7 @@ func TestFullConfig(t *testing.T) { "notes": "SVqApqeM", "status": "XXkVoZXt", "script": "IXLZTM6E", + "args": ["wD05Bvao", "rLYB7kQC"], "http": "kyICZsn8", "header": { "4ebP5vL4": [ "G20SrL5Q", "DwPKlMbo" ], @@ -2214,6 +2220,7 @@ func TestFullConfig(t *testing.T) { "status": "pDQKEhWL", "notes": "Yt8EDLev", "script": "MDu7wjlD", + "args": ["81EDZLPa", "bPY5X8xd"], "http": "qzHYvmJO", "header": { "UkpmZ3a3": ["2dfzXuxZ"], @@ -2245,6 +2252,7 @@ func TestFullConfig(t *testing.T) { "notes": "CQy86DH0", "status": "P0SWDvrk", "script": "6BhLJ7R9", + "args": ["EXvkYIuG", "BATOyt6h"], "http": "u97ByEiW", "header": { "MUlReo8L": [ "AUZG7wHG", "gsN0Dc2N" ], @@ -2266,6 +2274,7 @@ func TestFullConfig(t *testing.T) { "notes": "jKChDOdl", "status": "5qFz6OZn", "script": "PbdxFZ3K", + "args": ["NMtYWlT9", "vj74JXsm"], "http": "1LBDJhw4", "header": { "cXPmnv1M": [ "imDqfaBx", "NFxZ1bQe" ], @@ -2338,6 +2347,11 @@ func TestFullConfig(t *testing.T) { "datacenter": "GyE6jpeW", "key": "j9lF1Tve", "handler": "90N7S4LN" + }, { + "type": "keyprefix", + "datacenter": "fYrl3F5d", + "key": "sl3Dffu7", + "args": ["dltjDJ2a", "flEa7C2d"] } ] }`, @@ -2383,6 +2397,7 @@ func TestFullConfig(t *testing.T) { token = "oo4BCTgJ" status = "qLykAl5u" script = "dhGfIF8n" + args = ["f3BemRjy", "e5zgpef7"] http = "29B93haH" header = { hBq0zn1q = [ "2a9o9ZKP", "vKwA5lR6" ] @@ -2407,6 +2422,7 @@ func TestFullConfig(t *testing.T) { token = "toO59sh8" status = "9RlWsXMV" script = "8qbd8tWw" + args = ["4BAJttck", "4D2NPtTQ"] http = "dohLcyQ2" header = { "ZBfTin3L" = [ "1sDbEqYG", "lJGASsWK" ] @@ -2430,6 +2446,7 @@ func TestFullConfig(t *testing.T) { token = "a3nQzHuy" status = "irj26nf3" script = "FJsI1oXt" + args = ["9s526ogY", "gSlOHj1w"] http = "yzhgsQ7Y" header = { "zcqwA8dO" = [ "qb1zx0DL", "sXCxPFsD" ] @@ -2555,6 +2572,7 @@ func TestFullConfig(t *testing.T) { status = "rCvn53TH" notes = "fti5lfF3" script = "rtj34nfd" + args = ["16WRUmwS", "QWk7j7ae"] http = "dl3Fgme3" header = { rjm4DEd3 = [ "2m3m2Fls" ] @@ -2577,6 +2595,7 @@ func TestFullConfig(t *testing.T) { notes = "yP5nKbW0" status = "7oLMEyfu" script = "NlUQ3nTE" + args = ["5wEZtZpv", "0Ihyk8cS"] http = "KyDjGY9H" header = { "gv5qefTz" = [ "5Olo2pMG", "PvvKWQU5" ] @@ -2598,6 +2617,7 @@ func TestFullConfig(t *testing.T) { notes = "SVqApqeM" status = "XXkVoZXt" script = "IXLZTM6E" + args = ["wD05Bvao", "rLYB7kQC"] http = "kyICZsn8" header = { "4ebP5vL4" = [ "G20SrL5Q", "DwPKlMbo" ] @@ -2630,6 +2650,7 @@ func TestFullConfig(t *testing.T) { status = "pDQKEhWL" notes = "Yt8EDLev" script = "MDu7wjlD" + args = ["81EDZLPa", "bPY5X8xd"] http = "qzHYvmJO" header = { UkpmZ3a3 = [ "2dfzXuxZ" ] @@ -2661,6 +2682,7 @@ func TestFullConfig(t *testing.T) { notes = "CQy86DH0" status = "P0SWDvrk" script = "6BhLJ7R9" + args = ["EXvkYIuG", "BATOyt6h"] http = "u97ByEiW" header = { "MUlReo8L" = [ "AUZG7wHG", "gsN0Dc2N" ] @@ -2682,6 +2704,7 @@ func TestFullConfig(t *testing.T) { notes = "jKChDOdl" status = "5qFz6OZn" script = "PbdxFZ3K" + args = ["NMtYWlT9", "vj74JXsm"] http = "1LBDJhw4" header = { "cXPmnv1M" = [ "imDqfaBx", "NFxZ1bQe" ], @@ -2753,6 +2776,11 @@ func TestFullConfig(t *testing.T) { datacenter = "GyE6jpeW" key = "j9lF1Tve" handler = "90N7S4LN" + }, { + type = "keyprefix" + datacenter = "fYrl3F5d" + key = "sl3Dffu7" + args = ["dltjDJ2a", "flEa7C2d"] }] `} @@ -2929,14 +2957,15 @@ func TestFullConfig(t *testing.T) { CertFile: "7s4QAzDk", Checks: []*structs.CheckDefinition{ &structs.CheckDefinition{ - ID: "uAjE6m9Z", - Name: "QsZRGpYr", - Notes: "VJ7Sk4BY", - ServiceID: "lSulPcyz", - Token: "toO59sh8", - Status: "9RlWsXMV", - Script: "8qbd8tWw", - HTTP: "dohLcyQ2", + ID: "uAjE6m9Z", + Name: "QsZRGpYr", + Notes: "VJ7Sk4BY", + ServiceID: "lSulPcyz", + Token: "toO59sh8", + Status: "9RlWsXMV", + Script: "8qbd8tWw", + ScriptArgs: []string{"4BAJttck", "4D2NPtTQ"}, + HTTP: "dohLcyQ2", Header: map[string][]string{ "ZBfTin3L": []string{"1sDbEqYG", "lJGASsWK"}, "Ui0nU99X": []string{"LMccm3Qe", "k5H5RggQ"}, @@ -2952,14 +2981,15 @@ func TestFullConfig(t *testing.T) { DeregisterCriticalServiceAfter: 14232 * time.Second, }, &structs.CheckDefinition{ - ID: "Cqq95BhP", - Name: "3qXpkS0i", - Notes: "sb5qLTex", - ServiceID: "CmUUcRna", - Token: "a3nQzHuy", - Status: "irj26nf3", - Script: "FJsI1oXt", - HTTP: "yzhgsQ7Y", + ID: "Cqq95BhP", + Name: "3qXpkS0i", + Notes: "sb5qLTex", + ServiceID: "CmUUcRna", + Token: "a3nQzHuy", + Status: "irj26nf3", + Script: "FJsI1oXt", + ScriptArgs: []string{"9s526ogY", "gSlOHj1w"}, + HTTP: "yzhgsQ7Y", Header: map[string][]string{ "zcqwA8dO": []string{"qb1zx0DL", "sXCxPFsD"}, "qxvdnSE9": []string{"6wBPUYdF", "YYh8wtSZ"}, @@ -2975,14 +3005,15 @@ func TestFullConfig(t *testing.T) { DeregisterCriticalServiceAfter: 2366 * time.Second, }, &structs.CheckDefinition{ - ID: "fZaCAXww", - Name: "OOM2eo0f", - Notes: "zXzXI9Gt", - ServiceID: "L8G0QNmR", - Token: "oo4BCTgJ", - Status: "qLykAl5u", - Script: "dhGfIF8n", - HTTP: "29B93haH", + ID: "fZaCAXww", + Name: "OOM2eo0f", + Notes: "zXzXI9Gt", + ServiceID: "L8G0QNmR", + Token: "oo4BCTgJ", + Status: "qLykAl5u", + Script: "dhGfIF8n", + ScriptArgs: []string{"f3BemRjy", "e5zgpef7"}, + HTTP: "29B93haH", Header: map[string][]string{ "hBq0zn1q": {"2a9o9ZKP", "vKwA5lR6"}, "f3r6xFtM": {"RyuIdDWv", "QbxEcIUM"}, @@ -3090,12 +3121,13 @@ func TestFullConfig(t *testing.T) { EnableTagOverride: true, Checks: []*structs.CheckType{ &structs.CheckType{ - CheckID: "qmfeO5if", - Name: "atDGP7n5", - Status: "pDQKEhWL", - Notes: "Yt8EDLev", - Script: "MDu7wjlD", - HTTP: "qzHYvmJO", + CheckID: "qmfeO5if", + Name: "atDGP7n5", + Status: "pDQKEhWL", + Notes: "Yt8EDLev", + Script: "MDu7wjlD", + ScriptArgs: []string{"81EDZLPa", "bPY5X8xd"}, + HTTP: "qzHYvmJO", Header: map[string][]string{ "UkpmZ3a3": {"2dfzXuxZ"}, "cVFpko4u": {"gGqdEB6k", "9LsRo22u"}, @@ -3122,12 +3154,13 @@ func TestFullConfig(t *testing.T) { EnableTagOverride: true, Checks: structs.CheckTypes{ &structs.CheckType{ - CheckID: "GTti9hCo", - Name: "9OOS93ne", - Notes: "CQy86DH0", - Status: "P0SWDvrk", - Script: "6BhLJ7R9", - HTTP: "u97ByEiW", + CheckID: "GTti9hCo", + Name: "9OOS93ne", + Notes: "CQy86DH0", + Status: "P0SWDvrk", + Script: "6BhLJ7R9", + ScriptArgs: []string{"EXvkYIuG", "BATOyt6h"}, + HTTP: "u97ByEiW", Header: map[string][]string{ "MUlReo8L": {"AUZG7wHG", "gsN0Dc2N"}, "1UJXjVrT": {"OJgxzTfk", "xZZrFsq7"}, @@ -3143,12 +3176,13 @@ func TestFullConfig(t *testing.T) { DeregisterCriticalServiceAfter: 84282 * time.Second, }, &structs.CheckType{ - CheckID: "UHsDeLxG", - Name: "PQSaPWlT", - Notes: "jKChDOdl", - Status: "5qFz6OZn", - Script: "PbdxFZ3K", - HTTP: "1LBDJhw4", + CheckID: "UHsDeLxG", + Name: "PQSaPWlT", + Notes: "jKChDOdl", + Status: "5qFz6OZn", + Script: "PbdxFZ3K", + ScriptArgs: []string{"NMtYWlT9", "vj74JXsm"}, + HTTP: "1LBDJhw4", Header: map[string][]string{ "cXPmnv1M": {"imDqfaBx", "NFxZ1bQe"}, "vr7wY7CS": {"EtCoNPPL", "9vAarJ5s"}, @@ -3175,12 +3209,13 @@ func TestFullConfig(t *testing.T) { EnableTagOverride: true, Checks: structs.CheckTypes{ &structs.CheckType{ - CheckID: "Zv99e9Ka", - Name: "sgV4F7Pk", - Notes: "yP5nKbW0", - Status: "7oLMEyfu", - Script: "NlUQ3nTE", - HTTP: "KyDjGY9H", + CheckID: "Zv99e9Ka", + Name: "sgV4F7Pk", + Notes: "yP5nKbW0", + Status: "7oLMEyfu", + Script: "NlUQ3nTE", + ScriptArgs: []string{"5wEZtZpv", "0Ihyk8cS"}, + HTTP: "KyDjGY9H", Header: map[string][]string{ "gv5qefTz": {"5Olo2pMG", "PvvKWQU5"}, "SHOVq1Vv": {"jntFhyym", "GYJh32pp"}, @@ -3196,12 +3231,13 @@ func TestFullConfig(t *testing.T) { DeregisterCriticalServiceAfter: 8482 * time.Second, }, &structs.CheckType{ - CheckID: "G79O6Mpr", - Name: "IEqrzrsd", - Notes: "SVqApqeM", - Status: "XXkVoZXt", - Script: "IXLZTM6E", - HTTP: "kyICZsn8", + CheckID: "G79O6Mpr", + Name: "IEqrzrsd", + Notes: "SVqApqeM", + Status: "XXkVoZXt", + Script: "IXLZTM6E", + ScriptArgs: []string{"wD05Bvao", "rLYB7kQC"}, + HTTP: "kyICZsn8", Header: map[string][]string{ "4ebP5vL4": {"G20SrL5Q", "DwPKlMbo"}, "p2UI34Qz": {"UsG1D0Qh", "NHhRiB6s"}, @@ -3217,12 +3253,13 @@ func TestFullConfig(t *testing.T) { DeregisterCriticalServiceAfter: 4992 * time.Second, }, &structs.CheckType{ - CheckID: "RMi85Dv8", - Name: "iehanzuq", - Status: "rCvn53TH", - Notes: "fti5lfF3", - Script: "rtj34nfd", - HTTP: "dl3Fgme3", + CheckID: "RMi85Dv8", + Name: "iehanzuq", + Status: "rCvn53TH", + Notes: "fti5lfF3", + Script: "rtj34nfd", + ScriptArgs: []string{"16WRUmwS", "QWk7j7ae"}, + HTTP: "dl3Fgme3", Header: map[string][]string{ "rjm4DEd3": {"2m3m2Fls"}, "l4HwQ112": {"fk56MNlo", "dhLK56aZ"}, @@ -3297,6 +3334,12 @@ func TestFullConfig(t *testing.T) { "key": "j9lF1Tve", "handler": "90N7S4LN", }, + map[string]interface{}{ + "type": "keyprefix", + "datacenter": "fYrl3F5d", + "key": "sl3Dffu7", + "args": []interface{}{"dltjDJ2a", "flEa7C2d"}, + }, }, } @@ -3632,6 +3675,7 @@ func TestSanitize(t *testing.T) { "Name": "zoo", "Notes": "", "Script": "", + "ScriptArgs": [], "ServiceID": "", "Shell": "", "Status": "", @@ -3755,6 +3799,7 @@ func TestSanitize(t *testing.T) { "Name": "blurb", "Notes": "", "Script": "", + "ScriptArgs": [], "Shell": "", "Status": "", "TCP": "", diff --git a/agent/remote_exec.go b/agent/remote_exec.go index e2112922e8..5389486421 100644 --- a/agent/remote_exec.go +++ b/agent/remote_exec.go @@ -49,6 +49,7 @@ type remoteExecEvent struct { // It is stored in the KV store type remoteExecSpec struct { Command string + Args []string Script []byte Wait time.Duration } @@ -160,7 +161,13 @@ func (a *Agent) handleRemoteExec(msg *UserEvent) { // Create the exec.Cmd a.logger.Printf("[INFO] agent: remote exec '%s'", script) - cmd, err := ExecScript(script) + var cmd *exec.Cmd + var err error + if len(spec.Args) > 0 { + cmd, err = ExecSubprocess(spec.Args) + } else { + cmd, err = ExecScript(script) + } if err != nil { a.logger.Printf("[DEBUG] agent: failed to start remote exec: %v", err) exitCode = 255 @@ -178,8 +185,7 @@ func (a *Agent) handleRemoteExec(msg *UserEvent) { cmd.Stderr = writer // Start execution - err = cmd.Start() - if err != nil { + if err := cmd.Start(); err != nil { a.logger.Printf("[DEBUG] agent: failed to start remote exec: %v", err) exitCode = 255 return diff --git a/agent/structs/check_definition.go b/agent/structs/check_definition.go index 60ec59ff4d..9765001f1a 100644 --- a/agent/structs/check_definition.go +++ b/agent/structs/check_definition.go @@ -22,6 +22,7 @@ type CheckDefinition struct { // ID (CheckID), Name, Status, Notes // Script string + ScriptArgs []string HTTP string Header map[string][]string Method string @@ -61,6 +62,7 @@ func (c *CheckDefinition) CheckType() *CheckType { Notes: c.Notes, Script: c.Script, + ScriptArgs: c.ScriptArgs, HTTP: c.HTTP, Header: c.Header, Method: c.Method, diff --git a/agent/structs/check_type.go b/agent/structs/check_type.go index 5fbfb3da3e..9455f451a5 100644 --- a/agent/structs/check_type.go +++ b/agent/structs/check_type.go @@ -24,6 +24,7 @@ type CheckType struct { // Update CheckDefinition when adding fields here Script string + ScriptArgs []string HTTP string Header map[string][]string Method string @@ -49,7 +50,7 @@ func (c *CheckType) Valid() bool { // IsScript checks if this is a check that execs some kind of script. func (c *CheckType) IsScript() bool { - return c.Script != "" + return c.Script != "" || len(c.ScriptArgs) > 0 } // IsTTL checks if this is a TTL type @@ -59,7 +60,7 @@ func (c *CheckType) IsTTL() bool { // IsMonitor checks if this is a Monitor type func (c *CheckType) IsMonitor() bool { - return c.Script != "" && c.DockerContainerID == "" && c.Interval != 0 + return c.IsScript() && c.DockerContainerID == "" && c.Interval != 0 } // IsHTTP checks if this is a HTTP type diff --git a/agent/util.go b/agent/util.go index 9811aac04b..3a13c2172a 100644 --- a/agent/util.go +++ b/agent/util.go @@ -6,6 +6,8 @@ import ( "fmt" "math" "os" + "os/exec" + "os/signal" osuser "os/user" "strconv" "time" @@ -113,3 +115,34 @@ GROUP: return nil } + +// ExecSubprocess returns a command to execute a subprocess directly. +func ExecSubprocess(args []string) (*exec.Cmd, error) { + if len(args) == 0 { + return nil, fmt.Errorf("need an executable to run") + } + + return exec.Command(args[0], args[1:]...), nil +} + +// ForwardSignals will fire up a goroutine to forward signals to the given +// subprocess until the shutdown channel is closed. +func ForwardSignals(cmd *exec.Cmd, logFn func(error), shutdownCh <-chan struct{}) { + go func() { + signalCh := make(chan os.Signal, 10) + signal.Notify(signalCh, os.Interrupt, os.Kill) + defer signal.Stop(signalCh) + + for { + select { + case sig := <-signalCh: + if err := cmd.Process.Signal(sig); err != nil { + logFn(fmt.Errorf("failed to send signal %q: %v", sig, err)) + } + + case <-shutdownCh: + return + } + } + }() +} diff --git a/agent/util_other.go b/agent/util_other.go index c911c261cb..fd3472aeda 100644 --- a/agent/util_other.go +++ b/agent/util_other.go @@ -7,7 +7,7 @@ import ( "os/exec" ) -// ExecScript returns a command to execute a script +// ExecScript returns a command to execute a script through a shell. func ExecScript(script string) (*exec.Cmd, error) { shell := "/bin/sh" if other := os.Getenv("SHELL"); other != "" { diff --git a/agent/util_windows.go b/agent/util_windows.go index 9ec95bac4e..a9b8517f7d 100644 --- a/agent/util_windows.go +++ b/agent/util_windows.go @@ -9,7 +9,7 @@ import ( "syscall" ) -// ExecScript returns a command to execute a script +// ExecScript returns a command to execute a script through a shell. func ExecScript(script string) (*exec.Cmd, error) { shell := "cmd" if other := os.Getenv("SHELL"); other != "" { diff --git a/agent/watch_handler.go b/agent/watch_handler.go index ecec7c2cf1..2012001f1b 100644 --- a/agent/watch_handler.go +++ b/agent/watch_handler.go @@ -7,6 +7,7 @@ import ( "io" "log" "os" + "os/exec" "strconv" "github.com/armon/circbuf" @@ -21,16 +22,36 @@ const ( ) // makeWatchHandler returns a handler for the given watch -func makeWatchHandler(logOutput io.Writer, params interface{}) watch.HandlerFunc { - script := params.(string) +func makeWatchHandler(logOutput io.Writer, handler interface{}) watch.HandlerFunc { + var args []string + var script string + + // Figure out whether to run in shell or raw subprocess mode + switch h := handler.(type) { + case string: + script = h + case []string: + args = h + default: + panic(fmt.Errorf("unknown handler type %T", handler)) + } + logger := log.New(logOutput, "", log.LstdFlags) fn := func(idx uint64, data interface{}) { // Create the command - cmd, err := ExecScript(script) + var cmd *exec.Cmd + var err error + + if len(args) > 0 { + cmd, err = ExecSubprocess(args) + } else { + cmd, err = ExecScript(script) + } if err != nil { logger.Printf("[ERR] agent: Failed to setup watch: %v", err) return } + cmd.Env = append(os.Environ(), "CONSUL_INDEX="+strconv.FormatUint(idx, 10), ) @@ -44,14 +65,14 @@ func makeWatchHandler(logOutput io.Writer, params interface{}) watch.HandlerFunc var inp bytes.Buffer enc := json.NewEncoder(&inp) if err := enc.Encode(data); err != nil { - logger.Printf("[ERR] agent: Failed to encode data for watch '%s': %v", script, err) + logger.Printf("[ERR] agent: Failed to encode data for watch '%v': %v", handler, err) return } cmd.Stdin = &inp // Run the handler if err := cmd.Run(); err != nil { - logger.Printf("[ERR] agent: Failed to invoke watch handler '%s': %v", script, err) + logger.Printf("[ERR] agent: Failed to run watch handler '%v': %v", handler, err) } // Get the output, add a message about truncation @@ -62,7 +83,7 @@ func makeWatchHandler(logOutput io.Writer, params interface{}) watch.HandlerFunc } // Log the output - logger.Printf("[DEBUG] agent: watch handler '%s' output: %s", script, outputStr) + logger.Printf("[DEBUG] agent: watch handler '%v' output: %s", handler, outputStr) } return fn } diff --git a/command/agent.go b/command/agent.go index 580f32ef41..bec702c075 100644 --- a/command/agent.go +++ b/command/agent.go @@ -363,7 +363,7 @@ func (cmd *AgentCommand) run(args []string) int { logGate.Flush() // wait for signal - signalCh := make(chan os.Signal, 4) + signalCh := make(chan os.Signal, 10) signal.Notify(signalCh, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP) signal.Notify(signalCh, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGPIPE) diff --git a/command/exec.go b/command/exec.go index 826638153b..450736d3d6 100644 --- a/command/exec.go +++ b/command/exec.go @@ -53,6 +53,7 @@ const ( // rExecConf is used to pass around configuration type rExecConf struct { prefix string + shell bool foreignDC bool localDC string @@ -66,6 +67,7 @@ type rExecConf struct { replWait time.Duration cmd string + args []string script []byte verbose bool @@ -83,6 +85,9 @@ type rExecSpec struct { // Command is a single command to run directly in the shell Command string `json:",omitempty"` + // Args is the list of arguments to run the subprocess directly + Args []string `json:",omitempty"` + // Script should be spilled to a file and executed Script []byte `json:",omitempty"` @@ -134,6 +139,8 @@ func (c *ExecCommand) Run(args []string) int { "Regular expression to filter on service tags. Must be used with -service.") f.StringVar(&c.conf.prefix, "prefix", rExecPrefix, "Prefix in the KV store to use for request data.") + f.BoolVar(&c.conf.shell, "shell", true, + "Use a shell to run the command.") f.DurationVar(&c.conf.wait, "wait", rExecQuietWait, "Period to wait with no responses before terminating execution.") f.DurationVar(&c.conf.replWait, "wait-repl", rExecReplicationWait, @@ -151,6 +158,11 @@ func (c *ExecCommand) Run(args []string) int { // If there is no command, read stdin for a script input if c.conf.cmd == "-" { + if !c.conf.shell { + c.UI.Error("Cannot configure -shell=false when reading from stdin") + return 1 + } + c.conf.cmd = "" var buf bytes.Buffer _, err := io.Copy(&buf, os.Stdin) @@ -161,10 +173,13 @@ func (c *ExecCommand) Run(args []string) int { return 1 } c.conf.script = buf.Bytes() + } else if !c.conf.shell { + c.conf.cmd = "" + c.conf.args = f.Args() } // Ensure we have a command or script - if c.conf.cmd == "" && len(c.conf.script) == 0 { + if c.conf.cmd == "" && len(c.conf.script) == 0 && len(c.conf.args) == 0 { c.UI.Error("Must specify a command to execute") c.UI.Error("") c.UI.Error(c.Help()) @@ -545,6 +560,7 @@ func (c *ExecCommand) destroySession() error { func (c *ExecCommand) makeRExecSpec() ([]byte, error) { spec := &rExecSpec{ Command: c.conf.cmd, + Args: c.conf.args, Script: c.conf.script, Wait: c.conf.wait, } diff --git a/command/exec_test.go b/command/exec_test.go index c7c807c26a..aa5b633d6c 100644 --- a/command/exec_test.go +++ b/command/exec_test.go @@ -34,7 +34,27 @@ func TestExecCommandRun(t *testing.T) { defer a.Shutdown() ui, c := testExecCommand(t) - args := []string{"-http-addr=" + a.HTTPAddr(), "-wait=500ms", "uptime"} + args := []string{"-http-addr=" + a.HTTPAddr(), "-wait=1s", "uptime"} + + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. Error:%#v (std)Output:%#v", code, ui.ErrorWriter.String(), ui.OutputWriter.String()) + } + + if !strings.Contains(ui.OutputWriter.String(), "load") { + t.Fatalf("bad: %#v", ui.OutputWriter.String()) + } +} + +func TestExecCommandRun_NoShell(t *testing.T) { + t.Parallel() + a := agent.NewTestAgent(t.Name(), ` + disable_remote_exec = false + `) + defer a.Shutdown() + + ui, c := testExecCommand(t) + args := []string{"-http-addr=" + a.HTTPAddr(), "-shell=false", "-wait=1s", "uptime"} code := c.Run(args) if code != 0 { diff --git a/command/lock.go b/command/lock.go index 4177186c2e..8b12f56023 100644 --- a/command/lock.go +++ b/command/lock.go @@ -3,6 +3,7 @@ package command import ( "fmt" "os" + "os/exec" "path" "strings" "sync" @@ -79,6 +80,7 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { var name string var passStdin bool var propagateChildCode bool + var shell bool var timeout time.Duration f := c.BaseCommand.NewFlagSet(c) @@ -101,6 +103,9 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { "is generated based on the provided child command.") f.BoolVar(&passStdin, "pass-stdin", false, "Pass stdin to the child process.") + f.BoolVar(&shell, "shell", true, + "Use a shell to run the command (can set a custom shell via the SHELL "+ + "environment variable).") f.DurationVar(&timeout, "timeout", 0, "Maximum amount of time to wait to acquire the lock, specified as a "+ "duration like \"1s\" or \"3h\". The default value is 0.") @@ -129,7 +134,6 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { } prefix := extra[0] prefix = strings.TrimPrefix(prefix, "/") - script := strings.Join(extra[1:], " ") if timeout < 0 { c.UI.Error("Timeout must be positive") @@ -138,7 +142,7 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { // Calculate a session name if none provided if name == "" { - name = fmt.Sprintf("Consul lock for '%s' at '%s'", script, prefix) + name = fmt.Sprintf("Consul lock for '%s' at '%s'", strings.Join(extra[1:], " "), prefix) } // Calculate oneshot @@ -200,7 +204,7 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { // Start the child process childErr = make(chan error, 1) go func() { - childErr <- c.startChild(script, passStdin) + childErr <- c.startChild(f.Args()[1:], passStdin, shell) }() // Monitor for shutdown, child termination, or lock loss @@ -335,12 +339,19 @@ func (c *LockCommand) setupSemaphore(client *api.Client, limit int, prefix, name // startChild is a long running routine used to start and // wait for the child process to exit. -func (c *LockCommand) startChild(script string, passStdin bool) error { +func (c *LockCommand) startChild(args []string, passStdin, shell bool) error { if c.verbose { - c.UI.Info(fmt.Sprintf("Starting handler '%s'", script)) + c.UI.Info("Starting handler") } + // Create the command - cmd, err := agent.ExecScript(script) + var cmd *exec.Cmd + var err error + if !shell { + cmd, err = agent.ExecSubprocess(args) + } else { + cmd, err = agent.ExecScript(strings.Join(args, " ")) + } if err != nil { c.UI.Error(fmt.Sprintf("Error executing handler: %s", err)) return err @@ -369,6 +380,14 @@ func (c *LockCommand) startChild(script string, passStdin bool) error { return err } + // Set up signal forwarding. + doneCh := make(chan struct{}) + defer close(doneCh) + logFn := func(err error) { + c.UI.Error(fmt.Sprintf("Warning, could not forward signal: %s", err)) + } + agent.ForwardSignals(cmd, logFn, doneCh) + // Setup the child info c.child = cmd.Process c.childLock.Unlock() diff --git a/command/lock_test.go b/command/lock_test.go index 0dd768b72d..3afd0a3aa6 100644 --- a/command/lock_test.go +++ b/command/lock_test.go @@ -1,7 +1,6 @@ package command import ( - "fmt" "io/ioutil" "path/filepath" "strings" @@ -53,8 +52,28 @@ func TestLockCommand_Run(t *testing.T) { ui, c := testLockCommand(t) filePath := filepath.Join(a.Config.DataDir, "test_touch") - touchCmd := fmt.Sprintf("touch '%s'", filePath) - args := []string{"-http-addr=" + a.HTTPAddr(), "test/prefix", touchCmd} + args := []string{"-http-addr=" + a.HTTPAddr(), "test/prefix", "touch", filePath} + + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + + // Check for the file + _, err := ioutil.ReadFile(filePath) + if err != nil { + t.Fatalf("err: %v", err) + } +} + +func TestLockCommand_Run_NoShell(t *testing.T) { + t.Parallel() + a := agent.NewTestAgent(t.Name(), ``) + defer a.Shutdown() + + ui, c := testLockCommand(t) + filePath := filepath.Join(a.Config.DataDir, "test_touch") + args := []string{"-http-addr=" + a.HTTPAddr(), "-shell=false", "test/prefix", "touch", filePath} code := c.Run(args) if code != 0 { @@ -75,8 +94,7 @@ func TestLockCommand_Try_Lock(t *testing.T) { ui, c := testLockCommand(t) filePath := filepath.Join(a.Config.DataDir, "test_touch") - touchCmd := fmt.Sprintf("touch '%s'", filePath) - args := []string{"-http-addr=" + a.HTTPAddr(), "-try=10s", "test/prefix", touchCmd} + args := []string{"-http-addr=" + a.HTTPAddr(), "-try=10s", "test/prefix", "touch", filePath} // Run the command. var lu *LockUnlock @@ -106,8 +124,7 @@ func TestLockCommand_Try_Semaphore(t *testing.T) { ui, c := testLockCommand(t) filePath := filepath.Join(a.Config.DataDir, "test_touch") - touchCmd := fmt.Sprintf("touch '%s'", filePath) - args := []string{"-http-addr=" + a.HTTPAddr(), "-n=3", "-try=10s", "test/prefix", touchCmd} + args := []string{"-http-addr=" + a.HTTPAddr(), "-n=3", "-try=10s", "test/prefix", "touch", filePath} // Run the command. var lu *LockUnlock @@ -137,8 +154,7 @@ func TestLockCommand_MonitorRetry_Lock_Default(t *testing.T) { ui, c := testLockCommand(t) filePath := filepath.Join(a.Config.DataDir, "test_touch") - touchCmd := fmt.Sprintf("touch '%s'", filePath) - args := []string{"-http-addr=" + a.HTTPAddr(), "test/prefix", touchCmd} + args := []string{"-http-addr=" + a.HTTPAddr(), "test/prefix", "touch", filePath} // Run the command. var lu *LockUnlock @@ -169,8 +185,7 @@ func TestLockCommand_MonitorRetry_Semaphore_Default(t *testing.T) { ui, c := testLockCommand(t) filePath := filepath.Join(a.Config.DataDir, "test_touch") - touchCmd := fmt.Sprintf("touch '%s'", filePath) - args := []string{"-http-addr=" + a.HTTPAddr(), "-n=3", "test/prefix", touchCmd} + args := []string{"-http-addr=" + a.HTTPAddr(), "-n=3", "test/prefix", "touch", filePath} // Run the command. var lu *LockUnlock @@ -201,8 +216,7 @@ func TestLockCommand_MonitorRetry_Lock_Arg(t *testing.T) { ui, c := testLockCommand(t) filePath := filepath.Join(a.Config.DataDir, "test_touch") - touchCmd := fmt.Sprintf("touch '%s'", filePath) - args := []string{"-http-addr=" + a.HTTPAddr(), "-monitor-retry=9", "test/prefix", touchCmd} + args := []string{"-http-addr=" + a.HTTPAddr(), "-monitor-retry=9", "test/prefix", "touch", filePath} // Run the command. var lu *LockUnlock @@ -233,8 +247,7 @@ func TestLockCommand_MonitorRetry_Semaphore_Arg(t *testing.T) { ui, c := testLockCommand(t) filePath := filepath.Join(a.Config.DataDir, "test_touch") - touchCmd := fmt.Sprintf("touch '%s'", filePath) - args := []string{"-http-addr=" + a.HTTPAddr(), "-n=3", "-monitor-retry=9", "test/prefix", touchCmd} + args := []string{"-http-addr=" + a.HTTPAddr(), "-n=3", "-monitor-retry=9", "test/prefix", "touch", filePath} // Run the command. var lu *LockUnlock @@ -265,7 +278,7 @@ func TestLockCommand_ChildExitCode(t *testing.T) { t.Run("clean exit", func(t *testing.T) { _, c := testLockCommand(t) - args := []string{"-http-addr=" + a.HTTPAddr(), "-child-exit-code", "test/prefix", "exit 0"} + args := []string{"-http-addr=" + a.HTTPAddr(), "-child-exit-code", "test/prefix", "sh", "-c", "exit", "0"} if got, want := c.Run(args), 0; got != want { t.Fatalf("got %d want %d", got, want) } @@ -273,7 +286,7 @@ func TestLockCommand_ChildExitCode(t *testing.T) { t.Run("error exit", func(t *testing.T) { _, c := testLockCommand(t) - args := []string{"-http-addr=" + a.HTTPAddr(), "-child-exit-code", "test/prefix", "exit 1"} + args := []string{"-http-addr=" + a.HTTPAddr(), "-child-exit-code", "test/prefix", "exit", "1"} if got, want := c.Run(args), 2; got != want { t.Fatalf("got %d want %d", got, want) } @@ -281,7 +294,7 @@ func TestLockCommand_ChildExitCode(t *testing.T) { t.Run("not propagated", func(t *testing.T) { _, c := testLockCommand(t) - args := []string{"-http-addr=" + a.HTTPAddr(), "test/prefix", "exit 1"} + args := []string{"-http-addr=" + a.HTTPAddr(), "test/prefix", "sh", "-c", "exit", "1"} if got, want := c.Run(args), 0; got != want { t.Fatalf("got %d want %d", got, want) } diff --git a/command/watch.go b/command/watch.go index 20c852a7ec..c5f711954a 100644 --- a/command/watch.go +++ b/command/watch.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "os" + "os/exec" "strconv" "strings" @@ -37,6 +38,7 @@ Usage: consul watch [options] [child...] func (c *WatchCommand) Run(args []string) int { var watchType, key, prefix, service, tag, passingOnly, state, name string + var shell bool f := c.BaseCommand.NewFlagSet(c) f.StringVar(&watchType, "type", "", @@ -54,6 +56,9 @@ func (c *WatchCommand) Run(args []string) int { f.StringVar(&passingOnly, "passingonly", "", "Specifies if only hosts passing all checks are displayed. "+ "Optional for 'service' type, must be one of `[true|false]`. Defaults false.") + f.BoolVar(&shell, "shell", true, + "Use a shell to run the command (can set a custom shell via the SHELL "+ + "environment variable).") f.StringVar(&state, "state", "", "Specifies the states to watch. Optional for 'checks' type.") f.StringVar(&name, "name", "", @@ -71,9 +76,6 @@ func (c *WatchCommand) Run(args []string) int { return 1 } - // Grab the script to execute if any - script := strings.Join(f.Args(), " ") - // Compile the watch parameters params := make(map[string]interface{}) if watchType != "" { @@ -140,7 +142,7 @@ func (c *WatchCommand) Run(args []string) int { // 0: false // 1: true errExit := 0 - if script == "" { + if len(f.Args()) == 0 { wp.Handler = func(idx uint64, data interface{}) { defer wp.Stop() buf, err := json.MarshalIndent(data, "", " ") @@ -152,10 +154,21 @@ func (c *WatchCommand) Run(args []string) int { } } else { wp.Handler = func(idx uint64, data interface{}) { + doneCh := make(chan struct{}) + defer close(doneCh) + logFn := func(err error) { + c.UI.Error(fmt.Sprintf("Warning, could not forward signal: %s", err)) + } + // Create the command var buf bytes.Buffer var err error - cmd, err := agent.ExecScript(script) + var cmd *exec.Cmd + if !shell { + cmd, err = agent.ExecSubprocess(f.Args()) + } else { + cmd, err = agent.ExecScript(strings.Join(f.Args(), " ")) + } if err != nil { c.UI.Error(fmt.Sprintf("Error executing handler: %s", err)) goto ERR @@ -173,8 +186,17 @@ func (c *WatchCommand) Run(args []string) int { cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - // Run the handler - if err := cmd.Run(); err != nil { + // Run the handler. + if err := cmd.Start(); err != nil { + c.UI.Error(fmt.Sprintf("Error starting handler: %s", err)) + goto ERR + } + + // Set up signal forwarding. + agent.ForwardSignals(cmd, logFn, doneCh) + + // Wait for the handler to complete. + if err := cmd.Wait(); err != nil { c.UI.Error(fmt.Sprintf("Error executing handler: %s", err)) goto ERR } diff --git a/website/source/docs/agent/checks.html.md b/website/source/docs/agent/checks.html.md index 7685ce2c43..886af999ea 100644 --- a/website/source/docs/agent/checks.html.md +++ b/website/source/docs/agent/checks.html.md @@ -97,7 +97,7 @@ A script check: "check": { "id": "mem-util", "name": "Memory utilization", - "script": "/usr/local/bin/check_mem.py", + "args": ["/usr/local/bin/check_mem.py", "-limit", "256MB"], "interval": "10s", "timeout": "1s" } @@ -157,7 +157,7 @@ A Docker check: "name": "Memory utilization", "docker_container_id": "f972c95ebf0e", "shell": "/bin/bash", - "script": "/usr/local/bin/check_mem.py", + "args": ["/usr/local/bin/check_mem.py"], "interval": "10s" } } @@ -218,6 +218,11 @@ In Consul 0.9.0 and later, the agent must be configured with [`enable_script_checks`](/docs/agent/options.html#_enable_script_checks) set to `true` in order to enable script checks. +Prior to Consul 1.0, checks used a single `script` field to define the command to run, and +would always run in a shell. In Consul 1.0, the `args` array was added so that checks can be +run without a shell. The `script` field is deprecated, and you should include the shell in +the `args` to run under a shell, eg. `"args": ["sh", "-c", "..."]`. + ## Initial Health Check Status By default, when checks are registered against a Consul agent, the state is set @@ -231,7 +236,7 @@ health check definition, like so: { "check": { "id": "mem", - "script": "/bin/check_mem", + "args": ["/bin/check_mem", "-limit", "256MB"], "interval": "10s", "status": "passing" } @@ -274,7 +279,7 @@ key in your configuration file. { "id": "chk1", "name": "mem", - "script": "/bin/check_mem", + "args": ["/bin/check_mem", "-limit", "256MB"], "interval": "5s" }, { diff --git a/website/source/docs/agent/watches.html.md b/website/source/docs/agent/watches.html.md index f355cadde2..306e4e15b8 100644 --- a/website/source/docs/agent/watches.html.md +++ b/website/source/docs/agent/watches.html.md @@ -45,6 +45,11 @@ Additionally, the `CONSUL_INDEX` environment variable will be set. This maps to the `X-Consul-Index` value in responses from the [HTTP API](/api/index.html). +Prior to Consul 1.0, watches used a single `handler` field to define the command to run, and +would always run in a shell. In Consul 1.0, the `args` array was added so that handlers can be +run without a shell. The `handler` field is deprecated, and you should include the shell in +the `args` to run under a shell, eg. `"args": ["sh", "-c", "..."]`. + ## Global Parameters In addition to the parameters supported by each option type, there @@ -52,7 +57,8 @@ are a few global parameters that all watches support: * `datacenter` - Can be provided to override the agent's default datacenter. * `token` - Can be provided to override the agent's default ACL token. -* `handler` - The handler to invoke when the data view updates. +* `args` - The handler subprocess and arguments to invoke when the data view updates. +* `handler` - The handler shell command to invoke when the data view updates. ## Watch Types @@ -80,7 +86,7 @@ Here is an example configuration: { "type": "key", "key": "foo/bar/baz", - "handler": "/usr/bin/my-key-handler.sh" + "args": ["/usr/bin/my-service-handler.sh", "-redis"] } ``` @@ -117,7 +123,7 @@ Here is an example configuration: { "type": "keyprefix", "prefix": "foo/", - "handler": "/usr/bin/my-prefix-handler.sh" + "args": ["/usr/bin/my-service-handler.sh", "-redis"] } ``` @@ -231,7 +237,7 @@ Here is an example configuration: { "type": "service", "service": "redis", - "handler": "/usr/bin/my-service-handler.sh" + "args": ["/usr/bin/my-service-handler.sh", "-redis"] } ``` @@ -322,13 +328,13 @@ Here is an example configuration: { "type": "event", "name": "web-deploy", - "handler": "/usr/bin/my-deploy-handler.sh" + "args": ["/usr/bin/my-service-handler.sh", "-web-deploy"] } ``` Or, using the watch command: - $ consul watch -type=event -name=web-deploy /usr/bin/my-deploy-handler.sh + $ consul watch -type=event -name=web-deploy /usr/bin/my-deploy-handler.sh -web-deploy An example of the output of this command: diff --git a/website/source/docs/commands/exec.html.markdown.erb b/website/source/docs/commands/exec.html.markdown.erb index 4da09b86cd..a28260ae59 100644 --- a/website/source/docs/commands/exec.html.markdown.erb +++ b/website/source/docs/commands/exec.html.markdown.erb @@ -61,6 +61,8 @@ completion as a script to evaluate. * `-service` - Regular expression to filter to only nodes with matching services. +* `-shell` - Optional, use a shell to run the command. The default value is true. + * `-tag` - Regular expression to filter to only nodes with a service that has a matching tag. This must be used with `-service`. As an example, you may do `-service mysql -tag secondary`. diff --git a/website/source/docs/commands/lock.html.markdown.erb b/website/source/docs/commands/lock.html.markdown.erb index 18f9103e57..301d49eed6 100644 --- a/website/source/docs/commands/lock.html.markdown.erb +++ b/website/source/docs/commands/lock.html.markdown.erb @@ -69,9 +69,12 @@ Windows has no POSIX compatible notion for `SIGTERM`. * `-name` - Optional name to associate with the underlying session. If not provided, one is generated based on the child command. +* `-shell` - Optional, use a shell to run the command (can set a custom shell via the + SHELL environment variable). The default value is true. + * `-pass-stdin` - Pass stdin to child process. -* `timeout` - Maximum amount of time to wait to acquire the lock, specified +* `-timeout` - Maximum amount of time to wait to acquire the lock, specified as a duration like `1s` or `3h`. The default value is 0. * `-try` - Attempt to acquire the lock up to the given timeout. The timeout is a @@ -87,4 +90,4 @@ default to `/bin/sh`. It should be noted that not all shells terminate child processes when they receive `SIGTERM`. Under Ubuntu, `/bin/sh` is linked to `dash`, which does **not** terminate its children. In order to ensure that child processes are killed when the lock is lost, be sure to set the `SHELL` environment variable -appropriately. +appropriately, or run without a shell by setting `-shell=false`. diff --git a/website/source/docs/commands/watch.html.markdown.erb b/website/source/docs/commands/watch.html.markdown.erb index b940aa8b40..50b29702ac 100644 --- a/website/source/docs/commands/watch.html.markdown.erb +++ b/website/source/docs/commands/watch.html.markdown.erb @@ -45,6 +45,9 @@ or optionally provided. There is more documentation on watch * `-service` - Service to watch. Required for `service` type, optional for `checks` type. +* `-shell` - Optional, use a shell to run the command (can set a custom shell via the + SHELL environment variable). The default value is true. + * `-state` - Check state to filter on. Optional for `checks` type. * `-tag` - Service tag to filter on. Optional for `service` type.