fix(network_linux): fix umount failure

pkg/util/machine/network_linux.go

@@ -32,6 +32,7 @@ import (
 	"strings"
 	"sync"
 	"syscall"
+	"time"
 	"unsafe"
 
 	"github.com/moby/sys/mountinfo"
@@ -48,11 +49,6 @@ import (
 	sysm "github.com/kubewharf/katalyst-core/pkg/util/sysfs/manager"
 )
 
-const (
-	sysFSDirNormal   = "/sys"
-	sysFSDirNetNSTmp = "/tmp/net_ns_sysfs"
-)
-
 const (
 	nicPathNameDeviceFormatPCI = "devices/pci"
 	nicPathNAMEBaseDir         = "class/net"
@@ -72,7 +68,7 @@ const (
 const (
 	DefaultNetNSDir    = "/var/run/netns"
 	DefaultNetNSSysDir = "/sys"
-	TmpNetNSSysDir     = "/tmp/net_ns_sysfs"
+	TmpNetNSSysDir     = "/qrm_tmp_sys/net_ns_sysfs"
 	ClassNetBasePath   = "class/net"
 	ClassNetFulPath    = "/sys/class/net"
 	VirtioNetDriver    = "virtio_net"
@@ -1234,8 +1230,28 @@ func (nsc *netnsSwitchContext) netnsExit() {
 
 	// umount tmp sysfs in the new netns
 	if nsc.sysDirRemounted && nsc.sysMountDir != "" {
+		unmountTried := 0
+	retry:
 		if err := syscall.Unmount(nsc.sysMountDir, 0); err != nil {
-			klog.Fatalf("failed to Unmount(%s), err %v", nsc.sysMountDir, err)
+			mounted, e := mountinfo.Mounted(nsc.sysMountDir)
+			if e == nil && !mounted {
+				klog.Warningf("%s has been unmountd, maybe unexpected unmount happened", nsc.sysMountDir)
+			} else {
+				if e != nil {
+					klog.Errorf("check mounted dir: %s failed with error: %v", nsc.sysMountDir, e)
+				}
+
+				if unmountTried < 3 {
+					unmountTried++
+					// generally, the reason of unmount failure is that some files in the mountpoint are
+					// being accessed by other processes.
+					// Wait a while for other processes to finish accessing the files in the mountpoint (nsc.sysMountDir).
+					time.Sleep(10 * time.Second)
+					goto retry
+				}
+
+				klog.Errorf("failed to Unmount(%s), err %v", nsc.sysMountDir, err)
+			}
 		}
 	}
 

pkg/util/machine/network_linux_test.go

@@ -232,7 +232,7 @@ func testDoNetNS(t *testing.T) {
 			mockSetup:       func() {},
 			expectError:     false,
 			cb: func(sysFsDir string) error {
-				So(sysFsDir, ShouldEqual, sysFSDirNormal)
+				So(sysFsDir, ShouldEqual, DefaultNetNSSysDir)
 				return nil
 			},
 		},
@@ -254,7 +254,7 @@ func testDoNetNS(t *testing.T) {
 			},
 			expectError: false,
 			cb: func(sysFsDir string) error {
-				So(sysFsDir, ShouldEqual, sysFSDirNetNSTmp)
+				So(sysFsDir, ShouldEqual, TmpNetNSSysDir)
 				return nil
 			},
 		},
@@ -323,7 +323,7 @@ func testDoNetNS(t *testing.T) {
 				Mock(os.Stat).Return(nil, fmt.Errorf("stat error")).Build()
 			},
 			expectError:            true,
-			expectedErrorSubstring: "failed to stat /tmp/net_ns_sysfs, err stat error",
+			expectedErrorSubstring: "failed to stat /qrm_tmp_sys/net_ns_sysfs, err stat error",
 			cb:                     func(sysFsDir string) error { return nil },
 		},
 		{
@@ -341,7 +341,7 @@ func testDoNetNS(t *testing.T) {
 				Mock(os.MkdirAll).Return(fmt.Errorf("mkdir error")).Build()
 			},
 			expectError:            true,
-			expectedErrorSubstring: "failed to create /tmp/net_ns_sysfs, err mkdir error",
+			expectedErrorSubstring: "failed to create /qrm_tmp_sys/net_ns_sysfs, err mkdir error",
 			cb:                     func(sysFsDir string) error { return nil },
 		},
 		{
@@ -359,7 +359,7 @@ func testDoNetNS(t *testing.T) {
 				Mock(mountinfo.Mounted).Return(false, fmt.Errorf("mounted check error")).Build()
 			},
 			expectError:            true,
-			expectedErrorSubstring: "check mounted dir: /tmp/net_ns_sysfs failed with error: mounted check error",
+			expectedErrorSubstring: "check mounted dir: /qrm_tmp_sys/net_ns_sysfs failed with error: mounted check error",
 			cb:                     func(sysFsDir string) error { return nil },
 		},
 		{
@@ -378,7 +378,7 @@ func testDoNetNS(t *testing.T) {
 				Mock(syscall.Mount).Return(fmt.Errorf("mount error")).Build()
 			},
 			expectError:            true,
-			expectedErrorSubstring: "failed to mount sysfs at /tmp/net_ns_sysfs, err mount error",
+			expectedErrorSubstring: "failed to mount sysfs at /qrm_tmp_sys/net_ns_sysfs, err mount error",
 			cb:                     func(sysFsDir string) error { return nil },
 		},
 		{
@@ -421,7 +421,7 @@ func testDoNetNS(t *testing.T) {
 			},
 			expectError: false,
 			cb: func(sysFsDir string) error {
-				So(sysFsDir, ShouldEqual, sysFSDirNetNSTmp)
+				So(sysFsDir, ShouldEqual, TmpNetNSSysDir)
 				return nil
 			},
 		},
@@ -444,7 +444,7 @@ func testDoNetNS(t *testing.T) {
 			},
 			expectError: false,
 			cb: func(sysFsDir string) error {
-				So(sysFsDir, ShouldEqual, sysFSDirNetNSTmp)
+				So(sysFsDir, ShouldEqual, TmpNetNSSysDir)
 				return nil
 			},
 		},
@@ -3280,3 +3280,45 @@ func Test_GetNetDevRxPackets(t *testing.T) {
 		})
 	})
 }
+
+func Test_netnsExit(t *testing.T) {
+	PatchConvey("Test_netnsExit", t, func() {
+		PatchConvey("Scenario1 : succeed to exit default netns", func() {
+			nsc := &netnsSwitchContext{
+				newNetNSName: DefaultNICNamespace,
+			}
+
+			nsc.netnsExit()
+		})
+
+		PatchConvey("Scenario 2: failed to exit netns (mountinfo.Mounted check failed)", func() {
+			nsc := &netnsSwitchContext{
+				newNetNSName:    "ns2",
+				sysMountDir:     TmpNetNSSysDir,
+				sysDirRemounted: true,
+				locked:          false,
+			}
+
+			originalNetNSHdl, _ := netns.Get()
+			nsc.originalNetNSHdl = originalNetNSHdl
+			umountErr := errors.New("failed to Unmount")
+
+			Mock(syscall.Unmount).To(func(target string, flags int) error {
+				return umountErr
+			}).Build()
+
+			Mock(time.Sleep).To(func(d time.Duration) {
+			}).Build()
+
+			Mock((*netns.NsHandle).Close).To(func() error {
+				return nil
+			}).Build()
+
+			Mock(netns.Set).To(func(ns netns.NsHandle) error {
+				return nil
+			}).Build()
+
+			nsc.netnsExit()
+		})
+	})
+}