From b37fdc5dd1db196209ebb860c88a37d67bb2cf98 Mon Sep 17 00:00:00 2001
From: Anthony Baire <Anthony.Baire@irisa.fr>
Date: Tue, 11 Nov 2014 10:18:22 +0100
Subject: [PATCH] fix missing layers when exporting a full repository

Therer is a bug in the 'skip' decision when exporting a repository
(`docker save repo`)

Only the layers of the first image are included in the archive (the
layers of the next images are missing)

Signed-off-by: Anthony Baire <Anthony.Baire@irisa.fr>
---
 graph/export.go                              | 36 ++++--------
 integration-cli/docker_cli_save_load_test.go | 62 ++++++++++++++++++++
 2 files changed, 73 insertions(+), 25 deletions(-)

diff --git a/graph/export.go b/graph/export.go
index 75314076ed..7a8054010e 100644
--- a/graph/export.go
+++ b/graph/export.go
@@ -30,24 +30,21 @@ func (s *TagStore) CmdImageExport(job *engine.Job) engine.Status {
 	defer os.RemoveAll(tempdir)
 
 	rootRepoMap := map[string]Repository{}
+	addKey := func(name string, tag string, id string) {
+		log.Debugf("add key [%s:%s]", name, tag)
+		if repo, ok := rootRepoMap[name]; !ok {
+			rootRepoMap[name] = Repository{tag: id}
+		} else {
+			repo[tag] = id
+		}
+	}
 	for _, name := range job.Args {
 		log.Debugf("Serializing %s", name)
 		rootRepo := s.Repositories[name]
 		if rootRepo != nil {
 			// this is a base repo name, like 'busybox'
-			for _, id := range rootRepo {
-				if _, ok := rootRepoMap[name]; !ok {
-					rootRepoMap[name] = rootRepo
-				} else {
-					log.Debugf("Duplicate key [%s]", name)
-					if rootRepoMap[name].Contains(rootRepo) {
-						log.Debugf("skipping, because it is present [%s:%q]", name, rootRepo)
-						continue
-					}
-					log.Debugf("updating [%s]: [%q] with [%q]", name, rootRepoMap[name], rootRepo)
-					rootRepoMap[name].Update(rootRepo)
-				}
-
+			for tag, id := range rootRepo {
+				addKey(name, tag, id)
 				if err := s.exportImage(job.Eng, id, tempdir); err != nil {
 					return job.Error(err)
 				}
@@ -65,18 +62,7 @@ func (s *TagStore) CmdImageExport(job *engine.Job) engine.Status {
 				// check this length, because a lookup of a truncated has will not have a tag
 				// and will not need to be added to this map
 				if len(repoTag) > 0 {
-					if _, ok := rootRepoMap[repoName]; !ok {
-						rootRepoMap[repoName] = Repository{repoTag: img.ID}
-					} else {
-						log.Debugf("Duplicate key [%s]", repoName)
-						newRepo := Repository{repoTag: img.ID}
-						if rootRepoMap[repoName].Contains(newRepo) {
-							log.Debugf("skipping, because it is present [%s:%q]", repoName, newRepo)
-							continue
-						}
-						log.Debugf("updating [%s]: [%q] with [%q]", repoName, rootRepoMap[repoName], newRepo)
-						rootRepoMap[repoName].Update(newRepo)
-					}
+					addKey(repoName, repoTag, img.ID)
 				}
 				if err := s.exportImage(job.Eng, img.ID, tempdir); err != nil {
 					return job.Error(err)
diff --git a/integration-cli/docker_cli_save_load_test.go b/integration-cli/docker_cli_save_load_test.go
index ceb73a571f..73df63dc55 100644
--- a/integration-cli/docker_cli_save_load_test.go
+++ b/integration-cli/docker_cli_save_load_test.go
@@ -8,6 +8,8 @@ import (
 	"os/exec"
 	"path/filepath"
 	"reflect"
+	"sort"
+	"strings"
 	"testing"
 
 	"github.com/docker/docker/vendor/src/github.com/kr/pty"
@@ -257,6 +259,66 @@ func TestSaveMultipleNames(t *testing.T) {
 	logDone("save - save by multiple names")
 }
 
+func TestSaveRepoWithMultipleImages(t *testing.T) {
+
+	makeImage := func(from string, tag string) string {
+		runCmd := exec.Command(dockerBinary, "run", "-d", from, "true")
+		var (
+			out string
+			err error
+		)
+		if out, _, err = runCommandWithOutput(runCmd); err != nil {
+			t.Fatalf("failed to create a container: %v %v", out, err)
+		}
+		cleanedContainerID := stripTrailingCharacters(out)
+
+		commitCmd := exec.Command(dockerBinary, "commit", cleanedContainerID, tag)
+		if out, _, err = runCommandWithOutput(commitCmd); err != nil {
+			t.Fatalf("failed to commit container: %v %v", out, err)
+		}
+		imageID := stripTrailingCharacters(out)
+
+		deleteContainer(cleanedContainerID)
+		return imageID
+	}
+
+	repoName := "foobar-save-multi-images-test"
+	tagFoo := repoName + ":foo"
+	tagBar := repoName + ":bar"
+
+	idFoo := makeImage("busybox:latest", tagFoo)
+	idBar := makeImage("busybox:latest", tagBar)
+
+	deleteImages(repoName)
+
+	// create the archive
+	saveCmdFinal := fmt.Sprintf("%v save %v | tar t | grep 'VERSION' |cut -d / -f1", dockerBinary, repoName)
+	saveCmd := exec.Command("bash", "-c", saveCmdFinal)
+	out, _, err := runCommandWithOutput(saveCmd)
+	if err != nil {
+		t.Fatalf("failed to save multiple images: %s, %v", out, err)
+	}
+	actual := strings.Split(stripTrailingCharacters(out), "\n")
+
+	// make the list of expected layers
+	historyCmdFinal := fmt.Sprintf("%v history -q --no-trunc %v", dockerBinary, "busybox:latest")
+	historyCmd := exec.Command("bash", "-c", historyCmdFinal)
+	out, _, err = runCommandWithOutput(historyCmd)
+	if err != nil {
+		t.Fatalf("failed to get history: %s, %v", out, err)
+	}
+
+	expected := append(strings.Split(stripTrailingCharacters(out), "\n"), idFoo, idBar)
+
+	sort.Strings(actual)
+	sort.Strings(expected)
+	if !reflect.DeepEqual(expected, actual) {
+		t.Fatalf("achive does not contains the right layers: got %v, expected %v", actual, expected)
+	}
+
+	logDone("save - save repository with multiple images")
+}
+
 // Issue #6722 #5892 ensure directories are included in changes
 func TestSaveDirectoryPermissions(t *testing.T) {
 	layerEntries := []string{"opt/", "opt/a/", "opt/a/b/", "opt/a/b/c"}