Merge pull request #321 from pivotal-cf/fix/failure-to-export-bosh-dn…

…s-aliases

Fix/failure to export bosh dns aliases

internal/commands/cache_compiled_releases.go

@@ -130,15 +130,20 @@ func (cmd CacheCompiledReleases) Execute(args []string) error {
 			if !component.IsErrNotFound(err) {
 				return fmt.Errorf("failed check for matched release: %w", err)
 			}
-			releasesToExport = append(releasesToExport, rel)
+			releasesToExport = append(releasesToExport, component.Lock{
+				Name:            rel.Name,
+				Version:         rel.Version,
+				StemcellOS:      lock.Stemcell.OS,
+				StemcellVersion: lock.Stemcell.Version,
+			})
 			continue
 		}
 
 		cmd.Logger.Printf("found %s/%s in %s\n", rel.Name, rel.Version, remote.RemoteSource)
 
 		sum, err := cmd.downloadAndComputeSHA(releaseStore, remote)
 		if err != nil {
-			cmd.Logger.Printf("unable to get hash sum for %s/%s", remote.Name, remote.Version)
+			cmd.Logger.Printf("unable to get hash sum for %s", remote.ReleaseSlug())
 			continue
 		}
 		remote.SHA1 = sum
@@ -160,7 +165,7 @@ func (cmd CacheCompiledReleases) Execute(args []string) error {
 	}
 
 	for _, rel := range releasesToExport {
-		cmd.Logger.Printf("\t%s %s compiled with %s %s not found in cache\n", rel.Name, rel.Version, lock.Stemcell.OS, lock.Stemcell.Version)
+		cmd.Logger.Printf("\t%s compiled with %s not found in cache\n", rel.ReleaseSlug(), rel.StemcellSlug())
 	}
 
 	bosh, err := cmd.Director(cmd.Options.ClientConfiguration, omAPI)
