From f9befb90a4c230c32dd233607b86af47b5263648 Mon Sep 17 00:00:00 2001
From: elbehery <melbeher@redhat.com>
Date: Tue, 13 Apr 2021 23:12:30 +0200
Subject: [PATCH] add pointer mutation review

---
 pkg/volume/nfs/nfs.go      | 15 +++++++++------
 pkg/volume/nfs/nfs_test.go | 30 +++++++++++++++++-------------
 2 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/pkg/volume/nfs/nfs.go b/pkg/volume/nfs/nfs.go
index cb64855a68b..29aa2e7dc3c 100644
--- a/pkg/volume/nfs/nfs.go
+++ b/pkg/volume/nfs/nfs.go
@@ -18,7 +18,7 @@ package nfs
 
 import (
 	"fmt"
-	"net"
+	netutil "k8s.io/utils/net"
 	"os"
 	"runtime"
 	"time"
@@ -122,10 +122,6 @@ func (plugin *nfsPlugin) newMounterInternal(spec *volume.Spec, pod *v1.Pod, moun
 	if err != nil {
 		return nil, err
 	}
-	// wrap ipv6 into `[ ]`
-	if net.ParseIP(source.Server).To16() != nil {
-		source.Server = fmt.Sprintf("[%s]", source.Server)
-	}
 	return &nfsMounter{
 		nfs: &nfs{
 			volName:         spec.Name(),
@@ -134,7 +130,7 @@ func (plugin *nfsPlugin) newMounterInternal(spec *volume.Spec, pod *v1.Pod, moun
 			plugin:          plugin,
 			MetricsProvider: volume.NewMetricsStatFS(getPath(pod.UID, spec.Name(), plugin.host)),
 		},
-		server:       source.Server,
+		server:       getServerFromSource(source),
 		exportPath:   source.Path,
 		readOnly:     readOnly,
 		mountOptions: util.MountOptionFromSpec(spec),
@@ -326,3 +322,10 @@ func getVolumeSource(spec *volume.Spec) (*v1.NFSVolumeSource, bool, error) {
 
 	return nil, false, fmt.Errorf("Spec does not reference a NFS volume type")
 }
+
+func getServerFromSource(source *v1.NFSVolumeSource) string {
+	if netutil.IsIPv6String(source.Server) {
+		return fmt.Sprintf("[%s]", source.Server)
+	}
+	return source.Server
+}
diff --git a/pkg/volume/nfs/nfs_test.go b/pkg/volume/nfs/nfs_test.go
index 9dc9fdd3ff6..c01e2787276 100644
--- a/pkg/volume/nfs/nfs_test.go
+++ b/pkg/volume/nfs/nfs_test.go
@@ -18,7 +18,6 @@ package nfs
 
 import (
 	"fmt"
-	"net"
 	"os"
 	"testing"
 
@@ -119,18 +118,6 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) {
 	if mounter == nil {
 		t.Errorf("Got a nil Mounter")
 	}
-	if nfsm, ok := mounter.(*nfsMounter); ok {
-		srvr := nfsm.server
-		vol, _, _ := getVolumeSource(spec)
-		expectedSrvr := vol.Server
-		// check cluster IP version
-		if net.ParseIP(expectedSrvr).To16() != nil {
-			expectedSrvr = fmt.Sprintf("[%s]", expectedSrvr)
-		}
-		if srvr != expectedSrvr {
-			t.Errorf("Unexpected nfs server, expected %q, got: %q", expectedSrvr, srvr)
-		}
-	}
 	volumePath := mounter.GetPath()
 	expectedPath := fmt.Sprintf("%s/pods/poduid/volumes/kubernetes.io~nfs/vol1", tmpDir)
 	if volumePath != expectedPath {
@@ -149,6 +136,23 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) {
 	if mounter.(*nfsMounter).readOnly {
 		t.Errorf("The volume source should not be read-only and it is.")
 	}
+	mntDevs, err := fake.List()
+	if err != nil {
+		t.Errorf("fakeMounter.List() failed: %v", err)
+	}
+	if len(mntDevs) != 1 {
+		t.Errorf("unexpected number of mounted devices. expected: %v, got %v", 1, len(mntDevs))
+	} else {
+		if mntDevs[0].Type != "nfs" {
+			t.Errorf("unexpected type of mounted devices. expected: %v, got %v", "nfs", mntDevs[0].Type)
+		}
+		src, _, _ := getVolumeSource(spec)
+		srvr := getServerFromSource(src)
+		expectedDevice := fmt.Sprintf("%s:%s", srvr, src.Path)
+		if mntDevs[0].Device != expectedDevice {
+			t.Errorf("unexpected nfs device, expected %q, got: %q", expectedDevice, mntDevs[0].Device)
+		}
+	}
 	log := fake.GetLog()
 	if len(log) != 1 {
 		t.Errorf("Mount was not called exactly one time. It was called %d times.", len(log))