_lifecycle ignore previous build failure during install

core/chaincode/lifecycle/lifecycle.go

@@ -9,6 +9,7 @@ package lifecycle
 import (
 	"bytes"
 	"fmt"
+	"sync"
 
 	cb "github.com/hyperledger/fabric-protos-go/common"
 	"github.com/hyperledger/fabric-protos-go/msp"
@@ -308,6 +309,8 @@ type ExternalFunctions struct {
 	InstalledChaincodesLister InstalledChaincodesLister
 	ChaincodeBuilder          ChaincodeBuilder
 	BuildRegistry             *container.BuildRegistry
+	mutex                     sync.Mutex
+	BuildLocks                map[string]sync.Mutex
 }
 
 // CheckCommitReadiness takes a chaincode definition, checks that
@@ -674,11 +677,22 @@ func (ef *ExternalFunctions) InstallChaincode(chaincodeInstallPackage []byte) (*
 		return nil, errors.WithMessage(err, "could not save cc install package")
 	}
 
+	buildLock := ef.getBuildLock(packageID)
+	buildLock.Lock()
+	defer buildLock.Unlock()
+
 	buildStatus, ok := ef.BuildRegistry.BuildStatus(packageID)
-	if !ok {
-		err := ef.ChaincodeBuilder.Build(packageID)
-		buildStatus.Notify(err)
+	if ok {
+		// another invocation of lifecycle has concurrently
+		// installed a chaincode with this package id
+		<-buildStatus.Done()
+		if buildStatus.Err() == nil {
+			return nil, errors.New("chaincode already successfully installed")
+		}
+		buildStatus = ef.BuildRegistry.ResetBuildStatus(packageID)
 	}
+	err = ef.ChaincodeBuilder.Build(packageID)
+	buildStatus.Notify(err)
 	<-buildStatus.Done()
 	if err := buildStatus.Err(); err != nil {
 		ef.Resources.ChaincodeStore.Delete(packageID)
@@ -697,6 +711,22 @@ func (ef *ExternalFunctions) InstallChaincode(chaincodeInstallPackage []byte) (*
 	}, nil
 }
 
+func (ef *ExternalFunctions) getBuildLock(packageID string) *sync.Mutex {
+	ef.mutex.Lock()
+	defer ef.mutex.Unlock()
+
+	if ef.BuildLocks == nil {
+		ef.BuildLocks = map[string]sync.Mutex{}
+	}
+
+	buildLock, ok := ef.BuildLocks[packageID]
+	if !ok {
+		ef.BuildLocks[packageID] = sync.Mutex{}
+	}
+
+	return &buildLock
+}
+
 // GetInstalledChaincodePackage retrieves the installed chaincode with the given package ID
 // from the peer's chaincode store.
 func (ef *ExternalFunctions) GetInstalledChaincodePackage(packageID string) ([]byte, error) {

core/chaincode/lifecycle/lifecycle_test.go

@@ -385,21 +385,31 @@ var _ = Describe("ExternalFunctions", func() {
 			})
 		})
 
-		When("the chaincode is already being built", func() {
+		When("the chaincode is already being built and succeeds", func() {
 			BeforeEach(func() {
 				bs, ok := ef.BuildRegistry.BuildStatus("fake-hash")
 				Expect(ok).To(BeFalse())
-				bs.Notify(fmt.Errorf("fake-other-builder-error"))
+				bs.Notify(nil)
 			})
 
-			It("does not attempt to build it itself", func() {
-				ef.InstallChaincode([]byte("cc-package"))
+			It("does not attempt to rebuild it itself", func() {
+				_, err := ef.InstallChaincode([]byte("cc-package"))
+				Expect(err).To(MatchError("chaincode already successfully installed"))
 				Expect(fakeChaincodeBuilder.BuildCallCount()).To(Equal(0))
 			})
+		})
+
+		When("the chaincode is already being built and fails", func() {
+			BeforeEach(func() {
+				bs, ok := ef.BuildRegistry.BuildStatus("fake-hash")
+				Expect(ok).To(BeFalse())
+				bs.Notify(fmt.Errorf("fake-other-builder-error"))
+			})
 
-			It("wraps and returns the error generated by the other caller", func() {
+			It("attempts to rebuild it itself", func() {
 				_, err := ef.InstallChaincode([]byte("cc-package"))
-				Expect(err).To(MatchError("could not build chaincode: fake-other-builder-error"))
+				Expect(err).To(BeNil())
+				Expect(fakeChaincodeBuilder.BuildCallCount()).To(Equal(1))
 			})
 		})
 

core/container/build_registry.go

@@ -35,6 +35,20 @@ func (br *BuildRegistry) BuildStatus(ccid string) (*BuildStatus, bool) {
 	return bs, ok
 }
 
+// ResetBuildStatus returns a new BuildStatus for the ccid. This build status
+// is new and the caller's responsibility. The caller must use external
+// locking to ensure the build status is not reset by another install request
+// and must call Notify with the error (or nil) upon completion.
+func (br *BuildRegistry) ResetBuildStatus(ccid string) *BuildStatus {
+	br.mutex.Lock()
+	defer br.mutex.Unlock()
+
+	bs := NewBuildStatus()
+	br.builds[ccid] = bs
+
+	return bs
+}
+
 type BuildStatus struct {
 	mutex sync.Mutex
 	doneC chan struct{}

core/container/build_registry_test.go

@@ -48,6 +48,24 @@ var _ = Describe("BuildRegistry", func() {
 			Expect(newBS).To(Equal(initialBS))
 		})
 	})
+
+	When("a previous build status had an error", func() {
+		BeforeEach(func() {
+			bs, ok := br.BuildStatus("ccid")
+			Expect(ok).To(BeFalse())
+			Expect(bs).NotTo(BeNil())
+			bs.Notify(fmt.Errorf("fake-error"))
+			Expect(bs.Done()).To(BeClosed())
+			Expect(bs.Err()).To(MatchError(fmt.Errorf("fake-error")))
+		})
+
+		It("can be reset", func() {
+			bs := br.ResetBuildStatus("ccid")
+			Expect(bs).NotTo(BeNil())
+			Expect(bs.Done()).NotTo(BeClosed())
+			Expect(bs.Err()).To(BeNil())
+		})
+	})
 })
 
 var _ = Describe("BuildStatus", func() {

integration/nwo/deploy.go

@@ -186,7 +186,7 @@ func InstallChaincode(n *Network, chaincode Chaincode, peers ...*Peer) {
 			ClientAuth:  n.ClientAuthRequired,
 		})
 		ExpectWithOffset(1, err).NotTo(HaveOccurred())
-		EventuallyWithOffset(1, sess, n.EventuallyTimeout).Should(gexec.Exit(0))
+		EventuallyWithOffset(1, sess, n.EventuallyTimeout).Should(gexec.Exit())
 
 		EnsureInstalled(n, chaincode.Label, chaincode.PackageID, p)
 	}