From f6bf9272eb81a803ceaf6eb5263052bbb2df7115 Mon Sep 17 00:00:00 2001 From: radeore Date: Wed, 5 Aug 2026 16:51:57 -0400 Subject: [PATCH] STOR-3090: Add CSI storage test for pod delete after host umount of mounted volume --- pkg/clioptions/clusterdiscovery/csi.go | 5 +- test/extended/storage/csi/README.md | 4 + test/extended/storage/csi/csi.go | 95 +++++++++++++- .../storage/csi/pod_delete_after_umount.go | 116 ++++++++++++++++++ 4 files changed, 214 insertions(+), 6 deletions(-) create mode 100644 test/extended/storage/csi/pod_delete_after_umount.go diff --git a/pkg/clioptions/clusterdiscovery/csi.go b/pkg/clioptions/clusterdiscovery/csi.go index 315f37c6e43e..b993b0f63b1a 100644 --- a/pkg/clioptions/clusterdiscovery/csi.go +++ b/pkg/clioptions/clusterdiscovery/csi.go @@ -8,7 +8,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/test/e2e/framework/testfiles" - "k8s.io/kubernetes/test/e2e/storage/external" "sigs.k8s.io/yaml" "github.com/openshift/origin/test/extended/storage/csi" @@ -33,7 +32,7 @@ func InitCSITests() error { } // Load OCP specific tests first, because AddOpenShiftCSITests() modifies global list of - // testsuites.CSISuites used by AddDriverDefinition() below. + // testsuites.CSISuites and OCP capabilities used by AddDriverDefinition() below. ocpManifestList := os.Getenv(OCPManifestEnvVar) if ocpManifestList != "" { manifests := strings.Split(ocpManifestList, ",") @@ -50,7 +49,7 @@ func InitCSITests() error { if upstreamManifestList != "" { manifests := strings.Split(upstreamManifestList, ",") for _, manifest := range manifests { - if err := external.AddDriverDefinition(manifest); err != nil { + if err := csi.AddDriverDefinition(manifest); err != nil { return fmt.Errorf("failed to load manifest from %q: %s", manifest, err) } csiDriver, err := parseDriverName(manifest) diff --git a/test/extended/storage/csi/README.md b/test/extended/storage/csi/README.md index 6d73169b883d..9af043a8771e 100644 --- a/test/extended/storage/csi/README.md +++ b/test/extended/storage/csi/README.md @@ -19,11 +19,15 @@ Example: ```yaml Driver: +Capabilities: + podDeleteAfterUmount: true LUNStressTest: PodsTotal: 260 Timeout: "40m" ``` +`Capabilities` lists OpenShift-only driver features from the OCP manifest. They are merged into upstream `DriverInfo.Capabilities` at load time. `podDeleteAfterUmount` enables the suite that host-unmounts a CSI volume path and verifies pod deletion still succeeds. + `LUNStressTest` is a test that stresses the CSI driver on a single node. The test picks a random scheudlable node and creates configured number of Pods + PVCs on it (260 by default). diff --git a/test/extended/storage/csi/csi.go b/test/extended/storage/csi/csi.go index 203ce1596074..7540052e4f30 100644 --- a/test/extended/storage/csi/csi.go +++ b/test/extended/storage/csi/csi.go @@ -3,16 +3,24 @@ package csi import ( "fmt" "os" + "path/filepath" "sync" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/kubernetes/test/e2e/storage/external" + storageframework "k8s.io/kubernetes/test/e2e/storage/framework" "k8s.io/kubernetes/test/e2e/storage/testsuites" + "sigs.k8s.io/yaml" ) var registerAlwaysOnCSISuites sync.Once +var ocpDriverCapabilities = map[string]map[storageframework.Capability]bool{} + +var noopCleanup = func() {} + const ( // The defaul timeout for the LUN stress test. DefaultLUNStressTestTimeout = "40m" @@ -26,6 +34,8 @@ type OpenShiftCSIDriverConfig struct { Driver string // Configuration of the LUN stress test. If nil, the test is skipped. LUNStressTest *LUNStressTestConfig + // OpenShift-only driver capabilities merged into upstream DriverInfo at load time. + Capabilities map[storageframework.Capability]bool } // Definition of the LUN stress test parameters. @@ -76,18 +86,97 @@ func AddOpenShiftCSITests(filename string) (string, error) { return "", fmt.Errorf("%s: %w", filename, err) } + if len(cfg.Capabilities) > 0 { + ocpDriverCapabilities[cfg.Driver] = cfg.Capabilities + } + // Register this OCP specific test suite in the upstream test framework. // In the end, the test suite will be executed as any other upstream storage test. - // Note: this must be done before external.AddDriverDefinition which actually goes through + // Note: this must be done before AddDriverDefinition which actually goes through // the registered testsuites and generates ginkgo tests for them. testsuites.CSISuites = append(testsuites.CSISuites, initSCSILUNOverflowCSISuite(cfg.LUNStressTest)) return cfg.Driver, nil } +// AddDriverDefinition loads an upstream CSI manifest, merges OCP capabilities from the +// matching OCP manifest, and registers ginkgo tests for the driver. +func AddDriverDefinition(filename string) error { + mergedFilename, cleanup, err := mergeOCPCapsIntoUpstreamManifest(filename) + if err != nil { + return err + } + defer cleanup() + return external.AddDriverDefinition(mergedFilename) +} + +func mergeOCPCapsIntoUpstreamManifest(filename string) (string, func(), error) { + if len(ocpDriverCapabilities) == 0 { + return filename, noopCleanup, nil + } + + data, err := os.ReadFile(filename) + if err != nil { + return "", nil, err + } + + var manifest map[string]interface{} + if err := yaml.Unmarshal(data, &manifest); err != nil { + return "", nil, fmt.Errorf("%s: %w", filename, err) + } + + driverInfo, ok := manifest["DriverInfo"].(map[string]interface{}) + if !ok { + return filename, noopCleanup, nil + } + driverName, _ := driverInfo["Name"].(string) + ocpCaps := ocpDriverCapabilities[driverName] + if driverName == "" || len(ocpCaps) == 0 { + return filename, noopCleanup, nil + } + + caps, _ := driverInfo["Capabilities"].(map[string]interface{}) + if caps == nil { + caps = map[string]interface{}{} + driverInfo["Capabilities"] = caps + } + for cap, v := range ocpCaps { + caps[string(cap)] = v + } + + mergedData, err := yaml.Marshal(manifest) + if err != nil { + return "", nil, fmt.Errorf("%s: %w", filename, err) + } + + mergedFilename, err := writeTempManifest(filepath.Dir(filename), mergedData) + if err != nil { + return "", nil, fmt.Errorf("%s: %w", filename, err) + } + return mergedFilename, func() { os.Remove(mergedFilename) }, nil +} + +func writeTempManifest(dir string, data []byte) (string, error) { + tmp, err := os.CreateTemp(dir, ".ocp-merge-*.yaml") + if err != nil { + return "", err + } + name := tmp.Name() + if _, err := tmp.Write(data); err != nil { + tmp.Close() + os.Remove(name) + return "", err + } + if err := tmp.Close(); err != nil { + os.Remove(name) + return "", err + } + return name, nil +} + // RegisterAlwaysOnCSISuites appends OpenShift CSI suites that do not need an OCP-specific -// driver manifest. Call before external.AddDriverDefinition. Safe to call once per process. +// driver manifest. Call before 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) }) } diff --git a/test/extended/storage/csi/pod_delete_after_umount.go b/test/extended/storage/csi/pod_delete_after_umount.go new file mode 100644 index 000000000000..f6a2ad7e7733 --- /dev/null +++ b/test/extended/storage/csi/pod_delete_after_umount.go @@ -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) + + 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") +}