[FAB-6333] Add chaincodeAddress to peer

As described in issue FAB-6333, when using hostname for fabric peers, the changed IP address after reconnect causes a problem as the original chaincode listener continues to listen on the old address that was resolved by the hostname when the peer was started. The CR will fix this issue by adding the param "chaincodeAddress" to peer, which is the connecting host name for the chaincode, while the param "chaincodeListenAddress" is stil used to set listen address. Also, it updates core.yaml to reflect this change. At last, it refactors getting chaincode endpoint part into a single function. What's more, it removes unused "peerAddress" from core.yaml chaincode section. Change-Id: I85e273a10b66859077f10c7b0774cda55d3e9cb8 Signed-off-by: Boliang Chen <[email protected]>
hyperledger · Nov 27, 2017 · 37714c2 · 37714c2
1 parent a2ebd1b
commit 37714c2
Show file tree

Hide file tree

Showing 5 changed files with 230 additions and 16 deletions.
diff --git a/core/chaincode/chaincode_support.go b/core/chaincode/chaincode_support.go
@@ -151,8 +151,15 @@ func NewChaincodeSupport(getCCEndpoint func() (*pb.PeerEndpoint, error), userrun
 
 	ccEndpoint, err := getCCEndpoint()
 	if err != nil {
-		chaincodeLogger.Errorf("Error getting chaincode endpoint using %s: %+v", peerAddressDefault, err)
-		theChaincodeSupport.peerAddress = peerAddressDefault
+		// getCCEndpoint has already done necessary checks,
+		// therefore it will panic if any error returns and it's in dev mode.
+		// peerAddressDefault is acceptable only in dev mode.
+		if IsDevMode() {
+			chaincodeLogger.Errorf("Error getting chaincode endpoint in dev mode, using %s: %+v", peerAddressDefault, err)
+			theChaincodeSupport.peerAddress = peerAddressDefault
+		} else {
+			chaincodeLogger.Panicf("Error getting chaincode endpoint: %+v", err)
+		}
 	} else {
 		theChaincodeSupport.peerAddress = ccEndpoint.Address
 	}

diff --git a/examples/e2e_cli/base/docker-compose-base.yaml b/examples/e2e_cli/base/docker-compose-base.yaml
@@ -59,7 +59,8 @@ services:
     environment:
       - CORE_PEER_ID=peer0.org1.example.com
       - CORE_PEER_ADDRESS=peer0.org1.example.com:7051
-      - CORE_PEER_CHAINCODELISTENADDRESS=peer0.org1.example.com:7052
+      - CORE_PEER_CHAINCODEADDRESS=peer0.org1.example.com:7052
+      - CORE_PEER_CHAINCODELISTENADDRESS=0.0.0.0:7052
       - CORE_PEER_GOSSIP_EXTERNALENDPOINT=peer0.org1.example.com:7051
       - CORE_PEER_LOCALMSPID=Org1MSP
     volumes:
@@ -79,7 +80,8 @@ services:
     environment:
       - CORE_PEER_ID=peer1.org1.example.com
       - CORE_PEER_ADDRESS=peer1.org1.example.com:7051
-      - CORE_PEER_CHAINCODELISTENADDRESS=peer1.org1.example.com:7052
+      - CORE_PEER_CHAINCODEADDRESS=peer1.org1.example.com:7052
+      - CORE_PEER_CHAINCODELISTENADDRESS=0.0.0.0:7052
       - CORE_PEER_GOSSIP_EXTERNALENDPOINT=peer1.org1.example.com:7051
       - CORE_PEER_GOSSIP_BOOTSTRAP=peer0.org1.example.com:7051
       - CORE_PEER_LOCALMSPID=Org1MSP
@@ -101,7 +103,8 @@ services:
     environment:
       - CORE_PEER_ID=peer0.org2.example.com
       - CORE_PEER_ADDRESS=peer0.org2.example.com:7051
-      - CORE_PEER_CHAINCODELISTENADDRESS=peer0.org2.example.com:7052
+      - CORE_PEER_CHAINCODEADDRESS=peer0.org2.example.com:7052
+      - CORE_PEER_CHAINCODELISTENADDRESS=0.0.0.0:7052
       - CORE_PEER_GOSSIP_EXTERNALENDPOINT=peer0.org2.example.com:7051
       - CORE_PEER_LOCALMSPID=Org2MSP
     volumes:
@@ -121,7 +124,8 @@ services:
     environment:
       - CORE_PEER_ID=peer1.org2.example.com
       - CORE_PEER_ADDRESS=peer1.org2.example.com:7051
-      - CORE_PEER_CHAINCODELISTENADDRESS=peer1.org2.example.com:7052
+      - CORE_PEER_CHAINCODEADDRESS=peer1.org2.example.com:7052
+      - CORE_PEER_CHAINCODELISTENADDRESS=0.0.0.0:7052
       - CORE_PEER_GOSSIP_EXTERNALENDPOINT=peer1.org2.example.com:7051
       - CORE_PEER_GOSSIP_BOOTSTRAP=peer0.org2.example.com:7051
       - CORE_PEER_LOCALMSPID=Org2MSP

diff --git a/peer/node/start.go b/peer/node/start.go
@@ -52,6 +52,7 @@ import (
 )
 
 const (
+	chaincodeAddrKey       = "peer.chaincodeAddress"
 	chaincodeListenAddrKey = "peer.chaincodeListenAddress"
 	defaultChaincodePort   = 7052
 )
@@ -339,24 +340,83 @@ func createChaincodeServer(caCert []byte, peerHostname string) (comm.GRPCServer,
 			return nil, err
 		}
 
-		ccendpoint := viper.GetString(chaincodeListenAddrKey)
-		if ccendpoint == "" {
-			return nil, fmt.Errorf("%s not specified", chaincodeListenAddrKey)
-		}
-
-		if _, _, err = net.SplitHostPort(ccendpoint); err != nil {
+		ccEndpoint, err := computeChaincodeEndpoint(peerHostname)
+		if err != nil {
 			return nil, err
 		}
 
 		return &pb.PeerEndpoint{
 			Id:      peerEndpoint.Id,
-			Address: ccendpoint,
+			Address: ccEndpoint,
 		}, nil
 	}
 
 	return srv, ccEpFunc
 }
 
