Skip to content

Commit

Permalink
Update Check API to use constants
Browse files Browse the repository at this point in the history
Use constants where appropriate to advocate their use.  Also add a deprecation notice re: `updateTTL`.
  • Loading branch information
sean- committed Apr 23, 2016
1 parent 7eb85dd commit e63d3a1
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 69 deletions.
36 changes: 26 additions & 10 deletions api/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand All @@ -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
Expand Down
40 changes: 20 additions & 20 deletions api/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

Expand All @@ -409,7 +409,7 @@ func TestAgent_CheckStartPassing(t *testing.T) {
reg := &AgentCheckRegistration{
Name: "foo",
AgentServiceCheck: AgentServiceCheck{
Status: "passing",
Status: HealthPassing,
},
}
reg.TTL = "15s"
Expand All @@ -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)
}

Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down
2 changes: 1 addition & 1 deletion api/catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}

Expand Down
10 changes: 5 additions & 5 deletions command/agent/agent_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -623,7 +623,7 @@ func TestHTTPAgentUpdateCheck(t *testing.T) {
}

update := checkUpdate{
Status: "passing",
Status: structs.HealthPassing,
}
req.Body = encodeReq(update)

Expand Down
40 changes: 20 additions & 20 deletions command/agent/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,51 +235,51 @@ 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()
}

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()
}

Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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()
}

Expand All @@ -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()
}

Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions command/agent/health_endpoint.go
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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)
}

Expand Down
Loading

0 comments on commit e63d3a1

Please sign in to comment.