FAB-16106 Persist bld dir for externalbuilders

This CR causes the bld directory for a chaincode to be persisted when
built by an externalbuilder.  If that externalbuilder is removed from
the system, then it returns an error.

Signed-off-by: Jason Yellick <jyellick@us.ibm.com>
Change-Id: I748e9d98bb5974d9df6b7ce02888629e1496fca5

Author: Jason Yellick

core/container/externalbuilders/externalbuilders.go

@@ -33,13 +33,13 @@ var (
 const MetadataFile = "metadata.json"
 
 type Instance struct {
-	PackageID   string
-	DurablePath string
-	Builder     *Builder
+	PackageID string
+	BldDir    string
+	Builder   *Builder
 }
 
 func (i *Instance) Start(peerConnection *ccintf.PeerConnection) error {
-	return i.Builder.Launch(i.PackageID, i.DurablePath, peerConnection)
+	return i.Builder.Launch(i.PackageID, i.BldDir, peerConnection)
 }
 
 func (i *Instance) Stop() error {
@@ -51,27 +51,63 @@ func (i *Instance) Wait() (int, error) {
 	select {}
 }
 
+type BuildInfo struct {
+	BuilderName string `json:"builder_name"`
+}
+
 type Detector struct {
 	DurablePath string
-	Builders    []peer.ExternalBuilder
+	Builders    []*Builder
 }
 
 func (d *Detector) Detect(buildContext *BuildContext) *Builder {
-	for _, detectBuilder := range d.Builders {
-		builder := &Builder{
-			Location:     detectBuilder.Path,
-			Logger:       logger.Named(filepath.Base(detectBuilder.Path)),
-			Name:         detectBuilder.Name,
-			EnvWhitelist: detectBuilder.EnvironmentWhitelist,
-		}
+	for _, builder := range d.Builders {
 		if builder.Detect(buildContext) {
 			return builder
 		}
 	}
-
 	return nil
 }
 
+// CachedBuild returns a build instance that was already built, or nil, or
+// when an unexpected error is encountered, an error.
+func (d *Detector) CachedBuild(ccid string) (*Instance, error) {
+	durablePath := filepath.Join(d.DurablePath, ccid)
+
+	_, err := os.Stat(durablePath)
+	if os.IsNotExist(err) {
+		return nil, nil
+	}
+
+	if err != nil {
+		return nil, errors.WithMessage(err, "existing build detected, but something went wrong inspecting it")
+	}
+
+	buildInfoPath := filepath.Join(durablePath, "build-info.json")
+	buildInfoData, err := ioutil.ReadFile(buildInfoPath)
+	if err != nil {
+		return nil, errors.WithMessagef(err, "could not read '%s' for build info", buildInfoPath)
+	}
+
+	buildInfo := &BuildInfo{}
+	err = json.Unmarshal(buildInfoData, buildInfo)
+	if err != nil {
+		return nil, errors.WithMessagef(err, "malformed build info at '%s'", buildInfoPath)
+	}
+
+	for _, builder := range d.Builders {
+		if builder.Name == buildInfo.BuilderName {
+			return &Instance{
+				PackageID: ccid,
+				Builder:   builder,
+				BldDir:    filepath.Join(durablePath, "bld"),
+			}, nil
+		}
+	}
+
+	return nil, errors.Errorf("chaincode '%s' was already built with builder '%s', but that builder is no longer available", ccid, buildInfo.BuilderName)
+}
+
 func (d *Detector) Build(ccid string, md *persistence.ChaincodePackageMetadata, codeStream io.Reader) (*Instance, error) {
 	if len(d.Builders) == 0 {
 		// A small optimization, especially while the launcher feature is under development
@@ -80,7 +116,16 @@ func (d *Detector) Build(ccid string, md *persistence.ChaincodePackageMetadata,
 		return nil, errors.Errorf("no builders defined")
 	}
 
-	buildContext, err := NewBuildContext(string(ccid), md, codeStream)
+	i, err := d.CachedBuild(ccid)
+	if err != nil {
+		return nil, errors.WithMessage(err, "existing build could not be restored")
+	}
+
+	if i != nil {
+		return i, nil
+	}
+
+	buildContext, err := NewBuildContext(ccid, md, codeStream)
 	if err != nil {
 		return nil, errors.WithMessage(err, "could not create build context")
 	}
@@ -89,37 +134,44 @@ func (d *Detector) Build(ccid string, md *persistence.ChaincodePackageMetadata,
 
 	builder := d.Detect(buildContext)
 	if builder == nil {
-		buildContext.Cleanup()
 		return nil, errors.Errorf("no builder found")
 	}
 
 	if err := builder.Build(buildContext); err != nil {
-		buildContext.Cleanup()
 		return nil, errors.WithMessage(err, "external builder failed to build")
 	}
 
 	durablePath := filepath.Join(d.DurablePath, ccid)
 
-	// cleanup anything which is already persisted
-	err = os.RemoveAll(durablePath)
+	err = os.Mkdir(durablePath, 0700)
 	if err != nil {
-		return nil, errors.WithMessagef(err, "could not clean dir '%s' to persist build ouput", durablePath)
+		return nil, errors.WithMessagef(err, "could not create dir '%s' to persist build ouput", durablePath)
 	}
 
-	err = os.Mkdir(durablePath, 0700)
+	buildInfo, err := json.Marshal(&BuildInfo{
+		BuilderName: builder.Name,
+	})
 	if err != nil {
-		return nil, errors.WithMessagef(err, "could not create dir '%s' to persist build ouput", durablePath)
+		os.RemoveAll(durablePath)
+		return nil, errors.WithMessage(err, "could not marshal for build-info.json")
+	}
+
+	err = ioutil.WriteFile(filepath.Join(durablePath, "build-info.json"), buildInfo, 0600)
+	if err != nil {
+		os.RemoveAll(durablePath)
+		return nil, errors.WithMessage(err, "could not write build-info.json")
 	}
 
-	err = os.Rename(buildContext.OutputDir, filepath.Join(durablePath, "bld"))
+	durableBldDir := filepath.Join(durablePath, "bld")
+	err = os.Rename(buildContext.BldDir, durableBldDir)
 	if err != nil {
 		return nil, errors.WithMessagef(err, "could not move build context bld to persistent location '%s'", durablePath)
 	}
 
 	return &Instance{
-		PackageID:   ccid,
-		Builder:     builder,
-		DurablePath: durablePath,
+		PackageID: ccid,
+		Builder:   builder,
+		BldDir:    durableBldDir,
 	}, nil
 }
 
@@ -129,7 +181,7 @@ type BuildContext struct {
 	ScratchDir  string
 	SourceDir   string
 	MetadataDir string
-	OutputDir   string
+	BldDir      string
 }
 
 var pkgIDreg = regexp.MustCompile("[^a-zA-Z0-9]+")
@@ -177,7 +229,7 @@ func NewBuildContext(ccid string, md *persistence.ChaincodePackageMetadata, code
 		ScratchDir:  scratchDir,
 		SourceDir:   sourceDir,
 		MetadataDir: metadataDir,
-		OutputDir:   outputDir,
+		BldDir:      outputDir,
 		Metadata:    md,
 		CCID:        ccid,
 	}, nil
@@ -214,6 +266,19 @@ type Builder struct {
 	Name         string
 }
 
+func CreateBuilders(builderConfs []peer.ExternalBuilder) []*Builder {
+	builders := []*Builder{}
+	for _, builderConf := range builderConfs {
+		builders = append(builders, &Builder{
+			Location:     builderConf.Path,
+			Name:         builderConf.Name,
+			EnvWhitelist: builderConf.EnvironmentWhitelist,
+			Logger:       logger.Named(builderConf.Name),
+		})
+	}
+	return builders
+}
+
 func (b *Builder) Detect(buildContext *BuildContext) bool {
 	detect := filepath.Join(b.Location, "bin", "detect")
 	cmd := NewCommand(
@@ -225,7 +290,7 @@ func (b *Builder) Detect(buildContext *BuildContext) bool {
 
 	err := RunCommand(b.Logger, cmd)
 	if err != nil {
-		logger.Debugf("Detection for builder '%s' failed: %s", b.Name, err)
+		logger.Debugf("Detection for builder '%s' detect failed: %s", b.Name, err)
 		// XXX, we probably also want to differentiate between a 'not detected'
 		// and a 'I failed nastily', but, again, good enough for now
 		return false
@@ -241,12 +306,12 @@ func (b *Builder) Build(buildContext *BuildContext) error {
 		b.EnvWhitelist,
 		buildContext.SourceDir,
 		buildContext.MetadataDir,
-		buildContext.OutputDir,
+		buildContext.BldDir,
 	)
 
 	err := RunCommand(b.Logger, cmd)
 	if err != nil {
-		return errors.Wrapf(err, "builder '%s' failed", b.Name)
+		return errors.Wrapf(err, "builder '%s' build failed", b.Name)
 	}
 
 	return nil
@@ -261,7 +326,7 @@ type LaunchConfig struct {
 	RootCert    []byte `json:"root_cert"`
 }
 
-func (b *Builder) Launch(ccid, durablePath string, peerConnection *ccintf.PeerConnection) error {
+func (b *Builder) Launch(ccid, bldDir string, peerConnection *ccintf.PeerConnection) error {
 	lc := &LaunchConfig{
 		PeerAddress: peerConnection.Address,
 		CCID:        ccid,
@@ -291,13 +356,13 @@ func (b *Builder) Launch(ccid, durablePath string, peerConnection *ccintf.PeerCo
 	cmd := NewCommand(
 		launch,
 		b.EnvWhitelist,
-		durablePath,
+		bldDir,
 		launchDir,
 	)
 
 	err = RunCommand(b.Logger, cmd)
 	if err != nil {
-		return errors.Wrapf(err, "builder '%s' failed", b.Name)
+		return errors.Wrapf(err, "builder '%s' launch failed", b.Name)
 	}
 
 	return nil

core/container/externalbuilders/externalbuilders_test.go

@@ -98,7 +98,7 @@ var _ = Describe("Externalbuilders", func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			detector = &externalbuilders.Detector{
-				Builders: []peer.ExternalBuilder{
+				Builders: externalbuilders.CreateBuilders([]peer.ExternalBuilder{
 					{
 						Path: "bad1",
 						Name: "bad1",
@@ -111,7 +111,7 @@ var _ = Describe("Externalbuilders", func() {
 						Path: "bad2",
 						Name: "bad2",
 					},
-				},
+				}),
 				DurablePath: durablePath,
 			}
 		})
@@ -140,6 +140,84 @@ var _ = Describe("Externalbuilders", func() {
 					Expect(err).To(MatchError("no builders defined"))
 				})
 			})
+
+			It("persists the build output", func() {
+				_, err := detector.Build("fake-package-id", md, codePackage)
+				Expect(err).NotTo(HaveOccurred())
+
+				_, err = os.Stat(filepath.Join(durablePath, "fake-package-id", "bld"))
+				Expect(err).NotTo(HaveOccurred())
+
+				_, err = os.Stat(filepath.Join(durablePath, "fake-package-id", "build-info.json"))
+				Expect(err).NotTo(HaveOccurred())
+			})
+
+			Context("when the durable path cannot be created", func() {
+				BeforeEach(func() {
+					detector.DurablePath = "/fake/path/to/nowhere"
+				})
+
+				It("wraps and returns the error", func() {
+					_, err := detector.Build("fake-package-id", md, codePackage)
+					Expect(err).To(MatchError("could not create dir '/fake/path/to/nowhere/fake-package-id' to persist build ouput: mkdir /fake/path/to/nowhere/fake-package-id: no such file or directory"))
+				})
+			})
+		})
+
+		Describe("CachedBuild", func() {
+			var (
+				existingInstance *externalbuilders.Instance
+			)
+
+			BeforeEach(func() {
+				var err error
+				existingInstance, err = detector.Build("fake-package-id", md, codePackage)
+				Expect(err).NotTo(HaveOccurred())
+
+				// ensure the builder will fail if invoked
+				detector.Builders[0].Location = "bad-path"
+			})
+
+			It("returns the existing built instance", func() {
+				newInstance, err := detector.Build("fake-package-id", md, codePackage)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(existingInstance).To(Equal(newInstance))
+			})
+
+			When("the build-info is missing", func() {
+				BeforeEach(func() {
+					err := os.RemoveAll(filepath.Join(durablePath, "fake-package-id", "build-info.json"))
+					Expect(err).NotTo(HaveOccurred())
+				})
+
+				It("returns an error", func() {
+					_, err := detector.Build("fake-package-id", md, codePackage)
+					Expect(err).To(MatchError(ContainSubstring("existing build could not be restored: could not read '")))
+				})
+			})
+
+			When("the build-info is corrupted", func() {
+				BeforeEach(func() {
+					err := ioutil.WriteFile(filepath.Join(durablePath, "fake-package-id", "build-info.json"), []byte("{corrupted"), 0600)
+					Expect(err).NotTo(HaveOccurred())
+				})
+
+				It("returns an error", func() {
+					_, err := detector.Build("fake-package-id", md, codePackage)
+					Expect(err).To(MatchError(ContainSubstring("invalid character 'c' looking for beginning of object key string")))
+				})
+			})
+
+			When("the builder is no longer available", func() {
+				BeforeEach(func() {
+					detector.Builders = detector.Builders[:1]
+				})
+
+				It("returns an error", func() {
+					_, err := detector.Build("fake-package-id", md, codePackage)
+					Expect(err).To(MatchError("existing build could not be restored: chaincode 'fake-package-id' was already built with builder 'testdata', but that builder is no longer available"))
+				})
+			})
 		})
 	})
 
@@ -207,7 +285,7 @@ var _ = Describe("Externalbuilders", func() {
 
 				It("returns an error", func() {
 					err := builder.Build(buildContext)
-					Expect(err).To(MatchError("builder 'testdata' failed: exit status 1"))
+					Expect(err).To(MatchError("builder 'testdata' build failed: exit status 1"))
 				})
 			})
 		})
@@ -252,7 +330,7 @@ var _ = Describe("Externalbuilders", func() {
 
 				It("returns an error", func() {
 					err := builder.Launch("test-ccid", bldDir, fakeConnection)
-					Expect(err).To(MatchError("builder 'testdata' failed: exit status 1"))
+					Expect(err).To(MatchError("builder 'testdata' launch failed: exit status 1"))
 				})
 			})
 		})

internal/peer/node/start.go

@@ -503,9 +503,12 @@ func serve(args []string) error {
 	}
 
 	externalBuilderOutput := filepath.Join(coreconfig.GetPath("peer.fileSystemPath"), "externalbuilders", "builds")
+	if err := os.MkdirAll(externalBuilderOutput, 0700); err != nil {
+		logger.Panicf("could not create externalbuilders build output dir: %s", err)
+	}
 
 	externalVM := &externalbuilders.Detector{
-		Builders:    coreConfig.ExternalBuilders,
+		Builders:    externalbuilders.CreateBuilders(coreConfig.ExternalBuilders),
 		DurablePath: externalBuilderOutput,
 	}