Skip to content

Commit

Permalink
[FAB-3727] Orderer restart broken
Browse files Browse the repository at this point in the history
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 <jyellick@us.ibm.com>
  • Loading branch information
Jason Yellick committed May 9, 2017
1 parent f533ae6 commit 975dc82
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 15 deletions.
3 changes: 2 additions & 1 deletion common/configtx/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 9 additions & 3 deletions common/configtx/initializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions common/mocks/configtx/configtx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions orderer/multichain/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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")
}

Expand Down
20 changes: 19 additions & 1 deletion orderer/multichain/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down

0 comments on commit 975dc82

Please sign in to comment.