Fix local images when daemon uses containerd storage #222
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import ( | |
| "io" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/docker/docker/api/types" | ||
| "github.com/docker/docker/client" | ||
|
|
@@ -24,10 +25,17 @@ func (i *Image) Save(additionalNames ...string) error { | |
| } | ||
|
|
||
| func (i *Image) SaveAs(name string, additionalNames ...string) error { | ||
| var ( | ||
| inspect types.ImageInspect | ||
| err error | ||
| ) | ||
| canOmitBaseLayers := !usesContainerdStorage(i.docker) | ||
| if canOmitBaseLayers { | ||
| // During the first save attempt some layers may be excluded. | ||
| // The docker daemon allows this if the given set of layers already exists in the daemon in the given order. | ||
| inspect, err = i.doSaveAs(name) | ||
| } | ||
| if !canOmitBaseLayers || err != nil { | ||
|
There was a problem hiding this comment. This is the sorriest part... it's going to lead to a huge performance penalty unless we can work around it. Maybe with containerd storage we could get individual layers from the daemon, IDK. If anyone has further context here it would be much appreciated. There was a problem hiding this comment. I also couldn't find a way around this. The trick of giving a tar header with no bytes does not work so we always have to pay the price it seems. At least until moby/moby#44369 ships |
||
| // populate all layer paths and try again without the above performance optimization. | ||
| if err := i.downloadBaseLayersOnce(); err != nil { | ||
| return err | ||
|
|
@@ -58,6 +66,14 @@ func (i *Image) SaveAs(name string, additionalNames ...string) error { | |
| return nil | ||
| } | ||
|
|
||
| func usesContainerdStorage(docker DockerClient) bool { | ||
| info, err := docker.Info(context.Background()) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| return strings.Contains(info.Driver, "stargz") | ||
| } | ||
|
|
||
| func (i *Image) doSaveAs(name string) (types.ImageInspect, error) { | ||
| ctx := context.Background() | ||
| done := make(chan error) | ||
|
|
@@ -94,58 +110,11 @@ func (i *Image) doSaveAs(name string) (types.ImageInspect, error) { | |
| }() | ||
|
|
||
| tw := tar.NewWriter(pw) | ||
| configHash, err := i.addImageToTar(tw, repoName) | ||
| if err != nil { | ||
| return types.ImageInspect{}, err | ||
| } | ||
| defer tw.Close() | ||
|
|
||
| tw.Close() | ||
| pw.Close() | ||
|
|
@@ -154,13 +123,16 @@ func (i *Image) doSaveAs(name string) (types.ImageInspect, error) { | |
| return types.ImageInspect{}, errors.Wrapf(err, "loading image %q. first error", i.repoName) | ||
| } | ||
|
|
||
| inspect, _, err := i.docker.ImageInspectWithRaw(context.Background(), i.repoName) | ||
|
There was a problem hiding this comment. With containerd storage the image ID is the sha of the manifest, which technically we could compute but it would be hard. To avoid collision problems we can validate that the data in inspect matches the config that we expect. |
||
| if err != nil { | ||
| if client.IsErrNotFound(err) { | ||
| return types.ImageInspect{}, errors.Wrapf(err, "saving image %q", i.repoName) | ||
| } | ||
| return types.ImageInspect{}, err | ||
| } | ||
| if err = i.validateInspect(inspect, configHash); err != nil { | ||
| return types.ImageInspect{}, err | ||
| } | ||
|
|
||
| return inspect, nil | ||
| } | ||
|
|
@@ -247,6 +219,73 @@ func (i *Image) downloadBaseLayers() error { | |
| return nil | ||
| } | ||
|
|
||
| func (i *Image) addImageToTar(tw *tar.Writer, repoName string) (string, error) { | ||
| configFile, err := i.newConfigFile() | ||
| if err != nil { | ||
| return "", errors.Wrap(err, "generating config file") | ||
| } | ||
|
|
||
| configHash := fmt.Sprintf("%x", sha256.Sum256(configFile)) | ||
| if err := addTextToTar(tw, configHash+".json", configFile); err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| var blankIdx int | ||
| var layerPaths []string | ||
| for _, path := range i.layerPaths { | ||
| if path == "" { | ||
| layerName := fmt.Sprintf("blank_%d", blankIdx) | ||
| blankIdx++ | ||
| hdr := &tar.Header{Name: layerName, Mode: 0644, Size: 0} | ||
| if err := tw.WriteHeader(hdr); err != nil { | ||
| return "", err | ||
| } | ||
| layerPaths = append(layerPaths, layerName) | ||
| } else { | ||
| layerName := fmt.Sprintf("/%x.tar", sha256.Sum256([]byte(path))) | ||
| f, err := os.Open(filepath.Clean(path)) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| defer f.Close() | ||
| if err := addFileToTar(tw, layerName, f); err != nil { | ||
| return "", err | ||
| } | ||
| f.Close() | ||
| layerPaths = append(layerPaths, layerName) | ||
| } | ||
| } | ||
|
|
||
| manifest, err := json.Marshal([]map[string]interface{}{ | ||
| { | ||
| "Config": configHash + ".json", | ||
| "RepoTags": []string{repoName}, | ||
| "Layers": layerPaths, | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| return configHash, addTextToTar(tw, "manifest.json", manifest) | ||
| } | ||
|
|
||
| func (i *Image) validateInspect(inspect types.ImageInspect, givenConfigHash string) error { | ||
| foundConfig, err := v1Config(inspect, i.createdAt, i.history) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get config file from inspect: %w", err) | ||
| } | ||
| foundConfigFile, err := json.Marshal(foundConfig) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to marshal config file: %w", err) | ||
| } | ||
| foundID := fmt.Sprintf("%x", sha256.Sum256(foundConfigFile)) | ||
| if foundID != givenConfigHash { | ||
| return fmt.Errorf("expected config hash %q; got %q", givenConfigHash, foundID) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // helpers | ||
|
|
||
| func checkResponseError(r io.Reader) error { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 tried explicitly setting this about here:
imgutil/local/save_file.go
Line 210 in 67824e9