i/{statemachine,helper}: Use the sparse option on tar

Uncompressed archive benefit the most out of this improvement. Compressed archives are not smaller (only tested on small archives, this might not be fully representative) but according to the doc it should be at least faster to generate. Also rework test to check the size so we could detect if the `--sparse` flag is removed in a future rework. Signed-off-by: Paul Mars <[email protected]>
canonical · Oct 10, 2024 · 1c720b0 · 1c720b0
1 parent 7bdc2a2
commit 1c720b0
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 23 deletions.
diff --git a/internal/helper/helper.go b/internal/helper/helper.go
@@ -389,6 +389,7 @@ func CreateTarArchive(src, dest, compression string, debug bool) error {
 		src,
 		"--xattrs",
 		"--xattrs-include=*",
+		"--sparse",
 		"--create",
 		"--file",
 		dest,

diff --git a/internal/statemachine/classic_states.go b/internal/statemachine/classic_states.go
@@ -1299,13 +1299,16 @@ var generateRootfsTarballState = stateFunc{"generate_rootfs_tarball", (*StateMac
 func (stateMachine *StateMachine) generateRootfsTarball() error {
 	classicStateMachine := stateMachine.parent.(*ClassicStateMachine)
 
-	// first create a vanilla uncompressed tar archive
-	rootfsSrc := filepath.Join(stateMachine.stateMachineFlags.WorkDir, "root")
-	rootfsDst := filepath.Join(stateMachine.commonFlags.OutputDir,
-		classicStateMachine.ImageDef.Artifacts.RootfsTar.RootfsTarName)
-	return helper.CreateTarArchive(rootfsSrc, rootfsDst,
+	tarDst := filepath.Join(
+		stateMachine.commonFlags.OutputDir,
+		classicStateMachine.ImageDef.Artifacts.RootfsTar.RootfsTarName,
+	)
+	return helper.CreateTarArchive(
+		stateMachine.tempDirs.rootfs,
+		tarDst,
 		classicStateMachine.ImageDef.Artifacts.RootfsTar.Compression,
-		stateMachine.commonFlags.Verbose, stateMachine.commonFlags.Debug)
+		stateMachine.commonFlags.Debug,
+	)
 }
 
 var makeQcow2ImgState = stateFunc{"make_qcow2_image", (*StateMachine).makeQcow2Img}

diff --git a/internal/statemachine/classic_test.go b/internal/statemachine/classic_test.go
@@ -4666,37 +4666,51 @@ func TestStateMachine_customizeFstab_fail(t *testing.T) {
 }
 
 // TestGenerateRootfsTarball tests that a rootfs tarball is generated
-// when appropriate and that it contains the correct files
+// when appropriate
 func TestGenerateRootfsTarball(t *testing.T) {
 	testCases := []struct {
 		name     string // the name will double as the compression type
 		tarPath  string
 		fileType string
+		// Define an interval since we cannot predict the exact
+		// size of the resulting archive due to changing atime/ctime
+		minArchiveSize int64
+		maxArchiveSize int64
 	}{
 		{
-			"uncompressed",
-			"test_generate_rootfs_tarball.tar",
-			"tar archive",
+			name:           "uncompressed",
+			tarPath:        "test_generate_rootfs_tarball.tar",
+			fileType:       "tar archive",
+			minArchiveSize: 61440,
+			maxArchiveSize: 61440, // 92160 without --sparse option
 		},
 		{
-			"bzip2",
-			"test_generate_rootfs_tarball.tar.bz2",
-			"bzip2 compressed data",
+			name:           "bzip2",
+			tarPath:        "test_generate_rootfs_tarball.tar.bz2",
+			fileType:       "bzip2 compressed data",
+			minArchiveSize: 32140,
+			maxArchiveSize: 32300,
 		},
 		{
-			"gzip",
-			"test_generate_rootfs_tarball.tar.gz",
-			"gzip compressed data",
+			name:           "gzip",
+			tarPath:        "test_generate_rootfs_tarball.tar.gz",
+			fileType:       "gzip compressed data",
+			minArchiveSize: 31700,
+			maxArchiveSize: 31900,
 		},
 		{
-			"xz",
-			"test_generate_rootfs_tarball.tar.xz",
-			"XZ compressed data",
+			name:           "xz",
+			tarPath:        "test_generate_rootfs_tarball.tar.xz",
+			fileType:       "XZ compressed data",
+			minArchiveSize: 31800,
+			maxArchiveSize: 31910,
 		},
 		{
-			"zstd",
-			"test_generate_rootfs_tarball.tar.zst",
-			"Zstandard compressed data",
+			name:           "zstd",
+			tarPath:        "test_generate_rootfs_tarball.tar.zst",
+			fileType:       "Zstandard compressed data",
+			minArchiveSize: 31500,
+			maxArchiveSize: 31700,
 		},
 	}
 	for _, tc := range testCases {
@@ -4724,6 +4738,27 @@ func TestGenerateRootfsTarball(t *testing.T) {
 			asserter.AssertErrNil(err, true)
 			stateMachine.commonFlags.OutputDir = stateMachine.stateMachineFlags.WorkDir
 
+			t.Cleanup(func() { os.RemoveAll(stateMachine.stateMachineFlags.WorkDir) })
+
+			// Copy a rootfs so the tar is not empty
+			rootfsSource := filepath.Join("testdata", "rootfs", "root")
+			err = osutilCopyFile(rootfsSource, stateMachine.stateMachineFlags.WorkDir, osutil.CopyFlagPreserveAll)
+			asserter.AssertErrNil(err, true)
+
+			// Make sure the root dir contains a sparse file
+			sparseFilePath := filepath.Join(stateMachine.tempDirs.rootfs, "bin", "sparseablefile")
+			sparseFilePathTemp := filepath.Join(stateMachine.tempDirs.rootfs, "bin", "sparseablefiletmp")
+			sparsifyOutputStep1, err := exec.Command("cp", "--sparse=always", sparseFilePath, sparseFilePathTemp).CombinedOutput()
+			if err != nil {
+				t.Error(string(sparsifyOutputStep1))
+				asserter.AssertErrNil(err, true)
+			}
+			sparsifyOutputStep2, err := exec.Command("mv", sparseFilePathTemp, sparseFilePath).CombinedOutput()
+			if err != nil {
+				t.Error(string(sparsifyOutputStep2))
+				asserter.AssertErrNil(err, true)
+			}
+
 			err = stateMachine.generateRootfsTarball()
 			asserter.AssertErrNil(err, true)
 
@@ -4734,13 +4769,24 @@ func TestGenerateRootfsTarball(t *testing.T) {
 			}
 
 			fullPath := filepath.Join(stateMachine.commonFlags.OutputDir, tc.tarPath)
-			fileCommand := *exec.Command("file", fullPath)
+			fileCommand := exec.Command("file", fullPath)
 			cmdOutput, err := fileCommand.CombinedOutput()
 			asserter.AssertErrNil(err, true)
 			if !strings.Contains(string(cmdOutput), tc.fileType) {
 				t.Errorf("File \"%s\" is the wrong file type. Expected \"%s\" but got \"%s\"",
 					fullPath, tc.fileType, string(cmdOutput))
 			}
+
+			fileInfo, err := os.Stat(fullPath)
+			asserter.AssertErrNil(err, true)
+
+			if fileInfo.Size() < tc.minArchiveSize {
+				asserter.Errorf("Archive too small.\ngot: %d\nwant a minimum of: %d", fileInfo.Size(), tc.minArchiveSize)
+			}
+
+			if fileInfo.Size() > tc.maxArchiveSize {
+				asserter.Errorf("Archive too big.\ngot: %d\nwant a maximum of: %d", fileInfo.Size(), tc.maxArchiveSize)
+			}
 		})
 	}
 }

diff --git a/internal/statemachine/testdata/rootfs/root/bin/badtocompress b/internal/statemachine/testdata/rootfs/root/bin/badtocompress
diff --git a/internal/statemachine/testdata/rootfs/root/bin/sh b/internal/statemachine/testdata/rootfs/root/bin/sh
@@ -0,0 +1 @@
+test
diff --git a/internal/statemachine/testdata/rootfs/root/bin/sparseablefile b/internal/statemachine/testdata/rootfs/root/bin/sparseablefile
diff --git a/internal/statemachine/testdata/rootfs/root/etc/fstab b/internal/statemachine/testdata/rootfs/root/etc/fstab
@@ -0,0 +1 @@
+sample file