Skip to content

Commit

Permalink
fix bug where cache-cleanup did not remove prefixed files
Browse files Browse the repository at this point in the history
Signed-off-by: JT Archie <jtarchie@gmail.com>
  • Loading branch information
Kira Boyle authored and jtarchie committed Jul 29, 2020
1 parent 0b8ff7e commit 151bfea
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 48 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ can be found in [Pivotal Documentation](docs.pivotal.io/platform-automation).
This ensures that commands are not kept in `bash` history.
The environment variable `OM_PASSWORD` will overwrite the password value in `env.yml`.

## 6.1.1

### Bug Fixes
- When using `cache-cleanup`, globbing was not correctly done for files that contain the metadata prefix.
This meant that files with `[pivnet-slug,pivnet-version]` will still laying around.

## 6.1.0

### Features
Expand Down
11 changes: 10 additions & 1 deletion acceptance/download_product_s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,14 @@ var _ = Describe("download-product command", func() {
Expect(err).ToNot(HaveOccurred())
Expect(string(contents)).To(MatchYAML(`{product: example-product, stemcell: "97.57"}`))

err = ioutil.WriteFile(filepath.Join(tmpDir, "[pivnet-example-slug,1.10.0]example-product.pivotal"), nil, 0777)
Expect(err).ToNot(HaveOccurred())
err = ioutil.WriteFile(filepath.Join(tmpDir, "example-product.pivotal"), nil, 0777)
Expect(err).ToNot(HaveOccurred())

By("running the command again, it uses the cache")
command = exec.Command(pathToMain, "download-product",
"--file-glob", "*.pivotal",
"--file-glob", "example*.pivotal",
"--pivnet-product-slug", "pivnet-example-slug",
"--product-version", "1.10.1",
"--output-directory", tmpDir,
Expand All @@ -95,6 +100,7 @@ var _ = Describe("download-product command", func() {
"--stemcell-iaas", "google",
"--s3-stemcell-path", "/another/stemcell",
"--s3-product-path", "/some/product",
"--cache-cleanup", "I acknowledge this will delete files in the output directories",
)

session, err = gexec.Start(command, GinkgoWriter, GinkgoWriter)
Expand All @@ -109,6 +115,9 @@ var _ = Describe("download-product command", func() {

Expect(filepath.Join(tmpDir, "[pivnet-example-slug,1.10.1]example-product.pivotal.partial")).ToNot(BeAnExistingFile())
Expect(filepath.Join(tmpDir, "[stemcells-ubuntu-xenial,97.57]light-bosh-stemcell-97.57-google-kvm-ubuntu-xenial-go_agent.tgz.partial")).ToNot(BeAnExistingFile())

Expect(filepath.Join(tmpDir, "[pivnet-example-slug,1.10.0]example-product.pivotal")).ToNot(BeAnExistingFile())
Expect(filepath.Join(tmpDir, "example-product.pivotal")).ToNot(BeAnExistingFile())
})
})

Expand Down
27 changes: 14 additions & 13 deletions commands/download_product.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type DownloadProductOptions struct {
Bucket string `long:"blobstore-bucket" alias:"s3-bucket,gcs-bucket,azure-container" description:"bucket name where the product resides in the s3|gcs|azure compatible blobstore"`
ProductPath string `long:"blobstore-product-path" alias:"s3-product-path,gcs-product-path,azure-product-path" description:"specify the lookup path where the s3|gcs|azure product artifacts are stored"`
StemcellPath string `long:"blobstore-stemcell-path" alias:"s3-stemcell-path,gcs-stemcell-path,azure-stemcell-path" description:"specify the lookup path where the s3|gcs|azure stemcell artifacts are stored"`
CacheCleanup string `env:"CACHE_CLEANUP" description:"Delete everything except the latest artifact in output-dir and stemcell-output-dir, set to 'I acknowledge this will delete files in the output directories' to accept these terms"`
CacheCleanup string `long:"cache-cleanup" env:"CACHE_CLEANUP" description:"Delete everything except the latest artifact in output-dir and stemcell-output-dir, set to 'I acknowledge this will delete files in the output directories' to accept these terms"`
CheckAlreadyUploaded bool `long:"check-already-uploaded" description:"Check if product is already uploaded on Ops Manager before downloading. This command is authenticated."`

AzureOptions
Expand Down Expand Up @@ -450,23 +450,24 @@ func (c *DownloadProduct) downloadProductFile(slug, version, glob, prefixPath st

func (c *DownloadProduct) cleanupCacheArtifacts(outputDir string, glob string, productFilePath string, slug string) error {
if c.Options.CacheCleanup == "I acknowledge this will delete files in the output directories" {
c.stderr.Println("Cleaning up cached artifacts...")
var prefixedGlob string
if c.Options.Source != "pivnet" || c.Options.Bucket == "" {
prefixedGlob = glob
} else {
prefixedGlob = fmt.Sprintf("\\[%s,*\\]%s", slug, glob)
}

outputDirContents, err := ioutil.ReadDir(outputDir)
if err != nil {
return err
}

for _, file := range outputDirContents {
dirFilePath := path.Join(outputDir, file.Name())
if matchGlob, _ := filepath.Match(prefixedGlob, file.Name()); matchGlob {
if dirFilePath != productFilePath {
_ = os.Remove(dirFilePath)
var prefixedGlob = fmt.Sprintf("\\[%s,*\\]%s", slug, glob)
var globs = []string{glob, prefixedGlob}
for _, fileGlob := range globs {
c.stderr.Printf("Cleaning up cached artifacts in directory '%s' with the glob '%s'", outputDir, fileGlob)
for _, file := range outputDirContents {
dirFilePath := path.Join(outputDir, file.Name())
c.stderr.Printf("checking if %q needs to cleaned up", file.Name())
if matchGlob, _ := filepath.Match(fileGlob, file.Name()); matchGlob {
if dirFilePath != productFilePath {
c.stderr.Printf("cleaning up cached file: %s", dirFilePath)
_ = os.Remove(dirFilePath)
}
}
}
}
Expand Down
34 changes: 0 additions & 34 deletions commands/download_product_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,40 +466,6 @@ var _ = Describe("DownloadProduct", func() {
Expect(previousDownloadedStemcell).ToNot(BeAnExistingFile())
})
})

When("--s3 bucket is provided", func() {
BeforeEach(func() {
commandArgs = []string{
"--pivnet-api-token", "token",
"--file-glob", "cf*.pivotal",
"--pivnet-product-slug", "elastic-runtime",
"--product-version", "2.0.0",
"--output-directory", productOutputDir,
"--stemcell-output-directory", stemcellOutputDir,
"--stemcell-iaas", "google",
"--s3-bucket", "there once was a man from a",
}

})

It("only deletes files that match the glob of the product and stemcell(s), ignoring the download prefix", func() {
alreadyDownloadedProduct := tempFile(productOutputDir, "[elastic-runtime,1.0.0]cf*.pivotal")
alreadyDownloadedLightStemcell := tempFile(stemcellOutputDir, "[stemcells-ubuntu-xenial,96.00]light-bosh-google-*.tgz")
unknownFileWeDontOwn := tempFile(productOutputDir, "no-delete")

err = command.Execute(commandArgs)
Expect(err).ToNot(HaveOccurred())

downloadedFilePath := path.Join(productOutputDir, "[elastic-runtime,2.0.0]cf-2.0-build.1.pivotal")
downloadedStemcellFilePath := path.Join(stemcellOutputDir, "[stemcells-ubuntu-xenial,97.190]stemcell.tgz")
Expect(downloadedFilePath).To(BeAnExistingFile())
Expect(downloadedStemcellFilePath).To(BeAnExistingFile())

Expect(alreadyDownloadedProduct).ToNot(BeAnExistingFile())
Expect(alreadyDownloadedLightStemcell).ToNot(BeAnExistingFile())
Expect(unknownFileWeDontOwn).To(BeAnExistingFile())
})
})
})
})

Expand Down

0 comments on commit 151bfea

Please sign in to comment.