Merge pull request #4643 from adrianreber/master

Correctly export the root file-system changes

4 files changed, 106 insertions, 57 deletions

diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 1b0570998..6ec06943f 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -593,22 +593,68 @@ 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 {
-		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())
+		// 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)
 		}
-		_, err = io.Copy(rootfsDiffFile, tarStream)
-		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)
+			}
+			defer rootfsDiffFile.Close()
+			_, err = io.Copy(rootfsDiffFile, rootfsTar)
+			if err != nil {
+				return err
+			}
+
+			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")
 		}
-		tarStream.Close()
-		rootfsDiffFile.Close()
-		includeFiles = append(includeFiles, "rootfs-diff.tar")
 	}
 
 	input, err := archive.TarWithOptions(c.bundlePath(), &archive.TarOptions{
@@ -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)
diff --git a/libpod/diff.go b/libpod/diff.go
index 925bda927..baa4d6ad7 100644
--- a/libpod/diff.go
+++ b/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)
diff --git a/pkg/adapter/checkpoint_restore.go b/pkg/adapter/checkpoint_restore.go
index 15f9e8105..7f80b782a 100644
--- a/pkg/adapter/checkpoint_restore.go
+++ b/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")
diff --git a/test/e2e/checkpoint_test.go b/test/e2e/checkpoint_test.go
index f208a4cf0..237223283 100644
--- a/test/e2e/checkpoint_test.go
+++ b/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)
 	})