Clean up some leftover autopilot differences from Consul

api/operator_autopilot.go

@@ -1,12 +1,8 @@
 package api
 
 import (
-	"bytes"
 	"encoding/json"
-	"fmt"
-	"io"
 	"strconv"
-	"strings"
 	"time"
 )
 
@@ -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
 }

api/operator_autopilot_test.go

@@ -7,54 +7,58 @@ 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)
+	var config *AutopilotConfiguration
+	retry.Run(t, func(r *retry.R) {
+		var err error
+		config, _, err = operator.AutopilotGetConfiguration(nil)
+		r.Check(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
@@ -63,9 +67,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)
 	}
 }
 
@@ -79,7 +83,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)
 		}

command/agent/operator_endpoint.go

@@ -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 {
@@ -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{
@@ -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
@@ -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

command/operator_autopilot_get.go

@@ -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

command/operator_autopilot_set.go

@@ -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
@@ -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

command/operator_autopilot_set_test.go

@@ -42,7 +42,7 @@ func TestOperatorAutopilotSetConfigCommmand(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	conf, err := client.Operator().AutopilotGetConfiguration(nil)
+	conf, _, err := client.Operator().AutopilotGetConfiguration(nil)
 	if err != nil {
 		t.Fatal(err)
 	}

nomad/server.go

@@ -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 {