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

Revert "Read-replica instead of non-voter (#10875)" #10890

Merged
merged 1 commit into from
Feb 10, 2021
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
2 changes: 1 addition & 1 deletion api/sys_raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type RaftJoinRequest struct {
LeaderClientCert string `json:"leader_client_cert"`
LeaderClientKey string `json:"leader_client_key"`
Retry bool `json:"retry"`
ReadReplica bool `json:"read_replica"`
NonVoter bool `json:"non_voter"`
}

// RaftJoin adds the node from which this call is invoked from to the raft
Expand Down
23 changes: 2 additions & 21 deletions command/operator_raft_join.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,13 @@ var _ cli.CommandAutocomplete = (*OperatorRaftJoinCommand)(nil)

type OperatorRaftJoinCommand struct {
flagRetry bool
flagNonVoter bool
flagLeaderCACert string
flagLeaderClientCert string
flagLeaderClientKey string
flagAutoJoinScheme string
flagAutoJoinPort uint
flagReadReplica bool
*BaseCommand

// Deprecated flags
flagNonVoter bool
}

func (c *OperatorRaftJoinCommand) Synopsis() string {
Expand Down Expand Up @@ -116,13 +113,6 @@ func (c *OperatorRaftJoinCommand) Flags() *FlagSets {
Name: "non-voter",
Target: &c.flagNonVoter,
Default: false,
Usage: "DEPRECATED: Use -read-replica instead.",
})

f.BoolVar(&BoolVar{
Name: "read-replica",
Target: &c.flagReadReplica,
Default: false,
Usage: "(Enterprise-only) This flag is used to make the server not participate in the Raft quorum, and have it only receive the data replication stream. This can be used to add read scalability to a cluster in cases where a high volume of reads to servers are needed.",
})

Expand Down Expand Up @@ -158,15 +148,6 @@ func (c *OperatorRaftJoinCommand) Run(args []string) int {
return 1
}

switch {
case c.flagReadReplica:
// Prioritize -read-replica flag.
c.flagNonVoter = true
case c.flagNonVoter:
// If the deprecated -non-voter is used, update the -read-replica flag value.
c.flagReadReplica = true
}