+// There could be following cases of computing chaincode endpoint:
+// Case A: if chaincodeAddrKey is set, use it
+// Case B: else if chaincodeListenAddrKey is set and not "0.0.0.0" or ("::"), use it
+// Case C: else use peer address if not "0.0.0.0" (or "::")
+// Case D: else return error
+func computeChaincodeEndpoint(peerHostname string) (ccEndpoint string, err error) {
+	logger.Infof("Entering computeChaincodeEndpoint with peerHostname: %s", peerHostname)
+	// set this to the host/ip the chaincode will resolve to. It could be
+	// the same address as the peer (such as in the sample docker env using
+	// the container name as the host name across the board)
+	ccEndpoint = viper.GetString(chaincodeAddrKey)
+	if ccEndpoint == "" {
+		// the chaincodeAddrKey is not set, try to get the address from listener
+		// (may finally use the peer address)
+		ccEndpoint = viper.GetString(chaincodeListenAddrKey)
+		if ccEndpoint == "" {
+			// Case C: chaincodeListenAddrKey is not set, use peer address
+			peerIp := net.ParseIP(peerHostname)
+			if peerIp != nil && peerIp.IsUnspecified() {
+				// Case D: all we have is "0.0.0.0" or "::" which chaincode cannot connect to
+				logger.Errorf("ChaincodeAddress and chaincodeListenAddress are nil and peerIP is %s", peerIp)
+				return "", errors.New("invalid endpoint for chaincode to connect")
+			}
+			// use peerAddress:defaultChaincodePort
+			ccEndpoint = fmt.Sprintf("%s:%d", peerHostname, defaultChaincodePort)
+
+		} else {
+			// Case B: chaincodeListenAddrKey is set
+			host, port, err := net.SplitHostPort(ccEndpoint)
+			if err != nil {
+				// and the listener was brought up above...
+				// so this should really not happen.. just a paranoid check
+				logger.Errorf("ChaincodeAddress is nil and fail to split chaincodeListenAddress: %s", err)
+				return "", err
+			}
+
+			ccListenerIp := net.ParseIP(host)
+			// ignoring other values such as Multicast address etc ...as the server
+			// wouldn't start up with this address anyway
+			if ccListenerIp != nil && ccListenerIp.IsUnspecified() {
+				// Case C: if "0.0.0.0" or "::", we have to use peer address with the listen port
+				peerIp := net.ParseIP(peerHostname)
+				if peerIp != nil && peerIp.IsUnspecified() {
+					// Case D: all we have is "0.0.0.0" or "::" which chaincode cannot connect to
+					logger.Error("ChaincodeAddress is nil while both chaincodeListenAddressIP and peerIP are 0.0.0.0")
+					return "", errors.New("invalid endpoint for chaincode to connect")
+				}
+				ccEndpoint = fmt.Sprintf("%s:%s", peerHostname, port)
+			}
+		}
+
+	} else {
+		// Case A: the chaincodeAddrKey is set
+		if _, _, err = net.SplitHostPort(ccEndpoint); err != nil {
+			logger.Errorf("Fail to split chaincodeAddress: %s", err)
+			return "", err
+		}
+	}
+
+	logger.Infof("Exit with ccEndpoint: %s", ccEndpoint)
+	return ccEndpoint, nil
+}
+
 //NOTE - when we implement JOIN we will no longer pass the chainID as param
 //The chaincode support will come up without registering system chaincodes
 //which will be registered only during join phase.

