From 782fe1fe8238f6f16f352a677320c8a6ec7546b7 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 8 Jan 2024 12:28:41 +0100 Subject: [PATCH] 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 --- .../builder-next/adapters/snapshot/layer.go | 2 +- .../adapters/snapshot/snapshot.go | 2 +- layer/migration.go | 40 +---- layer/migration_test.go | 145 +----------------- 4 files changed, 5 insertions(+), 184 deletions(-) diff --git a/builder/builder-next/adapters/snapshot/layer.go b/builder/builder-next/adapters/snapshot/layer.go index e606a8b472..73120ea70b 100644 --- a/builder/builder-next/adapters/snapshot/layer.go +++ b/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 }) diff --git a/builder/builder-next/adapters/snapshot/snapshot.go b/builder/builder-next/adapters/snapshot/snapshot.go index a559e15b4b..a0d28ad984 100644 --- a/builder/builder-next/adapters/snapshot/snapshot.go +++ b/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 { diff --git a/layer/migration.go b/layer/migration.go index 26e1553c65..e54b60a43b 100644 --- a/layer/migration.go +++ b/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 diff --git a/layer/migration_test.go b/layer/migration_test.go index 9d307fe5c2..4357f2ba71 100644 --- a/layer/migration_test.go +++ b/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) }