Skip to content

WIP: UPSTREAM: 141218: test: create snapshot metadata resources in PrepareTest when CapSnapshotMetadata is enabled #2739

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RomanBednar wants to merge 2 commits into
base: master
Choose a base branch
from
+21 −13
Open

WIP: UPSTREAM: 141218: test: create snapshot metadata resources in PrepareTest when CapSnapshotMetadata is enabled #2739

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0caf91a
e2e: move snapshot metadata resource handling to PrepareTest
RomanBednar Aug 10, 2026
1061046
DO NOT MERGE: pre-merge testing CSI_PROW_ENABLE_SNAPSHOT_METADATA=true
RomanBednar Aug 13, 2026
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
21 changes: 21 additions & 0 deletions test/e2e/storage/drivers/csi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ func InitHostPathCSIDriver() storageframework.TestDriver {
// added when patching the deployment.
storageframework.CapVolumeLimits: true,
}
// DO NOT MERGE: pre-merge testing functionality when this env var is true in openshift/release
err := os.Setenv("CSI_PROW_ENABLE_SNAPSHOT_METADATA", "true")
if err != nil {
framework.Failf("failed to set CSI_PROW_ENABLE_SNAPSHOT_METADATA: %v", err)
}
// TODO: It can be removed after the VolumeGroupSnapshot feature is default enabled
if os.Getenv("CSI_PROW_ENABLE_GROUP_SNAPSHOT") == "true" {
capabilities[storageframework.CapVolumeGroupSnapshot] = true
Expand Down Expand Up @@ -376,6 +381,22 @@ func (h *hostpathCSIDriver) PrepareTest(ctx context.Context, f *framework.Framew
framework.Failf("deploying %s driver: %v", h.driverInfo.Name, err)
}

if h.driverInfo.Capabilities[storageframework.CapSnapshotMetadata] {
// Create snapshot metadata resources (CRD is already created by test runner script)
ginkgo.By("Creating snapshot metadata resources")
err = utils.CreateSnapshotMetadataResources(ctx, f, config.Driver.GetDriverInfo().Name, driverns)
if err != nil {
Comment on lines +387 to +388

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

if rg -n '\bCreateSnapshotMetadataResources\s*\(' \
  test/e2e/storage/testsuites/snapshot-metadata.go; then
  echo "duplicate snapshot metadata resource creation remains"
  exit 1
fi

rg -n -C 3 '\bCreateSnapshotMetadataResources\s*\(' \
  test/e2e/storage/drivers/csi.go \
  test/e2e/storage/utils/snapshot-metadata.go

Repository: openshift/kubernetes

Length of output: 342

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- snapshot metadata suite ---'
sed -n '270,335p' test/e2e/storage/testsuites/snapshot-metadata.go

printf '%s\n' '--- CSI driver setup and cleanup ---'
sed -n '350,410p' test/e2e/storage/drivers/csi.go

printf '%s\n' '--- resource helper ---'
sed -n '1,220p' test/e2e/storage/utils/snapshot-metadata.go

printf '%s\n' '--- all relevant call sites and cleanup references ---'
rg -n -C 4 '\b(CreateSnapshotMetadataResources|DeleteSnapshotMetadataResources|snapshotMetadataTestSuite\.init|PrepareTest)\b' \
  test/e2e/storage

Repository: openshift/kubernetes

Length of output: 50376

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- helper implementation ---'
sed -n '219,305p' test/e2e/storage/utils/snapshot-metadata.go

printf '%s\n' '--- PrepareTest declaration and resource setup ---'
sed -n '240,270p' test/e2e/storage/drivers/csi.go
sed -n '370,402p' test/e2e/storage/drivers/csi.go

printf '%s\n' '--- exact call-site counts ---'
rg -n '\bCreateSnapshotMetadataResources\s*\(' \
  test/e2e/storage/drivers/csi.go \
  test/e2e/storage/testsuites/snapshot-metadata.go \
  test/e2e/storage/utils/snapshot-metadata.go

printf '%s\n' '--- cleanup function declarations and calls ---'
rg -n -C 5 '\bCleanupSnapshotMetadataResources\b' \
  test/e2e/storage/utils/snapshot-metadata.go \
  test/e2e/storage/drivers/csi.go

Repository: openshift/kubernetes

Length of output: 7211

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

suite = Path("test/e2e/storage/testsuites/snapshot-metadata.go").read_text()
driver = Path("test/e2e/storage/drivers/csi.go").read_text()
helper = Path("test/e2e/storage/utils/snapshot-metadata.go").read_text()

suite_calls = re.findall(r"\b(?:storageutils\.)?CreateSnapshotMetadataResources\s*\(", suite)
driver_calls = re.findall(r"\butils\.CreateSnapshotMetadataResources\s*\(", driver)
helper_creates = [
    name for name in ("createTLSSecret", "createSnapshotMetadataSVC", "createSnapshotMetdataServiceCR")
    if re.search(r"\b" + name + r"\s*\(", helper)
]

cleanup_block = re.search(
    r"ginkgo\.DeferCleanup\(func\(ctx context\.Context\) \{(?P<body>.*?)\n\t\t\}\)",
    driver,
    re.S,
)
body = cleanup_block.group("body") if cleanup_block else ""
print({
    "suite_create_call_count": len(suite_calls),
    "prepare_test_create_call_count": len(driver_calls),
    "resources_created_by_helper": helper_creates,
    "cleanup_result_assigned": bool(re.search(
        r"\b(?:cleanupErr|err)\s*=\s*utils\.CleanupSnapshotMetadataResources\s*\(",
        body,
    )),
    "cleanup_condition_uses_setup_err": "if err != nil" in body,
})
PY

Repository: openshift/kubernetes

Length of output: 417

Keep snapshot metadata resource creation in one owner.

PrepareTest creates the TLS Secret, Service, and SnapshotMetadataService. Remove the duplicate call from snapshotMetadataTestSuite.init.

Make CleanupSnapshotMetadataResources return deletion errors. Capture and handle that result in the deferred cleanup callback instead of checking the earlier setup error.

🤖 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/e2e/storage/drivers/csi.go` around lines 382 - 383, Remove the duplicate
utils.CreateSnapshotMetadataResources call from snapshotMetadataTestSuite.init
because PrepareTest already owns creation of the TLS Secret, Service, and
SnapshotMetadataService. Update CleanupSnapshotMetadataResources to return
deletion errors, then have the deferred cleanup callback capture and handle that
returned error rather than checking the earlier setup error.

framework.Failf("failed to create snapshot metadata resources: %v", err)
}
ginkgo.DeferCleanup(func(ctx context.Context) {
ginkgo.By("Cleaning up snapshot metadata resources")
err = utils.CleanupSnapshotMetadataResources(ctx, f, config.Driver.GetDriverInfo().Name, driverns)
if err != nil {
framework.Logf("Warning: failed to cleanup snapshot metadata resources: %v", err)
}
})
}

cleanupFunc := generateDriverCleanupFunc(
f,
h.driverInfo.Name,
Expand Down
13 changes: 0 additions & 13 deletions test/e2e/storage/testsuites/snapshot-metadata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,6 @@ func (s *snapshotMetadataTestSuite) DefineTests(driver storageframework.TestDriv

config = smDriver.PrepareTest(ctx, f)

// Create snapshot metadata resources (CRD is already created by test runner script)
ginkgo.By("Creating snapshot metadata resources")
err = storageutils.CreateSnapshotMetadataResources(ctx, f, config.Driver.GetDriverInfo().Name, config.DriverNamespace.Name)
framework.ExpectNoError(err, "Failed to create snapshot metadata resources")

pattern.VolMode = v1.PersistentVolumeBlock
volume = storageframework.CreateVolumeResource(ctx, smDriver, config, pattern, s.GetTestSuiteInfo().SupportedSizeRange)
testPVC = volume.Pvc
Expand Down Expand Up @@ -359,14 +354,6 @@ func (s *snapshotMetadataTestSuite) DefineTests(driver storageframework.TestDriv
backupClientPod = nil
}

// Cleanup snapshot metadata resources
if config != nil {
ginkgo.By("Cleaning up snapshot metadata resources")
err := storageutils.CleanupSnapshotMetadataResources(ctx, f, config.Driver.GetDriverInfo().Name, config.DriverNamespace.Name)
if err != nil {
framework.Logf("Warning: failed to cleanup snapshot metadata resources: %v", err)
}
}
})

ginkgo.It("should verify GetMetadataDelta", func(ctx context.Context) {
Expand Down