From 81dbad62d1497b3a12bbd106dde548e6c4d68a90 Mon Sep 17 00:00:00 2001 From: razzle Date: Wed, 24 Apr 2024 23:16:01 -0500 Subject: [PATCH 01/43] first stab at re-writing image pulls Signed-off-by: razzle --- src/internal/packager/images/pull.go | 364 +++++++++++---------------- src/pkg/packager/creator/normal.go | 9 +- src/pkg/utils/image.go | 9 +- 3 files changed, 150 insertions(+), 232 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index df839e4f07..d9b92b7704 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -6,6 +6,7 @@ package images import ( "context" + "errors" "fmt" "path/filepath" "strings" @@ -16,6 +17,7 @@ import ( "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/pkg/transform" "github.com/defenseunicorns/zarf/src/pkg/utils" + "github.com/docker/docker/errdefs" "github.com/google/go-containerregistry/pkg/crane" "github.com/google/go-containerregistry/pkg/logs" "github.com/google/go-containerregistry/pkg/name" @@ -26,30 +28,20 @@ import ( clayout "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/google/go-containerregistry/pkg/v1/partial" "github.com/moby/moby/client" + "golang.org/x/sync/errgroup" ) // ImgInfo wraps references/information about an image type ImgInfo struct { - RefInfo transform.Image - Img v1.Image - HasImageLayers bool + RefInfo transform.Image + Img v1.Image } // PullAll pulls all of the images in the provided tag map. -func (i *ImageConfig) PullAll() ([]ImgInfo, error) { - var ( - longer string - imageCount = len(i.ImageList) - refInfoToImage = map[transform.Image]v1.Image{} - referenceToDigest = make(map[string]string) - imgInfoList []ImgInfo - ) - - type digestInfo struct { - refInfo transform.Image - digest string - } +func (i *ImageConfig) PullAll(dst layout.Images) (list []ImgInfo, err error) { + var longer string + imageCount := len(i.ImageList) // Give some additional user feedback on larger image sets if imageCount > 15 { longer = "This step may take a couple of minutes to complete." @@ -57,200 +49,193 @@ func (i *ImageConfig) PullAll() ([]ImgInfo, error) { longer = "This step may take several seconds to complete." } - spinner := message.NewProgressSpinner("Loading metadata for %d images. %s", imageCount, longer) + if err := helpers.CreateDirectory(dst.Base, helpers.ReadExecuteAllWriteUser); err != nil { + return nil, fmt.Errorf("failed to create image path %s: %w", dst.Base, err) + } + + cranePath, err := clayout.FromPath(dst.Base) + if err != nil { + cranePath, err = clayout.Write(dst.Base, empty.Index) + if err != nil { + return nil, err + } + } + + spinner := message.NewProgressSpinner("Fetching %d images. %s", imageCount, longer) defer spinner.Stop() logs.Warn.SetOutput(&message.DebugWriter{}) logs.Progress.SetOutput(&message.DebugWriter{}) - metadataImageConcurrency := helpers.NewConcurrencyTools[ImgInfo, error](len(i.ImageList)) - - defer metadataImageConcurrency.Cancel() + ctx := context.TODO() + ctx, cancel := context.WithCancel(ctx) + eg, ectx := errgroup.WithContext(ctx) + defer cancel() + eg.SetLimit(10) - spinner.Updatef("Fetching image metadata (0 of %d)", len(i.ImageList)) - - // Spawn a goroutine for each image to load its metadata - for _, refInfo := range i.ImageList { - // Create a closure so that we can pass the src into the goroutine - refInfo := refInfo - go func() { + refInfoToImage := make(map[transform.Image]v1.Image) + totalBytes := int64(0) + processed := make(map[string]v1.Layer) + opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) - if metadataImageConcurrency.IsDone() { - return - } + for idx, refInfo := range i.ImageList { + refInfo, idx := refInfo, idx + eg.Go(func() error { + spinner.Updatef("Fetching image (%d of %d)", idx+1, len(i.ImageList)) - actualSrc := refInfo.Reference + actual := refInfo.Reference for k, v := range i.RegistryOverrides { if strings.HasPrefix(refInfo.Reference, k) { - actualSrc = strings.Replace(refInfo.Reference, k, v, 1) + actual = strings.Replace(refInfo.Reference, k, v, 1) } } - if metadataImageConcurrency.IsDone() { - return - } - - img, hasImageLayers, err := i.PullImage(actualSrc, spinner) - if err != nil { - metadataImageConcurrency.ErrorChan <- fmt.Errorf("failed to pull %s: %w", actualSrc, err) - return - } + var img v1.Image - if metadataImageConcurrency.IsDone() { - return + // load from local fs if it's a tarball + if strings.HasSuffix(actual, ".tar") || strings.HasSuffix(actual, ".tar.gz") || strings.HasSuffix(actual, ".tgz") { + img, err = crane.Load(actual, opts...) + if err != nil { + return fmt.Errorf("unable to load image %s: %w", refInfo.Reference, err) + } + } else { + _, err = crane.Head(actual, opts...) + if err != nil { + if errors.Is(err, context.Canceled) { + return err + } + + message.Notef("Falling back to local 'docker' images, failed to find the manifest on a remote: %s", err.Error()) + + reference, err := name.ParseReference(actual) + if err != nil { + return fmt.Errorf("failed to parse image reference: %w", err) + } + + // Attempt to connect to the local docker daemon. + cli, err := client.NewClientWithOpts(client.FromEnv) + if err != nil { + return fmt.Errorf("docker not available: %w", err) + } + cli.NegotiateAPIVersion(ectx) + + // Inspect the image to get the size. + rawImg, _, err := cli.ImageInspectWithRaw(ctx, actual) + if err != nil { + if errdefs.IsNotFound(err) { + cancel() + } + + return fmt.Errorf("failed to inspect image via docker: %w", err) + } + + // Warn the user if the image is large. + if rawImg.Size > 750*1000*1000 { + message.Warnf("%s is %s and may take a very long time to load via docker. "+ + "See https://docs.zarf.dev/faq for suggestions on how to improve large local image loading operations.", + actual, utils.ByteFormat(float64(rawImg.Size), 2)) + } + + // Use unbuffered opener to avoid OOM Kill issues https://github.com/defenseunicorns/zarf/issues/1214. + // This will also take forever to load large images. + img, err = daemon.Image(reference, daemon.WithUnbufferedOpener(), daemon.WithContext(ectx)) + if err != nil { + return fmt.Errorf("failed to load image from docker daemon: %w", err) + } + } else { + img, err = crane.Pull(actual, opts...) + if err != nil { + return fmt.Errorf("unable to pull image %s: %w", refInfo.Reference, err) + } + } } - metadataImageConcurrency.ProgressChan <- ImgInfo{RefInfo: refInfo, Img: img, HasImageLayers: hasImageLayers} - }() - } - - onMetadataProgress := func(finishedImage ImgInfo, iteration int) { - spinner.Updatef("Fetching image metadata (%d of %d): %s", iteration+1, len(i.ImageList), finishedImage.RefInfo.Reference) - refInfoToImage[finishedImage.RefInfo] = finishedImage.Img - imgInfoList = append(imgInfoList, finishedImage) - } - - onMetadataError := func(err error) error { - return err - } - - if err := metadataImageConcurrency.WaitWithProgress(onMetadataProgress, onMetadataError); err != nil { - return nil, err - } + img = cache.Image(img, cache.NewFilesystemCache(filepath.Join(config.GetAbsCachePath(), layout.ImagesDir))) - // Create the ImagePath directory - if err := helpers.CreateDirectory(i.ImagesPath, helpers.ReadExecuteAllWriteUser); err != nil { - return nil, fmt.Errorf("failed to create image path %s: %w", i.ImagesPath, err) - } + manifest, err := img.Manifest() + if err != nil { + return fmt.Errorf("unable to get manifest for image %s: %w", refInfo.Reference, err) + } + totalBytes += manifest.Config.Size - totalBytes := int64(0) - processedLayers := make(map[string]v1.Layer) - for refInfo, img := range refInfoToImage { - // Get the byte size for this image - layers, err := img.Layers() - if err != nil { - return nil, fmt.Errorf("unable to get layers for image %s: %w", refInfo.Reference, err) - } - for _, layer := range layers { - layerDigest, err := layer.Digest() + layers, err := img.Layers() if err != nil { - return nil, fmt.Errorf("unable to get digest for image layer: %w", err) + return fmt.Errorf("unable to get layers for image %s: %w", refInfo.Reference, err) } - // Only calculate this layer size if we haven't already looked at it - if _, ok := processedLayers[layerDigest.Hex]; !ok { - size, err := layer.Size() + for _, layer := range layers { + digest, err := layer.Digest() if err != nil { - return nil, fmt.Errorf("unable to get size of layer: %w", err) + return fmt.Errorf("unable to get digest for image layer: %w", err) } - totalBytes += size - processedLayers[layerDigest.Hex] = layer - } - } - } - spinner.Updatef("Preparing image sources and cache for image pulling") + if _, ok := processed[digest.Hex]; !ok { + processed[digest.Hex] = layer + size, err := layer.Size() + if err != nil { + return fmt.Errorf("unable to get size for image layer: %w", err) + } + totalBytes += size + } + } - // Create special sauce crane Path object - // If it already exists use it - cranePath, err := clayout.FromPath(i.ImagesPath) - // Use crane pattern for creating OCI layout if it doesn't exist - if err != nil { - // If it doesn't exist create it - cranePath, err = clayout.Write(i.ImagesPath, empty.Index) - if err != nil { - return nil, err - } + refInfoToImage[refInfo] = img + list = append(list, ImgInfo{RefInfo: refInfo, Img: img}) + return nil + }) } - for refInfo, img := range refInfoToImage { - imgDigest, err := img.Digest() - if err != nil { - return nil, fmt.Errorf("unable to get digest for image %s: %w", refInfo.Reference, err) - } - referenceToDigest[refInfo.Reference] = imgDigest.String() + if err := eg.Wait(); err != nil { + return nil, err } - spinner.Success() - // Create a thread to update a progress bar as we save the image files to disk doneSaving := make(chan error) - updateText := fmt.Sprintf("Pulling %d images", imageCount) - go utils.RenderProgressBarForLocalDirWrite(i.ImagesPath, totalBytes, doneSaving, updateText, updateText) + updateText := fmt.Sprintf("Packaging %d images", imageCount) + go utils.RenderProgressBarForLocalDirWrite(dst.Base, totalBytes, doneSaving, updateText, updateText) - imageSavingConcurrency := helpers.NewConcurrencyTools[digestInfo, error](len(refInfoToImage)) + referenceToDigest := make(map[string]string) - defer imageSavingConcurrency.Cancel() + eg, ectx = errgroup.WithContext(ctx) + eg.SetLimit(3) // Spawn a goroutine for each image to write it's config and manifest to disk using crane for refInfo, img := range refInfoToImage { // Create a closure so that we can pass the refInfo and img into the goroutine refInfo, img := refInfo, img - go func() { + eg.Go(func() error { if err := cranePath.WriteImage(img); err != nil { // Check if the cache has been invalidated, and warn the user if so if strings.HasPrefix(err.Error(), "error writing layer: expected blob size") { message.Warnf("Potential image cache corruption: %s - try clearing cache with \"zarf tools clear-cache\"", err.Error()) } - imageSavingConcurrency.ErrorChan <- fmt.Errorf("error when trying to save the img (%s): %w", refInfo.Reference, err) - return - } - - if imageSavingConcurrency.IsDone() { - return + return fmt.Errorf("error when trying to save the img (%s): %w", refInfo.Reference, err) } - // Get the image digest so we can set an annotation in the image.json later - imgDigest, err := img.Digest() + desc, err := partial.Descriptor(img) if err != nil { - imageSavingConcurrency.ErrorChan <- err - return + return err } - if imageSavingConcurrency.IsDone() { - return + if err := cranePath.AppendDescriptor(*desc); err != nil { + return err } - imageSavingConcurrency.ProgressChan <- digestInfo{digest: imgDigest.String(), refInfo: refInfo} - }() - } - - onImageSavingProgress := func(finishedImage digestInfo, _ int) { - referenceToDigest[finishedImage.refInfo.Reference] = finishedImage.digest - } + imgDigest, err := img.Digest() + if err != nil { + return err + } - onImageSavingError := func(err error) error { - // Send a signal to the progress bar that we're done and wait for the thread to finish - doneSaving <- err - <-doneSaving - message.WarnErr(err, "Failed to write image config or manifest, trying again up to 3 times...") - return err + referenceToDigest[refInfo.Reference] = imgDigest.String() + return nil + }) } - if err := imageSavingConcurrency.WaitWithProgress(onImageSavingProgress, onImageSavingError); err != nil { + if err := eg.Wait(); err != nil { return nil, err } - // for every image sequentially append OCI descriptor - for refInfo, img := range refInfoToImage { - desc, err := partial.Descriptor(img) - if err != nil { - return nil, err - } - - if err := cranePath.AppendDescriptor(*desc); err != nil { - return nil, err - } - - imgDigest, err := img.Digest() - if err != nil { - return nil, err - } - - referenceToDigest[refInfo.Reference] = imgDigest.String() - } - - if err := utils.AddImageNameAnnotation(i.ImagesPath, referenceToDigest); err != nil { + if err := utils.AddImageNameAnnotation(dst.Base, referenceToDigest); err != nil { return nil, fmt.Errorf("unable to format OCI layout: %w", err) } @@ -258,74 +243,5 @@ func (i *ImageConfig) PullAll() ([]ImgInfo, error) { doneSaving <- nil <-doneSaving - return imgInfoList, nil -} - -// PullImage returns a v1.Image either by loading a local tarball or pulling from the wider internet. -func (i *ImageConfig) PullImage(src string, spinner *message.Spinner) (img v1.Image, hasImageLayers bool, err error) { - cacheImage := false - // Load image tarballs from the local filesystem. - if strings.HasSuffix(src, ".tar") || strings.HasSuffix(src, ".tar.gz") || strings.HasSuffix(src, ".tgz") { - spinner.Updatef("Reading image tarball: %s", src) - img, err = crane.Load(src, config.GetCraneOptions(true, i.Architectures...)...) - if err != nil { - return nil, false, err - } - } else if _, err := crane.Manifest(src, config.GetCraneOptions(i.Insecure, i.Architectures...)...); err != nil { - // If crane is unable to pull the image, try to load it from the local docker daemon. - message.Notef("Falling back to local 'docker' images, failed to find the manifest on a remote: %s", err.Error()) - - // Parse the image reference to get the image name. - reference, err := name.ParseReference(src) - if err != nil { - return nil, false, fmt.Errorf("failed to parse image reference: %w", err) - } - - // Attempt to connect to the local docker daemon. - ctx := context.TODO() - cli, err := client.NewClientWithOpts(client.FromEnv) - if err != nil { - return nil, false, fmt.Errorf("docker not available: %w", err) - } - cli.NegotiateAPIVersion(ctx) - - // Inspect the image to get the size. - rawImg, _, err := cli.ImageInspectWithRaw(ctx, src) - if err != nil { - return nil, false, fmt.Errorf("failed to inspect image via docker: %w", err) - } - - // Warn the user if the image is large. - if rawImg.Size > 750*1000*1000 { - message.Warnf("%s is %s and may take a very long time to load via docker. "+ - "See https://docs.zarf.dev/docs/faq for suggestions on how to improve large local image loading operations.", - src, utils.ByteFormat(float64(rawImg.Size), 2)) - } - - // Use unbuffered opener to avoid OOM Kill issues https://github.com/defenseunicorns/zarf/issues/1214. - // This will also take for ever to load large images. - if img, err = daemon.Image(reference, daemon.WithUnbufferedOpener()); err != nil { - return nil, false, fmt.Errorf("failed to load image from docker daemon: %w", err) - } - } else { - // Manifest was found, so use crane to pull the image. - if img, err = crane.Pull(src, config.GetCraneOptions(i.Insecure, i.Architectures...)...); err != nil { - return nil, false, fmt.Errorf("failed to pull image: %w", err) - } - cacheImage = true - } - - hasImageLayers, err = utils.HasImageLayers(img) - if err != nil { - return nil, false, fmt.Errorf("failed to check image layer mediatype: %w", err) - } - - if hasImageLayers && cacheImage { - spinner.Updatef("Preparing image %s", src) - imageCachePath := filepath.Join(config.GetAbsCachePath(), layout.ImagesDir) - img = cache.Image(img, cache.NewFilesystemCache(imageCachePath)) - } - - return img, hasImageLayers, nil - + return list, nil } diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 9e742d6cee..81f90afebd 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -179,14 +179,13 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. doPull := func() error { imgConfig := images.ImageConfig{ - ImagesPath: dst.Images.Base, ImageList: imageList, Insecure: config.CommonOptions.Insecure, Architectures: []string{arch}, RegistryOverrides: pc.createOpts.RegistryOverrides, } - pulled, err = imgConfig.PullAll() + pulled, err = imgConfig.PullAll(dst.Images) return err } @@ -198,7 +197,11 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. if err := dst.Images.AddV1Image(imgInfo.Img); err != nil { return err } - if imgInfo.HasImageLayers { + ok, err := utils.HasImageLayers(imgInfo.Img) + if err != nil { + return fmt.Errorf("failed to validate %s is an image and not an artifact: %w", imgInfo, err) + } + if ok { sbomImageList = append(sbomImageList, imgInfo.RefInfo) } } diff --git a/src/pkg/utils/image.go b/src/pkg/utils/image.go index 5a05d987df..d548e9e8bc 100644 --- a/src/pkg/utils/image.go +++ b/src/pkg/utils/image.go @@ -48,13 +48,12 @@ func LoadOCIImage(imgPath string, refInfo transform.Image) (v1.Image, error) { func AddImageNameAnnotation(ociPath string, referenceToDigest map[string]string) error { indexPath := filepath.Join(ociPath, "index.json") - // Read the file contents and turn it into a usable struct that we can manipulate var index ocispec.Index - byteValue, err := os.ReadFile(indexPath) + b, err := os.ReadFile(indexPath) if err != nil { return fmt.Errorf("unable to read the contents of the file (%s) so we can add an annotation: %w", indexPath, err) } - if err = json.Unmarshal(byteValue, &index); err != nil { + if err = json.Unmarshal(b, &index); err != nil { return fmt.Errorf("unable to process the contents of the file (%s): %w", indexPath, err) } @@ -80,11 +79,11 @@ func AddImageNameAnnotation(ociPath string, referenceToDigest map[string]string) } // Write the file back to the package - indexJSONBytes, err := json.Marshal(index) + b, err = json.Marshal(index) if err != nil { return err } - return os.WriteFile(indexPath, indexJSONBytes, helpers.ReadWriteUser) + return os.WriteFile(indexPath, b, helpers.ReadWriteUser) } // HasImageLayers checks if any layers in the v1.Image are known image layers. From c4c72c184a421a237787195762a3a26953df45a3 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 25 Apr 2024 00:17:57 -0500 Subject: [PATCH 02/43] mutex and context Signed-off-by: razzle --- go.mod | 4 ++-- src/internal/packager/images/pull.go | 25 ++++++++++++++++++------- src/pkg/packager/creator/normal.go | 24 +++++++++--------------- 3 files changed, 29 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index e09830fca2..023e2973d8 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,7 @@ require ( github.com/defenseunicorns/pkg/oci v0.0.1 github.com/derailed/k9s v0.31.7 github.com/distribution/reference v0.5.0 + github.com/docker/docker v24.0.9+incompatible github.com/fairwindsops/pluto/v5 v5.18.4 github.com/fatih/color v1.16.0 github.com/fluxcd/helm-controller/api v0.37.4 @@ -201,7 +202,6 @@ require ( github.com/dimchansky/utfbom v1.1.1 // indirect github.com/docker/cli v26.0.0+incompatible // indirect github.com/docker/distribution v2.8.3+incompatible // indirect - github.com/docker/docker v24.0.9+incompatible // indirect github.com/docker/docker-credential-helpers v0.8.0 // indirect github.com/docker/go-connections v0.4.0 // indirect github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect @@ -472,7 +472,7 @@ require ( golang.org/x/mod v0.14.0 // indirect golang.org/x/net v0.22.0 // indirect golang.org/x/oauth2 v0.17.0 // indirect - golang.org/x/sync v0.6.0 // indirect + golang.org/x/sync v0.6.0 golang.org/x/sys v0.18.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.5.0 // indirect diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index d9b92b7704..c97619ace7 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -10,6 +10,7 @@ import ( "fmt" "path/filepath" "strings" + "sync" "github.com/defenseunicorns/pkg/helpers" "github.com/defenseunicorns/zarf/src/config" @@ -38,7 +39,7 @@ type ImgInfo struct { } // PullAll pulls all of the images in the provided tag map. -func (i *ImageConfig) PullAll(dst layout.Images) (list []ImgInfo, err error) { +func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []ImgInfo, err error) { var longer string imageCount := len(i.ImageList) @@ -61,19 +62,20 @@ func (i *ImageConfig) PullAll(dst layout.Images) (list []ImgInfo, err error) { } } - spinner := message.NewProgressSpinner("Fetching %d images. %s", imageCount, longer) + spinner := message.NewProgressSpinner("Fetching info for %d images. %s", imageCount, longer) defer spinner.Stop() logs.Warn.SetOutput(&message.DebugWriter{}) logs.Progress.SetOutput(&message.DebugWriter{}) - ctx := context.TODO() ctx, cancel := context.WithCancel(ctx) eg, ectx := errgroup.WithContext(ctx) defer cancel() eg.SetLimit(10) + // refInfoToImage := make(map[transform.Image]v1.Image) refInfoToImage := make(map[transform.Image]v1.Image) + var mu sync.Mutex totalBytes := int64(0) processed := make(map[string]v1.Layer) opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) @@ -81,7 +83,7 @@ func (i *ImageConfig) PullAll(dst layout.Images) (list []ImgInfo, err error) { for idx, refInfo := range i.ImageList { refInfo, idx := refInfo, idx eg.Go(func() error { - spinner.Updatef("Fetching image (%d of %d)", idx+1, len(i.ImageList)) + spinner.Updatef("Fetching image info (%d of %d)", idx+1, len(i.ImageList)) actual := refInfo.Reference for k, v := range i.RegistryOverrides { @@ -105,6 +107,11 @@ func (i *ImageConfig) PullAll(dst layout.Images) (list []ImgInfo, err error) { return err } + if strings.Contains(err.Error(), "unexpected status code 429 Too Many Requests") { + cancel() + return fmt.Errorf("rate limited by registry: %w", err) + } + message.Notef("Falling back to local 'docker' images, failed to find the manifest on a remote: %s", err.Error()) reference, err := name.ParseReference(actual) @@ -120,7 +127,7 @@ func (i *ImageConfig) PullAll(dst layout.Images) (list []ImgInfo, err error) { cli.NegotiateAPIVersion(ectx) // Inspect the image to get the size. - rawImg, _, err := cli.ImageInspectWithRaw(ctx, actual) + rawImg, _, err := cli.ImageInspectWithRaw(ectx, actual) if err != nil { if errdefs.IsNotFound(err) { cancel() @@ -179,7 +186,9 @@ func (i *ImageConfig) PullAll(dst layout.Images) (list []ImgInfo, err error) { } } + mu.Lock() refInfoToImage[refInfo] = img + mu.Unlock() list = append(list, ImgInfo{RefInfo: refInfo, Img: img}) return nil }) @@ -191,13 +200,13 @@ func (i *ImageConfig) PullAll(dst layout.Images) (list []ImgInfo, err error) { spinner.Success() // Create a thread to update a progress bar as we save the image files to disk doneSaving := make(chan error) - updateText := fmt.Sprintf("Packaging %d images", imageCount) + updateText := fmt.Sprintf("Pulling %d images", imageCount) go utils.RenderProgressBarForLocalDirWrite(dst.Base, totalBytes, doneSaving, updateText, updateText) referenceToDigest := make(map[string]string) eg, ectx = errgroup.WithContext(ctx) - eg.SetLimit(3) + eg.SetLimit(10) // Spawn a goroutine for each image to write it's config and manifest to disk using crane for refInfo, img := range refInfoToImage { @@ -226,7 +235,9 @@ func (i *ImageConfig) PullAll(dst layout.Images) (list []ImgInfo, err error) { return err } + mu.Lock() referenceToDigest[refInfo.Reference] = imgDigest.String() + mu.Unlock() return nil }) } diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 81f90afebd..d368c622d2 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -12,7 +12,6 @@ import ( "path/filepath" "strconv" "strings" - "time" "github.com/defenseunicorns/pkg/helpers" "github.com/defenseunicorns/pkg/oci" @@ -174,23 +173,18 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. dst.AddImages() - var pulled []images.ImgInfo - var err error - - doPull := func() error { - imgConfig := images.ImageConfig{ - ImageList: imageList, - Insecure: config.CommonOptions.Insecure, - Architectures: []string{arch}, - RegistryOverrides: pc.createOpts.RegistryOverrides, - } + ctx := context.TODO() - pulled, err = imgConfig.PullAll(dst.Images) - return err + imgConfig := images.ImageConfig{ + ImageList: imageList, + Insecure: config.CommonOptions.Insecure, + Architectures: []string{arch}, + RegistryOverrides: pc.createOpts.RegistryOverrides, } - if err := helpers.Retry(doPull, 3, 5*time.Second, message.Warnf); err != nil { - return fmt.Errorf("unable to pull images after 3 attempts: %w", err) + pulled, err := imgConfig.PullAll(ctx, dst.Images) + if err != nil { + return err } for _, imgInfo := range pulled { From 3372f321f8cf6bf878a3a0617c4a3658ebe6f878 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 25 Apr 2024 02:13:12 -0500 Subject: [PATCH 03/43] simpler annotations Signed-off-by: razzle --- src/internal/packager/images/pull.go | 34 +++++++--------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index c97619ace7..db7d161e85 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -27,8 +27,8 @@ import ( "github.com/google/go-containerregistry/pkg/v1/daemon" "github.com/google/go-containerregistry/pkg/v1/empty" clayout "github.com/google/go-containerregistry/pkg/v1/layout" - "github.com/google/go-containerregistry/pkg/v1/partial" "github.com/moby/moby/client" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "golang.org/x/sync/errgroup" ) @@ -69,8 +69,8 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im logs.Progress.SetOutput(&message.DebugWriter{}) ctx, cancel := context.WithCancel(ctx) - eg, ectx := errgroup.WithContext(ctx) defer cancel() + eg, ectx := errgroup.WithContext(ctx) eg.SetLimit(10) // refInfoToImage := make(map[transform.Image]v1.Image) @@ -203,8 +203,6 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im updateText := fmt.Sprintf("Pulling %d images", imageCount) go utils.RenderProgressBarForLocalDirWrite(dst.Base, totalBytes, doneSaving, updateText, updateText) - referenceToDigest := make(map[string]string) - eg, ectx = errgroup.WithContext(ctx) eg.SetLimit(10) @@ -213,7 +211,12 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im // Create a closure so that we can pass the refInfo and img into the goroutine refInfo, img := refInfo, img eg.Go(func() error { - if err := cranePath.WriteImage(img); err != nil { + annotations := map[string]string{ + ocispec.AnnotationBaseImageName: refInfo.Reference, + } + + // also have clayout.WithPlatform() as a future option to use + if err := cranePath.AppendImage(img, clayout.WithAnnotations(annotations)); err != nil { // Check if the cache has been invalidated, and warn the user if so if strings.HasPrefix(err.Error(), "error writing layer: expected blob size") { message.Warnf("Potential image cache corruption: %s - try clearing cache with \"zarf tools clear-cache\"", err.Error()) @@ -221,23 +224,6 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im return fmt.Errorf("error when trying to save the img (%s): %w", refInfo.Reference, err) } - desc, err := partial.Descriptor(img) - if err != nil { - return err - } - - if err := cranePath.AppendDescriptor(*desc); err != nil { - return err - } - - imgDigest, err := img.Digest() - if err != nil { - return err - } - - mu.Lock() - referenceToDigest[refInfo.Reference] = imgDigest.String() - mu.Unlock() return nil }) } @@ -246,10 +232,6 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im return nil, err } - if err := utils.AddImageNameAnnotation(dst.Base, referenceToDigest); err != nil { - return nil, fmt.Errorf("unable to format OCI layout: %w", err) - } - // Send a signal to the progress bar that we're done and wait for the thread to finish doneSaving <- nil <-doneSaving From ad3bc4da2c765ffaab42f0b0ffbf2fc4a0432e34 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 25 Apr 2024 02:22:33 -0500 Subject: [PATCH 04/43] simplify Signed-off-by: razzle --- src/internal/packager/images/pull.go | 43 +++++++++++++--------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index db7d161e85..a06a5915ba 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -73,11 +73,9 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im eg, ectx := errgroup.WithContext(ctx) eg.SetLimit(10) - // refInfoToImage := make(map[transform.Image]v1.Image) - refInfoToImage := make(map[transform.Image]v1.Image) var mu sync.Mutex totalBytes := int64(0) - processed := make(map[string]v1.Layer) + processed := make(map[string]bool) opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) for idx, refInfo := range i.ImageList { @@ -85,23 +83,27 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im eg.Go(func() error { spinner.Updatef("Fetching image info (%d of %d)", idx+1, len(i.ImageList)) - actual := refInfo.Reference + ref := refInfo.Reference for k, v := range i.RegistryOverrides { if strings.HasPrefix(refInfo.Reference, k) { - actual = strings.Replace(refInfo.Reference, k, v, 1) + ref = strings.Replace(refInfo.Reference, k, v, 1) } } var img v1.Image // load from local fs if it's a tarball - if strings.HasSuffix(actual, ".tar") || strings.HasSuffix(actual, ".tar.gz") || strings.HasSuffix(actual, ".tgz") { - img, err = crane.Load(actual, opts...) + if strings.HasSuffix(ref, ".tar") || strings.HasSuffix(ref, ".tar.gz") || strings.HasSuffix(ref, ".tgz") { + img, err = crane.Load(ref, opts...) if err != nil { return fmt.Errorf("unable to load image %s: %w", refInfo.Reference, err) } } else { - _, err = crane.Head(actual, opts...) + reference, err := name.ParseReference(ref) + if err != nil { + return fmt.Errorf("failed to parse image reference: %w", err) + } + _, err = crane.Head(ref, opts...) if err != nil { if errors.Is(err, context.Canceled) { return err @@ -114,11 +116,6 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im message.Notef("Falling back to local 'docker' images, failed to find the manifest on a remote: %s", err.Error()) - reference, err := name.ParseReference(actual) - if err != nil { - return fmt.Errorf("failed to parse image reference: %w", err) - } - // Attempt to connect to the local docker daemon. cli, err := client.NewClientWithOpts(client.FromEnv) if err != nil { @@ -127,7 +124,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im cli.NegotiateAPIVersion(ectx) // Inspect the image to get the size. - rawImg, _, err := cli.ImageInspectWithRaw(ectx, actual) + rawImg, _, err := cli.ImageInspectWithRaw(ectx, ref) if err != nil { if errdefs.IsNotFound(err) { cancel() @@ -140,7 +137,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im if rawImg.Size > 750*1000*1000 { message.Warnf("%s is %s and may take a very long time to load via docker. "+ "See https://docs.zarf.dev/faq for suggestions on how to improve large local image loading operations.", - actual, utils.ByteFormat(float64(rawImg.Size), 2)) + ref, utils.ByteFormat(float64(rawImg.Size), 2)) } // Use unbuffered opener to avoid OOM Kill issues https://github.com/defenseunicorns/zarf/issues/1214. @@ -150,7 +147,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im return fmt.Errorf("failed to load image from docker daemon: %w", err) } } else { - img, err = crane.Pull(actual, opts...) + img, err = crane.Pull(ref, opts...) if err != nil { return fmt.Errorf("unable to pull image %s: %w", refInfo.Reference, err) } @@ -177,7 +174,9 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im } if _, ok := processed[digest.Hex]; !ok { - processed[digest.Hex] = layer + mu.Lock() + processed[digest.Hex] = true + mu.Unlock() size, err := layer.Size() if err != nil { return fmt.Errorf("unable to get size for image layer: %w", err) @@ -186,9 +185,6 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im } } - mu.Lock() - refInfoToImage[refInfo] = img - mu.Unlock() list = append(list, ImgInfo{RefInfo: refInfo, Img: img}) return nil }) @@ -197,8 +193,9 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im if err := eg.Wait(); err != nil { return nil, err } + spinner.Success() - // Create a thread to update a progress bar as we save the image files to disk + doneSaving := make(chan error) updateText := fmt.Sprintf("Pulling %d images", imageCount) go utils.RenderProgressBarForLocalDirWrite(dst.Base, totalBytes, doneSaving, updateText, updateText) @@ -207,9 +204,9 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im eg.SetLimit(10) // Spawn a goroutine for each image to write it's config and manifest to disk using crane - for refInfo, img := range refInfoToImage { + for _, info := range list { // Create a closure so that we can pass the refInfo and img into the goroutine - refInfo, img := refInfo, img + refInfo, img := info.RefInfo, info.Img eg.Go(func() error { annotations := map[string]string{ ocispec.AnnotationBaseImageName: refInfo.Reference, From 24691a4e4e4583871e8f9de0bf8b4f21888f0220 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 25 Apr 2024 03:20:14 -0500 Subject: [PATCH 05/43] fix Signed-off-by: razzle --- src/internal/packager/images/pull.go | 27 +++++++++++++-------------- src/pkg/packager/creator/normal.go | 4 ++-- src/pkg/utils/image.go | 9 ++++----- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index a06a5915ba..d03e8b4e49 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -6,7 +6,6 @@ package images import ( "context" - "errors" "fmt" "path/filepath" "strings" @@ -39,7 +38,7 @@ type ImgInfo struct { } // PullAll pulls all of the images in the provided tag map. -func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []ImgInfo, err error) { +func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, dst layout.Images) (list []ImgInfo, err error) { var longer string imageCount := len(i.ImageList) @@ -68,9 +67,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im logs.Warn.SetOutput(&message.DebugWriter{}) logs.Progress.SetOutput(&message.DebugWriter{}) - ctx, cancel := context.WithCancel(ctx) - defer cancel() - eg, ectx := errgroup.WithContext(ctx) + eg, _ := errgroup.WithContext(ctx) eg.SetLimit(10) var mu sync.Mutex @@ -78,6 +75,12 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im processed := make(map[string]bool) opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) + // retry := func(cb func() error) func() error { + // return func() error { + // return helpers.Retry(cb, 3, 5*time.Second, message.Warnf) + // } + // } + for idx, refInfo := range i.ImageList { refInfo, idx := refInfo, idx eg.Go(func() error { @@ -105,10 +108,6 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im } _, err = crane.Head(ref, opts...) if err != nil { - if errors.Is(err, context.Canceled) { - return err - } - if strings.Contains(err.Error(), "unexpected status code 429 Too Many Requests") { cancel() return fmt.Errorf("rate limited by registry: %w", err) @@ -121,10 +120,10 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im if err != nil { return fmt.Errorf("docker not available: %w", err) } - cli.NegotiateAPIVersion(ectx) + cli.NegotiateAPIVersion(ctx) // Inspect the image to get the size. - rawImg, _, err := cli.ImageInspectWithRaw(ectx, ref) + rawImg, _, err := cli.ImageInspectWithRaw(ctx, ref) if err != nil { if errdefs.IsNotFound(err) { cancel() @@ -142,7 +141,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im // Use unbuffered opener to avoid OOM Kill issues https://github.com/defenseunicorns/zarf/issues/1214. // This will also take forever to load large images. - img, err = daemon.Image(reference, daemon.WithUnbufferedOpener(), daemon.WithContext(ectx)) + img, err = daemon.Image(reference, daemon.WithUnbufferedOpener(), daemon.WithContext(ctx)) if err != nil { return fmt.Errorf("failed to load image from docker daemon: %w", err) } @@ -194,13 +193,13 @@ func (i *ImageConfig) PullAll(ctx context.Context, dst layout.Images) (list []Im return nil, err } - spinner.Success() + spinner.Successf("Fetched info for %d images", imageCount) doneSaving := make(chan error) updateText := fmt.Sprintf("Pulling %d images", imageCount) go utils.RenderProgressBarForLocalDirWrite(dst.Base, totalBytes, doneSaving, updateText, updateText) - eg, ectx = errgroup.WithContext(ctx) + eg, _ = errgroup.WithContext(ctx) eg.SetLimit(10) // Spawn a goroutine for each image to write it's config and manifest to disk using crane diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index d368c622d2..2812da25aa 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -173,7 +173,7 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. dst.AddImages() - ctx := context.TODO() + ctx, cancel := context.WithCancel(context.TODO()) imgConfig := images.ImageConfig{ ImageList: imageList, @@ -182,7 +182,7 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. RegistryOverrides: pc.createOpts.RegistryOverrides, } - pulled, err := imgConfig.PullAll(ctx, dst.Images) + pulled, err := imgConfig.PullAll(ctx, cancel, dst.Images) if err != nil { return err } diff --git a/src/pkg/utils/image.go b/src/pkg/utils/image.go index d548e9e8bc..0074ea9036 100644 --- a/src/pkg/utils/image.go +++ b/src/pkg/utils/image.go @@ -86,7 +86,7 @@ func AddImageNameAnnotation(ociPath string, referenceToDigest map[string]string) return os.WriteFile(indexPath, b, helpers.ReadWriteUser) } -// HasImageLayers checks if any layers in the v1.Image are known image layers. +// HasImageLayers checks if all layers in the v1.Image are known image layers. func HasImageLayers(img v1.Image) (bool, error) { layers, err := img.Layers() if err != nil { @@ -97,10 +97,9 @@ func HasImageLayers(img v1.Image) (bool, error) { if err != nil { return false, err } - // Check if mediatype is a known image layer - if mediatype.IsLayer() { - return true, nil + if !mediatype.IsLayer() { + return false, nil } } - return false, nil + return true, nil } From df6a74cf6a26e31462d2d0bfb6c06a5a82bb6141 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 25 Apr 2024 04:26:39 -0500 Subject: [PATCH 06/43] block while layers are in progress, might revert Signed-off-by: razzle --- src/internal/packager/images/pull.go | 82 ++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 18 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index d03e8b4e49..50b3ca36bb 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -6,6 +6,7 @@ package images import ( "context" + "errors" "fmt" "path/filepath" "strings" @@ -72,7 +73,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds var mu sync.Mutex totalBytes := int64(0) - processed := make(map[string]bool) + processing := make(map[string]bool) opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) // retry := func(cb func() error) func() error { @@ -99,12 +100,12 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds if strings.HasSuffix(ref, ".tar") || strings.HasSuffix(ref, ".tar.gz") || strings.HasSuffix(ref, ".tgz") { img, err = crane.Load(ref, opts...) if err != nil { - return fmt.Errorf("unable to load image %s: %w", refInfo.Reference, err) + return fmt.Errorf("unable to load %s: %w", refInfo.Reference, err) } } else { reference, err := name.ParseReference(ref) if err != nil { - return fmt.Errorf("failed to parse image reference: %w", err) + return fmt.Errorf("failed to parse reference: %w", err) } _, err = crane.Head(ref, opts...) if err != nil { @@ -113,7 +114,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return fmt.Errorf("rate limited by registry: %w", err) } - message.Notef("Falling back to local 'docker' images, failed to find the manifest on a remote: %s", err.Error()) + message.Notef("Falling back to local 'docker', failed to find the manifest on a remote: %s", err.Error()) // Attempt to connect to the local docker daemon. cli, err := client.NewClientWithOpts(client.FromEnv) @@ -129,7 +130,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds cancel() } - return fmt.Errorf("failed to inspect image via docker: %w", err) + return fmt.Errorf("failed to inspect via docker: %w", err) } // Warn the user if the image is large. @@ -143,7 +144,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds // This will also take forever to load large images. img, err = daemon.Image(reference, daemon.WithUnbufferedOpener(), daemon.WithContext(ctx)) if err != nil { - return fmt.Errorf("failed to load image from docker daemon: %w", err) + return fmt.Errorf("failed to load from docker daemon: %w", err) } } else { img, err = crane.Pull(ref, opts...) @@ -157,13 +158,13 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds manifest, err := img.Manifest() if err != nil { - return fmt.Errorf("unable to get manifest for image %s: %w", refInfo.Reference, err) + return fmt.Errorf("unable to get manifest for %s: %w", refInfo.Reference, err) } totalBytes += manifest.Config.Size layers, err := img.Layers() if err != nil { - return fmt.Errorf("unable to get layers for image %s: %w", refInfo.Reference, err) + return fmt.Errorf("unable to get layers for %s: %w", refInfo.Reference, err) } for _, layer := range layers { @@ -172,9 +173,9 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return fmt.Errorf("unable to get digest for image layer: %w", err) } - if _, ok := processed[digest.Hex]; !ok { + if _, ok := processing[digest.Hex]; !ok { mu.Lock() - processed[digest.Hex] = true + processing[digest.Hex] = true mu.Unlock() size, err := layer.Size() if err != nil { @@ -189,6 +190,8 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds }) } + clear(processing) + if err := eg.Wait(); err != nil { return nil, err } @@ -202,25 +205,68 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds eg, _ = errgroup.WithContext(ctx) eg.SetLimit(10) - // Spawn a goroutine for each image to write it's config and manifest to disk using crane + layerInProgress := errors.New("layer already inprogress") + + markAsProcessing := func(layers []v1.Layer) error { + mu.Lock() + defer mu.Unlock() + for _, layer := range layers { + digest, err := layer.Digest() + if err != nil { + return err + } + + if _, ok := processing[digest.Hex]; ok { + return layerInProgress + } + processing[digest.Hex] = true + } + return nil + } + + unmarkAsProcessing := func(layers []v1.Layer) error { + mu.Lock() + defer mu.Unlock() + for _, layer := range layers { + digest, err := layer.Digest() + if err != nil { + return err + } + delete(processing, digest.Hex) + } + return nil + } + for _, info := range list { - // Create a closure so that we can pass the refInfo and img into the goroutine refInfo, img := info.RefInfo, info.Img eg.Go(func() error { + + layers, err := img.Layers() + if err != nil { + return fmt.Errorf("unable to get layers for %s: %w", refInfo.Reference, err) + } + + for { + if err := markAsProcessing(layers); err != nil { + if errors.Is(err, layerInProgress) { + continue + } + return err + } + break + } + + message.Debug("Pulling image %s", refInfo.Reference) annotations := map[string]string{ ocispec.AnnotationBaseImageName: refInfo.Reference, } // also have clayout.WithPlatform() as a future option to use if err := cranePath.AppendImage(img, clayout.WithAnnotations(annotations)); err != nil { - // Check if the cache has been invalidated, and warn the user if so - if strings.HasPrefix(err.Error(), "error writing layer: expected blob size") { - message.Warnf("Potential image cache corruption: %s - try clearing cache with \"zarf tools clear-cache\"", err.Error()) - } - return fmt.Errorf("error when trying to save the img (%s): %w", refInfo.Reference, err) + return fmt.Errorf("error when trying to save %s: %w", refInfo.Reference, err) } - return nil + return unmarkAsProcessing(layers) }) } From e99e16c76d017b0f1d6156907c86e7a52d47aeb1 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 25 Apr 2024 04:52:42 -0500 Subject: [PATCH 07/43] fix inifinite blocking Signed-off-by: razzle --- src/internal/packager/images/pull.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 50b3ca36bb..1f8461a0c5 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -11,6 +11,7 @@ import ( "path/filepath" "strings" "sync" + "time" "github.com/defenseunicorns/pkg/helpers" "github.com/defenseunicorns/zarf/src/config" @@ -190,12 +191,12 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds }) } - clear(processing) - if err := eg.Wait(); err != nil { return nil, err } + clear(processing) + spinner.Successf("Fetched info for %d images", imageCount) doneSaving := make(chan error) @@ -249,6 +250,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds for { if err := markAsProcessing(layers); err != nil { if errors.Is(err, layerInProgress) { + time.Sleep(1 * time.Second) continue } return err From 83b0b625f2b6d2ac5803d8a324f915855e383d40 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 25 Apr 2024 15:40:14 -0500 Subject: [PATCH 08/43] debug Signed-off-by: razzle --- src/internal/packager/images/pull.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 1f8461a0c5..ef919b5e4f 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -11,7 +11,6 @@ import ( "path/filepath" "strings" "sync" - "time" "github.com/defenseunicorns/pkg/helpers" "github.com/defenseunicorns/zarf/src/config" @@ -218,6 +217,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds } if _, ok := processing[digest.Hex]; ok { + message.Debug(helpers.Truncate(digest.Hex, 12, false), "in progress, skipping.") return layerInProgress } processing[digest.Hex] = true @@ -250,7 +250,8 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds for { if err := markAsProcessing(layers); err != nil { if errors.Is(err, layerInProgress) { - time.Sleep(1 * time.Second) + // message.Debug(processing) + // time.Sleep(1 * time.Second) continue } return err @@ -258,7 +259,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds break } - message.Debug("Pulling image %s", refInfo.Reference) + message.Debugf("Pulling image %s", refInfo.Reference) annotations := map[string]string{ ocispec.AnnotationBaseImageName: refInfo.Reference, } From 9176ed5d8c19c63c7cabcc91107c267d4a6555f7 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 25 Apr 2024 17:16:39 -0500 Subject: [PATCH 09/43] use channels Signed-off-by: razzle --- src/internal/packager/images/pull.go | 94 +++++++++++++--------------- src/pkg/packager/creator/normal.go | 5 ++ 2 files changed, 50 insertions(+), 49 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index ef919b5e4f..50a7e32193 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -6,7 +6,6 @@ package images import ( "context" - "errors" "fmt" "path/filepath" "strings" @@ -73,7 +72,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds var mu sync.Mutex totalBytes := int64(0) - processing := make(map[string]bool) + shas := make(map[string]bool) opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) // retry := func(cb func() error) func() error { @@ -173,9 +172,9 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return fmt.Errorf("unable to get digest for image layer: %w", err) } - if _, ok := processing[digest.Hex]; !ok { + if _, ok := shas[digest.Hex]; !ok { mu.Lock() - processing[digest.Hex] = true + shas[digest.Hex] = true mu.Unlock() size, err := layer.Size() if err != nil { @@ -194,7 +193,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return nil, err } - clear(processing) + clear(shas) spinner.Successf("Fetched info for %d images", imageCount) @@ -205,58 +204,43 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds eg, _ = errgroup.WithContext(ctx) eg.SetLimit(10) - layerInProgress := errors.New("layer already inprogress") - - markAsProcessing := func(layers []v1.Layer) error { - mu.Lock() - defer mu.Unlock() - for _, layer := range layers { - digest, err := layer.Digest() - if err != nil { - return err - } - - if _, ok := processing[digest.Hex]; ok { - message.Debug(helpers.Truncate(digest.Hex, 12, false), "in progress, skipping.") - return layerInProgress - } - processing[digest.Hex] = true - } - return nil - } - - unmarkAsProcessing := func(layers []v1.Layer) error { - mu.Lock() - defer mu.Unlock() - for _, layer := range layers { - digest, err := layer.Digest() - if err != nil { - return err - } - delete(processing, digest.Hex) - } - return nil - } - + processing := make(map[string]chan struct{}) for _, info := range list { - refInfo, img := info.RefInfo, info.Img + info := info + refInfo := info.RefInfo + img := info.Img eg.Go(func() error { - layers, err := img.Layers() if err != nil { return fmt.Errorf("unable to get layers for %s: %w", refInfo.Reference, err) } - for { - if err := markAsProcessing(layers); err != nil { - if errors.Is(err, layerInProgress) { - // message.Debug(processing) - // time.Sleep(1 * time.Second) - continue - } + layerChans := make([]chan struct{}, len(layers)) + mu.Lock() + for i, layer := range layers { + digest, err := layer.Digest() + if err != nil { + mu.Unlock() return err } - break + + if ch, ok := processing[digest.Hex]; !ok { + // If no channel exists, create one + ch = make(chan struct{}) + processing[digest.Hex] = ch + } else { + // If channel exists, prepare to wait on it + layerChans[i] = ch + } + } + mu.Unlock() + + // Wait for all required layers to be available + for idx, ch := range layerChans { + if ch != nil { + message.Debug(refInfo.Reference, "waiting for layer to be processed", idx) + <-ch // Wait for the channel to be closed + } } message.Debugf("Pulling image %s", refInfo.Reference) @@ -269,7 +253,19 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return fmt.Errorf("error when trying to save %s: %w", refInfo.Reference, err) } - return unmarkAsProcessing(layers) + // Signal that processing is done by closing channels + mu.Lock() + for _, layer := range layers { + digest, _ := layer.Digest() + ch, exists := processing[digest.Hex] + if exists { + close(ch) + delete(processing, digest.Hex) // Remove the channel from the map + } + } + mu.Unlock() + + return nil }) } diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 2812da25aa..8ebb5ced36 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -8,10 +8,12 @@ import ( "context" "errors" "fmt" + "math/rand" "os" "path/filepath" "strconv" "strings" + "time" "github.com/defenseunicorns/pkg/helpers" "github.com/defenseunicorns/pkg/oci" @@ -165,6 +167,9 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. } imageList = helpers.Unique(imageList) + rs := rand.NewSource(time.Now().UnixNano()) + rnd := rand.New(rs) + rnd.Shuffle(len(imageList), func(i, j int) { imageList[i], imageList[j] = imageList[j], imageList[i] }) var sbomImageList []transform.Image // Images are handled separately from other component assets. From e24e275ea4742a0ff66d19744ed8b6e6287859e9 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 25 Apr 2024 21:29:00 -0500 Subject: [PATCH 10/43] try parallel first, fallback to sequential Signed-off-by: razzle --- src/internal/packager/images/pull.go | 145 +++++++++++++++------------ 1 file changed, 81 insertions(+), 64 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 50a7e32193..fe659de852 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strings" "sync" + "time" "github.com/defenseunicorns/pkg/helpers" "github.com/defenseunicorns/zarf/src/config" @@ -26,6 +27,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/daemon" "github.com/google/go-containerregistry/pkg/v1/empty" clayout "github.com/google/go-containerregistry/pkg/v1/layout" + "github.com/google/go-containerregistry/pkg/v1/partial" "github.com/moby/moby/client" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "golang.org/x/sync/errgroup" @@ -201,81 +203,96 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds updateText := fmt.Sprintf("Pulling %d images", imageCount) go utils.RenderProgressBarForLocalDirWrite(dst.Base, totalBytes, doneSaving, updateText, updateText) - eg, _ = errgroup.WithContext(ctx) - eg.SetLimit(10) - - processing := make(map[string]chan struct{}) + toSave := map[string]v1.Image{} for _, info := range list { - info := info - refInfo := info.RefInfo - img := info.Img - eg.Go(func() error { - layers, err := img.Layers() - if err != nil { - return fmt.Errorf("unable to get layers for %s: %w", refInfo.Reference, err) - } + toSave[info.RefInfo.Reference] = info.Img + } - layerChans := make([]chan struct{}, len(layers)) - mu.Lock() - for i, layer := range layers { - digest, err := layer.Digest() - if err != nil { - mu.Unlock() - return err - } + sc := func() error { + saved, err := SaveConcurrent(ctx, cranePath, toSave) + if err != nil { + return err + } + for k := range saved { + delete(toSave, k) + } + return nil + } - if ch, ok := processing[digest.Hex]; !ok { - // If no channel exists, create one - ch = make(chan struct{}) - processing[digest.Hex] = ch - } else { - // If channel exists, prepare to wait on it - layerChans[i] = ch - } - } - mu.Unlock() + ss := func() error { + saved, err := SaveSequential(cranePath, toSave) + if err != nil { + return err + } + for k := range saved { + delete(toSave, k) + } + return nil + } - // Wait for all required layers to be available - for idx, ch := range layerChans { - if ch != nil { - message.Debug(refInfo.Reference, "waiting for layer to be processed", idx) - <-ch // Wait for the channel to be closed - } - } + if err := helpers.Retry(sc, 2, 5*time.Second, message.Warnf); err != nil { + message.Warnf("Failed to save images in parallel, falling back to sequential save: %s", err.Error()) - message.Debugf("Pulling image %s", refInfo.Reference) - annotations := map[string]string{ - ocispec.AnnotationBaseImageName: refInfo.Reference, - } + if err := helpers.Retry(ss, 2, 5*time.Second, message.Warnf); err != nil { + return nil, err + } + } - // also have clayout.WithPlatform() as a future option to use - if err := cranePath.AppendImage(img, clayout.WithAnnotations(annotations)); err != nil { - return fmt.Errorf("error when trying to save %s: %w", refInfo.Reference, err) - } + // Send a signal to the progress bar that we're done and wait for the thread to finish + doneSaving <- nil + <-doneSaving - // Signal that processing is done by closing channels - mu.Lock() - for _, layer := range layers { - digest, _ := layer.Digest() - ch, exists := processing[digest.Hex] - if exists { - close(ch) - delete(processing, digest.Hex) // Remove the channel from the map - } - } - mu.Unlock() + return list, nil +} - return nil - }) +// SaveSequential saves images sequentially. +func SaveSequential(cl clayout.Path, m map[string]v1.Image) (map[string]v1.Image, error) { + saved := map[string]v1.Image{} + for name, img := range m { + name, img := name, img + annotations := map[string]string{ + ocispec.AnnotationBaseImageName: name, + } + if err := cl.AppendImage(img, clayout.WithAnnotations(annotations)); err != nil { + return saved, fmt.Errorf("error when trying to save %s: %w", name, err) + } + saved[name] = img } + return saved, nil +} - if err := eg.Wait(); err != nil { - return nil, err +// SaveConcurrent saves images in a concurrent, bounded manner. +func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[string]v1.Image) (map[string]v1.Image, error) { + saved := map[string]v1.Image{} + + for name, img := range m { + name, img := name, img + desc, err := partial.Descriptor(img) + if err != nil { + return saved, err + } + annotations := map[string]string{ + ocispec.AnnotationBaseImageName: name, + } + desc.Annotations = annotations + if err := cl.AppendDescriptor(*desc); err != nil { + return saved, err + } } - // Send a signal to the progress bar that we're done and wait for the thread to finish - doneSaving <- nil - <-doneSaving + eg, _ := errgroup.WithContext(ctx) + eg.SetLimit(10) - return list, nil + for name, img := range m { + name, img := name, img + eg.Go(func() error { + if err := cl.WriteImage(img); err != nil { + return err + } + saved[name] = img + return nil + }) + } + + return saved, eg.Wait() } From 0db1209f3ab3ae5b5394f0975acc9ef75a940469 Mon Sep 17 00:00:00 2001 From: razzle Date: Fri, 26 Apr 2024 03:46:21 -0500 Subject: [PATCH 11/43] incremental and retryable saves Signed-off-by: razzle --- src/internal/packager/images/pull.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 746b7af354..63e0920eb5 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -235,24 +235,18 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds sc := func() error { saved, err := SaveConcurrent(ctx, cranePath, toSave) - if err != nil { - return err - } for k := range saved { delete(toSave, k) } - return nil + return err } ss := func() error { saved, err := SaveSequential(cranePath, toSave) - if err != nil { - return err - } for k := range saved { delete(toSave, k) } - return nil + return err } if err := helpers.Retry(sc, 2, 5*time.Second, message.Warnf); err != nil { From 186f915395ed7fe23d01cb85cdc2a97f08676f73 Mon Sep 17 00:00:00 2001 From: razzle Date: Fri, 26 Apr 2024 04:26:36 -0500 Subject: [PATCH 12/43] cleanup Signed-off-by: razzle --- src/internal/packager/images/pull.go | 50 ++++++++++++---------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 63e0920eb5..e0c0e9052c 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -29,6 +29,8 @@ import ( "github.com/google/go-containerregistry/pkg/v1/empty" clayout "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/google/go-containerregistry/pkg/v1/partial" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/types" "github.com/moby/moby/client" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "golang.org/x/sync/errgroup" @@ -78,12 +80,6 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds shas := make(map[string]bool) opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) - // retry := func(cb func() error) func() error { - // return func() error { - // return helpers.Retry(cb, 3, 5*time.Second, message.Warnf) - // } - // } - for idx, refInfo := range i.ImageList { refInfo, idx := refInfo, idx eg.Go(func() error { @@ -97,6 +93,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds } var img v1.Image + var desc *remote.Descriptor // load from local fs if it's a tarball if strings.HasSuffix(ref, ".tar") || strings.HasSuffix(ref, ".tar.gz") || strings.HasSuffix(ref, ".tgz") { @@ -109,7 +106,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds if err != nil { return fmt.Errorf("failed to parse reference: %w", err) } - _, err = crane.Head(ref, opts...) + desc, err = crane.Get(ref, opts...) if err != nil { if strings.Contains(err.Error(), "unexpected status code 429 Too Many Requests") { cancel() @@ -156,38 +153,34 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds } } - img = cache.Image(img, cache.NewFilesystemCache(filepath.Join(config.GetAbsCachePath(), layout.ImagesDir))) - - manifest, err := img.Manifest() - if err != nil { - return fmt.Errorf("unable to get manifest for %s: %w", refInfo.Reference, err) - } - totalBytes += manifest.Config.Size - - mt, err := img.MediaType() - if err != nil { - return fmt.Errorf("unable to get media type for %s: %w", refInfo.Reference, err) - } - - if refInfo.Digest != "" && mt.IsIndex() { + if refInfo.Digest != "" && desc != nil && types.MediaType(desc.MediaType).IsIndex() { message.Warn("Zarf does not currently support direct consumption of OCI image indexes or Docker manifest lists") var idx v1.IndexManifest - b, err := img.RawManifest() - if err != nil { - return fmt.Errorf("unable to get raw manifest for %s: %w", refInfo.Reference, err) - } - if err := json.Unmarshal(b, &idx); err != nil { + if err := json.Unmarshal(desc.Manifest, &idx); err != nil { return fmt.Errorf("unable to unmarshal index manifest: %w", err) } - message.Warn("The following images are available in the index:") + lines := []string{"The following images are available in the index:"} + name := refInfo.Name + if refInfo.Tag != "" { + name += ":" + refInfo.Tag + } for _, desc := range idx.Manifests { - message.Warnf("%s%s for platform %s", refInfo.Name, refInfo.TagOrDigest, desc.Platform) + lines = append(lines, fmt.Sprintf("\n(%s) %s@%s", desc.Platform, name, desc.Digest)) } + message.Warn(strings.Join(lines, "\n")) cancel() return fmt.Errorf("%s resolved to an index, please select a specific platform to use", refInfo.Reference) } + img = cache.Image(img, cache.NewFilesystemCache(filepath.Join(config.GetAbsCachePath(), layout.ImagesDir))) + + manifest, err := img.Manifest() + if err != nil { + return fmt.Errorf("unable to get manifest for %s: %w", refInfo.Reference, err) + } + totalBytes += manifest.Config.Size + layers, err := img.Layers() if err != nil { return fmt.Errorf("unable to get layers for %s: %w", refInfo.Reference, err) @@ -212,6 +205,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds } list = append(list, ImgInfo{RefInfo: refInfo, Img: img}) + message.Infof("%#v", ImgInfo{RefInfo: refInfo, Img: img}) return nil }) } From 8af1bf4d8edd8ec998c4abaa4bbda73cfa49981c Mon Sep 17 00:00:00 2001 From: razzle Date: Fri, 26 Apr 2024 13:30:55 -0500 Subject: [PATCH 13/43] cache cleanup Signed-off-by: razzle --- src/internal/packager/images/pull.go | 83 ++++++++++++++++++++++------ 1 file changed, 65 insertions(+), 18 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index e0c0e9052c..1f68ec77ac 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -8,6 +8,7 @@ import ( "context" "encoding/json" "fmt" + "os" "path/filepath" "strings" "sync" @@ -42,6 +43,8 @@ type ImgInfo struct { Img v1.Image } +var cacheDir = filepath.Join(config.GetAbsCachePath(), layout.ImagesDir) + // PullAll pulls all of the images in the provided tag map. func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, dst layout.Images) (list []ImgInfo, err error) { @@ -173,7 +176,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return fmt.Errorf("%s resolved to an index, please select a specific platform to use", refInfo.Reference) } - img = cache.Image(img, cache.NewFilesystemCache(filepath.Join(config.GetAbsCachePath(), layout.ImagesDir))) + img = cache.Image(img, cache.NewFilesystemCache(cacheDir)) manifest, err := img.Manifest() if err != nil { @@ -236,7 +239,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds } ss := func() error { - saved, err := SaveSequential(cranePath, toSave) + saved, err := SaveSequential(ctx, cranePath, toSave) for k := range saved { delete(toSave, k) } @@ -258,8 +261,43 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return list, nil } +// CleanupInProgressLayers removes incomplete layers from the cache. +func CleanupInProgressLayers(ctx context.Context, img v1.Image) error { + layers, err := img.Layers() + if err != nil { + return err + } + eg, _ := errgroup.WithContext(ctx) + eg.SetLimit(10) + for _, layer := range layers { + layer := layer + eg.Go(func() error { + digest, err := layer.Digest() + if err != nil { + return err + } + size, err := layer.Size() + if err != nil { + return err + } + location := filepath.Join(cacheDir, digest.Hex) + info, err := os.Stat(location) + if err != nil { + return err + } + if info.Size() != size { + if err := os.Remove(location); err != nil { + return fmt.Errorf("failed to remove incomplete layer %s: %w", digest.Hex, err) + } + } + return nil + }) + } + return nil +} + // SaveSequential saves images sequentially. -func SaveSequential(cl clayout.Path, m map[string]v1.Image) (map[string]v1.Image, error) { +func SaveSequential(ctx context.Context, cl clayout.Path, m map[string]v1.Image) (map[string]v1.Image, error) { saved := map[string]v1.Image{} for name, img := range m { name, img := name, img @@ -267,7 +305,10 @@ func SaveSequential(cl clayout.Path, m map[string]v1.Image) (map[string]v1.Image ocispec.AnnotationBaseImageName: name, } if err := cl.AppendImage(img, clayout.WithAnnotations(annotations)); err != nil { - return saved, fmt.Errorf("error when trying to save %s: %w", name, err) + if err = CleanupInProgressLayers(ctx, img); err != nil { + message.WarnErr(err, "failed to clean up in-progress layers, please remove them manually") + } + return saved, err } saved[name] = img } @@ -278,20 +319,7 @@ func SaveSequential(cl clayout.Path, m map[string]v1.Image) (map[string]v1.Image func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[string]v1.Image) (map[string]v1.Image, error) { saved := map[string]v1.Image{} - for name, img := range m { - name, img := name, img - desc, err := partial.Descriptor(img) - if err != nil { - return saved, err - } - annotations := map[string]string{ - ocispec.AnnotationBaseImageName: name, - } - desc.Annotations = annotations - if err := cl.AppendDescriptor(*desc); err != nil { - return saved, err - } - } + var mu sync.Mutex eg, _ := errgroup.WithContext(ctx) eg.SetLimit(10) @@ -299,9 +327,28 @@ func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[string]v1.Image) for name, img := range m { name, img := name, img eg.Go(func() error { + desc, err := partial.Descriptor(img) + if err != nil { + return err + } + if err := cl.WriteImage(img); err != nil { + if err = CleanupInProgressLayers(ctx, img); err != nil { + message.WarnErr(err, "failed to clean up in-progress layers, please remove them manually") + } return err } + + mu.Lock() + annotations := map[string]string{ + ocispec.AnnotationBaseImageName: name, + } + desc.Annotations = annotations + if err := cl.AppendDescriptor(*desc); err != nil { + return err + } + mu.Unlock() + saved[name] = img return nil }) From 4f7a323ee66f08a7e90deebf2b06dc6ad2553a62 Mon Sep 17 00:00:00 2001 From: razzle Date: Fri, 26 Apr 2024 13:37:38 -0500 Subject: [PATCH 14/43] cleanup Signed-off-by: razzle --- src/internal/packager/images/pull.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 1f68ec77ac..86681b11d4 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -208,7 +208,6 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds } list = append(list, ImgInfo{RefInfo: refInfo, Img: img}) - message.Infof("%#v", ImgInfo{RefInfo: refInfo, Img: img}) return nil }) } From bd56904061ac2bf9561743f532e461cae737f4da Mon Sep 17 00:00:00 2001 From: razzle Date: Fri, 26 Apr 2024 13:48:34 -0500 Subject: [PATCH 15/43] one map to rule them all Signed-off-by: razzle --- src/internal/packager/images/pull.go | 46 ++++++++++++++-------------- src/pkg/packager/creator/normal.go | 11 ++++--- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 86681b11d4..4a0476f3e4 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -8,6 +8,7 @@ import ( "context" "encoding/json" "fmt" + "maps" "os" "path/filepath" "strings" @@ -46,7 +47,7 @@ type ImgInfo struct { var cacheDir = filepath.Join(config.GetAbsCachePath(), layout.ImagesDir) // PullAll pulls all of the images in the provided tag map. -func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, dst layout.Images) (list []ImgInfo, err error) { +func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, dst layout.Images) (map[transform.Image]v1.Image, error) { var longer string imageCount := len(i.ImageList) @@ -83,6 +84,8 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds shas := make(map[string]bool) opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) + var fetched = map[transform.Image]v1.Image{} + for idx, refInfo := range i.ImageList { refInfo, idx := refInfo, idx eg.Go(func() error { @@ -207,7 +210,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds } } - list = append(list, ImgInfo{RefInfo: refInfo, Img: img}) + fetched[refInfo] = img return nil }) } @@ -224,23 +227,20 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds updateText := fmt.Sprintf("Pulling %d images", imageCount) go utils.RenderProgressBarForLocalDirWrite(dst.Base, totalBytes, doneSaving, updateText, updateText) - toSave := map[string]v1.Image{} - for _, info := range list { - toSave[info.RefInfo.Reference] = info.Img - } + toPull := maps.Clone(fetched) sc := func() error { - saved, err := SaveConcurrent(ctx, cranePath, toSave) + saved, err := SaveConcurrent(ctx, cranePath, toPull) for k := range saved { - delete(toSave, k) + delete(toPull, k) } return err } ss := func() error { - saved, err := SaveSequential(ctx, cranePath, toSave) + saved, err := SaveSequential(ctx, cranePath, toPull) for k := range saved { - delete(toSave, k) + delete(toPull, k) } return err } @@ -257,7 +257,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds doneSaving <- nil <-doneSaving - return list, nil + return fetched, nil } // CleanupInProgressLayers removes incomplete layers from the cache. @@ -296,12 +296,12 @@ func CleanupInProgressLayers(ctx context.Context, img v1.Image) error { } // SaveSequential saves images sequentially. -func SaveSequential(ctx context.Context, cl clayout.Path, m map[string]v1.Image) (map[string]v1.Image, error) { - saved := map[string]v1.Image{} - for name, img := range m { - name, img := name, img +func SaveSequential(ctx context.Context, cl clayout.Path, m map[transform.Image]v1.Image) (map[transform.Image]v1.Image, error) { + saved := map[transform.Image]v1.Image{} + for info, img := range m { + info, img := info, img annotations := map[string]string{ - ocispec.AnnotationBaseImageName: name, + ocispec.AnnotationBaseImageName: info.Reference, } if err := cl.AppendImage(img, clayout.WithAnnotations(annotations)); err != nil { if err = CleanupInProgressLayers(ctx, img); err != nil { @@ -309,22 +309,22 @@ func SaveSequential(ctx context.Context, cl clayout.Path, m map[string]v1.Image) } return saved, err } - saved[name] = img + saved[info] = img } return saved, nil } // SaveConcurrent saves images in a concurrent, bounded manner. -func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[string]v1.Image) (map[string]v1.Image, error) { - saved := map[string]v1.Image{} +func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[transform.Image]v1.Image) (map[transform.Image]v1.Image, error) { + saved := map[transform.Image]v1.Image{} var mu sync.Mutex eg, _ := errgroup.WithContext(ctx) eg.SetLimit(10) - for name, img := range m { - name, img := name, img + for info, img := range m { + info, img := info, img eg.Go(func() error { desc, err := partial.Descriptor(img) if err != nil { @@ -340,7 +340,7 @@ func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[string]v1.Image) mu.Lock() annotations := map[string]string{ - ocispec.AnnotationBaseImageName: name, + ocispec.AnnotationBaseImageName: info.Reference, } desc.Annotations = annotations if err := cl.AppendDescriptor(*desc); err != nil { @@ -348,7 +348,7 @@ func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[string]v1.Image) } mu.Unlock() - saved[name] = img + saved[info] = img return nil }) } diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 8ebb5ced36..f496870d68 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -192,16 +192,17 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. return err } - for _, imgInfo := range pulled { - if err := dst.Images.AddV1Image(imgInfo.Img); err != nil { + for info, img := range pulled { + info, img := info, img + if err := dst.Images.AddV1Image(img); err != nil { return err } - ok, err := utils.HasImageLayers(imgInfo.Img) + ok, err := utils.HasImageLayers(img) if err != nil { - return fmt.Errorf("failed to validate %s is an image and not an artifact: %w", imgInfo, err) + return fmt.Errorf("failed to validate %s is an image and not an artifact: %w", info, err) } if ok { - sbomImageList = append(sbomImageList, imgInfo.RefInfo) + sbomImageList = append(sbomImageList, info) } } } From c4b2c9f530700ed7a7ddb1c0a4ed297d5bcc2051 Mon Sep 17 00:00:00 2001 From: razzle Date: Fri, 26 Apr 2024 14:27:16 -0500 Subject: [PATCH 16/43] fix cache dir variable Signed-off-by: razzle --- src/internal/packager/images/pull.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 4a0476f3e4..36413ebae2 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -44,10 +44,9 @@ type ImgInfo struct { Img v1.Image } -var cacheDir = filepath.Join(config.GetAbsCachePath(), layout.ImagesDir) - // PullAll pulls all of the images in the provided tag map. func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, dst layout.Images) (map[transform.Image]v1.Image, error) { + cacheDir := filepath.Join(config.GetAbsCachePath(), layout.ImagesDir) var longer string imageCount := len(i.ImageList) @@ -279,6 +278,7 @@ func CleanupInProgressLayers(ctx context.Context, img v1.Image) error { if err != nil { return err } + cacheDir := filepath.Join(config.GetAbsCachePath(), layout.ImagesDir) location := filepath.Join(cacheDir, digest.Hex) info, err := os.Stat(location) if err != nil { From 8ca8e38f6f5e3977b1a3b46dee97e18e607c7a3b Mon Sep 17 00:00:00 2001 From: razzle Date: Fri, 26 Apr 2024 20:22:35 -0500 Subject: [PATCH 17/43] cleanup Signed-off-by: razzle --- src/internal/packager/images/pull.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 36413ebae2..d6ea23173d 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -118,7 +118,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return fmt.Errorf("rate limited by registry: %w", err) } - message.Notef("Falling back to local 'docker', failed to find the manifest on a remote: %s", err.Error()) + message.Warnf("Falling back to local 'docker', failed to find the manifest on a remote: %s", err.Error()) // Attempt to connect to the local docker daemon. cli, err := client.NewClientWithOpts(client.FromEnv) @@ -134,7 +134,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds cancel() } - return fmt.Errorf("failed to inspect via docker: %w", err) + return err } // Warn the user if the image is large. From a4ee1cd1696930e25796b2c8031018f292c0752b Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 29 Apr 2024 13:33:11 -0500 Subject: [PATCH 18/43] go mod tidy Signed-off-by: razzle --- go.mod | 2 +- go.sum | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/go.mod b/go.mod index ff27c01fe6..fc6a96f550 100644 --- a/go.mod +++ b/go.mod @@ -47,6 +47,7 @@ require ( github.com/stretchr/testify v1.9.0 github.com/xeipuuv/gojsonschema v1.2.0 golang.org/x/crypto v0.21.0 + golang.org/x/sync v0.6.0 golang.org/x/term v0.18.0 helm.sh/helm/v3 v3.14.2 k8s.io/api v0.29.1 @@ -474,7 +475,6 @@ require ( golang.org/x/mod v0.14.0 // indirect golang.org/x/net v0.22.0 // indirect golang.org/x/oauth2 v0.17.0 // indirect - golang.org/x/sync v0.6.0 golang.org/x/sys v0.18.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.5.0 // indirect diff --git a/go.sum b/go.sum index a45dbd3c96..908f40ea80 100644 --- a/go.sum +++ b/go.sum @@ -595,8 +595,6 @@ github.com/daviddengcn/go-colortext v1.0.0 h1:ANqDyC0ys6qCSvuEK7l3g5RaehL/Xck9EX github.com/daviddengcn/go-colortext v1.0.0/go.mod h1:zDqEI5NVUop5QPpVJUxE9UO10hRnmkD5G4Pmri9+m4c= github.com/defenseunicorns/gojsonschema v0.0.0-20231116163348-e00f069122d6 h1:gwevOZ0fxT2nzM9hrtdPbsiOHjFqDRIYMzJHba3/G6Q= github.com/defenseunicorns/gojsonschema v0.0.0-20231116163348-e00f069122d6/go.mod h1:StKLYMmPj1R5yIs6CK49EkcW1TvUYuw5Vri+LRk7Dy8= -github.com/defenseunicorns/pkg/helpers v1.1.0 h1:VU8Cr3IGFEDuZrfavxmo6YXsh5FZXhehy4clKXrHNGk= -github.com/defenseunicorns/pkg/helpers v1.1.0/go.mod h1:F4S5VZLDrlNWQKklzv4v9tFWjjZNhxJ1gT79j4XiLwk= github.com/defenseunicorns/pkg/helpers v1.1.1 h1:p3pKeK5SeFaoZUJZIX9sEsJqX1CGGMS8OpQMPgJtSqM= github.com/defenseunicorns/pkg/helpers v1.1.1/go.mod h1:F4S5VZLDrlNWQKklzv4v9tFWjjZNhxJ1gT79j4XiLwk= github.com/defenseunicorns/pkg/oci v0.0.1 h1:EFRp3NeiwzhOWKpQ6mAxi0l9chnrAvDcIgjMr0o0fkM= From ea2a51b0bf3804fa169ec0eabfd8d3a9d56fdd9f Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 29 Apr 2024 13:48:36 -0500 Subject: [PATCH 19/43] map locks Signed-off-by: razzle --- src/internal/packager/images/pull.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index d6ea23173d..c763725d33 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -78,7 +78,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds eg, _ := errgroup.WithContext(ctx) eg.SetLimit(10) - var mu sync.Mutex + var shaLock, fetchedLock sync.Mutex totalBytes := int64(0) shas := make(map[string]bool) opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) @@ -197,19 +197,21 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return fmt.Errorf("unable to get digest for image layer: %w", err) } + shaLock.Lock() if _, ok := shas[digest.Hex]; !ok { - mu.Lock() shas[digest.Hex] = true - mu.Unlock() size, err := layer.Size() if err != nil { return fmt.Errorf("unable to get size for image layer: %w", err) } totalBytes += size } + shaLock.Unlock() } + fetchedLock.Lock() fetched[refInfo] = img + fetchedLock.Unlock() return nil }) } From dd21856e3358a36c39e6feb86bcf8dbb1f5d3775 Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 29 Apr 2024 14:01:04 -0500 Subject: [PATCH 20/43] rwlock Signed-off-by: razzle --- src/internal/packager/images/pull.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index c763725d33..ea1f8a32dd 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -78,7 +78,8 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds eg, _ := errgroup.WithContext(ctx) eg.SetLimit(10) - var shaLock, fetchedLock sync.Mutex + var shaLock sync.RWMutex + var fetchedLock sync.Mutex totalBytes := int64(0) shas := make(map[string]bool) opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) @@ -197,16 +198,22 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return fmt.Errorf("unable to get digest for image layer: %w", err) } - shaLock.Lock() - if _, ok := shas[digest.Hex]; !ok { - shas[digest.Hex] = true - size, err := layer.Size() - if err != nil { - return fmt.Errorf("unable to get size for image layer: %w", err) + shaLock.RLock() + _, ok := shas[digest.Hex] + shaLock.RUnlock() + if !ok { + shaLock.Lock() + defer shaLock.Unlock() + + if _, ok := shas[digest.Hex]; !ok { + shas[digest.Hex] = true + size, err := layer.Size() + if err != nil { + return fmt.Errorf("unable to get size for image layer: %w", err) + } + totalBytes += size } - totalBytes += size } - shaLock.Unlock() } fetchedLock.Lock() From 094563ae764592e3004014591148102f9c48b77b Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 29 Apr 2024 14:49:09 -0500 Subject: [PATCH 21/43] go mod tidy Signed-off-by: razzle --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 25e32204c2..931d43fb71 100644 --- a/go.mod +++ b/go.mod @@ -47,6 +47,7 @@ require ( github.com/stretchr/testify v1.9.0 github.com/xeipuuv/gojsonschema v1.2.0 golang.org/x/crypto v0.21.0 + golang.org/x/sync v0.6.0 golang.org/x/term v0.19.0 helm.sh/helm/v3 v3.14.2 k8s.io/api v0.29.1 @@ -474,7 +475,6 @@ require ( golang.org/x/mod v0.14.0 // indirect golang.org/x/net v0.22.0 // indirect golang.org/x/oauth2 v0.17.0 // indirect - golang.org/x/sync v0.6.0 // indirect golang.org/x/sys v0.19.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.5.0 // indirect From cd1a0f40aa8c69a350a14d2981b761e16c476205 Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 29 Apr 2024 19:04:22 -0500 Subject: [PATCH 22/43] fix infinite blocking (again) Signed-off-by: razzle --- src/internal/packager/images/pull.go | 35 +++++++++++++--------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index ea1f8a32dd..8f0851b56e 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -13,6 +13,7 @@ import ( "path/filepath" "strings" "sync" + "sync/atomic" "time" "github.com/defenseunicorns/pkg/helpers" @@ -78,18 +79,20 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds eg, _ := errgroup.WithContext(ctx) eg.SetLimit(10) - var shaLock sync.RWMutex - var fetchedLock sync.Mutex + var fetchedLock, shaLock sync.Mutex totalBytes := int64(0) shas := make(map[string]bool) opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) var fetched = map[transform.Image]v1.Image{} - for idx, refInfo := range i.ImageList { - refInfo, idx := refInfo, idx + var counter atomic.Int64 + + for _, refInfo := range i.ImageList { + refInfo := refInfo eg.Go(func() error { - spinner.Updatef("Fetching image info (%d of %d)", idx+1, len(i.ImageList)) + idx := counter.Add(1) + spinner.Updatef("Fetching image info (%d of %d)", idx, len(i.ImageList)) ref := refInfo.Reference for k, v := range i.RegistryOverrides { @@ -198,22 +201,16 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return fmt.Errorf("unable to get digest for image layer: %w", err) } - shaLock.RLock() - _, ok := shas[digest.Hex] - shaLock.RUnlock() - if !ok { - shaLock.Lock() - defer shaLock.Unlock() - - if _, ok := shas[digest.Hex]; !ok { - shas[digest.Hex] = true - size, err := layer.Size() - if err != nil { - return fmt.Errorf("unable to get size for image layer: %w", err) - } - totalBytes += size + shaLock.Lock() + if _, ok := shas[digest.Hex]; !ok { + shas[digest.Hex] = true + size, err := layer.Size() + if err != nil { + return fmt.Errorf("unable to get size for image layer: %w", err) } + totalBytes += size } + shaLock.Unlock() } fetchedLock.Lock() From 416461645e1954a722af59c80ffdb47470a4057c Mon Sep 17 00:00:00 2001 From: razzle Date: Tue, 30 Apr 2024 09:12:28 -0500 Subject: [PATCH 23/43] changes from review Signed-off-by: razzle --- src/internal/packager/images/pull.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 8f0851b56e..f77e1e3d2e 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -80,13 +80,12 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds eg.SetLimit(10) var fetchedLock, shaLock sync.Mutex - totalBytes := int64(0) shas := make(map[string]bool) opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) var fetched = map[transform.Image]v1.Image{} - var counter atomic.Int64 + var counter, totalBytes atomic.Int64 for _, refInfo := range i.ImageList { refInfo := refInfo @@ -188,7 +187,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds if err != nil { return fmt.Errorf("unable to get manifest for %s: %w", refInfo.Reference, err) } - totalBytes += manifest.Config.Size + totalBytes.Add(manifest.Config.Size) layers, err := img.Layers() if err != nil { @@ -208,7 +207,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds if err != nil { return fmt.Errorf("unable to get size for image layer: %w", err) } - totalBytes += size + totalBytes.Add(size) } shaLock.Unlock() } @@ -230,7 +229,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds doneSaving := make(chan error) updateText := fmt.Sprintf("Pulling %d images", imageCount) - go utils.RenderProgressBarForLocalDirWrite(dst.Base, totalBytes, doneSaving, updateText, updateText) + go utils.RenderProgressBarForLocalDirWrite(dst.Base, totalBytes.Load(), doneSaving, updateText, updateText) toPull := maps.Clone(fetched) @@ -272,7 +271,7 @@ func CleanupInProgressLayers(ctx context.Context, img v1.Image) error { return err } eg, _ := errgroup.WithContext(ctx) - eg.SetLimit(10) + eg.SetLimit(-1) for _, layer := range layers { layer := layer eg.Go(func() error { From e5852714f6234fcf7a7ef48afb4e16a4f994981a Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 2 May 2024 00:58:03 -0500 Subject: [PATCH 24/43] a bigger refactor Signed-off-by: razzle --- src/cmd/tools/crane.go | 31 +++---- src/config/config.go | 42 ---------- src/internal/packager/images/common.go | 108 +++++++++++++++++++++++-- src/internal/packager/images/pull.go | 28 +++---- src/internal/packager/images/push.go | 99 +++++++++++------------ src/pkg/packager/creator/normal.go | 13 +-- src/pkg/packager/deploy.go | 22 +++-- src/pkg/packager/prepare.go | 4 +- 8 files changed, 196 insertions(+), 151 deletions(-) diff --git a/src/cmd/tools/crane.go b/src/cmd/tools/crane.go index b6fd47e5a0..792e4b555c 100644 --- a/src/cmd/tools/crane.go +++ b/src/cmd/tools/crane.go @@ -13,6 +13,7 @@ import ( "github.com/defenseunicorns/zarf/src/cmd/common" "github.com/defenseunicorns/zarf/src/config" "github.com/defenseunicorns/zarf/src/config/lang" + "github.com/defenseunicorns/zarf/src/internal/packager/images" "github.com/defenseunicorns/zarf/src/pkg/cluster" "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/pkg/transform" @@ -85,12 +86,12 @@ func init() { craneCopy := craneCmd.NewCmdCopy(&craneOptions) registryCmd.AddCommand(craneCopy) - registryCmd.AddCommand(zarfCraneCatalog(&craneOptions)) - registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdList, &craneOptions, lang.CmdToolsRegistryListExample, 0)) - registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdPush, &craneOptions, lang.CmdToolsRegistryPushExample, 1)) - registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdPull, &craneOptions, lang.CmdToolsRegistryPullExample, 0)) - registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdDelete, &craneOptions, lang.CmdToolsRegistryDeleteExample, 0)) - registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdDigest, &craneOptions, lang.CmdToolsRegistryDigestExample, 0)) + registryCmd.AddCommand(zarfCraneCatalog(craneOptions)) + registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdList, craneOptions, lang.CmdToolsRegistryListExample, 0)) + registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdPush, craneOptions, lang.CmdToolsRegistryPushExample, 1)) + registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdPull, craneOptions, lang.CmdToolsRegistryPullExample, 0)) + registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdDelete, craneOptions, lang.CmdToolsRegistryDeleteExample, 0)) + registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdDigest, craneOptions, lang.CmdToolsRegistryDigestExample, 0)) registryCmd.AddCommand(pruneCmd) registryCmd.AddCommand(craneCmd.NewCmdVersion()) @@ -103,8 +104,8 @@ func init() { } // Wrap the original crane catalog with a zarf specific version -func zarfCraneCatalog(cranePlatformOptions *[]crane.Option) *cobra.Command { - craneCatalog := craneCmd.NewCmdCatalog(cranePlatformOptions) +func zarfCraneCatalog(cranePlatformOptions []crane.Option) *cobra.Command { + craneCatalog := craneCmd.NewCmdCatalog(&cranePlatformOptions) craneCatalog.Example = lang.CmdToolsRegistryCatalogExample craneCatalog.Args = nil @@ -135,8 +136,8 @@ func zarfCraneCatalog(cranePlatformOptions *[]crane.Option) *cobra.Command { } // Add the correct authentication to the crane command options - authOption := config.GetCraneAuthOption(zarfState.RegistryInfo.PullUsername, zarfState.RegistryInfo.PullPassword) - *cranePlatformOptions = append(*cranePlatformOptions, authOption) + authOption := images.WithPullAuth(zarfState.RegistryInfo) + cranePlatformOptions = append(cranePlatformOptions, authOption) if tunnel != nil { message.Notef(lang.CmdToolsRegistryTunnel, registryEndpoint, zarfState.RegistryInfo.Address) @@ -151,8 +152,8 @@ func zarfCraneCatalog(cranePlatformOptions *[]crane.Option) *cobra.Command { } // Wrap the original crane list with a zarf specific version -func zarfCraneInternalWrapper(commandToWrap func(*[]crane.Option) *cobra.Command, cranePlatformOptions *[]crane.Option, exampleText string, imageNameArgumentIndex int) *cobra.Command { - wrappedCommand := commandToWrap(cranePlatformOptions) +func zarfCraneInternalWrapper(commandToWrap func(*[]crane.Option) *cobra.Command, cranePlatformOptions []crane.Option, exampleText string, imageNameArgumentIndex int) *cobra.Command { + wrappedCommand := commandToWrap(&cranePlatformOptions) wrappedCommand.Example = exampleText wrappedCommand.Args = nil @@ -190,8 +191,8 @@ func zarfCraneInternalWrapper(commandToWrap func(*[]crane.Option) *cobra.Command } // Add the correct authentication to the crane command options - authOption := config.GetCraneAuthOption(zarfState.RegistryInfo.PushUsername, zarfState.RegistryInfo.PushPassword) - *cranePlatformOptions = append(*cranePlatformOptions, authOption) + authOption := images.WithPushAuth(zarfState.RegistryInfo) + cranePlatformOptions = append(cranePlatformOptions, authOption) if tunnel != nil { message.Notef(lang.CmdToolsRegistryTunnel, tunnel.Endpoint(), zarfState.RegistryInfo.Address) @@ -245,7 +246,7 @@ func pruneImages(_ *cobra.Command, _ []string) error { } func doPruneImagesForPackages(zarfState *types.ZarfState, zarfPackages []types.DeployedPackage, registryEndpoint string) error { - authOption := config.GetCraneAuthOption(zarfState.RegistryInfo.PushUsername, zarfState.RegistryInfo.PushPassword) + authOption := images.WithPushAuth(zarfState.RegistryInfo) spinner := message.NewProgressSpinner(lang.CmdToolsRegistryPruneLookup) defer spinner.Stop() diff --git a/src/config/config.go b/src/config/config.go index 63396255b0..8aedce6abb 100644 --- a/src/config/config.go +++ b/src/config/config.go @@ -5,10 +5,8 @@ package config import ( - "crypto/tls" "embed" "fmt" - "net/http" "os" "path/filepath" "runtime" @@ -16,9 +14,6 @@ import ( "time" "github.com/defenseunicorns/zarf/src/types" - "github.com/google/go-containerregistry/pkg/authn" - "github.com/google/go-containerregistry/pkg/crane" - v1 "github.com/google/go-containerregistry/pkg/v1" ) // Zarf Global Configuration Constants. @@ -121,43 +116,6 @@ func GetDataInjectionMarker() string { return fmt.Sprintf(dataInjectionMarker, operationStartTime) } -// GetCraneOptions returns a crane option object with the correct options & platform. -func GetCraneOptions(insecure bool, archs ...string) []crane.Option { - var options []crane.Option - - // Handle insecure registry option - if insecure { - roundTripper := http.DefaultTransport.(*http.Transport).Clone() - roundTripper.TLSClientConfig = &tls.Config{ - InsecureSkipVerify: true, - } - options = append(options, crane.Insecure, crane.WithTransport(roundTripper)) - } - - if archs != nil { - options = append(options, crane.WithPlatform(&v1.Platform{OS: "linux", Architecture: GetArch(archs...)})) - } - - options = append(options, - crane.WithUserAgent("zarf"), - crane.WithNoClobber(true), - // TODO: (@WSTARR) this is set to limit pushes to registry pods and reduce the likelihood that crane will get stuck. - // We should investigate this further in the future to dig into more of what is happening (see https://github.com/defenseunicorns/zarf/issues/1568) - crane.WithJobs(1), - ) - - return options -} - -// GetCraneAuthOption returns a crane auth option with the provided credentials. -func GetCraneAuthOption(username string, secret string) crane.Option { - return crane.WithAuth( - authn.FromConfig(authn.AuthConfig{ - Username: username, - Password: secret, - })) -} - // GetAbsCachePath gets the absolute cache path for images and git repos. func GetAbsCachePath() string { return GetAbsHomePath(CommonOptions.CachePath) diff --git a/src/internal/packager/images/common.go b/src/internal/packager/images/common.go index 9a27edddbc..64bb88f17c 100644 --- a/src/internal/packager/images/common.go +++ b/src/internal/packager/images/common.go @@ -5,13 +5,35 @@ package images import ( + "net/http" + "time" + + "github.com/defenseunicorns/pkg/helpers" + "github.com/defenseunicorns/zarf/src/config" + "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/pkg/transform" "github.com/defenseunicorns/zarf/src/types" + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/crane" + v1 "github.com/google/go-containerregistry/pkg/v1" ) -// ImageConfig is the main struct for managing container images. -type ImageConfig struct { - ImagesPath string +// PullConfig is the configuration for pulling images. +type PullConfig struct { + DestinationDirectory string + + References []transform.Image + + Arch string + + RegistryOverrides map[string]string + + CacheDirectory string +} + +// PushConfig is the configuration for pushing images. +type PushConfig struct { + SourceDirectory string ImageList []transform.Image @@ -19,9 +41,83 @@ type ImageConfig struct { NoChecksum bool - Insecure bool + Arch string - Architectures []string + Retries int +} - RegistryOverrides map[string]string +// NoopOpt is a no-op option for crane. +func NoopOpt(*crane.Options) {} + +// WithGlobalInsecureFlag returns an option for crane that configures insecure +// based upon Zarf's global --insecure flag. +func WithGlobalInsecureFlag() []crane.Option { + opts := []crane.Option{} + if config.CommonOptions.Insecure { + opts = append(opts, crane.Insecure) + + // TODO: lets see what happens when we comment this guy out + // roundTripper := http.DefaultTransport.(*http.Transport).Clone() + // roundTripper.TLSClientConfig.InsecureSkipVerify = true + + // opts = append(opts, crane.WithTransport(roundTripper)) + return opts + } + // passing a nil option will cause panic + return []crane.Option{NoopOpt} +} + +// WithArchictecture sets the platform option for crane. +// +// This option is actually a slight mis-use of the platform option, as it is +// setting the architecture only and hard coding the OS to linux. +func WithArchictecture(arch string) crane.Option { + return crane.WithPlatform(&v1.Platform{OS: "linux", Architecture: arch}) +} + +// CommonOpts returns a set of common options for crane under Zarf. +func CommonOpts(arch string) []crane.Option { + opts := WithGlobalInsecureFlag() + opts = append(opts, WithArchictecture(arch)) + + opts = append(opts, + crane.WithUserAgent("zarf"), + crane.WithNoClobber(true), + crane.WithJobs(1), + ) + return opts +} + +// WithBasicAuth returns an option for crane that sets basic auth. +func WithBasicAuth(username, password string) crane.Option { + return crane.WithAuth(authn.FromConfig(authn.AuthConfig{ + Username: username, + Password: password, + })) +} + +// WithPullAuth returns an option for crane that sets pull auth from a given registry info. +func WithPullAuth(ri types.RegistryInfo) crane.Option { + return WithBasicAuth(ri.PullUsername, ri.PullPassword) +} + +// WithPushAuth returns an option for crane that sets push auth from a given registry info. +func WithPushAuth(ri types.RegistryInfo) crane.Option { + return WithBasicAuth(ri.PushUsername, ri.PushPassword) +} + +func createPushOpts(cfg PushConfig, pb *message.ProgressBar) []crane.Option { + opts := CommonOpts(cfg.Arch) + opts = append(opts, WithPushAuth(cfg.RegInfo)) + + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.TLSClientConfig.InsecureSkipVerify = config.CommonOptions.Insecure + // TODO (@WSTARR) This is set to match the TLSHandshakeTimeout to potentially mitigate effects of https://github.com/defenseunicorns/zarf/issues/1444 + transport.ResponseHeaderTimeout = 10 * time.Second + + transportWithProgressBar := helpers.NewTransport(transport, pb) + + opts = append(opts, crane.WithTransport(transportWithProgressBar)) + + return opts } diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index f77e1e3d2e..c4f16c1105 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -45,12 +45,10 @@ type ImgInfo struct { Img v1.Image } -// PullAll pulls all of the images in the provided tag map. -func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, dst layout.Images) (map[transform.Image]v1.Image, error) { - cacheDir := filepath.Join(config.GetAbsCachePath(), layout.ImagesDir) - +// Pull pulls all of the images from the given config. +func Pull(ctx context.Context, cancel context.CancelFunc, cfg PullConfig) (map[transform.Image]v1.Image, error) { var longer string - imageCount := len(i.ImageList) + imageCount := len(cfg.References) // Give some additional user feedback on larger image sets if imageCount > 15 { longer = "This step may take a couple of minutes to complete." @@ -58,13 +56,13 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds longer = "This step may take several seconds to complete." } - if err := helpers.CreateDirectory(dst.Base, helpers.ReadExecuteAllWriteUser); err != nil { - return nil, fmt.Errorf("failed to create image path %s: %w", dst.Base, err) + if err := helpers.CreateDirectory(cfg.DestinationDirectory, helpers.ReadExecuteAllWriteUser); err != nil { + return nil, fmt.Errorf("failed to create image path %s: %w", cfg.DestinationDirectory, err) } - cranePath, err := clayout.FromPath(dst.Base) + cranePath, err := clayout.FromPath(cfg.DestinationDirectory) if err != nil { - cranePath, err = clayout.Write(dst.Base, empty.Index) + cranePath, err = clayout.Write(cfg.DestinationDirectory, empty.Index) if err != nil { return nil, err } @@ -81,20 +79,20 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds var fetchedLock, shaLock sync.Mutex shas := make(map[string]bool) - opts := append(config.GetCraneOptions(i.Insecure, i.Architectures...), crane.WithContext(ctx)) + opts := append(CommonOpts(cfg.Arch), crane.WithContext(ctx)) var fetched = map[transform.Image]v1.Image{} var counter, totalBytes atomic.Int64 - for _, refInfo := range i.ImageList { + for _, refInfo := range cfg.References { refInfo := refInfo eg.Go(func() error { idx := counter.Add(1) - spinner.Updatef("Fetching image info (%d of %d)", idx, len(i.ImageList)) + spinner.Updatef("Fetching image info (%d of %d)", idx, imageCount) ref := refInfo.Reference - for k, v := range i.RegistryOverrides { + for k, v := range cfg.RegistryOverrides { if strings.HasPrefix(refInfo.Reference, k) { ref = strings.Replace(refInfo.Reference, k, v, 1) } @@ -181,7 +179,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds return fmt.Errorf("%s resolved to an index, please select a specific platform to use", refInfo.Reference) } - img = cache.Image(img, cache.NewFilesystemCache(cacheDir)) + img = cache.Image(img, cache.NewFilesystemCache(cfg.CacheDirectory)) manifest, err := img.Manifest() if err != nil { @@ -229,7 +227,7 @@ func (i *ImageConfig) PullAll(ctx context.Context, cancel context.CancelFunc, ds doneSaving := make(chan error) updateText := fmt.Sprintf("Pulling %d images", imageCount) - go utils.RenderProgressBarForLocalDirWrite(dst.Base, totalBytes.Load(), doneSaving, updateText, updateText) + go utils.RenderProgressBarForLocalDirWrite(cfg.DestinationDirectory, totalBytes.Load(), doneSaving, updateText, updateText) toPull := maps.Clone(fetched) diff --git a/src/internal/packager/images/push.go b/src/internal/packager/images/push.go index 900b925651..179c0913d0 100644 --- a/src/internal/packager/images/push.go +++ b/src/internal/packager/images/push.go @@ -6,11 +6,9 @@ package images import ( "fmt" - "net/http" "time" "github.com/defenseunicorns/pkg/helpers" - "github.com/defenseunicorns/zarf/src/config" "github.com/defenseunicorns/zarf/src/pkg/cluster" "github.com/defenseunicorns/zarf/src/pkg/k8s" "github.com/defenseunicorns/zarf/src/pkg/message" @@ -21,23 +19,20 @@ import ( v1 "github.com/google/go-containerregistry/pkg/v1" ) -// PushToZarfRegistry pushes a provided image into the configured Zarf registry -// This function will optionally shorten the image name while appending a checksum of the original image name. -func (i *ImageConfig) PushToZarfRegistry() error { - message.Debug("images.PushToZarfRegistry()") - +// Push pushes images to a registry. +func Push(cfg PushConfig) error { logs.Warn.SetOutput(&message.DebugWriter{}) logs.Progress.SetOutput(&message.DebugWriter{}) - refInfoToImage := map[transform.Image]v1.Image{} + toPush := map[transform.Image]v1.Image{} var totalSize int64 // Build an image list from the references - for _, refInfo := range i.ImageList { - img, err := utils.LoadOCIImage(i.ImagesPath, refInfo) + for _, refInfo := range cfg.ImageList { + img, err := utils.LoadOCIImage(cfg.SourceDirectory, refInfo) if err != nil { return err } - refInfoToImage[refInfo] = img + toPush[refInfo] = img imgSize, err := calcImgSize(img) if err != nil { return err @@ -46,42 +41,29 @@ func (i *ImageConfig) PushToZarfRegistry() error { } // If this is not a no checksum image push we will be pushing two images (the second will go faster as it checks the same layers) - if !i.NoChecksum { + if !cfg.NoChecksum { totalSize = totalSize * 2 } - httpTransport := http.DefaultTransport.(*http.Transport).Clone() - httpTransport.TLSClientConfig.InsecureSkipVerify = i.Insecure - // TODO (@WSTARR) This is set to match the TLSHandshakeTimeout to potentially mitigate effects of https://github.com/defenseunicorns/zarf/issues/1444 - httpTransport.ResponseHeaderTimeout = 10 * time.Second - progressBar := message.NewProgressBar(totalSize, fmt.Sprintf("Pushing %d images to the zarf registry", len(i.ImageList))) - defer progressBar.Stop() - craneTransport := helpers.NewTransport(httpTransport, progressBar) - - pushOptions := config.GetCraneOptions(i.Insecure, i.Architectures...) - pushOptions = append(pushOptions, config.GetCraneAuthOption(i.RegInfo.PushUsername, i.RegInfo.PushPassword)) - pushOptions = append(pushOptions, crane.WithTransport(craneTransport)) + progress := message.NewProgressBar(totalSize, fmt.Sprintf("Pushing %d images", len(cfg.ImageList))) + defer progress.Stop() var ( err error tunnel *k8s.Tunnel - registryURL string + registryURL = cfg.RegInfo.Address ) - registryURL = i.RegInfo.Address - c, _ := cluster.NewCluster() if c != nil { - registryURL, tunnel, err = c.ConnectToZarfRegistryEndpoint(i.RegInfo) + registryURL, tunnel, err = c.ConnectToZarfRegistryEndpoint(cfg.RegInfo) if err != nil { return err } - } - - if tunnel != nil { defer tunnel.Close() } + pushOptions := createPushOpts(cfg, progress) pushImage := func(img v1.Image, name string) error { if tunnel != nil { return tunnel.Wrap(func() error { return crane.Push(img, name, pushOptions...) }) @@ -90,41 +72,52 @@ func (i *ImageConfig) PushToZarfRegistry() error { return crane.Push(img, name, pushOptions...) } - for refInfo, img := range refInfoToImage { - refTruncated := helpers.Truncate(refInfo.Reference, 55, true) - progressBar.UpdateTitle(fmt.Sprintf("Pushing %s", refTruncated)) + if err := helpers.Retry(func() error { + pushed := []transform.Image{} + defer func() { + for _, refInfo := range pushed { + delete(toPush, refInfo) + } + }() + for refInfo, img := range toPush { + refTruncated := helpers.Truncate(refInfo.Reference, 55, true) + progress.UpdateTitle(fmt.Sprintf("Pushing %s", refTruncated)) + + // If this is not a no checksum image push it for use with the Zarf agent + if !cfg.NoChecksum { + offlineNameCRC, err := transform.ImageTransformHost(registryURL, refInfo.Reference) + if err != nil { + return err + } + + message.Debugf("push %s -> %s)", refInfo.Reference, offlineNameCRC) + + if err = pushImage(img, offlineNameCRC); err != nil { + return err + } + } - // If this is not a no checksum image push it for use with the Zarf agent - if !i.NoChecksum { - offlineNameCRC, err := transform.ImageTransformHost(registryURL, refInfo.Reference) + // To allow for other non-zarf workloads to easily see the images upload a non-checksum version + // (this may result in collisions but this is acceptable for this use case) + offlineName, err := transform.ImageTransformHostWithoutChecksum(registryURL, refInfo.Reference) if err != nil { return err } - message.Debugf("crane.Push() %s:%s -> %s)", i.ImagesPath, refInfo.Reference, offlineNameCRC) + message.Debugf("push %s -> %s)", refInfo.Reference, offlineName) - err = pushImage(img, offlineNameCRC) - if err != nil { + if err = pushImage(img, offlineName); err != nil { return err } - } - - // To allow for other non-zarf workloads to easily see the images upload a non-checksum version - // (this may result in collisions but this is acceptable for this use case) - offlineName, err := transform.ImageTransformHostWithoutChecksum(registryURL, refInfo.Reference) - if err != nil { - return err - } - - message.Debugf("crane.Push() %s:%s -> %s)", i.ImagesPath, refInfo.Reference, offlineName) - err = pushImage(img, offlineName) - if err != nil { - return err + pushed = append(pushed, refInfo) } + return nil + }, cfg.Retries, 5*time.Second, message.Warnf); err != nil { + return err } - progressBar.Successf("Pushed %d images to the zarf registry", len(i.ImageList)) + progress.Successf("Pushed %d images", len(cfg.ImageList)) return nil } diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index f496870d68..75885b79b0 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -180,14 +180,15 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. ctx, cancel := context.WithCancel(context.TODO()) - imgConfig := images.ImageConfig{ - ImageList: imageList, - Insecure: config.CommonOptions.Insecure, - Architectures: []string{arch}, - RegistryOverrides: pc.createOpts.RegistryOverrides, + pullCfg := images.PullConfig{ + DestinationDirectory: dst.Images.Base, + References: imageList, + Arch: arch, + RegistryOverrides: pc.createOpts.RegistryOverrides, + CacheDirectory: filepath.Join(config.GetAbsCachePath(), layout.ImagesDir), } - pulled, err := imgConfig.PullAll(ctx, cancel, dst.Images) + pulled, err := images.Pull(ctx, cancel, pullCfg) if err != nil { return err } diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index ef4495be0a..47333afe5e 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -496,18 +496,16 @@ func (p *Packager) pushImagesToRegistry(componentImages []string, noImgChecksum imageList := helpers.Unique(combinedImageList) - imgConfig := images.ImageConfig{ - ImagesPath: p.layout.Images.Base, - ImageList: imageList, - NoChecksum: noImgChecksum, - RegInfo: p.state.RegistryInfo, - Insecure: config.CommonOptions.Insecure, - Architectures: []string{p.cfg.Pkg.Build.Architecture}, - } - - return helpers.Retry(func() error { - return imgConfig.PushToZarfRegistry() - }, p.cfg.PkgOpts.Retries, 5*time.Second, message.Warnf) + pushCfg := images.PushConfig{ + SourceDirectory: p.layout.Images.Base, + ImageList: imageList, + RegInfo: p.state.RegistryInfo, + NoChecksum: noImgChecksum, + Arch: p.cfg.Pkg.Build.Architecture, + Retries: p.cfg.PkgOpts.Retries, + } + + return images.Push(pushCfg) } // Push all of the components git repos to the configured git server. diff --git a/src/pkg/packager/prepare.go b/src/pkg/packager/prepare.go index 6f5cbf0432..54c9158d5e 100644 --- a/src/pkg/packager/prepare.go +++ b/src/pkg/packager/prepare.go @@ -15,9 +15,9 @@ import ( "github.com/goccy/go-yaml" "github.com/defenseunicorns/pkg/helpers" - "github.com/defenseunicorns/zarf/src/config" "github.com/defenseunicorns/zarf/src/config/lang" "github.com/defenseunicorns/zarf/src/internal/packager/helm" + "github.com/defenseunicorns/zarf/src/internal/packager/images" "github.com/defenseunicorns/zarf/src/internal/packager/kustomize" "github.com/defenseunicorns/zarf/src/pkg/layout" "github.com/defenseunicorns/zarf/src/pkg/message" @@ -294,7 +294,7 @@ func (p *Packager) findImages() (imgMap map[string][]string, err error) { if sortedImages := sortImages(maybeImages, matchedImages); len(sortedImages) > 0 { var validImages []string for _, image := range sortedImages { - if descriptor, err := crane.Head(image, config.GetCraneOptions(config.CommonOptions.Insecure)...); err != nil { + if descriptor, err := crane.Head(image, images.WithGlobalInsecureFlag()...); err != nil { // Test if this is a real image, if not just quiet log to debug, this is normal message.Debugf("Suspected image does not appear to be valid: %#v", err) } else { From 6e8f458abb024589649b5211b4af67a393da8f0a Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 2 May 2024 01:11:51 -0500 Subject: [PATCH 25/43] wacky things Signed-off-by: razzle --- src/internal/packager/images/push.go | 32 ++++++++++++++++++---------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/internal/packager/images/push.go b/src/internal/packager/images/push.go index 179c0913d0..f39af4fe26 100644 --- a/src/internal/packager/images/push.go +++ b/src/internal/packager/images/push.go @@ -45,9 +45,6 @@ func Push(cfg PushConfig) error { totalSize = totalSize * 2 } - progress := message.NewProgressBar(totalSize, fmt.Sprintf("Pushing %d images", len(cfg.ImageList))) - defer progress.Stop() - var ( err error tunnel *k8s.Tunnel @@ -63,16 +60,21 @@ func Push(cfg PushConfig) error { defer tunnel.Close() } - pushOptions := createPushOpts(cfg, progress) - pushImage := func(img v1.Image, name string) error { - if tunnel != nil { - return tunnel.Wrap(func() error { return crane.Push(img, name, pushOptions...) }) - } - - return crane.Push(img, name, pushOptions...) - } + progress := message.NewProgressBar(totalSize, fmt.Sprintf("Pushing %d images", len(toPush))) + defer progress.Stop() if err := helpers.Retry(func() error { + progress = message.NewProgressBar(totalSize, fmt.Sprintf("Pushing %d images", len(toPush))) + pushOptions := createPushOpts(cfg, progress) + + pushImage := func(img v1.Image, name string) error { + if tunnel != nil { + return tunnel.Wrap(func() error { return crane.Push(img, name, pushOptions...) }) + } + + return crane.Push(img, name, pushOptions...) + } + pushed := []transform.Image{} defer func() { for _, refInfo := range pushed { @@ -83,6 +85,11 @@ func Push(cfg PushConfig) error { refTruncated := helpers.Truncate(refInfo.Reference, 55, true) progress.UpdateTitle(fmt.Sprintf("Pushing %s", refTruncated)) + size, err := calcImgSize(img) + if err != nil { + return err + } + // If this is not a no checksum image push it for use with the Zarf agent if !cfg.NoChecksum { offlineNameCRC, err := transform.ImageTransformHost(registryURL, refInfo.Reference) @@ -95,6 +102,8 @@ func Push(cfg PushConfig) error { if err = pushImage(img, offlineNameCRC); err != nil { return err } + + totalSize -= size } // To allow for other non-zarf workloads to easily see the images upload a non-checksum version @@ -111,6 +120,7 @@ func Push(cfg PushConfig) error { } pushed = append(pushed, refInfo) + totalSize -= size } return nil }, cfg.Retries, 5*time.Second, message.Warnf); err != nil { From c547ca5de7d0048d2d815b38ce145b1a8bd57df1 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 2 May 2024 01:39:35 -0500 Subject: [PATCH 26/43] wacky things Signed-off-by: razzle --- src/internal/packager/images/push.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/internal/packager/images/push.go b/src/internal/packager/images/push.go index f39af4fe26..64e592a5ab 100644 --- a/src/internal/packager/images/push.go +++ b/src/internal/packager/images/push.go @@ -51,19 +51,21 @@ func Push(cfg PushConfig) error { registryURL = cfg.RegInfo.Address ) - c, _ := cluster.NewCluster() - if c != nil { - registryURL, tunnel, err = c.ConnectToZarfRegistryEndpoint(cfg.RegInfo) - if err != nil { - return err - } - defer tunnel.Close() - } - progress := message.NewProgressBar(totalSize, fmt.Sprintf("Pushing %d images", len(toPush))) defer progress.Stop() if err := helpers.Retry(func() error { + c, _ := cluster.NewCluster() + if c != nil { + registryURL, tunnel, err = c.ConnectToZarfRegistryEndpoint(cfg.RegInfo) + if err != nil { + return err + } + if tunnel != nil { + defer tunnel.Close() + } + } + progress = message.NewProgressBar(totalSize, fmt.Sprintf("Pushing %d images", len(toPush))) pushOptions := createPushOpts(cfg, progress) From a6e93925350ba9a2c102195a7df193022fed9562 Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 6 May 2024 12:21:05 -0500 Subject: [PATCH 27/43] Update src/internal/packager/images/pull.go Co-authored-by: Philip Laine --- src/internal/packager/images/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index c4f16c1105..7fd3af922a 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -81,7 +81,7 @@ func Pull(ctx context.Context, cancel context.CancelFunc, cfg PullConfig) (map[t shas := make(map[string]bool) opts := append(CommonOpts(cfg.Arch), crane.WithContext(ctx)) - var fetched = map[transform.Image]v1.Image{} + fetched := map[transform.Image]v1.Image{} var counter, totalBytes atomic.Int64 From 6b931e8c77a7b46caca77f5227495d1faad37113 Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 6 May 2024 12:21:10 -0500 Subject: [PATCH 28/43] Update src/internal/packager/images/pull.go Co-authored-by: Philip Laine --- src/internal/packager/images/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 7fd3af922a..474dd2ed3b 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -78,7 +78,7 @@ func Pull(ctx context.Context, cancel context.CancelFunc, cfg PullConfig) (map[t eg.SetLimit(10) var fetchedLock, shaLock sync.Mutex - shas := make(map[string]bool) + shas := map[string]bool{} opts := append(CommonOpts(cfg.Arch), crane.WithContext(ctx)) fetched := map[transform.Image]v1.Image{} From f19fe0e57aab7fc52d5195ae87a4307cbb6d1269 Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 6 May 2024 12:36:06 -0500 Subject: [PATCH 29/43] handle layout not existing error explicitly Signed-off-by: razzle --- src/internal/packager/images/pull.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 474dd2ed3b..4a03425515 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -7,7 +7,9 @@ package images import ( "context" "encoding/json" + "errors" "fmt" + "io/fs" "maps" "os" "path/filepath" @@ -61,11 +63,13 @@ func Pull(ctx context.Context, cancel context.CancelFunc, cfg PullConfig) (map[t } cranePath, err := clayout.FromPath(cfg.DestinationDirectory) - if err != nil { + if errors.Is(err, fs.ErrNotExist) { cranePath, err = clayout.Write(cfg.DestinationDirectory, empty.Index) if err != nil { return nil, err } + } else { + return nil, err } spinner := message.NewProgressSpinner("Fetching info for %d images. %s", imageCount, longer) From 9997ab124dced0e4066837ce0ad423f22ff3da11 Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 6 May 2024 12:37:25 -0500 Subject: [PATCH 30/43] defer unlocks Signed-off-by: razzle --- src/internal/packager/images/pull.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 4a03425515..8fd863c3c7 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -203,6 +203,7 @@ func Pull(ctx context.Context, cancel context.CancelFunc, cfg PullConfig) (map[t } shaLock.Lock() + defer shaLock.Unlock() if _, ok := shas[digest.Hex]; !ok { shas[digest.Hex] = true size, err := layer.Size() @@ -211,7 +212,6 @@ func Pull(ctx context.Context, cancel context.CancelFunc, cfg PullConfig) (map[t } totalBytes.Add(size) } - shaLock.Unlock() } fetchedLock.Lock() @@ -346,6 +346,7 @@ func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[transform.Image] } mu.Lock() + defer mu.Unlock() annotations := map[string]string{ ocispec.AnnotationBaseImageName: info.Reference, } @@ -353,7 +354,6 @@ func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[transform.Image] if err := cl.AppendDescriptor(*desc); err != nil { return err } - mu.Unlock() saved[info] = img return nil From c10722c091b9661cd5250884ab9e6d00a0c05429 Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 6 May 2024 12:45:06 -0500 Subject: [PATCH 31/43] better use of context Signed-off-by: razzle --- src/internal/packager/images/pull.go | 12 +----------- src/pkg/packager/creator/normal.go | 4 ++-- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 8fd863c3c7..9755b85806 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -24,7 +24,6 @@ import ( "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/pkg/transform" "github.com/defenseunicorns/zarf/src/pkg/utils" - "github.com/docker/docker/errdefs" "github.com/google/go-containerregistry/pkg/crane" "github.com/google/go-containerregistry/pkg/logs" "github.com/google/go-containerregistry/pkg/name" @@ -48,7 +47,7 @@ type ImgInfo struct { } // Pull pulls all of the images from the given config. -func Pull(ctx context.Context, cancel context.CancelFunc, cfg PullConfig) (map[transform.Image]v1.Image, error) { +func Pull(ctx context.Context, cfg PullConfig) (map[transform.Image]v1.Image, error) { var longer string imageCount := len(cfg.References) // Give some additional user feedback on larger image sets @@ -119,7 +118,6 @@ func Pull(ctx context.Context, cancel context.CancelFunc, cfg PullConfig) (map[t desc, err = crane.Get(ref, opts...) if err != nil { if strings.Contains(err.Error(), "unexpected status code 429 Too Many Requests") { - cancel() return fmt.Errorf("rate limited by registry: %w", err) } @@ -135,10 +133,6 @@ func Pull(ctx context.Context, cancel context.CancelFunc, cfg PullConfig) (map[t // Inspect the image to get the size. rawImg, _, err := cli.ImageInspectWithRaw(ctx, ref) if err != nil { - if errdefs.IsNotFound(err) { - cancel() - } - return err } @@ -179,7 +173,6 @@ func Pull(ctx context.Context, cancel context.CancelFunc, cfg PullConfig) (map[t lines = append(lines, fmt.Sprintf("\n(%s) %s@%s", desc.Platform, name, desc.Digest)) } message.Warn(strings.Join(lines, "\n")) - cancel() return fmt.Errorf("%s resolved to an index, please select a specific platform to use", refInfo.Reference) } @@ -225,8 +218,6 @@ func Pull(ctx context.Context, cancel context.CancelFunc, cfg PullConfig) (map[t return nil, err } - clear(shas) - spinner.Successf("Fetched info for %d images", imageCount) doneSaving := make(chan error) @@ -273,7 +264,6 @@ func CleanupInProgressLayers(ctx context.Context, img v1.Image) error { return err } eg, _ := errgroup.WithContext(ctx) - eg.SetLimit(-1) for _, layer := range layers { layer := layer eg.Go(func() error { diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 75885b79b0..2f0fab990c 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -178,7 +178,7 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. dst.AddImages() - ctx, cancel := context.WithCancel(context.TODO()) + ctx := context.TODO() pullCfg := images.PullConfig{ DestinationDirectory: dst.Images.Base, @@ -188,7 +188,7 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. CacheDirectory: filepath.Join(config.GetAbsCachePath(), layout.ImagesDir), } - pulled, err := images.Pull(ctx, cancel, pullCfg) + pulled, err := images.Pull(ctx, pullCfg) if err != nil { return err } From 24e60389a193ecdb6b34668c4712e1bb428ee501 Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 6 May 2024 13:24:10 -0500 Subject: [PATCH 32/43] no need to re-declare if not using go-routines Signed-off-by: razzle --- src/pkg/packager/creator/normal.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 2f0fab990c..93675d3110 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -194,7 +194,6 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. } for info, img := range pulled { - info, img := info, img if err := dst.Images.AddV1Image(img); err != nil { return err } From 2c3eb00a0c847adfe55a763b9886d6325f4a8428 Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 6 May 2024 14:02:19 -0500 Subject: [PATCH 33/43] cleanup Signed-off-by: razzle --- src/pkg/packager/deploy.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index 47333afe5e..e4d25dec62 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -481,10 +481,6 @@ func (p *Packager) populatePackageVariableConfig() error { // Push all of the components images to the configured container registry. func (p *Packager) pushImagesToRegistry(componentImages []string, noImgChecksum bool) error { - if len(componentImages) == 0 { - return nil - } - var combinedImageList []transform.Image for _, src := range componentImages { ref, err := transform.ParseImageRef(src) From 903113c8e0a4a1b1e961ac27b45cd8c092659cec Mon Sep 17 00:00:00 2001 From: razzle Date: Tue, 7 May 2024 09:48:59 -0500 Subject: [PATCH 34/43] better use of context Signed-off-by: razzle --- src/internal/packager/images/pull.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 9755b85806..b3e271183c 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -77,12 +77,12 @@ func Pull(ctx context.Context, cfg PullConfig) (map[transform.Image]v1.Image, er logs.Warn.SetOutput(&message.DebugWriter{}) logs.Progress.SetOutput(&message.DebugWriter{}) - eg, _ := errgroup.WithContext(ctx) + eg, ectx := errgroup.WithContext(ctx) eg.SetLimit(10) var fetchedLock, shaLock sync.Mutex shas := map[string]bool{} - opts := append(CommonOpts(cfg.Arch), crane.WithContext(ctx)) + opts := CommonOpts(cfg.Arch) fetched := map[transform.Image]v1.Image{} @@ -128,10 +128,10 @@ func Pull(ctx context.Context, cfg PullConfig) (map[transform.Image]v1.Image, er if err != nil { return fmt.Errorf("docker not available: %w", err) } - cli.NegotiateAPIVersion(ctx) + cli.NegotiateAPIVersion(ectx) // Inspect the image to get the size. - rawImg, _, err := cli.ImageInspectWithRaw(ctx, ref) + rawImg, _, err := cli.ImageInspectWithRaw(ectx, ref) if err != nil { return err } @@ -145,7 +145,7 @@ func Pull(ctx context.Context, cfg PullConfig) (map[transform.Image]v1.Image, er // Use unbuffered opener to avoid OOM Kill issues https://github.com/defenseunicorns/zarf/issues/1214. // This will also take forever to load large images. - img, err = daemon.Image(reference, daemon.WithUnbufferedOpener(), daemon.WithContext(ctx)) + img, err = daemon.Image(reference, daemon.WithUnbufferedOpener()) if err != nil { return fmt.Errorf("failed to load from docker daemon: %w", err) } @@ -317,7 +317,7 @@ func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[transform.Image] var mu sync.Mutex - eg, _ := errgroup.WithContext(ctx) + eg, ectx := errgroup.WithContext(ctx) eg.SetLimit(10) for info, img := range m { @@ -329,7 +329,7 @@ func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[transform.Image] } if err := cl.WriteImage(img); err != nil { - if err = CleanupInProgressLayers(ctx, img); err != nil { + if err = CleanupInProgressLayers(ectx, img); err != nil { message.WarnErr(err, "failed to clean up in-progress layers, please remove them manually") } return err From 604917d1bfa2ebe7cf198f0123a10bfb3ff28b6b Mon Sep 17 00:00:00 2001 From: razzle Date: Wed, 8 May 2024 14:20:08 -0500 Subject: [PATCH 35/43] always create clayout Signed-off-by: razzle --- go.mod | 2 +- src/internal/packager/images/common.go | 6 +++--- src/internal/packager/images/pull.go | 11 ++--------- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 931d43fb71..414233a7cf 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,6 @@ require ( github.com/defenseunicorns/pkg/oci v0.0.1 github.com/derailed/k9s v0.31.7 github.com/distribution/reference v0.5.0 - github.com/docker/docker v24.0.9+incompatible github.com/fairwindsops/pluto/v5 v5.18.4 github.com/fatih/color v1.16.0 github.com/fluxcd/helm-controller/api v0.37.4 @@ -205,6 +204,7 @@ require ( github.com/dimchansky/utfbom v1.1.1 // indirect github.com/docker/cli v26.0.0+incompatible // indirect github.com/docker/distribution v2.8.3+incompatible // indirect + github.com/docker/docker v24.0.9+incompatible // indirect github.com/docker/docker-credential-helpers v0.8.0 // indirect github.com/docker/go-connections v0.4.0 // indirect github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect diff --git a/src/internal/packager/images/common.go b/src/internal/packager/images/common.go index 64bb88f17c..16602e3ef1 100644 --- a/src/internal/packager/images/common.go +++ b/src/internal/packager/images/common.go @@ -67,18 +67,18 @@ func WithGlobalInsecureFlag() []crane.Option { return []crane.Option{NoopOpt} } -// WithArchictecture sets the platform option for crane. +// WithArchitecture sets the platform option for crane. // // This option is actually a slight mis-use of the platform option, as it is // setting the architecture only and hard coding the OS to linux. -func WithArchictecture(arch string) crane.Option { +func WithArchitecture(arch string) crane.Option { return crane.WithPlatform(&v1.Platform{OS: "linux", Architecture: arch}) } // CommonOpts returns a set of common options for crane under Zarf. func CommonOpts(arch string) []crane.Option { opts := WithGlobalInsecureFlag() - opts = append(opts, WithArchictecture(arch)) + opts = append(opts, WithArchitecture(arch)) opts = append(opts, crane.WithUserAgent("zarf"), diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index b3e271183c..7486457cbb 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -7,9 +7,7 @@ package images import ( "context" "encoding/json" - "errors" "fmt" - "io/fs" "maps" "os" "path/filepath" @@ -61,13 +59,8 @@ func Pull(ctx context.Context, cfg PullConfig) (map[transform.Image]v1.Image, er return nil, fmt.Errorf("failed to create image path %s: %w", cfg.DestinationDirectory, err) } - cranePath, err := clayout.FromPath(cfg.DestinationDirectory) - if errors.Is(err, fs.ErrNotExist) { - cranePath, err = clayout.Write(cfg.DestinationDirectory, empty.Index) - if err != nil { - return nil, err - } - } else { + cranePath, err := clayout.Write(cfg.DestinationDirectory, empty.Index) + if err != nil { return nil, err } From ffe348d4691326a246bc966c37d9ab5859f34ed4 Mon Sep 17 00:00:00 2001 From: razzle Date: Wed, 8 May 2024 14:50:25 -0500 Subject: [PATCH 36/43] fix shaLock deadlock Signed-off-by: razzle --- src/internal/packager/images/pull.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 7486457cbb..abb2560fcd 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -73,7 +73,7 @@ func Pull(ctx context.Context, cfg PullConfig) (map[transform.Image]v1.Image, er eg, ectx := errgroup.WithContext(ctx) eg.SetLimit(10) - var fetchedLock, shaLock sync.Mutex + var shaLock sync.Mutex shas := map[string]bool{} opts := CommonOpts(cfg.Arch) @@ -182,14 +182,14 @@ func Pull(ctx context.Context, cfg PullConfig) (map[transform.Image]v1.Image, er return fmt.Errorf("unable to get layers for %s: %w", refInfo.Reference, err) } + shaLock.Lock() + defer shaLock.Unlock() for _, layer := range layers { digest, err := layer.Digest() if err != nil { return fmt.Errorf("unable to get digest for image layer: %w", err) } - shaLock.Lock() - defer shaLock.Unlock() if _, ok := shas[digest.Hex]; !ok { shas[digest.Hex] = true size, err := layer.Size() @@ -200,9 +200,12 @@ func Pull(ctx context.Context, cfg PullConfig) (map[transform.Image]v1.Image, er } } - fetchedLock.Lock() + if img == nil { + return fmt.Errorf("failed to fetch image %s", refInfo.Reference) + } + fetched[refInfo] = img - fetchedLock.Unlock() + return nil }) } From fd47445faaea676e32be6c219142656a3be5341a Mon Sep 17 00:00:00 2001 From: razzle Date: Fri, 10 May 2024 02:08:05 -0500 Subject: [PATCH 37/43] changes from review Signed-off-by: razzle --- src/internal/packager/images/common.go | 12 ++---------- src/internal/packager/images/pull.go | 10 ++-------- src/pkg/packager/creator/normal.go | 2 +- 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/src/internal/packager/images/common.go b/src/internal/packager/images/common.go index 16602e3ef1..9fca5c0a0c 100644 --- a/src/internal/packager/images/common.go +++ b/src/internal/packager/images/common.go @@ -22,7 +22,7 @@ import ( type PullConfig struct { DestinationDirectory string - References []transform.Image + ImageList []transform.Image Arch string @@ -52,16 +52,8 @@ func NoopOpt(*crane.Options) {} // WithGlobalInsecureFlag returns an option for crane that configures insecure // based upon Zarf's global --insecure flag. func WithGlobalInsecureFlag() []crane.Option { - opts := []crane.Option{} if config.CommonOptions.Insecure { - opts = append(opts, crane.Insecure) - - // TODO: lets see what happens when we comment this guy out - // roundTripper := http.DefaultTransport.(*http.Transport).Clone() - // roundTripper.TLSClientConfig.InsecureSkipVerify = true - - // opts = append(opts, crane.WithTransport(roundTripper)) - return opts + return []crane.Option{crane.Insecure} } // passing a nil option will cause panic return []crane.Option{NoopOpt} diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index abb2560fcd..92a2e6717d 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -38,16 +38,10 @@ import ( "golang.org/x/sync/errgroup" ) -// ImgInfo wraps references/information about an image -type ImgInfo struct { - RefInfo transform.Image - Img v1.Image -} - // Pull pulls all of the images from the given config. func Pull(ctx context.Context, cfg PullConfig) (map[transform.Image]v1.Image, error) { var longer string - imageCount := len(cfg.References) + imageCount := len(cfg.ImageList) // Give some additional user feedback on larger image sets if imageCount > 15 { longer = "This step may take a couple of minutes to complete." @@ -81,7 +75,7 @@ func Pull(ctx context.Context, cfg PullConfig) (map[transform.Image]v1.Image, er var counter, totalBytes atomic.Int64 - for _, refInfo := range cfg.References { + for _, refInfo := range cfg.ImageList { refInfo := refInfo eg.Go(func() error { idx := counter.Add(1) diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 860ed3fe16..0cb5c7d921 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -184,7 +184,7 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. pullCfg := images.PullConfig{ DestinationDirectory: dst.Images.Base, - References: imageList, + ImageList: imageList, Arch: arch, RegistryOverrides: pc.createOpts.RegistryOverrides, CacheDirectory: filepath.Join(config.GetAbsCachePath(), layout.ImagesDir), From b5d1479e58c0dfa597608b4782b5cabdb7a57bc2 Mon Sep 17 00:00:00 2001 From: razzle Date: Fri, 10 May 2024 13:09:26 -0500 Subject: [PATCH 38/43] changes from review Signed-off-by: razzle --- src/internal/packager/images/pull.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 92a2e6717d..bdcd57869d 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -286,7 +286,6 @@ func CleanupInProgressLayers(ctx context.Context, img v1.Image) error { func SaveSequential(ctx context.Context, cl clayout.Path, m map[transform.Image]v1.Image) (map[transform.Image]v1.Image, error) { saved := map[transform.Image]v1.Image{} for info, img := range m { - info, img := info, img annotations := map[string]string{ ocispec.AnnotationBaseImageName: info.Reference, } From 0679740fc0de6b7f2be97691d28c9355d68cdc68 Mon Sep 17 00:00:00 2001 From: razzle Date: Fri, 10 May 2024 13:09:52 -0500 Subject: [PATCH 39/43] Update src/internal/packager/images/pull.go Co-authored-by: Lucas Rodriguez --- src/internal/packager/images/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index bdcd57869d..1697156274 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -279,7 +279,7 @@ func CleanupInProgressLayers(ctx context.Context, img v1.Image) error { return nil }) } - return nil + return eg.Wait() } // SaveSequential saves images sequentially. From e867b07e554c9bd32b8b6da3cbe8bf7207dfbebe Mon Sep 17 00:00:00 2001 From: razzle Date: Fri, 10 May 2024 14:54:12 -0500 Subject: [PATCH 40/43] go mod tidy Signed-off-by: razzle --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index d2d9708f77..1a6fa37ec6 100644 --- a/go.mod +++ b/go.mod @@ -46,6 +46,7 @@ require ( github.com/stretchr/testify v1.9.0 github.com/xeipuuv/gojsonschema v1.2.0 golang.org/x/crypto v0.23.0 + golang.org/x/sync v0.6.0 golang.org/x/term v0.20.0 helm.sh/helm/v3 v3.14.2 k8s.io/api v0.29.1 @@ -474,7 +475,6 @@ require ( golang.org/x/mod v0.14.0 // indirect golang.org/x/net v0.25.0 // indirect golang.org/x/oauth2 v0.17.0 // indirect - golang.org/x/sync v0.6.0 // indirect golang.org/x/sys v0.20.0 // indirect golang.org/x/text v0.15.0 // indirect golang.org/x/time v0.5.0 // indirect From 08096fa61f26490d341da4221a300cffc7bf41c8 Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 13 May 2024 12:55:49 -0500 Subject: [PATCH 41/43] thanks austin Signed-off-by: razzle --- src/internal/packager/images/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 1697156274..3cbfc2a0ad 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -266,7 +266,7 @@ func CleanupInProgressLayers(ctx context.Context, img v1.Image) error { return err } cacheDir := filepath.Join(config.GetAbsCachePath(), layout.ImagesDir) - location := filepath.Join(cacheDir, digest.Hex) + location := filepath.Join(cacheDir, digest.String()) info, err := os.Stat(location) if err != nil { return err From 82ae27b096d277ada3dbb5635f61ca1099f3260d Mon Sep 17 00:00:00 2001 From: razzle Date: Mon, 13 May 2024 13:55:00 -0500 Subject: [PATCH 42/43] is now working w/ nvidia package Signed-off-by: razzle --- src/internal/packager/images/pull.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 3cbfc2a0ad..c440f8d33d 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -7,7 +7,9 @@ package images import ( "context" "encoding/json" + "errors" "fmt" + "io/fs" "maps" "os" "path/filepath" @@ -268,6 +270,9 @@ func CleanupInProgressLayers(ctx context.Context, img v1.Image) error { cacheDir := filepath.Join(config.GetAbsCachePath(), layout.ImagesDir) location := filepath.Join(cacheDir, digest.String()) info, err := os.Stat(location) + if errors.Is(err, fs.ErrNotExist) { + return nil + } if err != nil { return err } @@ -290,8 +295,8 @@ func SaveSequential(ctx context.Context, cl clayout.Path, m map[transform.Image] ocispec.AnnotationBaseImageName: info.Reference, } if err := cl.AppendImage(img, clayout.WithAnnotations(annotations)); err != nil { - if err = CleanupInProgressLayers(ctx, img); err != nil { - message.WarnErr(err, "failed to clean up in-progress layers, please remove them manually") + if err := CleanupInProgressLayers(ctx, img); err != nil { + message.WarnErr(err, "failed to clean up in-progress layers, please run `zarf tools clear-cache`") } return saved, err } @@ -318,8 +323,8 @@ func SaveConcurrent(ctx context.Context, cl clayout.Path, m map[transform.Image] } if err := cl.WriteImage(img); err != nil { - if err = CleanupInProgressLayers(ectx, img); err != nil { - message.WarnErr(err, "failed to clean up in-progress layers, please remove them manually") + if err := CleanupInProgressLayers(ectx, img); err != nil { + message.WarnErr(err, "failed to clean up in-progress layers, please run `zarf tools clear-cache`") } return err } From 66cc1f628b2aaa3c3cc88b0c228a9ffe1b02be90 Mon Sep 17 00:00:00 2001 From: razzle Date: Tue, 14 May 2024 13:44:30 -0500 Subject: [PATCH 43/43] go mod tidy Signed-off-by: razzle --- go.mod | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 0e58a689a1..9fe6c7a968 100644 --- a/go.mod +++ b/go.mod @@ -46,7 +46,7 @@ require ( github.com/stretchr/testify v1.9.0 github.com/xeipuuv/gojsonschema v1.2.0 golang.org/x/crypto v0.23.0 - golang.org/x/sync v0.6.0 + golang.org/x/sync v0.7.0 golang.org/x/term v0.20.0 helm.sh/helm/v3 v3.14.2 k8s.io/api v0.29.1 @@ -477,7 +477,6 @@ require ( golang.org/x/mod v0.14.0 // indirect golang.org/x/net v0.25.0 // indirect golang.org/x/oauth2 v0.20.0 // indirect - golang.org/x/sync v0.7.0 // indirect golang.org/x/sys v0.20.0 // indirect golang.org/x/text v0.15.0 // indirect golang.org/x/time v0.5.0 // indirect