From f00b5b542e1cc57c18913e6f0a236c5be18d8f66 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Wed, 21 Jan 2015 12:21:57 -0800 Subject: [PATCH 01/15] agent: support passing ?reason= for custom notes field values on maintenance checks --- command/agent/agent.go | 18 ++++++++--- command/agent/agent_endpoint.go | 5 +-- command/agent/agent_endpoint_test.go | 25 +++++++++++---- command/agent/agent_test.go | 46 +++++++++++++++++++++++++--- 4 files changed, 78 insertions(+), 16 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index 5ec4409f1d..1a22f859ad 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -1027,7 +1027,7 @@ func serviceMaintCheckID(serviceID string) string { // 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 { +func (a *Agent) EnableServiceMaintenance(serviceID, reason string) error { service, ok := a.state.Services()[serviceID] if !ok { return fmt.Errorf("No service registered with ID %q", serviceID) @@ -1039,12 +1039,17 @@ func (a *Agent) EnableServiceMaintenance(serviceID string) error { return nil } + // Use default notes if no reason provided + if reason == "" { + reason = "Maintenance mode is enabled for this service" + } + // Create and register the critical health check check := &structs.HealthCheck{ Node: a.config.NodeName, CheckID: checkID, Name: "Service Maintenance Mode", - Notes: "Maintenance mode is enabled for this service", + Notes: reason, ServiceID: service.ID, ServiceName: service.Service, Status: structs.HealthCritical, @@ -1076,18 +1081,23 @@ func (a *Agent) DisableServiceMaintenance(serviceID string) error { } // EnableNodeMaintenance places a node into maintenance mode. -func (a *Agent) EnableNodeMaintenance() { +func (a *Agent) EnableNodeMaintenance(reason string) { // Ensure node maintenance is not already enabled if _, ok := a.state.Checks()[nodeMaintCheckID]; ok { return } + // Use a default notes value + if reason == "" { + reason = "Maintenance mode is enabled for this node" + } + // Create and register the node maintenance check check := &structs.HealthCheck{ Node: a.config.NodeName, CheckID: nodeMaintCheckID, Name: "Node Maintenance Mode", - Notes: "Maintenance mode is enabled for this node", + Notes: reason, Status: structs.HealthCritical, } a.AddCheck(check, nil, true) diff --git a/command/agent/agent_endpoint.go b/command/agent/agent_endpoint.go index 817426286b..0636b01c83 100644 --- a/command/agent/agent_endpoint.go +++ b/command/agent/agent_endpoint.go @@ -220,7 +220,8 @@ func (s *HTTPServer) AgentServiceMaintenance(resp http.ResponseWriter, req *http } if enable { - if err = s.agent.EnableServiceMaintenance(serviceID); err != nil { + reason := params.Get("reason") + if err = s.agent.EnableServiceMaintenance(serviceID, reason); err != nil { resp.WriteHeader(404) resp.Write([]byte(err.Error())) } @@ -257,7 +258,7 @@ func (s *HTTPServer) AgentNodeMaintenance(resp http.ResponseWriter, req *http.Re } if enable { - s.agent.EnableNodeMaintenance() + s.agent.EnableNodeMaintenance(params.Get("reason")) } else { s.agent.DisableNodeMaintenance() } diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 189b90a3c1..c54f832f92 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -566,7 +566,7 @@ func TestHTTPAgent_EnableServiceMaintenance(t *testing.T) { } // Force the service into maintenance mode - req, _ := http.NewRequest("PUT", "/v1/agent/service/maintenance/test?enable=true", nil) + req, _ := http.NewRequest("PUT", "/v1/agent/service/maintenance/test?enable=true&reason=broken", nil) resp := httptest.NewRecorder() if _, err := srv.AgentServiceMaintenance(resp, req); err != nil { t.Fatalf("err: %s", err) @@ -577,9 +577,15 @@ func TestHTTPAgent_EnableServiceMaintenance(t *testing.T) { // Ensure the maintenance check was registered checkID := serviceMaintCheckID("test") - if _, ok := srv.agent.state.Checks()[checkID]; !ok { + check, ok := srv.agent.state.Checks()[checkID] + if !ok { t.Fatalf("should have registered maintenance check") } + + // Ensure the reason was set in notes + if check.Notes != "broken" { + t.Fatalf("bad: %#v", check) + } } func TestHTTPAgent_DisableServiceMaintenance(t *testing.T) { @@ -598,7 +604,7 @@ func TestHTTPAgent_DisableServiceMaintenance(t *testing.T) { } // Force the service into maintenance mode - if err := srv.agent.EnableServiceMaintenance("test"); err != nil { + if err := srv.agent.EnableServiceMaintenance("test", ""); err != nil { t.Fatalf("err: %s", err) } @@ -653,7 +659,8 @@ func TestHTTPAgent_EnableNodeMaintenance(t *testing.T) { defer srv.agent.Shutdown() // Force the node into maintenance mode - req, _ := http.NewRequest("PUT", "/v1/agent/self/maintenance?enable=true", nil) + req, _ := http.NewRequest( + "PUT", "/v1/agent/self/maintenance?enable=true&reason=broken", nil) resp := httptest.NewRecorder() if _, err := srv.AgentNodeMaintenance(resp, req); err != nil { t.Fatalf("err: %s", err) @@ -663,9 +670,15 @@ func TestHTTPAgent_EnableNodeMaintenance(t *testing.T) { } // Ensure the maintenance check was registered - if _, ok := srv.agent.state.Checks()[nodeMaintCheckID]; !ok { + check, ok := srv.agent.state.Checks()[nodeMaintCheckID] + if !ok { t.Fatalf("should have registered maintenance check") } + + // Ensure the reason was set in notes + if check.Notes != "broken" { + t.Fatalf("bad: %#v", check) + } } func TestHTTPAgent_DisableNodeMaintenance(t *testing.T) { @@ -675,7 +688,7 @@ func TestHTTPAgent_DisableNodeMaintenance(t *testing.T) { defer srv.agent.Shutdown() // Force the node into maintenance mode - srv.agent.EnableNodeMaintenance() + srv.agent.EnableNodeMaintenance("") // Leave maintenance mode req, _ := http.NewRequest("PUT", "/v1/agent/self/maintenance?enable=false", nil) diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index 5279b25c47..a09d789666 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -916,16 +916,22 @@ func TestAgent_ServiceMaintenanceMode(t *testing.T) { } // Enter maintenance mode for the service - if err := agent.EnableServiceMaintenance("redis"); err != nil { + if err := agent.EnableServiceMaintenance("redis", "broken"); err != nil { t.Fatalf("err: %s", err) } // Make sure the critical health check was added checkID := serviceMaintCheckID("redis") - if _, ok := agent.state.Checks()[checkID]; !ok { + check, ok := agent.state.Checks()[checkID] + if !ok { t.Fatalf("should have registered critical maintenance check") } + // Ensure the reason was set in notes + if check.Notes != "broken" { + t.Fatalf("bad: %#v", check) + } + // Leave maintenance mode if err := agent.DisableServiceMaintenance("redis"); err != nil { t.Fatalf("err: %s", err) @@ -935,6 +941,20 @@ func TestAgent_ServiceMaintenanceMode(t *testing.T) { if _, ok := agent.state.Checks()[checkID]; ok { t.Fatalf("should have deregistered maintenance check") } + + // Enter service maintenance mode without providing a reason + if err := agent.EnableServiceMaintenance("redis", ""); err != nil { + t.Fatalf("err: %s", err) + } + + // Ensure the check was registered with the default notes + check, ok = agent.state.Checks()[checkID] + if !ok { + t.Fatalf("should have registered critical check") + } + if check.Notes == "" { + t.Fatalf("bad: %#v", check) + } } func TestAgent_NodeMaintenanceMode(t *testing.T) { @@ -944,13 +964,19 @@ func TestAgent_NodeMaintenanceMode(t *testing.T) { defer agent.Shutdown() // Enter maintenance mode for the node - agent.EnableNodeMaintenance() + agent.EnableNodeMaintenance("broken") // Make sure the critical health check was added - if _, ok := agent.state.Checks()[nodeMaintCheckID]; !ok { + check, ok := agent.state.Checks()[nodeMaintCheckID] + if !ok { t.Fatalf("should have registered critical node check") } + // Ensure the reason was set in notes + if check.Notes != "broken" { + t.Fatalf("bad: %#v", check) + } + // Leave maintenance mode agent.DisableNodeMaintenance() @@ -958,4 +984,16 @@ func TestAgent_NodeMaintenanceMode(t *testing.T) { if _, ok := agent.state.Checks()[nodeMaintCheckID]; ok { t.Fatalf("should have deregistered critical node check") } + + // Enter maintenance mode without passing a reason + agent.EnableNodeMaintenance("") + + // Make sure the check was registered with the default note + check, ok = agent.state.Checks()[nodeMaintCheckID] + if !ok { + t.Fatalf("should have registered critical node check") + } + if check.Notes == "" { + t.Fatalf("bad: %#v", check) + } } From aa8672203cb22e4a803abaa84d07f0d92af51ee0 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Wed, 21 Jan 2015 13:00:14 -0800 Subject: [PATCH 02/15] command: starting maint command --- command/maint.go | 126 +++++++++++++++++++++++++++++++++++++++++++++++ commands.go | 6 +++ 2 files changed, 132 insertions(+) create mode 100644 command/maint.go diff --git a/command/maint.go b/command/maint.go new file mode 100644 index 0000000000..2c5319fa53 --- /dev/null +++ b/command/maint.go @@ -0,0 +1,126 @@ +package command + +import ( + "flag" + "fmt" + "strings" + + "github.com/hashicorp/consul/api" + "github.com/mitchellh/cli" +) + +// MaintCommand is a Command implementation that enables or disables +// node or service maintenance mode. +type MaintCommand struct { + Ui cli.Ui +} + +func (c *MaintCommand) Help() string { + helpText := ` +Usage: consul maint [options] + + Places a node or service into maintenance mode. During maintenance mode, + the node or service will be excluded from all queries through the DNS + or API interfaces, effectively taking it out of the pool of available + nodes. This is done by registering an additional critical health check. + + When enabling maintenance mode for a node or service, you may optionally + specify a reason string. This string will appear in the "Notes" field + of the critical health check which is registered against the node or + service. If no reason is provided, a default value will be used. + + Maintenance mode is persistent, and will be restored in the event of an + agent restart. It is therefore required to disable maintenance mode on + a given node or service before it will be placed back into the pool. + + By default, we operate on the node as a whole. By specifying the + "-service" argument, this behavior can be changed to enable or disable + only a specific service. + +Options: + + -enable Enable maintenance mode. + -disable Disable maintenance mode. + -reason= Text string describing the maintenance reason + -service= A specific service ID to enable/disable + -token="" ACL token to use. Defaults to that of agent. + -http-addr=127.0.0.1:8500 HTTP address of the Consul agent. +` + return strings.TrimSpace(helpText) +} + +func (c *MaintCommand) Run(args []string) int { + var enable bool + var disable bool + var reason string + var serviceID string + var token string + + cmdFlags := flag.NewFlagSet("maint", flag.ContinueOnError) + cmdFlags.Usage = func() { c.Ui.Output(c.Help()) } + + cmdFlags.BoolVar(&enable, "enable", false, "enable maintenance mode") + cmdFlags.BoolVar(&disable, "disable", false, "disable maintenance mode") + cmdFlags.StringVar(&reason, "reason", "", "maintenance reason") + cmdFlags.StringVar(&serviceID, "service", "", "service maintenance") + cmdFlags.StringVar(&token, "token", "", "") + httpAddr := HTTPAddrFlag(cmdFlags) + + if err := cmdFlags.Parse(args); err != nil { + return 1 + } + + // Ensure we don't have conflicting args + if !enable && !disable { + c.Ui.Error("One of -enable or -disable must be specified") + return 1 + } + if enable && disable { + c.Ui.Error("Only one of -enable or -disable may be provided") + return 1 + } + if disable && reason != "" { + c.Ui.Error("Reason may only be provided with -enable") + return 1 + } + + // Create and test the HTTP client + conf := api.DefaultConfig() + conf.Address = *httpAddr + conf.Token = token + client, err := api.NewClient(conf) + if err != nil { + c.Ui.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err)) + return 1 + } + agent := client.Agent() + if _, err := agent.NodeName(); err != nil { + c.Ui.Error(fmt.Sprintf("Error querying Consul agent: %s", err)) + return 1 + } + + if enable { + // Enable node maintenance + if serviceID == "" { + if err := agent.EnableNodeMaintenance(); err != nil { + c.Ui.Error(fmt.Sprintf("Error enabling node maintenance: %s", err)) + return 1 + } + c.Ui.Output("Node maintenance is now enabled") + return 0 + } + + // Enable service maintenance + if err := agent.EnableServiceMaintenance(serviceID); err != nil { + c.Ui.Error(fmt.Sprintf("Error enabling service maintenance: %s", err)) + return 1 + } + c.Ui.Output(fmt.Sprintf("Service maintenance is now enabled for %q", serviceID)) + } + + return 0 +} + +func (c *MaintCommand) Synopsis() string { + return "Controls node or service maintenance mode" +} diff --git a/commands.go b/commands.go index 13721975ea..d4e9aa0599 100644 --- a/commands.go +++ b/commands.go @@ -76,6 +76,12 @@ func init() { }, nil }, + "maint": func() (cli.Command, error) { + return &command.MaintCommand{ + Ui: ui, + }, nil + }, + "members": func() (cli.Command, error) { return &command.MembersCommand{ Ui: ui, From 089c4396c47f76d5f274734c74a9b6bdf6508769 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Wed, 21 Jan 2015 13:02:47 -0800 Subject: [PATCH 03/15] api: support reason flag for maintenance mode --- api/agent.go | 6 ++++-- api/agent_test.go | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/api/agent.go b/api/agent.go index 08b1441333..e2d1ad543c 100644 --- a/api/agent.go +++ b/api/agent.go @@ -276,9 +276,10 @@ func (a *Agent) ForceLeave(node string) error { // EnableServiceMaintenance toggles service maintenance mode on // for the given service ID. -func (a *Agent) EnableServiceMaintenance(serviceID string) error { +func (a *Agent) EnableServiceMaintenance(serviceID, reason string) error { r := a.c.newRequest("PUT", "/v1/agent/service/maintenance/"+serviceID) r.params.Set("enable", "true") + r.params.Set("reason", reason) _, resp, err := requireOK(a.c.doRequest(r)) if err != nil { return err @@ -302,9 +303,10 @@ func (a *Agent) DisableServiceMaintenance(serviceID string) error { // EnableNodeMaintenance toggles node maintenance mode on for the // agent we are connected to. -func (a *Agent) EnableNodeMaintenance() error { +func (a *Agent) EnableNodeMaintenance(reason string) error { r := a.c.newRequest("PUT", "/v1/agent/maintenance") r.params.Set("enable", "true") + r.params.Set("reason", reason) _, resp, err := requireOK(a.c.doRequest(r)) if err != nil { return err diff --git a/api/agent_test.go b/api/agent_test.go index e27887d6f8..37a00c573f 100644 --- a/api/agent_test.go +++ b/api/agent_test.go @@ -272,7 +272,7 @@ func TestServiceMaintenance(t *testing.T) { } // Enable maintenance mode - if err := agent.EnableServiceMaintenance("redis"); err != nil { + if err := agent.EnableServiceMaintenance("redis", "broken"); err != nil { t.Fatalf("err: %s", err) } @@ -285,7 +285,7 @@ func TestServiceMaintenance(t *testing.T) { for _, check := range checks { if strings.Contains(check.CheckID, "maintenance") { found = true - if check.Status != "critical" { + if check.Status != "critical" || check.Notes != "broken" { t.Fatalf("bad: %#v", checks) } } @@ -318,7 +318,7 @@ func TestNodeMaintenance(t *testing.T) { agent := c.Agent() // Enable maintenance mode - if err := agent.EnableNodeMaintenance(); err != nil { + if err := agent.EnableNodeMaintenance("broken"); err != nil { t.Fatalf("err: %s", err) } @@ -331,7 +331,7 @@ func TestNodeMaintenance(t *testing.T) { for _, check := range checks { if strings.Contains(check.CheckID, "maintenance") { found = true - if check.Status != "critical" { + if check.Status != "critical" || check.Notes != "broken" { t.Fatalf("bad: %#v", checks) } } From 7d663aa1ede533fc7ce48ee4bc2344aef2426040 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Wed, 21 Jan 2015 13:11:23 -0800 Subject: [PATCH 04/15] command: maint command works --- command/maint.go | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/command/maint.go b/command/maint.go index 2c5319fa53..e689541c9a 100644 --- a/command/maint.go +++ b/command/maint.go @@ -102,7 +102,7 @@ func (c *MaintCommand) Run(args []string) int { if enable { // Enable node maintenance if serviceID == "" { - if err := agent.EnableNodeMaintenance(); err != nil { + if err := agent.EnableNodeMaintenance(reason); err != nil { c.Ui.Error(fmt.Sprintf("Error enabling node maintenance: %s", err)) return 1 } @@ -111,11 +111,32 @@ func (c *MaintCommand) Run(args []string) int { } // Enable service maintenance - if err := agent.EnableServiceMaintenance(serviceID); err != nil { + if err := agent.EnableServiceMaintenance(serviceID, reason); err != nil { c.Ui.Error(fmt.Sprintf("Error enabling service maintenance: %s", err)) return 1 } c.Ui.Output(fmt.Sprintf("Service maintenance is now enabled for %q", serviceID)) + return 0 + } + + if disable { + // Disable node maintenance + if serviceID == "" { + if err := agent.DisableNodeMaintenance(); err != nil { + c.Ui.Error(fmt.Sprintf("Error disabling node maintenance: %s", err)) + return 1 + } + c.Ui.Output("Node maintenance is now disabled") + return 0 + } + + // Disable service maintenance + if err := agent.DisableServiceMaintenance(serviceID); err != nil { + c.Ui.Error(fmt.Sprintf("Error disabling service maintenance: %s", err)) + return 1 + } + c.Ui.Output(fmt.Sprintf("Service maintenance is now disabled for %q", serviceID)) + return 0 } return 0 From 61d17e65f50019efaf63c4e974386fdcca2434c4 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Wed, 21 Jan 2015 13:28:26 -0800 Subject: [PATCH 05/15] agent: prevent duplicate error messages for maintenance api --- command/agent/agent_endpoint.go | 5 ++++- command/agent/agent_endpoint_test.go | 3 --- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/command/agent/agent_endpoint.go b/command/agent/agent_endpoint.go index 0636b01c83..d7cfeb9e7f 100644 --- a/command/agent/agent_endpoint.go +++ b/command/agent/agent_endpoint.go @@ -224,14 +224,17 @@ func (s *HTTPServer) AgentServiceMaintenance(resp http.ResponseWriter, req *http if err = s.agent.EnableServiceMaintenance(serviceID, reason); err != nil { resp.WriteHeader(404) resp.Write([]byte(err.Error())) + return nil, nil } } else { if err = s.agent.DisableServiceMaintenance(serviceID); err != nil { resp.WriteHeader(404) resp.Write([]byte(err.Error())) + return nil, nil } } - return nil, err + + return nil, nil } func (s *HTTPServer) AgentNodeMaintenance(resp http.ResponseWriter, req *http.Request) (interface{}, error) { diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index c54f832f92..8e0fbe4f9a 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -542,9 +542,6 @@ func TestHTTPAgent_ServiceMaintenanceEndpoint_BadRequest(t *testing.T) { // 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) } From 47c4ab686327e80a517b8882d6c76c0664dcbe01 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Wed, 21 Jan 2015 13:37:32 -0800 Subject: [PATCH 06/15] command: test maint command --- command/maint.go | 6 +++ command/maint_test.go | 103 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 command/maint_test.go diff --git a/command/maint.go b/command/maint.go index e689541c9a..b0ee4d3112 100644 --- a/command/maint.go +++ b/command/maint.go @@ -70,6 +70,12 @@ func (c *MaintCommand) Run(args []string) int { return 1 } + // Print help if no args given + if len(args) == 0 { + c.Ui.Error(c.Help()) + return 1 + } + // Ensure we don't have conflicting args if !enable && !disable { c.Ui.Error("One of -enable or -disable must be specified") diff --git a/command/maint_test.go b/command/maint_test.go new file mode 100644 index 0000000000..c8c1a263a9 --- /dev/null +++ b/command/maint_test.go @@ -0,0 +1,103 @@ +package command + +import ( + "strings" + "testing" + + "github.com/hashicorp/consul/consul/structs" + "github.com/mitchellh/cli" +) + +func TestMaintCommand_implements(t *testing.T) { + var _ cli.Command = &MaintCommand{} +} + +func TestMaintCommandRun_NoArgs(t *testing.T) { + ui := new(cli.MockUi) + c := &MaintCommand{Ui: ui} + + if code := c.Run([]string{}); code != 1 { + t.Fatalf("expected return code 1, got %d", code) + } + + if strings.TrimSpace(ui.ErrorWriter.String()) != c.Help() { + t.Fatalf("bad:\n%s", ui.ErrorWriter.String()) + } +} + +func TestMaintCommandRun_EnableNodeMaintenance(t *testing.T) { + a1 := testAgent(t) + defer a1.Shutdown() + + ui := new(cli.MockUi) + c := &MaintCommand{Ui: ui} + + args := []string{ + "-http-addr=" + a1.httpAddr, + "-enable", + "-reason=broken", + } + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + + if !strings.Contains(ui.OutputWriter.String(), "now enabled") { + t.Fatalf("bad: %#v", ui.OutputWriter.String()) + } +} + +func TestMaintCommandRun_EnableServiceMaintenance(t *testing.T) { + a1 := testAgent(t) + defer a1.Shutdown() + + // Register the service + service := &structs.NodeService{ + ID: "test", + Service: "test", + } + if err := a1.agent.AddService(service, nil, false); err != nil { + t.Fatalf("err: %v", err) + } + + ui := new(cli.MockUi) + c := &MaintCommand{Ui: ui} + + args := []string{ + "-http-addr=" + a1.httpAddr, + "-enable", + "-service=test", + "-reason=broken", + } + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + + if !strings.Contains(ui.OutputWriter.String(), "now enabled") { + t.Fatalf("bad: %#v", ui.OutputWriter.String()) + } +} + +func TestMaintCommandRun_EnableServiceMaintenance_Fails(t *testing.T) { + a1 := testAgent(t) + defer a1.Shutdown() + + ui := new(cli.MockUi) + c := &MaintCommand{Ui: ui} + + args := []string{ + "-http-addr=" + a1.httpAddr, + "-enable", + "-service=redis", + "-reason=broken", + } + code := c.Run(args) + if code != 1 { + t.Fatalf("expected response code 1, got %d", code) + } + + if !strings.Contains(ui.ErrorWriter.String(), "No service registered") { + t.Fatalf("bad: %#v", ui.ErrorWriter.String()) + } +} From eaaa96922dc4a89d56201d49829300bd3b60ebd1 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Wed, 21 Jan 2015 13:49:10 -0800 Subject: [PATCH 07/15] command: more maint command tests --- command/maint.go | 2 +- command/maint_test.go | 67 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/command/maint.go b/command/maint.go index b0ee4d3112..41ac598a2c 100644 --- a/command/maint.go +++ b/command/maint.go @@ -42,7 +42,7 @@ Options: -enable Enable maintenance mode. -disable Disable maintenance mode. -reason= Text string describing the maintenance reason - -service= A specific service ID to enable/disable + -service= Control maintenance mode for a specific service ID -token="" ACL token to use. Defaults to that of agent. -http-addr=127.0.0.1:8500 HTTP address of the Consul agent. ` diff --git a/command/maint_test.go b/command/maint_test.go index c8c1a263a9..66d82f90bb 100644 --- a/command/maint_test.go +++ b/command/maint_test.go @@ -25,6 +25,19 @@ func TestMaintCommandRun_NoArgs(t *testing.T) { } } +func TestMaintCommandRun_ConflictingArgs(t *testing.T) { + ui := new(cli.MockUi) + c := &MaintCommand{Ui: ui} + + if code := c.Run([]string{"-enable", "-disable"}); code != 1 { + t.Fatalf("expected return code 1, got %d", code) + } + + if code := c.Run([]string{"-disable", "-reason=broken"}); code != 1 { + t.Fatalf("expected return code 1, got %d", code) + } +} + func TestMaintCommandRun_EnableNodeMaintenance(t *testing.T) { a1 := testAgent(t) defer a1.Shutdown() @@ -47,6 +60,27 @@ func TestMaintCommandRun_EnableNodeMaintenance(t *testing.T) { } } +func TestMaintCommandRun_DisableNodeMaintenance(t *testing.T) { + a1 := testAgent(t) + defer a1.Shutdown() + + ui := new(cli.MockUi) + c := &MaintCommand{Ui: ui} + + args := []string{ + "-http-addr=" + a1.httpAddr, + "-disable", + } + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + + if !strings.Contains(ui.OutputWriter.String(), "now disabled") { + t.Fatalf("bad: %#v", ui.OutputWriter.String()) + } +} + func TestMaintCommandRun_EnableServiceMaintenance(t *testing.T) { a1 := testAgent(t) defer a1.Shutdown() @@ -79,7 +113,38 @@ func TestMaintCommandRun_EnableServiceMaintenance(t *testing.T) { } } -func TestMaintCommandRun_EnableServiceMaintenance_Fails(t *testing.T) { +func TestMaintCommandRun_DisableServiceMaintenance(t *testing.T) { + a1 := testAgent(t) + defer a1.Shutdown() + + // Register the service + service := &structs.NodeService{ + ID: "test", + Service: "test", + } + if err := a1.agent.AddService(service, nil, false); err != nil { + t.Fatalf("err: %v", err) + } + + ui := new(cli.MockUi) + c := &MaintCommand{Ui: ui} + + args := []string{ + "-http-addr=" + a1.httpAddr, + "-disable", + "-service=test", + } + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + + if !strings.Contains(ui.OutputWriter.String(), "now disabled") { + t.Fatalf("bad: %#v", ui.OutputWriter.String()) + } +} + +func TestMaintCommandRun_ServiceMaintenance_NoService(t *testing.T) { a1 := testAgent(t) defer a1.Shutdown() From 6995e74d944d8d7932715332f965b4893fa9417e Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Wed, 21 Jan 2015 14:07:54 -0800 Subject: [PATCH 08/15] website: document maint command --- .../source/docs/commands/maint.html.markdown | 53 +++++++++++++++++++ website/source/layouts/docs.erb | 4 ++ 2 files changed, 57 insertions(+) create mode 100644 website/source/docs/commands/maint.html.markdown diff --git a/website/source/docs/commands/maint.html.markdown b/website/source/docs/commands/maint.html.markdown new file mode 100644 index 0000000000..90378bbbde --- /dev/null +++ b/website/source/docs/commands/maint.html.markdown @@ -0,0 +1,53 @@ +--- +layout: "docs" +page_title: "Commands: Maint" +sidebar_current: "docs-commands-maint" +description: > + The `maint` command provides control of both service and node maintenance mode +--- + +# Consul Maint + +Command: `consul maint` + +The `maint` command provides control of both service and node maintenance mode. +Using the command, it is possible to mark a service provided by a node or the +node as a whole as "under maintenance". In this mode of operation, the service +or node will not appear in DNS query results, or API results. This effectively +takes the service or node out of the pool of available "healthy" nodes. + +Under the hood, maintenance mode is activated by registering a health check in +critical status against a node or service, and deactivated by deregistering the +health check. + +## Usage + +Usage: `consul maint [options]` + +Exactly one of `-enable` or `-disable` are required. The rest of the command +line arguments are optional, and some are only usable in combination with +others. + +The list of available flags are: + +* `-enable` - Enable maintenance mode on a given service or node. If + combined with the `-service` flag, we operate on a specific service ID. + Otherwise, node maintenance mode is enabled. + +* `-disable` - Disable maintenance mode on a given service or node. If + combined with the `-service` flag, we operate on a specific service ID. + Otherwise, node maintenance mode is disabled. + +* `-reason` - An optional reason for placing the node or service into + maintenance mode. If provided, this reason will be visible in the newly- + registered critical check's "Notes" field. + +* `-service` - An optional service ID to control node maintenance mode for. By + providing this flag, the `-enable` and `-disable` flags functionality is + modified to operate on the given service ID. + +* `-token` - ACL token to use. Defaults to that of agent. + +* `-http-addr` - Address to the HTTP server of the agent you want to contact + to send this command. If this isn't specified, the command will contact + "127.0.0.1:8500" which is the default HTTP address of a Consul agent. diff --git a/website/source/layouts/docs.erb b/website/source/layouts/docs.erb index 87be2f79df..7b99f15fae 100644 --- a/website/source/layouts/docs.erb +++ b/website/source/layouts/docs.erb @@ -91,6 +91,10 @@ lock + > + maint + + > members From 5aa69827f82662cef49b3fc10b105fff25fbc4a5 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Wed, 21 Jan 2015 14:12:18 -0800 Subject: [PATCH 09/15] agent: fix test --- command/agent/agent_endpoint_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 8e0fbe4f9a..0387f38901 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -542,6 +542,9 @@ func TestHTTPAgent_ServiceMaintenanceEndpoint_BadRequest(t *testing.T) { // 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("err: %s", err) + } if resp.Code != 404 { t.Fatalf("expected 404, got %d", resp.Code) } From 52ff08b6c0472eabd8b0a4e6caed2a3fd67874ba Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Wed, 21 Jan 2015 14:33:01 -0800 Subject: [PATCH 10/15] website: document reason flag for http api --- website/source/docs/agent/http/agent.html.markdown | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/website/source/docs/agent/http/agent.html.markdown b/website/source/docs/agent/http/agent.html.markdown index ca15adebbb..1fa3f5a5cd 100644 --- a/website/source/docs/agent/http/agent.html.markdown +++ b/website/source/docs/agent/http/agent.html.markdown @@ -195,6 +195,10 @@ 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). +The `?reason` flag is optional, and can contain a text string explaining the +reason for placing the node into maintenance mode. If no reason is provided, +a default value will be used instead. + The return code is 200 on success. ### /v1/agent/join/\ @@ -355,4 +359,8 @@ on agent restart. The `?enable` flag is required, and its value must be `true` (to enter maintenance mode), or `false` (to resume normal operation). +The `?reason` flag is optional, and can contain a text string explaining the +reason for placing the service into maintenance mode. If no reason is provided, +a default value will be used instead. + The return code is 200 on success. From 124e7bfa7e7c337598c8f34aaa0e129bdd793f49 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Wed, 21 Jan 2015 14:45:09 -0800 Subject: [PATCH 11/15] agent: use const for default maintenance reason strings --- command/agent/agent.go | 10 ++++++++-- command/agent/agent_test.go | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index 1a22f859ad..7ec832e709 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -34,6 +34,12 @@ const ( // The ID of the faux health checks for maintenance mode serviceMaintCheckPrefix = "_service_maintenance" nodeMaintCheckID = "_node_maintenance" + + // Default reasons for node/service maintenance mode + defaultNodeMaintReason = "Maintenance mode is enabled for this node, " + + "but no reason was provided. This is a default message." + defaultServiceMaintReason = "Maintenance mode is enabled for this " + + "service, but no reason was provided. This is a default message." ) /* @@ -1041,7 +1047,7 @@ func (a *Agent) EnableServiceMaintenance(serviceID, reason string) error { // Use default notes if no reason provided if reason == "" { - reason = "Maintenance mode is enabled for this service" + reason = defaultServiceMaintReason } // Create and register the critical health check @@ -1089,7 +1095,7 @@ func (a *Agent) EnableNodeMaintenance(reason string) { // Use a default notes value if reason == "" { - reason = "Maintenance mode is enabled for this node" + reason = defaultNodeMaintReason } // Create and register the node maintenance check diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index a09d789666..0a702263ff 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -952,7 +952,7 @@ func TestAgent_ServiceMaintenanceMode(t *testing.T) { if !ok { t.Fatalf("should have registered critical check") } - if check.Notes == "" { + if check.Notes != defaultServiceMaintReason { t.Fatalf("bad: %#v", check) } } @@ -993,7 +993,7 @@ func TestAgent_NodeMaintenanceMode(t *testing.T) { if !ok { t.Fatalf("should have registered critical node check") } - if check.Notes == "" { + if check.Notes != defaultNodeMaintReason { t.Fatalf("bad: %#v", check) } } From 09fd2a7e944c837ff3fb445266b67393d070025e Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Thu, 22 Jan 2015 10:26:17 -0800 Subject: [PATCH 12/15] command/maint: display active maintenance when no args are passed --- command/agent/agent.go | 14 +++++----- command/maint.go | 52 +++++++++++++++++++++++++----------- command/maint_test.go | 60 +++++++++++++++++++++++++++++++++--------- 3 files changed, 90 insertions(+), 36 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index 7ec832e709..95ef455725 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -32,8 +32,8 @@ const ( "and try again." // The ID of the faux health checks for maintenance mode - serviceMaintCheckPrefix = "_service_maintenance" - nodeMaintCheckID = "_node_maintenance" + ServiceMaintCheckPrefix = "_service_maintenance" + NodeMaintCheckID = "_node_maintenance" // Default reasons for node/service maintenance mode defaultNodeMaintReason = "Maintenance mode is enabled for this node, " + @@ -1028,7 +1028,7 @@ func (a *Agent) unloadChecks() error { // serviceMaintCheckID returns the ID of a given service's maintenance check func serviceMaintCheckID(serviceID string) string { - return fmt.Sprintf("%s:%s", serviceMaintCheckPrefix, serviceID) + return fmt.Sprintf("%s:%s", ServiceMaintCheckPrefix, serviceID) } // EnableServiceMaintenance will register a false health check against the given @@ -1089,7 +1089,7 @@ func (a *Agent) DisableServiceMaintenance(serviceID string) error { // EnableNodeMaintenance places a node into maintenance mode. func (a *Agent) EnableNodeMaintenance(reason string) { // Ensure node maintenance is not already enabled - if _, ok := a.state.Checks()[nodeMaintCheckID]; ok { + if _, ok := a.state.Checks()[NodeMaintCheckID]; ok { return } @@ -1101,7 +1101,7 @@ func (a *Agent) EnableNodeMaintenance(reason string) { // Create and register the node maintenance check check := &structs.HealthCheck{ Node: a.config.NodeName, - CheckID: nodeMaintCheckID, + CheckID: NodeMaintCheckID, Name: "Node Maintenance Mode", Notes: reason, Status: structs.HealthCritical, @@ -1112,9 +1112,9 @@ func (a *Agent) EnableNodeMaintenance(reason string) { // DisableNodeMaintenance removes a node from maintenance mode func (a *Agent) DisableNodeMaintenance() { - if _, ok := a.state.Checks()[nodeMaintCheckID]; !ok { + if _, ok := a.state.Checks()[NodeMaintCheckID]; !ok { return } - a.RemoveCheck(nodeMaintCheckID, true) + a.RemoveCheck(NodeMaintCheckID, true) a.logger.Printf("[INFO] agent: node left maintenance mode") } diff --git a/command/maint.go b/command/maint.go index 41ac598a2c..eb8d6ef518 100644 --- a/command/maint.go +++ b/command/maint.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/command/agent" "github.com/mitchellh/cli" ) @@ -37,6 +38,9 @@ Usage: consul maint [options] "-service" argument, this behavior can be changed to enable or disable only a specific service. + If no arguments are given, the agent's maintenance status will be shown. + This will return blank if nothing is currently under maintenance. + Options: -enable Enable maintenance mode. @@ -70,17 +74,7 @@ func (c *MaintCommand) Run(args []string) int { return 1 } - // Print help if no args given - if len(args) == 0 { - c.Ui.Error(c.Help()) - return 1 - } - // Ensure we don't have conflicting args - if !enable && !disable { - c.Ui.Error("One of -enable or -disable must be specified") - return 1 - } if enable && disable { c.Ui.Error("Only one of -enable or -disable may be provided") return 1 @@ -99,16 +93,42 @@ func (c *MaintCommand) Run(args []string) int { c.Ui.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err)) return 1 } - agent := client.Agent() - if _, err := agent.NodeName(); err != nil { + a := client.Agent() + nodeName, err := a.NodeName() + if err != nil { c.Ui.Error(fmt.Sprintf("Error querying Consul agent: %s", err)) return 1 } + if !enable && !disable { + // List mode - list nodes/services in maintenance mode + checks, err := a.Checks() + if err != nil { + c.Ui.Output(fmt.Sprintf("Error getting checks: %s", err)) + return 1 + } + + for _, check := range checks { + if check.CheckID == agent.NodeMaintCheckID { + c.Ui.Output("Node:") + c.Ui.Output(" Name: " + nodeName) + c.Ui.Output(" Reason: " + check.Notes) + c.Ui.Output("") + } else if strings.HasPrefix(check.CheckID, agent.ServiceMaintCheckPrefix) { + c.Ui.Output("Service:") + c.Ui.Output(" ID: " + check.ServiceID) + c.Ui.Output(" Reason: " + check.Notes) + c.Ui.Output("") + } + } + + return 0 + } + if enable { // Enable node maintenance if serviceID == "" { - if err := agent.EnableNodeMaintenance(reason); err != nil { + if err := a.EnableNodeMaintenance(reason); err != nil { c.Ui.Error(fmt.Sprintf("Error enabling node maintenance: %s", err)) return 1 } @@ -117,7 +137,7 @@ func (c *MaintCommand) Run(args []string) int { } // Enable service maintenance - if err := agent.EnableServiceMaintenance(serviceID, reason); err != nil { + if err := a.EnableServiceMaintenance(serviceID, reason); err != nil { c.Ui.Error(fmt.Sprintf("Error enabling service maintenance: %s", err)) return 1 } @@ -128,7 +148,7 @@ func (c *MaintCommand) Run(args []string) int { if disable { // Disable node maintenance if serviceID == "" { - if err := agent.DisableNodeMaintenance(); err != nil { + if err := a.DisableNodeMaintenance(); err != nil { c.Ui.Error(fmt.Sprintf("Error disabling node maintenance: %s", err)) return 1 } @@ -137,7 +157,7 @@ func (c *MaintCommand) Run(args []string) int { } // Disable service maintenance - if err := agent.DisableServiceMaintenance(serviceID); err != nil { + if err := a.DisableServiceMaintenance(serviceID); err != nil { c.Ui.Error(fmt.Sprintf("Error disabling service maintenance: %s", err)) return 1 } diff --git a/command/maint_test.go b/command/maint_test.go index 66d82f90bb..7b60f2b0db 100644 --- a/command/maint_test.go +++ b/command/maint_test.go @@ -12,19 +12,6 @@ func TestMaintCommand_implements(t *testing.T) { var _ cli.Command = &MaintCommand{} } -func TestMaintCommandRun_NoArgs(t *testing.T) { - ui := new(cli.MockUi) - c := &MaintCommand{Ui: ui} - - if code := c.Run([]string{}); code != 1 { - t.Fatalf("expected return code 1, got %d", code) - } - - if strings.TrimSpace(ui.ErrorWriter.String()) != c.Help() { - t.Fatalf("bad:\n%s", ui.ErrorWriter.String()) - } -} - func TestMaintCommandRun_ConflictingArgs(t *testing.T) { ui := new(cli.MockUi) c := &MaintCommand{Ui: ui} @@ -38,6 +25,53 @@ func TestMaintCommandRun_ConflictingArgs(t *testing.T) { } } +func TestMaintCommandRun_NoArgs(t *testing.T) { + a1 := testAgent(t) + defer a1.Shutdown() + + // Register the service and put it into maintenance mode + service := &structs.NodeService{ + ID: "test", + Service: "test", + } + if err := a1.agent.AddService(service, nil, false); err != nil { + t.Fatalf("err: %v", err) + } + if err := a1.agent.EnableServiceMaintenance("test", "broken 1"); err != nil { + t.Fatalf("err: %s", err) + } + + // Enable node maintenance + a1.agent.EnableNodeMaintenance("broken 2") + + // Run consul maint with no args (list mode) + ui := new(cli.MockUi) + c := &MaintCommand{Ui: ui} + + args := []string{"-http-addr=" + a1.httpAddr} + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + + // Ensure the service shows up in the list + out := ui.OutputWriter.String() + if !strings.Contains(out, "test") { + t.Fatalf("bad:\n%s", out) + } + if !strings.Contains(out, "broken 1") { + t.Fatalf("bad:\n%s", out) + } + + // Ensure the node shows up in the list + if !strings.Contains(out, a1.config.NodeName) { + t.Fatalf("bad:\n%s", out) + } + if !strings.Contains(out, "broken 2") { + t.Fatalf("bad:\n%s", out) + } +} + func TestMaintCommandRun_EnableNodeMaintenance(t *testing.T) { a1 := testAgent(t) defer a1.Shutdown() From de0b6d06bb0d5efd064ea1bda097b2b4812bc7f6 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Thu, 22 Jan 2015 10:29:48 -0800 Subject: [PATCH 13/15] website: update maint command documentation for list mode --- .../source/docs/commands/maint.html.markdown | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/website/source/docs/commands/maint.html.markdown b/website/source/docs/commands/maint.html.markdown index 90378bbbde..f906332e23 100644 --- a/website/source/docs/commands/maint.html.markdown +++ b/website/source/docs/commands/maint.html.markdown @@ -24,9 +24,7 @@ health check. Usage: `consul maint [options]` -Exactly one of `-enable` or `-disable` are required. The rest of the command -line arguments are optional, and some are only usable in combination with -others. +All of the command line arguments are optional. The list of available flags are: @@ -51,3 +49,20 @@ The list of available flags are: * `-http-addr` - Address to the HTTP server of the agent you want to contact to send this command. If this isn't specified, the command will contact "127.0.0.1:8500" which is the default HTTP address of a Consul agent. + +## List mode + +If neither `-enable` nor `-disable` are passed, the `maint` command will +switch to "list mode", displaying any current maintenances. This may return +blank if nothing is currently under maintenance. The output will look like: + +``` +$ consul maint +Node: + Name: node1.local + Reason: This node is broken. + +Service: + ID: redis + Reason: Redis is currently offline. +``` From 44f3c20e502eddf5f2606cffc702582cd0ee6e9c Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Thu, 22 Jan 2015 11:14:28 -0800 Subject: [PATCH 14/15] command/maint: clean up --- command/agent/agent.go | 22 +++++++++++----------- command/maint.go | 5 ++--- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index 95ef455725..5aeb783a80 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -32,8 +32,8 @@ const ( "and try again." // The ID of the faux health checks for maintenance mode - ServiceMaintCheckPrefix = "_service_maintenance" - NodeMaintCheckID = "_node_maintenance" + serviceMaintCheckPrefix = "_service_maintenance" + nodeMaintCheckID = "_node_maintenance" // Default reasons for node/service maintenance mode defaultNodeMaintReason = "Maintenance mode is enabled for this node, " + @@ -1028,7 +1028,7 @@ func (a *Agent) unloadChecks() error { // serviceMaintCheckID returns the ID of a given service's maintenance check func serviceMaintCheckID(serviceID string) string { - return fmt.Sprintf("%s:%s", ServiceMaintCheckPrefix, serviceID) + return fmt.Sprintf("%s:%s", serviceMaintCheckPrefix, serviceID) } // EnableServiceMaintenance will register a false health check against the given @@ -1061,7 +1061,7 @@ func (a *Agent) EnableServiceMaintenance(serviceID, reason string) error { Status: structs.HealthCritical, } a.AddCheck(check, nil, true) - a.logger.Printf("[INFO] agent: service %q entered maintenance mode", serviceID) + a.logger.Printf("[INFO] agent: Service %q entered maintenance mode", serviceID) return nil } @@ -1081,7 +1081,7 @@ func (a *Agent) DisableServiceMaintenance(serviceID string) error { // Deregister the maintenance check a.RemoveCheck(checkID, true) - a.logger.Printf("[INFO] agent: service %q left maintenance mode", serviceID) + a.logger.Printf("[INFO] agent: Service %q left maintenance mode", serviceID) return nil } @@ -1089,7 +1089,7 @@ func (a *Agent) DisableServiceMaintenance(serviceID string) error { // EnableNodeMaintenance places a node into maintenance mode. func (a *Agent) EnableNodeMaintenance(reason string) { // Ensure node maintenance is not already enabled - if _, ok := a.state.Checks()[NodeMaintCheckID]; ok { + if _, ok := a.state.Checks()[nodeMaintCheckID]; ok { return } @@ -1101,20 +1101,20 @@ func (a *Agent) EnableNodeMaintenance(reason string) { // Create and register the node maintenance check check := &structs.HealthCheck{ Node: a.config.NodeName, - CheckID: NodeMaintCheckID, + CheckID: nodeMaintCheckID, Name: "Node Maintenance Mode", Notes: reason, Status: structs.HealthCritical, } a.AddCheck(check, nil, true) - a.logger.Printf("[INFO] agent: node entered maintenance mode") + a.logger.Printf("[INFO] agent: Node entered maintenance mode") } // DisableNodeMaintenance removes a node from maintenance mode func (a *Agent) DisableNodeMaintenance() { - if _, ok := a.state.Checks()[NodeMaintCheckID]; !ok { + if _, ok := a.state.Checks()[nodeMaintCheckID]; !ok { return } - a.RemoveCheck(NodeMaintCheckID, true) - a.logger.Printf("[INFO] agent: node left maintenance mode") + a.RemoveCheck(nodeMaintCheckID, true) + a.logger.Printf("[INFO] agent: Node left maintenance mode") } diff --git a/command/maint.go b/command/maint.go index eb8d6ef518..4c89c34825 100644 --- a/command/maint.go +++ b/command/maint.go @@ -6,7 +6,6 @@ import ( "strings" "github.com/hashicorp/consul/api" - "github.com/hashicorp/consul/command/agent" "github.com/mitchellh/cli" ) @@ -109,12 +108,12 @@ func (c *MaintCommand) Run(args []string) int { } for _, check := range checks { - if check.CheckID == agent.NodeMaintCheckID { + if check.CheckID == "_node_maintenance" { c.Ui.Output("Node:") c.Ui.Output(" Name: " + nodeName) c.Ui.Output(" Reason: " + check.Notes) c.Ui.Output("") - } else if strings.HasPrefix(check.CheckID, agent.ServiceMaintCheckPrefix) { + } else if strings.HasPrefix(check.CheckID, "_service_maintenance:") { c.Ui.Output("Service:") c.Ui.Output(" ID: " + check.ServiceID) c.Ui.Output(" Reason: " + check.Notes) From 5049a5eb7b1e7d1b8cb8830db828a7b23bb1caf2 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Thu, 22 Jan 2015 11:20:32 -0800 Subject: [PATCH 15/15] command/maint: better arg conflict checking --- command/maint.go | 8 ++++++-- command/maint_test.go | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/command/maint.go b/command/maint.go index 4c89c34825..e1319db526 100644 --- a/command/maint.go +++ b/command/maint.go @@ -78,10 +78,14 @@ func (c *MaintCommand) Run(args []string) int { c.Ui.Error("Only one of -enable or -disable may be provided") return 1 } - if disable && reason != "" { + if !enable && reason != "" { c.Ui.Error("Reason may only be provided with -enable") return 1 } + if !enable && !disable && serviceID != "" { + c.Ui.Error("Service requires either -enable or -disable") + return 1 + } // Create and test the HTTP client conf := api.DefaultConfig() @@ -103,7 +107,7 @@ func (c *MaintCommand) Run(args []string) int { // List mode - list nodes/services in maintenance mode checks, err := a.Checks() if err != nil { - c.Ui.Output(fmt.Sprintf("Error getting checks: %s", err)) + c.Ui.Error(fmt.Sprintf("Error getting checks: %s", err)) return 1 } diff --git a/command/maint_test.go b/command/maint_test.go index 7b60f2b0db..92bf02b767 100644 --- a/command/maint_test.go +++ b/command/maint_test.go @@ -23,6 +23,14 @@ func TestMaintCommandRun_ConflictingArgs(t *testing.T) { if code := c.Run([]string{"-disable", "-reason=broken"}); code != 1 { t.Fatalf("expected return code 1, got %d", code) } + + if code := c.Run([]string{"-reason=broken"}); code != 1 { + t.Fatalf("expected return code 1, got %d", code) + } + + if code := c.Run([]string{"-service=redis"}); code != 1 { + t.Fatalf("expected return code 1, got %d", code) + } } func TestMaintCommandRun_NoArgs(t *testing.T) {