Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions test/extended/storage/csi/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ LUNStressTest:

We strongly recommend to tests with 257 or more Pods and we suggest the test to finish in under 1 hour. There were cases where a CSI driver / RHCOS node configuration had issues with LUN numbers higher than 256. Even when a CSI driver does not use LUNs, it's a nice stress test that checks the CSI driver reports reasonable attach limit and can deal with some load.

The `OpenShift CSI extended - Pod delete after umount` suite is enabled with `podDeleteAfterUmount: true` under `DriverInfo.Capabilities` in the upstream manifest (`TEST_CSI_DRIVER_FILES`).

Example:

```yaml
DriverInfo:
Capabilities:
podDeleteAfterUmount: true
```

## Usage

### With `openshift-tests` binary
Expand Down
2 changes: 1 addition & 1 deletion test/extended/storage/csi/csi.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,6 @@ func AddOpenShiftCSITests(filename string) (string, error) {
// driver manifest. Call before external.AddDriverDefinition. Safe to call once per process.
func RegisterAlwaysOnCSISuites() {
registerAlwaysOnCSISuites.Do(func() {
testsuites.CSISuites = append(testsuites.CSISuites, initPVCCloneLargerCSISuite)
testsuites.CSISuites = append(testsuites.CSISuites, initPVCCloneLargerCSISuite, initPodDeleteAfterUmountCSISuite)
})
}
116 changes: 116 additions & 0 deletions test/extended/storage/csi/pod_delete_after_umount.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package csi

import (
"context"
"fmt"
"path/filepath"

g "github.com/onsi/ginkgo/v2"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
e2e "k8s.io/kubernetes/test/e2e/framework"
e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
e2evolume "k8s.io/kubernetes/test/e2e/framework/volume"
storageframework "k8s.io/kubernetes/test/e2e/storage/framework"
storageutils "k8s.io/kubernetes/test/e2e/storage/utils"
admissionapi "k8s.io/pod-security-admission/api"
)

// CapPodDeleteAfterUmount indicates the driver supports pod deletion after the volume
// was force-unmounted on the node.
const CapPodDeleteAfterUmount storageframework.Capability = "podDeleteAfterUmount"

func initPodDeleteAfterUmountCSISuite() storageframework.TestSuite {
return &podDeleteAfterUmountCSISuite{
tsInfo: storageframework.TestSuiteInfo{
Name: "OpenShift CSI extended - Pod delete after umount",
TestPatterns: []storageframework.TestPattern{
storageframework.DefaultFsDynamicPV,
},
SupportedSizeRange: e2evolume.SizeRange{
Min: "1Mi",
},
},
}
}

// podDeleteAfterUmountCSISuite verifies that a pod can be deleted after its CSI
// volume mount has already been unmounted on the node.
type podDeleteAfterUmountCSISuite struct {
tsInfo storageframework.TestSuiteInfo
}

var _ storageframework.TestSuite = &podDeleteAfterUmountCSISuite{}

func (s *podDeleteAfterUmountCSISuite) GetTestSuiteInfo() storageframework.TestSuiteInfo {
return s.tsInfo
}

func (s *podDeleteAfterUmountCSISuite) SkipUnsupportedTests(driver storageframework.TestDriver, pattern storageframework.TestPattern) {
dInfo := driver.GetDriverInfo()
if !dInfo.Capabilities[CapPodDeleteAfterUmount] {
e2eskipper.Skipf("Driver %q does not support pod delete after umount - skipping", dInfo.Name)
}
}

func (s *podDeleteAfterUmountCSISuite) DefineTests(driver storageframework.TestDriver, pattern storageframework.TestPattern) {
f := e2e.NewFrameworkWithCustomTimeouts("csi-pod-delete-umount", storageframework.GetDriverTimeouts(driver))
f.NamespacePodSecurityLevel = admissionapi.LevelPrivileged

g.It("should delete pod after volume directory was umounted on the node", func(ctx context.Context) {
config := driver.PrepareTest(ctx, f)
hostExec := storageutils.NewHostExec(f)
g.DeferCleanup(hostExec.Cleanup)

g.By("Creating a dynamically provisioned volume")
resource := storageframework.CreateVolumeResource(ctx, driver, config, pattern, s.GetTestSuiteInfo().SupportedSizeRange)
g.DeferCleanup(resource.CleanupResource)

g.By("Creating a pod that mounts the volume")
podConfig := e2epod.Config{
NS: f.Namespace.Name,
PVCs: []*v1.PersistentVolumeClaim{resource.Pvc},
SeLinuxLabel: e2epod.GetLinuxLabel(),
NodeSelection: config.ClientNodeSelection,
ImageID: e2epod.GetDefaultTestImageID(),
}
pod, err := e2epod.CreateSecPodWithNodeSelection(ctx, f.ClientSet, &podConfig, f.Timeouts.PodStart)
e2e.ExpectNoError(err, "creating pod with PVC")
g.DeferCleanup(e2epod.DeletePodWithWait, f.ClientSet, pod)

pvc, err := f.ClientSet.CoreV1().PersistentVolumeClaims(resource.Pvc.Namespace).Get(ctx, resource.Pvc.Name, metav1.GetOptions{})
e2e.ExpectNoError(err, "re-fetching PVC after pod is running")
pvName := pvc.Spec.VolumeName
if pvName == "" {
e2e.Failf("PVC %s has empty Spec.VolumeName after pod is running", pvc.Name)
}

node, err := f.ClientSet.CoreV1().Nodes().Get(ctx, pod.Spec.NodeName, metav1.GetOptions{})
e2e.ExpectNoError(err, "getting pod node %s", pod.Spec.NodeName)

mountPath := csiPodVolumeMountPath(string(pod.UID), pvName)
g.By("Verifying volume is mounted on the pod node")
err = hostExec.IssueCommand(ctx, fmt.Sprintf("mountpoint -q %q", mountPath), node)
e2e.ExpectNoError(err, "expected %s to be a mountpoint before umount", mountPath)

g.By("Unmounting and removing the volume directory on the node")
err = hostExec.IssueCommand(ctx, fmt.Sprintf("umount %q && rmdir %q", mountPath, mountPath), node)
e2e.ExpectNoError(err, "umount and rmdir of volume mount path %s", mountPath)
Comment on lines +97 to +99

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use a forced unmount for this test scenario.

Line 98 uses normal umount. The suite contract requires validation after a force-unmount. This command can test a different cleanup path and leave the intended regression untested. Add -f.

Proposed fix
- err = hostExec.IssueCommand(ctx, fmt.Sprintf("umount %q && rmdir %q", mountPath, mountPath), node)
+ err = hostExec.IssueCommand(ctx, fmt.Sprintf("umount -f %q && rmdir %q", mountPath, mountPath), node)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
g.By("Unmounting and removing the volume directory on the node")
err = hostExec.IssueCommand(ctx, fmt.Sprintf("umount %q && rmdir %q", mountPath, mountPath), node)
e2e.ExpectNoError(err, "umount and rmdir of volume mount path %s", mountPath)
g.By("Unmounting and removing the volume directory on the node")
err = hostExec.IssueCommand(ctx, fmt.Sprintf("umount -f %q && rmdir %q", mountPath, mountPath), node)
e2e.ExpectNoError(err, "umount and rmdir of volume mount path %s", mountPath)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extended/storage/csi/pod_delete_after_umount.go` around lines 97 - 99,
Update the unmount command in the volume cleanup step of the pod deletion test
to use forced unmount (`umount -f`) before removing the directory. Keep the
existing command sequencing and error validation unchanged.


g.By("Verifying the path is no longer a mountpoint")
err = hostExec.IssueCommand(ctx, fmt.Sprintf("mountpoint -q %q", mountPath), node)
if err == nil {
e2e.Failf("expected %s to not be a mountpoint after umount", mountPath)
}

g.By("Deleting the pod; TearDown must succeed despite the missing mount [OCPBUGS-10816]")
err = e2epod.DeletePodWithWait(ctx, f.ClientSet, pod)
e2e.ExpectNoError(err, "deleting pod after volume directory was umounted")
})
}

// csiPodVolumeMountPath returns the kubelet CSI NodePublish mount path for a pod volume.
func csiPodVolumeMountPath(podUID, pvName string) string {
return filepath.Join("/var/lib/kubelet/pods", podUID, "volumes", "kubernetes.io~csi", pvName, "mount")
}