Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up some leftover autopilot differences from Consul #3852

Merged
merged 1 commit into from
Feb 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}
44 changes: 24 additions & 20 deletions api/operator_autopilot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
}

Expand All @@ -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)
}
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: 1 addition & 1 deletion command/operator_autopilot_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
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())

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did this go away?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment: #3670 (comment)

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