Merge pull request #122530 from neolit123/1.30-v1beta4-control-reset-unmount

kubeadm: more verbose unmount logic on "reset"

cmd/kubeadm/app/apis/kubeadm/types.go

@@ -522,8 +522,24 @@ type ResetConfiguration struct {
 	// SkipPhases is a list of phases to skip during command execution.
 	// The list of phases can be obtained with the "kubeadm reset phase --help" command.
 	SkipPhases []string
+
+	// UnmountFlags is a list of unmount2() syscall flags that kubeadm can use when unmounting
+	// directories during "reset". A flag can be one of: MNT_FORCE, MNT_DETACH, MNT_EXPIRE, UMOUNT_NOFOLLOW.
+	// By default this list is empty.
+	UnmountFlags []string
 }
 
+const (
+	// UnmountFlagMNTForce represents the flag "MNT_FORCE"
+	UnmountFlagMNTForce = "MNT_FORCE"
+	// UnmountFlagMNTDetach represents the flag "MNT_DETACH"
+	UnmountFlagMNTDetach = "MNT_DETACH"
+	// UnmountFlagMNTExpire represents the flag "MNT_EXPIRE"
+	UnmountFlagMNTExpire = "MNT_EXPIRE"
+	// UnmountFlagUmountNoFollow represents the flag "UMOUNT_NOFOLLOW"
+	UnmountFlagUmountNoFollow = "UMOUNT_NOFOLLOW"
+)
+
 // ComponentConfigMap is a map between a group name (as in GVK group) and a ComponentConfig
 type ComponentConfigMap map[string]ComponentConfig
 

cmd/kubeadm/app/apis/kubeadm/v1beta4/types.go

@@ -517,6 +517,12 @@ type ResetConfiguration struct {
 	// The list of phases can be obtained with the "kubeadm reset phase --help" command.
 	// +optional
 	SkipPhases []string `json:"skipPhases,omitempty"`
+
+	// UnmountFlags is a list of unmount2() syscall flags that kubeadm can use when unmounting
+	// directories during "reset". A flag can be one of: MNT_FORCE, MNT_DETACH, MNT_EXPIRE, UMOUNT_NOFOLLOW.
+	// By default this list is empty.
+	// +optional
+	UnmountFlags []string `json:"unmountFlags,omitempty"`
 }
 
 // Arg represents an argument with a name and a value.

cmd/kubeadm/app/apis/kubeadm/v1beta4/zz_generated.conversion.go

@@ -894,6 +894,7 @@ func autoConvert_v1beta4_ResetConfiguration_To_kubeadm_ResetConfiguration(in *Re
 	out.Force = in.Force
 	out.IgnorePreflightErrors = *(*[]string)(unsafe.Pointer(&in.IgnorePreflightErrors))
 	out.SkipPhases = *(*[]string)(unsafe.Pointer(&in.SkipPhases))
+	out.UnmountFlags = *(*[]string)(unsafe.Pointer(&in.UnmountFlags))
 	return nil
 }
 
@@ -910,6 +911,7 @@ func autoConvert_kubeadm_ResetConfiguration_To_v1beta4_ResetConfiguration(in *ku
 	out.Force = in.Force
 	out.IgnorePreflightErrors = *(*[]string)(unsafe.Pointer(&in.IgnorePreflightErrors))
 	out.SkipPhases = *(*[]string)(unsafe.Pointer(&in.SkipPhases))
+	out.UnmountFlags = *(*[]string)(unsafe.Pointer(&in.UnmountFlags))
 	return nil
 }
 

cmd/kubeadm/app/apis/kubeadm/v1beta4/zz_generated.deepcopy.go

@@ -577,6 +577,11 @@ func (in *ResetConfiguration) DeepCopyInto(out *ResetConfiguration) {
 		*out = make([]string, len(*in))
 		copy(*out, *in)
 	}
+	if in.UnmountFlags != nil {
+		in, out := &in.UnmountFlags, &out.UnmountFlags
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 	return
 }
 

cmd/kubeadm/app/apis/kubeadm/validation/validation.go

@@ -707,6 +707,7 @@ func ValidateResetConfiguration(c *kubeadm.ResetConfiguration) field.ErrorList {
 	allErrs := field.ErrorList{}
 	allErrs = append(allErrs, ValidateSocketPath(c.CRISocket, field.NewPath("criSocket"))...)
 	allErrs = append(allErrs, ValidateAbsolutePath(c.CertificatesDir, field.NewPath("certificatesDir"))...)
+	allErrs = append(allErrs, ValidateUnmountFlags(c.UnmountFlags, field.NewPath("unmountFlags"))...)
 	return allErrs
 }
 
@@ -722,3 +723,19 @@ func ValidateExtraArgs(args []kubeadm.Arg, fldPath *field.Path) field.ErrorList
 
 	return allErrs
 }
+
+// ValidateUnmountFlags validates a set of unmount flags and collects all encountered errors
+func ValidateUnmountFlags(flags []string, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	for idx, flag := range flags {
+		switch flag {
+		case kubeadm.UnmountFlagMNTForce, kubeadm.UnmountFlagMNTDetach, kubeadm.UnmountFlagMNTExpire, kubeadm.UnmountFlagUmountNoFollow:
+			continue
+		default:
+			allErrs = append(allErrs, field.Invalid(fldPath, fmt.Sprintf("index %d", idx), fmt.Sprintf("unknown unmount flag %s", flag)))
+		}
+	}
+
+	return allErrs
+}

cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go

