From 975dc824e2874f24490851cbdd2461ca7d185263 Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Mon, 8 May 2017 17:25:41 -0400 Subject: [PATCH] [FAB-3727] Orderer restart broken There was a programming error introduced during the consortium refactoring which caused an interface return value to be checked for nil, which returns false even when the underlying value is nil because the dynamic type is a pointer kind. This error caused the orderer to recognize all channels as the system channel, and would cause a fatal error at startup. This changeset modifies the config interface to return a boolean along with the interface value, indicating whether the interface value was populated. Change-Id: Ia9bb89b765c7e8f35742895d2ac1dd2396d03908 Signed-off-by: Jason Yellick --- common/configtx/api/api.go | 3 ++- common/configtx/initializer.go | 12 +++++++++--- common/mocks/configtx/configtx.go | 4 ++-- orderer/multichain/manager.go | 16 ++++++++-------- orderer/multichain/manager_test.go | 20 +++++++++++++++++++- 5 files changed, 40 insertions(+), 15 deletions(-) diff --git a/common/configtx/api/api.go b/common/configtx/api/api.go index b8a49e82dc5..bdbbe94c7e5 100644 --- a/common/configtx/api/api.go +++ b/common/configtx/api/api.go @@ -62,7 +62,8 @@ type Resources interface { OrdererConfig() config.Orderer // ConsortiumsConfig() returns the config.Consortiums for the channel - ConsortiumsConfig() config.Consortiums + // and whether the consortiums config exists + ConsortiumsConfig() (config.Consortiums, bool) // ApplicationConfig returns the configtxapplication.SharedConfig for the channel ApplicationConfig() config.Application diff --git a/common/configtx/initializer.go b/common/configtx/initializer.go index 4ebdb8477ed..d2fea9ab7e4 100644 --- a/common/configtx/initializer.go +++ b/common/configtx/initializer.go @@ -56,9 +56,15 @@ func (r *resources) ApplicationConfig() config.Application { return r.configRoot.Application() } -// ConsortiumsConfig returns the api.ConsortiumsConfig for the chain -func (r *resources) ConsortiumsConfig() config.Consortiums { - return r.configRoot.Consortiums() +// ConsortiumsConfig returns the api.ConsortiumsConfig for the chain and whether or not +// this channel contains consortiums config +func (r *resources) ConsortiumsConfig() (config.Consortiums, bool) { + result := r.configRoot.Consortiums() + if result == nil { + return nil, false + } + + return result, true } // MSPManager returns the msp.MSPManager for the chain diff --git a/common/mocks/configtx/configtx.go b/common/mocks/configtx/configtx.go index 3dbfaaa655b..2d1aa53f024 100644 --- a/common/mocks/configtx/configtx.go +++ b/common/mocks/configtx/configtx.go @@ -66,8 +66,8 @@ func (r *Resources) ApplicationConfig() config.Application { return r.ApplicationConfigVal } -func (r *Resources) ConsortiumsConfig() config.Consortiums { - return r.ConsortiumsConfigVal +func (r *Resources) ConsortiumsConfig() (config.Consortiums, bool) { + return r.ConsortiumsConfigVal, r.ConsortiumsConfigVal != nil } // Returns the MSPManagerVal diff --git a/orderer/multichain/manager.go b/orderer/multichain/manager.go index c1db0bec7f4..135faac7bd7 100644 --- a/orderer/multichain/manager.go +++ b/orderer/multichain/manager.go @@ -100,18 +100,18 @@ func NewManagerImpl(ledgerFactory ledger.Factory, consenters map[string]Consente for _, chainID := range existingChains { rl, err := ledgerFactory.GetOrCreate(chainID) if err != nil { - logger.Fatalf("Ledger factory reported chainID %s but could not retrieve it: %s", chainID, err) + logger.Panicf("Ledger factory reported chainID %s but could not retrieve it: %s", chainID, err) } configTx := getConfigTx(rl) if configTx == nil { - logger.Fatalf("Could not find config transaction for chain %s", chainID) + logger.Panicf("Could not find config transaction for chain %s", chainID) } ledgerResources := ml.newLedgerResources(configTx) chainID := ledgerResources.ChainID() - if ledgerResources.ConsortiumsConfig() != nil { + if _, ok := ledgerResources.ConsortiumsConfig(); ok { if ml.systemChannelID != "" { - logger.Fatalf("There appear to be two system chains %s and %s", ml.systemChannelID, chainID) + logger.Panicf("There appear to be two system chains %s and %s", ml.systemChannelID, chainID) } chain := newChainSupport(createSystemChainFilters(ml, ledgerResources), ledgerResources, @@ -156,14 +156,14 @@ func (ml *multiLedger) newLedgerResources(configTx *cb.Envelope) *ledgerResource initializer := configtx.NewInitializer() configManager, err := configtx.NewManagerImpl(configTx, initializer, nil) if err != nil { - logger.Fatalf("Error creating configtx manager and handlers: %s", err) + logger.Panicf("Error creating configtx manager and handlers: %s", err) } chainID := configManager.ChainID() ledger, err := ml.ledgerFactory.GetOrCreate(chainID) if err != nil { - logger.Fatalf("Error getting ledger for %s", chainID) + logger.Panicf("Error getting ledger for %s", chainID) } return &ledgerResources{ @@ -237,8 +237,8 @@ func (ml *multiLedger) NewChannelConfig(envConfigUpdate *cb.Envelope) (configtxa } applicationGroup := cb.NewConfigGroup() - consortiumsConfig := ml.systemChannel.ConsortiumsConfig() - if consortiumsConfig == nil { + consortiumsConfig, ok := ml.systemChannel.ConsortiumsConfig() + if !ok { return nil, fmt.Errorf("The ordering system channel does not appear to support creating channels") } diff --git a/orderer/multichain/manager_test.go b/orderer/multichain/manager_test.go index 34a5f473c1e..135994e33d6 100644 --- a/orderer/multichain/manager_test.go +++ b/orderer/multichain/manager_test.go @@ -133,7 +133,7 @@ func TestGetConfigTxFailure(t *testing.T) { } -// This test essentially brings the entire system up and is ultimately what main.go will replicate +// This test checks to make sure the orderer refuses to come up if it cannot find a system channel func TestNoSystemChain(t *testing.T) { defer func() { if recover() == nil { @@ -149,6 +149,24 @@ func TestNoSystemChain(t *testing.T) { NewManagerImpl(lf, consenters, mockCrypto()) } +// This test checks to make sure that the orderer refuses to come up if there are multiple system channels +func TestMultiSystemChannel(t *testing.T) { + lf := ramledger.New(10) + + for _, id := range []string{"foo", "bar"} { + rl, err := lf.GetOrCreate(id) + assert.NoError(t, err) + + err = rl.Append(provisional.New(conf).GenesisBlockForChannel(id)) + assert.NoError(t, err) + } + + consenters := make(map[string]Consenter) + consenters[conf.Orderer.OrdererType] = &mockConsenter{} + + assert.Panics(t, func() { NewManagerImpl(lf, consenters, mockCrypto()) }, "Two system channels should have caused panic") +} + // This test essentially brings the entire system up and is ultimately what main.go will replicate func TestManagerImpl(t *testing.T) { lf, rl := NewRAMLedgerAndFactory(10)