From 65f9017c6396c3136a03ed1d1bd5e7e11fae545b Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Sat, 23 Apr 2016 16:01:59 -0700 Subject: [PATCH 1/4] Update Check API to use constants Use constants where appropriate to advocate their use. Also add a deprecation notice re: `updateTTL`. --- api/agent.go | 36 ++++++++++++++++++------- api/agent_test.go | 40 ++++++++++++++-------------- api/catalog_test.go | 2 +- command/agent/agent_endpoint_test.go | 10 +++---- command/agent/check_test.go | 40 ++++++++++++++-------------- command/agent/health_endpoint.go | 5 ++-- testutil/README.md | 6 ++--- testutil/server.go | 13 ++++----- watch/funcs_test.go | 5 ++-- 9 files changed, 88 insertions(+), 69 deletions(-) diff --git a/api/agent.go b/api/agent.go index 67855c67f..1211cc6fd 100644 --- a/api/agent.go +++ b/api/agent.go @@ -198,27 +198,42 @@ func (a *Agent) ServiceDeregister(serviceID string) error { return nil } -// PassTTL is used to set a TTL check to the passing state +// PassTTL is used to set a TTL check to the passing state. +// +// DEPRECATION NOTICE: This interface is deprecated in favor of UpdateTTL(). +// The client interface will be removed in 0.8 or changed to use +// UpdateTTL()'s endpoint and the server endpoints will be removed in 0.9. func (a *Agent) PassTTL(checkID, note string) error { return a.updateTTL(checkID, note, "pass") } -// WarnTTL is used to set a TTL check to the warning state +// WarnTTL is used to set a TTL check to the warning state. +// +// DEPRECATION NOTICE: This interface is deprecated in favor of UpdateTTL(). +// The client interface will be removed in 0.8 or changed to use +// UpdateTTL()'s endpoint and the server endpoints will be removed in 0.9. func (a *Agent) WarnTTL(checkID, note string) error { return a.updateTTL(checkID, note, "warn") } -// FailTTL is used to set a TTL check to the failing state +// FailTTL is used to set a TTL check to the failing state. +// +// DEPRECATION NOTICE: This interface is deprecated in favor of UpdateTTL(). +// The client interface will be removed in 0.8 or changed to use +// UpdateTTL()'s endpoint and the server endpoints will be removed in 0.9. func (a *Agent) FailTTL(checkID, note string) error { return a.updateTTL(checkID, note, "fail") } // updateTTL is used to update the TTL of a check. This is the internal -// method that uses the old API that's present in Consul versions prior -// to 0.6.4. Since Consul didn't have an analogous "update" API before it -// seemed ok to break this (former) UpdateTTL in favor of the new UpdateTTL -// below, but keep the old Pass/Warn/Fail methods using the old API under the -// hood. +// method that uses the old API that's present in Consul versions prior to +// 0.6.4. Since Consul didn't have an analogous "update" API before it seemed +// ok to break this (former) UpdateTTL in favor of the new UpdateTTL below, +// but keep the old Pass/Warn/Fail methods using the old API under the hood. +// +// DEPRECATION NOTICE: This interface is deprecated in favor of UpdateTTL(). +// The client interface will be removed in 0.8 and the server endpoints will +// be removed in 0.9. func (a *Agent) updateTTL(checkID, note, status string) error { switch status { case "pass": @@ -240,8 +255,9 @@ func (a *Agent) updateTTL(checkID, note, status string) error { // checkUpdate is the payload for a PUT for a check update. type checkUpdate struct { - // Status us one of the structs.Health* states, "passing", "warning", or - // "critical". + // Status us one of the structs.Health* states: HealthPassing + // ("passing"), HealthWarning ("warning"), or HealthCritical + // ("critical"). Status string // Output is the information to post to the UI for operators as the diff --git a/api/agent_test.go b/api/agent_test.go index a61245149..b188d7ce3 100644 --- a/api/agent_test.go +++ b/api/agent_test.go @@ -76,7 +76,7 @@ func TestAgent_Services(t *testing.T) { } // Checks should default to critical - if chk.Status != "critical" { + if chk.Status != HealthCritical { t.Fatalf("Bad: %#v", chk) } @@ -97,7 +97,7 @@ func TestAgent_Services_CheckPassing(t *testing.T) { Port: 8000, Check: &AgentServiceCheck{ TTL: "15s", - Status: "passing", + Status: HealthPassing, }, } if err := agent.ServiceRegister(reg); err != nil { @@ -121,7 +121,7 @@ func TestAgent_Services_CheckPassing(t *testing.T) { t.Fatalf("missing check: %v", checks) } - if chk.Status != "passing" { + if chk.Status != HealthPassing { t.Fatalf("Bad: %#v", chk) } if err := agent.ServiceDeregister("foo"); err != nil { @@ -320,47 +320,47 @@ func TestAgent_SetTTLStatus(t *testing.T) { if err := agent.WarnTTL("service:foo", "foo"); err != nil { t.Fatalf("err: %v", err) } - verify("warning", "foo") + verify(HealthWarning, "foo") if err := agent.PassTTL("service:foo", "bar"); err != nil { t.Fatalf("err: %v", err) } - verify("passing", "bar") + verify(HealthPassing, "bar") if err := agent.FailTTL("service:foo", "baz"); err != nil { t.Fatalf("err: %v", err) } - verify("critical", "baz") + verify(HealthCritical, "baz") if err := agent.UpdateTTL("service:foo", "foo", "warn"); err != nil { t.Fatalf("err: %v", err) } - verify("warning", "foo") + verify(HealthWarning, "foo") if err := agent.UpdateTTL("service:foo", "bar", "pass"); err != nil { t.Fatalf("err: %v", err) } - verify("passing", "bar") + verify(HealthPassing, "bar") if err := agent.UpdateTTL("service:foo", "baz", "fail"); err != nil { t.Fatalf("err: %v", err) } - verify("critical", "baz") + verify(HealthCritical, "baz") - if err := agent.UpdateTTL("service:foo", "foo", "warning"); err != nil { + if err := agent.UpdateTTL("service:foo", "foo", HealthWarning); err != nil { t.Fatalf("err: %v", err) } - verify("warning", "foo") + verify(HealthWarning, "foo") - if err := agent.UpdateTTL("service:foo", "bar", "passing"); err != nil { + if err := agent.UpdateTTL("service:foo", "bar", HealthPassing); err != nil { t.Fatalf("err: %v", err) } - verify("passing", "bar") + verify(HealthPassing, "bar") - if err := agent.UpdateTTL("service:foo", "baz", "critical"); err != nil { + if err := agent.UpdateTTL("service:foo", "baz", HealthCritical); err != nil { t.Fatalf("err: %v", err) } - verify("critical", "baz") + verify(HealthCritical, "baz") if err := agent.ServiceDeregister("foo"); err != nil { t.Fatalf("err: %v", err) @@ -390,7 +390,7 @@ func TestAgent_Checks(t *testing.T) { if !ok { t.Fatalf("missing check: %v", checks) } - if chk.Status != "critical" { + if chk.Status != HealthCritical { t.Fatalf("check not critical: %v", chk) } @@ -409,7 +409,7 @@ func TestAgent_CheckStartPassing(t *testing.T) { reg := &AgentCheckRegistration{ Name: "foo", AgentServiceCheck: AgentServiceCheck{ - Status: "passing", + Status: HealthPassing, }, } reg.TTL = "15s" @@ -425,7 +425,7 @@ func TestAgent_CheckStartPassing(t *testing.T) { if !ok { t.Fatalf("missing check: %v", checks) } - if chk.Status != "passing" { + if chk.Status != HealthPassing { t.Fatalf("check not passing: %v", chk) } @@ -580,7 +580,7 @@ func TestServiceMaintenance(t *testing.T) { for _, check := range checks { if strings.Contains(check.CheckID, "maintenance") { found = true - if check.Status != "critical" || check.Notes != "broken" { + if check.Status != HealthCritical || check.Notes != "broken" { t.Fatalf("bad: %#v", checks) } } @@ -627,7 +627,7 @@ func TestNodeMaintenance(t *testing.T) { for _, check := range checks { if strings.Contains(check.CheckID, "maintenance") { found = true - if check.Status != "critical" || check.Notes != "broken" { + if check.Status != HealthCritical || check.Notes != "broken" { t.Fatalf("bad: %#v", checks) } } diff --git a/api/catalog_test.go b/api/catalog_test.go index 691c05618..3a3395ae6 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -157,7 +157,7 @@ func TestCatalog_Registration(t *testing.T) { CheckID: "service:redis1", Name: "Redis health check", Notes: "Script based health check", - Status: "passing", + Status: HealthPassing, ServiceID: "redis1", } diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 2a3b14844..d11842fc1 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -526,9 +526,9 @@ func TestHTTPAgentUpdateCheck(t *testing.T) { } cases := []checkUpdate{ - checkUpdate{"passing", "hello-passing"}, - checkUpdate{"critical", "hello-critical"}, - checkUpdate{"warning", "hello-warning"}, + checkUpdate{structs.HealthPassing, "hello-passing"}, + checkUpdate{structs.HealthCritical, "hello-critical"}, + checkUpdate{structs.HealthWarning, "hello-warning"}, } for _, c := range cases { @@ -564,7 +564,7 @@ func TestHTTPAgentUpdateCheck(t *testing.T) { } update := checkUpdate{ - Status: "passing", + Status: structs.HealthPassing, Output: strings.Repeat("-= bad -=", 5*CheckBufSize), } req.Body = encodeReq(update) @@ -623,7 +623,7 @@ func TestHTTPAgentUpdateCheck(t *testing.T) { } update := checkUpdate{ - Status: "passing", + Status: structs.HealthPassing, } req.Body = encodeReq(update) diff --git a/command/agent/check_test.go b/command/agent/check_test.go index 053eb5148..e327be32d 100644 --- a/command/agent/check_test.go +++ b/command/agent/check_test.go @@ -235,25 +235,25 @@ func TestCheckHTTPCritical(t *testing.T) { server := mockHTTPServer(150) fmt.Println(server.URL) - expectHTTPStatus(t, server.URL, "critical") + expectHTTPStatus(t, server.URL, structs.HealthCritical) server.Close() // 2xx - 1 server = mockHTTPServer(199) - expectHTTPStatus(t, server.URL, "critical") + expectHTTPStatus(t, server.URL, structs.HealthCritical) server.Close() // 2xx + 1 server = mockHTTPServer(300) - expectHTTPStatus(t, server.URL, "critical") + expectHTTPStatus(t, server.URL, structs.HealthCritical) server.Close() server = mockHTTPServer(400) - expectHTTPStatus(t, server.URL, "critical") + expectHTTPStatus(t, server.URL, structs.HealthCritical) server.Close() server = mockHTTPServer(500) - expectHTTPStatus(t, server.URL, "critical") + expectHTTPStatus(t, server.URL, structs.HealthCritical) server.Close() } @@ -261,25 +261,25 @@ func TestCheckHTTPPassing(t *testing.T) { var server *httptest.Server server = mockHTTPServer(200) - expectHTTPStatus(t, server.URL, "passing") + expectHTTPStatus(t, server.URL, structs.HealthPassing) server.Close() server = mockHTTPServer(201) - expectHTTPStatus(t, server.URL, "passing") + expectHTTPStatus(t, server.URL, structs.HealthPassing) server.Close() server = mockHTTPServer(250) - expectHTTPStatus(t, server.URL, "passing") + expectHTTPStatus(t, server.URL, structs.HealthPassing) server.Close() server = mockHTTPServer(299) - expectHTTPStatus(t, server.URL, "passing") + expectHTTPStatus(t, server.URL, structs.HealthPassing) server.Close() } func TestCheckHTTPWarning(t *testing.T) { server := mockHTTPServer(429) - expectHTTPStatus(t, server.URL, "warning") + expectHTTPStatus(t, server.URL, structs.HealthWarning) server.Close() } @@ -323,7 +323,7 @@ func TestCheckHTTPTimeout(t *testing.T) { t.Fatalf("should have at least 2 updates %v", mock.updates) } - if mock.state["bar"] != "critical" { + if mock.state["bar"] != structs.HealthCritical { t.Fatalf("should be critical %v", mock.state) } } @@ -397,7 +397,7 @@ func TestCheckTCPCritical(t *testing.T) { ) tcpServer = mockTCPServer(`tcp`) - expectTCPStatus(t, `127.0.0.1:0`, "critical") + expectTCPStatus(t, `127.0.0.1:0`, structs.HealthCritical) tcpServer.Close() } @@ -407,11 +407,11 @@ func TestCheckTCPPassing(t *testing.T) { ) tcpServer = mockTCPServer(`tcp`) - expectTCPStatus(t, tcpServer.Addr().String(), "passing") + expectTCPStatus(t, tcpServer.Addr().String(), structs.HealthPassing) tcpServer.Close() tcpServer = mockTCPServer(`tcp6`) - expectTCPStatus(t, tcpServer.Addr().String(), "passing") + expectTCPStatus(t, tcpServer.Addr().String(), structs.HealthPassing) tcpServer.Close() } @@ -579,27 +579,27 @@ func expectDockerCheckStatus(t *testing.T, dockerClient DockerClient, status str } func TestDockerCheckWhenExecReturnsSuccessExitCode(t *testing.T) { - expectDockerCheckStatus(t, &fakeDockerClientWithNoErrors{}, "passing", "output") + expectDockerCheckStatus(t, &fakeDockerClientWithNoErrors{}, structs.HealthPassing, "output") } func TestDockerCheckWhenExecCreationFails(t *testing.T) { - expectDockerCheckStatus(t, &fakeDockerClientWithCreateExecFailure{}, "critical", "Unable to create Exec, error: Exec Creation Failed") + expectDockerCheckStatus(t, &fakeDockerClientWithCreateExecFailure{}, structs.HealthCritical, "Unable to create Exec, error: Exec Creation Failed") } func TestDockerCheckWhenExitCodeIsNonZero(t *testing.T) { - expectDockerCheckStatus(t, &fakeDockerClientWithExecNonZeroExitCode{}, "critical", "") + expectDockerCheckStatus(t, &fakeDockerClientWithExecNonZeroExitCode{}, structs.HealthCritical, "") } func TestDockerCheckWhenExitCodeIsone(t *testing.T) { - expectDockerCheckStatus(t, &fakeDockerClientWithExecExitCodeOne{}, "warning", "output") + expectDockerCheckStatus(t, &fakeDockerClientWithExecExitCodeOne{}, structs.HealthWarning, "output") } func TestDockerCheckWhenExecStartFails(t *testing.T) { - expectDockerCheckStatus(t, &fakeDockerClientWithStartExecFailure{}, "critical", "Unable to start Exec: Couldn't Start Exec") + expectDockerCheckStatus(t, &fakeDockerClientWithStartExecFailure{}, structs.HealthCritical, "Unable to start Exec: Couldn't Start Exec") } func TestDockerCheckWhenExecInfoFails(t *testing.T) { - expectDockerCheckStatus(t, &fakeDockerClientWithExecInfoErrors{}, "critical", "Unable to inspect Exec: Unable to query exec info") + expectDockerCheckStatus(t, &fakeDockerClientWithExecInfoErrors{}, structs.HealthCritical, "Unable to inspect Exec: Unable to query exec info") } func TestDockerCheckDefaultToSh(t *testing.T) { diff --git a/command/agent/health_endpoint.go b/command/agent/health_endpoint.go index e4a36f6a6..b252a4126 100644 --- a/command/agent/health_endpoint.go +++ b/command/agent/health_endpoint.go @@ -1,9 +1,10 @@ package agent import ( - "github.com/hashicorp/consul/consul/structs" "net/http" "strings" + + "github.com/hashicorp/consul/consul/structs" ) func (s *HTTPServer) HealthChecksInState(resp http.ResponseWriter, req *http.Request) (interface{}, error) { @@ -126,7 +127,7 @@ func (s *HTTPServer) HealthServiceNodes(resp http.ResponseWriter, req *http.Requ } // Filter to only passing if specified - if _, ok := params["passing"]; ok { + if _, ok := params[structs.HealthPassing]; ok { out.Nodes = filterNonPassing(out.Nodes) } diff --git a/testutil/README.md b/testutil/README.md index ebbbade5d..d47e76788 100644 --- a/testutil/README.md +++ b/testutil/README.md @@ -48,13 +48,13 @@ func TestMain(t *testing.T) { }) // Create a service - srv1.AddService("redis", "passing", []string{"master"}) + srv1.AddService("redis", structs.HealthPassing, []string{"master"}) // Create a service check - srv1.AddCheck("service:redis", "redis", "passing") + srv1.AddCheck("service:redis", "redis", structs.HealthPassing) // Create a node check - srv1.AddCheck("mem", "", "critical") + srv1.AddCheck("mem", "", structs.HealthCritical) // The HTTPAddr field contains the address of the Consul // API on the new test server instance. diff --git a/testutil/server.go b/testutil/server.go index 5574f668c..8a88a8b1c 100644 --- a/testutil/server.go +++ b/testutil/server.go @@ -25,6 +25,7 @@ import ( "strings" "sync/atomic" + "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/go-cleanhttp" ) @@ -444,11 +445,11 @@ func (s *TestServer) AddService(name, status string, tags []string) { s.put("/v1/agent/check/register", payload) switch status { - case "passing": + case structs.HealthPassing: s.put("/v1/agent/check/pass/"+chkName, nil) - case "warning": + case structs.HealthWarning: s.put("/v1/agent/check/warn/"+chkName, nil) - case "critical": + case structs.HealthCritical: s.put("/v1/agent/check/fail/"+chkName, nil) default: s.t.Fatalf("Unrecognized status: %s", status) @@ -472,11 +473,11 @@ func (s *TestServer) AddCheck(name, serviceID, status string) { s.put("/v1/agent/check/register", payload) switch status { - case "passing": + case structs.HealthPassing: s.put("/v1/agent/check/pass/"+name, nil) - case "warning": + case structs.HealthWarning: s.put("/v1/agent/check/warn/"+name, nil) - case "critical": + case structs.HealthCritical: s.put("/v1/agent/check/fail/"+name, nil) default: s.t.Fatalf("Unrecognized status: %s", status) diff --git a/watch/funcs_test.go b/watch/funcs_test.go index 2e0e345a5..78f93c90a 100644 --- a/watch/funcs_test.go +++ b/watch/funcs_test.go @@ -6,6 +6,7 @@ import ( "time" consulapi "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/consul/structs" ) var consulAddr string @@ -299,7 +300,7 @@ func TestChecksWatch_State(t *testing.T) { Node: "foobar", CheckID: "foobar", Name: "foobar", - Status: "warning", + Status: structs.HealthWarning, }, } catalog.Register(reg, nil) @@ -363,7 +364,7 @@ func TestChecksWatch_Service(t *testing.T) { Node: "foobar", CheckID: "foobar", Name: "foobar", - Status: "passing", + Status: structs.HealthPassing, ServiceID: "foobar", }, } From 5bc4a2c2abb04aae32b02d59188065ef7ddcaeeb Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Sat, 23 Apr 2016 16:06:58 -0700 Subject: [PATCH 2/4] consul/ uses structs.Health*, the api uses api.Health* --- api/agent.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/agent.go b/api/agent.go index 1211cc6fd..24404a3b9 100644 --- a/api/agent.go +++ b/api/agent.go @@ -255,7 +255,7 @@ func (a *Agent) updateTTL(checkID, note, status string) error { // checkUpdate is the payload for a PUT for a check update. type checkUpdate struct { - // Status us one of the structs.Health* states: HealthPassing + // Status us one of the api.Health* states: HealthPassing // ("passing"), HealthWarning ("warning"), or HealthCritical // ("critical"). Status string From 0f45d1b76dfb5d6d75461f9d731552dcd9a4d85b Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Sat, 23 Apr 2016 20:18:19 -0700 Subject: [PATCH 3/4] Correct a small typo --- api/agent.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/agent.go b/api/agent.go index 24404a3b9..3df013cc5 100644 --- a/api/agent.go +++ b/api/agent.go @@ -255,7 +255,7 @@ func (a *Agent) updateTTL(checkID, note, status string) error { // checkUpdate is the payload for a PUT for a check update. type checkUpdate struct { - // Status us one of the api.Health* states: HealthPassing + // Status is one of the api.Health* states: HealthPassing // ("passing"), HealthWarning ("warning"), or HealthCritical // ("critical"). Status string From 3c909585dff05d98e85384dd7f96a9ef1a4888b5 Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Sat, 23 Apr 2016 20:18:45 -0700 Subject: [PATCH 4/4] Clean up the test example in README This works without an import cycle and has been `go fmt`'ified --- testutil/README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/testutil/README.md b/testutil/README.md index d47e76788..dd953343c 100644 --- a/testutil/README.md +++ b/testutil/README.md @@ -13,14 +13,16 @@ from Consul's core and API client, meaning it can be easily imported and used in external unit tests for various applications. It works by invoking the Consul CLI, which means it is a requirement to have Consul installed in the `$PATH`. -Following is some example usage: +Following is an example usage: ```go -package main +package my_program import ( - "github.com/hashicorp/consul/testutil" "testing" + + "github.com/hashicorp/consul/consul/structs" + "github.com/hashicorp/consul/testutil" ) func TestMain(t *testing.T) {