Browse Source

Merge pull request #442 from dotcloud/fix_deleted_file_diff

Use aufs to handle parents whiteouts instead of doing it manually
Guillaume J. Charmes 12 years ago
parent
commit
03e4704ae5
2 changed files with 77 additions and 26 deletions
  1. 76 0
      container_test.go
  2. 1 26
      image.go

+ 76 - 0
container_test.go

@@ -150,6 +150,82 @@ func TestMultipleAttachRestart(t *testing.T) {
 	}
 }
 
+func TestDiff(t *testing.T) {
+	runtime, err := newTestRuntime()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer nuke(runtime)
+
+	// Create a container and remove a file
+	container1, err := runtime.Create(
+		&Config{
+			Image: GetTestImage(runtime).Id,
+			Cmd:   []string{"/bin/rm", "/etc/passwd"},
+		},
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer runtime.Destroy(container1)
+
+	if err := container1.Run(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Check the changelog
+	c, err := container1.Changes()
+	if err != nil {
+		t.Fatal(err)
+	}
+	success := false
+	for _, elem := range c {
+		if elem.Path == "/etc/passwd" && elem.Kind == 2 {
+			success = true
+		}
+	}
+	if !success {
+		t.Fatalf("/etc/passwd as been removed but is not present in the diff")
+	}
+
+	// Commit the container
+	rwTar, err := container1.ExportRw()
+	if err != nil {
+		t.Error(err)
+	}
+	img, err := runtime.graph.Create(rwTar, container1, "unit test commited image - diff", "")
+	if err != nil {
+		t.Error(err)
+	}
+
+	// Create a new container from the commited image
+	container2, err := runtime.Create(
+		&Config{
+			Image: img.Id,
+			Cmd:   []string{"cat", "/etc/passwd"},
+		},
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer runtime.Destroy(container2)
+
+	if err := container2.Run(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Check the changelog
+	c, err = container2.Changes()
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, elem := range c {
+		if elem.Path == "/etc/passwd" {
+			t.Fatalf("/etc/passwd should not be present in the diff after commit.")
+		}
+	}
+}
+
 func TestCommitRun(t *testing.T) {
 	runtime, err := newTestRuntime()
 	if err != nil {

+ 1 - 26
image.go

@@ -92,7 +92,7 @@ func MountAUFS(ro []string, rw string, target string) error {
 	rwBranch := fmt.Sprintf("%v=rw", rw)
 	roBranches := ""
 	for _, layer := range ro {
-		roBranches += fmt.Sprintf("%v=ro:", layer)
+		roBranches += fmt.Sprintf("%v=ro+wh:", layer)
 	}
 	branches := fmt.Sprintf("br:%v:%v", rwBranch, roBranches)
 
@@ -136,34 +136,9 @@ func (image *Image) Mount(root, rw string) error {
 	if err := os.Mkdir(rw, 0755); err != nil && !os.IsExist(err) {
 		return err
 	}
-	// FIXME: @creack shouldn't we do this after going over changes?
 	if err := MountAUFS(layers, rw, root); err != nil {
 		return err
 	}
-	// FIXME: Create tests for deletion
-	// FIXME: move this part to change.go
-	// Retrieve the changeset from the parent and apply it to the container
-	//  - Retrieve the changes
-	changes, err := Changes(layers, layers[0])
-	if err != nil {
-		return err
-	}
-	// Iterate on changes
-	for _, c := range changes {
-		// If there is a delete
-		if c.Kind == ChangeDelete {
-			// Make sure the directory exists
-			file_path, file_name := path.Dir(c.Path), path.Base(c.Path)
-			if err := os.MkdirAll(path.Join(rw, file_path), 0755); err != nil {
-				return err
-			}
-			// And create the whiteout (we just need to create empty file, discard the return)
-			if _, err := os.Create(path.Join(path.Join(rw, file_path),
-				".wh."+path.Base(file_name))); err != nil {
-				return err
-			}
-		}
-	}
 	return nil
 }