@@ -1464,3 +1464,42 @@ func TestValidateExtraArgs(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateUnmountFlags(t *testing.T) {
+	var tests = []struct {
+		name           string
+		flags          []string
+		expectedErrors int
+	}{
+		{
+			name:           "nil input",
+			flags:          nil,
+			expectedErrors: 0,
+		},
+		{
+			name: "all valid flags",
+			flags: []string{
+				kubeadmapi.UnmountFlagMNTForce,
+				kubeadmapi.UnmountFlagMNTDetach,
+				kubeadmapi.UnmountFlagMNTExpire,
+				kubeadmapi.UnmountFlagUmountNoFollow,
+			},
+			expectedErrors: 0,
+		},
+		{
+			name: "invalid two flags",
+			flags: []string{
+				"foo",
+				"bar",
+			},
+			expectedErrors: 2,
+		},
+	}
+
+	for _, tc := range tests {
+		actual := ValidateUnmountFlags(tc.flags, nil)
+		if len(actual) != tc.expectedErrors {
+			t.Errorf("case %q:\n\t expected errors: %v\n\t got: %v\n\t errors: %v", tc.name, tc.expectedErrors, len(actual), actual)
+		}
+	}
+}

cmd/kubeadm/app/apis/kubeadm/zz_generated.deepcopy.go

@@ -607,6 +607,11 @@ func (in *ResetConfiguration) DeepCopyInto(out *ResetConfiguration) {
 		*out = make([]string, len(*in))
 		copy(*out, *in)
 	}
+	if in.UnmountFlags != nil {
+		in, out := &in.UnmountFlags, &out.UnmountFlags
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 	return
 }
 

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()
+		kubeletRunDirectory, err := absoluteKubeletRunDirectory()
-		if err == nil {
+		if err != nil {
-			// Only clean absoluteKubeletRunDirectory if umountDirsCmd passed without error
+			return err
-			dirsToClean = append(dirsToClean, kubeletRunDir)
 		}
+		// Unmount all mount paths under kubeletRunDirectory
+		if err := unmountKubeletDirectory(kubeletRunDirectory, r.ResetCfg().UnmountFlags); 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 "", errors.Wrapf(err, "failed to evaluate the %q directory", kubeadmconstants.KubeletRunDirectory)
-		return "", err
-	}
-	err = unmountKubeletDirectory(absoluteKubeletRunDirectory)
-	if err != nil {
-		klog.Warningf("[reset] Failed to unmount mounted directories in %s \n", kubeadmconstants.KubeletRunDirectory)
-		return "", err
 	}
 	return absoluteKubeletRunDirectory, nil
 }

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, flags []string) error {
 	klog.Warning("Cannot unmount filesystems on current OS, all mounted file systems will need to be manually unmounted")
 	return nil
 }

cmd/kubeadm/app/cmd/phases/reset/unmount_linux.go

@@ -24,30 +24,56 @@ import (
 	"strings"
 	"syscall"
 
+	"github.com/pkg/errors"
+	"golang.org/x/sys/unix"
+
 	"k8s.io/klog/v2"
+
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+
+	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 )
 
+var flagMap = map[string]int{
+	kubeadmapi.UnmountFlagMNTForce:       unix.MNT_FORCE,
+	kubeadmapi.UnmountFlagMNTDetach:      unix.MNT_DETACH,
+	kubeadmapi.UnmountFlagMNTExpire:      unix.MNT_EXPIRE,
+	kubeadmapi.UnmountFlagUmountNoFollow: unix.UMOUNT_NOFOLLOW,
+}
+
+func flagsToInt(flags []string) int {
+	res := 0
+	for _, f := range flags {
+		res |= flagMap[f]
+	}
+	return res
+}
+
 // unmountKubeletDirectory unmounts all paths that contain KubeletRunDirectory
-func unmountKubeletDirectory(absoluteKubeletRunDirectory string) error {
+func unmountKubeletDirectory(kubeletRunDirectory string, flags []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")
+	flagsInt := flagsToInt(flags)
 	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
 		}
-		if err := syscall.Unmount(m[1], 0); err != nil {
+		klog.V(5).Infof("[reset] Unmounting %q", m[1])
-			klog.Warningf("[reset] Failed to unmount mounted directory in %s: %s", absoluteKubeletRunDirectory, m[1])
+		if err := syscall.Unmount(m[1], flagsInt); err != nil {
+			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)
 }

cmd/kubeadm/app/cmd/phases/reset/unmount_linux_test.go

@@ -0,0 +1,65 @@
+//go:build linux
+// +build linux
+
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package phases
+
+import (
+	"testing"
+
+	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+)
+
+func TestFlagsToInt(t *testing.T) {
+
+	tests := []struct {
+		name           string
+		input          []string
+		expectedOutput int
+	}{
+		{
+			name:           "nil input",
+			input:          nil,
+			expectedOutput: 0,
+		},
+		{
+			name:           "no flags",
+			input:          []string{},
+			expectedOutput: 0,
+		},
+		{
+			name: "all flags",
+			input: []string{
+				kubeadmapi.UnmountFlagMNTForce,
+				kubeadmapi.UnmountFlagMNTDetach,
+				kubeadmapi.UnmountFlagMNTExpire,
+				kubeadmapi.UnmountFlagUmountNoFollow,
+			},
+			expectedOutput: 15,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			out := flagsToInt(tc.input)
+			if tc.expectedOutput != out {
+				t.Errorf("expected output %d, got %d", tc.expectedOutput, out)
+			}
+		})
+	}
+}