-
Notifications
You must be signed in to change notification settings - Fork 508
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.
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
Start to flesh out crane optimize. #879
Changes from all commits
b6faed6
f4b6650
c1b9c5c
fbf72a6
4ca4734
48acc2c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
// Copyright 2020 Google LLC All Rights Reserved. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package cmd | ||
|
||
import ( | ||
"log" | ||
|
||
"github.com/google/go-containerregistry/pkg/crane" | ||
"github.com/spf13/cobra" | ||
) | ||
|
||
// NewCmdOptimize creates a new cobra.Command for the optimize subcommand. | ||
func NewCmdOptimize(options *[]crane.Option) *cobra.Command { | ||
var files []string | ||
|
||
cmd := &cobra.Command{ | ||
Use: "optimize SRC DST", | ||
Hidden: true, | ||
Aliases: []string{"opt"}, | ||
Short: "Optimize a remote container image from src to dst", | ||
Args: cobra.ExactArgs(2), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice if this allowed one arg and would just retag the optimized version, with an optional There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IDK how I feel about in-place optimization, but most of the places I want this for are things that aren't yet producing these images. |
||
Run: func(_ *cobra.Command, args []string) { | ||
src, dst := args[0], args[1] | ||
if err := crane.Optimize(src, dst, files, *options...); err != nil { | ||
log.Fatal(err) | ||
} | ||
}, | ||
} | ||
|
||
cmd.Flags().StringSliceVar(&files, "prioritize", nil, | ||
"The list of files to prioritize in the optimized image.") | ||
|
||
return cmd | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,179 @@ | ||
// Copyright 2020 Google LLC All Rights Reserved. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package crane | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
|
||
"github.com/containerd/stargz-snapshotter/estargz" | ||
"github.com/google/go-containerregistry/pkg/logs" | ||
"github.com/google/go-containerregistry/pkg/name" | ||
v1 "github.com/google/go-containerregistry/pkg/v1" | ||
"github.com/google/go-containerregistry/pkg/v1/empty" | ||
"github.com/google/go-containerregistry/pkg/v1/mutate" | ||
"github.com/google/go-containerregistry/pkg/v1/remote" | ||
"github.com/google/go-containerregistry/pkg/v1/tarball" | ||
"github.com/google/go-containerregistry/pkg/v1/types" | ||
) | ||
|
||
// Optimize optimizes a remote image or index from src to dst. | ||
// THIS API IS EXPERIMENTAL AND SUBJECT TO CHANGE WITHOUT WARNING. | ||
func Optimize(src, dst string, prioritize []string, opt ...Option) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My "as long as it's hidden" concession for the command also kind of applies here. I don't think we should expose this in pkg/crane until we're happy with exposing it in the CLI as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given how things are structured, that isn't particularly feasible (without deviating significantly from the broader style). What did you have in mind? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could keep this under cmd/crane/cmd, which is what I've done for |
||
o := makeOptions(opt...) | ||
srcRef, err := name.ParseReference(src, o.name...) | ||
if err != nil { | ||
return fmt.Errorf("parsing reference %q: %v", src, err) | ||
} | ||
|
||
dstRef, err := name.ParseReference(dst, o.name...) | ||
if err != nil { | ||
return fmt.Errorf("parsing reference for %q: %v", dst, err) | ||
} | ||
|
||
logs.Progress.Printf("Optimizing from %v to %v", srcRef, dstRef) | ||
desc, err := remote.Get(srcRef, o.remote...) | ||
if err != nil { | ||
return fmt.Errorf("fetching %q: %v", src, err) | ||
} | ||
|
||
switch desc.MediaType { | ||
case types.OCIImageIndex, types.DockerManifestList: | ||
// Handle indexes separately. | ||
if o.platform != nil { | ||
// If platform is explicitly set, don't optimize the whole index, just the appropriate image. | ||
if err := optimizeAndPushImage(desc, dstRef, prioritize, o); err != nil { | ||
return fmt.Errorf("failed to optimize image: %v", err) | ||
} | ||
} else { | ||
if err := optimizeAndPushIndex(desc, dstRef, prioritize, o); err != nil { | ||
return fmt.Errorf("failed to optimize index: %v", err) | ||
} | ||
} | ||
|
||
case types.DockerManifestSchema1, types.DockerManifestSchema1Signed: | ||
return errors.New("docker schema 1 images are not supported") | ||
|
||
default: | ||
// Assume anything else is an image, since some registries don't set mediaTypes properly. | ||
if err := optimizeAndPushImage(desc, dstRef, prioritize, o); err != nil { | ||
return fmt.Errorf("failed to optimize image: %v", err) | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func optimizeAndPushImage(desc *remote.Descriptor, dstRef name.Reference, prioritize []string, o options) error { | ||
img, err := desc.Image() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
oimg, err := optimizeImage(img, prioritize) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
return remote.Write(dstRef, oimg, o.remote...) | ||
} | ||
|
||
func optimizeImage(img v1.Image, prioritize []string) (v1.Image, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What you're doing follows the patterns from It might also make sense for some of this boilerplate to live in |
||
cfg, err := img.ConfigFile() | ||
if err != nil { | ||
return nil, err | ||
} | ||
ocfg := cfg.DeepCopy() | ||
ocfg.History = nil | ||
ocfg.RootFS.DiffIDs = nil | ||
|
||
oimg, err := mutate.ConfigFile(empty.Image, ocfg) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
layers, err := img.Layers() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
olayers := make([]mutate.Addendum, 0, len(layers)) | ||
for _, layer := range layers { | ||
olayer, err := tarball.LayerFromOpener(layer.Uncompressed, | ||
mattmoor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
tarball.WithEstargz, | ||
tarball.WithEstargzOptions(estargz.WithPrioritizedFiles(prioritize))) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
olayers = append(olayers, mutate.Addendum{ | ||
Layer: olayer, | ||
MediaType: types.DockerLayer, | ||
}) | ||
} | ||
|
||
return mutate.Append(oimg, olayers...) | ||
} | ||
|
||
func optimizeAndPushIndex(desc *remote.Descriptor, dstRef name.Reference, prioritize []string, o options) error { | ||
idx, err := desc.ImageIndex() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
oidx, err := optimizeIndex(idx, prioritize) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
return remote.WriteIndex(dstRef, oidx, o.remote...) | ||
} | ||
|
||
func optimizeIndex(idx v1.ImageIndex, prioritize []string) (v1.ImageIndex, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We really need a: func Map(f func(v1.Image) (v1.Image, error), i v1.ImageOrIndex) (v1.ImageOrIndex, error) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe I have one downstream somewhere 😇 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yeah you cced me on this earlier. My only issue is the pushing bit, we are missing an abstraction here for an ImageOrIndex thing (strawman here: #835) |
||
im, err := idx.IndexManifest() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// Build an image for each child from the base and append it to a new index to produce the result. | ||
adds := make([]mutate.IndexAddendum, 0, len(im.Manifests)) | ||
for _, desc := range im.Manifests { | ||
img, err := idx.Image(desc.Digest) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
oimg, err := optimizeImage(img, prioritize) | ||
if err != nil { | ||
return nil, err | ||
} | ||
adds = append(adds, mutate.IndexAddendum{ | ||
Add: oimg, | ||
Descriptor: v1.Descriptor{ | ||
URLs: desc.URLs, | ||
MediaType: desc.MediaType, | ||
Annotations: desc.Annotations, | ||
Platform: desc.Platform, | ||
}, | ||
Comment on lines
+164
to
+169
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could probably do something in |
||
}) | ||
} | ||
|
||
idxType, err := idx.MediaType() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return mutate.IndexMediaType(mutate.AppendManifests(empty.Index, adds...), idxType), nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this is hidden, I don't really care that much, but I would rather this be called something like
[e]stargz[ify]
(and documented as such), because "optimize" doesn't mean much.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll let you and @imjasonh fight about this. That was my original proposal and I care almost not at all.