PWX-38760: Handle NFS Mounter's Reload when both DNS and IPs are conf…

…igured (#2477) (#2478) * PWX-38760: Handle NFS Mounter's Reload when both DNS and IPs are configured. - While reloading for a specific source, we used to check if the in-memory mount table has an entry for that source. - Since source can be an IP or DNS, we need to check if any of the existing sources which are DNS entries resolve to the input source. If found use that mount table entry. * fixup: review comments * fixup: test case args --------- Signed-off-by: Aditya Dani <[email protected]> Signed-off-by: pnookala-px <[email protected]> Co-authored-by: pnookala-px <[email protected]>
libopenstorage · Aug 29, 2024 · bc42ffa · bc42ffa
1 parent b0a2c10
commit bc42ffa
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 19 deletions.
diff --git a/api/flexvolume/flexvolume.go b/api/flexvolume/flexvolume.go
@@ -96,7 +96,7 @@ func (c *flexVolumeClient) Mount(targetMountDir string, mountDevice string,
 		return err
 	}
 	// Update the deviceDriverMap
-	mountManager, err := mount.New(mount.DeviceMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, "")
+	mountManager, err := mount.New(mount.DeviceMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, "", false)
 	if err != nil {
 		logrus.Infof("Could not read mountpoints from /proc/self/mountinfo. Device - Driver mapping not saved!")
 		return nil
@@ -111,7 +111,7 @@ func (c *flexVolumeClient) Mount(targetMountDir string, mountDevice string,
 
 func (c *flexVolumeClient) Unmount(mountDir string, options map[string]string) error {
 	// Get the mountDevice from mount manager
-	mountManager, err := mount.New(mount.DeviceMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, "")
+	mountManager, err := mount.New(mount.DeviceMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, "", false)
 	if err != nil {
 		return ErrNoMountInfo
 	}

diff --git a/pkg/mount/device_test.go b/pkg/mount/device_test.go
@@ -66,7 +66,7 @@ func TestBasicDeviceMounter(t *testing.T) {
 	orderedDevices := []string{device1}
 	orderedPaths := []string{mountPath1}
 
-	dm, err := New(DeviceMount, nil, []*regexp.Regexp{regexp.MustCompile(osdDevicePrefix)}, nil, []string{}, "")
+	dm, err := New(DeviceMount, nil, []*regexp.Regexp{regexp.MustCompile(osdDevicePrefix)}, nil, []string{}, "", false)
 	require.NoError(t, err, "Unexpected error on mount.New")
 
 	// Inspect
@@ -106,7 +106,7 @@ func TestBasicDeviceMounterWithMultipleMounts(t *testing.T) {
 	orderedDevices := []string{device1, device1}
 	orderedPaths := []string{mountPath1, mountPath2}
 
-	dm, err := New(DeviceMount, nil, []*regexp.Regexp{regexp.MustCompile(osdDevicePrefix)}, nil, []string{}, "")
+	dm, err := New(DeviceMount, nil, []*regexp.Regexp{regexp.MustCompile(osdDevicePrefix)}, nil, []string{}, "", false)
 	require.NoError(t, err, "Unexpected error on mount.New")
 
 	// Inspect

diff --git a/pkg/mount/mount.go b/pkg/mount/mount.go
@@ -793,7 +793,7 @@ func (m *Mounter) removeMountPath(path string) error {
 	}
 
 	var bindMountPath string
-	bindMounter, err := New(BindMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, "")
+	bindMounter, err := New(BindMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, "", false)
 	if err != nil {
 		return err
 	}
@@ -964,6 +964,7 @@ func New(
 	customMounter CustomMounter,
 	allowedDirs []string,
 	trashLocation string,
+	handleDNSResolution bool,
 ) (Manager, error) {
 
 	if mountImpl == nil {
@@ -974,7 +975,7 @@ func New(
 	case DeviceMount:
 		return NewDeviceMounter(identifiers, mountImpl, allowedDirs, trashLocation)
 	case NFSMount:
-		return NewNFSMounter(identifiers, mountImpl, allowedDirs, trashLocation)
+		return NewNFSMounter(identifiers, mountImpl, allowedDirs, trashLocation, handleDNSResolution)
 	case BindMount:
 		return NewBindMounter(identifiers, mountImpl, allowedDirs, trashLocation)
 	case CustomMount:

diff --git a/pkg/mount/mount_test.go b/pkg/mount/mount_test.go
@@ -32,9 +32,15 @@ func setLogger(fn string, t *testing.T) {
 	require.NoError(t, err, "unable to create log file")
 	logrus.SetOutput(logFile)
 }
+func TestNFSMounterHandleDNSResolution(t *testing.T) {
+	setLogger("TestNFSMounterHandleDNSResolution", t)
+	setupNFS(t, true)
+	allTests(t, source, dest)
+}
+
 func TestNFSMounter(t *testing.T) {
 	setLogger("TestNFSMounter", t)
-	setupNFS(t)
+	setupNFS(t, false)
 	allTests(t, source, dest)
 }
 
@@ -69,9 +75,9 @@ func allTests(t *testing.T, source, dest string) {
 	shutdown(t, source, dest)
 }
 
-func setupNFS(t *testing.T) {
+func setupNFS(t *testing.T, handleDNSResolution bool) {
 	var err error
-	m, err = New(NFSMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, trashLocation)
+	m, err = New(NFSMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, trashLocation, handleDNSResolution)
 	if err != nil {
 		t.Fatalf("Failed to setup test %v", err)
 	}
@@ -81,7 +87,7 @@ func setupNFS(t *testing.T) {
 
 func setupBindMounter(t *testing.T) {
 	var err error
-	m, err = New(BindMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, trashLocation)
+	m, err = New(BindMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, trashLocation, false)
 	if err != nil {
 		t.Fatalf("Failed to setup test %v", err)
 	}
@@ -91,7 +97,7 @@ func setupBindMounter(t *testing.T) {
 
 func setupRawMounter(t *testing.T) {
 	var err error
-	m, err = New(RawMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, trashLocation)
+	m, err = New(RawMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, trashLocation, false)
 	if err != nil {
 		t.Fatalf("Failed to setup test %v", err)
 	}
@@ -511,9 +517,10 @@ func TestExtractSourcePath(t *testing.T) {
 		})
 	}
 }
+
 func TestSafeEmptyTrashDir(t *testing.T) {
 	sched.Init(time.Second)
-	m, err := New(NFSMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, "")
+	m, err := New(NFSMount, nil, []*regexp.Regexp{regexp.MustCompile("")}, nil, []string{}, "", true)
 	require.NoError(t, err, "Failed to setup test %v", err)
 
 	err = os.MkdirAll("/tmp/safe-empty-trash-dir-tests", 0755)

diff --git a/pkg/mount/nfs.go b/pkg/mount/nfs.go
@@ -9,8 +9,8 @@ import (
 	"regexp"
 	"strings"
 
-	"github.com/moby/sys/mountinfo"
 	"github.com/libopenstorage/openstorage/pkg/keylock"
+	"github.com/moby/sys/mountinfo"
 )
 
 const (
@@ -20,7 +20,8 @@ const (
 
 // nfsMounter implements Manager and keeps track of active mounts for volume drivers.
 type nfsMounter struct {
-	servers []*regexp.Regexp
+	servers             []*regexp.Regexp
+	handleDNSResolution bool
 	Mounter
 }
 
@@ -31,9 +32,11 @@ func NewNFSMounter(servers []*regexp.Regexp,
 	mountImpl MountImpl,
 	allowedDirs []string,
 	trashLocation string,
+	handleDNSResolution bool,
 ) (Manager, error) {
 	m := &nfsMounter{
-		servers: servers,
+		servers:             servers,
+		handleDNSResolution: handleDNSResolution,
 		Mounter: Mounter{
 			mountImpl:     mountImpl,
 			mounts:        make(DeviceMap),
@@ -51,11 +54,12 @@ func NewNFSMounter(servers []*regexp.Regexp,
 }
 
 // Reload reloads the mount table for the specified source/
-func (m *nfsMounter) Reload(source string) error {
+func (m *nfsMounter) Reload(inputSource string) error {
 	newNFSm, err := NewNFSMounter([]*regexp.Regexp{regexp.MustCompile(NFSAllServers)},
 		m.mountImpl,
 		m.Mounter.allowedDirs,
 		m.trashLocation,
+		m.handleDNSResolution,
 	)
 	if err != nil {
 		return err
@@ -66,11 +70,35 @@ func (m *nfsMounter) Reload(source string) error {
 		return fmt.Errorf("Internal error failed to convert %T",
 			newNFSmounter)
 	}
+	newM := newNFSmounter.mounts[inputSource]
 
-	return m.reload(source, newNFSmounter.mounts[source])
+	if m.handleDNSResolution && newM == nil {
+
+		// Check if the source is a IP:share combination which maps to an
+		// DNS:share combination.
+		resolvedInputSourceIPs := resolveToIPs(inputSource)
+		inputSourceExportPath := extractSourcePath(inputSource)
+		for existingSource, existingMountInfo := range newNFSmounter.mounts {
+			if inputSourceExportPath == extractSourcePath(existingSource) {
+				// Found a match for the same export path.
+				// Now lets check if the input source (IP or DNS) matches
+				// with the existing source (IP or DNS).
+				resolvedExistingSourceIPs := resolveToIPs(existingSource)
+				if areSameIPs(resolvedExistingSourceIPs, resolvedInputSourceIPs) {
+					// The input source and existing source are the same.
+					// So, we can use the existing mount info even if it was for a
+					// a DNS representation of the server and our input source is an IP.
+					newM = existingMountInfo
+					break
+				}
+			}
+		}
+
+	}
+	return m.reload(inputSource, newM)
 }
 
-//serverExists utility function to test if a server is part of driver config
+// serverExists utility function to test if a server is part of driver config
 func (m *nfsMounter) serverExists(server string) bool {
 	for _, v := range m.servers {
 		vStr := v.String()

diff --git a/volume/drivers/nfs/nfs.go b/volume/drivers/nfs/nfs.go
@@ -78,7 +78,7 @@ func Init(params map[string]string) (volume.VolumeDriver, error) {
 	}
 
 	// Create a mount manager for this NFS server. Blank sever is OK.
-	mounter, err := mount.New(mount.NFSMount, nil, serverRegexes, nil, []string{}, "")
+	mounter, err := mount.New(mount.NFSMount, nil, serverRegexes, nil, []string{}, "", true)
 	if err != nil {
 		logrus.Warnf("Failed to create mount manager for server: %v (%v)", server, err)
 		return nil, err