Correctly export the root file-system changes

[adrianreber]

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.

Fixes: #4606

[baude]

/approve

[rhatdan]

LGTM
@mheon @TomSweeneyRedHat @giuseppe @vrothberg @baude @QiWang19 @jwhonce PTAL

[vrothberg]

LGTM, but I prefer one more pair of eyes to run over

[rst0git]

@adrianreber when a folder inside the container is removed it looks like the complete root file-system becomes part of the checkpoint

podman run --name test -dit alpine
podman exec -l sh -c 'rm -rf srv'
podman container checkpoint -l -e podman.tar
tar tvf podman.tar
...
-rw-r--r-- root/root   5816320 2019-12-06 18:03 rootfs-diff.tar

[adrianreber]

@adrianreber when a folder inside the container is removed it looks like the complete root file-system becomes part of the checkpoint

podman run --name test -dit alpine
podman exec -l sh -c 'rm -rf srv'
podman container checkpoint -l -e podman.tar
tar tvf podman.tar
...
-rw-r--r-- root/root   5816320 2019-12-06 18:03 rootfs-diff.tar

I cannot reproduce that:

#  podman diff -l ; podman container checkpoint -l -e podman.tar.gz -R
A /SOME_FILE
D /root
b564fe891c82fdc019ac167bb7b729128191057e84d546df90e7b7e14ec3012a
#  tar xf podman.tar
# ls -la
drwxr-xr-x.  4 root root   176 Dec  6 18:44 .
dr-xr-x---. 10 root root  4096 Dec  6 14:09 ..
drwxr-xr-x.  2 root root     6 Dec  6 18:34 artifacts
drw-r-xr-x.  2 root root  4096 Dec  6 18:44 checkpoint
-rw-r--r--.  1 root root 50215 Dec  6 18:44 config.dump
-rw-------.  1 root root     0 Dec  6 18:34 ctr.log
-rw-------.  1 root root    16 Dec  6 18:44 deleted.files
-rw-r--r--.  1 root root   791 Dec  6 18:44 network.status
-rw-------.  1 root root 48841 Dec  6 18:44 podman.tar
-rw-r--r--.  1 root root  4096 Dec  6 18:44 rootfs-diff.tar
-rw-r--r--.  1 root root 45766 Dec  6 18:44 spec.dump
#️  tar tf rootfs-diff.tar 
SOME_FILE

Looks like your Podman binary has not the changes from this pull request. Can you double check?

[rst0git]

Looks like your Podman binary has not the changes from this pull request. Can you double check?

Yes, I updated the Podman binary with the changes from this pull request.
It occurs only when rootfsIncludeFiles = []. The following fix seems to work:

diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 25688db5..709e3655 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -622,26 +622,27 @@ func (c *Container) exportCheckpoint(dest string, ignoreRootfs bool) (err error)
 			}
 		}
 
-		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
+		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")
 		}
 
-		includeFiles = append(includeFiles, "rootfs-diff.tar")
-
 		if len(deletedFiles) > 0 {
 			formatJSON, err := json.MarshalIndent(deletedFiles, "", "     ")
 			if err != nil {

[adrianreber]

@rst0git Right. Only deleting files is not working. Thanks. Will fix it.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianreber, baude, rst0git

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [baude]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[adrianreber]

Can this be merged?

[rhatdan]

/lgtm

[stevekuznetsov]

/hold

"PR is unmergable. Do the Tide merge requirements match the GitHub settings for the repo? At least 1 approving review is required by reviewers with write access."

I suggest you remove that requirement... it's duplicated from /approve

[rhatdan]

/lgtm

[mheon]

Github appears to have stopped complaining, I have a manual merge option now. I'm going to remove the hold and see if branch protection kicks in.

/hold cancel

[mheon]

Still seems stuck.

I'm going to merge manually.

[mheon]

Oh, nevermind, it went.