Merge pull request hashicorp#148 from hashicorp/f-remove-empty-recover

Adds a warning about RecoverCluster and removes ability to recover an empty configuration.

api.go

@@ -209,6 +209,13 @@ func BootstrapCluster(conf *Config, logs LogStore, stable StableStore,
 // insert any new entries, which could cause conflicts with other servers with
 // different state.
 //
+// WARNING! This operation implicitly commits all entries in the Raft log, so
+// in general this is an extremely unsafe operation. If you've lost your other
+// servers and are performing a manual recovery, then you've also lost the
+// commit information, so this is likely the best you can do, but you should be
+// aware that calling this can cause Raft log entries that were in the process
+// of being replicated but not yet be committed to be committed.
+//
 // Note the FSM passed here is used for the snapshot operations and will be
 // left in a state that should not be used by the application. Be sure to
 // discard this FSM and any associated state and provide a fresh one when
@@ -228,15 +235,9 @@ func RecoverCluster(conf *Config, fsm FSM, logs LogStore, stable StableStore,
 		return err
 	}
 
-	// Sanity check the Raft peer configuration. Note that it's ok to
-	// recover to an empty configuration if you want to keep the data
-	// but not allow a given server to participate any more (you'd have
-	// to recover again, or add it back into the existing quorum from
-	// another server to use this server).
-	if len(configuration.Servers) > 0 {
-		if err := checkConfiguration(configuration); err != nil {
-			return err
-		}
+	// Sanity check the Raft peer configuration.
+	if err := checkConfiguration(configuration); err != nil {
+		return err
 	}
 
 	// Refuse to recover if there's no existing state. This would be safe to

raft_test.go

@@ -763,65 +763,6 @@ func TestRaft_RecoverCluster_NoState(t *testing.T) {
 	}
 }
 
-func TestRaft_RecoverCluster_EmptyConfiguration(t *testing.T) {
-	conf := inmemConfig(t)
-	conf.TrailingLogs = 10
-	c := MakeCluster(3, t, conf)
-	defer c.Close()
-
-	// Perform some commits.
-	leader := c.Leader()
-	for i := 0; i < 100; i++ {
-		future := leader.Apply([]byte(fmt.Sprintf("test%d", i)), 0)
-		if err := future.Error(); err != nil {
-			c.FailNowf("[ERR] apply err: %v", err)
-		}
-	}
-
-	// Remove the first Raft server from the cluster.
-	r := c.rafts[0]
-	future := leader.RemoveServer(r.localID, 0, 0)
-	if err := future.Error(); err != nil {
-		c.FailNowf("[ERR] remove err: %v", err)
-	}
-
-	// Shut down the first Raft server.
-	if err := r.Shutdown().Error(); err != nil {
-		c.FailNowf("[ERR] shutdown err: %v", err)
-	}
-
-	// Run recovery with an empty configuration.
-	if err := RecoverCluster(&r.conf, &MockFSM{}, r.logs, r.stable,
-		r.snapshots, r.trans, Configuration{}); err != nil {
-		c.FailNowf("[ERR] recover err: %v", err)
-	}
-
-	// Fire up the recovered Raft server. We have to patch
-	// up the cluster state manually since this is an unusual
-	// operation.
-	_, trans := NewInmemTransport(r.localAddr)
-	r2, err := NewRaft(&r.conf, &MockFSM{}, r.logs, r.stable, r.snapshots, trans)
-	if err != nil {
-		c.FailNowf("[ERR] new raft err: %v", err)
-	}
-	c.rafts[0] = r2
-	c.trans[0] = r2.trans.(*InmemTransport)
-	c.fsms[0] = r2.fsm.(*MockFSM)
-
-	// Fire it up and join it back in.
-	c.FullyConnect()
-	time.Sleep(c.propagateTimeout)
-	future = c.Leader().AddVoter(r2.localID, r2.localAddr, 0, 0)
-	if err := future.Error(); err != nil {
-		c.FailNowf("[ERR] add server err: %v", err)
-	}
-
-	// Let things settle and make sure we recovered.
-	c.EnsureLeader(t, c.Leader().localAddr)
-	c.EnsureSame(t)
-	c.EnsureSamePeers(t)
-}
-
 func TestRaft_RecoverCluster(t *testing.T) {
 	// Run with different number of applies which will cover no snapshot and
 	// snapshot + log scenarios. By sweeping through the trailing logs value