Merge pull request #20 from crosbymichael/force-driver-to-implement-differ

Force driver to implement differ

archive/archive.go

@@ -15,7 +15,15 @@ import (
 
 type Archive io.Reader
 
-type Compression uint32
+type Compression int
+
+type TarOptions struct {
+	Includes    []string
+	Excludes    []string
+	Recursive   bool
+	Compression Compression
+	CreateFiles []string
+}
 
 const (
 	Uncompressed Compression = iota
@@ -80,7 +88,7 @@ func (compression *Compression) Extension() string {
 // Tar creates an archive from the directory at `path`, and returns it as a
 // stream of bytes.
 func Tar(path string, compression Compression) (io.Reader, error) {
-	return TarFilter(path, compression, nil, true, nil)
+	return TarFilter(path, &TarOptions{Recursive: true, Compression: compression})
 }
 
 func escapeName(name string) string {
@@ -101,25 +109,29 @@ func escapeName(name string) string {
 
 // Tar creates an archive from the directory at `path`, only including files whose relative
 // paths are included in `filter`. If `filter` is nil, then all files are included.
-func TarFilter(path string, compression Compression, filter []string, recursive bool, createFiles []string) (io.Reader, error) {
+func TarFilter(path string, options *TarOptions) (io.Reader, error) {
 	args := []string{"tar", "--numeric-owner", "-f", "-", "-C", path, "-T", "-"}
-	if filter == nil {
-		filter = []string{"."}
+	if options.Includes == nil {
+		options.Includes = []string{"."}
+	}
+	args = append(args, "-c"+options.Compression.Flag())
+
+	for _, exclude := range options.Excludes {
+		args = append(args, fmt.Sprintf("--exclude=%s", exclude))
 	}
-	args = append(args, "-c"+compression.Flag())
 
-	if !recursive {
+	if !options.Recursive {
 		args = append(args, "--no-recursion")
 	}
 
 	files := ""
-	for _, f := range filter {
+	for _, f := range options.Includes {
 		files = files + escapeName(f) + "\n"
 	}
 
 	tmpDir := ""
 
-	if createFiles != nil {
+	if options.CreateFiles != nil {
 		var err error // Can't use := here or we override the outer tmpDir
 		tmpDir, err = ioutil.TempDir("", "docker-tar")
 		if err != nil {
@@ -127,7 +139,7 @@ func TarFilter(path string, compression Compression, filter []string, recursive
 		}
 
 		files = files + "-C" + tmpDir + "\n"
-		for _, f := range createFiles {
+		for _, f := range options.CreateFiles {
 			path := filepath.Join(tmpDir, f)
 			err := os.MkdirAll(filepath.Dir(path), 0600)
 			if err != nil {
@@ -155,7 +167,7 @@ func TarFilter(path string, compression Compression, filter []string, recursive
 // The archive may be compressed with one of the following algorithms:
 //  identity (uncompressed), gzip, bzip2, xz.
 // FIXME: specify behavior when target path exists vs. doesn't exist.
-func Untar(archive io.Reader, path string) error {
+func Untar(archive io.Reader, path string, options *TarOptions) error {
 	if archive == nil {
 		return fmt.Errorf("Empty archive")
 	}
@@ -176,8 +188,15 @@ func Untar(archive io.Reader, path string) error {
 	compression := DetectCompression(buf)
 
 	utils.Debugf("Archive compression detected: %s", compression.Extension())
+	args := []string{"--numeric-owner", "-f", "-", "-C", path, "-x" + compression.Flag()}
+
+	if options != nil {
+		for _, exclude := range options.Excludes {
+			args = append(args, fmt.Sprintf("--exclude=%s", exclude))
+		}
+	}
 
-	cmd := exec.Command("tar", "--numeric-owner", "-f", "-", "-C", path, "-x"+compression.Flag())
+	cmd := exec.Command("tar", args...)
 	cmd.Stdin = io.MultiReader(bytes.NewReader(buf), archive)
 	// Hardcode locale environment for predictable outcome regardless of host configuration.
 	//   (see https://github.com/dotcloud/docker/issues/355)
@@ -194,11 +213,11 @@ func Untar(archive io.Reader, path string) error {
 // TarUntar aborts and returns the error.
 func TarUntar(src string, filter []string, dst string) error {
 	utils.Debugf("TarUntar(%s %s %s)", src, filter, dst)
-	archive, err := TarFilter(src, Uncompressed, filter, true, nil)
+	archive, err := TarFilter(src, &TarOptions{Compression: Uncompressed, Includes: filter, Recursive: true})
 	if err != nil {
 		return err
 	}
-	return Untar(archive, dst)
+	return Untar(archive, dst, nil)
 }
 
 // UntarPath is a convenience function which looks for an archive
@@ -206,7 +225,7 @@ func TarUntar(src string, filter []string, dst string) error {
 func UntarPath(src, dst string) error {
 	if archive, err := os.Open(src); err != nil {
 		return err
-	} else if err := Untar(archive, dst); err != nil {
+	} else if err := Untar(archive, dst, nil); err != nil {
 		return err
 	}
 	return nil
@@ -275,7 +294,7 @@ func CopyFileWithTar(src, dst string) error {
 		return err
 	}
 	tw.Close()
-	return Untar(buf, filepath.Dir(dst))
+	return Untar(buf, filepath.Dir(dst), nil)
 }
 
 // CmdStream executes a command, and returns its stdout as a stream.

archive/archive_test.go

@@ -83,7 +83,7 @@ func tarUntar(t *testing.T, origin string, compression Compression) error {
 		return err
 	}
 	defer os.RemoveAll(tmp)
-	if err := Untar(archive, tmp); err != nil {
+	if err := Untar(archive, tmp, nil); err != nil {
 		return err
 	}
 	if _, err := os.Stat(tmp); err != nil {

archive/changes.go

@@ -207,24 +207,22 @@ func ChangesDirs(newDir, oldDir string) ([]Change, error) {
 	return changes, nil
 }
 
-
 func ExportChanges(root, rw string) (Archive, error) {
-        changes, err := ChangesDirs(root, rw)
-        if err != nil {
-                return nil, err
-        }
-        files := make([]string, 0)
-        deletions := make([]string, 0)
-        for _, change := range changes {
-                if change.Kind == ChangeModify || change.Kind == ChangeAdd {
-                        files = append(files, change.Path)
-                }
-                if change.Kind == ChangeDelete {
-                        base := filepath.Base(change.Path)
-                        dir := filepath.Dir(change.Path)
-                        deletions = append(deletions, filepath.Join(dir, ".wh."+base))
-                }
-        }
-        return TarFilter(root, Uncompressed, files, false, deletions)
+	changes, err := ChangesDirs(root, rw)
+	if err != nil {
+		return nil, err
+	}
+	files := make([]string, 0)
+	deletions := make([]string, 0)
+	for _, change := range changes {
+		if change.Kind == ChangeModify || change.Kind == ChangeAdd {
+			files = append(files, change.Path)
+		}
+		if change.Kind == ChangeDelete {
+			base := filepath.Base(change.Path)
+			dir := filepath.Dir(change.Path)
+			deletions = append(deletions, filepath.Join(dir, ".wh."+base))
+		}
+	}
+	return TarFilter(root, &TarOptions{Compression: Uncompressed, Recursive: false, Includes: files, CreateFiles: deletions})
 }
-

archive/diff.go

@@ -13,7 +13,7 @@ func ApplyLayer(dest string, layer Archive) error {
 	// Poor man's diff applyer in 2 steps:
 
 	// Step 1: untar everything in place
-	if err := Untar(layer, dest); err != nil {
+	if err := Untar(layer, dest, nil); err != nil {
 		return err
 	}
 

aufs/aufs.go

@@ -137,8 +137,7 @@ func (a *AufsDriver) createDirsFor(id string) error {
 	}
 
 	for _, p := range paths {
-		dir := path.Join(a.rootPath(), p, id)
-		if err := os.MkdirAll(dir, 0755); err != nil {
+		if err := os.MkdirAll(path.Join(a.rootPath(), p, id), 0755); err != nil {
 			return err
 		}
 	}
@@ -201,20 +200,19 @@ func (a *AufsDriver) Get(id string) (string, error) {
 
 // Returns an archive of the contents for the id
 func (a *AufsDriver) Diff(id string) (archive.Archive, error) {
-	p, err := a.Get(id)
-	if err != nil {
-		return nil, err
-	}
-	return archive.Tar(p, archive.Uncompressed)
+	return archive.TarFilter(path.Join(a.rootPath(), "diff", id), &archive.TarOptions{
+		Recursive:   true,
+		Compression: archive.Uncompressed,
+	})
+}
+
+func (a *AufsDriver) ApplyDiff(id string, diff archive.Archive) error {
+	return archive.Untar(diff, path.Join(a.rootPath(), "diff", id), nil)
 }
 
 // Returns the size of the contents for the id
-func (a *AufsDriver) DiffSize(id string) (int64, error) {
-	p, err := a.Get(id)
-	if err != nil {
-		return -1, err
-	}
-	return utils.TreeSize(p)
+func (a *AufsDriver) Size(id string) (int64, error) {
+	return utils.TreeSize(path.Join(a.rootPath(), "diff", id))
 }
 
 func (a *AufsDriver) Changes(id string) ([]archive.Change, error) {

buildfile.go

@@ -476,7 +476,7 @@ func (b *buildFile) Build(context io.Reader) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	if err := archive.Untar(context, name); err != nil {
+	if err := archive.Untar(context, name, nil); err != nil {
 		return "", err
 	}
 	defer os.RemoveAll(name)

commands.go

@@ -1911,7 +1911,7 @@ func (cli *DockerCli) CmdCp(args ...string) error {
 
 	if statusCode == 200 {
 		r := bytes.NewReader(data)
-		if err := archive.Untar(r, copyData.HostPath); err != nil {
+		if err := archive.Untar(r, copyData.HostPath, nil); err != nil {
 			return err
 		}
 	}

container.go

@@ -1479,10 +1479,13 @@ func validateID(id string) error {
 
 // GetSize, return real size, virtual size
 func (container *Container) GetSize() (int64, int64) {
-	var sizeRw, sizeRootfs int64
+	var (
+		sizeRw, sizeRootfs int64
+		err                error
+		driver             = container.runtime.driver
+	)
 
-	driver := container.runtime.driver
-	sizeRw, err := driver.DiffSize(container.ID)
+	sizeRw, err = driver.Size(container.ID)
 	if err != nil {
 		utils.Errorf("Warning: driver %s couldn't return diff size of container %s: %s", driver, container.ID, err)
 		// FIXME: GetSize should return an error. Not changing it now in case
@@ -1524,7 +1527,11 @@ func (container *Container) Copy(resource string) (archive.Archive, error) {
 		filter = []string{path.Base(basePath)}
 		basePath = path.Dir(basePath)
 	}
-	return archive.TarFilter(basePath, archive.Uncompressed, filter, true, nil)
+	return archive.TarFilter(basePath, &archive.TarOptions{
+		Compression: archive.Uncompressed,
+		Includes:    filter,
+		Recursive:   true,
+	})
 }
 
 // Returns true if the container exposes a certain port

container_test.go

@@ -1680,3 +1680,67 @@ func TestRestartGhost(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+func TestRemoveFile(t *testing.T) {
+	runtime := mkRuntime(t)
+	defer nuke(runtime)
+
+	container1, _ := mkContainer(runtime, []string{"_", "/bin/sh", "-c", "touch test.txt"}, t)
+	defer runtime.Destroy(container1)
+
+	if container1.State.Running {
+		t.Errorf("Container shouldn't be running")
+	}
+	if err := container1.Run(); err != nil {
+		t.Fatal(err)
+	}
+	if container1.State.Running {
+		t.Errorf("Container shouldn't be running")
+	}
+
+	commit := func(container *Container) (*Image, error) {
+		rwTar, err := container.ExportRw()
+		if err != nil {
+			return nil, err
+		}
+		img, err := runtime.graph.Create(rwTar, container, "unit test commited image", "", nil)
+		if err != nil {
+			return nil, err
+		}
+		return img, nil
+	}
+
+	img, err := commit(container1)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	container2, _ := mkContainer(runtime, []string{img.ID, "/bin/sh", "-c", "rm /test.txt"}, t)
+	defer runtime.Destroy(container2)
+
+	if err := container2.Run(); err != nil {
+		t.Fatal(err)
+	}
+
+	containerMount, err := runtime.driver.Get(container2.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := os.Stat(path.Join(containerMount, "test.txt")); err == nil {
+		t.Fatalf("test.txt should not exist")
+	}
+
+	img, err = commit(container2)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	mountPoint, err := runtime.driver.Get(img.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	file := path.Join(mountPoint, "test.txt")
+	if _, err := os.Stat(file); err == nil {
+		t.Fatalf("The file %s should not exist\n", file)
+	}
+}

devmapper/driver.go

@@ -56,7 +56,7 @@ func (d *Driver) Get(id string) (string, error) {
 	return mp, nil
 }
 
-func (d *Driver) DiffSize(id string) (int64, error) {
+func (d *Driver) Size(id string) (int64, error) {
 	return -1, fmt.Errorf("Not implemented")
 }
 

graph.go

@@ -151,6 +151,7 @@ func (graph *Graph) Register(jsonData []byte, layerData archive.Archive, img *Im
 	if err != nil {
 		return fmt.Errorf("Driver %s failed to get image rootfs %s: %s", graph.driver, img.ID, err)
 	}
+	img.graph = graph
 	if err := StoreImage(img, jsonData, layerData, tmp, rootfs); err != nil {
 		return err
 	}
@@ -158,7 +159,6 @@ func (graph *Graph) Register(jsonData []byte, layerData archive.Archive, img *Im
 	if err := os.Rename(tmp, graph.imageRoot(img.ID)); err != nil {
 		return err
 	}
-	img.graph = graph
 	graph.idIndex.Add(img.ID)
 	return nil
 }

graphdriver/driver.go

@@ -15,18 +15,15 @@ type Driver interface {
 	Remove(id string) error
 
 	Get(id string) (dir string, err error)
-
-	DiffSize(id string) (bytes int64, err error)
+	Size(id string) (bytes int64, err error)
 
 	Cleanup() error
 }
 
-type Changer interface {
-	Changes(id string) ([]archive.Change, error)
-}
-
 type Differ interface {
 	Diff(id string) (archive.Archive, error)
+	Changes(id string) ([]archive.Change, error)
+	ApplyDiff(id string, diff archive.Archive) error
 }
 
 var (

graphdriver/dummy/driver.go

@@ -73,6 +73,6 @@ func (d *Driver) Get(id string) (string, error) {
 	return dir, nil
 }
 
-func (d *Driver) DiffSize(id string) (int64, error) {
+func (d *Driver) Size(id string) (int64, error) {
 	return -1, fmt.Errorf("Not implemented")
 }

image.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"github.com/dotcloud/docker/archive"
+	"github.com/dotcloud/docker/graphdriver"
 	"github.com/dotcloud/docker/utils"
 	"io"
 	"io/ioutil"
@@ -70,12 +71,18 @@ func StoreImage(img *Image, jsonData []byte, layerData archive.Archive, root, ro
 
 	// If layerData is not nil, unpack it into the new layer
 	if layerData != nil {
-		start := time.Now()
-		utils.Debugf("Start untar layer")
-		if err := archive.Untar(layerData, layer); err != nil {
-			return err
+		if differ, ok := img.graph.driver.(graphdriver.Differ); ok {
+			if err := differ.ApplyDiff(img.ID, layerData); err != nil {
+				return err
+			}
+		} else {
+			start := time.Now()
+			utils.Debugf("Start untar layer")
+			if err := archive.ApplyLayer(layer, layerData); err != nil {
+				return err
+			}
+			utils.Debugf("Untar time: %vs", time.Now().Sub(start).Seconds())
 		}
-		utils.Debugf("Untar time: %vs", time.Now().Sub(start).Seconds())
 	}
 
 	// If raw json is provided, then use it

runtime.go

@@ -734,8 +734,8 @@ func (runtime *Runtime) Unmount(container *Container) error {
 }
 
 func (runtime *Runtime) Changes(container *Container) ([]archive.Change, error) {
-	if changer, ok := runtime.driver.(graphdriver.Changer); ok {
-		return changer.Changes(container.ID)
+	if differ, ok := runtime.driver.(graphdriver.Differ); ok {
+		return differ.Changes(container.ID)
 	}
 	cDir, err := runtime.driver.Get(container.ID)
 	if err != nil {
@@ -775,8 +775,13 @@ func (runtime *Runtime) Diff(container *Container) (archive.Archive, error) {
 			deletions = append(deletions, filepath.Join(dir, ".wh."+base))
 		}
 	}
-
-	return archive.TarFilter(cDir, archive.Uncompressed, files, false, deletions)
+	// 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,
+	})
 }
 
 func linkLxcStart(root string) error {

runtime_test.go

@@ -143,7 +143,7 @@ func setupBaseImage() {
 	if img, err := runtime.repositories.LookupImage(unitTestImageName); err != nil || img.ID != unitTestImageID {
 		// Retrieve the Image
 		if err := srv.ImagePull(unitTestImageName, "", os.Stdout, utils.NewStreamFormatter(false), nil, nil, true); err != nil {
-			log.Fatalf("Unable to pull the test image:", err)
+			log.Fatalf("Unable to pull the test image: %s", err)
 		}
 	}
 }