From 27caae2b2af505caf95ce91e013b641ac1b89514 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 10 Apr 2025 09:19:25 -0400 Subject: [PATCH] api: make attempting to remove peer by address a no-op (#25599) In Nomad 1.4.0 we removed support for Raft Protocol v2 entirely. But the `Operator.RemoveRaftPeerByAddress` RPC handler was left in place, along with its supporting HTTP API and command line flags. Using this API will always result in the Raft library error "operation not supported with current protocol version". Unfortunately it's still possible in unit tests to exercise this code path, and these tests are quite flaky. This changeset turns the RPC handler and HTTP API into a no-op, removes the associated command line flags, and removes the flaky tests. I've also cleaned up the test for `RemoveRaftPeerByID` to consolidate test servers and use `shoenig/test`. Fixes: https://hashicorp.atlassian.net/browse/NET-12413 Ref: https://github.com/hashicorp/nomad/pull/13467 Ref: https://developer.hashicorp.com/nomad/docs/upgrade/upgrade-specific#raft-protocol-version-2-unsupported Ref: https://github.com/hashicorp/nomad-enterprise/actions/runs/13201513025/job/36855234398?pr=2302 --- .changelog/25599.txt | 3 + api/operator.go | 4 + api/operator_test.go | 13 - command/agent/operator_endpoint.go | 31 +-- command/agent/operator_endpoint_test.go | 25 +- command/operator_raft_remove.go | 45 +--- command/operator_raft_remove_test.go | 56 +---- nomad/operator_endpoint.go | 61 +---- nomad/operator_endpoint_test.go | 223 +++--------------- website/content/api-docs/operator/raft.mdx | 10 - .../commands/operator/raft/remove-peer.mdx | 5 +- .../content/docs/upgrade/upgrade-specific.mdx | 10 + 12 files changed, 87 insertions(+), 399 deletions(-) create mode 100644 .changelog/25599.txt diff --git a/.changelog/25599.txt b/.changelog/25599.txt new file mode 100644 index 000000000..e5517886d --- /dev/null +++ b/.changelog/25599.txt @@ -0,0 +1,3 @@ +```release-note:breaking-change +api: The non-functional option -peer-address has been removed from the operator raft remove-peer command and equivalent API +``` diff --git a/api/operator.go b/api/operator.go index a6f11f45e..0b29e37fd 100644 --- a/api/operator.go +++ b/api/operator.go @@ -82,6 +82,10 @@ func (op *Operator) RaftGetConfiguration(q *QueryOptions) (*RaftConfiguration, e // RaftRemovePeerByAddress is used to kick a stale peer (one that it in the Raft // quorum but no longer known to Serf or the catalog) by address in the form of // "IP:port". +// +// DEPRECATED: this method supported Raft Protocol v2, which was removed from +// Nomad in 1.4.0. The address parameter of the HTTP endpoint has been made +// non-function in Nomad 1.10.x and will be removed in Nomad 1.12.0. func (op *Operator) RaftRemovePeerByAddress(address string, q *WriteOptions) error { r, err := op.c.newRequest("DELETE", "/v1/operator/raft/peer") if err != nil { diff --git a/api/operator_test.go b/api/operator_test.go index c04b3252e..cec717d55 100644 --- a/api/operator_test.go +++ b/api/operator_test.go @@ -24,19 +24,6 @@ func TestOperator_RaftGetConfiguration(t *testing.T) { must.True(t, out.Servers[0].Voter) } -func TestOperator_RaftRemovePeerByAddress(t *testing.T) { - testutil.Parallel(t) - - c, s := makeClient(t, nil, nil) - defer s.Stop() - - // If we get this error, it proves we sent the address all the way - // through. - operator := c.Operator() - err := operator.RaftRemovePeerByAddress("nope", nil) - must.ErrorContains(t, err, `address "nope" was not found in the Raft configuration`) -} - func TestOperator_RaftRemovePeerByID(t *testing.T) { testutil.Parallel(t) diff --git a/command/agent/operator_endpoint.go b/command/agent/operator_endpoint.go index 5140bd28f..d262fee79 100644 --- a/command/agent/operator_endpoint.go +++ b/command/agent/operator_endpoint.go @@ -68,31 +68,20 @@ func (s *HTTPServer) OperatorRaftPeer(resp http.ResponseWriter, req *http.Reques _, hasID := params["id"] _, hasAddress := params["address"] - if !hasID && !hasAddress { - return nil, CodedError(http.StatusBadRequest, "Must specify either ?id with the server's ID or ?address with IP:port of peer to remove") + if hasAddress { + return nil, CodedError(http.StatusBadRequest, "Removing a peer by address is not supported in the current raft protocol version") } - if hasID && hasAddress { - return nil, CodedError(http.StatusBadRequest, "Must specify only one of ?id or ?address") + if !hasID { + return nil, CodedError(http.StatusBadRequest, "Must specify the peer's ID to remove") } - if hasID { - var args structs.RaftPeerByIDRequest - s.parseWriteRequest(req, &args.WriteRequest) + var args structs.RaftPeerByIDRequest + s.parseWriteRequest(req, &args.WriteRequest) - var reply struct{} - args.ID = raft.ServerID(params.Get("id")) - if err := s.agent.RPC("Operator.RaftRemovePeerByID", &args, &reply); err != nil { - return nil, err - } - } else { - var args structs.RaftPeerByAddressRequest - s.parseWriteRequest(req, &args.WriteRequest) - - var reply struct{} - args.Address = raft.ServerAddress(params.Get("address")) - if err := s.agent.RPC("Operator.RaftRemovePeerByAddress", &args, &reply); err != nil { - return nil, err - } + var reply struct{} + args.ID = raft.ServerID(params.Get("id")) + if err := s.agent.RPC("Operator.RaftRemovePeerByID", &args, &reply); err != nil { + return nil, err } return nil, nil diff --git a/command/agent/operator_endpoint_test.go b/command/agent/operator_endpoint_test.go index b1054073e..cc1ccb8fa 100644 --- a/command/agent/operator_endpoint_test.go +++ b/command/agent/operator_endpoint_test.go @@ -14,7 +14,6 @@ import ( "os" "path" "path/filepath" - "strings" "testing" "time" @@ -26,7 +25,6 @@ import ( "github.com/hashicorp/nomad/nomad/structs" "github.com/shoenig/test/must" "github.com/shoenig/test/wait" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -61,35 +59,18 @@ func TestHTTP_OperatorRaftConfiguration(t *testing.T) { func TestHTTP_OperatorRaftPeer(t *testing.T) { ci.Parallel(t) - assert := assert.New(t) - httpTest(t, nil, func(s *TestAgent) { - body := bytes.NewBuffer(nil) - req, err := http.NewRequest(http.MethodDelete, "/v1/operator/raft/peer?address=nope", body) - assert.Nil(err) - - // If we get this error, it proves we sent the address all the - // way through. - resp := httptest.NewRecorder() - _, err = s.Server.OperatorRaftPeer(resp, req) - if err == nil || !strings.Contains(err.Error(), - "address \"nope\" was not found in the Raft configuration") { - t.Fatalf("err: %v", err) - } - }) httpTest(t, nil, func(s *TestAgent) { body := bytes.NewBuffer(nil) req, err := http.NewRequest(http.MethodDelete, "/v1/operator/raft/peer?id=nope", body) - assert.Nil(err) + must.NoError(t, err) // If we get this error, it proves we sent the address all the // way through. resp := httptest.NewRecorder() _, err = s.Server.OperatorRaftPeer(resp, req) - if err == nil || !strings.Contains(err.Error(), - "id \"nope\" was not found in the Raft configuration") { - t.Fatalf("err: %v", err) - } + must.ErrorContains(t, err, + "id \"nope\" was not found in the Raft configuration") }) } diff --git a/command/operator_raft_remove.go b/command/operator_raft_remove.go index ee28382fe..58fbe590e 100644 --- a/command/operator_raft_remove.go +++ b/command/operator_raft_remove.go @@ -7,7 +7,6 @@ import ( "fmt" "strings" - "github.com/hashicorp/nomad/api" "github.com/posener/complete" ) @@ -19,7 +18,7 @@ func (c *OperatorRaftRemoveCommand) Help() string { helpText := ` Usage: nomad operator raft remove-peer [options] - Remove the Nomad server with given -peer-address from the Raft configuration. + Remove the Nomad server with given -peer-id from the Raft configuration. There are rare cases where a peer may be left behind in the Raft quorum even though the server is no longer present and known to the cluster. This command @@ -36,9 +35,6 @@ General Options: Remove Peer Options: - -peer-address="IP:port" - Remove a Nomad server with given address from the Raft configuration. - -peer-id="id" Remove a Nomad server with the given ID from the Raft configuration. ` @@ -48,8 +44,7 @@ Remove Peer Options: func (c *OperatorRaftRemoveCommand) AutocompleteFlags() complete.Flags { return mergeAutocompleteFlags(c.Meta.AutocompleteFlags(FlagSetClient), complete.Flags{ - "-peer-address": complete.PredictAnything, - "-peer-id": complete.PredictAnything, + "-peer-id": complete.PredictAnything, }) } @@ -64,18 +59,20 @@ func (c *OperatorRaftRemoveCommand) Synopsis() string { func (c *OperatorRaftRemoveCommand) Name() string { return "operator raft remove-peer" } func (c *OperatorRaftRemoveCommand) Run(args []string) int { - var peerAddress string var peerID string flags := c.Meta.FlagSet("raft", FlagSetClient) flags.Usage = func() { c.Ui.Output(c.Help()) } - flags.StringVar(&peerAddress, "peer-address", "", "") flags.StringVar(&peerID, "peer-id", "", "") if err := flags.Parse(args); err != nil { c.Ui.Error(fmt.Sprintf("Failed to parse args: %v", err)) return 1 } + if peerID == "" { + c.Ui.Error("Missing peer id required") + return 1 + } // Set up a client. client, err := c.Meta.Client() @@ -85,37 +82,11 @@ func (c *OperatorRaftRemoveCommand) Run(args []string) int { } operator := client.Operator() - if err := raftRemovePeers(peerAddress, peerID, operator); err != nil { + if err := operator.RaftRemovePeerByID(peerID, nil); err != nil { c.Ui.Error(fmt.Sprintf("Error removing peer: %v", err)) return 1 } - if peerAddress != "" { - c.Ui.Output(fmt.Sprintf("Removed peer with address %q", peerAddress)) - } else { - c.Ui.Output(fmt.Sprintf("Removed peer with id %q", peerID)) - } + c.Ui.Output(fmt.Sprintf("Removed peer with id %q", peerID)) return 0 } - -func raftRemovePeers(address, id string, operator *api.Operator) error { - if len(address) == 0 && len(id) == 0 { - return fmt.Errorf("an address or id is required for the peer to remove") - } - if len(address) > 0 && len(id) > 0 { - return fmt.Errorf("cannot give both an address and id") - } - - // Try to kick the peer. - if len(address) > 0 { - if err := operator.RaftRemovePeerByAddress(address, nil); err != nil { - return err - } - } else { - if err := operator.RaftRemovePeerByID(id, nil); err != nil { - return err - } - } - - return nil -} diff --git a/command/operator_raft_remove_test.go b/command/operator_raft_remove_test.go index 1278c0504..9b0ba55eb 100644 --- a/command/operator_raft_remove_test.go +++ b/command/operator_raft_remove_test.go @@ -11,66 +11,24 @@ import ( "github.com/shoenig/test/must" ) -func TestOperator_Raft_RemovePeers_Implements(t *testing.T) { - ci.Parallel(t) - var _ cli.Command = &OperatorRaftRemoveCommand{} -} - func TestOperator_Raft_RemovePeer(t *testing.T) { ci.Parallel(t) + var _ cli.Command = &OperatorRaftRemoveCommand{} s, _, addr := testServer(t, false, nil) defer s.Shutdown() ui := cli.NewMockUi() c := &OperatorRaftRemoveCommand{Meta: Meta{Ui: ui}} - args := []string{"-address=" + addr, "-peer-address=nope", "-peer-id=nope"} - - // Give both an address and ID + args := []string{"-address=" + addr} code := c.Run(args) - if code != 1 { - t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) - } + must.One(t, code) + must.StrContains(t, ui.ErrorWriter.String(), "Missing peer id required") - must.StrContains(t, ui.ErrorWriter.String(), "cannot give both an address and id") - - // Neither address nor ID present - args = args[:1] + ui.ErrorWriter.Reset() + args = []string{"-address=" + addr, "-peer-id=nope"} code = c.Run(args) must.One(t, code) - must.StrContains(t, ui.ErrorWriter.String(), "an address or id is required for the peer to remove") -} - -func TestOperator_Raft_RemovePeerAddress(t *testing.T) { - ci.Parallel(t) - - s, _, addr := testServer(t, false, nil) - defer s.Shutdown() - - ui := cli.NewMockUi() - c := &OperatorRaftRemoveCommand{Meta: Meta{Ui: ui}} - args := []string{"-address=" + addr, "-peer-address=nope"} - - code := c.Run(args) - must.One(t, code) - - // If we get this error, it proves we sent the address all they through. - must.StrContains(t, ui.ErrorWriter.String(), "address \"nope\" was not found in the Raft configuration") -} - -func TestOperator_Raft_RemovePeerID(t *testing.T) { - ci.Parallel(t) - - s, _, addr := testServer(t, false, nil) - defer s.Shutdown() - - ui := cli.NewMockUi() - c := &OperatorRaftRemoveCommand{Meta: Meta{Ui: ui}} - args := []string{"-address=" + addr, "-peer-id=nope"} - - code := c.Run(args) - must.One(t, code) - - // If we get this error, it proves we sent the address all they through. + // If we get this error, it proves we sent the peer ID all the way thru must.StrContains(t, ui.ErrorWriter.String(), "id \"nope\" was not found in the Raft configuration") } diff --git a/nomad/operator_endpoint.go b/nomad/operator_endpoint.go index 1c54fe157..0315116e4 100644 --- a/nomad/operator_endpoint.go +++ b/nomad/operator_endpoint.go @@ -104,62 +104,11 @@ func (op *Operator) RaftGetConfiguration(args *structs.GenericRequest, reply *st return nil } -// RaftRemovePeerByAddress is used to kick a stale peer (one that it in the Raft -// quorum but no longer known to Serf or the catalog) by address in the form of -// "IP:port". The reply argument is not used, but it required to fulfill the RPC -// interface. -func (op *Operator) RaftRemovePeerByAddress(args *structs.RaftPeerByAddressRequest, reply *struct{}) error { - - authErr := op.srv.Authenticate(op.ctx, args) - if done, err := op.srv.forward("Operator.RaftRemovePeerByAddress", args, args, reply); done { - return err - } - op.srv.MeasureRPCRate("operator", structs.RateMetricWrite, args) - if authErr != nil { - return structs.ErrPermissionDenied - } - - // Check management permissions - if aclObj, err := op.srv.ResolveACL(args); err != nil { - return err - } else if !aclObj.IsManagement() { - return structs.ErrPermissionDenied - } - - // Since this is an operation designed for humans to use, we will return - // an error if the supplied address isn't among the peers since it's - // likely a mistake. - { - future := op.srv.raft.GetConfiguration() - if err := future.Error(); err != nil { - return err - } - for _, s := range future.Configuration().Servers { - if s.Address == args.Address { - goto REMOVE - } - } - return fmt.Errorf("address %q was not found in the Raft configuration", - args.Address) - } - -REMOVE: - // The Raft library itself will prevent various forms of foot-shooting, - // like making a configuration with no voters. Some consideration was - // given here to adding more checks, but it was decided to make this as - // low-level and direct as possible. We've got ACL coverage to lock this - // down, and if you are an operator, it's assumed you know what you are - // doing if you are calling this. If you remove a peer that's known to - // Serf, for example, it will come back when the leader does a reconcile - // pass. - future := op.srv.raft.RemovePeer(args.Address) - if err := future.Error(); err != nil { - op.logger.Warn("failed to remove Raft peer", "peer", args.Address, "error", err) - return err - } - - op.logger.Warn("removed Raft peer", "peer", args.Address) - return nil +// COMPAT(1.12.0): RaftRemovePeerByAddress was used to support Raft Protocol v2, +// which was removed in Nomad 1.4.0 but the API was not removed. Remove this RPC +// entirely in Nomad 1.12.0. +func (op *Operator) RaftRemovePeerByAddress(_ *structs.RaftPeerByAddressRequest, _ *struct{}) error { + return structs.NewErrRPCCoded(400, "Operator.RaftRemovePeerByAddress has been removed") } // RaftRemovePeerByID is used to kick a stale peer (one that is in the Raft diff --git a/nomad/operator_endpoint_test.go b/nomad/operator_endpoint_test.go index 58a0f0f8f..a5110856c 100644 --- a/nomad/operator_endpoint_test.go +++ b/nomad/operator_endpoint_test.go @@ -13,7 +13,6 @@ import ( "os" "path" "reflect" - "strings" "sync" "testing" "time" @@ -147,198 +146,31 @@ func TestOperator_RaftGetConfiguration_ACL(t *testing.T) { } } -func TestOperator_RaftRemovePeerByAddress(t *testing.T) { - ci.Parallel(t) - - s1, cleanupS1 := TestServer(t, func(c *Config) { - c.RaftConfig.ProtocolVersion = raft.ProtocolVersion(2) - }) - defer cleanupS1() - codec := rpcClient(t, s1) - testutil.WaitForLeader(t, s1.RPC) - - ports := ci.PortAllocator.Grab(1) - - // Try to remove a peer that's not there. - arg := structs.RaftPeerByAddressRequest{ - Address: raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", ports[0])), - } - arg.Region = s1.config.Region - var reply struct{} - err := msgpackrpc.CallWithCodec(codec, "Operator.RaftRemovePeerByAddress", &arg, &reply) - if err == nil || !strings.Contains(err.Error(), "not found in the Raft configuration") { - t.Fatalf("err: %v", err) - } - - // Add it manually to Raft. - { - future := s1.raft.AddPeer(arg.Address) - if err := future.Error(); err != nil { - t.Fatalf("err: %v", err) - } - } - - // Make sure it's there. - { - future := s1.raft.GetConfiguration() - if err := future.Error(); err != nil { - t.Fatalf("err: %v", err) - } - configuration := future.Configuration() - if len(configuration.Servers) != 2 { - t.Fatalf("bad: %v", configuration) - } - } - - // Remove it, now it should go through. - if err := msgpackrpc.CallWithCodec(codec, "Operator.RaftRemovePeerByAddress", &arg, &reply); err != nil { - t.Fatalf("err: %v", err) - } - - // Make sure it's not there. - { - future := s1.raft.GetConfiguration() - if err := future.Error(); err != nil { - t.Fatalf("err: %v", err) - } - configuration := future.Configuration() - if len(configuration.Servers) != 1 { - t.Fatalf("bad: %v", configuration) - } - } -} - -func TestOperator_RaftRemovePeerByAddress_ACL(t *testing.T) { - ci.Parallel(t) - - s1, root, cleanupS1 := TestACLServer(t, func(c *Config) { - c.RaftConfig.ProtocolVersion = raft.ProtocolVersion(2) - }) - - defer cleanupS1() - codec := rpcClient(t, s1) - testutil.WaitForLeader(t, s1.RPC) - assert := assert.New(t) - state := s1.fsm.State() - - // Create ACL token - invalidToken := mock.CreatePolicyAndToken(t, state, 1001, "test-invalid", mock.NodePolicy(acl.PolicyWrite)) - - ports := ci.PortAllocator.Grab(1) - - arg := structs.RaftPeerByAddressRequest{ - Address: raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", ports[0])), - } - arg.Region = s1.config.Region - - // Add peer manually to Raft. - { - future := s1.raft.AddPeer(arg.Address) - assert.Nil(future.Error()) - } - - var reply struct{} - - // Try with no token and expect permission denied - { - err := msgpackrpc.CallWithCodec(codec, "Operator.RaftRemovePeerByAddress", &arg, &reply) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) - } - - // Try with an invalid token and expect permission denied - { - arg.AuthToken = invalidToken.SecretID - err := msgpackrpc.CallWithCodec(codec, "Operator.RaftRemovePeerByAddress", &arg, &reply) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) - } - - // Try with a management token - { - arg.AuthToken = root.SecretID - err := msgpackrpc.CallWithCodec(codec, "Operator.RaftRemovePeerByAddress", &arg, &reply) - assert.Nil(err) - } -} - func TestOperator_RaftRemovePeerByID(t *testing.T) { ci.Parallel(t) - s1, cleanupS1 := TestServer(t, func(c *Config) { - c.RaftConfig.ProtocolVersion = 3 - }) + s1, root, cleanupS1 := TestACLServer(t, nil) defer cleanupS1() codec := rpcClient(t, s1) testutil.WaitForLeader(t, s1.RPC) + store := s1.fsm.State() + + var reply struct{} // Try to remove a peer that's not there. arg := structs.RaftPeerByIDRequest{ - ID: raft.ServerID("e35bde83-4e9c-434f-a6ef-453f44ee21ea"), + ID: raft.ServerID(uuid.Generate()), + WriteRequest: structs.WriteRequest{ + Region: s1.config.Region, AuthToken: root.SecretID}, } - arg.Region = s1.config.Region - var reply struct{} err := msgpackrpc.CallWithCodec(codec, "Operator.RaftRemovePeerByID", &arg, &reply) - if err == nil || !strings.Contains(err.Error(), "not found in the Raft configuration") { - t.Fatalf("err: %v", err) - } - - ports := ci.PortAllocator.Grab(1) - - // Add it manually to Raft. - { - future := s1.raft.AddVoter(arg.ID, raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", ports[0])), 0, 0) - if err := future.Error(); err != nil { - t.Fatalf("err: %v", err) - } - } - - // Make sure it's there. - { - future := s1.raft.GetConfiguration() - if err := future.Error(); err != nil { - t.Fatalf("err: %v", err) - } - configuration := future.Configuration() - if len(configuration.Servers) != 2 { - t.Fatalf("bad: %v", configuration) - } - } - - // Remove it, now it should go through. - if err := msgpackrpc.CallWithCodec(codec, "Operator.RaftRemovePeerByID", &arg, &reply); err != nil { - t.Fatalf("err: %v", err) - } - - // Make sure it's not there. - { - future := s1.raft.GetConfiguration() - if err := future.Error(); err != nil { - t.Fatalf("err: %v", err) - } - configuration := future.Configuration() - if len(configuration.Servers) != 1 { - t.Fatalf("bad: %v", configuration) - } - } -} - -func TestOperator_RaftRemovePeerByID_ACL(t *testing.T) { - ci.Parallel(t) - - s1, root, cleanupS1 := TestACLServer(t, func(c *Config) { - c.RaftConfig.ProtocolVersion = 3 - }) - defer cleanupS1() - codec := rpcClient(t, s1) - testutil.WaitForLeader(t, s1.RPC) - assert := assert.New(t) - state := s1.fsm.State() + must.ErrorContains(t, err, "not found in the Raft configuration") // Create ACL token - invalidToken := mock.CreatePolicyAndToken(t, state, 1001, "test-invalid", mock.NodePolicy(acl.PolicyWrite)) + invalidToken := mock.CreatePolicyAndToken(t, + store, 1001, "test-invalid", mock.NodePolicy(acl.PolicyWrite)) - arg := structs.RaftPeerByIDRequest{ + arg = structs.RaftPeerByIDRequest{ ID: raft.ServerID("e35bde83-4e9c-434f-a6ef-453f44ee21ea"), } arg.Region = s1.config.Region @@ -347,32 +179,49 @@ func TestOperator_RaftRemovePeerByID_ACL(t *testing.T) { // Add peer manually to Raft. { - future := s1.raft.AddVoter(arg.ID, raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", ports[0])), 0, 0) - assert.Nil(future.Error()) + future := s1.raft.AddVoter(arg.ID, + raft.ServerAddress(fmt.Sprintf("127.0.0.1:%d", ports[0])), 0, 0) + must.NoError(t, future.Error()) } - var reply struct{} + // Make sure it's there. + { + future := s1.raft.GetConfiguration() + err := future.Error() + must.NoError(t, err) + + configuration := future.Configuration() + must.Len(t, 2, configuration.Servers) + } // Try with no token and expect permission denied { err := msgpackrpc.CallWithCodec(codec, "Operator.RaftRemovePeerByID", &arg, &reply) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + must.EqError(t, err, structs.ErrPermissionDenied.Error()) } // Try with an invalid token and expect permission denied { arg.AuthToken = invalidToken.SecretID err := msgpackrpc.CallWithCodec(codec, "Operator.RaftRemovePeerByID", &arg, &reply) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + must.EqError(t, err, structs.ErrPermissionDenied.Error()) } // Try with a management token { arg.AuthToken = root.SecretID err := msgpackrpc.CallWithCodec(codec, "Operator.RaftRemovePeerByID", &arg, &reply) - assert.Nil(err) + must.NoError(t, err) + } + + // Make sure it's removed. + { + future := s1.raft.GetConfiguration() + err := future.Error() + must.NoError(t, err) + + configuration := future.Configuration() + must.Len(t, 1, configuration.Servers) } } diff --git a/website/content/api-docs/operator/raft.mdx b/website/content/api-docs/operator/raft.mdx index fb77f6d8e..7b06c6786 100644 --- a/website/content/api-docs/operator/raft.mdx +++ b/website/content/api-docs/operator/raft.mdx @@ -130,20 +130,10 @@ The table below shows this endpoint's support for ### Parameters -- `address` `(string: )` - Specifies the Raft **Address** of the - server to remove as provided in the output of `/v1/operator/raft/configuration` - API endpoint or the `nomad operator raft list-peers` command. - - `id` `(string: )` - Specifies the Raft **ID** of the server to remove as provided in the output of `/v1/operator/raft/configuration` API endpoint or the `nomad operator raft list-peers` command. - - - Either `address` or `id` must be provided, but not both. - - - ### Sample Request diff --git a/website/content/docs/commands/operator/raft/remove-peer.mdx b/website/content/docs/commands/operator/raft/remove-peer.mdx index 9c0121ff0..4345cdef5 100644 --- a/website/content/docs/commands/operator/raft/remove-peer.mdx +++ b/website/content/docs/commands/operator/raft/remove-peer.mdx @@ -7,7 +7,7 @@ description: | # `nomad operator raft remove-peer` command reference -Remove the Nomad server with given address from the Raft configuration. +Remove the Nomad server with peer ID from the Raft configuration. There are rare cases where a peer may be left behind in the Raft quorum even though the server is no longer present and known to the cluster. This command @@ -34,9 +34,6 @@ If ACLs are enabled, this command requires a management token. ## Remove Peer options -- `-peer-address`: Remove a Nomad server with given address from the Raft - configuration. The format is "IP:port" - - `-peer-id`: Remove a Nomad server with the given ID from the Raft configuration. The format is "id" diff --git a/website/content/docs/upgrade/upgrade-specific.mdx b/website/content/docs/upgrade/upgrade-specific.mdx index b4417641c..3a244a9da 100644 --- a/website/content/docs/upgrade/upgrade-specific.mdx +++ b/website/content/docs/upgrade/upgrade-specific.mdx @@ -12,6 +12,16 @@ upgrade. However, specific versions of Nomad may have more details provided for their upgrades as a result of new features or changed behavior. This page is used to document those details separately from the standard upgrade flow. +## Nomad 1.10.1 + +#### Remove Raft peer by address removed + +Nomad 1.4.0 removed support for Raft Protocol v2, and this removed the ability +to remove Raft peers by address instead of peer ID. Nomad 1.10.1 removes the +non-functional `-peer-address` option for the [`operator raft +peer-remove`](/nomad/docs/commands/operator/raft/remove-peer) command, and the +`address` parameter for the `DELETE /v1/operator/raft/peer` API. + ## Nomad 1.10.0 @include 'release-notes/v1-10/deprecate-variable-limits.mdx'