Skip to content

Commit

Permalink
Clean up some leftover autopilot differences from Consul
Browse files Browse the repository at this point in the history
  • Loading branch information
kyhavlov committed Feb 8, 2018
1 parent 5fad228 commit 96894cb
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 146 deletions.
84 changes: 20 additions & 64 deletions api/operator_autopilot.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
package api

import (
"bytes"
"encoding/json"
"fmt"
"io"
"strconv"
"strings"
"time"
)

Expand Down Expand Up @@ -178,85 +174,45 @@ type OperatorHealthReply struct {
}

// AutopilotGetConfiguration is used to query the current Autopilot configuration.
func (op *Operator) AutopilotGetConfiguration(q *QueryOptions) (*AutopilotConfiguration, error) {
r, err := op.c.newRequest("GET", "/v1/operator/autopilot/configuration")
func (op *Operator) AutopilotGetConfiguration(q *QueryOptions) (*AutopilotConfiguration, *QueryMeta, error) {
var resp AutopilotConfiguration
qm, err := op.c.query("/v1/operator/autopilot/configuration", &resp, q)
if err != nil {
return nil, err
}
r.setQueryOptions(q)
_, resp, err := requireOK(op.c.doRequest(r))
if err != nil {
return nil, err
}
defer resp.Body.Close()

var out AutopilotConfiguration
if err := decodeBody(resp, &out); err != nil {
return nil, err
return nil, nil, err
}

return &out, nil
return &resp, qm, nil
}

// AutopilotSetConfiguration is used to set the current Autopilot configuration.
func (op *Operator) AutopilotSetConfiguration(conf *AutopilotConfiguration, q *WriteOptions) error {
r, err := op.c.newRequest("PUT", "/v1/operator/autopilot/configuration")
if err != nil {
return err
}
r.setWriteOptions(q)
r.obj = conf
_, resp, err := requireOK(op.c.doRequest(r))
func (op *Operator) AutopilotSetConfiguration(conf *AutopilotConfiguration, q *WriteOptions) (*WriteMeta, error) {
var out bool
wm, err := op.c.write("/v1/operator/autopilot/configuration", conf, &out, q)
if err != nil {
return err
return nil, err
}
resp.Body.Close()
return nil
return wm, nil
}

// AutopilotCASConfiguration is used to perform a Check-And-Set update on the
// Autopilot configuration. The ModifyIndex value will be respected. Returns
// true on success or false on failures.
func (op *Operator) AutopilotCASConfiguration(conf *AutopilotConfiguration, q *WriteOptions) (bool, error) {
r, err := op.c.newRequest("PUT", "/v1/operator/autopilot/configuration")
func (op *Operator) AutopilotCASConfiguration(conf *AutopilotConfiguration, q *WriteOptions) (bool, *WriteMeta, error) {
var out bool
wm, err := op.c.write("/v1/operator/autopilot/configuration?cas="+strconv.FormatUint(conf.ModifyIndex, 10), conf, &out, q)
if err != nil {
return false, err
return false, nil, err
}
r.setWriteOptions(q)
r.params.Set("cas", strconv.FormatUint(conf.ModifyIndex, 10))
r.obj = conf
_, resp, err := requireOK(op.c.doRequest(r))
if err != nil {
return false, err
}
defer resp.Body.Close()

var buf bytes.Buffer
if _, err := io.Copy(&buf, resp.Body); err != nil {
return false, fmt.Errorf("Failed to read response: %v", err)
}
res := strings.Contains(buf.String(), "true")

return res, nil
return out, wm, nil
}

// AutopilotServerHealth is used to query Autopilot's top-level view of the health
// of each Nomad server.
func (op *Operator) AutopilotServerHealth(q *QueryOptions) (*OperatorHealthReply, error) {
r, err := op.c.newRequest("GET", "/v1/operator/autopilot/health")
if err != nil {
return nil, err
}
r.setQueryOptions(q)
_, resp, err := requireOK(op.c.doRequest(r))
if err != nil {
return nil, err
}
defer resp.Body.Close()

func (op *Operator) AutopilotServerHealth(q *QueryOptions) (*OperatorHealthReply, *QueryMeta, error) {
var out OperatorHealthReply
if err := decodeBody(resp, &out); err != nil {
return nil, err
qm, err := op.c.query("/v1/operator/autopilot/health", &out, q)
if err != nil {
return nil, nil, err
}
return &out, nil
return &out, qm, nil
}
40 changes: 20 additions & 20 deletions api/operator_autopilot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,54 +7,54 @@ import (

"github.com/hashicorp/consul/testutil/retry"
"github.com/hashicorp/nomad/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestAPI_OperatorAutopilotGetSetConfiguration(t *testing.T) {
t.Parallel()
assert := assert.New(t)
require := require.New(t)
c, s := makeClient(t, nil, nil)
defer s.Stop()

operator := c.Operator()
var config *AutopilotConfiguration
retry.Run(t, func(r *retry.R) {
var err error
config, err = operator.AutopilotGetConfiguration(nil)
config, _, err = operator.AutopilotGetConfiguration(nil)
r.Check(err)
})
assert.True(config.CleanupDeadServers)
require.True(config.CleanupDeadServers)

// Change a config setting
newConf := &AutopilotConfiguration{CleanupDeadServers: false}
err := operator.AutopilotSetConfiguration(newConf, nil)
assert.Nil(err)
_, err := operator.AutopilotSetConfiguration(newConf, nil)
require.Nil(err)

config, err = operator.AutopilotGetConfiguration(nil)
assert.Nil(err)
assert.False(config.CleanupDeadServers)
config, _, err = operator.AutopilotGetConfiguration(nil)
require.Nil(err)
require.False(config.CleanupDeadServers)
}

func TestAPI_OperatorAutopilotCASConfiguration(t *testing.T) {
t.Parallel()
assert := assert.New(t)
require := require.New(t)
c, s := makeClient(t, nil, nil)
defer s.Stop()

operator := c.Operator()
config, err := operator.AutopilotGetConfiguration(nil)
assert.Nil(err)
assert.True(config.CleanupDeadServers)
config, _, err := operator.AutopilotGetConfiguration(nil)
require.Nil(err)
require.True(config.CleanupDeadServers)

// Pass an invalid ModifyIndex
{
newConf := &AutopilotConfiguration{
CleanupDeadServers: false,
ModifyIndex: config.ModifyIndex - 1,
}
resp, err := operator.AutopilotCASConfiguration(newConf, nil)
assert.Nil(err)
assert.False(resp)
resp, _, err := operator.AutopilotCASConfiguration(newConf, nil)
require.Nil(err)
require.False(resp)
}

// Pass a valid ModifyIndex
Expand All @@ -63,9 +63,9 @@ func TestAPI_OperatorAutopilotCASConfiguration(t *testing.T) {
CleanupDeadServers: false,
ModifyIndex: config.ModifyIndex,
}
resp, err := operator.AutopilotCASConfiguration(newConf, nil)
assert.Nil(err)
assert.True(resp)
resp, _, err := operator.AutopilotCASConfiguration(newConf, nil)
require.Nil(err)
require.True(resp)
}
}

Expand All @@ -79,7 +79,7 @@ func TestAPI_OperatorAutopilotServerHealth(t *testing.T) {

operator := c.Operator()
retry.Run(t, func(r *retry.R) {
out, err := operator.AutopilotServerHealth(nil)
out, _, err := operator.AutopilotServerHealth(nil)
if err != nil {
r.Fatalf("err: %v", err)
}
Expand Down
28 changes: 8 additions & 20 deletions command/agent/operator_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,18 @@ func (s *HTTPServer) OperatorRaftConfiguration(resp http.ResponseWriter, req *ht
// removing peers by address.
func (s *HTTPServer) OperatorRaftPeer(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
if req.Method != "DELETE" {
resp.WriteHeader(http.StatusMethodNotAllowed)
return nil, nil
return nil, CodedError(404, ErrInvalidMethod)
}

params := req.URL.Query()
_, hasID := params["id"]
_, hasAddress := params["address"]

if !hasID && !hasAddress {
resp.WriteHeader(http.StatusBadRequest)
fmt.Fprint(resp, "Must specify either ?id with the server's ID or ?address with IP:port of peer to remove")
return nil, nil
return nil, CodedError(http.StatusBadRequest, "Must specify either ?id with the server's ID or ?address with IP:port of peer to remove")
}
if hasID && hasAddress {
resp.WriteHeader(http.StatusBadRequest)
fmt.Fprint(resp, "Must specify only one of ?id or ?address")
return nil, nil
return nil, CodedError(http.StatusBadRequest, "Must specify only one of ?id or ?address")
}

if hasID {
Expand Down Expand Up @@ -125,14 +120,11 @@ func (s *HTTPServer) OperatorAutopilotConfiguration(resp http.ResponseWriter, re

case "PUT":
var args structs.AutopilotSetConfigRequest
s.parseRegion(req, &args.Region)
s.parseToken(req, &args.AuthToken)
s.parseWriteRequest(req, &args.WriteRequest)

var conf api.AutopilotConfiguration
if err := decodeBody(req, &conf); err != nil {
resp.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(resp, "Error parsing autopilot config: %v", err)
return nil, nil
return nil, CodedError(http.StatusBadRequest, fmt.Sprintf("Error parsing autopilot config: %v", err))
}

args.Config = structs.AutopilotConfig{
Expand All @@ -150,9 +142,7 @@ func (s *HTTPServer) OperatorAutopilotConfiguration(resp http.ResponseWriter, re
if _, ok := params["cas"]; ok {
casVal, err := strconv.ParseUint(params.Get("cas"), 10, 64)
if err != nil {
resp.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(resp, "Error parsing cas value: %v", err)
return nil, nil
return nil, CodedError(http.StatusBadRequest, fmt.Sprintf("Error parsing cas value: %v", err))
}
args.Config.ModifyIndex = casVal
args.CAS = true
Expand All @@ -170,16 +160,14 @@ func (s *HTTPServer) OperatorAutopilotConfiguration(resp http.ResponseWriter, re
return reply, nil

default:
resp.WriteHeader(http.StatusMethodNotAllowed)
return nil, nil
return nil, CodedError(404, ErrInvalidMethod)
}
}

// OperatorServerHealth is used to get the health of the servers in the given Region.
func (s *HTTPServer) OperatorServerHealth(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
if req.Method != "GET" {
resp.WriteHeader(http.StatusMethodNotAllowed)
return nil, nil
return nil, CodedError(404, ErrInvalidMethod)
}

var args structs.GenericRequest
Expand Down
2 changes: 1 addition & 1 deletion command/operator_autopilot_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (c *OperatorAutopilotGetCommand) Run(args []string) int {
}

// Fetch the current configuration.
config, err := client.Operator().AutopilotGetConfiguration(nil)
config, _, err := client.Operator().AutopilotGetConfiguration(nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error querying Autopilot configuration: %s", err))
return 1
Expand Down
4 changes: 2 additions & 2 deletions command/operator_autopilot_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (c *OperatorAutopilotSetCommand) Run(args []string) int {

// Fetch the current configuration.
operator := client.Operator()
conf, err := operator.AutopilotGetConfiguration(nil)
conf, _, err := operator.AutopilotGetConfiguration(nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error querying for Autopilot configuration: %s", err))
return 1
Expand All @@ -82,7 +82,7 @@ func (c *OperatorAutopilotSetCommand) Run(args []string) int {
serverStabilizationTime.Merge(&conf.ServerStabilizationTime)

// Check-and-set the new configuration.
result, err := operator.AutopilotCASConfiguration(conf, nil)
result, _, err := operator.AutopilotCASConfiguration(conf, nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error setting Autopilot configuration: %s", err))
return 1
Expand Down
2 changes: 0 additions & 2 deletions nomad/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -893,8 +893,6 @@ func (s *Server) setupRPC(tlsWrap tlsutil.RegionWrapper) error {
return err
}

s.logger.Printf("[INFO] nomad: RPC listening on %q", s.rpcListener.Addr().String())

if s.config.RPCAdvertise != nil {
s.rpcAdvertise = s.config.RPCAdvertise
} else {
Expand Down
Loading

0 comments on commit 96894cb

Please sign in to comment.