Correctly export the root file-system changes

When doing a checkpoint with --export the root file-system diff was not
working as expected. Instead of getting the changes from the running
container to the highest storage layer it got the changes from the
highest layer to that parent's layer. For a one layer container this
could mean that the complete root file-system is part of the checkpoint.

With this commit this changes to use the same functionality as 'podman
diff'. This actually enables to correctly diff the root file-system
including tracking deleted files.

This also removes the non-working helper functions from libpod/diff.go.

Signed-off-by: Adrian Reber <areber@redhat.com>

libpod/container_internal_linux.go

@@ -593,24 +593,70 @@ func (c *Container) exportCheckpoint(dest string, ignoreRootfs bool) (err error)
 
 	// Get root file-system changes included in the checkpoint archive
 	rootfsDiffPath := filepath.Join(c.bundlePath(), "rootfs-diff.tar")
+	deleteFilesList := filepath.Join(c.bundlePath(), "deleted.files")
 	if !ignoreRootfs {
+		// To correctly track deleted files, let's go through the output of 'podman diff'
+		tarFiles, err := c.runtime.GetDiff("", c.ID())
+		if err != nil {
+			return errors.Wrapf(err, "error exporting root file-system diff to %q", rootfsDiffPath)
+		}
+		var rootfsIncludeFiles []string
+		var deletedFiles []string
+
+		for _, file := range tarFiles {
+			if file.Kind == archive.ChangeAdd {
+				rootfsIncludeFiles = append(rootfsIncludeFiles, file.Path)
+				continue
+			}
+			if file.Kind == archive.ChangeDelete {
+				deletedFiles = append(deletedFiles, file.Path)
+				continue
+			}
+			fileName, err := os.Stat(file.Path)
+			if err != nil {
+				continue
+			}
+			if !fileName.IsDir() && file.Kind == archive.ChangeModify {
+				rootfsIncludeFiles = append(rootfsIncludeFiles, file.Path)
+				continue
+			}
+		}
+
+		if len(rootfsIncludeFiles) > 0 {
+			rootfsTar, err := archive.TarWithOptions(c.state.Mountpoint, &archive.TarOptions{
+				Compression:      archive.Uncompressed,
+				IncludeSourceDir: true,
+				IncludeFiles:     rootfsIncludeFiles,
+			})
+			if err != nil {
+				return errors.Wrapf(err, "error exporting root file-system diff to %q", rootfsDiffPath)
+			}
 			rootfsDiffFile, err := os.Create(rootfsDiffPath)
 			if err != nil {
 				return errors.Wrapf(err, "error creating root file-system diff file %q", rootfsDiffPath)
 			}
-		tarStream, err := c.runtime.GetDiffTarStream("", c.ID())
+			defer rootfsDiffFile.Close()
+			_, err = io.Copy(rootfsDiffFile, rootfsTar)
 			if err != nil {
-			return errors.Wrapf(err, "error exporting root file-system diff to %q", rootfsDiffPath)
+				return err
 			}
-		_, err = io.Copy(rootfsDiffFile, tarStream)
-		if err != nil {
-			return errors.Wrapf(err, "error exporting root file-system diff to %q", rootfsDiffPath)
-		}
-		tarStream.Close()
-		rootfsDiffFile.Close()
+
 			includeFiles = append(includeFiles, "rootfs-diff.tar")
 		}
 
+		if len(deletedFiles) > 0 {
+			formatJSON, err := json.MarshalIndent(deletedFiles, "", "     ")
+			if err != nil {
+				return errors.Wrapf(err, "error creating delete files list file %q", deleteFilesList)
+			}
+			if err := ioutil.WriteFile(deleteFilesList, formatJSON, 0600); err != nil {
+				return errors.Wrapf(err, "error creating delete files list file %q", deleteFilesList)
+			}
+
+			includeFiles = append(includeFiles, "deleted.files")
+		}
+	}
+
 	input, err := archive.TarWithOptions(c.bundlePath(), &archive.TarOptions{
 		Compression:      archive.Gzip,
 		IncludeSourceDir: true,
@@ -637,6 +683,7 @@ func (c *Container) exportCheckpoint(dest string, ignoreRootfs bool) (err error)
 	}
 
 	os.Remove(rootfsDiffPath)
+	os.Remove(deleteFilesList)
 
 	return nil
 }
@@ -941,10 +988,35 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti
 			if err != nil {
 				return errors.Wrapf(err, "Failed to open root file-system diff file %s", rootfsDiffPath)
 			}
+			defer rootfsDiffFile.Close()
 			if err := c.runtime.ApplyDiffTarStream(c.ID(), rootfsDiffFile); err != nil {
 				return errors.Wrapf(err, "Failed to apply root file-system diff file %s", rootfsDiffPath)
 			}
-			rootfsDiffFile.Close()
+		}
+		deletedFilesPath := filepath.Join(c.bundlePath(), "deleted.files")
+		if _, err := os.Stat(deletedFilesPath); err == nil {
+			deletedFilesFile, err := os.Open(deletedFilesPath)
+			if err != nil {
+				return errors.Wrapf(err, "Failed to open deleted files file %s", deletedFilesPath)
+			}
+			defer deletedFilesFile.Close()
+
+			var deletedFiles []string
+			deletedFilesJSON, err := ioutil.ReadAll(deletedFilesFile)
+			if err != nil {
+				return errors.Wrapf(err, "Failed to read deleted files file %s", deletedFilesPath)
+			}
+			if err := json.Unmarshal(deletedFilesJSON, &deletedFiles); err != nil {
+				return errors.Wrapf(err, "Failed to read deleted files file %s", deletedFilesPath)
+			}
+			for _, deleteFile := range deletedFiles {
+				// Using RemoveAll as deletedFiles, which is generated from 'podman diff'
+				// lists completely deleted directories as a single entry: 'D /root'.
+				err = os.RemoveAll(filepath.Join(c.state.Mountpoint, deleteFile))
+				if err != nil {
+					return errors.Wrapf(err, "Failed to delete file %s from container %s during restore", deletedFilesPath, c.ID())
+				}
+			}
 		}
 	}
 
