diff --git a/command/agent/agent.go b/command/agent/agent.go index f7a68130ca..08d4faf2a2 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -1002,23 +1002,14 @@ func (a *Agent) unloadChecks() error { // EnableServiceMaintenance will register a false health check against the given // service ID with critical status. This will exclude the service from queries. func (a *Agent) EnableServiceMaintenance(serviceID string) error { - var service *structs.NodeService - for _, svc := range a.state.Services() { - if svc.ID == serviceID { - service = svc - } - } - - // Ensure the service exists - if service == nil { + service, ok := a.state.Services()[serviceID] + if !ok { return fmt.Errorf("No service registered with ID %q", serviceID) } // Ensure maintenance mode is not already enabled - for _, check := range a.state.Checks() { - if check.CheckID == maintCheckID { - return fmt.Errorf("Maintenance mode already enabled for service %q", serviceID) - } + if _, ok := a.state.Checks()[maintCheckID]; ok { + return nil } // Create and register the critical health check @@ -1039,29 +1030,11 @@ func (a *Agent) EnableServiceMaintenance(serviceID string) error { // DisableServiceMaintenance will deregister the fake maintenance mode check // if the service has been marked as in maintenance. func (a *Agent) DisableServiceMaintenance(serviceID string) error { - var service *structs.NodeService - for _, svc := range a.state.Services() { - if svc.ID == serviceID { - service = svc - } - } - - // Ensure the service exists - if service == nil { + if _, ok := a.state.Services()[serviceID]; !ok { return fmt.Errorf("No service registered with ID %q", serviceID) } - // Ensure maintenance mode is enabled - for _, check := range a.state.Checks() { - if check.CheckID == maintCheckID { - goto DEREGISTER - } - } - return fmt.Errorf("Maintenance mode not enabled for service %q", serviceID) - -DEREGISTER: // Deregister the maintenance check a.RemoveCheck(maintCheckID, true) - return nil } diff --git a/command/agent/agent_endpoint.go b/command/agent/agent_endpoint.go index 6757179371..6e89ae09b8 100644 --- a/command/agent/agent_endpoint.go +++ b/command/agent/agent_endpoint.go @@ -216,12 +216,12 @@ func (s *HTTPServer) AgentServiceMaintenance(resp http.ResponseWriter, req *http var err error if enable { if err = s.agent.EnableServiceMaintenance(serviceID); err != nil { - resp.WriteHeader(409) + resp.WriteHeader(404) resp.Write([]byte(err.Error())) } } else { if err = s.agent.DisableServiceMaintenance(serviceID); err != nil { - resp.WriteHeader(409) + resp.WriteHeader(404) resp.Write([]byte(err.Error())) } } diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 8881d18c31..c147d5b195 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -507,7 +507,7 @@ func TestHTTPAgent_ServiceMaintenanceEndpoint_BadRequest(t *testing.T) { t.Fatalf("err: %s", err) } if resp.Code != 405 { - t.Fatalf("expected 405 for non-PUT request") + t.Fatalf("expected 405, got %d", resp.Code) } // Fails when no enable flag provided @@ -517,7 +517,7 @@ func TestHTTPAgent_ServiceMaintenanceEndpoint_BadRequest(t *testing.T) { t.Fatalf("err: %s", err) } if resp.Code != 400 { - t.Fatalf("expected 400 for missing enable flag") + t.Fatalf("expected 400, got %d", resp.Code) } // Fails when no service ID provided @@ -527,7 +527,17 @@ func TestHTTPAgent_ServiceMaintenanceEndpoint_BadRequest(t *testing.T) { t.Fatalf("err: %s", err) } if resp.Code != 400 { - t.Fatalf("expected 400 for missing service ID") + t.Fatalf("expected 400, got %d", resp.Code) + } + + // Fails when bad service ID provided + req, _ = http.NewRequest("PUT", "/v1/agent/service/maintenance/_nope_?enable=true", nil) + resp = httptest.NewRecorder() + if _, err := srv.AgentServiceMaintenance(resp, req); err == nil { + t.Fatalf("should have errored") + } + if resp.Code != 404 { + t.Fatalf("expected 404, got %d", resp.Code) } } @@ -546,29 +556,9 @@ func TestHTTPAgent_EnableServiceMaintenance(t *testing.T) { t.Fatalf("err: %v", err) } - // Force into maintenance mode - if err := srv.agent.EnableServiceMaintenance("test"); err != nil { - t.Fatalf("err: %s", err) - } - - // Fails when service is already in maintenance mode + // Force the service into maintenance mode req, _ := http.NewRequest("PUT", "/v1/agent/service/maintenance/test?enable=true", nil) resp := httptest.NewRecorder() - if _, err := srv.AgentServiceMaintenance(resp, req); err == nil { - t.Fatalf("should have errored") - } - if resp.Code != 409 { - t.Fatalf("expected 409, got %d", resp.Code) - } - - // Remove maintenance mode - if err := srv.agent.DisableServiceMaintenance("test"); err != nil { - t.Fatalf("err: %s", err) - } - - // Force the service into maintenance mode - req, _ = http.NewRequest("PUT", "/v1/agent/service/maintenance/test?enable=true", nil) - resp = httptest.NewRecorder() if _, err := srv.AgentServiceMaintenance(resp, req); err != nil { t.Fatalf("err: %s", err) } @@ -597,28 +587,23 @@ func TestHTTPAgent_DisableServiceMaintenance(t *testing.T) { t.Fatalf("err: %v", err) } - // Fails when the service is not in maintenance mode - req, _ := http.NewRequest("PUT", "/v1/agent/service/maintenance/test?enable=false", nil) - resp := httptest.NewRecorder() - if _, err := srv.AgentServiceMaintenance(resp, req); err == nil { - t.Fatalf("should have failed") - } - if resp.Code != 409 { - t.Fatalf("expected 409, got %d", resp.Code) - } - // Force the service into maintenance mode if err := srv.agent.EnableServiceMaintenance("test"); err != nil { t.Fatalf("err: %s", err) } // Leave maintenance mode - req, _ = http.NewRequest("PUT", "/v1/agent/service/maintenance/test?enable=false", nil) - resp = httptest.NewRecorder() + req, _ := http.NewRequest("PUT", "/v1/agent/service/maintenance/test?enable=false", nil) + resp := httptest.NewRecorder() if _, err := srv.AgentServiceMaintenance(resp, req); err != nil { t.Fatalf("err: %s", err) } if resp.Code != 200 { t.Fatalf("expected 200, got %d", resp.Code) } + + // Ensure the maintenance check was removed + if _, ok := srv.agent.state.Checks()[maintCheckID]; ok { + t.Fatalf("should have removed maintenance check") + } } diff --git a/website/source/docs/agent/http.html.markdown b/website/source/docs/agent/http.html.markdown index 5019e3728a..cb0e72f273 100644 --- a/website/source/docs/agent/http.html.markdown +++ b/website/source/docs/agent/http.html.markdown @@ -553,13 +553,12 @@ The return code is 200 on success. The service maintenance endpoint allows placing a given service into "maintenance mode". During maintenance mode, the service will be marked as -unavailable, and will not be present in DNS or API queries. +unavailable, and will not be present in DNS or API queries. This API call is +idempotent. Maintenance mode is persistent and will be automatically restored +on agent restart. The `?enable` flag is required, and its value must be `true` (to enter -maintenance mode), or `false` (to resume normal operation). It is an error to -enable maintenance mode while it is already enabled, or disable it while it is -already disabled. You will receive a 409 if either of these conflicts are -observed. +maintenance mode), or `false` (to resume normal operation). The return code is 200 on success.