Copy if rename fails during external build

External builder attempts to persist build and release content by renaming them to another dir from temp. However this would not always work if temp dir is on a different mounted fs. This CR changes it to try copy if rename fails. FAB-17119 Change-Id: Ib6970807f637c7dd304efe66c479ce7a8b0bfa38 Signed-off-by: Jay Guo <guojiannan1101@gmail.com>
hyperledger · Dec 10, 2019 · 76765db · 76765db
1 parent 3e3b0fc
commit 76765db
Show file tree

Hide file tree

Showing 3 changed files with 207 additions and 6 deletions.
diff --git a/core/container/externalbuilder/copy.go b/core/container/externalbuilder/copy.go
@@ -0,0 +1,108 @@
+/*
+Copyright IBM Corp. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package externalbuilder
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+
+	"github.com/hyperledger/fabric/common/flogging"
+	"github.com/pkg/errors"
+)
+
+// MoveOrCopyDir attempts to copy src to dest by firstly trying to move, then copy upon failure.
+func MoveOrCopyDir(logger *flogging.FabricLogger, srcroot, destroot string) error {
+	mvErr := os.Rename(srcroot, destroot)
+	if mvErr == nil {
+		return nil
+	}
+
+	logger.Debugf("Failed to move %s to %s: %s, try copy instead", srcroot, destroot, mvErr)
+
+	info, err := os.Stat(srcroot)
+	if err != nil {
+		return errors.WithMessagef(err, "failed to stat dir: %s", srcroot)
+	}
+
+	if err = os.MkdirAll(destroot, info.Mode()); err != nil {
+		return errors.WithMessagef(err, "failed to make dir: %s", destroot)
+	}
+
+	cpErr := CopyDir(srcroot, destroot)
+	if cpErr == nil {
+		return nil
+	}
+
+	logger.Errorf("Failed to copy %s to %s: %s", srcroot, destroot, cpErr)
+
+	rmErr := os.RemoveAll(destroot)
+	if rmErr != nil {
+		logger.Errorf("Failed to clean targeting dir %s: %s", destroot, rmErr)
+	}
+
+	return errors.WithMessagef(cpErr, "failed to copy %s to %s", srcroot, destroot)
+}
+
+// CopyDir creates a copy of a dir
+func CopyDir(srcroot, destroot string) error {
+	return filepath.Walk(srcroot, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		srcsubpath, err := filepath.Rel(srcroot, path)
+		if err != nil {
+			return err
+		}
+		destpath := filepath.Join(destroot, srcsubpath)
+
+		if info.IsDir() { // its a dir, make corresponding dir in the dest
+			if err = os.MkdirAll(destpath, info.Mode()); err != nil {
+				return err
+			}
+			return nil
+		}
+
+		// its a file, copy to corresponding path in the dest.
+		// Intermediate directories are ensured to exist because parent
+		// node is always visited before children in `filepath.Walk`.
+		if err = copyFile(path, destpath); err != nil {
+			return err
+		}
+		return nil
+	})
+}
+
+func copyFile(srcpath, destpath string) error {
+	srcFile, err := os.Open(srcpath)
+	if err != nil {
+		return err
+	}
+	defer srcFile.Close()
+
+	info, err := srcFile.Stat()
+	if err != nil {
+		return err
+	}
+
+	destFile, err := os.Create(destpath)
+	if err != nil {
+		return err
+	}
+	defer destFile.Close()
+
+	if err = os.Chmod(destFile.Name(), info.Mode()); err != nil {
+		return err
+	}
+
+	if _, err = io.Copy(destFile, srcFile); err != nil {
+		return err
+	}
+
+	return nil
+}
diff --git a/core/container/externalbuilder/copy_test.go b/core/container/externalbuilder/copy_test.go
@@ -0,0 +1,95 @@
+/*
+Copyright IBM Corp. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package externalbuilder
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	"github.com/hyperledger/fabric/common/flogging"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"go.uber.org/zap"
+)
+
+var _ = Describe("copy", func() {
+	var (
+		logger                             *flogging.FabricLogger
+		srcRootDir, srcSubDir, destRootDir string
+		srcRootFile, srcSubFile            *os.File
+		err                                error
+	)
+
+	BeforeEach(func() {
+		srcRootDir, err = ioutil.TempDir("", "copy-test-")
+		Expect(err).NotTo(HaveOccurred())
+		srcSubDir, err = ioutil.TempDir(srcRootDir, "sub-")
+		Expect(err).NotTo(HaveOccurred())
+		srcRootFile, err = ioutil.TempFile(srcRootDir, "file-")
+		Expect(err).NotTo(HaveOccurred())
+		srcSubFile, err = ioutil.TempFile(srcSubDir, "subfile-")
+		Expect(err).NotTo(HaveOccurred())
+		err = os.Chmod(srcSubFile.Name(), os.ModePerm)
+		Expect(err).NotTo(HaveOccurred())
+
+		logger = flogging.NewFabricLogger(zap.NewNop())
+	})
+
+	AfterEach(func() {
+		os.RemoveAll(srcRootDir)
+		os.RemoveAll(destRootDir)
+	})
+
+	When("dest dir does not exist", func() {
+		BeforeEach(func() {
+			destRootDir, err = ioutil.TempDir("", "dest-")
+			Expect(err).NotTo(HaveOccurred())
+			err = os.RemoveAll(destRootDir)
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("make copy by simply moving", func() {
+			err = MoveOrCopyDir(logger, srcRootDir, destRootDir)
+			Expect(err).NotTo(HaveOccurred())
+
+			_, err = os.Stat(srcRootDir)
+			Expect(os.IsNotExist(err)).To(BeTrue())
+
+			_, err = os.Stat(filepath.Join(destRootDir, filepath.Base(srcRootFile.Name())))
+			Expect(err).NotTo(HaveOccurred())
+			_, err = os.Stat(filepath.Join(destRootDir, filepath.Base(srcSubDir)))
+			Expect(err).NotTo(HaveOccurred())
+			f, err := os.Stat(filepath.Join(destRootDir, filepath.Base(srcSubDir), filepath.Base(srcSubFile.Name())))
+			Expect(err).NotTo(HaveOccurred())
+			Expect(f.Mode()).To(Equal(os.ModePerm))
+		})
+	})
+
+	When("dest dir exits", func() {
+		BeforeEach(func() {
+			destRootDir, err = ioutil.TempDir("", "dest-")
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("fails to move and try copy", func() {
+			err = MoveOrCopyDir(logger, srcRootDir, destRootDir)
+			Expect(err).NotTo(HaveOccurred())
+
+			_, err = os.Stat(srcRootDir)
+			Expect(err).NotTo(HaveOccurred())
+
+			_, err = os.Stat(filepath.Join(destRootDir, filepath.Base(srcRootFile.Name())))
+			Expect(err).NotTo(HaveOccurred())
+			_, err = os.Stat(filepath.Join(destRootDir, filepath.Base(srcSubDir)))
+			Expect(err).NotTo(HaveOccurred())
+			f, err := os.Stat(filepath.Join(destRootDir, filepath.Base(srcSubDir), filepath.Base(srcSubFile.Name())))
+			Expect(err).NotTo(HaveOccurred())
+			Expect(f.Mode()).To(Equal(os.ModePerm))
+		})
+	})
+})
diff --git a/core/container/externalbuilder/externalbuilder.go b/core/container/externalbuilder/externalbuilder.go
@@ -152,17 +152,15 @@ func (d *Detector) Build(ccid string, md *persistence.ChaincodePackageMetadata,
 	}
 
 	durableReleaseDir := filepath.Join(durablePath, "release")
-	err = os.Rename(buildContext.ReleaseDir, durableReleaseDir)
+	err = MoveOrCopyDir(logger, buildContext.ReleaseDir, durableReleaseDir)
 	if err != nil {
-		os.RemoveAll(durablePath)
-		return nil, errors.WithMessagef(err, "could not move build context release to persistent location '%s'", durablePath)
+		return nil, errors.WithMessagef(err, "could not move or copy build context release to persistent location '%s'", durablePath)
 	}
 
 	durableBldDir := filepath.Join(durablePath, "bld")
-	err = os.Rename(buildContext.BldDir, durableBldDir)
+	err = MoveOrCopyDir(logger, buildContext.BldDir, durableBldDir)
 	if err != nil {
-		os.RemoveAll(durablePath)
-		return nil, errors.WithMessagef(err, "could not move build context bld to persistent location '%s'", durablePath)
+		return nil, errors.WithMessagef(err, "could not move or copy build context bld to persistent location '%s'", durablePath)
 	}
 
 	return &Instance{