@@ -965,7 +1037,7 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti
 		if err != nil {
 			logrus.Debugf("Non-fatal: removal of checkpoint directory (%s) failed: %v", c.CheckpointPath(), err)
 		}
-		cleanup := [...]string{"restore.log", "dump.log", "stats-dump", "stats-restore", "network.status", "rootfs-diff.tar"}
+		cleanup := [...]string{"restore.log", "dump.log", "stats-dump", "stats-restore", "network.status", "rootfs-diff.tar", "deleted.files"}
 		for _, del := range cleanup {
 			file := filepath.Join(c.bundlePath(), del)
 			err = os.Remove(file)

libpod/diff.go

@@ -1,7 +1,6 @@
 package libpod
 
 import (
-	"archive/tar"
 	"io"
 
 	"github.com/containers/libpod/libpod/layers"
@@ -47,49 +46,6 @@ func (r *Runtime) GetDiff(from, to string) ([]archive.Change, error) {
 	return rchanges, err
 }
 
-// skipFileInTarAchive is an archive.TarModifierFunc function
-// which tells archive.ReplaceFileTarWrapper to skip files
-// from the tarstream
-func skipFileInTarAchive(path string, header *tar.Header, content io.Reader) (*tar.Header, []byte, error) {
-	return nil, nil, nil
-}
-
-// GetDiffTarStream returns the differences between the two images, layers, or containers.
-// It is the same functionality as GetDiff() except that it returns a tarstream
-func (r *Runtime) GetDiffTarStream(from, to string) (io.ReadCloser, error) {
-	toLayer, err := r.getLayerID(to)
-	if err != nil {
-		return nil, err
-	}
-	fromLayer := ""
-	if from != "" {
-		fromLayer, err = r.getLayerID(from)
-		if err != nil {
-			return nil, err
-		}
-	}
-	rc, err := r.store.Diff(fromLayer, toLayer, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	// Skip files in the tar archive which are listed
-	// in containerMounts map. Just as in the GetDiff()
-	// function from above
-	filterMap := make(map[string]archive.TarModifierFunc)
-	for key := range containerMounts {
-		filterMap[key[1:]] = skipFileInTarAchive
-		// In the tarstream directories always include a trailing '/'.
-		// For simplicity this duplicates every entry from
-		// containerMounts with a trailing '/', as containerMounts
-		// does not use trailing '/' for directories.
-		filterMap[key[1:]+"/"] = skipFileInTarAchive
-	}
-
-	filteredTarStream := archive.ReplaceFileTarWrapper(rc, filterMap)
-	return filteredTarStream, nil
-}
-
 // ApplyDiffTarStream applies the changes stored in 'diff' to the layer 'to'
 func (r *Runtime) ApplyDiffTarStream(to string, diff io.Reader) error {
 	toLayer, err := r.getLayerID(to)

pkg/adapter/checkpoint_restore.go

@@ -60,6 +60,7 @@ func crImportCheckpoint(ctx context.Context, runtime *libpod.Runtime, input stri
 			"ctr.log",
 			"rootfs-diff.tar",
 			"network.status",
+			"deleted.files",
 		},
 	}
 	dir, err := ioutil.TempDir("", "checkpoint")

test/e2e/checkpoint_test.go

@@ -439,6 +439,18 @@ var _ = Describe("Podman checkpoint", func() {
 		result.WaitWithDefaultTimeout()
 		Expect(result.ExitCode()).To(Equal(0))
 
+		result = podmanTest.Podman([]string{"exec", "-l", "/bin/sh", "-c", "rm /etc/motd"})
+		result.WaitWithDefaultTimeout()
+		Expect(result.ExitCode()).To(Equal(0))
+
+		result = podmanTest.Podman([]string{"diff", "-l"})
+		result.WaitWithDefaultTimeout()
+		Expect(result.ExitCode()).To(Equal(0))
+		Expect(result.OutputToString()).To(ContainSubstring("C /etc"))
+		Expect(result.OutputToString()).To(ContainSubstring("A /test.output"))
+		Expect(result.OutputToString()).To(ContainSubstring("D /etc/motd"))
+		Expect(len(result.OutputToStringArray())).To(Equal(3))
+
 		// Checkpoint the container
 		result = podmanTest.Podman([]string{"container", "checkpoint", "-l", "-e", fileName})
 		result.WaitWithDefaultTimeout()
@@ -462,6 +474,14 @@ var _ = Describe("Podman checkpoint", func() {
 		Expect(result.ExitCode()).To(Equal(0))
 		Expect(result.OutputToString()).To(ContainSubstring("test" + cid + "test"))
 
+		result = podmanTest.Podman([]string{"diff", "-l"})
+		result.WaitWithDefaultTimeout()
+		Expect(result.ExitCode()).To(Equal(0))
+		Expect(result.OutputToString()).To(ContainSubstring("C /etc"))
+		Expect(result.OutputToString()).To(ContainSubstring("A /test.output"))
+		Expect(result.OutputToString()).To(ContainSubstring("D /etc/motd"))
+		Expect(len(result.OutputToStringArray())).To(Equal(3))
+
 		// Remove exported checkpoint
 		os.Remove(fileName)
 	})