Browse Source

layer: ChecksumForGraphID: remove unused code for for migrating v1 layers

The only use is in `builder/builder-next/adapters/snapshot.EnsureLayer()`,
which always calls the function with an _empty_ `oldTarDataPath`;
https://github.com/moby/moby/blob/7082aecd540db38ef48c730505ca3a37bdfca32a/builder/builder-next/adapters/snapshot/layer.go#L81

When called with an empty `oldTarDataPath`, this function was an alias for
`checksumForGraphIDNoTarsplit`, so let's make it that.

Note that this code was added in 500e77bad0b19b3b1c8e6ac195485adcb70daef1, as
part of the migration from "v1" images to "v2" (content-addressable) images.

Given that the remaining code lives in a "migration" file, possibly more code
can be removed.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn 1 year ago
parent
commit
782fe1fe82

+ 1 - 1
builder/builder-next/adapters/snapshot/layer.go

@@ -78,7 +78,7 @@ func (s *snapshotter) EnsureLayer(ctx context.Context, key string) ([]layer.Diff
 				parent, _ = s.getGraphDriverID(info.Parent)
 			}
 		}
-		diffID, size, err = s.reg.ChecksumForGraphID(id, parent, "", tarSplitPath)
+		diffID, size, err = s.reg.ChecksumForGraphID(id, parent, tarSplitPath)
 		return err
 	})
 

+ 1 - 1
builder/builder-next/adapters/snapshot/snapshot.go

