Fix/ub 1722 soft link issue on uncleared node

controller/controller.go

@@ -685,6 +685,28 @@ func getK8sPodsBaseDir(k8sMountPoint string) (string, error ){
 	return tempMountPoint, nil
 }
 
+func checkMountPointIsMounted(mountPoint string, logger logs.Logger, executer utils.Executor) (bool, error){
+	defer logger.Trace(logs.INFO, logs.Args{{"mountPoint", mountPoint}})()
+
+	mountPointStat, err := executer.Stat(mountPoint)
+	if err != nil{
+		return false, logger.ErrorRet(err, "Failed to get stat from k8s slink file.", logs.Args{{"k8sMountPoint", mountPoint}})
+	}
+
+
+	rootDirStat, err := executer.Lstat(filepath.Dir(mountPoint))
+	if err != nil{
+		return false, logger.ErrorRet(err, "Failed to get stat from root directory.", logs.Args{{"directory", mountPoint}})
+	}
+
+	if executer.GetDeviceForFileStat(mountPointStat) != executer.GetDeviceForFileStat(rootDirStat){
+		return true, nil
+	}	 
+
+	return false, nil
+}
+
+
 func checkSlinkAlreadyExistsOnMountPoint(mountPoint string, k8sMountPoint string, logger logs.Logger, executer utils.Executor) (error){
 	/*
 		the mountpoint parameter is the actual mountpoint we are pointing to: /ubiquity/WWN
@@ -729,7 +751,17 @@ func checkSlinkAlreadyExistsOnMountPoint(mountPoint string, k8sMountPoint string
 
 		isSameFile := executer.IsSameFile(fileStat, mountStat)
 		if isSameFile{
-			slinks = append(slinks, file)
+			isInUse, err := checkMountPointIsMounted(mountPoint, logger, executer)
+			if err != nil {
+				logger.Warning("Failed to check whether slink is in use.", logs.Args{{"file", file}})
+				continue
+			}
+			if isInUse{
+				slinks = append(slinks, file)
+			} else {
+				logger.Warning("Found a different Pod that uses the same PVC, but the slink is pointing to a directory that is NOT mounted. It may be a stale POD", logs.Args{{"pod directory", k8sMountPoint}, {"link target directory", mountPoint}})
+			}
+
 		}
 	}
 

controller/controller_internal_test.go

@@ -64,12 +64,23 @@ var _ = Describe("controller_internal_tests", func() {
 		})
 		It("should return an error if this mountpoint already has links", func() {
 			file := "/tmp/file1"
+			fakeExecutor.GetDeviceForFileStatReturnsOnCall(0, 12)
+			fakeExecutor.GetDeviceForFileStatReturnsOnCall(1, 15)
 			fakeExecutor.GetGlobFilesReturns([]string{file}, nil)
 			fakeExecutor.IsSameFileReturns(true)
 			err:= checkSlinkAlreadyExistsOnMountPoint("mountPoint", mountPoint, logs.GetLogger(), fakeExecutor)
 			Expect(err).ToNot(BeNil())
 			Expect(err).To(Equal(&PVIsAlreadyUsedByAnotherPod{"mountPoint", []string{file}}))
 		})
+		It("should not return an error if this mountpoint already has links that are not mounted", func() {
+			file := "/tmp/file1"
+			fakeExecutor.GetDeviceForFileStatReturnsOnCall(0, 12)
+			fakeExecutor.GetDeviceForFileStatReturnsOnCall(1, 12)
+			fakeExecutor.GetGlobFilesReturns([]string{file}, nil)
+			fakeExecutor.IsSameFileReturns(true)
+			err:= checkSlinkAlreadyExistsOnMountPoint("mountPoint", mountPoint, logs.GetLogger(), fakeExecutor)
+			Expect(err).To(BeNil())
+		})
 		It("should return no errors if this mountpoint has only one links and it is the current pvc", func() {
 			file := mountPoint
 			fakeExecutor.GetGlobFilesReturns([]string{file}, nil)
@@ -104,5 +115,43 @@ var _ = Describe("controller_internal_tests", func() {
 			Expect(err).ToNot(HaveOccurred())
 		})
 	})
+	Context(".checkMountPointIsMounted", func() {
+		var (
+			fakeExecutor *fakes.FakeExecutor
+			mountPoint string
+			errstrObj error
+		)
+		BeforeEach(func() {
+			fakeExecutor = new(fakes.FakeExecutor)
+			mountPoint = "/ubiquity/6001738CFC9035E8000000000091E219"
+			errstrObj = fmt.Errorf("An error ooccured")
+		})
+		It("should return error if fails to get stat of mountpoint", func() {
+			fakeExecutor.StatReturnsOnCall(0, nil, errstrObj)
+			_, err := checkMountPointIsMounted(mountPoint, logs.GetLogger(), fakeExecutor)
+			Expect(err).To(HaveOccurred())
+			Expect(err).To(Equal(errstrObj))
+		})
+		It("should return error if fails to get lstat of mountpoint/..", func() {
+			fakeExecutor.LstatReturnsOnCall(0, nil, errstrObj)
+			_, err := checkMountPointIsMounted(mountPoint, logs.GetLogger(), fakeExecutor)
+			Expect(err).To(HaveOccurred())
+			Expect(err).To(Equal(errstrObj))
+		})
+		It("should return true if directory is mounted", func() {
+			fakeExecutor.GetDeviceForFileStatReturnsOnCall(0, 12)
+			fakeExecutor.GetDeviceForFileStatReturnsOnCall(1, 15)
+			res, err := checkMountPointIsMounted(mountPoint, logs.GetLogger(), fakeExecutor)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(res).To(BeTrue())
+		})
+		It("should return false if directory is not mounted", func() {
+			fakeExecutor.GetDeviceForFileStatReturnsOnCall(0, 12)
+			fakeExecutor.GetDeviceForFileStatReturnsOnCall(1, 12)
+			res, err := checkMountPointIsMounted(mountPoint, logs.GetLogger(), fakeExecutor)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(res).To(BeFalse())
+		})
+	})
 })
 

glide.yaml

@@ -110,7 +110,7 @@ import:
   subpackages:
   - lib
 - package: github.com/IBM/ubiquity
-  version: 95e25e89f30d43ef4cf7c287fe4efc4370563f56
+  version: e698dedd85e5e3cc7f59f3ce1a1320c9b471ea21
   subpackages:
   - remote
   - resources