From bc42ffa7620946206f43d1cde8a2abb2326d739f Mon Sep 17 00:00:00 2001 From: Aditya Dani Date: Thu, 29 Aug 2024 14:26:17 -0700 Subject: [PATCH] PWX-38760: Handle NFS Mounter's Reload when both DNS and IPs are configured (#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 Signed-off-by: pnookala-px Co-authored-by: pnookala-px --- api/flexvolume/flexvolume.go | 4 ++-- pkg/mount/device_test.go | 4 ++-- pkg/mount/mount.go | 5 +++-- pkg/mount/mount_test.go | 19 +++++++++++------ pkg/mount/nfs.go | 40 ++++++++++++++++++++++++++++++------ volume/drivers/nfs/nfs.go | 2 +- 6 files changed, 55 insertions(+), 19 deletions(-) diff --git a/api/flexvolume/flexvolume.go b/api/flexvolume/flexvolume.go index 8cdc9361a..a1564966d 100644 --- 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 index 27c2d6fd2..dfef40a12 100644 --- 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 index c15af9a4a..1014d2967 100644 --- 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 index 6d4eaa733..58ba2007f 100644 --- 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 index 61d483feb..624248637 100644 --- 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 index 1084b6eb9..f127465ae 100644 --- 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