[FAB-10523] Fix peer command

This CR does two main things:
1) Patch error msg when core.yaml is not found
2) Restore the use of version and help commands in case of config error.

The version of Viper currently used gives a confusing error message
when the config file is not found:

$ ./peer version
2018-06-02 21:00:42.397 EDT [main] main -> ERRO 001 Fatal error when
 initializing core config : error when reading core config file:
 Unsupported Config Type ""

With this update the error clearly spells out what the problem is:

$ .build/bin/peer node start
2018-07-17 09:17:54.148 CEST [main] main -> ERRO 001 Fatal error when
 initializing core config : Could not find config file. Please make sure
 that FABRIC_CFG_PATH or --configPath is set to a path which contains
 core.yaml

In addition, if really only the version is requeted this works even if
the config file isn't found. Same is true with help.

$ ./peer version
peer:
 Version: 1.3.0
 Commit SHA: 27cda7a1a
 Go version: go1.10.3
 OS/Arch: darwin/amd64
 Experimental features: true
 Chaincode:
  Base Image Version: 0.4.10
  Base Docker Namespace: hyperledger
  Base Docker Label: org.hyperledger.fabric
  Docker Namespace: hyperledger

Patch-set 2: limit change to merely what's needed to address the problem
Patch-set 3: fix error msg displayed by Fabric when config file is not found,
limit init config to commands that need it,
plus minor fix on format of crypto config not found error msg.
Patch-set 4: fix unit test depending on modified error msg.
Patch-set 5: some clean up to fix a few more unit tests
Patch-set 6: more clean up

Change-Id: I7c84c121c1493c48ea92c83b62ecd99c887ca316
Signed-off-by: Arnaud J Le Hors <lehors@us.ibm.com>

peer/chaincode/chaincode.go

@@ -87,10 +87,13 @@ var (
 )
 
 var chaincodeCmd = &cobra.Command{
-	Use:              chainFuncName,
-	Short:            fmt.Sprint(chainCmdDes),
-	Long:             fmt.Sprint(chainCmdDes),
-	PersistentPreRun: common.SetOrdererEnv,
+	Use:   chainFuncName,
+	Short: fmt.Sprint(chainCmdDes),
+	Long:  fmt.Sprint(chainCmdDes),
+	PersistentPreRun: func(cmd *cobra.Command, args []string) {
+		common.InitCmd(cmd, args)
+		common.SetOrdererEnv(cmd, args)
+	},
 }
 
 var flags *pflag.FlagSet

peer/chaincode/flags_test.go

@@ -9,6 +9,7 @@ package chaincode
 import (
 	"testing"
 
+	"github.com/hyperledger/fabric/peer/common"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
@@ -34,7 +35,11 @@ func TestOrdererFlags(t *testing.T) {
 			assert.Equal(t, sn, viper.GetString("orderer.tls.serverhostoverride"))
 			assert.Equal(t, true, viper.GetBool("orderer.tls.enabled"))
 			assert.Equal(t, true, viper.GetBool("orderer.tls.clientAuthRequired"))
-		}}
+		},
+		PersistentPreRun: func(cmd *cobra.Command, args []string) {
+			common.SetOrdererEnv(cmd, args)
+		},
+	}
 
 	runCmd := Cmd(nil)
 

peer/channel/channel.go

@@ -91,10 +91,13 @@ func attachFlags(cmd *cobra.Command, names []string) {
 }
 
 var channelCmd = &cobra.Command{
-	Use:              "channel",
-	Short:            "Operate a channel: create|fetch|join|list|update|signconfigtx|getinfo.",
-	Long:             "Operate a channel: create|fetch|join|list|update|signconfigtx|getinfo.",
-	PersistentPreRun: common.SetOrdererEnv,
+	Use:   "channel",
+	Short: "Operate a channel: create|fetch|join|list|update|signconfigtx|getinfo.",
+	Long:  "Operate a channel: create|fetch|join|list|update|signconfigtx|getinfo.",
+	PersistentPreRun: func(cmd *cobra.Command, args []string) {
+		common.InitCmd(cmd, args)
+		common.SetOrdererEnv(cmd, args)
+	},
 }
 
 type BroadcastClientFactory func() (common.BroadcastClient, error)

peer/channel/flags_test.go

@@ -9,6 +9,7 @@ package channel
 import (
 	"testing"
 
+	"github.com/hyperledger/fabric/peer/common"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
@@ -35,7 +36,11 @@ func TestOrdererFlags(t *testing.T) {
 			assert.Equal(t, sn, viper.GetString("orderer.tls.serverhostoverride"))
 			assert.Equal(t, true, viper.GetBool("orderer.tls.enabled"))
 			assert.Equal(t, true, viper.GetBool("orderer.tls.clientAuthRequired"))
-		}}
+		},
+		PersistentPreRun: func(cmd *cobra.Command, args []string) {
+			common.SetOrdererEnv(cmd, args)
+		},
+	}
 
 	runCmd := Cmd(nil)
 

