Do not require reboot when re-adding consenter

FAB-13552

Change-Id: I5d77dd613db3ad4d1296f2e8b960859a785f04ec
Signed-off-by: Jay Guo <[email protected]>
Signed-off-by: Jason Yellick <[email protected]>

integration/e2e/etcdraft_reconfig_test.go

@@ -687,10 +687,10 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 			firstEvictedNode := findLeader(ordererRunners) - 1
 
 			By("Removing the leader from 3-node channel")
-			serverCertBytes, err := ioutil.ReadFile(filepath.Join(network.OrdererLocalTLSDir(orderers[firstEvictedNode]), "server.crt"))
+			server1CertBytes, err := ioutil.ReadFile(filepath.Join(network.OrdererLocalTLSDir(orderers[firstEvictedNode]), "server.crt"))
 			Expect(err).To(Not(HaveOccurred()))
 
-			nwo.RemoveConsenter(network, peer, network.Orderers[(firstEvictedNode+1)%3], "systemchannel", serverCertBytes)
+			nwo.RemoveConsenter(network, peer, network.Orderers[(firstEvictedNode+1)%3], "systemchannel", server1CertBytes)
 			fmt.Fprintln(GinkgoWriter, "Ensuring the other orderers detect the eviction of the node on channel", "systemchannel")
 			Eventually(ordererRunners[(firstEvictedNode+1)%3].Err(), network.EventuallyTimeout, time.Second).Should(gbytes.Say("Deactivated node"))
 			Eventually(ordererRunners[(firstEvictedNode+2)%3].Err(), network.EventuallyTimeout, time.Second).Should(gbytes.Say("Deactivated node"))
@@ -722,10 +722,10 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 			}
 
 			By("Removing the leader from 2-node channel")
-			serverCertBytes, err = ioutil.ReadFile(filepath.Join(network.OrdererLocalTLSDir(orderers[secondEvictedNode]), "server.crt"))
+			server2CertBytes, err := ioutil.ReadFile(filepath.Join(network.OrdererLocalTLSDir(orderers[secondEvictedNode]), "server.crt"))
 			Expect(err).To(Not(HaveOccurred()))
 
-			nwo.RemoveConsenter(network, peer, orderers[surviver], "systemchannel", serverCertBytes)
+			nwo.RemoveConsenter(network, peer, orderers[surviver], "systemchannel", server2CertBytes)
 			fmt.Fprintln(GinkgoWriter, "Ensuring the other orderer detect the eviction of the node on channel", "systemchannel")
 			Eventually(ordererRunners[secondEvictedNode].Err(), network.EventuallyTimeout, time.Second).Should(gbytes.Say(stopMSg))
 
@@ -734,6 +734,24 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 
 			By("Asserting the only remaining node elects itself")
 			findLeader([]*ginkgomon.Runner{ordererRunners[surviver]})
