Blobstore writes file in chunks

- Add defer file closing after success

[#102091246](https://www.pivotaltracker.com/story/show/102091246)
Signed-off-by: Luke Woydziak <[email protected]>
Signed-off-by: Kam Leung <[email protected]>

.gitignore

@@ -10,6 +10,7 @@ gobin/*
 *.sw*
 
 test/bosh/*.json
+test/micro_bosh
 
 /.idea
 /.vagrant

bin/test-unit

@@ -25,17 +25,10 @@ for i in `ls -1` ; do
 done
 let "result+=$?"
 
-echo -e "\n Checking with golint..."
-$bin/golint
-let "result+=$?"
-
 echo -e "\n Installing ginkgo..."
 $bin/go install ./internal/github.com/onsi/ginkgo/ginkgo
 let "result+=$?"
 
-echo -e "\n Govetting"
-$bin/govet
-let "result+=$?"
 
 echo -e "\n Checking for unhandled errors"
 $bin/test-unhandled-errors
@@ -57,4 +50,13 @@ if [ ! $QUIET ]; then
   fi
 fi
 
+
+echo -e "\n Checking with golint..."
+$bin/golint
+let "result+=$?"
+
+echo -e "\n Govetting"
+$bin/govet
+let "result+=$?"
+
 exit $result

internal/github.com/cloudfoundry/bosh-utils/blobstore/blob_manager.go

@@ -1,8 +1,10 @@
 package blobstore
 
 import (
+	"io"
 	"os"
 	"path/filepath"
+	"strings"
 
 	bosherr "github.com/cloudfoundry/bosh-utils/errors"
 	boshsys "github.com/cloudfoundry/bosh-agent/internal/github.com/cloudfoundry/bosh-utils/system"
@@ -19,22 +21,36 @@ func NewBlobManager(fs boshsys.FileSystem, blobstorePath string) (manager BlobMa
 	return
 }
 
-func (manager BlobManager) Fetch(blobID string) (readOnlyFile boshsys.File, err error) {
+func (manager BlobManager) Fetch(blobID string) (boshsys.File, error, int) {
 	blobPath := filepath.Join(manager.blobstorePath, blobID)
 
-	readOnlyFile, err = manager.fs.OpenFile(blobPath, os.O_RDONLY, os.ModeDir)
+	readOnlyFile, err := manager.fs.OpenFile(blobPath, os.O_RDONLY, os.ModeDir)
 	if err != nil {
-		err = bosherr.WrapError(err, "Reading blob")
+		statusCode := 500
+		if strings.Contains(err.Error(), "no such file") {
+			statusCode = 404
+		}
+		return nil, bosherr.WrapError(err, "Reading blob"), statusCode
 	}
-	return
+
+	return readOnlyFile, nil, 200
 }
 
-func (manager BlobManager) Write(blobID string, blobBytes []byte) (err error) {
+func (manager BlobManager) Write(blobID string, reader io.Reader) error {
 	blobPath := filepath.Join(manager.blobstorePath, blobID)
 
-	err = manager.fs.WriteFile(blobPath, blobBytes)
+	writeOnlyFile, err := manager.fs.OpenFile(blobPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+	if err != nil {
+		err = bosherr.WrapError(err, "Opening blob store file")
+		return err
+	}
+
+	defer func() {
+		_ = writeOnlyFile.Close()
+	}()
+	_, err = io.Copy(writeOnlyFile, reader)
 	if err != nil {
 		err = bosherr.WrapError(err, "Updating blob")
 	}
-	return
+	return err
 }

internal/github.com/cloudfoundry/bosh-utils/blobstore/blob_manager_test.go

@@ -1,34 +1,49 @@
 package blobstore_test
 
 import (
+	"bytes"
+	. "github.com/cloudfoundry/bosh-agent/internal/github.com/cloudfoundry/bosh-utils/blobstore"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-
-	. "github.com/cloudfoundry/bosh-agent/internal/github.com/cloudfoundry/bosh-utils/blobstore"
 	boshlog "github.com/cloudfoundry/bosh-utils/logger"
 	boshsys "github.com/cloudfoundry/bosh-agent/internal/github.com/cloudfoundry/bosh-utils/system"
+	boshsysfake "github.com/cloudfoundry/bosh-agent/internal/github.com/cloudfoundry/bosh-utils/system/fakes"
+	"io"
+	"os"
+	"path/filepath"
 )
 
-func createBlobManager() (blobManager BlobManager, fs boshsys.FileSystem) {
-	logger := boshlog.NewLogger(boshlog.LevelNone)
-	fs = boshsys.NewOsFileSystem(logger)
-	blobManager = NewBlobManager(fs, "/tmp")
-	return
-}
-
-func readFile(fileIO boshsys.File) (fileBytes []byte) {
-	fileStat, _ := fileIO.Stat()
-	fileBytes = make([]byte, fileStat.Size())
-	fileIO.Read(fileBytes)
-	return
-}
-
-var _ = Describe("Testing with Ginkgo", func() {
-	It("fetch", func() {
-		blobManager, fs := createBlobManager()
-		fs.WriteFileString("/tmp/105d33ae-655c-493d-bf9f-1df5cf3ca847", "some data")
-
-		readOnlyFile, err := blobManager.Fetch("105d33ae-655c-493d-bf9f-1df5cf3ca847")
+var _ = Describe("Blob Manager", func() {
+	var (
+		fs       boshsys.FileSystem
+		logger   boshlog.Logger
+		basePath string
+		blobPath string
+		blobId   string
+		toWrite  io.Reader
+	)
+
+	BeforeEach(func() {
+		logger = boshlog.NewLogger(boshlog.LevelNone)
+		fs = boshsys.NewOsFileSystem(logger)
+		blobId = "105d33ae-655c-493d-bf9f-1df5cf3ca847"
+		basePath = "/tmp"
+		blobPath = filepath.Join(basePath, blobId)
+		toWrite = bytes.NewReader([]byte("new data"))
+	})
+
+	readFile := func(fileIO boshsys.File) []byte {
+		fileStat, _ := fileIO.Stat()
+		fileBytes := make([]byte, fileStat.Size())
+		fileIO.Read(fileBytes)
+		return fileBytes
+	}
+
+	It("fetches", func() {
+		blobManager := NewBlobManager(fs, basePath)
+		fs.WriteFileString(blobPath, "some data")
+
+		readOnlyFile, err, _ := blobManager.Fetch(blobId)
 		defer fs.RemoveAll(readOnlyFile.Name())
 
 		Expect(err).ToNot(HaveOccurred())
@@ -37,17 +52,38 @@ var _ = Describe("Testing with Ginkgo", func() {
 		Expect(string(fileBytes)).To(Equal("some data"))
 	})
 
-	It("write", func() {
-
-		blobManager, fs := createBlobManager()
-		fs.WriteFileString("/tmp/105d33ae-655c-493d-bf9f-1df5cf3ca847", "some data")
-		defer fs.RemoveAll("/tmp/105d33ae-655c-493d-bf9f-1df5cf3ca847")
+	It("writes", func() {
+		blobManager := NewBlobManager(fs, basePath)
+		fs.WriteFileString(blobPath, "some data")
+		defer fs.RemoveAll(blobPath)
 
-		err := blobManager.Write("105d33ae-655c-493d-bf9f-1df5cf3ca847", []byte("new data"))
+		err := blobManager.Write(blobId, toWrite)
 		Expect(err).ToNot(HaveOccurred())
 
-		contents, err := fs.ReadFileString("/tmp/105d33ae-655c-493d-bf9f-1df5cf3ca847")
+		contents, err := fs.ReadFileString(blobPath)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(contents).To(Equal("new data"))
 	})
+
+	Context("when it writes", func() {
+		It("creates and closes the file", func() {
+			fs_ := boshsysfake.NewFakeFileSystem()
+			blobManager := NewBlobManager(fs_, basePath)
+			err := blobManager.Write(blobId, toWrite)
+			Expect(err).ToNot(HaveOccurred())
+			fileStats, err := fs_.FindFileStats(blobPath)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(fileStats.Open).To(BeFalse())
+		})
+		It("creates file with correct permissions", func() {
+			fs_ := boshsysfake.NewFakeFileSystem()
+			blobManager := NewBlobManager(fs_, basePath)
+			err := blobManager.Write(blobId, toWrite)
+			fileStats, err := fs_.FindFileStats(blobPath)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(fileStats.FileMode).To(Equal(os.FileMode(0666)))
+			Expect(fileStats.Flags).To(Equal(os.O_WRONLY | os.O_CREATE | os.O_TRUNC))
+		})
+	})
+
 })

internal/github.com/cloudfoundry/bosh-utils/system/fakes/fake_file_system.go

@@ -11,10 +11,10 @@ import (
 	"strings"
 	"sync"
 
-	gouuid "github.com/cloudfoundry/bosh-utils/internal/github.com/nu7hatch/gouuid"
+	gouuid "github.com/nu7hatch/gouuid"
 
-	boshsys "github.com/cloudfoundry/bosh-agent/internal/github.com/cloudfoundry/bosh-utils/system"
 	bosherr "github.com/cloudfoundry/bosh-utils/errors"
+	boshsys "github.com/cloudfoundry/bosh-agent/internal/github.com/cloudfoundry/bosh-utils/system"
 )
 
 type FakeFileType string
@@ -87,8 +87,11 @@ type FakeFileStats struct {
 	FileType FakeFileType
 
 	FileMode os.FileMode
+	Flags    int
 	Username string
 
+	Open bool
+
 	SymlinkTarget string
 
 	Content []byte
@@ -134,14 +137,12 @@ func NewFakeFile(path string, fs *FakeFileSystem) *FakeFile {
 		path: path,
 		fs:   fs,
 	}
-	fmt.Println("path")
-	fmt.Println(fakeFile.Contents)
-	if fs.files[path] != nil {
-		fakeFile.Contents = fs.files[path].Content
+	me := fs.files[path]
+	if me != nil {
+		fakeFile.Contents = me.Content
+		fakeFile.Stats = me
+		fakeFile.Stats.Open = true
 	}
-
-	fmt.Println("path1")
-	fmt.Println(fakeFile.Contents)
 	return fakeFile
 }
 
@@ -179,6 +180,9 @@ func (f *FakeFile) ReadAt(b []byte, offset int64) (int, error) {
 }
 
 func (f *FakeFile) Close() error {
+	if f.Stats != nil {
+		f.Stats.Open = false
+	}
 	return f.CloseErr
 }
 
@@ -254,6 +258,13 @@ func (fs *FakeFileSystem) RegisterOpenFile(path string, file *FakeFile) {
 	fs.openFiles[path] = file
 }
 
+func (fs *FakeFileSystem) FindFileStats(path string) (*FakeFileStats, error) {
+	if fs.files[path] != nil {
+		return fs.files[path], nil
+	}
+	return nil, fmt.Errorf("Path does not exist: %s", path)
+}
+
 func (fs *FakeFileSystem) OpenFile(path string, flag int, perm os.FileMode) (boshsys.File, error) {
 	fs.filesLock.Lock()
 	defer fs.filesLock.Unlock()
@@ -267,6 +278,7 @@ func (fs *FakeFileSystem) OpenFile(path string, flag int, perm os.FileMode) (bos
 	// Make sure to record a reference for FileExist, etc. to work
 	stats := fs.getOrCreateFile(path)
 	stats.FileMode = perm
+	stats.Flags = flag
 	stats.FileType = FakeFileTypeFile
 
 	if fs.openFiles[path] != nil {
@@ -444,6 +456,7 @@ func (fs *FakeFileSystem) Rename(oldPath, newPath string) error {
 	newStats.Content = stats.Content
 	newStats.FileMode = stats.FileMode
 	newStats.FileType = stats.FileType
+	newStats.Flags = stats.Flags
 
 	// Ignore error from RemoveAll
 	fs.removeAll(oldPath)

internal/vendor.json

@@ -26,29 +26,29 @@
 			"canonical": "github.com/cloudfoundry/bosh-utils/blobstore",
 			"comment": "",
 			"local": "github.com/cloudfoundry/bosh-agent/internal/github.com/cloudfoundry/bosh-utils/blobstore",
-			"revision": "3a53b0c78f8f7363c728a8b680fe9fbb9cbe913e",
-			"revisionTime": "2015-09-16T15:18:55-07:00"
+			"revision": "bcb196ba47141075059fc87f6eccafb3a49df1a0",
+			"revisionTime": "2015-09-21T12:11:41-07:00"
 		},
 		{
 			"canonical": "github.com/cloudfoundry/bosh-utils/blobstore/fakes",
 			"comment": "",
 			"local": "github.com/cloudfoundry/bosh-agent/internal/github.com/cloudfoundry/bosh-utils/blobstore/fakes",
-			"revision": "3a53b0c78f8f7363c728a8b680fe9fbb9cbe913e",
-			"revisionTime": "2015-09-16T15:18:55-07:00"
+			"revision": "bcb196ba47141075059fc87f6eccafb3a49df1a0",
+			"revisionTime": "2015-09-21T12:11:41-07:00"
 		},
 		{
 			"canonical": "github.com/cloudfoundry/bosh-utils/system",
 			"comment": "",
 			"local": "github.com/cloudfoundry/bosh-agent/internal/github.com/cloudfoundry/bosh-utils/system",
-			"revision": "3a53b0c78f8f7363c728a8b680fe9fbb9cbe913e",
-			"revisionTime": "2015-09-16T15:18:55-07:00"
+			"revision": "f77da8e892b6bff1f254fbec8d1850f816da20de",
+			"revisionTime": "2015-09-21T12:11:41-07:00"
 		},
 		{
 			"canonical": "github.com/cloudfoundry/bosh-utils/system/fakes",
 			"comment": "",
 			"local": "github.com/cloudfoundry/bosh-agent/internal/github.com/cloudfoundry/bosh-utils/system/fakes",
-			"revision": "3a53b0c78f8f7363c728a8b680fe9fbb9cbe913e",
-			"revisionTime": "2015-09-16T15:18:55-07:00"
+			"revision": "f77da8e892b6bff1f254fbec8d1850f816da20de",
+			"revisionTime": "2015-09-21T12:11:41-07:00"
 		},
 		{
 			"canonical": "github.com/cloudfoundry/bosh-init/registry",

micro/https_handler.go

@@ -135,17 +135,7 @@ func (h HTTPSHandler) putBlob(w http.ResponseWriter, r *http.Request) {
 	_, blobID := path.Split(r.URL.Path)
 	blobManager := blobstore.NewBlobManager(h.fs, h.dirProvider.MicroStore())
 
-	payload, err := ioutil.ReadAll(r.Body)
-	if err != nil {
-		w.WriteHeader(500)
-		if _, wErr := w.Write([]byte(err.Error())); wErr != nil {
-			h.logger.Error("https_handler", "Failed to write response body: %s", wErr.Error())
-		}
-
-		return
-	}
-
-	err = blobManager.Write(blobID, payload)
+	err := blobManager.Write(blobID, r.Body)
 	if err != nil {
 		w.WriteHeader(500)
 		if _, wErr := w.Write([]byte(err.Error())); wErr != nil {
@@ -161,11 +151,17 @@ func (h HTTPSHandler) getBlob(w http.ResponseWriter, r *http.Request) {
 	_, blobID := path.Split(r.URL.Path)
 	blobManager := blobstore.NewBlobManager(h.fs, h.dirProvider.MicroStore())
 
-	file, err := blobManager.Fetch(blobID)
+	file, err, statusCode := blobManager.Fetch(blobID)
 
 	if err != nil {
-		w.WriteHeader(404)
+		h.logger.Error("https_handler", "Failed to fetch blob: %s", err.Error())
+
+		w.WriteHeader(statusCode)
+
 	} else {
+		defer func() {
+			_ = file.Close()
+		}()
 		reader := bufio.NewReader(file)
 		if _, wErr := io.Copy(w, reader); wErr != nil {
 			h.logger.Error("https_handler", "Failed to write response body: %s", wErr.Error())