Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

optimize checking for current digest #1665

Merged
merged 2 commits into from Dec 12, 2022
Merged

optimize checking for current digest #1665

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
049c540
optimize checking for current digest
pkwarren Dec 9, 2022
f28ddcb
Merge branch 'main' into pkw/check-current-digest-first
pkwarren Dec 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 41 additions & 14 deletions private/buf/cmd/buf/command/alpha/plugin/pluginpush/pluginpush.go
View file
Open in desktop
Expand Up @@ -236,6 +236,7 @@ func run(
Revision: 0, // get latest revision for the plugin version.
}),
)
var currentImageDigest string
var nextRevision uint32
if err != nil {
if connect.CodeOf(err) != connect.CodeNotFound {
Expand All @@ -244,6 +245,7 @@ func run(
nextRevision = 1
} else {
nextRevision = latestPluginResp.Msg.Plugin.Revision + 1
currentImageDigest = latestPluginResp.Msg.Plugin.ContainerImageDigest
}
machine, err := netrc.GetMachineForName(container, pluginConfig.Name.Remote())
if err != nil {
Expand All @@ -255,7 +257,7 @@ func run(
authConfig.Username = machine.Login()
authConfig.Password = machine.Password()
}
imageDigest, err := findExistingDigestForImageID(ctx, pluginConfig, authConfig, imageID)
imageDigest, err := findExistingDigestForImageID(ctx, pluginConfig, authConfig, imageID, currentImageDigest)
if err != nil {
return err
}
Expand Down Expand Up @@ -366,7 +368,13 @@ func pushImage(
// - For each tag:
// - Fetch image: GET /v2/{owner}/{plugin}/manifests/{tag}
// - If image manifest matches imageID, we can use the image digest for the image.
func findExistingDigestForImageID(ctx context.Context, plugin *bufpluginconfig.Config, authConfig *bufplugindocker.RegistryAuthConfig, imageID string) (string, error) {
func findExistingDigestForImageID(
ctx context.Context,
plugin *bufpluginconfig.Config,
authConfig *bufplugindocker.RegistryAuthConfig,
imageID string,
currentImageDigest string,
) (string, error) {
pluginsRemote := plugin.Name.Remote()
if !strings.HasPrefix(pluginsRemote, bufplugindocker.PluginsImagePrefix) {
pluginsRemote = bufplugindocker.PluginsImagePrefix + pluginsRemote
Expand All @@ -376,7 +384,19 @@ func findExistingDigestForImageID(ctx context.Context, plugin *bufpluginconfig.C
return "", err
}
auth := &authn.Basic{Username: authConfig.Username, Password: authConfig.Password}
tags, err := remote.List(repo, remote.WithContext(ctx), remote.WithAuth(auth))
remoteOpts := []remote.Option{remote.WithContext(ctx), remote.WithAuth(auth)}
// First attempt to see if the current image digest matches the image ID
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot faster in the case of the image already existing in the BSR with the same digest as before. It only has to check one digest (the current one) and not potentially search through dozens of tags.

if currentImageDigest != "" {
remoteImageID, _, err := getImageIDAndDigestFromReference(repo.Digest(currentImageDigest), remoteOpts...)
if err != nil {
return "", err
}
if remoteImageID == imageID {
return currentImageDigest, nil
}
}
// List all tags and check for a match
tags, err := remote.List(repo, remoteOpts...)
if err != nil {
structuredErr := new(transport.Error)
if errors.As(err, &structuredErr) {
Expand All @@ -388,27 +408,34 @@ func findExistingDigestForImageID(ctx context.Context, plugin *bufpluginconfig.C
}
existingImageDigest := ""
for _, tag := range tags {
image, err := remote.Image(repo.Tag(tag), remote.WithContext(ctx), remote.WithAuth(auth))
remoteImageID, imageDigest, err := getImageIDAndDigestFromReference(repo.Tag(tag), remoteOpts...)
if err != nil {
return "", err
}
manifest, err := image.Manifest()
if err != nil {
return "", err
}
remoteImageID := manifest.Config.Digest.String()
if remoteImageID == imageID {
imageDigest, err := image.Digest()
if err != nil {
return "", err
}
existingImageDigest = imageDigest.String()
existingImageDigest = imageDigest
break
}
}
return existingImageDigest, nil
}

func getImageIDAndDigestFromReference(ref name.Reference, options ...remote.Option) (string, string, error) {
image, err := remote.Image(ref, options...)
if err != nil {
return "", "", err
}
imageDigest, err := image.Digest()
if err != nil {
return "", "", err
}
manifest, err := image.Manifest()
if err != nil {
return "", "", err
}
return manifest.Config.Digest.String(), imageDigest.String(), nil
}

func unzipPluginToSourceBucket(ctx context.Context, pluginZip string, size int64, bucket storage.ReadWriteBucket) (retErr error) {
f, err := os.Open(pluginZip)
if err != nil {
Expand Down