+
+			By("Re-adding first evicted orderer")
+			nwo.AddConsenter(network, peer, network.Orderers[surviver], "systemchannel", etcdraft.Consenter{
+				Host:          "127.0.0.1",
+				Port:          uint32(network.OrdererPort(orderers[firstEvictedNode], nwo.ClusterPort)),
+				ClientTlsCert: server1CertBytes,
+				ServerTlsCert: server1CertBytes,
+			})
+
+			By("Ensuring re-added orderer starts serving system channel")
+			assertBlockReception(map[string]int{
+				"systemchannel": 3,
+			}, []*nwo.Orderer{orderers[firstEvictedNode]}, peer, network)
+
+			env := CreateBroadcastEnvelope(network, orderers[secondEvictedNode], network.SystemChannel.Name, []byte("foo"))
+			resp, err := Broadcast(network, orderers[surviver], env)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(resp.Status).To(Equal(common.Status_SUCCESS))
 		})
 
 		It("notices it even if it is down at the time of its eviction", func() {

orderer/common/cluster/replication.go

@@ -132,6 +132,15 @@ func (r *Replicator) ReplicateChains() []string {
 			if err != nil {
 				r.Logger.Panicf("Failed to create a ledger for channel %s: %v", channel.ChannelName, err)
 			}
+
+			if channel.GenesisBlock == nil {
+				if ledger.Height() == 0 {
+					r.Logger.Panicf("Expecting channel %s to at least contain genesis block, but it doesn't", channel.ChannelName)
+				}
+
+				continue
+			}
+
 			gb, err := ChannelCreationBlockToGenesisBlock(channel.GenesisBlock)
 			if err != nil {
 				r.Logger.Panicf("Failed converting channel creation block for channel %s to genesis block: %v",

orderer/common/server/onboarding.go

@@ -183,11 +183,9 @@ type chainCreation struct {
 // TrackChain tracks a chain with the given name, and calls the given callback
 // when this chain should be activated.
 func (dc *inactiveChainReplicator) TrackChain(chain string, genesisBlock *common.Block, createChainCallback etcdraft.CreateChainCallback) {
-	if genesisBlock == nil {
-		dc.logger.Panicf("Called with a nil genesis block")
-	}
 	dc.lock.Lock()
 	defer dc.lock.Unlock()
+
 	dc.logger.Infof("Adding %s to the set of chains to track", chain)
 	dc.chains2CreationCallbacks[chain] = chainCreation{
 		genesisBlock: genesisBlock,

orderer/common/server/onboarding_test.go

@@ -846,18 +846,12 @@ func TestInactiveChainReplicatorChannels(t *testing.T) {
 		chains2CreationCallbacks: make(map[string]chainCreation),
 	}
 	icr.TrackChain("foo", &common.Block{}, func() {})
+	assert.Contains(t, icr.Channels(), cluster.ChannelGenesisBlock{ChannelName: "foo", GenesisBlock: &common.Block{}})
 
-	assert.Equal(t, []cluster.ChannelGenesisBlock{{ChannelName: "foo", GenesisBlock: &common.Block{}}}, icr.Channels())
-	icr.Close()
-}
+	icr.TrackChain("bar", nil, func() {})
+	assert.Contains(t, icr.Channels(), cluster.ChannelGenesisBlock{ChannelName: "bar", GenesisBlock: nil})
 
-func TestTrackChainNilGenesisBlock(t *testing.T) {
-	icr := &inactiveChainReplicator{
-		logger: flogging.MustGetLogger("test"),
-	}
-	assert.PanicsWithValue(t, "Called with a nil genesis block", func() {
-		icr.TrackChain("foo", nil, func() {})
-	})
+	icr.Close()
 }
 
 func TestLedgerFactory(t *testing.T) {

orderer/consensus/etcdraft/chain.go

@@ -191,6 +191,8 @@ type Chain struct {
 	logger  *flogging.FabricLogger
 
 	periodicChecker *PeriodicCheck
+
+	haltCallback func()
 }
 
 // NewChain constructs a chain object.
@@ -200,6 +202,7 @@ func NewChain(
 	conf Configurator,
 	rpc RPC,
 	f CreateBlockPuller,
+	haltCallback func(),
 	observeC chan<- raft.SoftState) (*Chain, error) {
 
 	lg := opts.Logger.With("channel", support.ChainID(), "node", opts.RaftID)
@@ -258,6 +261,7 @@ func NewChain(
 		confState:        cc,
 		createPuller:     f,
 		clock:            opts.Clock,
+		haltCallback:     haltCallback,
 		Metrics: &Metrics{
 			ClusterSize:             opts.Metrics.ClusterSize.With("channel", support.ChainID()),
 			IsLeader:                opts.Metrics.IsLeader.With("channel", support.ChainID()),
@@ -484,6 +488,10 @@ func (c *Chain) Halt() {
 		return
 	}
 	<-c.doneC
+
+	if c.haltCallback != nil {
+		c.haltCallback()
+	}
 }
 
 func (c *Chain) isRunning() error {

orderer/consensus/etcdraft/chain_test.go

@@ -175,7 +175,7 @@ var _ = Describe("Chain", func() {
 		}
 
 		JustBeforeEach(func() {
-			chain, err = etcdraft.NewChain(support, opts, configurator, nil, noOpBlockPuller, observeC)
+			chain, err = etcdraft.NewChain(support, opts, configurator, nil, noOpBlockPuller, nil, observeC)
 			Expect(err).NotTo(HaveOccurred())
 
 			chain.Start()
@@ -850,7 +850,7 @@ var _ = Describe("Chain", func() {
 								os.Chmod(path.Join(walDir, f.Name()), 0300)
 							}
 
-							c, err := etcdraft.NewChain(support, opts, configurator, nil, noOpBlockPuller, observeC)
+							c, err := etcdraft.NewChain(support, opts, configurator, nil, noOpBlockPuller, nil, observeC)
 							Expect(c).To(BeNil())
 							Expect(err).To(MatchError(ContainSubstring("permission denied")))
 						})
@@ -1266,6 +1266,7 @@ var _ = Describe("Chain", func() {
 							configurator,
 							nil,
 							nil,
+							nil,
 							observeC)
 						Expect(chain).NotTo(BeNil())
 						Expect(err).NotTo(HaveOccurred())
@@ -1298,6 +1299,7 @@ var _ = Describe("Chain", func() {
 							nil,
 							nil,
 							noOpBlockPuller,
+							nil,
 							nil)
 						Expect(chain).NotTo(BeNil())
 						Expect(err).ToNot(HaveOccurred())
@@ -1326,6 +1328,7 @@ var _ = Describe("Chain", func() {
 							nil,
 							nil,
 							noOpBlockPuller,
+							nil,
 							nil)
 						Expect(chain).To(BeNil())
 						Expect(err).To(MatchError(ContainSubstring("failed to initialize WAL: mkdir")))
@@ -3610,6 +3613,7 @@ func (c *chain) init() {
 		c.configurator,
 		c.rpc,
 		func() (etcdraft.BlockPuller, error) { return c.puller, nil },
+		nil,
 		c.observe,
 	)
 	Expect(err).NotTo(HaveOccurred())

orderer/consensus/etcdraft/consenter.go

@@ -222,6 +222,9 @@ func (c *Consenter) HandleChain(support consensus.ConsenterSupport, metadata *co
 		c.Communication,
 		rpc,
 		func() (BlockPuller, error) { return newBlockPuller(support, c.Dialer, c.OrdererConfig.General.Cluster) },
+		func() {
+			c.InactiveChainRegistry.TrackChain(support.ChainID(), nil, func() { c.CreateChain(support.ChainID()) })
+		},
 		nil,
 	)
 }