From b407b35816695025a20de54b2455950072771865 Mon Sep 17 00:00:00 2001 From: Dave Walter Date: Fri, 10 Jan 2025 18:51:31 -0800 Subject: [PATCH] Add a without-download flag to update-stemcell Allows Kiln to skip downloading a release if the remote source can provide the SHA1 for the release tarball. --- internal/commands/update_release.go | 10 +- internal/commands/update_stemcell.go | 32 +-- internal/commands/update_stemcell_test.go | 203 ++++++++++++++---- .../component/artifactory_release_source.go | 2 +- pkg/cargo/bosh_release.go | 2 +- 5 files changed, 187 insertions(+), 62 deletions(-) diff --git a/internal/commands/update_release.go b/internal/commands/update_release.go index ef7f1593..191cbc72 100644 --- a/internal/commands/update_release.go +++ b/internal/commands/update_release.go @@ -16,11 +16,11 @@ type UpdateRelease struct { Options struct { flags.Standard - Name string `short:"n" long:"name" required:"true" description:"name of release to update"` - Version string `short:"v" long:"version" required:"true" description:"desired version of release"` - ReleasesDir string `short:"rd" long:"releases-directory" default:"releases" description:"path to a directory to download releases into"` - AllowOnlyPublishableReleases bool `long:"allow-only-publishable-releases" description:"include releases that would not be shipped with the tile (development builds)"` - WithoutDownload bool `long:"without-download" description:"updates releases without downloading them"` + Name string `short:"n" long:"name" required:"true" description:"name of release to update"` + Version string `short:"v" long:"version" required:"true" description:"desired version of release"` + ReleasesDir string `short:"rd" long:"releases-directory" default:"releases" description:"path to a directory to download releases into"` + AllowOnlyPublishableReleases bool ` long:"allow-only-publishable-releases" default:"false" description:"include releases that would not be shipped with the tile (development builds)"` + WithoutDownload bool `short:"wd" long:"without-download" default:"false" description:"updates releases without downloading them"` } multiReleaseSourceProvider MultiReleaseSourceProvider filesystem billy.Filesystem diff --git a/internal/commands/update_stemcell.go b/internal/commands/update_stemcell.go index 88e4505e..21bf3189 100644 --- a/internal/commands/update_stemcell.go +++ b/internal/commands/update_stemcell.go @@ -19,9 +19,10 @@ type UpdateStemcell struct { Options struct { flags.Standard - Version string `short:"v" long:"version" required:"true" description:"desired version of stemcell"` - ReleasesDir string `short:"rd" long:"releases-directory" default:"releases" description:"path to a directory to download releases into"` - UpdateReleases bool `short:"ur" long:"update-releases" description:"finds latest matching releases for new stemcell version"` + Version string `short:"v" long:"version" required:"true" description:"desired version of stemcell"` + ReleasesDir string `short:"rd" long:"releases-directory" default:"releases" description:"path to a directory to download releases into"` + UpdateReleases bool `short:"ur" long:"update-releases" default:"false" description:"finds latest matching releases for new stemcell version"` + WithoutDownload bool `short:"wd" long:"without-download" default:"false" description:"updates stemcell releases without downloading releases"` } FS billy.Filesystem MultiReleaseSourceProvider MultiReleaseSourceProvider @@ -69,45 +70,50 @@ func (update UpdateStemcell) Execute(args []string) error { releaseSource := update.MultiReleaseSourceProvider(kilnfile, false) for i, rel := range kilnfileLock.Releases { - update.Logger.Printf("Updating release %q with stemcell %s %s...", rel.Name, kilnfileLock.Stemcell.OS, trimmedInputVersion) + update.Logger.Printf("Updating release %s %s with stemcell %s %s...", rel.Name, rel.Version, kilnfileLock.Stemcell.OS, trimmedInputVersion) spec, err := kilnfile.BOSHReleaseTarballSpecification(rel.Name) if err != nil { return err } + spec.StemcellOS = kilnfileLock.Stemcell.OS spec.StemcellVersion = trimmedInputVersion var remote cargo.BOSHReleaseTarballLock - if update.Options.UpdateReleases { - remote, err = releaseSource.FindReleaseVersion(spec, true) - } else { + + if !update.Options.UpdateReleases { spec.Version = rel.Version remote, err = releaseSource.GetMatchedRelease(spec) + } else { + remote, err = releaseSource.FindReleaseVersion(spec, true) } if err != nil { - return fmt.Errorf("while finding release %q, encountered error: %w", rel.Name, err) + return fmt.Errorf("while finding release %s %s, encountered error: %w", rel.Name, rel.Version, err) } + if component.IsErrNotFound(err) { - return fmt.Errorf("couldn't find release %q", rel.Name) + return fmt.Errorf("couldn't find release %s %s", rel.Name, rel.Version) } if remote.RemotePath == rel.RemotePath && remote.RemoteSource == rel.RemoteSource { - update.Logger.Printf("No change for release %q\n", rel.Name) + update.Logger.Printf("No change for release %s %s\n", rel.Name, rel.Version) continue } - lock := &kilnfileLock.Releases[i] + lock := &kilnfileLock.Releases[i] lock.RemotePath = remote.RemotePath lock.RemoteSource = remote.RemoteSource lock.SHA1 = remote.SHA1 - if remote.SHA1 == "" || remote.SHA1 == "not-calculated" { + + if !update.Options.WithoutDownload || lock.SHA1 == "" || lock.SHA1 == "not-calculated" { // release source needs to download. local, err := releaseSource.DownloadRelease(update.Options.ReleasesDir, remote) if err != nil { - return fmt.Errorf("while downloading release %q, encountered error: %w", rel.Name, err) + return fmt.Errorf("while downloading release %s %s, encountered error: %w", rel.Name, rel.Version, err) } + lock.SHA1 = local.Lock.SHA1 } } diff --git a/internal/commands/update_stemcell_test.go b/internal/commands/update_stemcell_test.go index 3c4edf6f..81307291 100644 --- a/internal/commands/update_stemcell_test.go +++ b/internal/commands/update_stemcell_test.go @@ -110,7 +110,8 @@ var _ = Describe("UpdateStemcell", func() { switch requirement.Name { case release1Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release1Name, Version: release1Version, + Name: release1Name, + Version: release1Version, RemotePath: newRelease1RemotePath, RemoteSource: publishableReleaseSourceID, SHA1: "", @@ -118,7 +119,8 @@ var _ = Describe("UpdateStemcell", func() { return remote, nil case release2Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release2Name, Version: release2Version, + Name: release2Name, + Version: release2Version, RemotePath: newRelease2RemotePath, RemoteSource: unpublishableReleaseSourceID, SHA1: "not-calculated", @@ -126,7 +128,8 @@ var _ = Describe("UpdateStemcell", func() { return remote, nil case release3Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release3Name, Version: release3Version, + Name: release3Name, + Version: release3Version, RemotePath: newRelease3RemotePath, RemoteSource: publishableReleaseSourceID, SHA1: newRelease3SHA, @@ -141,7 +144,8 @@ var _ = Describe("UpdateStemcell", func() { switch requirement.Name { case release1Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release1Name, Version: release1Version, + Name: release1Name, + Version: release1Version, RemotePath: newRelease1RemotePath, RemoteSource: publishableReleaseSourceID, SHA1: "", @@ -149,7 +153,8 @@ var _ = Describe("UpdateStemcell", func() { return remote, nil case release2Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release2Name, Version: release2Version, + Name: release2Name, + Version: release2Version, RemotePath: newRelease2RemotePath, RemoteSource: unpublishableReleaseSourceID, SHA1: "not-calculated", @@ -157,7 +162,8 @@ var _ = Describe("UpdateStemcell", func() { return remote, nil case release3Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release3Name, Version: release3Version, + Name: release3Name, + Version: release3Version, RemotePath: newRelease3RemotePath, RemoteSource: publishableReleaseSourceID, SHA1: newRelease3SHA, @@ -172,13 +178,31 @@ var _ = Describe("UpdateStemcell", func() { switch remote.Name { case release1Name: local := component.Local{ - Lock: cargo.BOSHReleaseTarballLock{Name: release1Name, Version: release1Version, SHA1: newRelease1SHA}, + Lock: cargo.BOSHReleaseTarballLock{ + Name: release1Name, + Version: release1Version, + SHA1: newRelease1SHA, + }, LocalPath: "not-used", } return local, nil case release2Name: local := component.Local{ - Lock: cargo.BOSHReleaseTarballLock{Name: release2Name, Version: release2Version, SHA1: newRelease2SHA}, + Lock: cargo.BOSHReleaseTarballLock{ + Name: release2Name, + Version: release2Version, + SHA1: newRelease2SHA, + }, + LocalPath: "not-used", + } + return local, nil + case release3Name: + local := component.Local{ + Lock: cargo.BOSHReleaseTarballLock{ + Name: release3Name, + Version: release3Version, + SHA1: newRelease3SHA, + }, LocalPath: "not-used", } return local, nil @@ -260,26 +284,33 @@ var _ = Describe("UpdateStemcell", func() { }) Expect(err).NotTo(HaveOccurred()) + Expect(releaseSource.FindReleaseVersionCallCount()).To(Equal(0)) Expect(releaseSource.GetMatchedReleaseCallCount()).To(Equal(3)) req1 := releaseSource.GetMatchedReleaseArgsForCall(0) Expect(req1).To(Equal(cargo.BOSHReleaseTarballSpecification{ - Name: release1Name, Version: release1Version, - StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, + Name: release1Name, + Version: release1Version, + StemcellOS: newStemcellOS, + StemcellVersion: newStemcellVersion, GitHubRepository: "https://example.com/lemon", })) req2 := releaseSource.GetMatchedReleaseArgsForCall(1) Expect(req2).To(Equal(cargo.BOSHReleaseTarballSpecification{ - Name: release2Name, Version: release2Version, - StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, + Name: release2Name, + Version: release2Version, + StemcellOS: newStemcellOS, + StemcellVersion: newStemcellVersion, GitHubRepository: "https://example.com/orange", })) req3 := releaseSource.GetMatchedReleaseArgsForCall(2) Expect(req3).To(Equal(cargo.BOSHReleaseTarballSpecification{ - Name: release3Name, Version: release3Version, - StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, + Name: release3Name, + Version: release3Version, + StemcellOS: newStemcellOS, + StemcellVersion: newStemcellVersion, GitHubRepository: "https://example.com/pomelo", })) }) @@ -290,37 +321,45 @@ var _ = Describe("UpdateStemcell", func() { Expect(err).NotTo(HaveOccurred()) Expect(releaseSource.FindReleaseVersionCallCount()).To(Equal(3)) + Expect(releaseSource.GetMatchedReleaseCallCount()).To(Equal(0)) req1, noDownload1 := releaseSource.FindReleaseVersionArgsForCall(0) Expect(req1).To(Equal(cargo.BOSHReleaseTarballSpecification{ - Name: release1Name, Version: "*", - StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, + Name: release1Name, + Version: "*", + StemcellOS: newStemcellOS, + StemcellVersion: newStemcellVersion, GitHubRepository: "https://example.com/lemon", })) Expect(noDownload1).To(BeTrue()) req2, noDownload2 := releaseSource.FindReleaseVersionArgsForCall(1) Expect(req2).To(Equal(cargo.BOSHReleaseTarballSpecification{ - Name: release2Name, Version: "*", - StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, + Name: release2Name, + Version: "*", + StemcellOS: newStemcellOS, + StemcellVersion: newStemcellVersion, GitHubRepository: "https://example.com/orange", })) Expect(noDownload2).To(BeTrue()) }) - It("downloads 2 of the 3 correct releases, ", func() { + It("downloads all of the releases", func() { err := update.Execute([]string{ - "--kilnfile", kilnfilePath, "--version", newStemcellVersion, "--releases-directory", releasesDirPath, + "--kilnfile", kilnfilePath, + "--version", newStemcellVersion, + "--releases-directory", releasesDirPath, }) Expect(err).NotTo(HaveOccurred()) - Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(2)) + Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(3)) actualDir, remote1 := releaseSource.DownloadReleaseArgsForCall(0) Expect(actualDir).To(Equal(releasesDirPath)) Expect(remote1).To(Equal( cargo.BOSHReleaseTarballLock{ - Name: release1Name, Version: release1Version, + Name: release1Name, + Version: release1Version, RemotePath: newRelease1RemotePath, RemoteSource: publishableReleaseSourceID, SHA1: "", @@ -331,12 +370,111 @@ var _ = Describe("UpdateStemcell", func() { Expect(actualDir).To(Equal(releasesDirPath)) Expect(remote2).To(Equal( cargo.BOSHReleaseTarballLock{ - Name: release2Name, Version: release2Version, + Name: release2Name, + Version: release2Version, RemotePath: newRelease2RemotePath, RemoteSource: unpublishableReleaseSourceID, SHA1: "not-calculated", }, )) + + actualDir, remote3 := releaseSource.DownloadReleaseArgsForCall(2) + Expect(actualDir).To(Equal(releasesDirPath)) + Expect(remote3).To(Equal( + cargo.BOSHReleaseTarballLock{ + Name: release3Name, + Version: release3Version, + RemotePath: newRelease3RemotePath, + RemoteSource: publishableReleaseSourceID, + SHA1: "new-sha1-3", + }, + )) + }) + + When("the remote information for a release doesn't change", func() { + BeforeEach(func() { + kilnfileLock.Releases[1].RemoteSource = unpublishableReleaseSourceID + kilnfileLock.Releases[1].RemotePath = newRelease2RemotePath + }) + + It("still downloads the release", func() { + err := update.Execute([]string{ + "--kilnfile", kilnfilePath, + "--version", newStemcellVersion, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(2)) + + _, remote := releaseSource.DownloadReleaseArgsForCall(0) + Expect(remote.Name).To(Equal(release1Name)) + + _, remote = releaseSource.DownloadReleaseArgsForCall(1) + Expect(remote.Name).To(Equal(release3Name)) + + Expect(string(outputBuffer.Contents())).To(ContainSubstring("No change for release " + release2Name)) + }) + }) + + When("when the --without-download flag is specified", func() { + It("downloads 2 of the 3 correct releases", func() { + err := update.Execute([]string{ + "--kilnfile", kilnfilePath, + "--version", newStemcellVersion, + "--releases-directory", releasesDirPath, + "--without-download", + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(2)) + + actualDir, remote1 := releaseSource.DownloadReleaseArgsForCall(0) + Expect(actualDir).To(Equal(releasesDirPath)) + Expect(remote1).To(Equal( + cargo.BOSHReleaseTarballLock{ + Name: release1Name, + Version: release1Version, + RemotePath: newRelease1RemotePath, + RemoteSource: publishableReleaseSourceID, + SHA1: "", + }, + )) + + actualDir, remote2 := releaseSource.DownloadReleaseArgsForCall(1) + Expect(actualDir).To(Equal(releasesDirPath)) + Expect(remote2).To(Equal( + cargo.BOSHReleaseTarballLock{ + Name: release2Name, + Version: release2Version, + RemotePath: newRelease2RemotePath, + RemoteSource: unpublishableReleaseSourceID, + SHA1: "not-calculated", + }, + )) + }) + + When("the remote information for a release doesn't change", func() { + BeforeEach(func() { + kilnfileLock.Releases[1].RemoteSource = unpublishableReleaseSourceID + kilnfileLock.Releases[1].RemotePath = newRelease2RemotePath + }) + + It("doesn't download the release", func() { + err := update.Execute([]string{ + "--kilnfile", kilnfilePath, + "--version", newStemcellVersion, + "--without-download", + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(1)) + _, remote := releaseSource.DownloadReleaseArgsForCall(0) + Expect(remote.Name).To(Equal(release1Name)) + + Expect(string(outputBuffer.Contents())).To(ContainSubstring("No change")) + Expect(string(outputBuffer.Contents())).To(ContainSubstring(release2Name)) + }) + }) }) When("the version input is invalid", func() { @@ -470,25 +608,6 @@ var _ = Describe("UpdateStemcell", func() { }) }) - When("the remote information for a release doesn't change", func() { - BeforeEach(func() { - kilnfileLock.Releases[1].RemoteSource = unpublishableReleaseSourceID - kilnfileLock.Releases[1].RemotePath = newRelease2RemotePath - }) - - It("doesn't download the release", func() { - err := update.Execute([]string{"--kilnfile", kilnfilePath, "--version", newStemcellVersion}) - Expect(err).NotTo(HaveOccurred()) - - Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(1)) - _, remote := releaseSource.DownloadReleaseArgsForCall(0) - Expect(remote.Name).To(Equal(release1Name)) - - Expect(string(outputBuffer.Contents())).To(ContainSubstring("No change")) - Expect(string(outputBuffer.Contents())).To(ContainSubstring(release2Name)) - }) - }) - When("the release can't be found", func() { BeforeEach(func() { releaseSource.GetMatchedReleaseReturns(cargo.BOSHReleaseTarballLock{}, component.ErrNotFound) diff --git a/internal/component/artifactory_release_source.go b/internal/component/artifactory_release_source.go index 4d8435b9..d0428c8b 100644 --- a/internal/component/artifactory_release_source.go +++ b/internal/component/artifactory_release_source.go @@ -178,7 +178,7 @@ func (ars *ArtifactoryReleaseSource) GetMatchedRelease(spec cargo.BOSHReleaseTar default: return cargo.BOSHReleaseTarballLock{}, fmt.Errorf("unexpected http status: %s", http.StatusText(response.StatusCode)) } - + matchedRelease := cargo.BOSHReleaseTarballLock{ Name: spec.Name, Version: spec.Version, diff --git a/pkg/cargo/bosh_release.go b/pkg/cargo/bosh_release.go index 9befde7f..8b6ca8fd 100644 --- a/pkg/cargo/bosh_release.go +++ b/pkg/cargo/bosh_release.go @@ -155,7 +155,7 @@ func ReadBOSHReleaseTarball(tarballPath string, r io.Reader) (BOSHReleaseTarball if err != nil { return BOSHReleaseTarball{}, err } - _, err = io.CopyBuffer(io.Discard, r, make([]byte, 1024 * 64)) + _, err = io.CopyBuffer(io.Discard, r, make([]byte, 1024*64)) return BOSHReleaseTarball{ Manifest: m, SHA1: hex.EncodeToString(sum.Sum(nil)),