kernel: require a valid marker before skipping a KernelInstall rebuild

The KernelInstall idempotency guard only checked directory existence,
not completion. Since this path builds directly into destDir (no
scratch dir), an interrupted build or a marker-less tree from an older
snapd would be wrongly treated as already done. Require
DriversTreeNeedsCheck to confirm a current marker before skipping the
rebuild, and stop discarding DirExists/DriversTreeNeedsCheck errors.
Rebuilding in place on a stale/missing marker is safe: destDir is never
live-mounted at either KernelInstall call site.

Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>

kernel/kernel_drivers.go

@@ -420,7 +420,7 @@ type ModulesCompMountPoints struct {
 // from the initramfs). To consider all cases, we need to run depmod with links
 // to the currently available content, and then replace those links with the
 // expected mounts in the running system.
-func EnsureKernelDriversTree(kMntPts MountPoints, compsMntPts []ModulesCompMountPoints, destDir string, opts *KernelDriversTreeOptions) (err error) {
+func EnsureKernelDriversTree(kMntPts MountPoints, compsMntPts []ModulesCompMountPoints, destDir string, opts *KernelDriversTreeOptions) (retErr error) {
 	// The temporal dir when installing only components can be fixed as a
 	// task installing/updating a kernel-modules component must conflict
 	// with changes containing this same task. This helps with clean-ups if
@@ -430,28 +430,40 @@ func EnsureKernelDriversTree(kMntPts MountPoints, compsMntPts []ModulesCompMount
 	targetDir := destDir + "_tmp"
 	if opts.KernelInstall {
 		targetDir = destDir
-		exists, isDir, _ := osutil.DirExists(targetDir)
+		exists, isDir, err := osutil.DirExists(targetDir)
+		if err != nil {
+			return err
+		}
 		if exists && isDir {
-			logger.Debugf("device tree %q already created on installation, not re-creating",
+			// Require a current marker which is written last when building the
+			// tree. Otherwise fall through and rebuild in place (safe: destDir
+			// is never live-mounted here).
+			needsCheck, err := DriversTreeNeedsCheck(targetDir)
+			if err != nil {
+				return err
+			}
+			if !needsCheck {
+				logger.Debugf("device tree %q already created on installation, not re-creating",
+					targetDir)
+				return nil
+			}
+			logger.Debugf("device tree %q exists but is not up to date (missing or stale marker), rebuilding",
 				targetDir)
-			// Nothing was built here, so the existing marker (if any) is
-			// left untouched.
-			return nil
 		}
 	}
 	// Initial clean-up to make the function idempotent
 	if rmErr := RemoveKernelDriversTree(targetDir); rmErr != nil &&
-		!errors.Is(err, fs.ErrNotExist) {
+		!errors.Is(rmErr, fs.ErrNotExist) {
 		logger.Noticef("while removing old kernel tree: %v", rmErr)
 	}
 
 	defer func() {
 		// Remove on return if error or if temporary tree
-		if err == nil && opts.KernelInstall {
+		if retErr == nil && opts.KernelInstall {
 			return
 		}
 		if rmErr := RemoveKernelDriversTree(targetDir); rmErr != nil &&
-			!errors.Is(err, fs.ErrNotExist) {
+			!errors.Is(rmErr, fs.ErrNotExist) {
 			logger.Noticef("while cleaning up kernel tree: %v", rmErr)
 		}
 	}()

kernel/kernel_drivers_test.go

@@ -1003,3 +1003,66 @@ func (s *kernelDriversTestSuite) TestKernelInstallMarkerWriteFailureDiscardsTree
 
 	c.Check(osutil.FileExists(destDir), Equals, false)
 }
+
+func (s *kernelDriversTestSuite) TestKernelInstallRebuildsExistingTreeWithoutMarker(c *C) {
+	kversion := "5.15.0-78-generic"
+	mountDir := filepath.Join(dirs.SnapMountDir, "pc-kernel/1")
+	createKernelSnapFiles(c, kversion, mountDir, createKernelSnapFilesOpts{})
+
+	destDir := kernel.DriversTreeDir(dirs.GlobalRootDir, "pc-kernel", snap.R(1))
+	kMntPts := kernel.MountPoints{Current: mountDir, Target: mountDir}
+
+	// Simulate a tree that already exists as a directory, but was never
+	// actually finished being built (e.g. snapd was killed or the system
+	// rebooted mid-build), or was built by an older, marker-less snapd
+	// (e.g. reinstated via a revert): a directory is there, but there is
+	// no completion marker.
+	c.Assert(os.MkdirAll(destDir, 0755), IsNil)
+	strayPath := filepath.Join(destDir, "stray-leftover")
+	c.Assert(os.WriteFile(strayPath, []byte("junk"), 0644), IsNil)
+
+	err := kernel.EnsureKernelDriversTree(kMntPts, nil, destDir,
+		&kernel.KernelDriversTreeOptions{KernelInstall: true})
+	c.Assert(err, IsNil)
+
+	// The tree was actually (re)built this time, not silently left as-is:
+	// the stray leftover from the incomplete/legacy tree is gone, and a
+	// valid, current marker is now present.
+	c.Check(osutil.FileExists(strayPath), Equals, false)
+	v, err := kernel.ReadDriversTreeGeneratorVersion(destDir)
+	c.Assert(err, IsNil)
+	c.Check(v, Equals, kernel.KernelDriversTreeGeneratorVersion())
+
+	modsRoot := filepath.Join(destDir, "lib", "modules", kversion)
+	c.Check(osutil.FileExists(filepath.Join(modsRoot, "modules.dep.bin")), Equals, true)
+}
+
+func (s *kernelDriversTestSuite) TestKernelInstallSkipsRebuildWhenMarkerIsForwardOnly(c *C) {
+	kversion := "5.15.0-78-generic"
+	mountDir := filepath.Join(dirs.SnapMountDir, "pc-kernel/1")
+	createKernelSnapFiles(c, kversion, mountDir, createKernelSnapFilesOpts{})
+
+	destDir := kernel.DriversTreeDir(dirs.GlobalRootDir, "pc-kernel", snap.R(1))
+	kMntPts := kernel.MountPoints{Current: mountDir, Target: mountDir}
+
+	// Build once so the tree exists and carries a marker.
+	err := kernel.EnsureKernelDriversTree(kMntPts, nil, destDir,
+		&kernel.KernelDriversTreeOptions{KernelInstall: true})
+	c.Assert(err, IsNil)
+
+	// Simulate a marker written by a newer generator than what is
+	// currently running (e.g. after a snapd revert).
+	restore := kernel.MockKernelDriversTreeGeneratorVersion(100)
+	c.Assert(kernel.WriteDriversTreeMeta(destDir), IsNil)
+	restore()
+
+	strayPath := filepath.Join(destDir, "stray-leftover")
+	c.Assert(os.WriteFile(strayPath, []byte("junk"), 0644), IsNil)
+
+	err = kernel.EnsureKernelDriversTree(kMntPts, nil, destDir,
+		&kernel.KernelDriversTreeOptions{KernelInstall: true})
+	c.Assert(err, IsNil)
+
+	// Nothing was touched: the shortcut fired and left the tree as-is.
+	c.Check(osutil.FileExists(strayPath), Equals, true)
+}