Change StemcellStore to return local image paths when they exist

- Fix test polution, use explicit configs when needed

Author: Micah Young

src/bosh-vmrun-cpi/action/create_stemcell.go

@@ -26,45 +26,43 @@ func NewCreateStemcellMethod(driverClient driver.Client, stemcellClient stemcell
 }
 
 func (c CreateStemcellMethod) CreateStemcell(localImagePath string, stemcellCloudProps apiv1.StemcellCloudProps) (apiv1.StemcellCID, error) {
+	defer c.stemcellStore.Cleanup()
+
 	stemcellUuid, _ := c.uuidGen.Generate()
 	stemcellId := "cs-" + stemcellUuid
 	stemcellCID := apiv1.NewStemcellCID(stemcellUuid)
-	imagePath := ""
+	storeImagePath := ""
 
-	c.logger.DebugWithDetails("cpi", "stemcellCloudProps:", stemcellCloudProps)
+	c.logger.Debug("cpi", "stemcellCloudProps: %#+v", stemcellCloudProps)
 	stemcellProps, err := stemcell.NewStemcellProps(stemcellCloudProps)
 	if err != nil {
 		return stemcellCID, err
 	}
 
-	c.logger.DebugWithDetails("cpi", "LocalImagePath:", localImagePath)
+	c.logger.Debug("cpi", "LocalImagePath: %s", localImagePath)
+
+	storeImagePath, err = c.stemcellStore.GetByImagePathMapping(localImagePath)
+	if err != nil {
+		c.logger.Debug("cpi", "Stemcell image found at path:", localImagePath)
+
+		return stemcellCID, err
+	}
 
-	if c.fs.FileExists(localImagePath) {
-		imagePath = localImagePath
-	} else {
-		defer c.stemcellStore.Cleanup()
-		storeImagePath, err := c.stemcellStore.GetByImagePathMapping(localImagePath)
+	if storeImagePath == "" {
+		storeImagePath, err = c.stemcellStore.GetByMetadata(stemcellProps.Name, stemcellProps.Version)
 		if err != nil {
+			c.logger.Debug("cpi", "Stemcell image found by metadata")
 			return stemcellCID, err
 		}
+	}
 
-		if storeImagePath == "" {
-			storeImagePath, err = c.stemcellStore.GetByMetadata(stemcellProps.Name, stemcellProps.Version)
-			if err != nil {
-				return stemcellCID, err
-			}
-		}
-
-		c.logger.DebugWithDetails("cpi", "StoreImagePath:", storeImagePath)
+	c.logger.DebugWithDetails("cpi", "StoreImagePath:", storeImagePath)
 
-		if c.fs.FileExists(storeImagePath) {
-			imagePath = storeImagePath
-		} else {
-			return stemcellCID, errors.New("stemcell image not found locally or in image store")
-		}
+	if storeImagePath == "" {
+		return stemcellCID, errors.New("stemcell image not found locally or in image store")
 	}
 
-	ovfPath, err := c.stemcellClient.ExtractOvf(imagePath)
+	ovfPath, err := c.stemcellClient.ExtractOvf(storeImagePath)
 	if err != nil {
 		return stemcellCID, err
 	}

src/bosh-vmrun-cpi/action/create_stemcell_test.go

@@ -42,14 +42,14 @@ var _ = Describe("CreateStemcell", func() {
 		err = fs.MkdirAll("image-path", 0777)
 		Expect(err).ToNot(HaveOccurred())
 
+		localImagePath := "/path/to/image"
+
+		stemcellStore.GetByImagePathMappingReturns(localImagePath, nil)
 		stemcellClient.ExtractOvfReturns("extracted-path", nil)
 
 		var resourceCloudProps apiv1.CloudPropsImpl
 		json.Unmarshal([]byte(`{}`), &resourceCloudProps)
 
-		imageFile, _ := fs.TempFile("image")
-		localImagePath := imageFile.Name()
-
 		cid, err := m.CreateStemcell(localImagePath, resourceCloudProps)
 		Expect(err).ToNot(HaveOccurred())
 
@@ -63,15 +63,15 @@ var _ = Describe("CreateStemcell", func() {
 		Expect(stemcellClient.CleanupCallCount()).To(Equal(1))
 	})
 
-	It("uses the stemcell store", func() {
+	It("uses the stemcell store if supplied image path does not exist", func() {
 		var err error
 
 		err = fs.MkdirAll("image-path", 0777)
 		Expect(err).ToNot(HaveOccurred())
 
-		imageFile, _ := fs.TempFile("image")
-		storeImagePath := imageFile.Name()
+		storeImagePath := "/path/to/store/image"
 
+		stemcellStore.GetByImagePathMappingReturns("", nil)
 		stemcellStore.GetByMetadataReturns(storeImagePath, nil)
 		stemcellClient.ExtractOvfReturns("extracted-path", nil)
 

src/bosh-vmrun-cpi/go.mod

@@ -21,7 +21,7 @@ require (
 	golang.org/x/crypto v0.0.0-20190513172903-22d7a77e9e5f
 	golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 // indirect
 	gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect
-	gopkg.in/yaml.v2 v2.2.2 // indirect
+	gopkg.in/yaml.v2 v2.2.2
 )
 
 go 1.13

src/bosh-vmrun-cpi/go.sum

@@ -7,6 +7,7 @@ github.com/charlievieth/fs v0.0.0-20170613215519-7dc373669fa1/go.mod h1:sAoA1zHC
 github.com/cloudfoundry/bosh-utils v0.0.0-20180226215852-d73926502f7c h1:CMe6bHlZDS8iHIZSgHNiHj1yGXtuOpBH1SM6Tky2ODo=
 github.com/cloudfoundry/bosh-utils v0.0.0-20180226215852-d73926502f7c/go.mod h1:JCrKwetZGjxbfq1U139TZuXDBfdGLtjOEAfxMWKV/QM=
 github.com/cloudfoundry/bosh-utils v0.0.0-20191218225925-e190b792a340 h1:yjPS9iDPtWum9yHpm+4fSybSIq7zqriVcs7a87pn+Uw=
+github.com/cloudfoundry/bosh-utils v0.0.0-20191228100201-5f094101dfe7 h1:b1YddTIFo/z16pBowCiujI8aK2BLen8r3Pn6lYXtp9k=
 github.com/cppforlife/bosh-cpi-go v0.0.0-20171102051050-24a661ddbeef h1:8qA38RJGsce6BJYlolLPYCiS6ZN3ma18g8a1Ad4WrYc=
 github.com/cppforlife/bosh-cpi-go v0.0.0-20171102051050-24a661ddbeef/go.mod h1:WkeM0jfnGyGPPqo7JoBL8pwutlzeKgGElbkIV9ieSlU=
 github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=

src/bosh-vmrun-cpi/integration/cpi_test.go

@@ -20,6 +20,8 @@ var _ = Describe("cpi integration", func() {
 		cpiBin, err := gexec.Build("bosh-vmrun-cpi/cmd/cpi")
 		Expect(err).ToNot(HaveOccurred())
 
+		generateCPIConfig(CpiConfigPath, DirectCPIConfig)
+
 		request := fmt.Sprintf(`{ "method": "info", "arguments": [] }`)
 
 		session, stdin := GexecCommandWithStdin(cpiBin, "-configPath", CpiConfigPath)

src/bosh-vmrun-cpi/integration/driver_test.go

@@ -38,6 +38,7 @@ var _ = Describe("driver integration", func() {
 		retryFileLock := driver.NewRetryFileLock(logger)
 		vmxBuilder = vmx.NewVmxBuilder(logger)
 
+		generateCPIConfig(CpiConfigPath, DirectCPIConfig)
 		cpiConfigJson, err := fs.ReadFileString(CpiConfigPath)
 		Expect(err).ToNot(HaveOccurred())
 		cpiConfig, err := cpiconfig.NewConfigFromJson(cpiConfigJson)

src/bosh-vmrun-cpi/integration/installer_test.go

@@ -143,8 +143,6 @@ var _ = Describe("installer integration", func() {
 				nonExistantDirsSSHConfig := SSHCPIConfig
 				nonExistantDirsSSHConfig.VmStorePath = template.JSEscapeString(notPrexistingVmStoreDir)
 				nonExistantDirsSSHConfig.StemcellStorePath = template.JSEscapeString(notPrexistingStemcellStoreDir)
-				VmStoreDir = notPrexistingVmStoreDir
-				StemcellStoreDir = notPrexistingStemcellStoreDir
 				generateCPIConfig(CpiConfigPath, nonExistantDirsSSHConfig)
 			})
 
@@ -205,9 +203,9 @@ version: '97.16'
 
 					Eventually(session).Should(gexec.Exit(0))
 
-					expectedStemcellPath := filepath.Join(StemcellStoreDir, "bosh-stemcell-97.16-vsphere-esxi-ubuntu-xenial-go_agent.tgz")
+					expectedStemcellPath := filepath.Join(notPrexistingStemcellStoreDir, "bosh-stemcell-97.16-vsphere-esxi-ubuntu-xenial-go_agent.tgz")
 					Expect(expectedStemcellPath).To(BeAnExistingFile())
-					expectedMappingPath := filepath.Join(StemcellStoreDir, "mappings", fmt.Sprintf("%x.mapping", sha1.Sum([]byte(stemcellImagePath))))
+					expectedMappingPath := filepath.Join(notPrexistingStemcellStoreDir, "mappings", fmt.Sprintf("%x.mapping", sha1.Sum([]byte(stemcellImagePath))))
 					Expect(expectedMappingPath).To(BeAnExistingFile())
 
 					// running again will reuse stemcell but regenerate mapping

src/bosh-vmrun-cpi/integration/integration_suite_test.go

@@ -245,7 +245,6 @@ var _ = BeforeEach(func() {
 	configFile.Close()
 
 	CpiConfigPath, err = filepath.Abs(configFile.Name())
-	generateCPIConfig(CpiConfigPath, DirectCPIConfig)
 })
 
 var _ = AfterEach(func() {

src/bosh-vmrun-cpi/integration/stemcell_store_test.go

@@ -1,6 +1,7 @@
 package integration_test
 
 import (
+	"io/ioutil"
 	"os"
 
 	. "github.com/onsi/ginkgo"
@@ -17,6 +18,8 @@ import (
 var _ = Describe("StemcellStore integration", func() {
 	var stemcellStore stemcell.StemcellStore
 	BeforeEach(func() {
+		generateCPIConfig(CpiConfigPath, DirectCPIConfig)
+
 		logger := boshlog.NewWriterLogger(boshlog.LevelWarn, os.Stderr)
 		fs := boshsys.NewOsFileSystem(logger)
 
@@ -44,14 +47,39 @@ var _ = Describe("StemcellStore integration", func() {
 
 			Expect(stemcellImagePath).To(BeAnExistingFile())
 		})
+
+		It("returns empty when metadata doesnt match", func() {
+			stemcellImagePath, err := stemcellStore.GetByMetadata("non-existant-stemcell", "1234.5")
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(stemcellImagePath).To(Equal(""))
+		})
 	})
 
 	Context("GetByImagePathMapping", func() {
-		It("finds stemcells by image path mapping", func() {
+		It("finds stemcells when path exists", func() {
+			tmpImageFile, err := ioutil.TempFile("", "image")
+			Expect(err).ToNot(HaveOccurred())
+			defer os.RemoveAll(tmpImageFile.Name())
+
+			stemcellImagePath, err := stemcellStore.GetByImagePathMapping(tmpImageFile.Name())
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(stemcellImagePath).To(Equal(tmpImageFile.Name()))
+		})
+
+		It("finds stemcells when path does not exist but image path mapping exists", func() {
 			stemcellImagePath, err := stemcellStore.GetByImagePathMapping("/test-stemcell-tmp-image-path")
 			Expect(err).ToNot(HaveOccurred())
 
 			Expect(stemcellImagePath).To(BeAnExistingFile())
 		})
+
+		It("returns empty when path and mapping don't exist", func() {
+			stemcellImagePath, err := stemcellStore.GetByImagePathMapping("/non-existing/image")
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(stemcellImagePath).To(Equal(""))
+		})
 	})
 })

src/bosh-vmrun-cpi/stemcell/store.go

@@ -33,6 +33,10 @@ func (s stemcellStoreImpl) GetByImagePathMapping(imagePath string) (string, erro
 	var err error
 	var extractedImagePath string
 
+	if s.fs.FileExists(imagePath) {
+		return imagePath, nil
+	}
+
 	expectedMappingFileName := fmt.Sprintf("%x.mapping", sha1.Sum([]byte(imagePath)))
 	expectedMappingPath := filepath.Join(s.storePath, "mappings", expectedMappingFileName)
 	var stemcellTarballPathBytes []byte