Skip to content

Commit

Permalink
Fix local images when daemon uses containerd storage
Browse files Browse the repository at this point in the history 
Signed-off-by: Natalie Arellano <[email protected]>
  • Loading branch information
@natalieparellano
natalieparellano committed Oct 25, 2023
1 parent 4ec9360 commit 67824e9
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 144 deletions.
6 changes: 6 additions & 0 deletions acceptance/reproducibility_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,12 @@ func compare(t *testing.T, img1, img2 string) {
cfg2, err := v1img2.ConfigFile()
h.AssertNil(t, err)

// images that were created locally may have `DockerVersion` equal to "dev" and missing `Config.Image`
cfg1.DockerVersion = ""
cfg2.DockerVersion = ""
cfg1.Config.Image = ""
cfg2.Config.Image = ""

h.AssertEq(t, cfg1, cfg2)

h.AssertEq(t, ref1.Identifier(), ref2.Identifier())
Expand Down
35 changes: 17 additions & 18 deletions local/local_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1114,15 +1114,15 @@ func testImage(t *testing.T, when spec.G, it spec.S) {
when("#Rebase", func() {
when("image exists", func() {
var (
repoName = newTestImageName()
oldBase, oldTopLayer, newBase, origID string
oldBaseLayer1DiffID string
oldBaseLayer2DiffID string
newBaseLayer1DiffID string
newBaseLayer2DiffID string
imgLayer1DiffID string
imgLayer2DiffID string
origNumLayers int
repoName = newTestImageName()
oldBase, oldTopLayer, newBase string
oldBaseLayer1DiffID string
oldBaseLayer2DiffID string
newBaseLayer1DiffID string
newBaseLayer2DiffID string
imgLayer1DiffID string
imgLayer2DiffID string
origNumLayers int
)

it.Before(func() {
Expand Down Expand Up @@ -1198,11 +1198,10 @@ func testImage(t *testing.T, when spec.G, it spec.S) {
inspect, _, err = dockerClient.ImageInspectWithRaw(context.TODO(), repoName)
h.AssertNil(t, err)
origNumLayers = len(inspect.RootFS.Layers)
origID = inspect.ID
})

it.After(func() {
h.AssertNil(t, h.DockerRmi(dockerClient, repoName, oldBase, newBase, origID))
h.AssertNil(t, h.DockerRmi(dockerClient, repoName, oldBase, newBase))
})

it("switches the base", func() {
Expand Down Expand Up @@ -1604,6 +1603,8 @@ func testImage(t *testing.T, when spec.G, it spec.S) {
repoName = newTestImageName()
prevLayer1SHA string
prevLayer2SHA string
layer1Path string
layer2Path string
)

it.Before(func() {
Expand All @@ -1616,13 +1617,11 @@ func testImage(t *testing.T, when spec.G, it spec.S) {
)
h.AssertNil(t, err)

layer1Path, err := h.CreateSingleFileLayerTar("/layer-1.txt", "old-layer-1", daemonOS)
layer1Path, err = h.CreateSingleFileLayerTar("/layer-1.txt", "old-layer-1", daemonOS)
h.AssertNil(t, err)
defer os.Remove(layer1Path)

layer2Path, err := h.CreateSingleFileLayerTar("/layer-2.txt", "old-layer-2", daemonOS)
layer2Path, err = h.CreateSingleFileLayerTar("/layer-2.txt", "old-layer-2", daemonOS)
h.AssertNil(t, err)
defer os.Remove(layer2Path)

h.AssertNil(t, prevImage.AddLayer(layer1Path))
h.AssertNil(t, prevImage.AddLayer(layer2Path))
Expand All @@ -1638,6 +1637,8 @@ func testImage(t *testing.T, when spec.G, it spec.S) {

it.After(func() {
h.AssertNil(t, h.DockerRmi(dockerClient, repoName, prevImageName))
h.AssertNil(t, os.RemoveAll(layer1Path))
h.AssertNil(t, os.RemoveAll(layer2Path))
})

it("reuses a layer", func() {
Expand Down Expand Up @@ -1869,7 +1870,7 @@ func testImage(t *testing.T, when spec.G, it spec.S) {

it.After(func() {
h.AssertNil(t, os.Remove(tarPath))
h.AssertNil(t, h.DockerRmi(dockerClient, repoName, origID))
h.AssertNil(t, h.DockerRmi(dockerClient, repoName))
})

it("saved image overrides image with new ID", func() {
Expand Down Expand Up @@ -1909,7 +1910,6 @@ func testImage(t *testing.T, when spec.G, it spec.S) {

h.AssertEq(t, inspect.Created, imgutil.NormalizedDateTime.Format(time.RFC3339))
h.AssertEq(t, inspect.Container, "")
h.AssertEq(t, inspect.DockerVersion, "")

history, err := dockerClient.ImageHistory(context.TODO(), repoName)
h.AssertNil(t, err)
Expand Down Expand Up @@ -1940,7 +1940,6 @@ func testImage(t *testing.T, when spec.G, it spec.S) {

h.AssertEq(t, inspect.Created, expectedTime.Format(time.RFC3339))
h.AssertEq(t, inspect.Container, "")
h.AssertEq(t, inspect.DockerVersion, "")

history, err := dockerClient.ImageHistory(context.TODO(), repoName)
h.AssertNil(t, err)
Expand Down
147 changes: 93 additions & 54 deletions local/save.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io"
"os"
"path/filepath"
"strings"

"github.com/docker/docker/api/types"
"github.com/docker/docker/client"
Expand All @@ -24,10 +25,17 @@ func (i *Image) Save(additionalNames ...string) error {
}

func (i *Image) SaveAs(name string, additionalNames ...string) error {
// 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 err != nil {
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 {
// populate all layer paths and try again without the above performance optimization.
if err := i.downloadBaseLayersOnce(); err != nil {
return err
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -94,58 +110,11 @@ func (i *Image) doSaveAs(name string) (types.ImageInspect, error) {
}()

tw := tar.NewWriter(pw)
defer tw.Close()

configFile, err := i.newConfigFile()
if err != nil {
return types.ImageInspect{}, errors.Wrap(err, "generating config file")
}

id := fmt.Sprintf("%x", sha256.Sum256(configFile))
if err := addTextToTar(tw, id+".json", configFile); err != nil {
return types.ImageInspect{}, 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 types.ImageInspect{}, 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 types.ImageInspect{}, err
}
defer f.Close()
if err := addFileToTar(tw, layerName, f); err != nil {
return types.ImageInspect{}, err
}
f.Close()
layerPaths = append(layerPaths, layerName)
}
}

manifest, err := json.Marshal([]map[string]interface{}{
{
"Config": id + ".json",
"RepoTags": []string{repoName},
"Layers": layerPaths,
},
})
configHash, err := i.addImageToTar(tw, repoName)
if err != nil {
return types.ImageInspect{}, err
}

if err := addTextToTar(tw, "manifest.json", manifest); err != nil {
return types.ImageInspect{}, err
}
defer tw.Close()

tw.Close()
pw.Close()
Expand All @@ -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(), id)
inspect, _, err := i.docker.ImageInspectWithRaw(context.Background(), i.repoName)
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
}
Expand Down Expand Up @@ -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 {
Expand Down
63 changes: 8 additions & 55 deletions local/save_file.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package local
import (
"archive/tar"
"context"
"crypto/sha256"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -31,9 +30,10 @@ func (i *Image) SaveFile() (string, error) {
}
}()

// All layers need to be present here. Missing layers are either due to utilization of: (1) WithPreviousImage(),
// or (2) FromBaseImage(). The former is only relevant if ReuseLayers() has been called which takes care of
// resolving them. The latter case needs to be handled explicitly.
// All layers need to be present here. Missing layers are either due to utilization of
// (1) WithPreviousImage(), or (2) FromBaseImage().
// The former is only relevant if ReuseLayers() has been called which takes care of resolving them.
// The latter case needs to be handled explicitly.
if err := i.downloadBaseLayersOnce(); err != nil {
return "", errors.Wrap(err, "failed to fetch base layers")
}
Expand All @@ -55,63 +55,16 @@ func (i *Image) SaveFile() (string, error) {
tw := tar.NewWriter(pw)
defer tw.Close()

config, err := i.newConfigFile()
if err != nil {
return errors.Wrap(err, "failed to generate config file")
}

configName := fmt.Sprintf("/%x.json", sha256.Sum256(config))
if err := addTextToTar(tw, configName, config); err != nil {
return errors.Wrap(err, "failed to add config file to tar archive")
}

for _, path := range i.layerPaths {
err := func() error {
f, err := os.Open(filepath.Clean(path))
if err != nil {
return errors.Wrapf(err, "failed to open layer path: %s", path)
}
defer f.Close()

layerName := fmt.Sprintf("/%x.tar", sha256.Sum256([]byte(path)))
if err := addFileToTar(tw, layerName, f); err != nil {
return errors.Wrapf(err, "failed to add layer to tar archive from path: %s", path)
}

return nil
}()

if err != nil {
return err
}
}

t, err := registryName.NewTag(i.repoName, registryName.WeakValidation)
if err != nil {
return errors.Wrap(err, "failed to create tag")
}

layers := make([]string, 0, len(i.layerPaths))
for _, path := range i.layerPaths {
layers = append(layers, fmt.Sprintf("/%x.tar", sha256.Sum256([]byte(path))))
}
// returns valid 'name:tag' appending 'latest', if missing tag
repoName := t.Name()

manifest, err := json.Marshal([]map[string]interface{}{
{
"Config": configName,
"RepoTags": []string{t.Name()},
"Layers": layers,
},
})
if err != nil {
return errors.Wrap(err, "failed to create manifest")
}

if err := addTextToTar(tw, "/manifest.json", manifest); err != nil {
return errors.Wrap(err, "failed to add manifest to tar archive")
}

return nil
_, err = i.addImageToTar(tw, repoName)
return err
})

err = errs.Wait()
Expand Down
Loading

0 comments on commit 67824e9

Please sign in to comment.