Checking existence of image in registry before attempting push

[AdamSvetec]

Hi,

I am hitting some performance issues when using rules_k8s to generate all of my resource yamls.

With the number of images and environments we have, a huge number of image pushes are being done for images that already exist in the registry. This large amount of concurrent connections is putting load on our registry service and causing issues when scaled across all our build machines.

I hacked something up to enable checking the remote registry for the given digest/tag before trying to upload which seems to work. Is there any reason this isn't done already? I can get PR up if conceptually you think this would make sense to check remote registry with locally generated digest before attempting upload.

[AdamSvetec]

To add some color, we are seeing this degradation in performance after upgrading from v0.1 to v0.5. Previously in containerregistry, the session.Push() call checked for existence of remote image before pushing and would not push if digest already existed remotely. Now in go-containerregistry, the remote.Write() call does not do this check, and causes large increase in network traffic to registry.

It seems this can be solved with remote.Get(). does that sounds reasonable? I can open PR if you agree.

[pcmoritz]

@AdamSvetec If you have the patch for this available, this would be very useful for us too :)

[AdamSvetec]

Hey @pcmoritz , sorry I never got the chance to push this. Attached is diff, if you wouldn't mind getting this over the finish line, you'd be my hero.

`diff --git a/container/go/cmd/pusher/pusher.go b/container/go/cmd/pusher/pusher.go index 0f00b6b..4fac73c 100644 --- a/container/go/cmd/pusher/pusher.go +++ b/container/go/cmd/pusher/pusher.go @@ -22,7 +22,6 @@ import ( "log" "net/http" "os"

•   "io"
    "path"
    "strings"
  

@@ -47,35 +46,12 @@ var ( skipUnchangedDigest = flag.Bool("skip-unchanged-digest", false, "If set to true, will only push images where the digest has changed.") layers utils.ArrayStringFlags stampInfoFile utils.ArrayStringFlags

• Info *log.Logger
• Warning *log.Logger
• Error *log.Logger

)

type dockerHeaders struct { HTTPHeaders map[string]string json:"HttpHeaders,omitempty" }

-func InitLogger(

• infoHandle io.Writer,
• warningHandle io.Writer,
• errorHandle io.Writer) {
• Info = log.New(infoHandle,

•    "INFO: ",
  

•    log.Ldate|log.Ltime|log.Lshortfile)
  

• Warning = log.New(warningHandle,

•    "WARNING: ",
  

•    log.Ldate|log.Ltime|log.Lshortfile)
  

• Error = log.New(errorHandle,

•    "ERROR: ",
  

•    log.Ldate|log.Ltime|log.Lshortfile)
  

-}

// checkClientConfig ensures the given string represents a valid docker client // config by ensuring: // 1. It's a valid filesystem path. @@ -95,26 +71,25 @@ func checkClientConfig(configDir string) error { }

func main() {

•   InitLogger(os.Stdout, os.Stdout, os.Stderr)
    flag.Var(&layers, "layer", "One or more layers with the following comma separated values (Compressed layer tarball, Uncompressed layer tarball, digest file, diff ID file). e.g., --layer layer.tar.gz,layer.tar,<file with digest>,<file with diffID>.")
    flag.Var(&stampInfoFile, "stamp-info-file", "The list of paths to the stamp info files used to substitute supported attribute when a python format placeholder is provivided in dst, e.g., {BUILD_USER}.")
    flag.Parse()
  
    if *dst == "" {
  

•           Error.Fatalln("Required option -dst was not specified.")
  

•           log.Fatalln("Required option -dst was not specified.")
    }
    if *format == "" {
  

•           Error.Fatalln("Required option -format was not specified.")
  

•           log.Fatalln("Required option -format was not specified.")
    }
    if *imgConfig == "" {
  

•           Error.Fatalln("Option --config is required.")
  

•           log.Fatalln("Option --config is required.")
    }
  
    // If the user provided a client config directory, ensure it's a valid
    // directory and instruct the keychain resolver to use it to look for the
    // docker client config.
    if err := checkClientConfig(*clientConfigDir); err != nil {
  

•           Error.Fatalf("Failed to validate the Docker client config dir %q specified via --client-config-dir: %v", *clientConfigDir, err)
  

•           log.Fatalf("Failed to validate the Docker client config dir %q specified via --client-config-dir: %v", *clientConfigDir, err)
    }
    if *clientConfigDir != "" {
            os.Setenv("DOCKER_CONFIG", *clientConfigDir)
  

@@ -122,38 +97,43 @@ func main() {

    imgParts, err := compat.ImagePartsFromArgs(*imgConfig, *baseManifest, *imgTarball, layers)
    if err != nil {

•           Error.Fatalf("Unable to determine parts of the image from the specified arguments: %v", err)
  

•           log.Fatalf("Unable to determine parts of the image from the specified arguments: %v", err)
    }
    img, err := compat.ReadImage(imgParts)
    if err != nil {
  

•           Error.Fatalf("Error reading image: %v", err)
  

•           log.Fatalf("Error reading image: %v", err)
    }
    if *format == "OCI" {
            img, err = oci.AsOCIImage(img)
            if err != nil {
  

•                   Error.Fatalf("Failed to convert image to OCI format: %v", err)
  

•                   log.Fatalf("Failed to convert image to OCI format: %v", err)
            }
    }
  
    stamper, err := compat.NewStamper(stampInfoFile)
    if err != nil {
  

•           Error.Fatalf("Failed to initialize the stamper: %v", err)
  

•           log.Fatalf("Failed to initialize the stamper: %v", err)
    }
  
    // Infer stamp info if provided and perform substitutions in the provided tag name.
    stamped := stamper.Stamp(*dst)
    if stamped != *dst {
  

•           Info.Printf("Destination %s was resolved to %s after stamping.", *dst, stamped)
  

•           log.Printf("Destination %s was resolved to %s after stamping.", *dst, stamped)
    }
  
    if err := push(stamped, img); err != nil {
  

•           Error.Fatalf("Error pushing image to %s: %v", stamped, err)
  

•           log.Fatalf("Error pushing image to %s: %v", stamped, err)
    }
  

•   log.Printf("Successfully pushed %s image to %s", *format, stamped)
  

}

// digestExists checks whether an image's digest exists in a repository. -func digestExists(dst string, img v1.Image, digest v1.Hash) (bool, error) { +func digestExists(dst string, img v1.Image) (bool, error) {

•   digest, err := img.Digest()
  

•   if err != nil {
  

•           return false, errors.Wrapf(err, "unable to get local image digest")
  

•   }
    digestRef, err := name.NewDigest(fmt.Sprintf("%s@%s", dst, digest))
    if err != nil {
            return false, errors.Wrapf(err, "couldn't create ref from digest")
  

@@ -180,20 +160,13 @@ func push(dst string, img v1.Image) error { return errors.Wrapf(err, "error parsing %q as an image reference", dst) }

•   digest, err := img.Digest()
  

•   if err != nil {
  

•           return errors.Wrapf(err, "unable to get local image digest")
  

•   } else {
  

•           Info.Printf("Calculated local image digest to be %s", digest)
  

•   }
  

•   if *skipUnchangedDigest {
  

•           exists, err := digestExists(dst, img, digest)
  

•           exists, err := digestExists(dst, img)
            if err != nil {
  

•                   Warning.Printf("Error checking if digest already exists %v. Still pushing", err)
  

•                   log.Printf("Error checking if digest already exists %v. Still pushing", err)
            }
            if exists {
  

•                   Info.Print("Skipping push of unchanged digest")
  

•                   log.Print("Skipping push of unchanged digest")
                    return nil
            }
    }
  

@@ -225,7 +198,6 @@ func push(dst string, img v1.Image) error { return errors.Wrapf(err, "unable to push image to %s", dst) }

•   Info.Printf("Successfully pushed image to %s", dst)
    return nil
  

} `