diff --git a/archive/changes.go b/archive/changes.go index d94d8bc33c..9305fe2311 100644 --- a/archive/changes.go +++ b/archive/changes.go @@ -280,11 +280,7 @@ func ChangesDirs(newDir, oldDir string) ([]Change, error) { return newRoot.Changes(oldRoot), nil } -func ExportChanges(root, rw string) (Archive, error) { - changes, err := ChangesDirs(root, rw) - if err != nil { - return nil, err - } +func ExportChanges(dir string, changes []Change) (Archive, error) { files := make([]string, 0) deletions := make([]string, 0) for _, change := range changes { @@ -297,5 +293,11 @@ func ExportChanges(root, rw string) (Archive, error) { deletions = append(deletions, filepath.Join(dir, ".wh."+base)) } } - return TarFilter(root, &TarOptions{Compression: Uncompressed, Recursive: false, Includes: files, CreateFiles: deletions}) + // FIXME: Why do we create whiteout files inside Tar code ? + return TarFilter(dir, &TarOptions{ + Compression: Uncompressed, + Includes: files, + Recursive: false, + CreateFiles: deletions, + }) } diff --git a/archive/changes_test.go b/archive/changes_test.go new file mode 100644 index 0000000000..37a3869e96 --- /dev/null +++ b/archive/changes_test.go @@ -0,0 +1,297 @@ +package archive + +import ( + "io/ioutil" + "os" + "os/exec" + "path" + "sort" + "testing" + "time" +) + +func max(x, y int) int { + if x >= y { + return x + } + return y +} + +func copyDir(src, dst string) error { + cmd := exec.Command("cp", "-a", src, dst) + if err := cmd.Run(); err != nil { + return err + } + return nil +} + +// Helper to sort []Change by path +type byPath struct{ changes []Change } + +func (b byPath) Less(i, j int) bool { return b.changes[i].Path < b.changes[j].Path } +func (b byPath) Len() int { return len(b.changes) } +func (b byPath) Swap(i, j int) { b.changes[i], b.changes[j] = b.changes[j], b.changes[i] } + +type FileType uint32 + +const ( + Regular FileType = iota + Dir + Symlink +) + +type FileData struct { + filetype FileType + path string + contents string + permissions os.FileMode +} + +func createSampleDir(t *testing.T, root string) { + files := []FileData{ + {Regular, "file1", "file1\n", 0600}, + {Regular, "file2", "file2\n", 0666}, + {Regular, "file3", "file3\n", 0404}, + {Regular, "file4", "file4\n", 0600}, + {Regular, "file5", "file5\n", 0600}, + {Regular, "file6", "file6\n", 0600}, + {Regular, "file7", "file7\n", 0600}, + {Dir, "dir1", "", 0740}, + {Regular, "dir1/file1-1", "file1-1\n", 01444}, + {Regular, "dir1/file1-2", "file1-2\n", 0666}, + {Dir, "dir2", "", 0700}, + {Regular, "dir2/file2-1", "file2-1\n", 0666}, + {Regular, "dir2/file2-2", "file2-2\n", 0666}, + {Dir, "dir3", "", 0700}, + {Regular, "dir3/file3-1", "file3-1\n", 0666}, + {Regular, "dir3/file3-2", "file3-2\n", 0666}, + {Dir, "dir4", "", 0700}, + {Regular, "dir4/file3-1", "file4-1\n", 0666}, + {Regular, "dir4/file3-2", "file4-2\n", 0666}, + {Symlink, "symlink1", "target1", 0666}, + {Symlink, "symlink2", "target2", 0666}, + } + for _, info := range files { + if info.filetype == Dir { + if err := os.MkdirAll(path.Join(root, info.path), info.permissions); err != nil { + t.Fatal(err) + } + } else if info.filetype == Regular { + if err := ioutil.WriteFile(path.Join(root, info.path), []byte(info.contents), info.permissions); err != nil { + t.Fatal(err) + } + } else if info.filetype == Symlink { + if err := os.Symlink(info.contents, path.Join(root, info.path)); err != nil { + t.Fatal(err) + } + } + } +} + +// Create an directory, copy it, make sure we report no changes between the two +func TestChangesDirsEmpty(t *testing.T) { + src, err := ioutil.TempDir("", "docker-changes-test") + if err != nil { + t.Fatal(err) + } + createSampleDir(t, src) + dst := src + "-copy" + if err := copyDir(src, dst); err != nil { + t.Fatal(err) + } + changes, err := ChangesDirs(dst, src) + if err != nil { + t.Fatal(err) + } + + if len(changes) != 0 { + t.Fatalf("Reported changes for identical dirs: %v", changes) + } + os.RemoveAll(src) + os.RemoveAll(dst) +} + +func mutateSampleDir(t *testing.T, root string) { + // Remove a regular file + if err := os.RemoveAll(path.Join(root, "file1")); err != nil { + t.Fatal(err) + } + + // Remove a directory + if err := os.RemoveAll(path.Join(root, "dir1")); err != nil { + t.Fatal(err) + } + + // Remove a symlink + if err := os.RemoveAll(path.Join(root, "symlink1")); err != nil { + t.Fatal(err) + } + + // Rewrite a file + if err := ioutil.WriteFile(path.Join(root, "file2"), []byte("fileN\n"), 0777); err != nil { + t.Fatal(err) + } + + // Replace a file + if err := os.RemoveAll(path.Join(root, "file3")); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(path.Join(root, "file3"), []byte("fileM\n"), 0404); err != nil { + t.Fatal(err) + } + + // Touch file + if err := os.Chtimes(path.Join(root, "file4"), time.Now(), time.Now()); err != nil { + t.Fatal(err) + } + + // Replace file with dir + if err := os.RemoveAll(path.Join(root, "file5")); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(path.Join(root, "file5"), 0666); err != nil { + t.Fatal(err) + } + + // Create new file + if err := ioutil.WriteFile(path.Join(root, "filenew"), []byte("filenew\n"), 0777); err != nil { + t.Fatal(err) + } + + // Create new dir + if err := os.MkdirAll(path.Join(root, "dirnew"), 0766); err != nil { + t.Fatal(err) + } + + // Create a new symlink + if err := os.Symlink("targetnew", path.Join(root, "symlinknew")); err != nil { + t.Fatal(err) + } + + // Change a symlink + if err := os.RemoveAll(path.Join(root, "symlink2")); err != nil { + t.Fatal(err) + } + if err := os.Symlink("target2change", path.Join(root, "symlink2")); err != nil { + t.Fatal(err) + } + + // Replace dir with file + if err := os.RemoveAll(path.Join(root, "dir2")); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(path.Join(root, "dir2"), []byte("dir2\n"), 0777); err != nil { + t.Fatal(err) + } + + // Touch dir + if err := os.Chtimes(path.Join(root, "dir3"), time.Now(), time.Now()); err != nil { + t.Fatal(err) + } +} + +func TestChangesDirsMutated(t *testing.T) { + src, err := ioutil.TempDir("", "docker-changes-test") + if err != nil { + t.Fatal(err) + } + createSampleDir(t, src) + dst := src + "-copy" + if err := copyDir(src, dst); err != nil { + t.Fatal(err) + } + mutateSampleDir(t, dst) + + changes, err := ChangesDirs(dst, src) + if err != nil { + t.Fatal(err) + } + + sort.Sort(byPath{changes}) + + expectedChanges := []Change{ + {"/dir1", ChangeDelete}, + {"/dir2", ChangeModify}, + {"/dir3", ChangeModify}, + {"/dirnew", ChangeAdd}, + {"/file1", ChangeDelete}, + {"/file2", ChangeModify}, + {"/file3", ChangeModify}, + {"/file4", ChangeModify}, + {"/file5", ChangeModify}, + {"/filenew", ChangeAdd}, + {"/symlink1", ChangeDelete}, + {"/symlink2", ChangeModify}, + {"/symlinknew", ChangeAdd}, + } + + i := 0 + for ; i < max(len(changes), len(expectedChanges)); i++ { + if i >= len(expectedChanges) { + t.Fatalf("unexpected change %s\n", changes[i].String()) + } + if i >= len(changes) { + t.Fatalf("no change for expected change %s\n", expectedChanges[i].String()) + } + if changes[i].Path == expectedChanges[i].Path { + if changes[i] != expectedChanges[i] { + t.Fatalf("Wrong change for %s, expected %s, got %d\n", changes[i].Path, changes[i].String(), expectedChanges[i].String()) + } + } else if changes[i].Path < expectedChanges[i].Path { + t.Fatalf("unexpected change %s\n", changes[i].String()) + } else { + t.Fatalf("no change for expected change %s\n", expectedChanges[i].String()) + } + } + for ; i < len(expectedChanges); i++ { + } + + os.RemoveAll(src) + os.RemoveAll(dst) +} + +func TestApplyLayer(t *testing.T) { + return // Disable this for now as it is broken + + src, err := ioutil.TempDir("", "docker-changes-test") + if err != nil { + t.Fatal(err) + } + createSampleDir(t, src) + dst := src + "-copy" + if err := copyDir(src, dst); err != nil { + t.Fatal(err) + } + mutateSampleDir(t, dst) + + changes, err := ChangesDirs(dst, src) + if err != nil { + t.Fatal(err) + } + + layer, err := ExportChanges(dst, changes) + if err != nil { + t.Fatal(err) + } + + layerCopy, err := NewTempArchive(layer, "") + if err != nil { + t.Fatal(err) + } + + if err := ApplyLayer(src, layerCopy); err != nil { + t.Fatal(err) + } + + changes2, err := ChangesDirs(src, dst) + if err != nil { + t.Fatal(err) + } + + if len(changes2) != 0 { + t.Fatalf("Unexpected differences after re applying mutation: %v", changes) + } + + os.RemoveAll(src) + os.RemoveAll(dst) +} diff --git a/archive/diff.go b/archive/diff.go index 0e4fe3fcc2..877d076c8b 100644 --- a/archive/diff.go +++ b/archive/diff.go @@ -49,7 +49,7 @@ func ApplyLayer(dest string, layer Archive) error { rmTargetPath := filepath.Join(filepath.Dir(fullPath), rmTargetName) // Remove the file targeted by the whiteout log.Printf("Removing whiteout target %s", rmTargetPath) - _ = os.Remove(rmTargetPath) + _ = os.RemoveAll(rmTargetPath) // Remove the whiteout itself log.Printf("Removing whiteout %s", fullPath) _ = os.RemoveAll(fullPath) diff --git a/runtime.go b/runtime.go index 5072a00fdd..bfe1aee5a7 100644 --- a/runtime.go +++ b/runtime.go @@ -18,7 +18,6 @@ import ( "os" "os/exec" "path" - "path/filepath" "sort" "strings" "time" @@ -739,25 +738,7 @@ func (runtime *Runtime) Diff(container *Container) (archive.Archive, error) { return nil, fmt.Errorf("Error getting container rootfs %s from driver %s: %s", container.ID, container.runtime.driver, err) } - files := make([]string, 0) - deletions := make([]string, 0) - for _, change := range changes { - if change.Kind == archive.ChangeModify || change.Kind == archive.ChangeAdd { - files = append(files, change.Path) - } - if change.Kind == archive.ChangeDelete { - base := filepath.Base(change.Path) - dir := filepath.Dir(change.Path) - deletions = append(deletions, filepath.Join(dir, ".wh."+base)) - } - } - // FIXME: Why do we create whiteout files inside Tar code ? - return archive.TarFilter(cDir, &archive.TarOptions{ - Compression: archive.Uncompressed, - Includes: files, - Recursive: false, - CreateFiles: deletions, - }) + return archive.ExportChanges(cDir, changes) } func linkLxcStart(root string) error {