leaderCACert, err := parseFlagFile(c.flagLeaderCACert)
if err != nil {
c.UI.Error(fmt.Sprintf("Failed to parse leader CA certificate: %s", err))
Expand Down Expand Up @@ -201,7 +182,7 @@ func (c *OperatorRaftJoinCommand) Run(args []string) int {
LeaderClientCert: leaderClientCert,
LeaderClientKey: leaderClientKey,
Retry: c.flagRetry,
ReadReplica: c.flagReadReplica,
NonVoter: c.flagNonVoter,
}

if strings.Contains(leaderInfo, "provider=") {
Expand Down
8 changes: 4 additions & 4 deletions http/sys_raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ func handleSysRaftJoinPost(core *vault.Core, w http.ResponseWriter, r *http.Requ
return
}

if req.ReadReplica && !readReplicasAllowed {
respondError(w, http.StatusBadRequest, errors.New("read-replica nodes not allowed"))
if req.NonVoter && !nonVotersAllowed {
respondError(w, http.StatusBadRequest, errors.New("non-voting nodes not allowed"))
return
}

Expand Down Expand Up @@ -83,7 +83,7 @@ func handleSysRaftJoinPost(core *vault.Core, w http.ResponseWriter, r *http.Requ
},
}

joined, err := core.JoinRaftCluster(context.Background(), leaderInfos, req.ReadReplica)
joined, err := core.JoinRaftCluster(context.Background(), leaderInfos, req.NonVoter)
if err != nil {
respondError(w, http.StatusInternalServerError, err)
return
Expand All @@ -109,5 +109,5 @@ type JoinRequest struct {
LeaderClientKey string `json:"leader_client_key"`
LeaderTLSServerName string `json:"leader_tls_servername"`
Retry bool `json:"retry"`
ReadReplica bool `json:"read_replica"`
NonVoter bool `json:"non_voter"`
}
2 changes: 1 addition & 1 deletion http/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var (

additionalRoutes = func(mux *http.ServeMux, core *vault.Core) {}

readReplicasAllowed = false
nonVotersAllowed = false
)

func rateLimitQuotaWrapping(handler http.Handler, core *vault.Core) http.Handler {
Expand Down
2 changes: 1 addition & 1 deletion physical/raft/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ func (b *RaftBackend) SetupCluster(ctx context.Context, opts SetupOpts) error {
// Non-voting servers are only allowed in enterprise. If Suffrage is disabled,
// error out to indicate that it isn't allowed.
for idx := range recoveryConfig.Servers {
if !readReplicasAllowed && recoveryConfig.Servers[idx].Suffrage == raft.Nonvoter {
if !nonVotersAllowed && recoveryConfig.Servers[idx].Suffrage == raft.Nonvoter {
return fmt.Errorf("raft recovery failed to parse configuration for node %q: setting `non_voter` is only supported in enterprise", recoveryConfig.Servers[idx].ID)
}
}
Expand Down
7 changes: 3 additions & 4 deletions physical/raft/raft_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@ import (
"errors"
)

const readReplicasAllowed = false
const nonVotersAllowed = false

// AddReadReplicaPeer adds a new server to the raft cluster which does not have
// voting rights but gets all the data replicated to it.
func (b *RaftBackend) AddReadReplicaPeer(ctx context.Context, peerID, clusterAddr string) error {
// AddPeer adds a new server to the raft cluster
func (b *RaftBackend) AddNonVotingPeer(ctx context.Context, peerID, clusterAddr string) error {
return errors.New("not implemented")
}
2 changes: 1 addition & 1 deletion vault/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ type raftInformation struct {
challenge *wrapping.EncryptedBlobInfo
leaderClient *api.Client
leaderBarrierConfig *SealConfig
readReplica bool
nonVoter bool
joinInProgress bool
}

Expand Down
16 changes: 3 additions & 13 deletions vault/logical_system_raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ func (b *SystemBackend) raftStoragePaths() []*framework.Path {
Type: framework.TypeString,
},
"non_voter": {
Type: framework.TypeBool,
Deprecated: true,
},
"read_replica": {
Type: framework.TypeBool,
},
},
Expand Down Expand Up @@ -260,13 +256,7 @@ func (b *SystemBackend) handleRaftBootstrapAnswerWrite() framework.OperationFunc
return logical.ErrorResponse("no cluster_addr provided"), logical.ErrInvalidRequest
}

// Prioritize read_replica parameter
readReplica := d.Get("read_replica").(bool)

// If the deprecated non_voter is used, consider that as well
if !readReplica && d.Get("non_voter").(bool) {
readReplica = true
}
nonVoter := d.Get("non_voter").(bool)

answer, err := base64.StdEncoding.DecodeString(answerRaw)
if err != nil {
Expand Down Expand Up @@ -296,9 +286,9 @@ func (b *SystemBackend) handleRaftBootstrapAnswerWrite() framework.OperationFunc
return nil, errors.New("could not decode raft TLS configuration")
}

switch readReplica {
switch nonVoter {
case true:
err = raftBackend.AddReadReplicaPeer(ctx, serverID, clusterAddr)
err = raftBackend.AddNonVotingPeer(ctx, serverID, clusterAddr)
default:
err = raftBackend.AddPeer(ctx, serverID, clusterAddr)
}
Expand Down
6 changes: 3 additions & 3 deletions vault/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ func (c *Core) InitiateRetryJoin(ctx context.Context) error {
return nil
}

func (c *Core) JoinRaftCluster(ctx context.Context, leaderInfos []*raft.LeaderJoinInfo, readReplica bool) (bool, error) {
func (c *Core) JoinRaftCluster(ctx context.Context, leaderInfos []*raft.LeaderJoinInfo, nonVoter bool) (bool, error) {
raftBackend := c.getRaftBackend()
if raftBackend == nil {
return false, errors.New("raft backend not in use")
Expand Down Expand Up @@ -881,7 +881,7 @@ func (c *Core) JoinRaftCluster(ctx context.Context, leaderInfos []*raft.LeaderJo
challenge: eBlob,
leaderClient: apiClient,
leaderBarrierConfig: &sealConfig,
readReplica: readReplica,
nonVoter: nonVoter,
}

// If we're using Shamir and using raft for both physical and HA, we
Expand Down Expand Up @@ -1077,7 +1077,7 @@ func (c *Core) joinRaftSendAnswer(ctx context.Context, sealAccess *seal.Access,
"answer": base64.StdEncoding.EncodeToString(plaintext),
"cluster_addr": clusterAddr,
"server_id": raftBackend.NodeID(),
"read_replica": raftInfo.readReplica,
"non_voter": raftInfo.nonVoter,
}); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion vendor/github.com/hashicorp/vault/api/sys_raft.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 0 additions & 13 deletions website/content/api-docs/system/storage/raft.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,6 @@ leader node.
- `leader_client_key` `(string: "")` - Client key used to communicate with
Raft's leader node.

- `auto_join` `(string: "")` - Defines any cloud auto-join metadata. If
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you mean to revert this part?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. I wanted to keep this a clean revert. I'll PR that separately.

supplied, Vault will attempt to automatically discover peers in addition to what
can be provided via 'leader_api_addr'.

- `auto_join_scheme` `(string: "https")` - URI scheme to be used for `auto_join`.

- `auto_join_port` `(int: 8200)` - Port to be used for `auto_join`.

- `-read-replica` `(bool: false) (enterprise)` - This flag is used to make the
server not participate in the Raft quorum, and have it only receive the data
replication stream. This can be used to add read scalability to a cluster in
cases where a high volume of reads to servers are needed. The default is false.

### Sample Payload

```json
Expand Down