diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index 0ebfdc204..7068eab2d 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -667,7 +667,6 @@ func TestAgent_RegisterCheck(t *testing.T) { a := NewTestAgent(t.Name(), "") defer a.Shutdown() - // Register node args := &structs.CheckDefinition{ Name: "test", TTL: 15 * time.Second, @@ -703,12 +702,105 @@ func TestAgent_RegisterCheck(t *testing.T) { } } +// This verifies all the forms of the new args-style check that we need to +// support as a result of https://github.com/hashicorp/consul/issues/3587. +func TestAgent_RegisterCheck_Scripts(t *testing.T) { + t.Parallel() + a := NewTestAgent(t.Name(), ` + enable_script_checks = true +`) + defer a.Shutdown() + + tests := []struct { + name string + check map[string]interface{} + }{ + { + "< Consul 1.0.0", + map[string]interface{}{ + "Name": "test", + "Interval": "2s", + "Script": "true", + }, + }, + { + "== Consul 1.0.0", + map[string]interface{}{ + "Name": "test", + "Interval": "2s", + "ScriptArgs": []string{"true"}, + }, + }, + { + "> Consul 1.0.0 (fixup)", + map[string]interface{}{ + "Name": "test", + "Interval": "2s", + "script_args": []string{"true"}, + }, + }, + { + "> Consul 1.0.0", + map[string]interface{}{ + "Name": "test", + "Interval": "2s", + "Args": []string{"true"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name+" as node check", func(t *testing.T) { + req, _ := http.NewRequest("PUT", "/v1/agent/check/register", jsonReader(tt.check)) + resp := httptest.NewRecorder() + if _, err := a.srv.AgentRegisterCheck(resp, req); err != nil { + t.Fatalf("err: %v", err) + } + if resp.Code != http.StatusOK { + t.Fatalf("bad: %d", resp.Code) + } + }) + + t.Run(tt.name+" as top-level service check", func(t *testing.T) { + args := map[string]interface{}{ + "Name": "a", + "Port": 1234, + "Check": tt.check, + } + + req, _ := http.NewRequest("PUT", "/v1/agent/service/register", jsonReader(args)) + resp := httptest.NewRecorder() + if _, err := a.srv.AgentRegisterService(resp, req); err != nil { + t.Fatalf("err: %v", err) + } + if resp.Code != http.StatusOK { + t.Fatalf("bad: %d", resp.Code) + } + }) + + t.Run(tt.name+" as slice-based service check", func(t *testing.T) { + args := map[string]interface{}{ + "Name": "a", + "Port": 1234, + "Checks": []map[string]interface{}{tt.check}, + } + + req, _ := http.NewRequest("PUT", "/v1/agent/service/register", jsonReader(args)) + resp := httptest.NewRecorder() + if _, err := a.srv.AgentRegisterService(resp, req); err != nil { + t.Fatalf("err: %v", err) + } + if resp.Code != http.StatusOK { + t.Fatalf("bad: %d", resp.Code) + } + }) + } +} + func TestAgent_RegisterCheck_Passing(t *testing.T) { t.Parallel() a := NewTestAgent(t.Name(), "") defer a.Shutdown() - // Register node args := &structs.CheckDefinition{ Name: "test", TTL: 15 * time.Second, @@ -744,7 +836,6 @@ func TestAgent_RegisterCheck_BadStatus(t *testing.T) { a := NewTestAgent(t.Name(), "") defer a.Shutdown() - // Register node args := &structs.CheckDefinition{ Name: "test", TTL: 15 * time.Second, @@ -795,7 +886,6 @@ func TestAgent_DeregisterCheck(t *testing.T) { t.Fatalf("err: %v", err) } - // Register node req, _ := http.NewRequest("PUT", "/v1/agent/check/deregister/test", nil) obj, err := a.srv.AgentDeregisterCheck(nil, req) if err != nil { diff --git a/agent/check.go b/agent/check.go index 438642916..440fc36ae 100644 --- a/agent/check.go +++ b/agent/check.go @@ -86,7 +86,6 @@ func (c *CheckMonitor) Stop() { func (c *CheckMonitor) run() { // Get the randomized initial pause time initialPauseTime := lib.RandomStagger(c.Interval) - c.Logger.Printf("[DEBUG] agent: pausing %v before first invocation of %s", initialPauseTime, c.Script) next := time.After(initialPauseTime) for { select { @@ -594,7 +593,6 @@ func (c *CheckDocker) Stop() { func (c *CheckDocker) run() { firstWait := lib.RandomStagger(c.Interval) - c.Logger.Printf("[DEBUG] agent: pausing %v before first invocation of %s -c %s in container %s", firstWait, c.Shell, c.Script, c.DockerContainerID) next := time.After(firstWait) for { select { diff --git a/agent/config.go b/agent/config.go index 84052986b..bb7d8a2dc 100644 --- a/agent/config.go +++ b/agent/config.go @@ -17,9 +17,13 @@ func FixupCheckType(raw interface{}) error { return nil } - // see https://github.com/hashicorp/consul/pull/3557 why we need this - // and why we should get rid of it. + // See https://github.com/hashicorp/consul/pull/3557 why we need this + // and why we should get rid of it. In Consul 1.0 we also didn't map + // Args correctly, so we ended up exposing (and need to carry forward) + // ScriptArgs, see https://github.com/hashicorp/consul/issues/3587. config.TranslateKeys(rawMap, map[string]string{ + "args": "ScriptArgs", + "script_args": "ScriptArgs", "deregister_critical_service_after": "DeregisterCriticalServiceAfter", "docker_container_id": "DockerContainerID", "tls_skip_verify": "TLSSkipVerify", diff --git a/api/agent.go b/api/agent.go index ac57415c1..533b24557 100644 --- a/api/agent.go +++ b/api/agent.go @@ -80,7 +80,8 @@ type AgentCheckRegistration struct { // AgentServiceCheck is used to define a node or service level check type AgentServiceCheck struct { - Script string `json:",omitempty"` + Args []string `json:"ScriptArgs,omitempty"` + Script string `json:",omitempty"` // Deprecated, use Args. DockerContainerID string `json:",omitempty"` Shell string `json:",omitempty"` // Only supported for Docker. Interval string `json:",omitempty"` diff --git a/api/agent_test.go b/api/agent_test.go index 11666f71e..89c2283a8 100644 --- a/api/agent_test.go +++ b/api/agent_test.go @@ -498,6 +498,69 @@ func TestAPI_AgentChecks(t *testing.T) { } } +func TestAPI_AgentScriptCheck(t *testing.T) { + t.Parallel() + c, s := makeClientWithConfig(t, nil, func(c *testutil.TestServerConfig) { + c.EnableScriptChecks = true + }) + defer s.Stop() + + agent := c.Agent() + + t.Run("node script check", func(t *testing.T) { + reg := &AgentCheckRegistration{ + Name: "foo", + AgentServiceCheck: AgentServiceCheck{ + Interval: "10s", + Args: []string{"sh", "-c", "false"}, + }, + } + if err := agent.CheckRegister(reg); err != nil { + t.Fatalf("err: %v", err) + } + + checks, err := agent.Checks() + if err != nil { + t.Fatalf("err: %v", err) + } + if _, ok := checks["foo"]; !ok { + t.Fatalf("missing check: %v", checks) + } + }) + + t.Run("service script check", func(t *testing.T) { + reg := &AgentServiceRegistration{ + Name: "bar", + Port: 1234, + Checks: AgentServiceChecks{ + &AgentServiceCheck{ + Interval: "10s", + Args: []string{"sh", "-c", "false"}, + }, + }, + } + if err := agent.ServiceRegister(reg); err != nil { + t.Fatalf("err: %v", err) + } + + services, err := agent.Services() + if err != nil { + t.Fatalf("err: %v", err) + } + if _, ok := services["bar"]; !ok { + t.Fatalf("missing service: %v", services) + } + + checks, err := agent.Checks() + if err != nil { + t.Fatalf("err: %v", err) + } + if _, ok := checks["service:bar"]; !ok { + t.Fatalf("missing check: %v", checks) + } + }) +} + func TestAPI_AgentCheckStartPassing(t *testing.T) { t.Parallel() c, s := makeClient(t) diff --git a/website/source/api/agent/check.html.md b/website/source/api/agent/check.html.md index c51b8fc63..da64214c7 100644 --- a/website/source/api/agent/check.html.md +++ b/website/source/api/agent/check.html.md @@ -110,6 +110,11 @@ The table below shows this endpoint's support for `Script` field is deprecated, and you should include the shell in the `Args` to run under a shell, eg. `"args": ["sh", "-c", "..."]`. + -> **Note:** Consul 1.0 shipped with an issue where `Args` was erroneously named + `ScriptArgs` in this API. Please use `ScriptArgs` with Consul 1.0 (that will + continue to be accepted in future versions of Consul), and `Args` in Consul + 1.0.1 and later. + - `DockerContainerID` `(string: "")` - Specifies that the check is a Docker check, and Consul will evaluate the script every `Interval` in the given container using the specified `Shell`. Note that `Shell` is currently only