Merge pull request #341 from vmware-tanzu/describe-command

Describe command

pkg/imgpkg/bundle/bundle.go

@@ -5,25 +5,33 @@ package bundle
 
 import (
 	"fmt"
-	"io"
 	"path/filepath"
 	"strings"
+	"sync"
 
 	goui "github.com/cppforlife/go-cli-ui/ui"
 	regname "github.com/google/go-containerregistry/pkg/name"
 	regv1 "github.com/google/go-containerregistry/pkg/v1"
 	regremote "github.com/google/go-containerregistry/pkg/v1/remote"
 	ctlimg "github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/image"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/imageset"
+	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/internal/util"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/lockconfig"
 	plainimg "github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/plainimage"
-	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/util"
 )
 
 const (
 	BundleConfigLabel = "dev.carvel.imgpkg.bundle"
 )
 
+// Logger Interface used for logging
+type Logger interface {
+	Errorf(msg string, args ...interface{})
+	Warnf(msg string, args ...interface{})
+	Debugf(msg string, args ...interface{})
+	Tracef(msg string, args ...interface{})
+}
+
 //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . ImagesLockReader
 type ImagesLockReader interface {
 	Read(img regv1.Image) (lockconfig.ImagesLock, error)
@@ -44,7 +52,7 @@ type Bundle struct {
 	// cachedImageRefs stores set of ImageRefs that were
 	// discovered as part of reading the bundle.
 	// Includes refs only directly referenced by the bundle.
-	cachedImageRefs map[string]ImageRef
+	cachedImageRefs imageRefCache
 }
 
 func NewBundle(ref string, imagesMetadata ImagesMetadata) *Bundle {
@@ -61,21 +69,33 @@ func NewBundleWithReader(ref string, imagesMetadata ImagesMetadata, imagesLockRe
 		imgRetriever: imagesMetadata, imagesLockReader: imagesLockReader}
 }
 
+// DigestRef Bundle full location including registry, repository and digest
 func (o *Bundle) DigestRef() string { return o.plainImg.DigestRef() }
 func (o *Bundle) Repo() string      { return o.plainImg.Repo() }
 func (o *Bundle) Tag() string       { return o.plainImg.Tag() }
 
-func (o *Bundle) updateCachedImageRef(ref ImageRef) {
-	o.cachedImageRefs[ref.Image] = ref.DeepCopy()
+func (o *Bundle) updateCachedImageRefWithoutAnnotations(ref ImageRef) {
+	imgRef, found := o.cachedImageRefs.ImageRef(ref.Image)
+	img := ref.DeepCopy()
+	if !found {
+		o.cachedImageRefs.StoreImageRef(img)
+		return
+	}
+
+	img.Annotations = imgRef.Annotations
+	if img.ImageType == "" {
+		img.ImageType = imgRef.ImageType
+	}
+	o.cachedImageRefs.StoreImageRef(img)
 }
 
 func (o *Bundle) findCachedImageRef(digestRef string) (ImageRef, bool) {
-	ref, found := o.cachedImageRefs[digestRef]
+	ref, found := o.cachedImageRefs.ImageRef(digestRef)
 	if found {
 		return ref.DeepCopy(), true
 	}
 
-	for _, imgRef := range o.cachedImageRefs {
+	for _, imgRef := range o.cachedImageRefs.All() {
 		for _, loc := range imgRef.Locations() {
 			if loc == digestRef {
 				return imgRef.DeepCopy(), true
@@ -86,14 +106,6 @@ func (o *Bundle) findCachedImageRef(digestRef string) (ImageRef, bool) {
 	return ImageRef{}, false
 }
 
-func (o *Bundle) allCachedImageRefs() []ImageRef {
-	var imgsRef []ImageRef
-	for _, ref := range o.cachedImageRefs {
-		imgsRef = append(imgsRef, ref.DeepCopy())
-	}
-	return imgsRef
-}
-
 // NoteCopy writes an image-location representing the bundle / images that have been copied
 func (o *Bundle) NoteCopy(processedImages *imageset.ProcessedImages, reg ImagesMetadataWriter, ui util.UIWithLevels) error {
 	locationsCfg := ImageLocationsConfig{
@@ -114,8 +126,8 @@ func (o *Bundle) NoteCopy(processedImages *imageset.ProcessedImages, reg ImagesM
 		}
 	}
 
-	if len(locationsCfg.Images) != len(o.cachedImageRefs) {
-		panic(fmt.Sprintf("Expected: %d images to be written to Location OCI. Actual: %d were written", len(o.cachedImageRefs), len(locationsCfg.Images)))
+	if len(locationsCfg.Images) != o.cachedImageRefs.Size() {
+		panic(fmt.Sprintf("Expected: %d images to be written to Location OCI. Actual: %d were written", o.cachedImageRefs.Size(), len(locationsCfg.Images)))
 	}
 
 	destinationRef, err := regname.NewDigest(bundleProcessedImage.DigestRef)
@@ -267,13 +279,43 @@ func (o *Bundle) checkedImage() (regv1.Image, error) {
 	return img, err
 }
 
-type uiBlockWriter struct {
-	ui goui.UI
+func newImageRefCache() imageRefCache {
+	return imageRefCache{
+		cache: map[string]ImageRef{},
+		mutex: &sync.Mutex{},
+	}
 }
 
-var _ io.Writer = uiBlockWriter{}
+type imageRefCache struct {
+	cache map[string]ImageRef
+	mutex *sync.Mutex
+}
+
+func (i *imageRefCache) ImageRef(imageRef string) (ImageRef, bool) {
+	i.mutex.Lock()
+	defer i.mutex.Unlock()
+	foundImgRef, found := i.cache[imageRef]
+	return foundImgRef, found
+}
+
+func (i *imageRefCache) Size() int {
+	i.mutex.Lock()
+	defer i.mutex.Unlock()
+	return len(i.cache)
+}
+
+func (i *imageRefCache) All() []ImageRef {
+	i.mutex.Lock()
+	defer i.mutex.Unlock()
+	var result []ImageRef
+	for _, ref := range i.cache {
+		result = append(result, ref.DeepCopy())
+	}
+	return result
+}
 
-func (w uiBlockWriter) Write(p []byte) (n int, err error) {
-	w.ui.PrintBlock(p)
-	return len(p), nil
+func (i *imageRefCache) StoreImageRef(imageRef ImageRef) {
+	i.mutex.Lock()
+	defer i.mutex.Unlock()
+	i.cache[imageRef.Image] = imageRef
 }

pkg/imgpkg/bundle/bundle_images_lock.go

@@ -14,12 +14,17 @@ import (
 	regname "github.com/google/go-containerregistry/pkg/name"
 	regv1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/types"
+	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/internal/util"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/lockconfig"
-	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/util"
 )
 
-// AllImagesRefs returns a flat list of nested bundles and every image reference for a specific bundle
-func (o *Bundle) AllImagesRefs(concurrency int, ui util.UIWithLevels) ([]*Bundle, ImageRefs, error) {
+// ImagesRefs Retrieve the references for the Images of this particular bundle
+func (o *Bundle) ImagesRefs() []ImageRef {
+	return o.cachedImageRefs.All()
+}
+
+// AllImagesLockRefs returns a flat list of nested bundles and every image reference for a specific bundle
+func (o *Bundle) AllImagesLockRefs(concurrency int, ui util.UIWithLevels) ([]*Bundle, ImageRefs, error) {
 	throttleReq := util.NewThrottle(concurrency)
 
 	bundles, allImageRefs, err := o.buildAllImagesLock(&throttleReq, &processedImages{processedImgs: map[string]struct{}{}}, ui)
@@ -31,12 +36,14 @@ func (o *Bundle) AllImagesRefs(concurrency int, ui util.UIWithLevels) ([]*Bundle
 	// This loop needs to happen because we skipped some images for some bundle, and only at this point we have
 	// the full list of ImageRefs created and can fill the gaps inside each bundle
 	for _, bundle := range bundles {
-		for _, ref := range bundle.allCachedImageRefs() {
+		for _, ref := range bundle.cachedImageRefs.All() {
 			imgRef, found := allImageRefs.Find(ref.Image)
 			if !found {
 				panic(fmt.Sprintf("Internal inconsistency: The Image '%s' cannot be found in the total list of images", ref.Image))
 			}
-			bundle.updateCachedImageRef(imgRef)
+
+			// We want to keep the annotations, only ensure the rest of the information is copied
+			bundle.updateCachedImageRefWithoutAnnotations(imgRef)
 		}
 	}
 
@@ -45,7 +52,7 @@ func (o *Bundle) AllImagesRefs(concurrency int, ui util.UIWithLevels) ([]*Bundle
 
 // UpdateImageRefs updates the bundle cached images without talking to the registry
 func (o *Bundle) UpdateImageRefs(bundles []*Bundle) error {
-	o.cachedImageRefs = map[string]ImageRef{}
+	o.cachedImageRefs = newImageRefCache()
 
 	img, err := o.checkedImage()
 	if err != nil {
@@ -68,13 +75,14 @@ func (o *Bundle) UpdateImageRefs(bundles []*Bundle) error {
 			}
 		}
 		image.IsBundle = &isBundle
-		o.updateCachedImageRef(image)
+		// We want to keep the annotations, only ensure the rest of the information is copied
+		o.updateCachedImageRefWithoutAnnotations(image)
 	}
 	return nil
 }
 
 func (o *Bundle) buildAllImagesLock(throttleReq *util.Throttle, processedImgs *processedImages, ui util.UIWithLevels) ([]*Bundle, ImageRefs, error) {
-	o.cachedImageRefs = map[string]ImageRef{}
+	o.cachedImageRefs = newImageRefCache()
 
 	img, err := o.checkedImage()
 	if err != nil {
@@ -103,7 +111,7 @@ func (o *Bundle) buildAllImagesLock(throttleReq *util.Throttle, processedImgs *p
 	mutex := &sync.Mutex{}
 
 	for _, image := range imageRefsToProcess.ImageRefs() {
-		o.updateCachedImageRef(image)
+		o.cachedImageRefs.StoreImageRef(image.DeepCopy())
 
 		if skip := processedImgs.CheckAndAddImage(image.Image); skip {
 			errChan <- nil
@@ -112,7 +120,9 @@ func (o *Bundle) buildAllImagesLock(throttleReq *util.Throttle, processedImgs *p
 
 		// Check if this image is not a bundle and skips
 		if image.IsBundle != nil && *image.IsBundle == false {
-			processedImageRefs.AddImagesRef(image)
+			typedImageRef := NewContentImageRef(image.ImageRef).DeepCopy()
+			processedImageRefs.AddImagesRef(typedImageRef)
+			o.cachedImageRefs.StoreImageRef(typedImageRef)
 			errChan <- nil
 			continue
 		}
@@ -130,11 +140,16 @@ func (o *Bundle) buildAllImagesLock(throttleReq *util.Throttle, processedImgs *p
 			bundles = append(bundles, nestedBundles...)
 
 			// Adds Image to the resulting ImagesLock
-			processedImageRefs.AddImagesRef(
-				NewImageRef(imgRef,
-					len(nestedBundles) > 0, // nestedBundles have Bundles when the image is a bundle
-				),
-			)
+			isBundle := len(nestedBundles) > 0 // nestedBundles have Bundles when the image is a bundle
+			var typedImgRef ImageRef
+			if isBundle {
+				typedImgRef = NewBundleImageRef(imgRef)
+			} else {
+				typedImgRef = NewContentImageRef(imgRef)
+			}
+			o.cachedImageRefs.StoreImageRef(typedImgRef)
+			processedImageRefs.AddImagesRef(typedImgRef)
+
 			processedImageRefs.AddImagesRef(nestedBundlesProcessedImageRefs.ImageRefs()...)
 			errChan <- nil
 		}()

pkg/imgpkg/bundle/bundle_images_lock_test.go

@@ -15,8 +15,8 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/bundle"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/bundle/bundlefakes"
+	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/internal/util"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/lockconfig"
-	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/util"
 	"github.com/vmware-tanzu/carvel-imgpkg/test/helpers"
 )
 
@@ -444,7 +444,7 @@ func TestBundle_AllImagesLock_NoLocations_AllImagesCollocated(t *testing.T) {
 			fmt.Println("============")
 
 			subject := bundle.NewBundleWithReader(topBundleInfo, registryFakeBuilder.Build(), fakeImagesLockReader)
-			bundles, imagesRefs, err := subject.AllImagesRefs(6, uiLogger)
+			bundles, imagesRefs, err := subject.AllImagesLockRefs(6, uiLogger)
 			require.NoError(t, err)
 			runAssertions(t, test.assertions, imagesRefs, imagesTree)
 			checkBundlesPresence(t, bundles, imagesTree)
@@ -669,7 +669,7 @@ func TestBundle_AllImagesLock_NoLocations_ImagesNotCollocated(t *testing.T) {
 			fmt.Println("============")
 
 			subject := bundle.NewBundleWithReader(topBundleInfo, registryFakeBuilder.Build(), fakeImagesLockReader)
-			bundles, imagesRefs, err := subject.AllImagesRefs(1, uiLogger)
+			bundles, imagesRefs, err := subject.AllImagesLockRefs(1, uiLogger)
 			require.NoError(t, err)
 			runAssertions(t, test.assertions, imagesRefs, imagesTree)
 			checkBundlesPresence(t, bundles, imagesTree)
@@ -1101,7 +1101,7 @@ func TestBundle_AllImagesLock_Locations_AllImagesCollocated(t *testing.T) {
 			fmt.Println("============")
 
 			subject := bundle.NewBundleWithReader(topBundleInfo, registryFakeBuilder.Build(), fakeImagesLockReader)
-			bundles, imagesRefs, err := subject.AllImagesRefs(6, uiLogger)
+			bundles, imagesRefs, err := subject.AllImagesLockRefs(6, uiLogger)
 			require.NoError(t, err)
 			runAssertions(t, test.assertions, imagesRefs, imagesTree)
 			checkBundlesPresence(t, bundles, imagesTree)
@@ -1151,7 +1151,7 @@ func TestBundle_AllImagesLock_Locations_AllImagesCollocated(t *testing.T) {
 		fmt.Println("============")
 
 		subject := bundle.NewBundleWithReader(topBundleInfo, registryFakeBuilder.Build(), fakeImagesLockReader)
-		bundles, _, err := subject.AllImagesRefs(6, uiLogger)
+		bundles, _, err := subject.AllImagesLockRefs(6, uiLogger)
 		require.NoError(t, err)
 		checkBundlesPresence(t, bundles, imagesTree)
 
@@ -1419,12 +1419,15 @@ func (i imageNode) GenerateImagesRef() bundle.ImageRefs {
 	}
 
 	for _, node := range i.bundleImages {
-		allImageRefs.AddImagesRef(
-			bundle.NewImageRef(
-				lockconfig.ImageRef{Image: node.imageRef},
-				node.IsBundle(),
-			),
-		)
+		if node.IsBundle() {
+			allImageRefs.AddImagesRef(
+				bundle.NewBundleImageRef(lockconfig.ImageRef{Image: node.imageRef}),
+			)
+		} else {
+			allImageRefs.AddImagesRef(
+				bundle.NewContentImageRef(lockconfig.ImageRef{Image: node.imageRef}),
+			)
+		}
 	}
 
 	return allImageRefs

pkg/imgpkg/bundle/bundle_test.go

@@ -18,9 +18,9 @@ import (
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/bundle"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/bundle/bundlefakes"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/imageset"
+	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/internal/util"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/lockconfig"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/plainimage"
-	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/util"
 	"github.com/vmware-tanzu/carvel-imgpkg/test/helpers"
 )
 
@@ -826,7 +826,7 @@ func TestNoteCopy(t *testing.T) {
 		uiLogger := util.NewUILevelLogger(util.LogDebug, confUI)
 
 		subject := bundle.NewBundleFromPlainImage(plainimage.NewFetchedPlainImageWithTag(rootBundle.RefDigest, "", rootBundle.Image), reg)
-		_, _, err = subject.AllImagesRefs(1, uiLogger)
+		_, _, err = subject.AllImagesLockRefs(1, uiLogger)
 		assert.NoError(t, err)
 
 		processedImages := imageset.NewProcessedImages()