@@ -181,22 +186,23 @@ func (cmd CacheCompiledReleases) Execute(args []string) error {
 	}
 
 	for _, rel := range releasesToExport {
-		requirement := component.Spec{
-			Name:            rel.Name,
-			Version:         rel.Version,
-			StemcellOS:      stagedStemcellOS,
-			StemcellVersion: stagedStemcellVersion,
-		}
+		releaseSlug := rel.ReleaseSlug()
+		stemcellSlug := boshdir.NewOSVersionSlug(stagedStemcellOS, stagedStemcellVersion)
 
-		if hasRelease, err := bosh.HasRelease(rel.Name, rel.Version, requirement.OSVersionSlug()); err != nil {
-			return fmt.Errorf("failed to find release %s: %w", requirement.ReleaseSlug(), err)
-		} else if !hasRelease {
-			return fmt.Errorf("%[1]s compiled with %[2]s is not found on bosh director (it might have been uploaded as a compiled release and the director can't recompile it for the compilation target %[2]s)", requirement.ReleaseSlug(), requirement.OSVersionSlug())
+		hasRelease, err := hasRequiredCompiledPackages(bosh, rel.ReleaseSlug(), stemcellSlug)
+		if err != nil {
+			if !errors.Is(err, errNoPackages) {
+				return fmt.Errorf("failed to find release %s: %w", rel.ReleaseSlug(), err)
+			}
+			cmd.Logger.Printf("%s does not have any packages\n", rel)
+		}
+		if !hasRelease {
+			return fmt.Errorf("%[1]s compiled with %[2]s is not found on bosh director (it might have been uploaded as a compiled release and the director can't recompile it for the compilation target %[2]s)", releaseSlug, stemcellSlug)
 		}
 
-		newRemote, err := cmd.cacheRelease(bosh, releaseStore, deployment, requirement)
+		newRemote, err := cmd.cacheRelease(bosh, releaseStore, deployment, releaseSlug, stemcellSlug)
 		if err != nil {
-			cmd.Logger.Printf("\tfailed to cache release %s for %s: %s\n", requirement.ReleaseSlug(), requirement.OSVersionSlug(), err)
+			cmd.Logger.Printf("\tfailed to cache release %s for %s: %s\n", releaseSlug, stemcellSlug, err)
 			continue
 		}
 
@@ -216,6 +222,37 @@ func (cmd CacheCompiledReleases) Execute(args []string) error {
 	return nil
 }
 
+var errNoPackages = errors.New("release has no packages")
+
+// hasRequiredCompiledPackages implementation is copied from the boshdir.DirectorImpl HasRelease method. It adds the check
+// for the length of the packages slice to allow for BOSH releases that do not have any packages. One example of a BOSH
+// release without packages is https://github.com/cloudfoundry/bosh-dns-aliases-release.
+func hasRequiredCompiledPackages(d boshdir.Director, releaseSlug boshdir.ReleaseSlug, stemcell boshdir.OSVersionSlug) (bool, error) {
+	release, err := d.FindRelease(releaseSlug)
+	if err != nil {
+		return false, err
+	}
+
+	pkgs, err := release.Packages()
+	if err != nil {
+		return false, err
+	}
+
+	if len(pkgs) == 0 {
+		return true, errNoPackages
+	}
+
+	for _, pkg := range pkgs {
+		for _, compiledPkg := range pkg.CompiledPackages {
+			if compiledPkg.Stemcell == stemcell {
+				return true, nil
+			}
+		}
+	}
+
+	return false, nil
+}
+
 func (cmd CacheCompiledReleases) fetchProductDeploymentData() (_ OpsManagerReleaseCacheSource, deploymentName, stemcellOS, stemcellVersion string, _ error) {
 	omAPI, err := cmd.OpsManager(cmd.Options.ClientConfiguration)
 	if err != nil {
@@ -252,21 +289,26 @@ func (cmd CacheCompiledReleases) fetchProductDeploymentData() (_ OpsManagerRelea
 	return omAPI, manifest.Name, stagedStemcell.OS, stagedStemcell.Version, nil
 }
 
-func (cmd CacheCompiledReleases) cacheRelease(bosh boshdir.Director, rc ReleaseStorage, deployment boshdir.Deployment, req component.Spec) (component.Lock, error) {
-	cmd.Logger.Printf("\texporting %s\n", req.ReleaseSlug())
-	result, err := deployment.ExportRelease(req.ReleaseSlug(), req.OSVersionSlug(), nil)
+func (cmd CacheCompiledReleases) cacheRelease(bosh boshdir.Director, rc ReleaseStorage, deployment boshdir.Deployment, releaseSlug boshdir.ReleaseSlug, stemcellSlug boshdir.OSVersionSlug) (component.Lock, error) {
+	cmd.Logger.Printf("\texporting %s\n", releaseSlug)
+	result, err := deployment.ExportRelease(releaseSlug, stemcellSlug, nil)
 	if err != nil {
 		return component.Lock{}, err
 	}
 
-	cmd.Logger.Printf("\tdownloading %s\n", req.ReleaseSlug())
-	releaseFilePath, _, sha1sum, err := cmd.saveReleaseLocally(bosh, cmd.Options.ReleasesDir, req, result)
+	cmd.Logger.Printf("\tdownloading %s\n", releaseSlug)
+	releaseFilePath, _, sha1sum, err := cmd.saveReleaseLocally(bosh, cmd.Options.ReleasesDir, releaseSlug, stemcellSlug, result)
 	if err != nil {
 		return component.Lock{}, err
 	}
 
-	cmd.Logger.Printf("\tuploading %s %s\n", req.Name, req.Version)
-	remoteRelease, err := cmd.uploadLocalRelease(req, releaseFilePath, rc)
+	cmd.Logger.Printf("\tuploading %s\n", releaseSlug)
+	remoteRelease, err := cmd.uploadLocalRelease(cargo.ComponentSpec{
+		Name:            releaseSlug.Name(),
+		Version:         releaseSlug.Version(),
+		StemcellOS:      stemcellSlug.OS(),
+		StemcellVersion: stemcellSlug.Version(),
+	}, releaseFilePath, rc)
 	if err != nil {
 		return component.Lock{}, err
 	}
@@ -308,8 +350,8 @@ func (cmd *CacheCompiledReleases) uploadLocalRelease(spec component.Spec, fp str
 	return uploader.UploadRelease(spec, f)
 }
 
-func (cmd *CacheCompiledReleases) saveReleaseLocally(director boshdir.Director, relDir string, req component.Spec, res boshdir.ExportReleaseResult) (string, string, string, error) {
-	fileName := fmt.Sprintf("%s-%s-%s-%s.tgz", req.Name, req.Version, req.StemcellOS, req.StemcellVersion)
+func (cmd *CacheCompiledReleases) saveReleaseLocally(director boshdir.Director, relDir string, releaseSlug boshdir.ReleaseSlug, stemcellSlug boshdir.OSVersionSlug, res boshdir.ExportReleaseResult) (string, string, string, error) {
+	fileName := fmt.Sprintf("%s-%s-%s-%s.tgz", releaseSlug.Name(), releaseSlug.Version(), stemcellSlug.OS(), stemcellSlug.Version())
 	filePath := filepath.Join(relDir, fileName)
 
 	f, err := cmd.FS.Create(filePath)

internal/commands/cache_compiled_releases_test.go

@@ -182,7 +182,18 @@ func TestCacheCompiledReleases_Execute_when_one_release_is_cached_another_is_alr
 		_, _ = writer.Write(releaseInBlobstore)
 		return nil
 	})
-	bosh.HasReleaseReturns(true, nil)
+	bosh.FindReleaseStub = func(slug director.ReleaseSlug) (director.Release, error) {
+		switch slug.Name() {
+		default:
+			panic(fmt.Errorf("FindReleaseStub input not handled: %#v", slug))
+		case "lemon":
+			return &boshdirFakes.FakeRelease{
+				PackagesStub: func() ([]director.Package, error) {
+					return []director.Package{{CompiledPackages: []director.CompiledPackage{{Stemcell: director.NewOSVersionSlug("alpine", "9.0.0")}}}}, nil
+				},
+			}, nil
+		}
+	}
 
 	releaseStorage := new(fakes.ReleaseStorage)
 	releaseStorage.GetMatchedReleaseCalls(fakeCacheData)
@@ -232,7 +243,7 @@ func TestCacheCompiledReleases_Execute_when_one_release_is_cached_another_is_alr
 	please.Expect(requestedID).To(Ω.Equal("some-blob-id"))
 
 	please.Expect(output.String()).To(Ω.ContainSubstring("1 release needs to be exported and cached"))
-	please.Expect(output.String()).To(Ω.ContainSubstring("lemon 3.0.0 compiled with alpine 9.0.0 not found in cache"))
+	please.Expect(output.String()).To(Ω.ContainSubstring("lemon/3.0.0 compiled with alpine/9.0.0 not found in cache"))
 	please.Expect(output.String()).To(Ω.ContainSubstring("exporting from bosh deployment cf-some-id"))
 	please.Expect(output.String()).To(Ω.ContainSubstring("exporting lemon"))
 	please.Expect(output.String()).To(Ω.ContainSubstring("downloading lemon"))
@@ -299,25 +310,25 @@ func TestCacheCompiledReleases_Execute_when_a_release_is_not_compiled_with_the_c
 	opsManager := new(fakes.OpsManagerReleaseCacheSource)
 	opsManager.GetStagedProductManifestReturns(`{"name": "cf-some-id", "stemcells": [{"os": "alpine", "version": "8.0.0"}]}`, nil)
 
-	releaseInBlobstore := []byte(`lemon-release-buffer`)
-
 	deployment := new(boshdirFakes.FakeDeployment)
+	deployment.ExportReleaseReturns(director.ExportReleaseResult{}, nil)
 	bosh := new(boshdirFakes.FakeDirector)
 	bosh.FindDeploymentReturns(deployment, nil)
-
-	bosh.HasReleaseReturns(false, nil) // <- this is the important thing
-
-	deployment.ExportReleaseReturns(director.ExportReleaseResult{}, nil)
-	bosh.DownloadResourceUncheckedCalls(func(_ string, writer io.Writer) error {
-		_, _ = writer.Write(releaseInBlobstore)
-		return nil
-	})
+	bosh.FindReleaseStub = func(slug director.ReleaseSlug) (director.Release, error) {
+		switch slug.Name() {
+		default:
+			panic(fmt.Errorf("FindReleaseStub input not handled: %#v", slug))
+		case "banana":
+			return &boshdirFakes.FakeRelease{
+				PackagesStub: func() ([]director.Package, error) {
+					return make([]director.Package, 1), nil
+				},
+			}, nil
+		}
+	}
 
 	releaseStorage := new(fakes.ReleaseStorage)
 	releaseStorage.GetMatchedReleaseCalls(fakeCacheData)
-	releaseStorage.UploadReleaseCalls(func(_ component.Spec, reader io.Reader) (component.Lock, error) {
-		return component.Lock{}, nil
-	})
 
 	var output bytes.Buffer
 	logger := log.New(&output, "", 0)
@@ -346,16 +357,18 @@ func TestCacheCompiledReleases_Execute_when_a_release_is_not_compiled_with_the_c
 
 	please.Expect(err).To(Ω.MatchError(Ω.ContainSubstring("not found on bosh director")))
 
+	please.Expect(bosh.DownloadResourceUncheckedCallCount()).To(Ω.Equal(0))
+	please.Expect(bosh.HasReleaseCallCount()).To(Ω.Equal(0))
+	please.Expect(bosh.FindReleaseCallCount()).To(Ω.Equal(1))
+
 	{
-		requestedReleaseName, requestedReleaseVersion, requestedStemcellSlug := bosh.HasReleaseArgsForCall(0)
-		please.Expect(requestedReleaseName).To(Ω.Equal("banana"))
-		please.Expect(requestedReleaseVersion).To(Ω.Equal("2.0.0"))
-		please.Expect(requestedStemcellSlug.Version()).To(Ω.Equal("8.0.0"))
-		please.Expect(requestedStemcellSlug.OS()).To(Ω.Equal("alpine"))
+		requestedReleaseSlug := bosh.FindReleaseArgsForCall(0)
+		please.Expect(requestedReleaseSlug.Name()).To(Ω.Equal("banana"))
+		please.Expect(requestedReleaseSlug.Version()).To(Ω.Equal("2.0.0"))
 	}
 
 	please.Expect(output.String()).To(Ω.ContainSubstring("1 release needs to be exported and cached"))
-	please.Expect(output.String()).To(Ω.ContainSubstring("banana 2.0.0 compiled with alpine 8.0.0 not found in cache"))
+	please.Expect(output.String()).To(Ω.ContainSubstring("banana/2.0.0 compiled with alpine/8.0.0 not found in cache"))
 	please.Expect(output.String()).To(Ω.ContainSubstring("exporting from bosh deployment cf-some-id"))
 	please.Expect(output.String()).NotTo(Ω.ContainSubstring("exporting lemon"))
 	please.Expect(output.String()).NotTo(Ω.ContainSubstring("DON'T FORGET TO MAKE A COMMIT AND PR"))
@@ -373,8 +386,143 @@ func TestCacheCompiledReleases_Execute_when_a_release_is_not_compiled_with_the_c
 	}), "it should not override the in-correct element in the Kilnfile.lock")
 }
 
-// this test ensures make it so that we don't have to iterate over all the releases
-// before failing due to a stemcell mismatch
+// this test covers
+// - when a release does not contain packages
+func TestCacheCompiledReleases_Execute_when_a_release_has_no_packages(t *testing.T) {
+	please := Ω.NewWithT(t)
+
+	// setup
+
+	fs := memfs.New()
+
+	please.Expect(fsWriteYAML(fs, "Kilnfile", cargo.Kilnfile{
+		ReleaseSources: []cargo.ReleaseSourceConfig{
+			{
+				ID:           "cached-compiled-releases",
+				Publishable:  true,
+				PathTemplate: "{{.Release}}-{{.Version}}.tgz",
+			},
+			{
+				ID:           "new-releases",
+				Publishable:  false,
+				PathTemplate: "{{.Release}}-{{.Version}}.tgz",
+			},
+		},
+		Releases: []cargo.ComponentSpec{
+			{
+				Name: "banana",
+			},
+		},
+	})).NotTo(Ω.HaveOccurred())
+	please.Expect(fsWriteYAML(fs, "Kilnfile.lock", cargo.KilnfileLock{
+		Releases: []cargo.ComponentLock{
+			{
+				Name:    "banana",
+				Version: "2.0.0",
+
+				RemoteSource: "cached-compiled-releases",
+				RemotePath:   "banana-2.0.0-alpine-5.5.5",
+
+				SHA1: "fake-checksum",
+			},
+		},
+		Stemcell: cargo.Stemcell{
+			OS:      "alpine",
+			Version: "8.0.0",
+		},
+	})).NotTo(Ω.HaveOccurred())
+
+	opsManager := new(fakes.OpsManagerReleaseCacheSource)
+	opsManager.GetStagedProductManifestReturns(`{"name": "cf-some-id", "stemcells": [{"os": "alpine", "version": "8.0.0"}]}`, nil)
+
+	deployment := new(boshdirFakes.FakeDeployment)
+	deployment.ExportReleaseReturns(director.ExportReleaseResult{SHA1: "sha256:7dd4f2f077e449b47215359e8020c0b6c81e184d2c614486246cb8f70cac7a70"}, nil)
+	bosh := new(boshdirFakes.FakeDirector)
+	bosh.DownloadResourceUncheckedCalls(func(_ string, writer io.Writer) error {
+		_, _ = writer.Write([]byte("greetings"))
+		return nil
+	})
+	bosh.FindDeploymentReturns(deployment, nil)
+	bosh.FindReleaseStub = func(slug director.ReleaseSlug) (director.Release, error) {
+		switch slug.Name() {
+		default:
+			panic(fmt.Errorf("FindReleaseStub input not handled: %#v", slug))
+		case "banana":
+			return &boshdirFakes.FakeRelease{
+				PackagesStub: func() ([]director.Package, error) {
+					return make([]director.Package, 0), nil
+				},
+			}, nil
+		}
+	}
+
+	releaseStorage := new(fakes.ReleaseStorage)
+	releaseStorage.GetMatchedReleaseCalls(fakeCacheData)
+	releaseStorage.UploadReleaseStub = func(spec cargo.ComponentSpec, reader io.Reader) (cargo.ComponentLock, error) {
+		l := spec.Lock()
+		l.RemotePath = "BANANA.tgz"
+		l.RemoteSource = "BASKET"
+		return l, nil
+	}
+
+	var output bytes.Buffer
+	logger := log.New(&output, "", 0)
+
+	cmd := commands.CacheCompiledReleases{
+		FS:     fs,
+		Logger: logger,
+		ReleaseSourceAndCache: func(kilnfile cargo.Kilnfile, targetID string) (commands.ReleaseStorage, error) {
+			return releaseStorage, nil
+		},
+		OpsManager: func(configuration om.ClientConfiguration) (commands.OpsManagerReleaseCacheSource, error) {
+			return opsManager, nil
+		},
+		Director: func(configuration om.ClientConfiguration, provider om.GetBoshEnvironmentAndSecurityRootCACertificateProvider) (director.Director, error) {
+			return bosh, nil
+		},
+	}
+
+	// run
+
+	err := cmd.Execute([]string{
+		"--upload-target-id", "cached-compiled-releases",
+	})
+
+	// check
+
+	please.Expect(bosh.DownloadResourceUncheckedCallCount()).To(Ω.Equal(1))
+	please.Expect(bosh.HasReleaseCallCount()).To(Ω.Equal(0))
+	please.Expect(bosh.FindReleaseCallCount()).To(Ω.Equal(1))
+
+	{
+		requestedReleaseSlug := bosh.FindReleaseArgsForCall(0)
+		please.Expect(requestedReleaseSlug.Name()).To(Ω.Equal("banana"))
+		please.Expect(requestedReleaseSlug.Version()).To(Ω.Equal("2.0.0"))
+	}
+
+	please.Expect(output.String()).To(Ω.ContainSubstring("1 release needs to be exported and cached"))
+	please.Expect(output.String()).To(Ω.ContainSubstring("banana/2.0.0 compiled with alpine/8.0.0 not found in cache"))
+	please.Expect(output.String()).To(Ω.ContainSubstring("exporting from bosh deployment cf-some-id"))
+	please.Expect(output.String()).To(Ω.ContainSubstring("oes not have any packages"))
+	please.Expect(output.String()).To(Ω.ContainSubstring("exporting banana"))
+
+	var updatedKilnfile cargo.KilnfileLock
+	please.Expect(fsReadYAML(fs, "Kilnfile.lock", &updatedKilnfile)).NotTo(Ω.HaveOccurred())
+	please.Expect(updatedKilnfile.Releases).To(Ω.ContainElement(component.Lock{
+		Name:    "banana",
+		Version: "2.0.0",
+
+		RemoteSource: "BASKET",
+		RemotePath:   "BANANA.tgz",
+
+		SHA1: "fake-checksum",
+	}), "it should not override the in-correct element in the Kilnfile.lock")
+
+	please.Expect(err).NotTo(Ω.HaveOccurred())
+
+	please.Expect(output.String()).To(Ω.ContainSubstring("DON'T FORGET TO MAKE A COMMIT AND PR"))
+}
+
 func TestCacheCompiledReleases_Execute_staged_and_lock_stemcells_are_not_the_same(t *testing.T) {
 	please := Ω.NewWithT(t)