From 44be42c196bcf1ebfeb224b8c9db636ad3aa4160 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Mon, 17 Sep 2018 16:37:23 -0700 Subject: [PATCH 1/3] Include PV wait confirmation for Disruptive PV tests --- test/e2e/framework/pv_util.go | 3 +++ test/e2e/storage/generic_persistent_volume-disruptive.go | 9 +++++---- test/e2e/storage/nfs_persistent_volume-disruptive.go | 9 ++++++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/test/e2e/framework/pv_util.go b/test/e2e/framework/pv_util.go index cedb857f213..be9a157d933 100644 --- a/test/e2e/framework/pv_util.go +++ b/test/e2e/framework/pv_util.go @@ -507,6 +507,9 @@ func testPodSuccessOrFail(c clientset.Interface, ns string, pod *v1.Pod) error { // Deletes the passed-in pod and waits for the pod to be terminated. Resilient to the pod // not existing. func DeletePodWithWait(f *Framework, c clientset.Interface, pod *v1.Pod) error { + if pod == nil { + return nil + } return DeletePodWithWaitByName(f, c, pod.GetName(), pod.GetNamespace()) } diff --git a/test/e2e/storage/generic_persistent_volume-disruptive.go b/test/e2e/storage/generic_persistent_volume-disruptive.go index 92b04b2af96..fdba7e78433 100644 --- a/test/e2e/storage/generic_persistent_volume-disruptive.go +++ b/test/e2e/storage/generic_persistent_volume-disruptive.go @@ -61,10 +61,11 @@ var _ = utils.SIGDescribe("GenericPersistentVolume[Disruptive]", func() { var ( clientPod *v1.Pod pvc *v1.PersistentVolumeClaim + pv *v1.PersistentVolume ) BeforeEach(func() { framework.Logf("Initializing pod and pvcs for test") - clientPod, pvc = createPodPVCFromSC(f, c, ns) + clientPod, pvc, pv = createPodPVCFromSC(f, c, ns) }) for _, test := range disruptiveTestTable { func(t disruptiveTest) { @@ -76,13 +77,13 @@ var _ = utils.SIGDescribe("GenericPersistentVolume[Disruptive]", func() { } AfterEach(func() { framework.Logf("Tearing down test spec") - tearDownTestCase(c, f, ns, clientPod, pvc, nil) + tearDownTestCase(c, f, ns, clientPod, pvc, pv, false) pvc, clientPod = nil, nil }) }) }) -func createPodPVCFromSC(f *framework.Framework, c clientset.Interface, ns string) (*v1.Pod, *v1.PersistentVolumeClaim) { +func createPodPVCFromSC(f *framework.Framework, c clientset.Interface, ns string) (*v1.Pod, *v1.PersistentVolumeClaim, *v1.PersistentVolume) { var err error test := storageClassTest{ name: "default", @@ -99,5 +100,5 @@ func createPodPVCFromSC(f *framework.Framework, c clientset.Interface, ns string By("Creating a pod with dynamically provisioned volume") pod, err := framework.CreateNginxPod(c, ns, nil, pvcClaims) Expect(err).NotTo(HaveOccurred(), "While creating pods for kubelet restart test") - return pod, pvc + return pod, pvc, pvs[0] } diff --git a/test/e2e/storage/nfs_persistent_volume-disruptive.go b/test/e2e/storage/nfs_persistent_volume-disruptive.go index 1ea9fe192bf..6482050a869 100644 --- a/test/e2e/storage/nfs_persistent_volume-disruptive.go +++ b/test/e2e/storage/nfs_persistent_volume-disruptive.go @@ -208,7 +208,7 @@ var _ = utils.SIGDescribe("NFSPersistentVolumes[Disruptive][Flaky]", func() { AfterEach(func() { framework.Logf("Tearing down test spec") - tearDownTestCase(c, f, ns, clientPod, pvc, pv) + tearDownTestCase(c, f, ns, clientPod, pvc, pv, true /* force PV delete */) pv, pvc, clientPod = nil, nil, nil }) @@ -277,11 +277,14 @@ func initTestCase(f *framework.Framework, c clientset.Interface, pvConfig framew } // tearDownTestCase destroy resources created by initTestCase. -func tearDownTestCase(c clientset.Interface, f *framework.Framework, ns string, client *v1.Pod, pvc *v1.PersistentVolumeClaim, pv *v1.PersistentVolume) { +func tearDownTestCase(c clientset.Interface, f *framework.Framework, ns string, client *v1.Pod, pvc *v1.PersistentVolumeClaim, pv *v1.PersistentVolume, forceDeletePV bool) { // Ignore deletion errors. Failing on them will interrupt test cleanup. framework.DeletePodWithWait(f, c, client) framework.DeletePersistentVolumeClaim(c, pvc.Name, ns) - if pv != nil { + if forceDeletePV { framework.DeletePersistentVolume(c, pv.Name) + return } + err := framework.WaitForPersistentVolumeDeleted(c, pv.Name, 5*time.Second, 5*time.Minute) + framework.ExpectNoError(err, "Persistent Volume %v not deleted by dynamic provisioner", pv.Name) } From 9d207b3e3c11cb9fd1eaf2ba73d3019c4a3c5939 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Mon, 17 Sep 2018 16:57:05 -0700 Subject: [PATCH 2/3] GetMountRefs should not fail if the path supplied does not exist anymore. It has no mount references --- pkg/util/mount/mount_linux.go | 3 +++ pkg/volume/util/operationexecutor/operation_generator.go | 2 ++ test/e2e/storage/nfs_persistent_volume-disruptive.go | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index 6b1c0010442..180de0ffed9 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -1005,6 +1005,9 @@ func (mounter *Mounter) SafeMakeDir(subdir string, base string, perm os.FileMode } func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { + if _, err := os.Stat(pathname); os.IsNotExist(err) { + return []string{}, nil + } realpath, err := filepath.EvalSymlinks(pathname) if err != nil { return nil, err diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 57c7adfa7cb..154e3c8a2df 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -1433,6 +1433,8 @@ func isDeviceOpened(deviceToDetach AttachedVolume, mounter mount.Interface) (boo //TODO: refer to #36092 glog.V(3).Infof("The path isn't device path or doesn't exist. Skip checking device path: %s", deviceToDetach.DevicePath) deviceOpened = false + } else if devicePathErr != nil { + return false, deviceToDetach.GenerateErrorDetailed("PathIsDevice failed", devicePathErr) } else { deviceOpened, deviceOpenedErr = mounter.DeviceOpened(deviceToDetach.DevicePath) if deviceOpenedErr != nil { diff --git a/test/e2e/storage/nfs_persistent_volume-disruptive.go b/test/e2e/storage/nfs_persistent_volume-disruptive.go index 6482050a869..f081809dab8 100644 --- a/test/e2e/storage/nfs_persistent_volume-disruptive.go +++ b/test/e2e/storage/nfs_persistent_volume-disruptive.go @@ -281,7 +281,7 @@ func tearDownTestCase(c clientset.Interface, f *framework.Framework, ns string, // Ignore deletion errors. Failing on them will interrupt test cleanup. framework.DeletePodWithWait(f, c, client) framework.DeletePersistentVolumeClaim(c, pvc.Name, ns) - if forceDeletePV { + if forceDeletePV && pv != nil { framework.DeletePersistentVolume(c, pv.Name) return } From 704573d304096c4c0e769a3e7ffdd4d23737b99f Mon Sep 17 00:00:00 2001 From: David Zhu Date: Mon, 17 Sep 2018 17:50:27 -0700 Subject: [PATCH 3/3] GetMountRefs shouldn't error when file doesn'g exist in Windows and nsenter. Add unit test --- pkg/util/mount/mount_linux.go | 2 ++ pkg/util/mount/mount_linux_test.go | 4 ++++ pkg/util/mount/mount_windows.go | 5 +++++ pkg/util/mount/nsenter_mount.go | 7 +++++++ 4 files changed, 18 insertions(+) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index 180de0ffed9..1df073218f9 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -1007,6 +1007,8 @@ func (mounter *Mounter) SafeMakeDir(subdir string, base string, perm os.FileMode func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { if _, err := os.Stat(pathname); os.IsNotExist(err) { return []string{}, nil + } else if err != nil { + return nil, err } realpath, err := filepath.EvalSymlinks(pathname) if err != nil { diff --git a/pkg/util/mount/mount_linux_test.go b/pkg/util/mount/mount_linux_test.go index 530899d5ab0..417592602da 100644 --- a/pkg/util/mount/mount_linux_test.go +++ b/pkg/util/mount/mount_linux_test.go @@ -110,6 +110,10 @@ func TestGetMountRefs(t *testing.T) { "/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd2", }, }, + { + "/var/fake/directory/that/doesnt/exist", + []string{}, + }, } for i, test := range tests { diff --git a/pkg/util/mount/mount_windows.go b/pkg/util/mount/mount_windows.go index b3206d18c1c..048e442c023 100644 --- a/pkg/util/mount/mount_windows.go +++ b/pkg/util/mount/mount_windows.go @@ -459,6 +459,11 @@ func getAllParentLinks(path string) ([]string, error) { } func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { + if _, err := os.Stat(normalizeWindowsPath(pathname)); os.IsNotExist(err) { + return []string{}, nil + } else if err != nil { + return nil, err + } refs, err := getAllParentLinks(normalizeWindowsPath(pathname)) if err != nil { return nil, err diff --git a/pkg/util/mount/nsenter_mount.go b/pkg/util/mount/nsenter_mount.go index a798defe9bf..d627a8f1fe4 100644 --- a/pkg/util/mount/nsenter_mount.go +++ b/pkg/util/mount/nsenter_mount.go @@ -337,6 +337,13 @@ func (mounter *NsenterMounter) SafeMakeDir(subdir string, base string, perm os.F } func (mounter *NsenterMounter) GetMountRefs(pathname string) ([]string, error) { + exists, err := mounter.ExistsPath(pathname) + if err != nil { + return nil, err + } + if !exists { + return []string{}, nil + } hostpath, err := mounter.ne.EvalSymlinks(pathname, true /* mustExist */) if err != nil { return nil, err