peer/clilogging/logging.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"github.com/hyperledger/fabric/common/flogging"
+	"github.com/hyperledger/fabric/peer/common"
 
 	"github.com/spf13/cobra"
 )
@@ -41,7 +42,8 @@ func Cmd(cf *LoggingCmdFactory) *cobra.Command {
 }
 
 var loggingCmd = &cobra.Command{
-	Use:   loggingFuncName,
-	Short: fmt.Sprint(loggingCmdDes),
-	Long:  fmt.Sprint(loggingCmdDes),
+	Use:              loggingFuncName,
+	Short:            fmt.Sprint(loggingCmdDes),
+	Long:             fmt.Sprint(loggingCmdDes),
+	PersistentPreRun: common.InitCmd,
 }

peer/common/common.go

@@ -9,8 +9,10 @@ package common
 import (
 	"crypto/tls"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
+	"runtime"
 	"strings"
 	"time"
 
@@ -29,12 +31,14 @@ import (
 	putils "github.com/hyperledger/fabric/protos/utils"
 	"github.com/op/go-logging"
 	"github.com/pkg/errors"
+	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"golang.org/x/net/context"
 )
 
 // UndefinedParamValue defines what undefined parameters in the command line will initialise to
 const UndefinedParamValue = ""
+const CmdRoot = "core"
 
 var (
 	defaultConnTimeout = 3 * time.Second
@@ -71,6 +75,9 @@ var (
 
 	// GetCertificateFnc is a function that returns the client TLS certificate
 	GetCertificateFnc func() (tls.Certificate, error)
+
+	mainLogger *logging.Logger
+	logOutput  io.Writer
 )
 
 type commonClient struct {
@@ -87,18 +94,29 @@ func init() {
 	GetDeliverClientFnc = GetDeliverClient
 	GetPeerDeliverClientFnc = GetPeerDeliverClient
 	GetCertificateFnc = GetCertificate
+	mainLogger = flogging.MustGetLogger("main")
+	logOutput = os.Stderr
 }
 
 // InitConfig initializes viper config
 func InitConfig(cmdRoot string) error {
+
 	err := config.InitViper(nil, cmdRoot)
 	if err != nil {
 		return err
 	}
 
 	err = viper.ReadInConfig() // Find and read the config file
 	if err != nil {            // Handle errors reading the config file
-		return errors.WithMessage(err, fmt.Sprintf("error when reading %s config file", cmdRoot))
+		// The version of Viper we use claims the config type isn't supported when in fact the file hasn't been found
+		// Display a more helpful message to avoid confusing the user.
+		if strings.Contains(fmt.Sprint(err), "Unsupported Config Type") {
+			return errors.New(fmt.Sprintf("Could not find config file. "+
+				"Please make sure that FABRIC_CFG_PATH or --configPath is set to a path "+
+				"which contains %s.yaml", cmdRoot))
+		} else {
+			return errors.WithMessage(err, fmt.Sprintf("error when reading %s config file", cmdRoot))
+		}
 	}
 
 	return nil
@@ -111,7 +129,7 @@ func InitCrypto(mspMgrConfigDir, localMSPID, localMSPType string) error {
 	fi, err := os.Stat(mspMgrConfigDir)
 	if os.IsNotExist(err) || !fi.IsDir() {
 		// No need to try to load MSP from folder which is not available
-		return errors.Errorf("cannot init crypto, missing %s folder", mspMgrConfigDir)
+		return errors.Errorf("cannot init crypto, folder \"%s\" does not exist", mspMgrConfigDir)
 	}
 	// Check whether localMSPID exists
 	if localMSPID == "" {
@@ -277,3 +295,42 @@ func configFromEnv(prefix string) (address, override string, clientConfig comm.C
 	clientConfig.SecOpts = secOpts
 	return
 }
+
+func InitCmd(cmd *cobra.Command, args []string) {
+
+	err := InitConfig(CmdRoot)
+	if err != nil { // Handle errors reading the config file
+		mainLogger.Errorf("Fatal error when initializing %s config : %s", CmdRoot, err)
+		os.Exit(1)
+	}
+
+	// setup system-wide logging backend based on settings from core.yaml
+	flogging.InitBackend(flogging.SetFormat(viper.GetString("logging.format")), logOutput)
+
+	// check for --logging-level pflag first, which should override all other
+	// log settings. if --logging-level is not set, use CORE_LOGGING_LEVEL
+	// (environment variable takes priority; otherwise, the value set in
+	// core.yaml)
+	var loggingSpec string
+	if viper.GetString("logging_level") != "" {
+		loggingSpec = viper.GetString("logging_level")
+	} else {
+		loggingSpec = viper.GetString("logging.level")
+	}
+	flogging.InitFromSpec(loggingSpec)
+
+	// Init the MSP
+	var mspMgrConfigDir = config.GetPath("peer.mspConfigPath")
+	var mspID = viper.GetString("peer.localMspId")
+	var mspType = viper.GetString("peer.localMspType")
+	if mspType == "" {
+		mspType = msp.ProviderTypeToString(msp.FABRIC)
+	}
+	err = InitCrypto(mspMgrConfigDir, mspID, mspType)
+	if err != nil { // Handle errors reading the config file
+		mainLogger.Errorf("Cannot run peer because %s", err.Error())
+		os.Exit(1)
+	}
+
+	runtime.GOMAXPROCS(viper.GetInt("peer.gomaxprocs"))
+}

peer/common/common_test.go

@@ -61,7 +61,7 @@ func TestInitCryptoMissingDir(t *testing.T) {
 	dir := os.TempDir() + "/" + util.GenerateUUID()
 	err := common.InitCrypto(dir, "SampleOrg", msp.ProviderTypeToString(msp.FABRIC))
 	assert.Error(t, err, "Should be able to initialize crypto with non-existing directory")
-	assert.Contains(t, err.Error(), fmt.Sprintf("missing %s folder", dir))
+	assert.Contains(t, err.Error(), fmt.Sprintf("folder \"%s\" does not exist", dir))
 }
 
 func TestInitCrypto(t *testing.T) {

peer/main.go

@@ -9,12 +9,8 @@ package main
 import (
 	_ "net/http/pprof"
 	"os"
-	"runtime"
 	"strings"
 
-	"github.com/hyperledger/fabric/common/flogging"
-	"github.com/hyperledger/fabric/core/config"
-	"github.com/hyperledger/fabric/msp"
 	"github.com/hyperledger/fabric/peer/chaincode"
 	"github.com/hyperledger/fabric/peer/channel"
 	"github.com/hyperledger/fabric/peer/clilogging"
@@ -25,20 +21,15 @@ import (
 	"github.com/spf13/viper"
 )
 
-var logger = flogging.MustGetLogger("main")
-var logOutput = os.Stderr
-
-// Constants go here.
-const cmdRoot = "core"
-
 // The main command describes the service and
 // defaults to printing the help message.
 var mainCmd = &cobra.Command{
 	Use: "peer"}
 
 func main() {
+
 	// For environment variables.
-	viper.SetEnvPrefix(cmdRoot)
+	viper.SetEnvPrefix(common.CmdRoot)
 	viper.AutomaticEnv()
 	replacer := strings.NewReplacer(".", "_")
 	viper.SetEnvKeyReplacer(replacer)
@@ -56,42 +47,6 @@ func main() {
 	mainCmd.AddCommand(clilogging.Cmd(nil))
 	mainCmd.AddCommand(channel.Cmd(nil))
 
-	err := common.InitConfig(cmdRoot)
-	if err != nil { // Handle errors reading the config file
-		logger.Errorf("Fatal error when initializing %s config : %s", cmdRoot, err)
-		os.Exit(1)
-	}
-
-	// setup system-wide logging backend based on settings from core.yaml
-	flogging.InitBackend(flogging.SetFormat(viper.GetString("logging.format")), logOutput)
-
-	// check for --logging-level pflag first, which should override all other
-	// log settings. if --logging-level is not set, use CORE_LOGGING_LEVEL
-	// (environment variable takes priority; otherwise, the value set in
-	// core.yaml)
-	var loggingSpec string
-	if viper.GetString("logging_level") != "" {
-		loggingSpec = viper.GetString("logging_level")
-	} else {
-		loggingSpec = viper.GetString("logging.level")
-	}
-	flogging.InitFromSpec(loggingSpec)
-
-	// Init the MSP
-	var mspMgrConfigDir = config.GetPath("peer.mspConfigPath")
-	var mspID = viper.GetString("peer.localMspId")
-	var mspType = viper.GetString("peer.localMspType")
-	if mspType == "" {
-		mspType = msp.ProviderTypeToString(msp.FABRIC)
-	}
-	err = common.InitCrypto(mspMgrConfigDir, mspID, mspType)
-	if err != nil { // Handle errors reading the config file
-		logger.Errorf("Cannot run peer because %s", err.Error())
-		os.Exit(1)
-	}
-
-	runtime.GOMAXPROCS(viper.GetInt("peer.gomaxprocs"))
-
 	// On failure Cobra prints the usage message and error string, so we only
 	// need to exit with a non-0 status
 	if mainCmd.Execute() != nil {

peer/node/node.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"github.com/hyperledger/fabric/common/flogging"
+	"github.com/hyperledger/fabric/peer/common"
 	"github.com/spf13/cobra"
 )
 
@@ -39,7 +40,8 @@ func Cmd() *cobra.Command {
 }
 
 var nodeCmd = &cobra.Command{
-	Use:   nodeFuncName,
-	Short: fmt.Sprint(nodeCmdDes),
-	Long:  fmt.Sprint(nodeCmdDes),
+	Use:              nodeFuncName,
+	Short:            fmt.Sprint(nodeCmdDes),
+	Long:             fmt.Sprint(nodeCmdDes),
+	PersistentPreRun: common.InitCmd,
 }