diff --git a/peer/node/start_test.go b/peer/node/start_test.go
@@ -38,6 +38,7 @@ func TestStartCmd(t *testing.T) {
 	viper.Set("peer.chaincodeListenAddress", "0.0.0.0:6052")
 	viper.Set("peer.fileSystemPath", "/tmp/hyperledger/test")
 	viper.Set("chaincode.executetimeout", "30s")
+	viper.Set("chaincode.mode", "dev")
 	overrideLogModules := []string{"msp", "gossip", "ledger", "cauthdsl", "policies", "grpc"}
 	for _, module := range overrideLogModules {
 		viper.Set("logging."+module, "INFO")
@@ -142,3 +143,142 @@ func TestHandlerMap(t *testing.T) {
 	assert.Equal(t, "/opt/lib/filter1.so", libConf.AuthFilters[0].Library)
 	assert.Equal(t, "filter2", libConf.AuthFilters[1].Name)
 }
+
+func TestCreateChaincodeServerForChaincodeAddress(t *testing.T) {
+	var peerAddress, ccListenAddress, ccAddress string
+	peerAddress = "127.0.0.1"
+	peerAddress0 := "0.0.0.0"
+	defaultAddress := "127.0.0.1:7052"
+	defaultAddress0 := "0.0.0.0:7052"
+	defaultEqualAddress0 := "[::]:7052"
+
+	/*** Scenario 1: do not set chaincodeAddress or chaincodeListenAddress ***/
+	// Scenario 1.1: peer address is not 0.0.0.0
+	// ccListenAddress will be the default one (peerAddress:7052)
+	// ccAddress will be the same to ccListenAddress
+	viper.Set(chaincodeListenAddrKey, nil)
+	viper.Set(chaincodeAddrKey, nil)
+	ccListenAddress, ccAddress, err := createChaincodeServerReturnAddress(peerAddress)
+	assert.Equal(t, defaultAddress, ccListenAddress)
+	assert.Equal(t, defaultAddress, ccAddress)
+	assert.NoError(t, err)
+	// Scenario 1.1: peer address is 0.0.0.0
+	// ccListenAddress will be the default one (peerAddress:7052)
+	// ccAddress will not be set and get an error
+	viper.Set(chaincodeListenAddrKey, nil)
+	viper.Set(chaincodeAddrKey, nil)
+	ccListenAddress, ccAddress, err = createChaincodeServerReturnAddress(peerAddress0)
+	assert.True(t, true,
+		defaultAddress0 == ccListenAddress ||
+			defaultEqualAddress0 == ccListenAddress)
+	assert.Error(t, err)
+	assert.Equal(t, "", ccAddress)
+
+	/*** Scenario 2: set up chaincodeListenAddress only ***/
+	// Scenario 2.1: chaincodeListenAddress is not 0.0.0.0 nor "::"
+	// ccListenAddress and ccAddress will be chaincodeListenAddress
+	settingChaincodeListenAddress := "127.0.0.1:8052"
+	viper.Set(chaincodeListenAddrKey, settingChaincodeListenAddress)
+	viper.Set(chaincodeAddrKey, nil)
+	ccListenAddress, ccAddress, err = createChaincodeServerReturnAddress(peerAddress)
+	assert.NoError(t, err)
+	assert.Equal(t, settingChaincodeListenAddress, ccListenAddress)
+	assert.Equal(t, settingChaincodeListenAddress, ccAddress)
+	// Scenario 2.2: chaincodeListenAddress is 0.0.0.0 and peerAddress is not 0.0.0.0
+	// ccListenAddress will be chaincodeListenAddress
+	// ccAddress will be peerAddress:8052
+	// Tips: 0.0.0.0:8052 is the equal to [::]:8052
+	settingChaincodeListenAddress = "0.0.0.0:8052"
+	settingEqualChaincodeListenAddress := "[::]:8052"
+	viper.Set(chaincodeListenAddrKey, settingChaincodeListenAddress)
+	viper.Set(chaincodeAddrKey, nil)
+	ccListenAddress, ccAddress, err = createChaincodeServerReturnAddress(peerAddress)
+	assert.NoError(t, err)
+	assert.True(t, true,
+		ccListenAddress == settingChaincodeListenAddress ||
+			ccListenAddress == settingEqualChaincodeListenAddress)
+	assert.Equal(t, peerAddress+":8052", ccAddress)
+	// Scenario 2.3: both chaincodeListenAddress and peerAddress are 0.0.0.0
+	// ccListenAddress will be chaincodeListenAddress
+	// ccAddress will not be set and get an error
+	viper.Set(chaincodeListenAddrKey, settingChaincodeListenAddress)
+	viper.Set(chaincodeAddrKey, nil)
+	ccListenAddress, ccAddress, err = createChaincodeServerReturnAddress(peerAddress0)
+	assert.Error(t, err)
+	assert.True(t, true,
+		ccListenAddress == settingChaincodeListenAddress ||
+			ccListenAddress == settingEqualChaincodeListenAddress)
+	assert.Equal(t, "", ccAddress)
+
+	/*** Scenario 3: set up chaincodeAddress only ***/
+	// Scenario 3.1: chaincodeAddress is valid
+	// ccListenAddress will be the default one (peerAddress:7052)
+	// ccAddress will be set
+	settingChaincodeAddress := "127.0.0.2:8052"
+	viper.Set(chaincodeAddrKey, settingChaincodeAddress)
+	viper.Set(chaincodeListenAddrKey, nil)
+	ccListenAddress, ccAddress, err = createChaincodeServerReturnAddress(peerAddress)
+	assert.NoError(t, err)
+	assert.Equal(t, defaultAddress, ccListenAddress)
+	assert.Equal(t, settingChaincodeAddress, ccAddress)
+	// Scenario 3.2: chaincodeAddress is invalid
+	// ccListenAddress will be the default one (peerAddress:7052)
+	// ccAddress will not be set and get an error
+	viper.Set(chaincodeAddrKey, "abc")
+	viper.Set(chaincodeListenAddrKey, nil)
+	ccListenAddress, ccAddress, err = createChaincodeServerReturnAddress(peerAddress)
+	assert.Error(t, err)
+	assert.Equal(t, defaultAddress, ccListenAddress)
+	assert.Equal(t, "", ccAddress)
+
+	/*** Scenario 4: set up both chaincodeListenAddress and chaincodeAddress ***/
+	// ccListenAddress and ccAddress will be the corresponding values
+	settingChaincodeListenAddress = "127.0.0.1:8052"
+	viper.Set(chaincodeListenAddrKey, settingChaincodeListenAddress)
+	settingChaincodeAddress = "127.0.0.2:8052"
+	viper.Set(chaincodeAddrKey, settingChaincodeAddress)
+	ccListenAddress, ccAddress, err = createChaincodeServerReturnAddress(peerAddress)
+	assert.NoError(t, err)
+	assert.Equal(t, settingChaincodeListenAddress, ccListenAddress)
+	assert.Equal(t, settingChaincodeAddress, ccAddress)
+}
+
+// TestComputeChaincodeEndpointForInvalidCCListenAddr will test those codes
+// which are not covered by TestCreateChaincodeServerForChaincodeAddress
+func TestComputeChaincodeEndpointForInvalidCCListenAddr(t *testing.T) {
+	// Scenario 1: chaincodeAddress and chaincodeListenAddress are not set
+	// Scenario 1.1: peer address is 0.0.0.0
+	// computeChaincodeEndpoint will return error
+	viper.Set(chaincodeAddrKey, nil)
+	viper.Set(chaincodeListenAddrKey, nil)
+	peerAddress0 := "0.0.0.0"
+	ccEndpoint, err := computeChaincodeEndpoint(peerAddress0)
+	assert.Error(t, err)
+	assert.Equal(t, "", ccEndpoint)
+	// Scenario 1.2: peer address is not 0.0.0.0
+	// chaincodeEndpoint will be peerAddress:7052
+	peerAddress := "127.0.0.1"
+	ccEndpoint, err = computeChaincodeEndpoint(peerAddress)
+	assert.NoError(t, err)
+	assert.Equal(t, peerAddress+":7052", ccEndpoint)
+
+	// Scenario 2: chaincodeListenAddress is invalid and chaincodeAddress is not set
+	viper.Set(chaincodeAddrKey, nil)
+	viper.Set(chaincodeListenAddrKey, "abc")
+	ccEndpoint, err = computeChaincodeEndpoint(peerAddress)
+	assert.Error(t, err)
+	assert.Equal(t, "", ccEndpoint)
+}
+
+func createChaincodeServerReturnAddress(peerHostname string) (ccListenAddress, ccAddress string, ccEpFuncErr error) {
+	ccSrv, ccEpFunc := createChaincodeServer(nil, peerHostname)
+	ccListenAddress = ccSrv.Address()
+	// release listener
+	ccSrv.Listener().Close()
+	endPoint, ccEpFuncErr := ccEpFunc()
+	if ccEpFuncErr != nil {
+		return ccListenAddress, "", ccEpFuncErr
+	}
+	ccAddress = endPoint.Address
+	return ccListenAddress, ccAddress, ccEpFuncErr
+}
diff --git a/sampleconfig/core.yaml b/sampleconfig/core.yaml
@@ -79,6 +79,12 @@ peer:
     # the peer's address (see below) with port 7052
     # chaincodeListenAddress: 0.0.0.0:7052
 
+    # The endpoint the chaincode for this peer uses to connect to the peer.
+    # If this is not specified, the chaincodeListenAddress address is selected.
+    # And if chaincodeListenAddress is not specified, address is selected from
+    # peer listenAddress.
+    # chaincodeAddress: 0.0.0.0:7052
+
     # When used as peer config, this represents the endpoint to other peers
     # in the same organization. For peers in other organization, see
     # gossip.externalEndpoint for more info.
@@ -386,9 +392,6 @@ vm:
 #
 ###############################################################################
 chaincode:
-    # This is used if chaincode endpoint resolution fails with the
-    # chaincodeListenAddress property
-    peerAddress:
 
     # The id is used by the Chaincode stub to register the executing Chaincode
     # ID with the Peer and is generally supplied through ENV variables