diff --git a/controller/controller.go b/controller/controller.go index b6e09b54e..24b6eb92b 100644 --- a/controller/controller.go +++ b/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}}) + } + } } diff --git a/controller/controller_internal_test.go b/controller/controller_internal_test.go index 1676f5be5..5c3cbf909 100644 --- a/controller/controller_internal_test.go +++ b/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()) + }) + }) }) diff --git a/glide.yaml b/glide.yaml index a7192c31d..4b605803c 100644 --- a/glide.yaml +++ b/glide.yaml @@ -110,7 +110,7 @@ import: subpackages: - lib - package: github.com/IBM/ubiquity - version: 95e25e89f30d43ef4cf7c287fe4efc4370563f56 + version: e698dedd85e5e3cc7f59f3ce1a1320c9b471ea21 subpackages: - remote - resources