@@ -45,7 +45,7 @@ type graphIDRegistrar interface {
 }
 
 type checksumCalculator interface {
-	ChecksumForGraphID(id, parent, oldTarDataPath, newTarDataPath string) (diffID layer.DiffID, size int64, err error)
+	ChecksumForGraphID(id, parent, newTarDataPath string) (diffID layer.DiffID, size int64, err error)
 }
 
 type snapshotter struct {

+ 1 - 39
layer/migration.go

@@ -13,45 +13,7 @@ import (
 	"github.com/vbatts/tar-split/tar/storage"
 )
 
-func (ls *layerStore) ChecksumForGraphID(id, parent, oldTarDataPath, newTarDataPath string) (diffID DiffID, size int64, err error) {
-	defer func() {
-		if err != nil {
-			diffID, size, err = ls.checksumForGraphIDNoTarsplit(id, parent, newTarDataPath)
-		}
-	}()
-
-	if oldTarDataPath == "" {
-		err = errors.New("no tar-split file")
-		return
-	}
-
-	tarDataFile, err := os.Open(oldTarDataPath)
-	if err != nil {
-		return
-	}
-	defer tarDataFile.Close()
-	uncompressed, err := gzip.NewReader(tarDataFile)
-	if err != nil {
-		return
-	}
-
-	dgst := digest.Canonical.Digester()
-	err = ls.assembleTarTo(id, uncompressed, &size, dgst.Hash())
-	if err != nil {
-		return
-	}
-
-	diffID = DiffID(dgst.Digest())
-	err = os.RemoveAll(newTarDataPath)
-	if err != nil {
-		return
-	}
-	err = os.Link(oldTarDataPath, newTarDataPath)
-
-	return
-}
-
-func (ls *layerStore) checksumForGraphIDNoTarsplit(id, parent, newTarDataPath string) (diffID DiffID, size int64, err error) {
+func (ls *layerStore) ChecksumForGraphID(id, parent, newTarDataPath string) (diffID DiffID, size int64, err error) {
 	rawarchive, err := ls.driver.Diff(id, parent)
 	if err != nil {
 		return

+ 2 - 143
layer/migration_test.go

@@ -2,7 +2,6 @@ package layer // import "github.com/docker/docker/layer"
 
 import (
 	"bytes"
-	"compress/gzip"
 	"io"
 	"os"
 	"path/filepath"
@@ -11,148 +10,8 @@ import (
 
 	"github.com/docker/docker/daemon/graphdriver"
 	"github.com/docker/docker/pkg/stringid"
-	"github.com/vbatts/tar-split/tar/asm"
-	"github.com/vbatts/tar-split/tar/storage"
 )
 
-func writeTarSplitFile(name string, tarContent []byte) error {
-	f, err := os.OpenFile(name, os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0o644)
-	if err != nil {
-		return err
-	}
-	defer f.Close()
-
-	fz := gzip.NewWriter(f)
-
-	metaPacker := storage.NewJSONPacker(fz)
-	defer fz.Close()
-
-	rdr, err := asm.NewInputTarStream(bytes.NewReader(tarContent), metaPacker, nil)
-	if err != nil {
-		return err
-	}
-
-	if _, err := io.Copy(io.Discard, rdr); err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func TestLayerMigration(t *testing.T) {
-	// TODO Windows: Figure out why this is failing
-	if runtime.GOOS == "windows" {
-		t.Skip("Failing on Windows")
-	}
-	td, err := os.MkdirTemp("", "migration-test-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.RemoveAll(td)
-
-	layer1Files := []FileApplier{
-		newTestFile("/root/.bashrc", []byte("# Boring configuration"), 0o644),
-		newTestFile("/etc/profile", []byte("# Base configuration"), 0o644),
-	}
-
-	layer2Files := []FileApplier{
-		newTestFile("/root/.bashrc", []byte("# Updated configuration"), 0o644),
-	}
-
-	tar1, err := tarFromFiles(layer1Files...)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	tar2, err := tarFromFiles(layer2Files...)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	graph, err := newVFSGraphDriver(filepath.Join(td, "graphdriver-"))
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	graphID1 := stringid.GenerateRandomID()
-	if err := graph.Create(graphID1, "", nil); err != nil {
-		t.Fatal(err)
-	}
-	if _, err := graph.ApplyDiff(graphID1, "", bytes.NewReader(tar1)); err != nil {
-		t.Fatal(err)
-	}
-
-	tf1 := filepath.Join(td, "tar1.json.gz")
-	if err := writeTarSplitFile(tf1, tar1); err != nil {
-		t.Fatal(err)
-	}
-
-	root := filepath.Join(td, "layers")
-	ls, err := newStoreFromGraphDriver(root, graph)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	newTarDataPath := filepath.Join(td, ".migration-tardata")
-	diffID, size, err := ls.(*layerStore).ChecksumForGraphID(graphID1, "", tf1, newTarDataPath)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	layer1a, err := ls.(*layerStore).RegisterByGraphID(graphID1, "", diffID, newTarDataPath, size)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	layer1b, err := ls.Register(bytes.NewReader(tar1), "")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	assertReferences(t, layer1a, layer1b)
-	// Attempt register, should be same
-	layer2a, err := ls.Register(bytes.NewReader(tar2), layer1a.ChainID())
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	graphID2 := stringid.GenerateRandomID()
-	if err := graph.Create(graphID2, graphID1, nil); err != nil {
-		t.Fatal(err)
-	}
-	if _, err := graph.ApplyDiff(graphID2, graphID1, bytes.NewReader(tar2)); err != nil {
-		t.Fatal(err)
-	}
-
-	tf2 := filepath.Join(td, "tar2.json.gz")
-	if err := writeTarSplitFile(tf2, tar2); err != nil {
-		t.Fatal(err)
-	}
-	diffID, size, err = ls.(*layerStore).ChecksumForGraphID(graphID2, graphID1, tf2, newTarDataPath)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	layer2b, err := ls.(*layerStore).RegisterByGraphID(graphID2, layer1a.ChainID(), diffID, tf2, size)
-	if err != nil {
-		t.Fatal(err)
-	}
-	assertReferences(t, layer2a, layer2b)
-
-	if metadata, err := ls.Release(layer2a); err != nil {
-		t.Fatal(err)
-	} else if len(metadata) > 0 {
-		t.Fatalf("Unexpected layer removal after first release: %#v", metadata)
-	}
-
-	metadata, err := ls.Release(layer2b)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	assertMetadata(t, metadata, createMetadata(layer2a))
-}
-
 func tarFromFilesInGraph(graph graphdriver.Driver, graphID, parentID string, files ...FileApplier) ([]byte, error) {
 	t, err := tarFromFiles(files...)
 	if err != nil {
@@ -219,7 +78,7 @@ func TestLayerMigrationNoTarsplit(t *testing.T) {
 	}
 
 	newTarDataPath := filepath.Join(td, ".migration-tardata")
-	diffID, size, err := ls.(*layerStore).ChecksumForGraphID(graphID1, "", "", newTarDataPath)
+	diffID, size, err := ls.(*layerStore).ChecksumForGraphID(graphID1, "", newTarDataPath)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -242,7 +101,7 @@ func TestLayerMigrationNoTarsplit(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	diffID, size, err = ls.(*layerStore).ChecksumForGraphID(graphID2, graphID1, "", newTarDataPath)
+	diffID, size, err = ls.(*layerStore).ChecksumForGraphID(graphID2, graphID1, newTarDataPath)
 	if err != nil {
 		t.Fatal(err)
 	}