From d6bfd7daeb886ba9b4f273138319289646c57da3 Mon Sep 17 00:00:00 2001
From: "Lubomir I. Ivanov" <lubomirivanov@vmware.com>
Date: Sat, 30 Dec 2023 10:49:18 +0200
Subject: [PATCH] kubeadm: throw errors on unmount instead of warnings

Instead of warnings when syscall.Unmount() causes errors,
store all the errors in an aggregate. Abort the reset operation if
at least one unmount error was encountered.
---
 .../app/cmd/phases/reset/cleanupnode.go       | 20 +++++++++----------
 cmd/kubeadm/app/cmd/phases/reset/unmount.go   |  2 +-
 .../app/cmd/phases/reset/unmount_linux.go     | 19 ++++++++++++------
 3 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/cmd/kubeadm/app/cmd/phases/reset/cleanupnode.go b/cmd/kubeadm/app/cmd/phases/reset/cleanupnode.go
index d78a2926d91..01bbab89f4d 100644
--- a/cmd/kubeadm/app/cmd/phases/reset/cleanupnode.go
+++ b/cmd/kubeadm/app/cmd/phases/reset/cleanupnode.go
@@ -84,11 +84,15 @@ func runCleanupNode(c workflow.RunData) error {
 		// Try to unmount mounted directories under kubeadmconstants.KubeletRunDirectory in order to be able to remove the kubeadmconstants.KubeletRunDirectory directory later
 		fmt.Printf("[reset] Unmounting mounted directories in %q\n", kubeadmconstants.KubeletRunDirectory)
 		// In case KubeletRunDirectory holds a symbolic link, evaluate it
-		kubeletRunDir, err := absoluteKubeletRunDirectory()
-		if err == nil {
-			// Only clean absoluteKubeletRunDirectory if umountDirsCmd passed without error
-			dirsToClean = append(dirsToClean, kubeletRunDir)
+		kubeletRunDirectory, err := absoluteKubeletRunDirectory()
+		if err != nil {
+			return err
 		}
+		// Unmount all mount paths under kubeletRunDirectory
+		if err := unmountKubeletDirectory(kubeletRunDirectory); err != nil {
+			return err
+		}
+		dirsToClean = append(dirsToClean, kubeletRunDirectory)
 	} else {
 		fmt.Printf("[reset] Would unmount mounted directories in %q\n", kubeadmconstants.KubeletRunDirectory)
 	}
@@ -131,13 +135,7 @@ func runCleanupNode(c workflow.RunData) error {
 func absoluteKubeletRunDirectory() (string, error) {
 	absoluteKubeletRunDirectory, err := filepath.EvalSymlinks(kubeadmconstants.KubeletRunDirectory)
 	if err != nil {
-		klog.Warningf("[reset] Failed to evaluate the %q directory. Skipping its unmount and cleanup: %v\n", kubeadmconstants.KubeletRunDirectory, err)
-		return "", err
-	}
-	err = unmountKubeletDirectory(absoluteKubeletRunDirectory)
-	if err != nil {
-		klog.Warningf("[reset] Failed to unmount mounted directories in %s \n", kubeadmconstants.KubeletRunDirectory)
-		return "", err
+		return "", errors.Wrapf(err, "failed to evaluate the %q directory", kubeadmconstants.KubeletRunDirectory)
 	}
 	return absoluteKubeletRunDirectory, nil
 }
diff --git a/cmd/kubeadm/app/cmd/phases/reset/unmount.go b/cmd/kubeadm/app/cmd/phases/reset/unmount.go
index 1796da898b9..c6fbda56d87 100644
--- a/cmd/kubeadm/app/cmd/phases/reset/unmount.go
+++ b/cmd/kubeadm/app/cmd/phases/reset/unmount.go
@@ -24,7 +24,7 @@ import (
 )
 
 // unmountKubeletDirectory is a NOOP on all but linux.
-func unmountKubeletDirectory(absoluteKubeletRunDirectory string) error {
+func unmountKubeletDirectory(kubeletRunDirectory string) error {
 	klog.Warning("Cannot unmount filesystems on current OS, all mounted file systems will need to be manually unmounted")
 	return nil
 }
diff --git a/cmd/kubeadm/app/cmd/phases/reset/unmount_linux.go b/cmd/kubeadm/app/cmd/phases/reset/unmount_linux.go
index 993ab888dad..f7914b91026 100644
--- a/cmd/kubeadm/app/cmd/phases/reset/unmount_linux.go
+++ b/cmd/kubeadm/app/cmd/phases/reset/unmount_linux.go
@@ -24,30 +24,37 @@ import (
 	"strings"
 	"syscall"
 
+	"github.com/pkg/errors"
+
 	"k8s.io/klog/v2"
+
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 )
 
 // unmountKubeletDirectory unmounts all paths that contain KubeletRunDirectory
-func unmountKubeletDirectory(absoluteKubeletRunDirectory string) error {
+func unmountKubeletDirectory(kubeletRunDirectory string) error {
 	raw, err := os.ReadFile("/proc/mounts")
 	if err != nil {
 		return err
 	}
 
-	if !strings.HasSuffix(absoluteKubeletRunDirectory, "/") {
+	if !strings.HasSuffix(kubeletRunDirectory, "/") {
 		// trailing "/" is needed to ensure that possibly mounted /var/lib/kubelet is skipped
-		absoluteKubeletRunDirectory += "/"
+		kubeletRunDirectory += "/"
 	}
 
+	var errList []error
 	mounts := strings.Split(string(raw), "\n")
 	for _, mount := range mounts {
 		m := strings.Split(mount, " ")
-		if len(m) < 2 || !strings.HasPrefix(m[1], absoluteKubeletRunDirectory) {
+		if len(m) < 2 || !strings.HasPrefix(m[1], kubeletRunDirectory) {
 			continue
 		}
+		klog.V(5).Infof("[reset] Unmounting %q", m[1])
 		if err := syscall.Unmount(m[1], 0); err != nil {
-			klog.Warningf("[reset] Failed to unmount mounted directory in %s: %s", absoluteKubeletRunDirectory, m[1])
+			errList = append(errList, errors.WithMessagef(err, "failed to unmount %q", m[1]))
 		}
 	}
-	return nil
+	return errors.Wrapf(utilerrors.NewAggregate(errList),
+		"encountered the following errors while unmounting directories in %q", kubeletRunDirectory)
 }