Merge pull request #148 from ecordell/failfast-builder

Bug 1790785: feat(appr): exit build early if no manifests were downloaded

Author: openshift-merge-robot

pkg/appregistry/builder.go

@@ -6,6 +6,7 @@ import (
 	"compress/gzip"
 	"context"
 	"database/sql"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -90,6 +91,11 @@ func (b *AppregistryImageBuilder) Build() error {
 	if err := downloader.DownloadManifests(b.ManifestDir, b.AppRegistryOrg); err != nil {
 		return err
 	}
+
+	if !hasManifests(b.ManifestDir) {
+		return fmt.Errorf("no manifests downloaded from appregistry %s/%s", b.AppRegistryEndpoint, b.AppRegistryOrg)
+	}
+
 	klog.V(4).Infof("downloaded manifests to %s\n", b.ManifestDir)
 
 	if err := BuildDatabase(b.ManifestDir, b.DatabasePath); err != nil {
@@ -212,3 +218,11 @@ func BuildLayer(directory string) (string, error) {
 
 	return archive.Name(), nil
 }
+
+func hasManifests(path string) bool {
+	files, err := ioutil.ReadDir(path)
+	if err != nil {
+		return false
+	}
+	return len(files) > 0
+}

pkg/appregistry/builder_test.go

@@ -20,63 +20,93 @@ var _ = Describe("Image building", func() {
 		operator *apprclient.OperatorMetadata
 	)
 
-	JustBeforeEach(func() {
-		var err error
-		builder, err = NewAppregistryImageBuilder(options...)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(builder).ToNot(BeNil())
-		Expect(builder.Build()).To(Succeed())
-	})
+	Context("with no manifests returned", func() {
+		BeforeEach(func() {
+			var noopAppender ImageAppendFunc = func(from, to, layer string) error {
+				return nil
+			}
 
-	BeforeEach(func() {
-		var noopAppender ImageAppendFunc = func(from, to, layer string) error {
-			return nil
-		}
-
-		client := &apprclientfakes.FakeClient{}
-		pkg = &apprclient.RegistryMetadata{
-			Namespace: "marsupials",
-			Name:      "koala",
-		}
-		client.ListPackagesReturns([]*apprclient.RegistryMetadata{pkg}, nil)
-
-		pkgBlob, err := ioutil.ReadFile("testdata/golden/marsupials_pkg.tar")
-		Expect(err).ToNot(HaveOccurred())
-
-		operator = &apprclient.OperatorMetadata{
-			RegistryMetadata: *pkg,
-			Blob:             pkgBlob,
-		}
-		client.RetrieveOneReturns(operator, nil)
-
-		options = append(options,
-			WithAppender(noopAppender),
-			WithAppRegistryOrg("metatheria"),
-			WithClient(client),
-		)
-	})
+			client := &apprclientfakes.FakeClient{}
+			client.RetrieveOneReturns(nil, nil)
 
-	Context("with a custom cache dir", func() {
-		var (
-			cacheDir      string
-			manifestsGlob string
-		)
+			options = append(options,
+				WithAppender(noopAppender),
+				WithAppRegistryOrg("metatheria"),
+				WithClient(client),
+			)
+		})
 
-		BeforeEach(func() {
-			cacheDir = filepath.Join("testdata", "custom-cache")
-			options = append(options, WithCacheDir(cacheDir))
-			manifestsGlob = filepath.Join(cacheDir, "manifests-*/koala/marsupials")
+		It("should show an error", func() {
+			var err error
+			builder, err = NewAppregistryImageBuilder(options...)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(builder).ToNot(BeNil())
+			err = builder.Build()
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(Equal("no manifests downloaded from appregistry https://quay.io/cnr/metatheria"))
 		})
+	})
+
+	Context("with good manifests returned", func() {
+		BeforeEach(func() {
+			var noopAppender ImageAppendFunc = func(from, to, layer string) error {
+				return nil
+			}
+
+			client := &apprclientfakes.FakeClient{}
+			pkg = &apprclient.RegistryMetadata{
+				Namespace: "marsupials",
+				Name:      "koala",
+			}
+			client.ListPackagesReturns([]*apprclient.RegistryMetadata{pkg}, nil)
 
-		AfterEach(func() {
-			Expect(os.RemoveAll(cacheDir)).To(Succeed())
+			pkgBlob, err := ioutil.ReadFile("testdata/golden/marsupials_pkg.tar")
+			Expect(err).ToNot(HaveOccurred())
+
+			operator = &apprclient.OperatorMetadata{
+				RegistryMetadata: *pkg,
+				Blob:             pkgBlob,
+			}
+			client.RetrieveOneReturns(operator, nil)
+
+			options = append(options,
+				WithAppender(noopAppender),
+				WithAppRegistryOrg("metatheria"),
+				WithClient(client),
+			)
 		})
 
-		It("should retain unpacked operator manifests", func() {
-			Expect(cacheDir).To(BeADirectory())
-			Expect(filepath.Glob(filepath.Join(manifestsGlob, "package.yaml"))).To(HaveLen(1))
-			Expect(filepath.Glob(filepath.Join(manifestsGlob, "v1.0.0", "koala.v1.0.0.clusterserviceversion.yaml"))).To(HaveLen(1))
+		JustBeforeEach(func() {
+			var err error
+			builder, err = NewAppregistryImageBuilder(options...)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(builder).ToNot(BeNil())
+			Expect(builder.Build()).To(Succeed())
 		})
 
+		Context("with a custom cache dir", func() {
+			var (
+				cacheDir      string
+				manifestsGlob string
+			)
+
+			BeforeEach(func() {
+				cacheDir = filepath.Join("testdata", "custom-cache")
+				options = append(options, WithCacheDir(cacheDir))
+				manifestsGlob = filepath.Join(cacheDir, "manifests-*/koala/marsupials")
+			})
+
+			AfterEach(func() {
+				Expect(os.RemoveAll(cacheDir)).To(Succeed())
+			})
+
+			It("should retain unpacked operator manifests", func() {
+				Expect(cacheDir).To(BeADirectory())
+				Expect(filepath.Glob(filepath.Join(manifestsGlob, "package.yaml"))).To(HaveLen(1))
+				Expect(filepath.Glob(filepath.Join(manifestsGlob, "v1.0.0", "koala.v1.0.0.clusterserviceversion.yaml"))).To(HaveLen(1))
+			})
+
+		})
 	})
+
 })